> On 8 May 2025, at 8:03, Markus Armbruster <arm...@redhat.com> wrote:
>
> Markus Armbruster <arm...@redhat.com> writes:
>
>> 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?
>>
>> 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.
>
> false is a security hazard, as secure-coding-practices.rst points out.
> I think it should be pointed out right here as well. Especially since
> "security hazard" is the default!
>
> I'd do it in a separate patch, but I'm a compulsive patch splitter.
>
I agree, we need to warn about this unexpected behavior.
