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

[Eric Blake]

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 at redhat.com>
---
 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
@@ -11044,6 +11044,12 @@ qemuDomainGetBlockInfo(virDomainPtr dom,
     }

     disk = vm->def->disks[idx];
+    if (virStorageSourceIsEmpty(disk->src)) {
+        virReportError(VIR_ERR_INVALID_ARG,
+                       _("disk '%s' does not currently have a source assigned"),
+                       path);
+        goto endjob;
+    }

     /* FIXME: For an offline domain, we always want to check current
      * on-disk statistics (as users have been known to change offline
@@ -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);
@@ -11092,37 +11094,17 @@ qemuDomainGetBlockInfo(virDomainPtr dom,
         if (virStorageFileInitAs(disk->src, cfg->user, cfg->group) < 0)
             goto endjob;

-        if ((len = virStorageFileReadHeader(disk->src, VIR_STORAGE_MAX_HEADER,
-                                            &buf)) < 0)
-            goto endjob;
-
         if (virStorageFileStat(disk->src, &sb) < 0) {
             virReportSystemError(errno, _("failed to stat remote file '%s'"),
                                  NULLSTR(disk->src->path));
             goto endjob;
         }
-    }
-
-    /* Probe for magic formats */
-    if (virDomainDiskGetFormat(disk)) {
-        format = virDomainDiskGetFormat(disk);
-    } else {
-        if (!cfg->allowDiskFormatProbing) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("no disk format for %s and probing is disabled"),
-                           path);
-            goto endjob;
-        }

-        if ((format = virStorageFileProbeFormatFromBuf(disk->src->path,
-                                                       buf, len)) < 0)
+        if ((len = virStorageFileReadHeader(disk->src, VIR_STORAGE_MAX_HEADER,
+                                            &buf)) < 0)
             goto endjob;
     }

-    if (!(meta = virStorageFileGetMetadataFromBuf(disk->src->path, buf, len,
-                                                  format, NULL)))
-        goto endjob;
-
     /* Get info for normal formats */
     if (S_ISREG(sb.st_mode) || fd == -1) {
 #ifndef WIN32
@@ -11151,6 +11133,21 @@ qemuDomainGetBlockInfo(virDomainPtr dom,

     /* If the file we probed has a capacity set, then override
      * what we calculated from file/block extents */
+    if (!(format = virDomainDiskGetFormat(disk))) {
+        if (!cfg->allowDiskFormatProbing) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("no disk format for %s and probing is disabled"),
+                           path);
+            goto endjob;
+        }
+
+        if ((format = virStorageFileProbeFormatFromBuf(disk->src->path,
+                                                       buf, len)) < 0)
+            goto endjob;
+    }
+    if (!(meta = virStorageFileGetMetadataFromBuf(disk->src->path, buf, len,
+                                                  format, NULL)))
+        goto endjob;
     if (meta->capacity)
         disk->src->capacity = meta->capacity;

-- 
1.9.3