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

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3445 bytes --]

On 11.09.20 17:17, Eric Blake wrote:
> On 9/11/20 3:31 AM, Max Reitz wrote:
>> On 09.09.20 14:33, 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 the source
>>> image, so that we aren't worrying about competing BDS visiting the
>>> backing image as both a read-only backing of the source and the
>>> writeable destination.
>>>
>>> Fixes: http://bugzilla.redhat.com/1877209
>>> Reported-by: Eyal Shenitzky <eshenitz@redhat.com>
>>> Signed-off-by: Eric Blake <eblake@redhat.com>
>>> ---
>>>   qemu-img.c                 |  3 +-
>>>   tests/qemu-iotests/291     | 12 ++++++++
>>>   tests/qemu-iotests/291.out | 56 ++++++++++++++++++++++++++++++++++++++
>>>   3 files changed, 70 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/qemu-img.c b/qemu-img.c
>>> index eb2fc1f86243..b15098a2f9b3 100644
>>> --- a/qemu-img.c
>>> +++ b/qemu-img.c
>>> @@ -4755,7 +4755,8 @@ static int img_bitmap(int argc, char **argv)
>>>       }
>>>       bs = blk_bs(blk);
>>>       if (src_filename) {
>>> -        src = img_open(false, src_filename, src_fmt, 0, false,
>>> false, false);
>>> +        src = img_open(false, src_filename, src_fmt, BDRV_O_NO_BACKING,
>>> +                       false, false, false);
>>
>> Why not do the same for the destination BB?
> 
> Yeah, that should work, too.
> 
>>
>>>           if (!src) {
>>>               goto out;
>>>           }
>>> diff --git a/tests/qemu-iotests/291 b/tests/qemu-iotests/291
>>> index 1e0bb76959bb..4f837b205655 100755
>>> --- a/tests/qemu-iotests/291
>>> +++ b/tests/qemu-iotests/291
>>
>> [...]
>>
>>> @@ -107,6 +116,9 @@ $QEMU_IMG map --output=json --image-opts \
>>>   nbd_server_start_unix_socket -r -f qcow2 -B b2 "$TEST_IMG"
>>>   $QEMU_IMG map --output=json --image-opts \
>>>       "$IMG,x-dirty-bitmap=qemu:dirty-bitmap:b2" | _filter_qemu_img_map
>>> +nbd_server_start_unix_socket -r -f qcow2 -B b3 "$TEST_IMG"
>>
>> Why not look into $TEST_IMG.base to see specifically whether the bitmap
>> is there?
> 
> We did just that, several lines earlier, with the qemu-img info
> --backing-chain.

OK, perfect.

>>
>> (I also am quite surprised that it’s even possible to export bitmaps
>> from backing nodes, but, well.)
> 
> I actually ought to call that out in the commit message.  It used to be
> that we were inconsistent on what we could see from the backing chain (a
> filter would make it so we can't), but as soon as your filter patches
> land, then we _do_ want to always be able to find a bitmap from the
> backing chain (incremental backup depends on that: we create an overlay
> disk to run the block-copy job as a filter, and _want_ to expose that
> overlay image with the bitmap it inherits from the original image).  So
> being able to export bitmaps from a backing node is normally a feature;
> and it is only in 'qemu-img bitmap' where we don't want accidental
> inheritance to get in the way from what we are actually merging.

Understood.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]