[libvirt] [PATCH 2/2] conf: Allow error reporting in virDomainDiskSourceIsBlockType

John Ferlan jferlan at redhat.com
Sat Jul 18 11:43:10 UTC 2015


Rather than provide a somewhat generic error message when the API
returns false, allow the caller to supply a "report = true" option
in order to cause virReportError's to describe which of the 3 paths
that can cause failure.

Some callers don't care about what caused the failure, they just want
to have a true/false - for those, calling with report = false should
be sufficient.

Signed-off-by: John Ferlan <jferlan at redhat.com>
---
 src/conf/domain_conf.c  | 21 ++++++++++++++++++---
 src/conf/domain_conf.h  |  2 +-
 src/lxc/lxc_cgroup.c    |  2 +-
 src/lxc/lxc_driver.c    |  6 ++----
 src/qemu/qemu_command.c |  5 +----
 src/qemu/qemu_conf.c    |  6 +++---
 6 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5a9a88d..7320d5b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -23814,10 +23814,16 @@ virDomainDefFindDevice(virDomainDefPtr def,
  * Return true if its source is block type, or false otherwise.
  */
 bool
-virDomainDiskSourceIsBlockType(virStorageSourcePtr src)
+virDomainDiskSourceIsBlockType(virStorageSourcePtr src,
+                               bool report)
 {
-    if (!src->path)
+    if (!src->path) {
+        if (report)
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("source path not found for device='lun' "
+                             "using type='%d'"), src->type);
         return false;
+    }
 
     if (src->type == VIR_STORAGE_TYPE_BLOCK)
         return true;
@@ -23833,11 +23839,20 @@ virDomainDiskSourceIsBlockType(virStorageSourcePtr src)
          * (e.g. set sgio=filtered|unfiltered for it) in libvirt.
          */
          if (src->srcpool->pooltype == VIR_STORAGE_POOL_ISCSI &&
-             src->srcpool->mode == VIR_STORAGE_SOURCE_POOL_MODE_DIRECT)
+             src->srcpool->mode == VIR_STORAGE_SOURCE_POOL_MODE_DIRECT) {
+             if (report)
+                 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                                _("disk device='lun' for iSCSI is not "
+                                  "supported with mode='direct'."));
              return false;
+         }
 
         return true;
     }
+    if (report)
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("disk device='lun' is only valid for block "
+                         "type disk source"));
     return false;
 }
 
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 50750c1..34e1b99 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3126,7 +3126,7 @@ int virDomainDefFindDevice(virDomainDefPtr def,
                            virDomainDeviceDefPtr dev,
                            bool reportError);
 
-bool virDomainDiskSourceIsBlockType(virStorageSourcePtr src)
+bool virDomainDiskSourceIsBlockType(virStorageSourcePtr src, bool report)
     ATTRIBUTE_NONNULL(1);
 
 void virDomainChrSourceDefClear(virDomainChrSourceDefPtr def);
diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
index 507d567..e9caa3e 100644
--- a/src/lxc/lxc_cgroup.c
+++ b/src/lxc/lxc_cgroup.c
@@ -382,7 +382,7 @@ static int virLXCCgroupSetupDeviceACL(virDomainDefPtr def,
 
     VIR_DEBUG("Allowing any disk block devs");
     for (i = 0; i < def->ndisks; i++) {
-        if (!virDomainDiskSourceIsBlockType(def->disks[i]->src))
+        if (!virDomainDiskSourceIsBlockType(def->disks[i]->src, false))
             continue;
 
         if (virCgroupAllowDevicePath(cgroup,
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 81bb711..913e007 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -4069,11 +4069,9 @@ lxcDomainAttachDeviceDiskLive(virLXCDriverPtr driver,
         goto cleanup;
     }
 
-    if (!virDomainDiskSourceIsBlockType(def->src)) {
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                       _("Can't setup disk for non-block device"));
+    if (!virDomainDiskSourceIsBlockType(def->src, false))
         goto cleanup;
-    }
+
     src = virDomainDiskGetSource(def);
     if (src == NULL) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 42906a8..dc2d515 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3472,10 +3472,7 @@ qemuCheckDiskConfig(virDomainDiskDefPtr disk)
                                virStorageNetProtocolTypeToString(disk->src->protocol));
                 goto error;
             }
-        } else if (!virDomainDiskSourceIsBlockType(disk->src)) {
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                           _("disk device='lun' is only valid for block "
-                             "type disk source"));
+        } else if (!virDomainDiskSourceIsBlockType(disk->src, true)) {
             goto error;
         }
         if (disk->wwn) {
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 38d4a86..fbe4df3 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1210,7 +1210,7 @@ qemuAddSharedDisk(virQEMUDriverPtr driver,
     char *key = NULL;
     int ret = -1;
 
-    if (!disk->src->shared || !virDomainDiskSourceIsBlockType(disk->src))
+    if (!disk->src->shared || !virDomainDiskSourceIsBlockType(disk->src, false))
         return 0;
 
     qemuDriverLock(driver);
@@ -1355,7 +1355,7 @@ qemuRemoveSharedDisk(virQEMUDriverPtr driver,
     char *key = NULL;
     int ret = -1;
 
-    if (!disk->src->shared || !virDomainDiskSourceIsBlockType(disk->src))
+    if (!disk->src->shared || !virDomainDiskSourceIsBlockType(disk->src, false))
         return 0;
 
     qemuDriverLock(driver);
@@ -1443,7 +1443,7 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev)
         disk = dev->data.disk;
 
         if (disk->device != VIR_DOMAIN_DISK_DEVICE_LUN ||
-            !virDomainDiskSourceIsBlockType(disk->src))
+            !virDomainDiskSourceIsBlockType(disk->src, false))
             return 0;
 
         path = virDomainDiskGetSource(disk);
-- 
2.1.0




More information about the libvir-list mailing list