On 4/13/21 4:39 PM, Jonathon Jongsma wrote:
From: Erik Skultety <eskultet@xxxxxxxxxx>
Currently there are dedicated wrappers to construct mdevctl command.
These are mostly fine except for the one that translates both "start"
and "define" commands, only because mdevctl takes the same set of
arguments. Instead, keep the wrappers, but let them call a single
global translator that handles all the mdevctl command differences and
commonalities.
Signed-off-by: Erik Skultety <eskultet@xxxxxxxxxx>
Signed-off-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
---
src/node_device/node_device_driver.c | 125 ++++++++++++++++-----------
src/node_device/node_device_driver.h | 6 +-
tests/nodedevmdevctltest.c | 4 +-
3 files changed, 79 insertions(+), 56 deletions(-)
Coverity found some issues...
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index da55e386f0..bbb01c3967 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -702,42 +702,75 @@ nodeDeviceFindAddressByName(const char *name)
}
-/* the mdevctl 'start' and 'define' commands accept almost the exact same
- * arguments, so provide a common implementation that can be wrapped by a more
- * specific function */
-static virCommand*
-nodeDeviceGetMdevctlDefineCreateCommand(virNodeDeviceDef *def,
- const char *subcommand,
- char **uuid_out,
- char **errmsg)
+static virCommand *
+nodeDeviceGetMdevctlCommand(virNodeDeviceDef *def,
+ virMdevctlCommand cmd_type,
+ char **outbuf,
+ char **errbuf)
{
- virCommand *cmd;
- g_autofree char *json = NULL;
- g_autofree char *parent_addr = nodeDeviceFindAddressByName(def->parent);
-
- if (!parent_addr) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unable to find parent device '%s'"), def->parent);
- return NULL;
+ g_autofree char *parent_addr = NULL;
+ virCommand *cmd = NULL;
+ const char *subcommand = virMdevctlCommandTypeToString(cmd_type);
+ g_autofree char *inbuf = NULL;
+
+ switch (cmd_type) {
+ case MDEVCTL_CMD_CREATE:
+ /* now is the time to make sure "create" is replaced with "start" on
+ * mdevctl cmdline */
+ cmd = virCommandNewArgList(MDEVCTL, "start", NULL);
+ break;
+ case MDEVCTL_CMD_STOP:
+ case MDEVCTL_CMD_START:
+ case MDEVCTL_CMD_DEFINE:
+ case MDEVCTL_CMD_UNDEFINE:
+ cmd = virCommandNewArgList(MDEVCTL, subcommand, NULL);
+ break;
+ case MDEVCTL_CMD_LAST:
+ default:
+ /* SHOULD NEVER HAPPEN */
Shouldn't happen, but allows code to fall thru w/ later access to @cmd
which will fail. It's a false positive technically...
+ break;
}
- if (nodeDeviceDefToMdevctlConfig(def, &json) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("couldn't convert node device def to mdevctl JSON"));
- return NULL;
- }
+ switch (cmd_type) {
+ case MDEVCTL_CMD_CREATE:
+ case MDEVCTL_CMD_DEFINE:
+ parent_addr = nodeDeviceFindAddressByName(def->parent);
- cmd = virCommandNewArgList(MDEVCTL, subcommand, NULL);
- virCommandAddArgPair(cmd, "--parent", parent_addr);
- virCommandAddArgPair(cmd, "--jsonfile", "/dev/stdin");
+ if (!parent_addr) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unable to find parent device '%s'"), def->parent);
Leaks @cmd
+ return NULL;
+ }
+
+ if (nodeDeviceDefToMdevctlConfig(def, &inbuf) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("couldn't convert node device def to mdevctl JSON"));
Leaks @cmd
+ return NULL;
+ }
- virCommandSetInputBuffer(cmd, json);
+ virCommandAddArgPair(cmd, "--parent", parent_addr);
+ virCommandAddArgPair(cmd, "--jsonfile", "/dev/stdin");
+
+ virCommandSetInputBuffer(cmd, inbuf);
+ virCommandSetOutputBuffer(cmd, outbuf);
+ break;
+
+ case MDEVCTL_CMD_UNDEFINE:
+ case MDEVCTL_CMD_STOP:
+ case MDEVCTL_CMD_START:
+ /* No special handling here, we only need to pass UUID with these */
+ break;
+ case MDEVCTL_CMD_LAST:
+ default:
+ /* SHOULD NEVER HAPPEN */
+ break;
+ }
[...]
Hopefully I haven't missed some other patch along the way as I don't
keep up to date as much any more. Coverity just keeps running though...
John
