From d36ce2f2528b4ac1113691ab331870794254fd93 Mon Sep 17 00:00:00 2001
From: Boris Fiuczynski <fiuczy@linux.ibm.com>
Date: Thu, 22 Feb 2024 14:02:09 +0100
Subject: [PATCH] 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@linux.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
---
 NEWS.rst                             |  5 +++
 src/libvirt-nodedev.c                |  4 ++-
 src/node_device/node_device_driver.c | 48 ++++++++++++++++++++++++----
 tools/virsh-nodedev.c                |  8 ++---
 4 files changed, 54 insertions(+), 11 deletions(-)

diff --git a/NEWS.rst b/NEWS.rst
index bb264a4bcc..def5a5edd0 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -45,6 +45,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 d39438b339..ce42b1ca63 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -1547,16 +1547,39 @@ nodeDeviceUpdateMediatedDevice(virNodeDeviceDef *def,
 }
 
 
+static virNodeDeviceObj*
+findPersistentMdevNodeDevice(virNodeDeviceDef *def)
+{
+    virNodeDeviceObj *obj = NULL;
+
+    if (!nodeDeviceHasCapability(def, VIR_NODE_DEV_CAP_MDEV))
+        return NULL;
+
+    if (def->caps->data.mdev.uuid &&
+        def->caps->data.mdev.parent_addr &&
+        (obj = virNodeDeviceObjListFindMediatedDeviceByUUID(driver->devs,
+                                                            def->caps->data.mdev.uuid,
+                                                            def->caps->data.mdev.parent_addr)) &&
+        !virNodeDeviceObjIsPersistent(obj)) {
+        virNodeDeviceObjEndAPI(&obj);
+    }
+
+    return obj;
+}
+
+
 virNodeDevice*
 nodeDeviceDefineXML(virConnect *conn,
                     const char *xmlDesc,
                     unsigned int flags)
 {
     g_autoptr(virNodeDeviceDef) def = NULL;
+    virNodeDeviceObj *persistent_obj = NULL;
     const char *virt_type = NULL;
     g_autofree char *uuid = NULL;
     g_autofree char *name = NULL;
     bool validate = flags & VIR_NODE_DEVICE_DEFINE_XML_VALIDATE;
+    bool modify_failed = false;
 
     virCheckFlags(VIR_NODE_DEVICE_DEFINE_XML_VALIDATE, NULL);
 
@@ -1584,13 +1607,26 @@ nodeDeviceDefineXML(virConnect *conn,
         return NULL;
     }
 
-    if (virMdevctlDefine(def, &uuid) < 0) {
-        return NULL;
-    }
+    if ((persistent_obj = findPersistentMdevNodeDevice(def))) {
+        /* virNodeDeviceObjUpdateModificationImpact() is not required we
+         * will modify the persistent config only.
+         * nodeDeviceDefValidateUpdate() is not required as uuid and
+         * parent are matching if def was found and changing the type in
+         * the persistent config is allowed. */
+        VIR_DEBUG("Update node device '%s' with mdevctl", def->name);
+        modify_failed = (virMdevctlModify(def, true, false) < 0);
+        virNodeDeviceObjEndAPI(&persistent_obj);
+        if (modify_failed)
+            return NULL;
+    } else {
+        VIR_DEBUG("Define node device '%s' with mdevctl", def->name);
+        if (virMdevctlDefine(def, &uuid) < 0)
+            return NULL;
 
-    if (uuid && uuid[0]) {
-        g_free(def->caps->data.mdev.uuid);
-        def->caps->data.mdev.uuid = g_steal_pointer(&uuid);
+        if (uuid && uuid[0]) {
+            g_free(def->caps->data.mdev.uuid);
+            def->caps->data.mdev.uuid = g_steal_pointer(&uuid);
+        }
     }
 
     mdevGenerateDeviceName(def);
diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
index 3f7896dd9a..c08de65a96 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}
 };