[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:42:02 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.0 |
On 08.06.2016 16:40, Max Reitz wrote:
> 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.
As a clarification: I mean the backing chain inside QEMU (in the BDS
graph), not the on-disk backing chain, i.e. how the physical image files
link to each other.
Max
> 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.
>
> Max
>
>> 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
>>
>
>
signature.asc
Description: OpenPGP digital signature
- [Qemu-block] [PATCH v2 0/3] block/mirror: Fix target backing BDS, Max Reitz, 2016/06/06
- [Qemu-block] [PATCH v2 3/3] iotests: Add test for post-mirror backing chains, Max Reitz, 2016/06/06
- [Qemu-block] [PATCH v2 1/3] block: Allow replacement of a BDS by its overlay, Max Reitz, 2016/06/06
- [Qemu-block] [PATCH v2 2/3] block/mirror: Fix target backing BDS, Max Reitz, 2016/06/06
- Re: [Qemu-block] [PATCH v2 2/3] block/mirror: Fix target backing BDS, Max Reitz, 2016/06/08
- Re: [Qemu-block] [PATCH v2 2/3] block/mirror: Fix target backing BDS, Max Reitz, 2016/06/08
- Re: [Qemu-block] [PATCH v2 2/3] block/mirror: Fix target backing BDS, Nir Soffer, 2016/06/08
- Re: [Qemu-block] [PATCH v2 2/3] block/mirror: Fix target backing BDS, Kevin Wolf, 2016/06/09
- Re: [Qemu-block] [PATCH v2 2/3] block/mirror: Fix target backing BDS, Nir Soffer, 2016/06/09