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

[Ján Tomko]

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 at 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
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20200825/e719de9f/attachment-0001.sig>