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.