External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-12-23 06:05:27 +00:00
Code Issues Packages Projects Releases Wiki Activity

nodedev: handle mdevctl errors consistently

Browse Source 
Currently, we have three different types of mdevctl errors:
 1. the command cannot be constructed ecause of unsatisfied
    preconditions
 2. the command cannot be executed due to some error
 3. the command is executed, but returns an error status

These different failures are handled differently. Some cases set an
error and return and error status, and some return a error message but
do not set an error.

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 ensuring that virReportError() is called for
all error conditions rather than returning an error message back to the
calling function.

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
This commit is contained in:
Jonathon Jongsma 2021-06-22 14:53:35 -05:00 committed by Michal Privoznik
parent 5cf6f18d5d
commit bcdcaa2d08
1 changed files with 65 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

114
src/node_device/node_device_driver.c
View File

@ -746,6 +746,8 @@ nodeDeviceGetMdevctlCommand(virNodeDeviceDef *def,
case MDEVCTL_CMD_LAST:
default:
/* SHOULD NEVER HAPPEN */
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unknown Command '%i'"), cmd_type);
return NULL;
}
@ -795,48 +797,62 @@ nodeDeviceGetMdevctlCommand(virNodeDeviceDef *def,
static int
virMdevctlCreate(virNodeDeviceDef *def, char **uuid, char **errmsg)
virMdevctlCreate(virNodeDeviceDef *def, char **uuid)
{
int status;
g_autofree char *errmsg = NULL;
g_autoptr(virCommand) cmd = nodeDeviceGetMdevctlCommand(def,
MDEVCTL_CMD_CREATE,
uuid,
errmsg);
&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)
if (virCommandRun(cmd, &status) < 0)
return -1;
if (status != 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to start mediated device: %s"),
MDEVCTL_ERROR(errmsg));
return -1;
}
/* remove newline */
*uuid = g_strstrip(*uuid);
return 0;
}
static int
virMdevctlDefine(virNodeDeviceDef *def, char **uuid, char **errmsg)
virMdevctlDefine(virNodeDeviceDef *def, char **uuid)
{
int status;
g_autofree char *errmsg = NULL;
g_autoptr(virCommand) cmd = nodeDeviceGetMdevctlCommand(def,
MDEVCTL_CMD_DEFINE,
uuid, errmsg);
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)
if (virCommandRun(cmd, &status) < 0)
return -1;
if (status != 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to define mediated device: %s"),
MDEVCTL_ERROR(errmsg));
return -1;
}
/* remove newline */
*uuid = g_strstrip(*uuid);
return 0;
}
@ -846,7 +862,6 @@ nodeDeviceCreateXMLMdev(virConnectPtr conn,
virNodeDeviceDef *def)
{
g_autofree char *uuid = NULL;
g_autofree char *errmsg = NULL;
if (!def->parent) {
virReportError(VIR_ERR_XML_ERROR, "%s",
@ -854,11 +869,7 @@ nodeDeviceCreateXMLMdev(virConnectPtr conn,
return NULL;
}
if (virMdevctlCreate(def, &uuid, &errmsg) < 0) {
if (errmsg)
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to start mediated device: %s"),
errmsg);
if (virMdevctlCreate(def, &uuid) < 0) {
return NULL;
}
@ -928,55 +939,79 @@ nodeDeviceCreateXML(virConnectPtr conn,
static int
virMdevctlStop(virNodeDeviceDef *def, char **errmsg)
virMdevctlStop(virNodeDeviceDef *def)
{
int status;
g_autoptr(virCommand) cmd = NULL;
g_autofree char *errmsg = NULL;
cmd = nodeDeviceGetMdevctlCommand(def, MDEVCTL_CMD_STOP, NULL, errmsg);
cmd = nodeDeviceGetMdevctlCommand(def, MDEVCTL_CMD_STOP, NULL, &errmsg);
if (!cmd)
return -1;
if (virCommandRun(cmd, &status) < 0 || status != 0)
if (virCommandRun(cmd, &status) < 0)
return -1;
if (status != 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to destroy '%s': %s"), def->name,
MDEVCTL_ERROR(errmsg));
return -1;
}
return 0;
}
static int
virMdevctlUndefine(virNodeDeviceDef *def, char **errmsg)
virMdevctlUndefine(virNodeDeviceDef *def)
{
int status;
g_autoptr(virCommand) cmd = NULL;
g_autofree char *errmsg = NULL;
cmd = nodeDeviceGetMdevctlCommand(def, MDEVCTL_CMD_UNDEFINE, NULL, errmsg);
cmd = nodeDeviceGetMdevctlCommand(def, MDEVCTL_CMD_UNDEFINE, NULL, &errmsg);
if (!cmd)
return -1;
if (virCommandRun(cmd, &status) < 0 || status != 0)
if (virCommandRun(cmd, &status) < 0)
return -1;
if (status != 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to undefine mediated device: %s"),
MDEVCTL_ERROR(errmsg));
return -1;
}
return 0;
}
static int
virMdevctlStart(virNodeDeviceDef *def, char **errmsg)
virMdevctlStart(virNodeDeviceDef *def)
{
int status;
g_autoptr(virCommand) cmd = NULL;
g_autofree char *errmsg = NULL;
cmd = nodeDeviceGetMdevctlCommand(def, MDEVCTL_CMD_START, NULL, errmsg);
cmd = nodeDeviceGetMdevctlCommand(def, MDEVCTL_CMD_START, NULL, &errmsg);
if (!cmd)
return -1;
if (virCommandRun(cmd, &status) < 0 || status != 0)
if (virCommandRun(cmd, &status) < 0)
return -1;
if (status != 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to create mediated device: %s"),
MDEVCTL_ERROR(errmsg));
return -1;
}
return 0;
}
@ -1207,7 +1242,6 @@ nodeDeviceDestroy(virNodeDevicePtr device)
g_autofree char *vfiogroup =
virMediatedDeviceGetIOMMUGroupDev(def->caps->data.mdev.uuid);
VIR_AUTOCLOSE fd = -1;
g_autofree char *errmsg = NULL;
if (!vfiogroup)
goto cleanup;
@ -1221,13 +1255,9 @@ nodeDeviceDestroy(virNodeDevicePtr device)
goto cleanup;
}
if (virMdevctlStop(def, &errmsg) < 0) {
if (errmsg)
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to destroy '%s': %s"), def->name,
errmsg);
if (virMdevctlStop(def) < 0)
goto cleanup;
}
ret = 0;
} else {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@ -1302,7 +1332,6 @@ nodeDeviceDefineXML(virConnect *conn,
g_autoptr(virNodeDeviceDef) def = NULL;
const char *virt_type = NULL;
g_autofree char *uuid = NULL;
g_autofree char *errmsg = NULL;
g_autofree char *name = NULL;
virCheckFlags(0, NULL);
@ -1330,10 +1359,7 @@ nodeDeviceDefineXML(virConnect *conn,
return NULL;
}
if (virMdevctlDefine(def, &uuid, &errmsg) < 0) {
if (errmsg)
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to define mediated device: %s"), errmsg);
if (virMdevctlDefine(def, &uuid) < 0) {
return NULL;
}
@ -1388,14 +1414,9 @@ nodeDeviceUndefine(virNodeDevice *device,
}
if (nodeDeviceHasCapability(def, VIR_NODE_DEV_CAP_MDEV)) {
g_autofree char *errmsg = NULL;
if (virMdevctlUndefine(def, &errmsg) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to undefine mediated device: %s"),
MDEVCTL_ERROR(errmsg));
if (virMdevctlUndefine(def) < 0)
goto cleanup;
}
ret = 0;
} else {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@ -1435,14 +1456,9 @@ nodeDeviceCreate(virNodeDevice *device,
goto cleanup;
if (nodeDeviceHasCapability(def, VIR_NODE_DEV_CAP_MDEV)) {
g_autofree char *errmsg = NULL;
if (virMdevctlStart(def, &errmsg) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to create mediated device: %s"),
MDEVCTL_ERROR(errmsg));
if (virMdevctlStart(def) < 0)
goto cleanup;
}
ret = 0;
} else {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",