Commit id '8dc27259' introduced virStorageSourceUpdateBlockPhysicalSize
in order to retrieve the physical size for a block backed source device
since commit id '15fa84ac' changed to use the qemuMonitorGetAllBlockStatsInfo
and qemuMonitorBlockStatsUpdateCapacity API's to (essentially) retrieve the
"actual-size" from a 'query-block' operation for the source device.

However, the code only was made functional for a BLOCK backing type
and it neglected to use qemuOpenFile, instead using just open. After
the open the block lseek would find the end of the block and set the
physical value, close the fd and return.

Since the code would return 0 immediately if the source device wasn't
a BLOCK backed device, the physical would be displayed incorrectly,
such as follows in domblkinfo for a file backed source device:

Capacity:       1073741824
Allocation:     0
Physical:       0

This patch will modify the algorithm to get the physical size for other
backing types and it will make use of the qemuDomainStorageOpenStat
helper in order to open/stat the source file depending on its type.
The qemuDomainGetStatsOneBlock will no longer inhibit printing errors,
but it will still ignore them leaving the physical value set to 0.

Signed-off-by: John Ferlan <jfer...@redhat.com>
---
 src/libvirt_private.syms  |  2 +-
 src/qemu/qemu_driver.c    | 25 +++++++++++++++----
 src/util/virstoragefile.c | 61 ++++++++++++++++++++++++++++-------------------
 src/util/virstoragefile.h |  6 +++--
 4 files changed, 62 insertions(+), 32 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 43220e0..eaaa088 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2442,7 +2442,7 @@ virStorageSourceParseRBDColonString;
 virStorageSourcePoolDefFree;
 virStorageSourcePoolModeTypeFromString;
 virStorageSourcePoolModeTypeToString;
