Re: [PATCH v2 10/11] nodedev: Implement virNodeDeviceUpdateXML

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 spec file to ensure support exists in mdevctl the support is dynamically checked before using mdevctl. Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com&gt; --- NEWS.rst | 7 + libvirt.spec.in | 1 + src/node_device/node_device_driver.c | 256 +++++++++++++++++- 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 | 1 + tests/nodedevmdevctltest.c | 90 +++++- 9 files changed, 405 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 e2796fd8b2..aa2fee3533 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 persisted, + active or both configurations of an mediated device. + **Note:** The support is only available with at least mdevctl v1.3.0 installed. + * qemu: Support clusters in CPU topology It is now possible to configure the guest CPU topology to use clusters. diff --git a/libvirt.spec.in b/libvirt.spec.in index 8413e3c19a..a8dff4a32e 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -613,6 +613,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 baff49a0ae..5bece1a30a 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,100 @@ 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) @@ -2106,3 +2203,158 @@ nodeDeviceIsActive(virNodeDevice *device) virNodeDeviceObjEndAPI(&obj); return ret; } + + +static int +nodeDeviceDefCloneMdevConfigs(virNodeDeviceDef *def_upd) +{ + virNodeDevCapsDef *caps = NULL; + virNodeDevCapMdev *cap_mdev_upd = NULL; + virMediatedDeviceConfig *src_mdev_config = NULL; + virMediatedDeviceConfig *dst_mdev_config = NULL; + + 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; + + src_mdev_config = &cap_mdev_upd->defined_config; + dst_mdev_config = &cap_mdev_upd->active_config; + + if (STRNEQ_NULLABLE(dst_mdev_config->type, src_mdev_config->type)) { + g_free(dst_mdev_config->type); + dst_mdev_config->type = g_strdup(src_mdev_config->type); + } + + virMediatedDeviceAttrsCopy(dst_mdev_config, src_mdev_config); + + return 0; +} + + +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, + _("Cannot update device '%1$s, uuid mismatch (current uuid '%2$s')"), + def_cur->name, + cap_mdev_cur->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, + _("Cannot update device '%1$s', type mismatch (current type '%2$s')"), + def_cur->name, + cap_mdev_cur->active_config.type); + return -1; + } + if (STRNEQ_NULLABLE(cap_mdev_cur->parent_addr, cap_mdev_upd->parent_addr)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Cannot update device '%1$s', parent address mismatch (current parent address '%2$s')"), + def_cur->name, + cap_mdev_cur->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; + + // Before live update clone defined to active config + if (flags & VIR_NODE_DEVICE_UPDATE_XML_AFFECT_LIVE && + nodeDeviceDefCloneMdevConfigs(def_upd) < 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")); + 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..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/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 @@ +<device> + <name>mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c_0_0_0052</name> + <path>/sys/devices/css0/0.0.0052/c60cc60c-c60c-c60c-c60c-c60cc60cc60c</path> + <parent>css_0_0_0052</parent> + <driver> + <name>vfio_ccw_mdev</name> + </driver> + <capability type='mdev'> + <type id='vfio_ccw-io'/> + <uuid>c60cc60c-c60c-c60c-c60c-c60cc60cc60c</uuid> + <parent_addr>0.0.0052</parent_addr> + <attr name='add_attr_1' value='val1'/> + <attr name='add_attr_2' value='val2'/> + <iommuGroup number='4'/> + </capability> +</device> 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..7ae8c73ce3 --- /dev/null +++ b/tests/nodedevmdevctldata/mdevctl-modify.json @@ -0,0 +1 @@ +{"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..11273e3049 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, 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);