[libvirt PATCH v3 11/21] nodedev: Refresh mdev devices when changes are detected

Jonathon Jongsma jjongsma at redhat.com
Thu Dec 24 14:14:35 UTC 2020


We need to periodically query mdevctl for changes to device definitions
since an administrator can define new devices with mdevctl outside of
libvirt.

In the future, mdevctl may add a way to signal device add/remove via
events, but for now we resort to a bit of a workaround: monitoring the
mdevctl config directories for changes to files. When a change is
detected, we query mdevctl and update our device list. The mdevctl
querying is handled by the existing udev thread.

Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>
---
 src/node_device/node_device_udev.c | 204 ++++++++++++++++++++++++-----
 1 file changed, 171 insertions(+), 33 deletions(-)

diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index d7f7ab4370..5729fea264 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -41,6 +41,7 @@
 #include "virnetdev.h"
 #include "virmdev.h"
 #include "virutil.h"
+#include <gio/gio.h>
 
 #include "configmake.h"
 
@@ -66,6 +67,10 @@ struct _udevEventData {
     virCond threadCond;
     bool threadQuit;
     bool udevReady;
+    bool mdevReady;
+
+    GList *mdevctlMonitors;
+    int mdevctlTimeout;
 };
 
 static virClassPtr udevEventDataClass;
@@ -85,6 +90,7 @@ udevEventDataDispose(void *obj)
     udev = udev_monitor_get_udev(priv->udev_monitor);
     udev_monitor_unref(priv->udev_monitor);
     udev_unref(udev);
+    g_list_free_full(priv->mdevctlMonitors, g_object_unref);
 
     virCondDestroy(&priv->threadCond);
 }
