On Thu, Jun 9, 2016 at 11:58 AM, Kevin Wolf <kw...@redhat.com> wrote: > Am 08.06.2016 um 17:39 hat Nir Soffer geschrieben: >> On Wed, Jun 8, 2016 at 12:32 PM, Kevin Wolf <kw...@redhat.com> wrote: >> > 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. >> >> >> >> On the other hand, we want to reuse the existing backing chain of the >> >> source instead of opening everything anew, because the latter results in >> >> having multiple BDSs for a single physical file and thus potentially >> >> concurrent access which we should try to avoid. >> > >> > 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. >> > >> > I know that such cases were discussed when mirroring was introduced, I'm >> > not sure whether it's actually used. We need some input there: >> > >> > Eric, can you tell us whether libvirt makes use of such a setup? >> > >> > Nir, I'm not sure who is the right person in oVirt these days, but do >> > you either know yourself whether oVirt requires this to work, or do you >> > know who else would know? >> >> I'm the right person, thanks for keeping me in the loop. >> >> What you describe is how we migrate a disk from one storage to another: >> >> 1. Create a vm snapshot >> 2. Create a volume on the destination storage for the snapshot >> 3. Start mirroring from the source snapshot to the destination snapshot >> using libvirt virDomainBlockCopy: >> https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainBlockCopy > > With VIR_DOMAIN_BLOCK_COPY_SHALLOW set, right? (That is, sync=top in QMP > speech.)
Yes, actually we use: VIR_DOMAIN_BLOCK_COPY_SHALLOW | VIR_DOMAIN_BLOCK_COPY_REUSE_EXT >> 4. Copy the reset of the chain from source to destination using qemu-img >> convert >> 5. Pivot to the new chain using libvirt virDomainBlockJobAbort >> >> https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainBlockJobAbort >> 6. Remove the old chain >> >> source and target can be files or block device, and we plan to support also >> rbd and gluster volumes as target, maybe also as source. > > Thanks, Nir, we should then do our best not to break it. > > Max, maybe we can add a qemu-iotests case that does the exact same thing > as oVirt does? > > Kevin
