[libvirt] [PATCH] qemu: read backing chain names from qemu

[Daniel P. Berrange]

On Wed, Mar 11, 2015 at 03:03:00PM -0600, Eric Blake wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1199182 documents that
> after a series of disk snapshots into existing destination images,
> followed by active commits of the top image, it is possible for
> qemu 2.2 and earlier to end up tracking a different name for the
> image than what it would have had when opening the chain afresh.
> That is, when starting with the chain 'a <- b <- c', the name
> associated with 'b' is how it was spelled in the metadata of 'c',
> but when starting with 'a', taking two snapshots into 'a <- b <- c',
> then committing 'c' back into 'b', the name associated with 'b' is
> now the name used when taking the first snapshot.
> 
> Sadly, older qemu doesn't know how to treat different spellings of
> the same filename as identical files (it uses strcmp() instead of
> checking for the same inode), which means libvirt's attempt to
> commit an image using solely the names learned from qcow2 metadata
> fails with a cryptic:
> 
> error: internal error: unable to execute QEMU command 'block-commit': Top image file /tmp/images/c/../b/b not found
> 
> even though the file exists.  Trying to teach libvirt the rules on
> which name qemu will expect is not worth the effort (besides, we'd
> have to remember it across libvirtd restarts, and track whether a
> file was opened via metadata or via snapshot creation for a given
> qemu process); it is easier to just always directly ask qemu what
> string it expects to see in the first place.

If you're going to ask QEMU for the names, then libvirt needs to
validate that the name it gets back resolves to the same inode
as the name it originally had. We cannot trust QEMU to tell the
truth and cannot let it trick us into relabelling the wrong files
by giving us the name of something completely different.


>      qemuDomainObjEnterMonitor(driver, vm);
> -    ret = qemuMonitorBlockJob(priv->mon, device, basePath, backingPath,
> -                              speed, mode, async);
> +    if (baseSource)
> +        basePath = qemuMonitorDiskNameLookup(priv->mon, device, disk->src,
> +                                             baseSource);

So this needs to valid basePath vs baseSource inodes.


> @@ -16739,9 +16734,14 @@ qemuDomainBlockCommit(virDomainPtr dom,
>          disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT;
>      }
>      qemuDomainObjEnterMonitor(driver, vm);
> -    ret = qemuMonitorBlockCommit(priv->mon, device,
> -                                 topPath, basePath, backingPath,
> -                                 speed);
> +    basePath = qemuMonitorDiskNameLookup(priv->mon, device, disk->src,
> +                                         baseSource);

And here

> +    topPath = qemuMonitorDiskNameLookup(priv->mon, device, disk->src,
> +                                        topSource);

And here


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|