nodedev: capture and report stderror from mdevctl

When an mdevctl command fails, there is not much information available
to the user about why it failed. This is partly because we were not
making use of the error message that mdevctl itself prints upon failure.

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>

diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index 543e5bb90a07c27e1479325cbe4bdebae2f84dc7..d9f215c79618b0d699880fd83c4a8fb883e6ad70 100644 (file)
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -700,7 +700,8 @@ nodeDeviceFindAddressByName(const char *name)
 
 virCommandPtr
 nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr def,
-                                 char **uuid_out)
+                                 char **uuid_out,
+                                 char **errmsg)
 {
     virCommandPtr cmd;
     g_autofree char *json = NULL;
@@ -725,15 +726,17 @@ nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr def,
 
     virCommandSetInputBuffer(cmd, json);
     virCommandSetOutputBuffer(cmd, uuid_out);
+    virCommandSetErrorBuffer(cmd, errmsg);
 
     return cmd;
 }
 
 static int
-virMdevctlStart(virNodeDeviceDefPtr def, char **uuid)
+virMdevctlStart(virNodeDeviceDefPtr def, char **uuid, char **errmsg)
 {
     int status;
-    g_autoptr(virCommand) cmd = nodeDeviceGetMdevctlStartCommand(def, uuid);
+    g_autoptr(virCommand) cmd = nodeDeviceGetMdevctlStartCommand(def, uuid,
+                                                                 errmsg);
     if (!cmd)
         return -1;
 
@@ -754,6 +757,7 @@ nodeDeviceCreateXMLMdev(virConnectPtr conn,
                         virNodeDeviceDefPtr def)
 {
     g_autofree char *uuid = NULL;
+    g_autofree char *errmsg = NULL;
 
     if (!def->parent) {
         virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -761,9 +765,11 @@ nodeDeviceCreateXMLMdev(virConnectPtr conn,
         return NULL;
     }
 
-    if (virMdevctlStart(def, &uuid) < 0) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("Unable to start mediated device"));
+    if (virMdevctlStart(def, &uuid, &errmsg) < 0) {
+        if (errmsg)
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("Unable to start mediated device '%s': %s"),
+                           def->name, errmsg);
         return NULL;
     }
 
@@ -828,23 +834,25 @@ nodeDeviceCreateXML(virConnectPtr conn,
 
 
 virCommandPtr
-nodeDeviceGetMdevctlStopCommand(const char *uuid)
-{
-    return virCommandNewArgList(MDEVCTL,
-                                "stop",
-                                "-u",
-                                uuid,
-                                NULL);
+nodeDeviceGetMdevctlStopCommand(const char *uuid, char **errmsg)
+{
+    virCommandPtr cmd = virCommandNewArgList(MDEVCTL,
+                                             "stop",
+                                             "-u",
+                                             uuid,
+                                             NULL);
+    virCommandSetErrorBuffer(cmd, errmsg);
+    return cmd;
 
 }
 
 static int
-virMdevctlStop(virNodeDeviceDefPtr def)
+virMdevctlStop(virNodeDeviceDefPtr def, char **errmsg)
 {
     int status;
     g_autoptr(virCommand) cmd = NULL;
 
-    cmd = nodeDeviceGetMdevctlStopCommand(def->caps->data.mdev.uuid);
+    cmd = nodeDeviceGetMdevctlStopCommand(def->caps->data.mdev.uuid, errmsg);
 
     if (virCommandRun(cmd, &status) < 0 || status != 0)
         return -1;
@@ -901,9 +909,13 @@ nodeDeviceDestroy(virNodeDevicePtr device)
 
         ret = 0;
     } else if (nodeDeviceHasCapability(def, VIR_NODE_DEV_CAP_MDEV)) {
-        if (virMdevctlStop(def) < 0) {
-            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                           _("Unable to stop mediated device"));
+        g_autofree char *errmsg = NULL;
+
+        if (virMdevctlStop(def, &errmsg) < 0) {
+            if (errmsg)
+                virReportError(VIR_ERR_INTERNAL_ERROR,
+                               _("Unable to destroy '%s': %s"), def->name,
+                               errmsg);
             goto cleanup;
         }
         ret = 0;

diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h
index 2113d2b0a553385515bcd8412ccfd1692969732b..4a40aa51f6c21880b3187d0c52dd17dcfaae638e 100644 (file)
--- a/src/node_device/node_device_driver.h
+++ b/src/node_device/node_device_driver.h
@@ -115,6 +115,8 @@ nodeConnectNodeDeviceEventDeregisterAny(virConnectPtr conn,
 
 virCommandPtr
 nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr def,
-                                 char **uuid_out);
+                                 char **uuid_out,
+                                 char **errmsg);
 virCommandPtr
-nodeDeviceGetMdevctlStopCommand(const char *uuid);
+nodeDeviceGetMdevctlStopCommand(const char *uuid,
+                                char **errmsg);

diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c
index 1bad65549b18e99cb33f40d050ead755d7b70f7a..c12feaac55ad5910bd9b289a8d1e36f1edf162fd 100644 (file)
--- a/tests/nodedevmdevctltest.c
+++ b/tests/nodedevmdevctltest.c
@@ -58,6 +58,7 @@ testMdevctlStart(const char *virt_type,
     const char *actualCmdline = NULL;
     int ret = -1;
     g_autofree char *uuid = NULL;
+    g_autofree char *errmsg = NULL;
     g_autofree char *stdinbuf = NULL;
     g_autoptr(virCommand) cmd = NULL;
 
@@ -66,7 +67,7 @@ testMdevctlStart(const char *virt_type,
 
     /* this function will set a stdin buffer containing the json configuration
      * of the device. The json value is captured in the callback above */
-    cmd = nodeDeviceGetMdevctlStartCommand(def, &uuid);
+    cmd = nodeDeviceGetMdevctlStartCommand(def, &uuid, &errmsg);
 
     if (!cmd)
         goto cleanup;
@@ -117,11 +118,12 @@ testMdevctlStop(const void *data)
     const char *actualCmdline = NULL;
     int ret = -1;
     g_autoptr(virCommand) cmd = NULL;
+    g_autofree char *errmsg = NULL;
     g_autofree char *cmdlinefile =
         g_strdup_printf("%s/nodedevmdevctldata/mdevctl-stop.argv",
                         abs_srcdir);
 
-    cmd = nodeDeviceGetMdevctlStopCommand(uuid);
+    cmd = nodeDeviceGetMdevctlStopCommand(uuid, &errmsg);
 
     if (!cmd)
         goto cleanup;