nodedev: add mdevctl devices to node device list

At startup, query devices that are defined by 'mdevctl' and add them to
the node device list.

This adds a complication: we now have two potential sources of
information for a node device:
 - udev for all devices and for activated mediated devices
 - mdevctl for persistent mediated devices

Unfortunately, neither backend returns full information for a mediated
device. For example, if a persistent mediated device in the list (with
information provided from mdevctl) is 'started', that same device will
now be detected by udev. If we simply overwrite the existing device
definition with the new one provided by the udev backend, we will lose
extra information that was provided by mdevctl (e.g. attributes, etc).
To avoid this, make sure to copy the extra information into the new
device definition.

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>

diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index 65b683c4dc5db59a6b5855807b23b63ed278fff8..da3355f5f4a4595121f26f854394032033f8226d 100644 (file)
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -1169,3 +1169,158 @@ nodeDeviceGenerateName(virNodeDeviceDef *def,
             *(def->name + i) = '_';
     }
 }
+
+
+static int
+virMdevctlListDefined(virNodeDeviceDef ***devs, char **errmsg)
+{
+    int status;
+    g_autofree char *output = NULL;
+    g_autoptr(virCommand) cmd = nodeDeviceGetMdevctlListCommand(true, &output, errmsg);
+
+    if (virCommandRun(cmd, &status) < 0 || status != 0) {
+        return -1;
+    }
+
+    if (!output)
+        return -1;
+
+    return nodeDeviceParseMdevctlJSON(output, devs);
+}
+
+
+int
+nodeDeviceUpdateMediatedDevices(void)
+{
+    g_autofree virNodeDeviceDef **defs = NULL;
+    g_autofree char *errmsg = NULL;
+    int ndefs;
+    size_t i;
+
+    if ((ndefs = virMdevctlListDefined(&defs, &errmsg)) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("failed to query mdevs from mdevctl: %s"), errmsg);
+        return -1;
+    }
+
+    for (i = 0; i < ndefs; i++) {
+        virNodeDeviceObj *obj;
+        virObjectEvent *event;
+        g_autoptr(virNodeDeviceDef) def = defs[i];
+        g_autofree char *name = g_strdup(def->name);
+        bool defined = false;
+
+        def->driver = g_strdup("vfio_mdev");
+
+        if (!(obj = virNodeDeviceObjListFindByName(driver->devs, def->name))) {
+            virNodeDeviceDef *d = g_steal_pointer(&def);
+            if (!(obj = virNodeDeviceObjListAssignDef(driver->devs, d))) {
+                virNodeDeviceDefFree(d);
+                return -1;
+            }
+        } else {
+            bool changed;
+            virNodeDeviceDef *olddef = virNodeDeviceObjGetDef(obj);
+
+            defined = virNodeDeviceObjIsPersistent(obj);
+            /* Active devices contain some additional information (e.g. sysfs
+             * path) that is not provided by mdevctl, so re-use the existing
+             * definition and copy over new mdev data */
+            changed = nodeDeviceDefCopyFromMdevctl(olddef, def);
+
+            if (defined && !changed) {
+                /* if this device was already defined and the definition
+                 * hasn't changed, there's nothing to do for this device */
+                virNodeDeviceObjEndAPI(&obj);
+                continue;
+            }
+        }
+
+        /* all devices returned by virMdevctlListDefined() are persistent */
+        virNodeDeviceObjSetPersistent(obj, true);
+
+        if (!defined)
+            event = virNodeDeviceEventLifecycleNew(name,
+                                                   VIR_NODE_DEVICE_EVENT_DEFINED,
+                                                   0);
+        else
+            event = virNodeDeviceEventUpdateNew(name);
+
+        virNodeDeviceObjEndAPI(&obj);
+        virObjectEventStateQueue(driver->nodeDeviceEventState, event);
+    }
+
+    return 0;
+}
+
+
+/* returns true if any attributes were copied, else returns false */
+static bool
+virMediatedDeviceAttrsCopy(virNodeDevCapMdev *dst,
+                           virNodeDevCapMdev *src)
+{
+    bool ret = false;
+    size_t i;
+
+    if (src->nattributes != dst->nattributes) {
+        ret = true;
+        for (i = 0; i < dst->nattributes; i++)
+            virMediatedDeviceAttrFree(dst->attributes[i]);
+        g_free(dst->attributes);
+
+        dst->nattributes = src->nattributes;
+        dst->attributes = g_new0(virMediatedDeviceAttr*,
+                                 src->nattributes);
+        for (i = 0; i < dst->nattributes; i++)
+            dst->attributes[i] = virMediatedDeviceAttrNew();
+    }
+
+    for (i = 0; i < src->nattributes; i++) {
+        if (STRNEQ_NULLABLE(src->attributes[i]->name,
+                            dst->attributes[i]->name)) {
+            ret = true;
+            g_free(dst->attributes[i]->name);
+            dst->attributes[i]->name =
+                g_strdup(src->attributes[i]->name);
+        }
+        if (STRNEQ_NULLABLE(src->attributes[i]->value,
+                            dst->attributes[i]->value)) {
+            ret = true;
+            g_free(dst->attributes[i]->value);
+            dst->attributes[i]->value =
+                g_strdup(src->attributes[i]->value);
+        }
+    }
+
+    return ret;
+}
+
+
+/* A mediated device definitions from mdevctl contains additional info that is
+ * not available from udev. Transfer this data to the new definition.
+ * Returns true if anything was copied, else returns false */
+bool
+nodeDeviceDefCopyFromMdevctl(virNodeDeviceDef *dst,
+                             virNodeDeviceDef *src)
+{
+    bool ret = false;
+    virNodeDevCapMdev *srcmdev = &src->caps->data.mdev;
+    virNodeDevCapMdev *dstmdev = &dst->caps->data.mdev;
+
+    if (STRNEQ_NULLABLE(dstmdev->type, srcmdev->type)) {
+        ret = true;
+        g_free(dstmdev->type);
+        dstmdev->type = g_strdup(srcmdev->type);
+    }
+
+    if (STRNEQ_NULLABLE(dstmdev->uuid, srcmdev->uuid)) {
+        ret = true;
+        g_free(dstmdev->uuid);
+        dstmdev->uuid = g_strdup(srcmdev->uuid);
+    }
+
+    if (virMediatedDeviceAttrsCopy(dstmdev, srcmdev))
+        ret = true;
+
+    return ret;
+}

diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h
index 167e6bfa534598af1c6f4df1aedf3133d38d3834..31476a2f34a7b6fabf0f62d3d9f25c2a7a1ed7e6 100644 (file)
--- a/src/node_device/node_device_driver.h
+++ b/src/node_device/node_device_driver.h
@@ -130,8 +130,14 @@ int
 nodeDeviceParseMdevctlJSON(const char *jsonstring,
                            virNodeDeviceDef ***devs);
 
+int
+nodeDeviceUpdateMediatedDevices(void);
+
 void
 nodeDeviceGenerateName(virNodeDeviceDef *def,
                        const char *subsystem,
                        const char *sysname,
                        const char *s);
+
+bool nodeDeviceDefCopyFromMdevctl(virNodeDeviceDef *dst,
+                                  virNodeDeviceDef *src);

diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index eae301cc95258dc93c23062ba3a95b508b46230c..38ebe7b5c5e6c47a764e6901e86ebec4894c1dd4 100644 (file)
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1421,9 +1421,17 @@ udevRemoveOneDeviceSysPath(const char *path)
                                            VIR_NODE_DEVICE_EVENT_DELETED,
                                            0);
 
-    VIR_DEBUG("Removing device '%s' with sysfs path '%s'",
-              def->name, path);
-    virNodeDeviceObjListRemove(driver->devs, obj);
+    /* If the device is a mediated device that has been 'stopped', it may still
+     * be defined by mdevctl and can therefore be started again. Don't drop it
+     * from the list of node devices */
+    if (virNodeDeviceObjIsPersistent(obj)) {
+        VIR_FREE(def->sysfs_path);
+        virNodeDeviceObjSetActive(obj, false);
+    } else {
+        VIR_DEBUG("Removing device '%s' with sysfs path '%s'",
+                  def->name, path);
+        virNodeDeviceObjListRemove(driver->devs, obj);
+    }
     virNodeDeviceObjEndAPI(&obj);
 
     virObjectEventStateQueue(driver->nodeDeviceEventState, event);
@@ -1481,7 +1489,6 @@ udevSetParent(struct udev_device *device,
     return 0;
 }
 
-
 static int
 udevAddOneDevice(struct udev_device *device)
 {
@@ -1491,6 +1498,7 @@ udevAddOneDevice(struct udev_device *device)
     virObjectEventPtr event = NULL;
     bool new_device = true;
     int ret = -1;
+    bool was_persistent = false;
 
     def = g_new0(virNodeDeviceDef, 1);
 
@@ -1514,14 +1522,23 @@ udevAddOneDevice(struct udev_device *device)
         goto cleanup;
 
     if ((obj = virNodeDeviceObjListFindByName(driver->devs, def->name))) {
+        objdef = virNodeDeviceObjGetDef(obj);
+
+        if (def->caps->data.type == VIR_NODE_DEV_CAP_MDEV)
+            nodeDeviceDefCopyFromMdevctl(def, objdef);
+        was_persistent = virNodeDeviceObjIsPersistent(obj);
+        /* If the device was defined by mdevctl and was never instantiated, it
+         * won't have a sysfs path. We need to emit a CREATED event... */
+        new_device = (objdef->sysfs_path == NULL);
+
         virNodeDeviceObjEndAPI(&obj);
-        new_device = false;
     }
 
     /* If this is a device change, the old definition will be freed
      * and the current definition will take its place. */
     if (!(obj = virNodeDeviceObjListAssignDef(driver->devs, def)))
         goto cleanup;
+    virNodeDeviceObjSetPersistent(obj, was_persistent);
     objdef = virNodeDeviceObjGetDef(obj);
 
     if (new_device)
@@ -1941,6 +1958,10 @@ nodeStateInitializeEnumerate(void *opaque)
     /* Populate with known devices */
     if (udevEnumerateDevices(udev) != 0)
         goto error;
+    /* Load persistent mdevs (which might not be activated yet) and additional
+     * information about active mediated devices from mdevctl */
+    if (nodeDeviceUpdateMediatedDevices() != 0)
+        goto error;
 
     nodeDeviceLock();
     driver->initialized = true;