> On 8 May 2025, at 7:37, Markus Armbruster <arm...@redhat.com> wrote:
> 
> Nir Soffer <nir...@gmail.com> writes:
> 
>> If read-zeroes is not set, we did not report BDRV_BLOCK_DATA or
>> BDRV_BLOCK_ZERO. This is not consistent with other drivers and can
>> confuse users or other programs:
>> 
>>    % qemu-img map --output json "json:{'driver': 'raw', 'file': {'driver': 
>> 'null-co', 'size': '1g'}}"
>>    [{ "start": 0, "length": 1073741824, "depth": 0, "present": false, 
>> "zero": false, "data": false, "compressed": false}]
>> 
>>    % qemu-nbd "json:{'driver': 'raw', 'file': {'driver': 'null-co', 'size': 
>> '1g'}}" &
>> 
>>    % nbdinfo --map nbd://127.0.0.1
>>             0  1073741824    1  hole
>> 
>> With this change we report DATA in this case:
>> 
>>    % ./qemu-img map --output json "json:{'driver': 'raw', 'file': {'driver': 
>> 'null-co', 'size': '1g'}}"
>>    [{ "start": 0, "length": 1073741824, "depth": 0, "present": true, "zero": 
>> false, "data": true, "compressed": false, "offset": 0}]
>> 
>>    % ./qemu-nbd "json:{'driver': 'raw', 'file': {'driver': 'null-co', 
>> 'size': '1g'}}" &
>> 
>>    % nbdinfo --map nbd://127.0.0.1
>>             0  1073741824    0  data
>> 
>> Signed-off-by: Nir Soffer <nir...@gmail.com>
>> ---
>> block/null.c         | 4 +---
>> qapi/block-core.json | 5 +++--
>> 2 files changed, 4 insertions(+), 5 deletions(-)
>> 
>> diff --git a/block/null.c b/block/null.c
>> index dc0b1fdbd9..7ba87bd9a9 100644
>> --- a/block/null.c
>> +++ b/block/null.c
>> @@ -239,9 +239,7 @@ static int coroutine_fn 
>> null_co_block_status(BlockDriverState *bs,
>>     *map = offset;
>>     *file = bs;
>> 
>> -    if (s->read_zeroes) {
>> -        ret |= BDRV_BLOCK_ZERO;
>> -    }
>> +    ret |= s->read_zeroes ? BDRV_BLOCK_ZERO : BDRV_BLOCK_DATA;
>>     return ret;
>> }
>> 
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index b1937780e1..7c95c9e36a 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -3293,8 +3293,9 @@
>> #     requests.  Default to zero which completes requests immediately.
>> #     (Since 2.4)
>> #
>> -# @read-zeroes: if true, reads from the device produce zeroes; if
>> -#     false, the buffer is left unchanged.
>> +# @read-zeroes: if true, emulate a sparse image, and reads from the
>> +#     device produce zeroes; if false, emulate an allocated image but
>> +#     reads from the device leave the buffer unchanged.
>> #     (default: false; since: 4.1)
>> #
>> # Since: 2.9
> 
> Possibly dumb question: how is this doc change related to the code fix?

Before this change we returned BDRV_BLOCK_ZERO if read-zeroes is true, and no 
flags when it was false.
Now we return BDRV_BLOCK_ZERO when read-zeroes is true, and BDRV_BLOCK_DATA 
when false.

In other words, we read-zeroes: true emulate a spare image, and read-zeros: 
false an allocated image.

> 
> Suggest to split the sentence for easier reading:
> 
>   # @read-zeroes: If true, emulate a sparse image, and reads from the
>   #     device produce zeroes.  If false, emulate an allocated image,
>   #     but reads from the device leave the buffer unchanged.
> 

Sure, it looks better.


Reply via email to