Re: [PATCH v2] qemu-img: Support bitmap --merge into backing image

[Max Reitz]

On 14.09.20 21:10, Eric Blake wrote:
> If you have the chain 'base.qcow2 <- top.qcow2' and want to merge a
> bitmap from top into base, qemu-img was failing with:
> 
> qemu-img: Could not open 'top.qcow2': Could not open backing file: Failed to 
> get shared "write" lock
> Is another process using the image [base.qcow2]?
> 
> The easiest fix is to not open the entire backing chain of either
> image (source or destination); after all, the point of 'qemu-img
> bitmap' is solely to manipulate bitmaps directly within a single qcow2
> image, and this is made more precise if we don't pay attention to
> other images in the chain that may happen to have a bitmap by the same
> name.
> 
> However, note that during normal usage, it is a feature that qemu will
> allow a bitmap from a backing image to be exposed by an overlay BDS;
> doing so makes it easier to perform incremental backup, where we have:
> 
> Base <- Active <- temporrary

*temporary

(Also it’s a bit strange that “Base” and “Active” are capitalized, but
“temporary” isn’t)

>           \--block job ->/
> 
> with temporary being fed by a block-copy 'sync' job; when exposing

s/block-copy 'sync'/backup 'sync=none'/?

> temporary over NBD, referring to a bitmap that lives only in Active is
> less effort than having to copy a bitmap into temporary [1].  So the
> testsuite additions in this patch check both where bitmaps get
> allocated (the qemu-img info output), and, when NOT using 'qemu-img
> bitmap', that bitmaps are indeed visible through a backing chain.

Well.  It is useful over NBD but I would doubt that it isn’t useful in
general.  For example, the QMP commands that refer to bitmaps always do
so through a node-name + bitmap-name combination, and they require that
the given bitmap is exactly on the given node.

So I think this is a very much a case-by-case question.  (And in
practice, NBD seems to be the outlier, not qemu-img bitmap.)

> [1] Full disclosure: prior to the recent commit 374eedd1c4 and
> friends, we were NOT able to see bitmaps through filters, which meant
> that we actually did not have nice clean semantics for uniformly being
> able to pick up bitmaps from anywhere in the backing chain (seen as a
> change in behavior between qemu 4.1 and 4.2 at commit 00e30f05de, when
> block-copy swapped from a one-off to a filter).  Which means libvirt
> was already coded to copy bitmaps around for the sake of older qemu,
> even though modern qemu no longer needs it.  Oh well.
> 
> Fixes: http://bugzilla.redhat.com/1877209
> Reported-by: Eyal Shenitzky <eshenitz@redhat.com>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> 
> In v2:
> - also use BDRV_O_NO_BACKING on source [Max]
> - improved commit message [Max]
> 
>  qemu-img.c                 | 11 ++++++--
>  tests/qemu-iotests/291     | 12 ++++++++
>  tests/qemu-iotests/291.out | 56 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 76 insertions(+), 3 deletions(-)

The code looks good to me, but I wonder whether in the commit message it
should be noted that we don’t want to let bitmaps from deeper nodes
shine through by default everywhere, but just in specific cases where
that’s useful (i.e. only NBD so far AFAIA).

Max

signature.asc
Description: OpenPGP digital signature