We need to query mdevctl for changes to device definitions since an
administrator can define new devices by executing 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 directory for changes to files. When a change is
detected, we query mdevctl and update our device list. The mdevctl
querying is handled in a throwaway thread, and these threads are
synchronized with a mutex.

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

diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 038941ec51..8a1210cffb 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -19,6 +19,7 @@
  */
 
 #include <config.h>
+#include <gio/gio.h>
 #include <libudev.h>
 #include <pciaccess.h>
 #include <scsi/scsi.h>
@@ -66,6 +67,10 @@ struct _udevEventData {
     virCond threadCond;
     bool threadQuit;
     bool dataReady;
+
+    GList *mdevctlMonitors;
+    virMutex mdevctlLock;
+    int mdevctlTimeout;
 };
 
 static virClassPtr udevEventDataClass;
@@ -86,6 +91,9 @@ udevEventDataDispose(void *obj)
     udev_monitor_unref(priv->udev_monitor);
     udev_unref(udev);
 
+    g_list_free_full(priv->mdevctlMonitors, g_object_unref);
+    virMutexDestroy(&priv->mdevctlLock);
+
     virCondDestroy(&priv->threadCond);
 }
 
@@ -117,6 +125,11 @@ udevEventDataNew(void)
         return NULL;
     }
 
+    if (virMutexInit(&ret->mdevctlLock) < 0) {
+        virObjectUnref(ret);
+        return NULL;
+    }
+
     ret->watch = -1;
     return ret;
 }
@@ -1998,6 +2011,133 @@ udevPCITranslateInit(bool privileged G_GNUC_UNUSED)
 }
 
 
+static void
+mdevctlHandlerThread(void *opaque G_GNUC_UNUSED)
+{
+    udevEventDataPtr priv = driver->privateData;
+
+    /* ensure only a single thread can query mdevctl at a time */
+    virMutexLock(&priv->mdevctlLock);
+    if (nodeDeviceUpdateMediatedDevices() < 0)
+        VIR_WARN("mdevctl failed to updated mediated devices");
+    virMutexUnlock(&priv->mdevctlLock);
+}
+
+
+static void
+scheduleMdevctlHandler(int timer G_GNUC_UNUSED, void *opaque)
+{
+    udevEventDataPtr priv = opaque;
+    virThread thread;
+
+    if (priv->mdevctlTimeout > 0) {
+        virEventRemoveTimeout(priv->mdevctlTimeout);
+        priv->mdevctlTimeout = -1;
+    }
+
+    if (virThreadCreateFull(&thread, false, mdevctlHandlerThread,
+                            "mdevctl-thread", false, NULL) < 0) {
+        virReportSystemError(errno, "%s",
+                             _("failed to create mdevctl thread"));
+    }
+}
+
+
+static void
+mdevctlEventHandleCallback(GFileMonitor *monitor G_GNUC_UNUSED,
+                           GFile *file,
+                           GFile *other_file G_GNUC_UNUSED,
+                           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)))
+        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;
+
+        if (g_file_query_file_type(child, G_FILE_QUERY_INFO_NONE, NULL) ==
+            G_FILE_TYPE_DIRECTORY) {
+
+            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);
+    g_clear_error(&error);
+    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);
+}
+
+
 static int
 nodeStateInitialize(bool privileged,
                     const char *root,
@@ -2007,6 +2147,8 @@ nodeStateInitialize(bool privileged,
     udevEventDataPtr priv = NULL;
     struct udev *udev = NULL;
     virThread enumThread;
+    g_autoptr(GFile) mdevctlConfigDir = g_file_new_for_path("/etc/mdevctl.d");
+    GError *error = NULL;
 
     if (root != NULL) {
         virReportError(VIR_ERR_INVALID_ARG, "%s",
@@ -2108,6 +2250,22 @@ 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 */
+    if (!(priv->mdevctlMonitors = monitorFileRecursively(priv,
+                                                         mdevctlConfigDir))) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("failed to monitor mdevctl config directory: %s"),
+                       error->message);
+        g_clear_error(&error);
+
+        goto cleanup;
+    }
+
     virObjectUnlock(priv);
 
     /* Create a fictional 'computer' device to root the device tree. */
-- 
2.26.2

On Wed, Feb 03, 2021 at 11:38:56AM -0600, Jonathon Jongsma wrote:
> We need to query mdevctl for changes to device definitions since an
> administrator can define new devices by executing 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 directory for changes to files. When a change is
> detected, we query mdevctl and update our device list. The mdevctl
> querying is handled in a throwaway thread, and these threads are
> synchronized with a mutex.
> 
> Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
> ---
>  src/node_device/node_device_udev.c | 158 +++++++++++++++++++++++++++++
>  1 file changed, 158 insertions(+)
> 
> diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
> index 038941ec51..8a1210cffb 100644
> --- a/src/node_device/node_device_udev.c
> +++ b/src/node_device/node_device_udev.c
> @@ -19,6 +19,7 @@
>   */
>  
>  #include <config.h>
> +#include <gio/gio.h>
>  #include <libudev.h>
>  #include <pciaccess.h>
>  #include <scsi/scsi.h>
> @@ -66,6 +67,10 @@ struct _udevEventData {
>      virCond threadCond;
>      bool threadQuit;
>      bool dataReady;
> +
> +    GList *mdevctlMonitors;
> +    virMutex mdevctlLock;
> +    int mdevctlTimeout;
>  };

Sorry for the delay in review, it took me a while to wrap my head around the
amount of GLib black magic going on in this patch.

...

> +
> +
> +static void
> +mdevctlEventHandleCallback(GFileMonitor *monitor G_GNUC_UNUSED,
> +                           GFile *file,
> +                           GFile *other_file G_GNUC_UNUSED,
> +                           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)))
> +        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);

