On 6/22/21 9:53 PM, Jonathon Jongsma wrote:
In commit 68580a51, I removed the checks for NULL cmd variables because
virCommandRun() already handles the case where it is called with a NULL
cmd. Unfortunately, it handles this case by raising a generic error
which is both unhelpful and overwrites our existing error message. So
for example, when I attempt to create a mediated device with an invalid
parent, I get the following output:

     virsh # nodedev-create mdev-test.xml
     error: Failed to create node device from mdev-test.xml
     error: internal error: invalid use of command API

With this patch, I now get a useful error message again:

     virsh # nodedev-create mdev-test.xml
     error: Failed to create node device from mdev-test.xml
     error: internal error: unable to find parent device 'pci_0000_00_03_0'

Signed-off-by: Jonathon Jongsma <jjong...@redhat.com>
---
  src/node_device/node_device_driver.c | 16 ++++++++++++++++
  1 file changed, 16 insertions(+)

diff --git a/src/node_device/node_device_driver.c 
b/src/node_device/node_device_driver.c
index 0f13cb4849..43a8c1bf60 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -799,6 +799,10 @@ virMdevctlCreate(virNodeDeviceDef *def, char **uuid, char 
**errmsg)
                                                              
MDEVCTL_CMD_CREATE,
                                                              uuid,
                                                              errmsg);
+
+    if (!cmd)
+        return -1;
+
      /* an auto-generated uuid is returned via stdout if no uuid is specified 
in
       * the mdevctl args */
      if (virCommandRun(cmd, &status) < 0 || status != 0)
@@ -819,6 +823,9 @@ virMdevctlDefine(virNodeDeviceDef *def, char **uuid, char 
**errmsg)
                                                              
MDEVCTL_CMD_DEFINE,
                                                              uuid, errmsg);
+ if (!cmd)
+        return -1;
+
      /* an auto-generated uuid is returned via stdout if no uuid is specified 
in
       * the mdevctl args */
      if (virCommandRun(cmd, &status) < 0 || status != 0)
@@ -925,6 +932,9 @@ virMdevctlStop(virNodeDeviceDef *def, char **errmsg)
cmd = nodeDeviceGetMdevctlCommand(def, MDEVCTL_CMD_STOP, NULL, errmsg); + if (!cmd)
+        return -1;
+
      if (virCommandRun(cmd, &status) < 0 || status != 0)
          return -1;
@@ -940,6 +950,9 @@ virMdevctlUndefine(virNodeDeviceDef *def, char **errmsg) cmd = nodeDeviceGetMdevctlCommand(def, MDEVCTL_CMD_UNDEFINE, NULL, errmsg); + if (!cmd)
+        return -1;
+
      if (virCommandRun(cmd, &status) < 0 || status != 0)
          return -1;
@@ -955,6 +968,9 @@ virMdevctlStart(virNodeDeviceDef *def, char **errmsg) cmd = nodeDeviceGetMdevctlCommand(def, MDEVCTL_CMD_START, NULL, errmsg); + if (!cmd)
+        return -1;
+
      if (virCommandRun(cmd, &status) < 0 || status != 0)
          return -1;

Reviewed-by: Boris Fiuczynski <fiu...@linux.ibm.com>

--
Mit freundlichen Grüßen/Kind regards
   Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294


Reply via email to