On 05/22/2014 07:47 AM, Peter Krempa wrote:
Refresh the disk backing chains when reconnecting to a qemu process after daemon restart. There are a few internal fields that don't get refreshed from the XML. Until we are able to do that, let's reload all the metadata by the backing chain crawler.
Not necessarily ideal. Reading metadata from a file that is also simultaneously opened read-write by qemu might, in super-rare circumstances, hit a race where qemu is in the middle of updating metadata due to the completion of some job that was started before the libvirtd restart. Although qcow2 has a hard cap of header fitting in one cluster (typically 64k) (at least, after CVE-2014-0144, qemu.git commit 24342f2ca), it is still plausible that qemu is updating the qcow2 header via sectors (perhaps as small as 512-byte chunks) rather than by full clusters; and there are scenarios where an update touches two non-adjacent sectors (such as adjusting the backing file name), such that if a reader sees the old content of one sector but the new content of the other, then it is completely broken in interpreting the data. A slightly safer alternative might be using a QMP command to ask qemu what the backing chain currently is when we reconnect - although that may require correlation back to filenames we passed in, since in the face of fd passing, qemu may only know the name "/dev/fdset/NNN" instead of the filename the earlier libvirtd opened and passed in by fd. And that presupposes that qemu can always be trusted (we don't want to introduce a CVE where a guest that compromises qemu into returning bogus QMP results can then cause libvirt to mismanage the host filesystem). So with those points made, a metadata read race is extremely rare, and I see no point in implementing a half-way solution that requires parsing a QMP command if we are eventually going to switch over to full XML tracking anyways. Therefore, since _this_ patch is a clear improvement (even if not theoretically perfect), we shouldn't hold it up waiting for some other solution. ACK as-is.
src/qemu/qemu_process.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a83780f..4aa9ca3 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3213,6 +3213,11 @@ qemuProcessReconnect(void *opaque) if (qemuTranslateDiskSourcePool(conn, obj->def->disks[i]) < 0) goto error;
+ /* XXX we should be able to restore all data from XML in the future */
and thanks for the comment to remind us what we should improve.
+ if (qemuDomainDetermineDiskChain(driver, obj, + obj->def->disks[i], true) < 0) + goto error; + dev.type = VIR_DOMAIN_DEVICE_DISK; dev.data.disk = obj->def->disks[i]; if (qemuAddSharedDevice(driver, &dev, obj->def->name) < 0)
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org