----- Original Message ----- > From: "Kevin Wolf" <kw...@redhat.com> > To: "Max Reitz" <mre...@redhat.com> > Cc: qemu-bl...@nongnu.org, qemu-devel@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. 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
