Re: [PATCH v3 12/12] nodedev: allow modify on define of a persistent node device

Allow to modify a node device by using virNodeDeviceDefineXML() to align its behavior with other drivers define methods. Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com&gt; --- NEWS.rst | 5 +++ src/libvirt-nodedev.c | 4 +- src/node_device/node_device_driver.c | 64 ++++++++++++++++++++++------ tools/virsh-nodedev.c | 8 ++-- 4 files changed, 64 insertions(+), 17 deletions(-) diff --git a/NEWS.rst b/NEWS.rst index aa2fee3533..9b8b94dea4 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -32,6 +32,11 @@ v10.1.0 (unreleased) * **Improvements** +* nodedev: Add ability to update persistent mediated devices by defining them + + Existing persistent mediated devices can now also be updated by + ``virNodeDeviceDefineXML()`` as long as parent and UUID remain unchanged. + * **Bug fixes** * qemu_process: Skip over non-virtio non-TAP NIC models when refreshing rx-filter diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c index b97c199a3a..f242c0a8f6 100644 --- a/src/libvirt-nodedev.c +++ b/src/libvirt-nodedev.c @@ -780,7 +780,9 @@ virNodeDeviceDestroy(virNodeDevicePtr dev) * @xmlDesc: string containing an XML description of the device to be defined * @flags: bitwise-OR of supported virNodeDeviceDefineXMLFlags * - * Define a new device on the VM host machine, for example, a mediated device + * Define a new inactive persistent device or modify an existing persistent + * one from the XML description on the VM host machine, for example, a mediated + * device. * * virNodeDeviceFree should be used to free the resources after the * node device object is no longer needed. diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 11bc9af92e..66de46513d 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -1544,12 +1544,33 @@ nodeDeviceUpdateMediatedDevice(virNodeDeviceDef *def, } +static virNodeDeviceObj* +findPersistentMdevNodeDevice(virNodeDeviceDef *def) +{ + virNodeDeviceObj *obj = NULL; + + if (!nodeDeviceHasCapability(def, VIR_NODE_DEV_CAP_MDEV) && !def->parent) + return NULL;

+ + if (def->caps->data.mdev.uuid && def->caps->data.mdev.parent_addr) { > + mdevGenerateDeviceName(def);

+ if ((obj = nodeDeviceObjFindByName(def->name)) && + virNodeDeviceObjIsPersistent(obj)) + return obj; + } + + return NULL; +} + + virNodeDevice* nodeDeviceDefineXML(virConnect *conn, const char *xmlDesc, unsigned int flags) { g_autoptr(virNodeDeviceDef) def = NULL; + virNodeDevicePtr new_nodedev = NULL; + virNodeDeviceObj *persistent_obj = NULL; const char *virt_type = NULL; g_autofree char *uuid = NULL; g_autofree char *name = NULL; @@ -1566,28 +1587,43 @@ nodeDeviceDefineXML(virConnect *conn, &driver->parserCallbacks, NULL, validate))) return NULL; + persistent_obj = findPersistentMdevNodeDevice(def); + if (virNodeDeviceDefineXMLEnsureACL(conn, def) < 0) - return NULL; + goto cleanup; if (!nodeDeviceHasCapability(def, VIR_NODE_DEV_CAP_MDEV)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Unsupported device type")); - return NULL; + goto cleanup; } if (!def->parent) { virReportError(VIR_ERR_XML_ERROR, "%s", _("cannot define a mediated device without a parent")); - return NULL; + goto cleanup; } - if (virMdevctlDefine(def, &uuid) < 0) { - return NULL; - } + if (persistent_obj) { + // virNodeDeviceObjUpdateModificationImpact() is not required we will + // modify the persitent config only.

+ // nodeDeviceDefValidateUpdate() is not required as uuid and parent are + // machting if def was found and changing the type in the persistent

+ // config is allowed. + VIR_DEBUG("Update node device '%s' with mdevctl", def->name); + if (virMdevctlModify(def, true, false) < 0) + goto cleanup; - if (uuid && uuid[0]) { - g_free(def->caps->data.mdev.uuid); - def->caps->data.mdev.uuid = g_steal_pointer(&uuid); + virNodeDeviceObjEndAPI(&persistent_obj);

+ } else { + VIR_DEBUG("Define node device '%s' with mdevctl", def->name); + if (virMdevctlDefine(def, &uuid) < 0) + goto cleanup; + + if (uuid && uuid[0]) { + g_free(def->caps->data.mdev.uuid); + def->caps->data.mdev.uuid = g_steal_pointer(&uuid); + } } mdevGenerateDeviceName(def); @@ -1601,9 +1637,13 @@ nodeDeviceDefineXML(virConnect *conn, * add the provisional device to the list and return it immediately and * avoid this long delay. */ if (nodeDeviceUpdateMediatedDevice(g_steal_pointer(&def), true) < 0) > - return NULL; + goto cleanup; + + new_nodedev = virGetNodeDevice(conn, name); - return virGetNodeDevice(conn, name); + cleanup: + virNodeDeviceObjEndAPI(&persistent_obj); + return new_nodedev; } @@ -2237,7 +2277,7 @@ nodeDeviceDefValidateUpdate(virNodeDeviceDef *def, 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 */ diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index 7b80747819..912975dfed 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -1083,12 +1083,12 @@ cmdNodeDeviceUndefine(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED) */ static const vshCmdInfo info_node_device_define[] = { {.name = "help", - .data = N_("Define a device by an xml file on a node") + .data = N_("Define or modify a device by an xml file on a node") }, {.name = "desc", - .data = N_("Defines a persistent device on the node that can be " - "assigned to a domain. The device must be started before " - "it can be assigned to a domain.") + .data = N_("Defines or modifies a persistent device on the node that " + "can be assigned to a domain. The device must be started " + "before it can be assigned to a domain.") }, {.name = NULL} };