Re: [PATCH] qemu-{img,nbd}: Don't report zeroed cluster as a hole

[Eric Blake]

On Tue, Jun 08, 2021 at 07:38:10PM +0300, Nir Soffer wrote:
> The example I provided was not detailed enough, what we actually do is:
> 
>     qemu-nbd .. 'json:{"driver": "qcow2", "backing": null, "file":
> {"driver": "file", "filename": "top.qcow2"}}'
> 
> So there is no backing chain and allocation depth is not relevant.
> - Allocated areas should be reported with flags 0
> - Zero areas which are not holes should be reported as NBD_STATE_ZERO
> - Zero areas which are holes (not allocated in this image) should be
> reported as NBD_STATE_HOLE

Thinking about this a bit more, here's something I noticed:

$ qemu-img map --output=json -f raw base.raw
[{ "start": 0, "length": 196608, "depth": 0, "zero": false, "data": true, 
"offset": 0},
{ "start": 196608, "length": 65536, "depth": 0, "zero": true, "data": false, 
"offset": 196608}]

which matches what I've said elsewhere in this thread: the entire
image is reported as "depth":0 because the raw file is responsible for
100% of the content.

But:

$ qemu-img map --output=json -f qcow2 json:'{"driver":"qcow2","backing":null, \
  "file":{"driver":"file","filename":"top.qcow2"}}'
[{ "start": 0, "length": 65536, "depth": 0, "zero": true, "data": false},
{ "start": 65536, "length": 65536, "depth": 0, "zero": false, "data": true, 
"offset": 327680},
{ "start": 131072, "length": 131072, "depth": 0, "zero": true, "data": false}]

also reports the entire file at "depth":0, which is misleading, since
we have just been arguing from the qemu:allocation-depth perspective
(and also from bdrv_block_status) that the qcow2 image is NOT 100%
allocated (in the sense where allocation == data comes locally).
Perhaps it might be better if we tweaked the above qemu-img map to
produce:

[{ "start": 0, "length": 65536, "depth": -1, "zero": true, "data": false},
{ "start": 65536, "length": 65536, "depth": 0, "zero": false, "data": true, 
"offset": 327680},
{ "start": 131072, "length": 65536, "depth": 0, "zero": true, "data": false},
{ "start": 196608, "length": 65536, "depth": -1, "zero": true, "data": false}]

that is, use "depth":-1 to explicitly denote portions of a qcow2 file
which are NOT provided locally, and which are not found anywhere in
the backing chain.  In other words, make it explicit in qemu-img map
output what is possible with qemu:allocation-depth==0.

Or tweak it slightly to mean that "depth":-1 corresponds to "cluster
is not provided by the current layer, but we could not determine if it
is provided by a particular backing layer or if it was unallocated
overall".  Then positive depth means we know which point in the
backing chain we deferred to, 0 is local, and negative depth means
that we defer to a backing layer (but could not report WHICH layer, if
any).  This tweak would make it easier for my thoughts of having qemu
NBD clients automatically request qemu:allocation-depth without having
to resort to x-dirty-bitmap hacks, and still be able to expose the
information via qemu-img map.

I'm off to another round of code hacking to see how it looks...

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org