@@ -1821,7 +1827,7 @@ udevEventHandleThread(void *opaque G_GNUC_UNUSED)
     /* continue rather than break from the loop on non-fatal errors */
     while (1) {
         virObjectLock(priv);
-        while (!priv->udevReady && !priv->threadQuit) {
+        while (!priv->udevReady && !priv->mdevReady && !priv->threadQuit) {
             if (virCondWait(&priv->threadCond, &priv->parent.lock)) {
                 virReportSystemError(errno, "%s",
                                      _("handler failed to wait on condition"));
@@ -1835,46 +1841,58 @@ udevEventHandleThread(void *opaque G_GNUC_UNUSED)
             return;
         }
 
-        errno = 0;
-        device = udev_monitor_receive_device(priv->udev_monitor);
-        virObjectUnlock(priv);
+        if (priv->udevReady) {
+            errno = 0;
+            device = udev_monitor_receive_device(priv->udev_monitor);
+            virObjectUnlock(priv);
 
-        if (!device) {
-            if (errno == 0) {
-                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                               _("failed to receive device from udev monitor"));
-                return;
-            }
+            if (!device) {
+                if (errno == 0) {
+                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                                   _("failed to receive device from udev monitor"));
+                    return;
+                }
+
+                /* POSIX allows both EAGAIN and EWOULDBLOCK to be used
+                 * interchangeably when the read would block or timeout was fired
+                 */
+                VIR_WARNINGS_NO_WLOGICALOP_EQUAL_EXPR
+                if (errno != EAGAIN && errno != EWOULDBLOCK) {
+                VIR_WARNINGS_RESET
+                    virReportSystemError(errno, "%s",
+                                         _("failed to receive device from udev "
+                                           "monitor"));
+                    return;
+                }
+
+                /* Trying to move the reset of the @priv->udevReady flag to
+                 * after the udev_monitor_receive_device wouldn't help much
+                 * due to event mgmt and scheduler timing. */
+                virObjectLock(priv);
+                priv->udevReady = false;
+                virObjectUnlock(priv);
 
-            /* POSIX allows both EAGAIN and EWOULDBLOCK to be used
-             * interchangeably when the read would block or timeout was fired
-             */
-            VIR_WARNINGS_NO_WLOGICALOP_EQUAL_EXPR
-            if (errno != EAGAIN && errno != EWOULDBLOCK) {
-            VIR_WARNINGS_RESET
-                virReportSystemError(errno, "%s",
-                                     _("failed to receive device from udev "
-                                       "monitor"));
-                return;
+                continue;
             }
 
-            /* Trying to move the reset of the @priv->udevReady flag to
-             * after the udev_monitor_receive_device wouldn't help much
-             * due to event mgmt and scheduler timing. */
-            virObjectLock(priv);
-            priv->udevReady = false;
-            virObjectUnlock(priv);
+            udevHandleOneDevice(device);
+            udev_device_unref(device);
 
-            continue;
+            /* Instead of waiting for the next event after processing @device
+             * data, let's keep reading from the udev monitor and only wait
+             * for the next event once either a EAGAIN or a EWOULDBLOCK error
+             * is encountered. */
         }
 
-        udevHandleOneDevice(device);
-        udev_device_unref(device);
+        if (priv->mdevReady) {
+            virObjectUnlock(priv);
+            if (nodeDeviceUpdateMediatedDevices() < 0)
+                VIR_WARN("mdevctl failed to updated mediated devices");
 
-        /* Instead of waiting for the next event after processing @device
-         * data, let's keep reading from the udev monitor and only wait
-         * for the next event once either a EAGAIN or a EWOULDBLOCK error
-         * is encountered. */
+            virObjectLock(priv);
+            priv->mdevReady = false;
+            virObjectUnlock(priv);
+        }
     }
 }
 
@@ -1899,6 +1917,117 @@ udevEventHandleCallback(int watch G_GNUC_UNUSED,
 }
 
 
+static void
+scheduleMdevctlHandler(int timer G_GNUC_UNUSED, void *opaque)
+{
+    udevEventDataPtr priv = opaque;
+
+    if (priv->mdevctlTimeout > 0) {
+        virEventRemoveTimeout(priv->mdevctlTimeout);
+        priv->mdevctlTimeout = -1;
+    }
+
+    virObjectLock(priv);
+    priv->mdevReady = true;
+    virCondSignal(&priv->threadCond);
+    virObjectUnlock(priv);
+}
+
+
+static void
+mdevctlEventHandleCallback(GFileMonitor *monitor,
+                           GFile *file,
+                           GFile *other_file,
+                           GFileMonitorEvent event_type,
+                           gpointer user_data);
+
+
+/* Recursively monitors a directory and its subdirectories for file changes and
+ * returns a GList of GFileMonitor objects */
+static GList*
+monitorFileRecursively(udevEventDataPtr udev,
+                       GFile *file)
+{
+    GList *monitors = NULL;
+    g_autoptr(GError) error = NULL;
+    g_autoptr(GFileEnumerator) children = NULL;
+    GFileMonitor *mon;
+
+    if (!(children = g_file_enumerate_children(file, "standard::*",
+                                               G_FILE_QUERY_INFO_NONE, NULL, &error))) {
+        if (error->code == G_IO_ERROR_NOT_DIRECTORY)
+            return NULL;
+        goto error;
+    }
+
+    if (!(mon = g_file_monitor(file, G_FILE_MONITOR_NONE, NULL, &error)))
+        goto error;
+
+    g_signal_connect(mon, "changed",
+                     G_CALLBACK(mdevctlEventHandleCallback), udev);
+    monitors = g_list_append(monitors, mon);
+
+    while (true) {
+        GFileInfo *info;
+        GFile *child;
+        GList *child_monitors = NULL;
+
+        if (!g_file_enumerator_iterate(children, &info, &child, NULL, &error))
+            goto error;
+        if (!info)
+            break;
+
+        child_monitors = monitorFileRecursively(udev, child);
+        if (child_monitors)
+            monitors = g_list_concat(monitors, child_monitors);
+    }
+
+    return monitors;
+
+ error:
+    g_list_free_full(monitors, g_object_unref);
+    virReportError(VIR_ERR_INTERNAL_ERROR,
+                   _("Unable to monitor directory: %s"), error->message);
+    return NULL;
+}
+
+
+static void
+mdevctlEventHandleCallback(GFileMonitor *monitor G_GNUC_UNUSED,
+                           GFile *file,
+                           GFile *other_file G_GNUC_UNUSED,
+                           GFileMonitorEvent event_type,
+                           gpointer user_data)
+{
+    udevEventDataPtr priv = user_data;
+
+    /* if a new directory appears, monitor that directory for changes */
+    if (event_type == G_FILE_MONITOR_EVENT_CREATED &&
+        g_file_query_file_type(file, G_FILE_QUERY_INFO_NONE, NULL) ==
+        G_FILE_TYPE_DIRECTORY) {
+        GList *newmonitors = monitorFileRecursively(priv, file);
+        priv->mdevctlMonitors = g_list_concat(priv->mdevctlMonitors, newmonitors);
+    }
+
+    /* When mdevctl creates a device, it can result in multiple notify events
+     * emitted for a single logical change (e.g. several CHANGED events, or a
+     * CREATED and CHANGED event followed by CHANGES_DONE_HINT). To avoid
+     * spawning a mdevctl thread multiple times for a single logical
+     * configuration change, try to coalesce these changes by waiting for the
+     * CHANGES_DONE_HINT event. As a fallback,  add a timeout to trigger the
+     * signal if that event never comes */
+    if (event_type != G_FILE_MONITOR_EVENT_CHANGES_DONE_HINT) {
+        if (priv->mdevctlTimeout > 0)
+            virEventRemoveTimeout(priv->mdevctlTimeout);
+        priv->mdevctlTimeout = virEventAddTimeout(100, scheduleMdevctlHandler,
+                                                  priv, NULL);
+        return;
+    }
+
+    scheduleMdevctlHandler(-1, priv);
+}
+
+
 /* DMI is intel-compatible specific */
 #if defined(__x86_64__) || defined(__i386__) || defined(__amd64__)
 static void
@@ -2052,6 +2181,7 @@ nodeStateInitialize(bool privileged,
     udevEventDataPtr priv = NULL;
     struct udev *udev = NULL;
     virThread enumThread;
+    g_autoptr(GFile) mdevctlConfigDir = g_file_new_for_path("/etc/mdevctl.d");
 
     if (root != NULL) {
         virReportError(VIR_ERR_INVALID_ARG, "%s",
@@ -2153,6 +2283,14 @@ nodeStateInitialize(bool privileged,
     if (priv->watch == -1)
         goto unlock;
 
+    /* mdevctl may add notification events in the future:
+     * https://github.com/mdevctl/mdevctl/issues/27. For now, fall back to
+     * monitoring the mdevctl configuration directory for changes.
+     * mdevctl configuration is stored in a directory tree within
+     * /etc/mdevctl.d/. There is a directory for each parent device, which
+     * contains a file defining each mediated device */
+    priv->mdevctlMonitors = monitorFileRecursively(priv, mdevctlConfigDir);
+
     virObjectUnlock(priv);
 
     /* Create a fictional 'computer' device to root the device tree. */
-- 
2.26.2




More information about the libvir-list mailing list