On Tue, Jun 15, 2021 at 11:28:39 -0500, 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 <jjongsma@xxxxxxxxxx> > --- > src/node_device/node_device_driver.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c > index 0f13cb4849..e6d4e6ccb1 100644 > --- a/src/node_device/node_device_driver.c > +++ b/src/node_device/node_device_driver.c > @@ -799,9 +799,10 @@ virMdevctlCreate(virNodeDeviceDef *def, char **uuid, char **errmsg) > MDEVCTL_CMD_CREATE, > uuid, > errmsg); > + > /* an auto-generated uuid is returned via stdout if no uuid is specified in > * the mdevctl args */ > - if (virCommandRun(cmd, &status) < 0 || status != 0) > + if (!cmd || virCommandRun(cmd, &status) < 0 || status != 0) > return -1; Preferably make the '!cmd' condition separate or around the call to 'nodeDeviceGetMdevctlCommand' so that it's obvious that the error originates from there. Additionally virCommandRun if the @exitstatus is non-NULL and the command returns a non-zero value, which you check for will not set an error, so you still have inconsistent behaviour in error reporting.
