On 1/19/24 10:38 AM, Boris Fiuczynski wrote:
Implement the API functions in the node device driver by using
mdevctl modify with the options defined and live.
Increase the minimum mdevctl version to 1.3.0 in spec file to ensure
support exists in mdevctl.
mdevctl 1.3.0 was only released about 3 weeks ago, so I don't think we
can add an unconditional dependency on it. I think we'll have to make
sure that we can degrade gracefully when the required version isn't
available.
Signed-off-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxx>
---
libvirt.spec.in | 2 +-
src/node_device/node_device_driver.c | 181 ++++++++++++++++++-
src/node_device/node_device_driver.h | 11 ++
src/node_device/node_device_udev.c | 1 +
tests/nodedevmdevctldata/mdevctl-modify.argv | 19 ++
tests/nodedevmdevctltest.c | 64 +++++++
6 files changed, 275 insertions(+), 3 deletions(-)
create mode 100644 tests/nodedevmdevctldata/mdevctl-modify.argv
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 8413e3c19a..3ed14fa63c 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -613,7 +613,7 @@ Requires: libvirt-libs = %{version}-%{release}
# needed for device enumeration
Requires: systemd >= 185
# For managing persistent mediated devices
-Requires: mdevctl
+Requires: mdevctl >= 1.3.0
# for modprobe of pci devices
Requires: module-init-tools
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index d67c95d68d..dd57e9ca5b 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -54,7 +54,7 @@ virNodeDeviceDriverState *driver;
VIR_ENUM_IMPL(virMdevctlCommand,
MDEVCTL_CMD_LAST,
- "start", "stop", "define", "undefine", "create"
+ "start", "stop", "define", "undefine", "create", "modify"
);
@@ -754,6 +754,7 @@ nodeDeviceGetMdevctlCommand(virNodeDeviceDef *def,
case MDEVCTL_CMD_START:
case MDEVCTL_CMD_DEFINE:
case MDEVCTL_CMD_UNDEFINE:
+ case MDEVCTL_CMD_MODIFY:
cmd = virCommandNewArgList(MDEVCTL, subcommand, NULL);
break;
case MDEVCTL_CMD_LAST:
@@ -767,6 +768,7 @@ nodeDeviceGetMdevctlCommand(virNodeDeviceDef *def,
switch (cmd_type) {
case MDEVCTL_CMD_CREATE:
case MDEVCTL_CMD_DEFINE:
+ case MDEVCTL_CMD_MODIFY:
if (!def->caps->data.mdev.parent_addr) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unable to find parent device '%1$s'"), def->parent);
@@ -783,7 +785,8 @@ nodeDeviceGetMdevctlCommand(virNodeDeviceDef *def,
virCommandAddArgPair(cmd, "--jsonfile", "/dev/stdin");
virCommandSetInputBuffer(cmd, inbuf);
- virCommandSetOutputBuffer(cmd, outbuf);
+ if (outbuf)
+ virCommandSetOutputBuffer(cmd, outbuf);
break;
case MDEVCTL_CMD_UNDEFINE:
@@ -868,6 +871,61 @@ virMdevctlDefine(virNodeDeviceDef *def, char **uuid)
}
+/* gets a virCommand object that executes a mdevctl command to modify a
+ * a device to an updated version
+ */
+virCommand*
+nodeDeviceGetMdevctlModifyCommand(virNodeDeviceDef *def,
+ bool defined,
+ bool live,
+ char **errmsg)
+{
+ virCommand *cmd = nodeDeviceGetMdevctlCommand(def,
+ MDEVCTL_CMD_MODIFY,
+ NULL, errmsg);
+
+ if (!cmd)
+ return NULL;
+
+ if (defined)
+ virCommandAddArg(cmd, "--defined");
+
+ if (live)
+ virCommandAddArg(cmd, "--live");
+
+ return cmd;
+}
+
+
+static int
+virMdevctlModify(virNodeDeviceDef *def,
+ bool defined,
+ bool live)
+{
+ int status;
+ g_autofree char *errmsg = NULL;
+ g_autoptr(virCommand) cmd = nodeDeviceGetMdevctlModifyCommand(def,
+ defined,
+ live,
+ &errmsg);
+
+ if (!cmd)
+ return -1;
+
+ if (virCommandRun(cmd, &status) < 0)
+ return -1;
+
+ if (status != 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unable to modify mediated device: %1$s"),
+ MDEVCTL_ERROR(errmsg));
+ return -1;
+ }
+
+ return 0;
+}
+
+
static virNodeDevicePtr
nodeDeviceCreateXMLMdev(virConnectPtr conn,
virNodeDeviceDef *def)
@@ -2108,3 +2166,122 @@ nodeDeviceIsActive(virNodeDevice *device)
virNodeDeviceObjEndAPI(&obj);
return ret;
}
+
+
+static int
+nodeDeviceDefCompareMdevs(virNodeDeviceDef *def_cur,
+ virNodeDeviceDef *def_upd,
+ bool live)
+{
+ virNodeDevCapsDef *caps = NULL;
+ virNodeDevCapMdev *cap_mdev_cur = NULL;
+ virNodeDevCapMdev *cap_mdev_upd = NULL;
+
+ for (caps = def_cur->caps; caps != NULL; caps = caps->next) {
+ if (caps->data.type == VIR_NODE_DEV_CAP_MDEV) {
+ cap_mdev_cur = &caps->data.mdev;
+ }
+ }
+ if (!cap_mdev_cur)
+ return -1;
+
+ for (caps = def_upd->caps; caps != NULL; caps = caps->next) {
+ if (caps->data.type == VIR_NODE_DEV_CAP_MDEV) {
+ cap_mdev_upd = &caps->data.mdev;
+ }
+ }
+ if (!cap_mdev_upd)
+ return -1;
+
+ if (STRNEQ_NULLABLE(cap_mdev_cur->uuid, cap_mdev_upd->uuid)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("uuid mismatch (mdev:'%1$s' update:'%2$s')"),
I'd suggest a more complete description of the error so the context is a
bit more clear when scanning the log. Something like:
"Cannot update device $NAME, uuid mismatch". I'm not sure if we need to
log the actual values.
+ cap_mdev_cur->uuid,
+ cap_mdev_upd->uuid);
+ return -1;
+ }
+ // for a live update the types of the active configs must match!
+ if (live &&
+ STRNEQ_NULLABLE(cap_mdev_cur->active_config.type, cap_mdev_upd->active_config.type)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("type mismatch (mdev:'%1$s' update:'%2$s')"),
same
+ cap_mdev_cur->active_config.type,
+ cap_mdev_upd->active_config.type);
+ return -1;
+ }
+ if (STRNEQ_NULLABLE(cap_mdev_cur->parent_addr, cap_mdev_upd->parent_addr)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("parent address mismatch (mdev:'%1$s' update:'%2$s')"),
same
+ cap_mdev_cur->parent_addr,
+ cap_mdev_upd->parent_addr);
+ return -1;
+ }
+
+ return 0;
+}
+
+
+int
+nodeDeviceUpdateXML(virNodeDevice *device,
+ const char *xmlDesc,
+ unsigned int flags)
+{
+ int ret = -1;
+ virNodeDeviceObj *obj = NULL;
+ virNodeDeviceDef *def_cur;
+ g_autoptr(virNodeDeviceDef) def_upd = NULL;
+ const char *virt_type = NULL;
+ bool updated = false;
+
+ virCheckFlags(VIR_NODE_DEVICE_UPDATE_XML_AFFECT_LIVE |
+ VIR_NODE_DEVICE_UPDATE_XML_AFFECT_CONFIG, -1);
+
+ if (nodeDeviceInitWait() < 0)
+ return -1;
+
+ if (!(obj = nodeDeviceObjFindByName(device->name)))
+ return -1;
+ def_cur = virNodeDeviceObjGetDef(obj);
+
+ virt_type = virConnectGetType(device->conn);
+
+ if (virNodeDeviceUpdateXMLEnsureACL(device->conn, def_cur, flags) < 0)
+ goto cleanup;
+
+ if (!(def_upd = virNodeDeviceDefParse(xmlDesc, NULL, EXISTING_DEVICE, virt_type,
+ &driver->parserCallbacks, NULL, true)))
+ goto cleanup;
+
+ if (nodeDeviceHasCapability(def_cur, VIR_NODE_DEV_CAP_MDEV) &&
+ nodeDeviceHasCapability(def_upd, VIR_NODE_DEV_CAP_MDEV)) {
+ // Checks flags are valid for the state and sets flags for current if flags not set
+ if (virNodeDeviceObjUpdateModificationImpact(obj, &flags) < 0)
+ goto cleanup;
+
+ // Compare def_cur and def_upd for compatibleness e.g. parent, type and uuid
+ if (nodeDeviceDefCompareMdevs(def_cur, def_upd,
+ (flags & VIR_NODE_DEVICE_UPDATE_XML_AFFECT_LIVE)) < 0)
+ goto cleanup;
+
+ // Update now
+ VIR_DEBUG("Update node device '%s' with mdevctl", def_cur->name);
+ if (virMdevctlModify(def_upd,
+ (flags & VIR_NODE_DEVICE_UPDATE_XML_AFFECT_CONFIG),
+ (flags & VIR_NODE_DEVICE_UPDATE_XML_AFFECT_LIVE)) < 0) {
+ goto cleanup;
+ };
+ // Detect updates and also trigger events
+ updated = true;
+ } else {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Unsupported device type"));
you need to goto cleanup here otherwise the return value will be set to 0.
+ }
+
+ ret = 0;
+ cleanup:
+ virNodeDeviceObjEndAPI(&obj);
+ if (updated)
+ nodeDeviceUpdateMediatedDevices();
+
+ return ret;
+}
diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h
index 4dce7e6f17..5751625eda 100644
--- a/src/node_device/node_device_driver.h
+++ b/src/node_device/node_device_driver.h
@@ -39,6 +39,7 @@ typedef enum {
* separation makes our code more readable in terms of knowing when we're
* starting a defined device and when we're creating a transient one */
MDEVCTL_CMD_CREATE,
+ MDEVCTL_CMD_MODIFY,
MDEVCTL_CMD_LAST,
} virMdevctlCommand;
@@ -186,3 +187,13 @@ virCommand*
nodeDeviceGetMdevctlSetAutostartCommand(virNodeDeviceDef *def,
bool autostart,
char **errmsg);
+
+virCommand*
+nodeDeviceGetMdevctlModifyCommand(virNodeDeviceDef *def,
+ bool defined,
+ bool live,
+ char **errmsg);
+int
+nodeDeviceUpdateXML(virNodeDevice *dev,
+ const char *xmlDesc,
+ unsigned int flags);
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 57368a96c3..4cd9d285fa 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -2403,6 +2403,7 @@ static virNodeDeviceDriver udevNodeDeviceDriver = {
.nodeDeviceGetAutostart = nodeDeviceGetAutostart, /* 7.8.0 */
.nodeDeviceIsPersistent = nodeDeviceIsPersistent, /* 7.8.0 */
.nodeDeviceIsActive = nodeDeviceIsActive, /* 7.8.0 */
+ .nodeDeviceUpdateXML = nodeDeviceUpdateXML, /* 10.1.0 */
};
diff --git a/tests/nodedevmdevctldata/mdevctl-modify.argv b/tests/nodedevmdevctldata/mdevctl-modify.argv
new file mode 100644
index 0000000000..6acb9ca0bf
--- /dev/null
+++ b/tests/nodedevmdevctldata/mdevctl-modify.argv
@@ -0,0 +1,19 @@
+mdevctl \
+modify \
+--parent=0.0.0052 \
+--jsonfile=/dev/stdin \
+--uuid=c60cc60c-c60c-c60c-c60c-c60cc60cc60c \
+--defined
+mdevctl \
+modify \
+--parent=0.0.0052 \
+--jsonfile=/dev/stdin \
+--uuid=c60cc60c-c60c-c60c-c60c-c60cc60cc60c \
+--live
+mdevctl \
+modify \
+--parent=0.0.0052 \
+--jsonfile=/dev/stdin \
+--uuid=c60cc60c-c60c-c60c-c60c-c60cc60cc60c \
+--defined \
+--live
diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c
index 852d9ed6e7..37978c9a5c 100644
--- a/tests/nodedevmdevctltest.c
+++ b/tests/nodedevmdevctltest.c
@@ -63,6 +63,7 @@ testMdevctlCmd(virMdevctlCommand cmd_type,
case MDEVCTL_CMD_START:
case MDEVCTL_CMD_STOP:
case MDEVCTL_CMD_UNDEFINE:
+ case MDEVCTL_CMD_MODIFY:
create = EXISTING_DEVICE;
break;
case MDEVCTL_CMD_LAST:
@@ -173,6 +174,64 @@ testMdevctlAutostart(const void *data G_GNUC_UNUSED)
return ret;
}
+
+static int
+testMdevctlModify(const void *data G_GNUC_UNUSED)
+{
+ g_autoptr(virNodeDeviceDef) def = NULL;
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+ const char *actualCmdline = NULL;
+ int ret = -1;
+ g_autoptr(virCommand) definedcmd = NULL;
+ g_autoptr(virCommand) livecmd = NULL;
+ g_autoptr(virCommand) bothcmd = NULL;
+ g_autofree char *errmsg = NULL;
+ /* just concatenate both calls into the same output file */
+ g_autofree char *cmdlinefile =
+ g_strdup_printf("%s/nodedevmdevctldata/mdevctl-modify.argv",
+ abs_srcdir);
+ g_autofree char *mdevxml =
+ g_strdup_printf("%s/nodedevschemadata/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c.xml",
+ abs_srcdir);
+ g_autoptr(virCommandDryRunToken) dryRunToken = virCommandDryRunTokenNew();
+
+ if (!(def = virNodeDeviceDefParse(NULL, mdevxml, CREATE_DEVICE, VIRT_TYPE,
+ &parser_callbacks, NULL, false)))
+ return -1;
+
+ virCommandSetDryRun(dryRunToken, &buf, true, true, NULL, NULL);
You should also capture the stdin since that's really the majority of
the command input.
+
+ if (!(definedcmd = nodeDeviceGetMdevctlModifyCommand(def, true, false, &errmsg)))
+ goto cleanup;
+
+ if (virCommandRun(definedcmd, NULL) < 0)
+ goto cleanup;
+
+ if (!(livecmd = nodeDeviceGetMdevctlModifyCommand(def, false, true, &errmsg)))
+ goto cleanup;
+
+ if (virCommandRun(livecmd, NULL) < 0)
+ goto cleanup;
+
+ if (!(bothcmd = nodeDeviceGetMdevctlModifyCommand(def, true, true, &errmsg)))
+ goto cleanup;
+
+ if (virCommandRun(bothcmd, NULL) < 0)
+ goto cleanup;
+
+ if (!(actualCmdline = virBufferCurrentContent(&buf)))
+ goto cleanup;
+
+ if (virTestCompareToFileFull(actualCmdline, cmdlinefile, false) < 0)
+ goto cleanup;
+
+ ret = 0;
+
+ cleanup:
+ virBufferFreeAndReset(&buf);
+ return ret;
+}
+
static int
testMdevctlListDefined(const void *data G_GNUC_UNUSED)
{
@@ -457,6 +516,9 @@ mymain(void)
#define DO_TEST_AUTOSTART() \
DO_TEST_FULL("autostart mdevs", testMdevctlAutostart, NULL)
+#define DO_TEST_MODIFY() \
+ DO_TEST_FULL("modify mdevs", testMdevctlModify, NULL)
+
#define DO_TEST_PARSE_JSON(filename) \
DO_TEST_FULL("parse mdevctl json " filename, testMdevctlParse, filename)
@@ -485,6 +547,8 @@ mymain(void)
DO_TEST_AUTOSTART();
+ DO_TEST_MODIFY();
+
done:
nodedevTestDriverFree(driver);
Reviewed-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
