[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt PATCH v2 09/16] nodedev: add an mdevctl thread



On a Tuesday in 2020, Jonathon Jongsma wrote:
We need to peridocally query mdevctl for changes to device definitions
since an administrator can define new devices with mdevctl outside of
libvirt.

In order to do this, a new thread is created to handle the querying. 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.

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

diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index e06584a3dc..a85418e549 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1858,6 +1969,58 @@ udevPCITranslateInit(bool privileged G_GNUC_UNUSED)
}


+/* Recursively monitors dir and any subdirectory for file changes and returns a
+ * GList of GFileMonitor objects */
+static GList*
+monitorDirRecursively(GFile *dir,
+                      GCallback changed_cb,
+                      gpointer user_data)
+{
+    GList *monitors = NULL;

+    g_autoptr(GFileInfo) dirinfo = NULL;

../src/node_device/node_device_udev.c:1980:26: error: unused variable 'dirinfo' [-Werror,-Wunused-variable]
    g_autoptr(GFileInfo) dirinfo = NULL;
                         ^


+    g_autoptr(GError) error = NULL;
+    g_autoptr(GFileEnumerator) children = NULL;
+    GFileMonitor *mon;
+
+    if (!(children = g_file_enumerate_children(dir, "standard::*",
+                                               G_FILE_QUERY_INFO_NONE, NULL, &error))) {
+        if (error->code == G_IO_ERROR_NOT_DIRECTORY)
+            return NULL;
+        goto bail;

s/bail/error/

+    }
+
+    if (!(mon = g_file_monitor(dir, G_FILE_MONITOR_NONE, NULL, &error)))
+        goto bail;
+
+    g_signal_connect(mon, "changed", changed_cb, user_data);
+    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 bail;
+        if (!info)
+            break;
+
+        child_monitors = monitorDirRecursively(child, changed_cb, user_data);
+        if (child_monitors)
+            monitors = g_list_concat(monitors, child_monitors);
+    }
+
+    return monitors;
+
+ bail:
+    if (error)
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Unable to monitor directory: %s"), error->message);

When returning an error code, any libvirt function should either report
an error in all cases or stay quiet, so something like:
  error ? error->message : _("unknown error")
is a tiny bit frendlier for debugging.

+
+    g_list_free_full(monitors, g_object_unref);
+    return NULL;
+}
+
static int
nodeStateInitialize(bool privileged,
                    const char *root,
@@ -1867,6 +2030,8 @@ nodeStateInitialize(bool privileged,
    udevEventDataPtr priv = NULL;
    struct udev *udev = NULL;
    virThread enumThread;
+    g_autoptr(GError) error = NULL;

../src/node_device/node_device_udev.c:2033:23: error: unused variable 'error' [-Werror,-Wunused-variable]
    g_autoptr(GError) error = NULL;
                      ^

Jano

+    g_autoptr(GFile) mdevctlConfigDir = g_file_new_for_path("/etc/mdevctl.d");

    if (root != NULL) {
        virReportError(VIR_ERR_INVALID_ARG, "%s",
@@ -1969,6 +2134,23 @@ nodeStateInitialize(bool privileged,
    if (priv->watch == -1)
        goto unlock;

+    if (virThreadCreateFull(&priv->mdevctlThread, true, mdevctlThread,
+                            "mdevctl-event", false, NULL) < 0) {
+        virReportSystemError(errno, "%s",
+                             _("failed to create mdevctl handler thread"));
+        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->mdevctl_monitors = monitorDirRecursively(mdevctlConfigDir,
+                                                   G_CALLBACK(mdevctlEventHandleCallback),
+                                                   priv);
+
    virObjectUnlock(priv);

    /* Create a fictional 'computer' device to root the device tree. */
--
2.26.2

Attachment: signature.asc
Description: PGP signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]