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

On 03/12/2015 03:14 AM, Daniel P. Berrange wrote: >> 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. > We are NOT relabelling files based on the information. I agree that if we act on a name returned by qemu, then we are vulnerable to mistakes if a guest manages to compromise qemu; but I don't think this is one of those situations. > >> 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. The only use of basePath is the string passed right back to qemu, to kick off the commit or pull operation. Since qemu is using strcmp() (rather than inode comparison), feeding it the name it just told us will make the commit operation work on the correct node. Everything else that libvirt does, such as relabelling files, is done solely on the information libvirt has been tracking (and not reliant on the name returned by qemu). But if it would satisfy your paranoia, I can certainly add a verification step that the string being returned by qemu resolves to the same inode being tracked by libvirt, at least in the case where the <disk> element resolves to a filename rather than a network disk.