From ab1703191b4afca19e7289e3db56fb8d87e50ffa Mon Sep 17 00:00:00 2001 From: Jonathon Jongsma Date: Tue, 23 Feb 2021 14:24:14 -0600 Subject: [PATCH] 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 Reviewed-by: Erik Skultety --- src/node_device/node_device_driver.c | 46 ++++++++++++++++++---------- src/node_device/node_device_driver.h | 6 ++-- tests/nodedevmdevctltest.c | 6 ++-- 3 files changed, 37 insertions(+), 21 deletions(-) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 543e5bb90a..d9f215c796 100644 --- 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) +nodeDeviceGetMdevctlStopCommand(const char *uuid, char **errmsg) { - return virCommandNewArgList(MDEVCTL, - "stop", - "-u", - uuid, - NULL); + 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 2113d2b0a5..4a40aa51f6 100644 --- 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 1bad65549b..c12feaac55 100644 --- 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;