Re: [libvirt] [PATCH v2 05/12] getstats: rearrange blockinfo gathering

We want to avoid read()ing a file while qemu is running. We still have to open() block devices to determine their physical size, but that is safer. This patch rearranges code to make it easier to skip the metadata collection when possible. * src/qemu/qemu_driver.c (qemuDomainGetBlockInfo): Check for empty disk up front. Perform stat first. Place metadata reading next to use. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 61 ++++++++++++++++++++++++-------------------------- 1 file changed, 29 insertions(+), 32 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7aaee96..f87c44b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c

@@ -11066,10 +11072,12 @@ qemuDomainGetBlockInfo(virDomainPtr dom, * change. */ if (virStorageSourceIsLocalStorage(disk->src)) { - if (!disk->src->path) { - virReportError(VIR_ERR_INVALID_ARG, - _("disk '%s' does not currently have a source assigned"), - path); + /* Yes, this is a mild TOCTTOU race, but if someone is + * changing files in the background behind libvirt's back, + * they deserve bogus information. */ + if (stat(disk->src->path, &sb) < 0) { + virReportSystemError(errno, + _("cannot stat file '%s'"), disk->src->path); goto endjob; }

@@ -11077,12 +11085,6 @@ qemuDomainGetBlockInfo(virDomainPtr dom, NULL, NULL)) == -1) goto endjob; - if (fstat(fd, &sb) < 0) { - virReportSystemError(errno, - _("cannot stat file '%s'"), disk->src->path); - goto endjob; - } - if ((len = virFileReadHeaderFD(fd, VIR_STORAGE_MAX_HEADER, &buf)) < 0) { virReportSystemError(errno, _("cannot read header '%s'"), disk->src->path);