From c98b451c7f34d5884a59ae5514cd6334065362b2 Mon Sep 17 00:00:00 2001

Message-Id: <c98b451c7f34d5884a59ae5514cd6334065362b2@dist-git>

From: John Ferlan <jferlan@redhat.com>

Date: Tue, 4 Aug 2015 08:05:52 -0400

Subject: [PATCH] conf: Allow error reporting in virDomainDiskSourceIsBlockType

https://bugzilla.redhat.com/show_bug.cgi?id=1238545

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.

(cherry picked from commit 36025c552cae6c79ab9e54f65e758907845846ac)

Signed-off-by: John Ferlan <jferlan@redhat.com>

Signed-off-by: Jiri Denemark <jdenemar@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 d990180..72d87dd 100644

--- a/src/conf/domain_conf.c

+++ b/src/conf/domain_conf.c

@@ -23812,10 +23812,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;

@@ -23831,11 +23837,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 0adaefc..604d0b8 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 cc1277b..5ecfc7a 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, true))

         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 48d2ee0..feba308 100644

--- a/src/qemu/qemu_command.c

+++ b/src/qemu/qemu_command.c

@@ -3453,10 +3453,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 d521886..7c2947c 100644

--- a/src/qemu/qemu_conf.c

+++ b/src/qemu/qemu_conf.c

@@ -1178,7 +1178,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);

@@ -1315,7 +1315,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);

@@ -1400,7 +1400,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.5.0