[libvirt PATCH 3/3] nodedev: handle mdevctl errors consistently

[Jonathon Jongsma]

Currently, we have two different types of mdevctl errors:
 1. the command cannot be executed due to some error
 2. the command is executed, but returns an error status

These two different failures are handled differently. Scenario 1 calls
virReportError() from within nodeDeviceGetMdevctlCommand() and returns
an error status (-1). Scenario 2 also returns -1, but does not call
virReportError() and instead passes the error message back in the
errmsg argument.

This means that the caller has to check both whether the return value is
negative and whether the errmsg parameter is non-NULL before deciding
whether to report the error or not. The situation is further complicated
by the fact that there are occasional instances where mdevctl exits with
an error status but does not print an error message.  This results in
errmsg being an empty string "" (i.e. non-NULL).

Simplify the situation by not calling virReportError() at all from
within nodeDeviceGetMdevctlCommand() and instead returning an error
message for those that previously called virReportError(). The caller is
now always responsible for reporting the error.

Also introduce a simple macro that converts NULL or empty errmsg to
"Unknown Error".

Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>
---
 src/node_device/node_device_driver.c | 34 ++++++++++++++--------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index e6d4e6ccb1..3cf9fc129f 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -752,14 +752,13 @@ nodeDeviceGetMdevctlCommand(virNodeDeviceDef *def,
         parent_addr = nodeDeviceFindAddressByName(def->parent);
 
         if (!parent_addr) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("unable to find parent device '%s'"), def->parent);
+            *errbuf = g_strdup_printf(_("unable to find parent device '%s'"),
+                                      def->parent);
             return NULL;
         }
 
         if (nodeDeviceDefToMdevctlConfig(def, &inbuf) < 0) {
-            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                           _("couldn't convert node device def to mdevctl JSON"));
+            *errbuf = g_strdup(_("couldn't convert node device def to mdevctl JSON"));
             return NULL;
         }
 
@@ -832,6 +831,9 @@ virMdevctlDefine(virNodeDeviceDef *def, char **uuid, char **errmsg)
 }
 
 
+#define MDEVCTL_ERROR(msg) msg && msg[0] != '\0' ? msg : _("Unknown error")
+
+
 static virNodeDevicePtr
 nodeDeviceCreateXMLMdev(virConnectPtr conn,
                         virNodeDeviceDef *def)
@@ -846,10 +848,9 @@ nodeDeviceCreateXMLMdev(virConnectPtr conn,
     }
 
     if (virMdevctlCreate(def, &uuid, &errmsg) < 0) {
-        if (errmsg)
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("Unable to start mediated device: %s"),
-                           errmsg);
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Unable to start mediated device: %s"),
+                       MDEVCTL_ERROR(errmsg));
         return NULL;
     }
 
@@ -1201,10 +1202,9 @@ nodeDeviceDestroy(virNodeDevicePtr device)
         }
 
         if (virMdevctlStop(def, &errmsg) < 0) {
-            if (errmsg)
-                virReportError(VIR_ERR_INTERNAL_ERROR,
-                               _("Unable to destroy '%s': %s"), def->name,
-                               errmsg);
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("Unable to destroy '%s': %s"), def->name,
+                           MDEVCTL_ERROR(errmsg));
             goto cleanup;
         }
         ret = 0;
@@ -1310,9 +1310,9 @@ nodeDeviceDefineXML(virConnect *conn,
     }
 
     if (virMdevctlDefine(def, &uuid, &errmsg) < 0) {
-        if (errmsg)
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("Unable to define mediated device: %s"), errmsg);
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Unable to define mediated device: %s"),
+                       MDEVCTL_ERROR(errmsg));
         return NULL;
     }
 
@@ -1372,7 +1372,7 @@ nodeDeviceUndefine(virNodeDevice *device,
         if (virMdevctlUndefine(def, &errmsg) < 0) {
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("Unable to undefine mediated device: %s"),
-                           errmsg && errmsg[0] ? errmsg : "Unknown Error");
+                           MDEVCTL_ERROR(errmsg));
             goto cleanup;
         }
         ret = 0;
@@ -1419,7 +1419,7 @@ nodeDeviceCreate(virNodeDevice *device,
         if (virMdevctlStart(def, &errmsg) < 0) {
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("Unable to create mediated device: %s"),
-                           errmsg && errmsg[0] ? errmsg : "Unknown Error");
+                           MDEVCTL_ERROR(errmsg));
             goto cleanup;
         }
         ret = 0;
-- 
2.31.1