From 582f27ff15f14fa0e913278c0a2ca9840c3f12f5 Mon Sep 17 00:00:00 2001 From: Boris Fiuczynski Date: Thu, 22 Feb 2024 14:02:07 +0100 Subject: [PATCH] nodedev: Implement virNodeDeviceUpdate Implement the API functions in the node device driver by using mdevctl modify with the options defined and live. Instead of increasing the minimum mdevctl version to 1.3.0 in the spec file to ensure support exists in mdevctl the support is dynamically checked before using mdevctl. Signed-off-by: Boris Fiuczynski Signed-off-by: Michal Privoznik Reviewed-by: Jonathon Jongsma Reviewed-by: Michal Privoznik --- NEWS.rst | 7 + libvirt.spec.in | 1 + src/node_device/node_device_driver.c | 228 +++++++++++++++++- src/node_device/node_device_driver.h | 11 + src/node_device/node_device_udev.c | 1 + ...60c_c60c_c60c_c60c_c60cc60cc60c_update.xml | 16 ++ tests/nodedevmdevctldata/mdevctl-modify.argv | 25 ++ tests/nodedevmdevctldata/mdevctl-modify.json | 4 + tests/nodedevmdevctltest.c | 90 ++++++- 9 files changed, 380 insertions(+), 3 deletions(-) create mode 100644 tests/nodedevmdevctldata/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c_update.xml create mode 100644 tests/nodedevmdevctldata/mdevctl-modify.argv create mode 100644 tests/nodedevmdevctldata/mdevctl-modify.json diff --git a/NEWS.rst b/NEWS.rst index cf9692130a..bb264a4bcc 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -17,6 +17,13 @@ v10.1.0 (unreleased) * **New features** + * nodedev: Support updating mdevs + + The node device driver has been extended to allow updating mediated node + devices. Options are available to target the update against the persistent, + active or both configurations of a mediated device. + **Note:** The support is only available with at least mdevctl v1.3.0 installed. + * qemu: Add support for /dev/userfaultfd On hosts with new enough kernel which supports /dev/userfaultfd libvirt will diff --git a/libvirt.spec.in b/libvirt.spec.in index a1386590e7..3358589413 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -620,6 +620,7 @@ Requires: libvirt-libs = %{version}-%{release} # needed for device enumeration Requires: systemd >= 185 # For managing persistent mediated devices +# Note: for nodedev-update support at least mdevctl v1.3.0 is required Requires: mdevctl # 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 37465da3a6..d39438b339 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,102 @@ 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; +} + + +/* checks if mdevctl supports on the command modify the options live, defined + * and jsonfile + */ +static int +nodeDeviceGetMdevctlModifySupportCheck(void) +{ + int status; + g_autoptr(virCommand) cmd = NULL; + const char *subcommand = virMdevctlCommandTypeToString(MDEVCTL_CMD_MODIFY); + + cmd = virCommandNewArgList(MDEVCTL, + subcommand, + "--defined", + "--live", + "--jsonfile", + "b", + "--help", + NULL); + + if (!cmd) + return -1; + + if (virCommandRun(cmd, &status) < 0) + return -1; + + if (status != 0) { + /* update is unsupported */ + return -1; + } + + return 0; +} + + +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 (nodeDeviceGetMdevctlModifySupportCheck() < 0) { + VIR_WARN("Installed mdevctl version does not support modify with options jsonfile, defined and live"); + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Unable to modify mediated device: modify unsupported")); + 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) @@ -2107,3 +2206,128 @@ nodeDeviceIsActive(virNodeDevice *device) virNodeDeviceObjEndAPI(&obj); return ret; } + + +static int +nodeDeviceDefValidateUpdate(virNodeDeviceDef *def, + virNodeDeviceDef *new_def, + bool live) +{ + virNodeDevCapsDef *caps = NULL; + virNodeDevCapMdev *cap_mdev = NULL; + virNodeDevCapMdev *cap_new_mdev = NULL; + + for (caps = def->caps; caps != NULL; caps = caps->next) { + if (caps->data.type == VIR_NODE_DEV_CAP_MDEV) { + cap_mdev = &caps->data.mdev; + } + } + if (!cap_mdev) + return -1; + + for (caps = new_def->caps; caps != NULL; caps = caps->next) { + if (caps->data.type == VIR_NODE_DEV_CAP_MDEV) { + cap_new_mdev = &caps->data.mdev; + } + } + if (!cap_new_mdev) + return -1; + + if (STRNEQ_NULLABLE(cap_mdev->uuid, cap_new_mdev->uuid)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Cannot update device '%1$s, uuid mismatch (current uuid '%2$s')"), + def->name, + cap_mdev->uuid); + return -1; + } + + /* A live update cannot change the mdev type. Since the new config is + * stored in defined_config, compare that to the mdev type of the current + * live config to make sure they match */ + if (live && + STRNEQ_NULLABLE(cap_mdev->active_config.type, cap_new_mdev->defined_config.type)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Cannot update device '%1$s', type mismatch (current type '%2$s')"), + def->name, + cap_mdev->active_config.type); + return -1; + } + if (STRNEQ_NULLABLE(cap_mdev->parent_addr, cap_new_mdev->parent_addr)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Cannot update device '%1$s', parent address mismatch (current parent address '%2$s')"), + def->name, + cap_mdev->parent_addr); + return -1; + } + + return 0; +} + + +int +nodeDeviceUpdate(virNodeDevice *device, + const char *xmlDesc, + unsigned int flags) +{ + int ret = -1; + virNodeDeviceObj *obj = NULL; + virNodeDeviceDef *def; + g_autoptr(virNodeDeviceDef) new_def = NULL; + const char *virt_type = NULL; + bool updated = false; + + virCheckFlags(VIR_NODE_DEVICE_UPDATE_AFFECT_LIVE | + VIR_NODE_DEVICE_UPDATE_AFFECT_CONFIG, -1); + + if (nodeDeviceInitWait() < 0) + return -1; + + if (!(obj = nodeDeviceObjFindByName(device->name))) + return -1; + def = virNodeDeviceObjGetDef(obj); + + virt_type = virConnectGetType(device->conn); + + if (virNodeDeviceUpdateEnsureACL(device->conn, def, flags) < 0) + goto cleanup; + + if (!(new_def = virNodeDeviceDefParse(xmlDesc, NULL, EXISTING_DEVICE, virt_type, + &driver->parserCallbacks, NULL, true))) + goto cleanup; + + if (nodeDeviceHasCapability(def, VIR_NODE_DEV_CAP_MDEV) && + nodeDeviceHasCapability(new_def, 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 and new_def for compatibility e.g. parent, type + * and uuid. */ + if (nodeDeviceDefValidateUpdate(def, new_def, + (flags & VIR_NODE_DEVICE_UPDATE_AFFECT_LIVE)) < 0) + goto cleanup; + + /* Update now. */ + VIR_DEBUG("Update node device '%s' with mdevctl", def->name); + if (virMdevctlModify(new_def, + (flags & VIR_NODE_DEVICE_UPDATE_AFFECT_CONFIG), + (flags & VIR_NODE_DEVICE_UPDATE_AFFECT_LIVE)) < 0) { + goto cleanup; + }; + /* Detect updates and also trigger events. */ + updated = true; + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Unsupported device type")); + goto cleanup; + } + + 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..b3bc4b2e96 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 +nodeDeviceUpdate(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..f1e402f8f7 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 */ + .nodeDeviceUpdate = nodeDeviceUpdate, /* 10.1.0 */ }; diff --git a/tests/nodedevmdevctldata/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c_update.xml b/tests/nodedevmdevctldata/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c_update.xml new file mode 100644 index 0000000000..17e3611bf4 --- /dev/null +++ b/tests/nodedevmdevctldata/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c_update.xml @@ -0,0 +1,16 @@ + + mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c_0_0_0052 + /sys/devices/css0/0.0.0052/c60cc60c-c60c-c60c-c60c-c60cc60cc60c + css_0_0_0052 + + vfio_ccw_mdev + + + + c60cc60c-c60c-c60c-c60c-c60cc60cc60c + 0.0.0052 + + + + + diff --git a/tests/nodedevmdevctldata/mdevctl-modify.argv b/tests/nodedevmdevctldata/mdevctl-modify.argv new file mode 100644 index 0000000000..341c5e7fd0 --- /dev/null +++ b/tests/nodedevmdevctldata/mdevctl-modify.argv @@ -0,0 +1,25 @@ +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 \ +--live +mdevctl \ +modify \ +--parent=0.0.0052 \ +--jsonfile=/dev/stdin \ +--uuid=c60cc60c-c60c-c60c-c60c-c60cc60cc60c \ +--defined \ +--live diff --git a/tests/nodedevmdevctldata/mdevctl-modify.json b/tests/nodedevmdevctldata/mdevctl-modify.json new file mode 100644 index 0000000000..a778730fac --- /dev/null +++ b/tests/nodedevmdevctldata/mdevctl-modify.json @@ -0,0 +1,4 @@ +{"mdev_type":"vfio_ccw-io","start":"manual"} +{"mdev_type":"vfio_ccw-io","start":"manual","attrs":[{"add_attr_1":"val1"},{"add_attr_2":"val2"}]} +{"mdev_type":"vfio_ccw-io","start":"manual"} +{"mdev_type":"vfio_ccw-io","start":"manual","attrs":[{"add_attr_1":"val1"},{"add_attr_2":"val2"}]} diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c index f49d668461..de688c982e 100644 --- a/tests/nodedevmdevctltest.c +++ b/tests/nodedevmdevctltest.c @@ -33,7 +33,10 @@ testCommandDryRunCallback(const char *const*args G_GNUC_UNUSED, { char **stdinbuf = opaque; - *stdinbuf = g_strdup(input); + if (*stdinbuf) + *stdinbuf = g_strconcat(*stdinbuf, "\n", input, NULL); + else + *stdinbuf = g_strdup(input); } typedef virCommand * (*MdevctlCmdFunc)(virNodeDeviceDef *, char **, char **); @@ -63,6 +66,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 +177,85 @@ testMdevctlAutostart(const void *data G_GNUC_UNUSED) return ret; } + +static int +testMdevctlModify(const void *data G_GNUC_UNUSED) +{ + g_autoptr(virNodeDeviceDef) def = NULL; + g_autoptr(virNodeDeviceDef) def_update = 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; + g_autofree char *stdinbuf = NULL; + g_autofree char *mdevxml = + g_strdup_printf("%s/nodedevschemadata/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c.xml", + abs_srcdir); + g_autofree char *mdevxml_update = + g_strdup_printf("%s/nodedevmdevctldata/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c_update.xml", + abs_srcdir); + /* just concatenate both calls into the same output files */ + g_autofree char *cmdlinefile = + g_strdup_printf("%s/nodedevmdevctldata/mdevctl-modify.argv", + abs_srcdir); + g_autofree char *jsonfile = + g_strdup_printf("%s/nodedevmdevctldata/mdevctl-modify.json", + 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, testCommandDryRunCallback, &stdinbuf); + + if (!(definedcmd = nodeDeviceGetMdevctlModifyCommand(def, true, false, &errmsg))) + goto cleanup; + + if (virCommandRun(definedcmd, NULL) < 0) + goto cleanup; + + if (!(def_update = virNodeDeviceDefParse(NULL, mdevxml_update, EXISTING_DEVICE, VIRT_TYPE, + &parser_callbacks, NULL, false))) + goto cleanup; + + if (!(livecmd = nodeDeviceGetMdevctlModifyCommand(def_update, false, true, &errmsg))) + goto cleanup; + + if (virCommandRun(livecmd, NULL) < 0) + goto cleanup; + + if (!(livecmd = nodeDeviceGetMdevctlModifyCommand(def, false, true, &errmsg))) + goto cleanup; + + if (virCommandRun(livecmd, NULL) < 0) + goto cleanup; + + if (!(bothcmd = nodeDeviceGetMdevctlModifyCommand(def_update, 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; + + if (virTestCompareToFile(stdinbuf, jsonfile) < 0) + goto cleanup; + + ret = 0; + + cleanup: + virBufferFreeAndReset(&buf); + return ret; +} + static int testMdevctlListDefined(const void *data G_GNUC_UNUSED) { @@ -457,6 +540,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 +571,8 @@ mymain(void) DO_TEST_AUTOSTART(); + DO_TEST_MODIFY(); + done: nodedevTestDriverFree(driver);