newline here^

> +    monitors = g_list_append(monitors, mon);
> +
> +    while (true) {
> +        GFileInfo *info;
> +        GFile *child;

We should initialize all the pointers - coverity might find creative ways of
complaining about it.

> +        GList *child_monitors = NULL;
> +
> +        if (!g_file_enumerator_iterate(children, &info, &child, NULL, &error))

I hope that the fact that ^this can block will never pose a problem for us.

> +            goto error;

newline here

> +        if (!info)
> +            break;
> +
> +        if (g_file_query_file_type(child, G_FILE_QUERY_INFO_NONE, NULL) ==
> +            G_FILE_TYPE_DIRECTORY) {
> +
> +            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);
> +    g_clear_error(&error);
> +    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);

priv->mdevctlMonitors is shared among threads, but you only acquire the
mutex just before exec'ing mdevctl in the mdevctlHandler thread, so ^this can
yield unexpected results in terms of directories monitored.

> +    }
> +
> +    /* 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 */

So there's no guaranteed pattern to monitor, is that so? IOW Even the
CHANGES_DONE_HINT may not actually arrive? That's a very poor design. I'd like
to ditch the timer as much as possible.

> +    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);
> +}
> +
> +
>  static int
>  nodeStateInitialize(bool privileged,
>                      const char *root,
> @@ -2007,6 +2147,8 @@ nodeStateInitialize(bool privileged,
>      udevEventDataPtr priv = NULL;
>      struct udev *udev = NULL;
>      virThread enumThread;
> +    g_autoptr(GFile) mdevctlConfigDir = g_file_new_for_path("/etc/mdevctl.d");
> +    GError *error = NULL;

^This error isn't passed anywhere...

>  
>      if (root != NULL) {
>          virReportError(VIR_ERR_INVALID_ARG, "%s",
> @@ -2108,6 +2250,22 @@ 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 */

One security aspect with this proposal that needs to mentioned is that we
should not make the directory to be monitored configurable, otherwise it's so
easy to deplete resources - simply because this patch doesn't introduce any
limit on number of spawned threads (like we have for thread pools).

> +    if (!(priv->mdevctlMonitors = monitorFileRecursively(priv,
> +                                                         mdevctlConfigDir))) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("failed to monitor mdevctl config directory: %s"),
> +                       error->message);

This error message is redundant, because monitorFileRecursively already reports
errors appending the respective GError, not to mentioned you didn't pass error
anywhere, so it would always be NULL.

Erik

On Wed, 17 Feb 2021 14:35:51 +0100
Erik Skultety <eskultet@redhat.com> wrote:

> > +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);  
> 
> priv->mdevctlMonitors is shared among threads, but you only acquire
> the mutex just before exec'ing mdevctl in the mdevctlHandler thread,
> so ^this can yield unexpected results in terms of directories
> monitored.

A) It doesn't really affect the directories being monitored, it only
affects our ability to free these monitors later in the destructor.

B) Aside from the very first time this variable is set in
nodeStateInitialize(), the only place that accesses the mdevctlMonitors
variable is this glib signal callback (to add new monitors to the list)
and the destructor (to free the monitors). And I believe those should
always run in the main thread. So I don't think there's really much of
a thread-safety issue here. Perhaps there's a slight race in
nodeStateInitialize() which appears to be called from a non-main
thread. If a directory gets created immediately after connecting to the
file monitor signal but before the function returns and the GList gets
assigned to priv->mdevctlMonitors, I suppose the signal handler could
run and it could get corrupted. Would you like me to protect all
accesses with a Mutex?


> 
> > +    }
> > +
> > +    /* 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 */  
> 
> So there's no guaranteed pattern to monitor, is that so? IOW Even the
> CHANGES_DONE_HINT may not actually arrive? That's a very poor design.
> I'd like to ditch the timer as much as possible.

