[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] qemu-img: Support bitmap --merge into backing image
From: |
Max Reitz |
Subject: |
Re: [PATCH] qemu-img: Support bitmap --merge into backing image |
Date: |
Mon, 14 Sep 2020 11:35:34 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 |
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
signature.asc
Description: OpenPGP digital signature