On 08.06.2016 13:28, Paolo Bonzini wrote: > > > ----- Original Message ----- >> From: "Kevin Wolf" <kw...@redhat.com> >> To: "Max Reitz" <mre...@redhat.com> >> Cc: qemu-block@nongnu.org, qemu-de...@nongnu.org, "Fam Zheng" >> <f...@redhat.com>, nsof...@redhat.com, >> ebl...@redhat.com, pbonz...@redhat.com >> 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. 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