[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH v2 2/3] block/mirror: Fix target backing BDS

From: Max Reitz
Subject: Re: [Qemu-block] [PATCH v2 2/3] block/mirror: Fix target backing BDS
Date: Wed, 8 Jun 2016 16:40:14 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.0

On 08.06.2016 13:28, Paolo Bonzini wrote:
> ----- Original Message -----
>> From: "Kevin Wolf" <address@hidden>
>> To: "Max Reitz" <address@hidden>
>> Cc: address@hidden, address@hidden, "Fam Zheng" <address@hidden>, 
>> address@hidden,
>> address@hidden, address@hidden
>> Sent: Wednesday, June 8, 2016 11:32:29 AM
>> Subject: Re: [PATCH v2 2/3] block/mirror: Fix target backing BDS
>> Am 06.06.2016 um 16:42 hat Max Reitz geschrieben:
>>> Currently, we are trying to move the backing BDS from the source to the
>>> target in bdrv_replace_in_backing_chain() which is called from
>>> mirror_exit(). However, mirror_complete() already tries to open the
>>> target's backing chain with a call to bdrv_open_backing_file().
>>> First, we should only set the target's backing BDS once. Second, the
>>> mirroring block job has a better idea of what to set it to than the
>>> generic code in bdrv_replace_in_backing_chain() (in fact, the latter's
>>> conditions on when to move the backing BDS from source to target are not
>>> really correct).
>>> Therefore, remove that code from bdrv_replace_in_backing_chain() and
>>> leave it to mirror_complete().
>>> However, mirror_complete() in turn pursues a questionable strategy by
>>> employing bdrv_open_backing_file(): On the one hand, because this may
>>> open the wrong backing file with drive-mirror in "existing" mode, or
>>> because it will not override a possibly wrong backing file in the
>>> blockdev-mirror case.
>> Careful, this "wrong" backing file might actually be intended!
>> Consider a case where you want to move an image with its whole backing
>> chain to different storage. In that case, you would copy all of the
>> backing files (cp is good enough, they are read-only), create the
>> destination image which already points at the copied backing chain, and
>> then mirror in "existing" mode.
>> The intention is obviously that after the job completion the new backing
>> chain is used and not the old one.
> Yes, this is the intention and it should not be changed.  In addition
> to what Kevin said, you can use drive-mirror to collapse the image to a
> single file; in this case, QEMU should not be using the backing files of
> the source.

That is an issue that we have right now. If you do drive-mirror in
absolute-paths mode with sync=full, the target will have the backing
chain of the source. This is something that this patch fixes.

In fact, I think if you do drive-mirror in existing mode or
blockdev-mirror and the target image does not have a backing file
(whatever sync mode you have used), the same will happen.


> bdrv_open_backing_file() is used because what we want to do is to
> "undo" the BDRV_O_NO_BACKING flag used by qmp_drive_mirror.
> If the contents change under the guest feet, it's the layers above
> QEMU that have screwed up.
> Paolo

Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]