9:52 a.m.
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(a)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