-virStorageSourceUpdateBlockPhysicalSize;
+virStorageSourceUpdatePhysicalSize;
 virStorageTypeFromString;
 virStorageTypeToString;
 
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a744fda..6563a1e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11601,10 +11601,23 @@ qemuDomainStorageCloseStat(virStorageSourcePtr src,
 
 
 static int
-qemuDomainStorageUpdatePhysical(virStorageSourcePtr src,
-                                bool report)
+qemuDomainStorageUpdatePhysical(virQEMUDriverPtr driver,
+                                virQEMUDriverConfigPtr cfg,
+                                virDomainObjPtr vm,
+                                virStorageSourcePtr src)
 {
-    return virStorageSourceUpdateBlockPhysicalSize(src, report);
+    int ret;
+    int fd = -1;
+    struct stat sb;
+
+    if (qemuDomainStorageOpenStat(driver, cfg, vm, src, &fd, &sb) < 0)
+        return -1;
+
+    ret = virStorageSourceUpdatePhysicalSize(src, fd, &sb);
+
+    qemuDomainStorageCloseStat(src, &fd);
+
+    return ret;
 }
 
 
@@ -11831,7 +11844,7 @@ qemuDomainGetBlockInfo(virDomainPtr dom,
     if (entry->physical) {
         info->physical = entry->physical;
     } else {
-        if (qemuDomainStorageUpdatePhysical(disk->src, true) < 0)
+        if (qemuDomainStorageUpdatePhysical(driver, cfg, vm, disk->src) < 0)
             goto endjob;
 
         info->physical = disk->src->physical;
@@ -19386,9 +19399,11 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver,
         QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx,
                                  "physical", entry->physical);
     } else {
-        if (qemuDomainStorageUpdatePhysical(src, false) == 0) {
+        if (qemuDomainStorageUpdatePhysical(driver, cfg, dom, src) == 0) {
             QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx,
                                      "physical", src->physical);
+        } else {
+            virResetLastError();
         }
     }
 
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 1011bd0..809d825 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -24,7 +24,6 @@
 #include <config.h>
 #include "virstoragefile.h"
 
-#include <sys/stat.h>
 #include <unistd.h>
 #include <fcntl.h>
 #include <stdlib.h>
@@ -3175,41 +3174,55 @@ virStorageSourceNewFromBacking(virStorageSourcePtr 
parent)
 
 
 /**
- * @src: disk source definiton structure
- * @report: report libvirt errors if set to true
+ * @src: disk source definition structure
+ * @fd: file descriptor
+ * @sb: stat buffer
  *
- * Updates src->physical for block devices since qemu doesn't report the 
current
- * size correctly for them. Returns 0 on success, -1 on error.
+ * Updates src->physical depending on the actual type of storage being used.
+ *
+ * Returns 0 on success, -1 on error.
  */
 int
-virStorageSourceUpdateBlockPhysicalSize(virStorageSourcePtr src,
-                                        bool report)
+virStorageSourceUpdatePhysicalSize(virStorageSourcePtr src,
+                                   int fd,
+                                   struct stat const *sb)
 {
-    int fd = -1;
     off_t end;
-    int ret = -1;
+    virStorageType actual_type = virStorageSourceGetActualType(src);
 
-    if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_BLOCK)
-        return 0;
+    switch (actual_type) {
+    case VIR_STORAGE_TYPE_FILE:
+    case VIR_STORAGE_TYPE_NETWORK:
+        src->physical = sb->st_size;
+        break;
 
-    if ((fd = open(src->path, O_RDONLY)) < 0) {
-        if (report)
-            virReportSystemError(errno, _("failed to open block device '%s'"),
+    case VIR_STORAGE_TYPE_BLOCK:
+        if ((end = lseek(fd, 0, SEEK_END)) == (off_t) -1) {
+            virReportSystemError(errno, _("failed to seek to end of '%s'"),
                                  src->path);
-        return -1;
-    }
+            return -1;
+        }
 
-    if ((end = lseek(fd, 0, SEEK_END)) == (off_t) -1) {
-        if (report)
-            virReportSystemError(errno,
-                                 _("failed to seek to end of '%s'"), 
src->path);
-    } else {
         src->physical = end;
-        ret = 0;
+        break;
+
+    case VIR_STORAGE_TYPE_DIR:
+        src->physical = 0;
+        break;
+
+    /* We shouldn't get VOLUME, but the switch requires all cases */
+    case VIR_STORAGE_TYPE_VOLUME:
+    case VIR_STORAGE_TYPE_NONE:
+    case VIR_STORAGE_TYPE_LAST:
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                      _("cannot retrieve physical for path '%s' type '%s'"),
+                      NULLSTR(src->path),
+                      virStorageTypeToString(actual_type));
+        return -1;
+        break;
     }
 
-    VIR_FORCE_CLOSE(fd);
-    return ret;
+    return 0;
 }
 
 
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 3d09468..e38d01f 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -24,6 +24,8 @@
 #ifndef __VIR_STORAGE_FILE_H__
 # define __VIR_STORAGE_FILE_H__
 
+# include <sys/stat.h>
+
 # include "virbitmap.h"
 # include "virseclabel.h"
 # include "virstorageencryption.h"
@@ -355,8 +357,8 @@ bool virStorageSourceIsEmpty(virStorageSourcePtr src);
 bool virStorageSourceIsBlockLocal(const virStorageSource *src);
 void virStorageSourceFree(virStorageSourcePtr def);
 void virStorageSourceBackingStoreClear(virStorageSourcePtr def);
-int virStorageSourceUpdateBlockPhysicalSize(virStorageSourcePtr src,
-                                            bool report);
+int virStorageSourceUpdatePhysicalSize(virStorageSourcePtr src,
+                                       int fd, struct stat const *sb);
 virStorageSourcePtr virStorageSourceNewFromBacking(virStorageSourcePtr parent);
 virStorageSourcePtr virStorageSourceCopy(const virStorageSource *src,
                                          bool backingChain)
-- 
2.7.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to