[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 2/3] block/mirror: Fix target backing BDS
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH v2 2/3] block/mirror: Fix target backing BDS |
Date: |
Wed, 8 Jun 2016 07:28:09 -0400 (EDT) |
----- 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.
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
- [Qemu-devel] [PATCH v2 0/3] block/mirror: Fix target backing BDS, Max Reitz, 2016/06/06
- [Qemu-devel] [PATCH v2 3/3] iotests: Add test for post-mirror backing chains, Max Reitz, 2016/06/06
- [Qemu-devel] [PATCH v2 1/3] block: Allow replacement of a BDS by its overlay, Max Reitz, 2016/06/06
- [Qemu-devel] [PATCH v2 2/3] block/mirror: Fix target backing BDS, Max Reitz, 2016/06/06
- Re: [Qemu-devel] [PATCH v2 2/3] block/mirror: Fix target backing BDS, Max Reitz, 2016/06/08
- Re: [Qemu-devel] [PATCH v2 2/3] block/mirror: Fix target backing BDS, Max Reitz, 2016/06/08
- Re: [Qemu-devel] [PATCH v2 2/3] block/mirror: Fix target backing BDS, Nir Soffer, 2016/06/08
- Re: [Qemu-devel] [PATCH v2 2/3] block/mirror: Fix target backing BDS, Kevin Wolf, 2016/06/09
- Re: [Qemu-devel] [PATCH v2 2/3] block/mirror: Fix target backing BDS, Nir Soffer, 2016/06/09