From: Eric Blake <eblake@redhat.com>
To: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Cc: nsoffer@redhat.com, Kevin Wolf <kwolf@redhat.com>,
qemu-devel@nongnu.org, qemu-block@nongnu.org,
Max Reitz <mreitz@redhat.com>
Subject: Re: [PATCH v2] qemu-img: Make unallocated part of backing chain obvious in map
Date: Fri, 11 Jun 2021 09:59:18 -0500 [thread overview]
Message-ID: <20210611145918.pslzaqiuxyjttcmj@redhat.com> (raw)
In-Reply-To: <6a0aabb6-441c-8671-fc07-a7113043ccc8@virtuozzo.com>
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
next prev parent reply other threads:[~2021-06-11 15:00 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-11 14:01 [PATCH v2] qemu-img: Make unallocated part of backing chain obvious in map Eric Blake
2021-06-11 14:35 ` Vladimir Sementsov-Ogievskiy
2021-06-11 14:59 ` Eric Blake [this message]
2021-06-11 18:13 ` Nir Soffer
2021-06-11 19:03 ` [PATCH v2 2/1] qemu-img: Add "backing":true to unallocated map segments Eric Blake
2021-06-15 8:54 ` Vladimir Sementsov-Ogievskiy
2021-06-15 13:09 ` Nir Soffer
2021-06-22 15:38 ` Kevin Wolf
2021-06-22 16:56 ` Nir Soffer
2021-06-23 8:57 ` Kevin Wolf
2021-06-23 13:58 ` Nir Soffer
2021-06-23 16:04 ` Kevin Wolf
2021-06-23 16:35 ` Nir Soffer
2021-06-28 17:42 ` Eric Blake
2021-06-29 7:23 ` Vladimir Sementsov-Ogievskiy
2021-06-29 14:40 ` Kevin Wolf
2021-06-29 15:53 ` Nir Soffer
2021-06-22 17:51 ` Vladimir Sementsov-Ogievskiy
2021-06-22 17:04 ` Nir Soffer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210611145918.pslzaqiuxyjttcmj@redhat.com \
--to=eblake@redhat.com \
--cc=kwolf@redhat.com \
--cc=mreitz@redhat.com \
--cc=nsoffer@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=vsementsov@virtuozzo.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).