Re: [PATCH v2] qemu-img: Make unallocated part of backing chain obvious in map

[Eric Blake]

On Fri, Jun 11, 2021 at 05:35:12PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> > An obvious solution is to make 'qemu-img map --output=json'
> > distinguish between clusters that have a local allocation from those
> > that are found nowhere in the chain.  We already have a one-off
> > mismatch between qemu-img map and NBD qemu:allocation-depth (the
> > former chose 0, and the latter 1 for the local layer), so exposing the
> > latter's choice of 0 for unallocated in the entire chain would mean
> > using using "depth":-1 in the former, but a negative depth may confuse
> > existing tools.  But there is an easy out: for any chain of length N,
> > we can simply represent an unallocated cluster as "depth":N+1.  This
> > does have a slight risk of confusing any tool that might try to
> > dereference NULL when finding the backing image for the last file in
> > the backing chain, but that risk sseems worth the more precise output.
> > The iotests have several examples where this distinction demonstrates
> > the additional accuracy.
> > 
> > Signed-off-by: Eric Blake <eblake@redhat.com>
> > ---
> > 
> > Replaces v1: 20210610213906.1313440-1-eblake@redhat.com
> > (qemu-img: Use "depth":-1 to make backing probes obvious)
> > 
> > Use N+1 instead of -1 for unallocated [Kevin]
> > 
> 
> Bit in contrast with -1, or with separate boolean flag, you lose the 
> possibility to distinguish case when we have 3 layers and the cluster is 
> absent in all of them, and the case when we have 4 layers and the cluster is 
> absent in top 3 but in 4 it is qcow2 UNALLOCATED_ZERO cluster.

Using just 'qemu-img map --output-json', you only see depth numbers.
You also have to use 'qemu-img info --backing-chain' to see what file
those depth numbers correspond to, at which point it becomes obvious
whether "depth":4 meant unallocated (because the chain was length 3)
or allocated at depth 4 (because the chain was length 4 or longer).
But that's no worse than pre-patch, where you had to use qemu-img info
--backing-chain to learn which file a particular "depth" maps to.

> 
> So, if someone use this API to reconstruct the chain, then for original 3 
> empty layers he will create 3 empty layers and 4rd additional ZERO layer. And 
> such reconstructed chain would not be equal to original chain (as if we take 
> these two chains and add additional backing file as a new bottom layer, 
> effect would be different).. I'm not sure is it a problem in the task you are 
> solving :\

It should be fairly easy to optimize the case of a backing chain where
EVERY listed cluster at the final depth was "data":false,"zero":true
to omit that file after all.

And in oVirt's case, Nir pointed out that we have one more tool at our
disposal in recreating a backing chain: if you use
json:{"driver":"qcow2", "backing":null, ...} as your image file, you
don't have to worry about arbitrary files in the backing chain, only
about recreating the top-most layer of a chain.  And in that case, it
becomes very obvious that "depth":0 is something you must recreate,
and "depth":1 would be a non-existent backing file because you just
passed "backing":null.

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