Maybe we can simply rely on the CHANGES_DONE_HINT. I tend to be
conservative and was trying to handle the case where the
CHANGES_DONE_HINT might not be delivered (it is called a "hint" after
all). Part of this was caused by my reading this comment in a test
case within the glib repository:

  /* Sometimes the emission of 'CHANGES_DONE_HINT' may be late because
   * it depends on the ability of file monitor implementation to report
   * 'CHANGES_DONE_HINT' itself. If the file monitor implementation
   * doesn't report 'CHANGES_DONE_HINT' itself, it may be emitted by
   * GLocalFileMonitor after a few seconds, which causes the event to
   * mix with results from different steps. Since 'CHANGES_DONE_HINT'
   * is just a hint, we don't require it to be reliable and we simply
   * ignore unexpected 'CHANGES_DONE_HINT' events here. */

So it is not reliably communicated if your file monitor backend doesn't
implement it. But the above comment seems to suggest that if the file monitor
backend doesn't support it, the event will be generated by glib itself and will
simply be delayed by a couple of seconds rather than missing altogether. I'd
have to read the glib code to tell whether it's possible for it to be omitted
altogether. I guess I don't see the timer as a horrible workaround, but I can
ditch it if you want.

Alternatively I suppose I could simply kick off a new thread for each file
change event and not bother trying to coalesce them at all. This will result in
perhaps a an extra thread or two being launched when mdevctl modifies
the mdev registry behind our backs, but that's unlikely to be a common
occurrence and so shouldn't really be a significant performance issue.

> 
> > +    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);
> > +}
> > +
> > +
> >  static int
> >  nodeStateInitialize(bool privileged,
> >                      const char *root,

Jonathon

On Thu, Feb 18, 2021 at 12:04:56PM -0600, Jonathon Jongsma wrote:
> On Wed, 17 Feb 2021 14:35:51 +0100
> Erik Skultety <eskultet@redhat.com> wrote:
> 
> > > +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);  
> > 
> > priv->mdevctlMonitors is shared among threads, but you only acquire
> > the mutex just before exec'ing mdevctl in the mdevctlHandler thread,
> > so ^this can yield unexpected results in terms of directories
> > monitored.
> 
> A) It doesn't really affect the directories being monitored, it only
> affects our ability to free these monitors later in the destructor.
> 
> B) Aside from the very first time this variable is set in
> nodeStateInitialize(), the only place that accesses the mdevctlMonitors
> variable is this glib signal callback (to add new monitors to the list)
> and the destructor (to free the monitors). And I believe those should
> always run in the main thread. So I don't think there's really much of
> a thread-safety issue here. Perhaps there's a slight race in
> nodeStateInitialize() which appears to be called from a non-main
> thread. If a directory gets created immediately after connecting to the
> file monitor signal but before the function returns and the GList gets
> assigned to priv->mdevctlMonitors, I suppose the signal handler could
> run and it could get corrupted. Would you like me to protect all
> accesses with a Mutex?

If for some reason we need to extend the code in the future, it's so easy to
miss a single spot. So, I'd say - better safe than sorry - let's protect the
variable the way it's supposed to be even though the race might not exist right
now.

Erik

> 
> 
> > 
> > > +    }
> > > +
> > > +    /* 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 */  
> > 
> > So there's no guaranteed pattern to monitor, is that so? IOW Even the
> > CHANGES_DONE_HINT may not actually arrive? That's a very poor design.
> > I'd like to ditch the timer as much as possible.
> 
> Maybe we can simply rely on the CHANGES_DONE_HINT. I tend to be
> conservative and was trying to handle the case where the
> CHANGES_DONE_HINT might not be delivered (it is called a "hint" after
> all). Part of this was caused by my reading this comment in a test
> case within the glib repository:
> 
>   /* Sometimes the emission of 'CHANGES_DONE_HINT' may be late because
>    * it depends on the ability of file monitor implementation to report
>    * 'CHANGES_DONE_HINT' itself. If the file monitor implementation
>    * doesn't report 'CHANGES_DONE_HINT' itself, it may be emitted by
>    * GLocalFileMonitor after a few seconds, which causes the event to
>    * mix with results from different steps. Since 'CHANGES_DONE_HINT'
>    * is just a hint, we don't require it to be reliable and we simply
>    * ignore unexpected 'CHANGES_DONE_HINT' events here. */
> 
> So it is not reliably communicated if your file monitor backend doesn't
> implement it. But the above comment seems to suggest that if the file monitor
> backend doesn't support it, the event will be generated by glib itself and will
> simply be delayed by a couple of seconds rather than missing altogether. I'd
> have to read the glib code to tell whether it's possible for it to be omitted
> altogether. I guess I don't see the timer as a horrible workaround, but I can
> ditch it if you want.
> 
> Alternatively I suppose I could simply kick off a new thread for each file
> change event and not bother trying to coalesce them at all. This will result in
> perhaps a an extra thread or two being launched when mdevctl modifies
> the mdev registry behind our backs, but that's unlikely to be a common
> occurrence and so shouldn't really be a significant performance issue.

If there may be multiple events connected to a single change, which one would
trigger the actual device list update? I guess I'm starting to be okay with the
timeout in the end.

Erik