[libvirt] [PATCH v5 4/6] udev: Convert udevEventHandleThread to an actual thread routine

Erik Skultety eskultet at redhat.com
Wed Oct 11 14:52:39 UTC 2017


Adjust udevEventHandleThread to be a proper thread routine running in an
infinite loop handling devices. The handler thread pulls all available
data from the udev monitor and only then waits until a wakeup signal for
new incoming data has been emitted by udevEventHandleCallback.

Signed-off-by: Erik Skultety <eskultet at redhat.com>
---
 src/node_device/node_device_udev.c | 125 +++++++++++++++++++++++++++----------
 1 file changed, 93 insertions(+), 32 deletions(-)

diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index f7646cd8a..a6d7e6d70 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -62,6 +62,12 @@ struct _udevEventData {
     struct udev_monitor *udev_monitor;
     int watch;
     bool privileged;
+
+    /* Thread data */
+    virThread th;
+    virCond threadCond;
+    bool threadQuit;
+    bool dataReady;
 };
 
 static virClassPtr udevEventDataClass;
@@ -80,6 +86,8 @@ udevEventDataDispose(void *obj)
 
     udev_unref(udev);
     udev_monitor_unref(data->udev_monitor);
+
+    virCondDestroy(&data->threadCond);
 }
 
 
@@ -108,9 +116,15 @@ udevEventDataNew(void)
     if (!(ret = virObjectLockableNew(udevEventDataClass)))
         return NULL;
 
+    if (virCondInit(&ret->threadCond) < 0) {
+        virObjectUnref(ret);
+        return NULL;
+    }
+
     ret->watch = -1;
     return ret;
-}
+};
+
 
 
 static bool
@@ -1625,15 +1639,25 @@ udevPCITranslateDeinit(void)
 static int
 nodeStateCleanup(void)
 {
+    udevEventDataPtr priv = NULL;
+
     if (!driver)
         return -1;
 
+    priv = driver->privateData;
+
     nodeDeviceLock();
 
-    virObjectUnref(driver->privateData);
+    virObjectLock(priv);
+    priv->threadQuit = true;
+    virCondSignal(&priv->threadCond);
+    virObjectUnlock(priv);
+    virThreadJoin(&priv->th);
+
+    virObjectUnref(priv);
     virObjectUnref(driver->nodeDeviceEventState);
-
     virNodeDeviceObjListFree(driver->devs);
+
     nodeDeviceUnlock();
     virMutexDestroy(&driver->lock);
     VIR_FREE(driver);
@@ -1691,30 +1715,60 @@ udevEventMonitorSanityCheck(udevEventDataPtr priv,
 
 
 static void
-udevEventHandleThread(void *opaque)
+udevEventHandleThread(void *opaque ATTRIBUTE_UNUSED)
 {
     udevEventDataPtr priv = driver->privateData;
-    int fd = (intptr_t) opaque;
     struct udev_device *device = NULL;
 
-    virObjectLock(priv);
+    /* continue rather than break from the loop on non-fatal errors */
+    while (1) {
+        virObjectLock(priv);
+        while (!priv->dataReady && !priv->threadQuit) {
+            if (virCondWait(&priv->threadCond, &priv->parent.lock)) {
+                virReportSystemError(errno, "%s",
+                                     _("handler failed to wait on condition"));
+                virObjectUnlock(priv);
+                return;
+            }
+        }
 
-    if (!udevEventMonitorSanityCheck(priv, fd)) {
+        if (priv->threadQuit) {
+            virObjectUnlock(priv);
+            return;
+        }
+
+        errno = 0;
+        device = udev_monitor_receive_device(priv->udev_monitor);
         virObjectUnlock(priv);
-        return;
-    }
 
-    device = udev_monitor_receive_device(priv->udev_monitor);
-    virObjectUnlock(priv);
+        if (!device) {
+            if (errno == 0) {
+                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                               _("udev_monitor_receive_device failed"));
+                return;
+            }
 
-    if (device == NULL) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("udev_monitor_receive_device returned NULL"));
-        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",
+                                     _("udev_monitor_receive_device failed"));
+                return;
+            }
+
+            virObjectLock(priv);
+            priv->dataReady = false;
+            virObjectUnlock(priv);
 
-    udevHandleOneDevice(device);
-    udev_device_unref(device);
+            continue;
+        }
+
+        udevHandleOneDevice(device);
+        udev_device_unref(device);
+    }
 }
 
 
@@ -1722,18 +1776,19 @@ static void
 udevEventHandleCallback(int watch ATTRIBUTE_UNUSED,
                         int fd,
                         int events ATTRIBUTE_UNUSED,
-                        void *data ATTRIBUTE_UNUSED)
+                        void *opaque ATTRIBUTE_UNUSED)
 {
     udevEventDataPtr priv = driver->privateData;
 
     virObjectLock(priv);
-    if (!udevEventMonitorSanityCheck(priv, fd)) {
-        virObjectUnlock(priv);
-        return;
-    }
+
+    if (!udevEventMonitorSanityCheck(priv, fd))
+        priv->threadQuit = true;
+    else
+        priv->dataReady = true;
+
+    virCondSignal(&priv->threadCond);
     virObjectUnlock(priv);
-
-    udevEventHandleThread((void *)(intptr_t) fd);
 }
 
 
@@ -1875,29 +1930,29 @@ nodeStateInitialize(bool privileged,
         return -1;
     }
 
-    nodeDeviceLock();
-
     if (!(driver->devs = virNodeDeviceObjListNew()) ||
         !(priv = udevEventDataNew()))
-        goto unlock;
+        goto cleanup;
 
     driver->privateData = priv;
     driver->nodeDeviceEventState = virObjectEventStateNew();
 
     if (udevPCITranslateInit(privileged) < 0)
-        goto unlock;
+        goto cleanup;
 
     udev = udev_new();
     if (!udev) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("failed to create udev context"));
-        goto unlock;
+        goto cleanup;
     }
 #if HAVE_UDEV_LOGGING
     /* cast to get rid of missing-format-attribute warning */
     udev_set_log_fn(udev, (udevLogFunctionPtr) udevLogFunction);
 #endif
 
+    virObjectLock(priv);
+
     priv->udev_monitor = udev_monitor_new_from_netlink(udev, "udev");
     if (!priv->udev_monitor) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -1916,6 +1971,12 @@ nodeStateInitialize(bool privileged,
                                              128 * 1024 * 1024);
 #endif
 
+    if (virThreadCreate(&priv->th, true, udevEventHandleThread, NULL) < 0) {
+        virReportSystemError(errno, "%s",
+                             _("failed to create udev handler thread"));
+        goto unlock;
+    }
+
     /* We register the monitor with the event callback so we are
      * notified by udev of device changes before we enumerate existing
      * devices because libvirt will simply recreate the device if we
@@ -1934,7 +1995,7 @@ nodeStateInitialize(bool privileged,
     if (udevSetupSystemDev() != 0)
         goto unlock;
 
-    nodeDeviceUnlock();
+    virObjectUnlock(priv);
 
     /* Populate with known devices */
     if (udevEnumerateDevices(udev) != 0)
@@ -1947,7 +2008,7 @@ nodeStateInitialize(bool privileged,
     return -1;
 
  unlock:
-    nodeDeviceUnlock();
+    virObjectUnlock(priv);
     goto cleanup;
 }
 
-- 
2.13.6




More information about the libvir-list mailing list