[PATCH v2 1/5] nodedev: Introduce udevCheckMonitorFD helper function

[Erik Skultety <eskultet@xxxxxxxxxx>]

We need to perform some sanity checks on the udev monitor before every
use so that we know nothing changed in the meantime. The reason for
moving the code to a separate function is to be able to perform the same
check from a worker thread that will replace the udevEventHandleCallback
in terms of poking udev for device events.

Signed-off-by: Erik Skultety <eskultet@xxxxxxxxxx>
---
 src/node_device/node_device_udev.c | 56 +++++++++++++++++++++++---------------
 1 file changed, 34 insertions(+), 22 deletions(-)

diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index dc9ed9e25..0b4e22f29 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1616,42 +1616,54 @@ udevHandleOneDevice(struct udev_device *device)
 }
 
 
-static void
-udevEventHandleCallback(int watch ATTRIBUTE_UNUSED,
-                        int fd,
-                        int events ATTRIBUTE_UNUSED,
-                        void *data ATTRIBUTE_UNUSED)
+/* the caller must be holding the driver lock prior to calling this function */
+static bool
+udevCheckMonitorFD(struct udev_monitor *udev_monitor, int fd)
 {
-    struct udev_device *device = NULL;
-    struct udev_monitor *udev_monitor = NULL;
-    int udev_fd = -1;
+    int real_fd = -1;
 
-    nodeDeviceLock();
-    if (!(udev_monitor = DRV_STATE_UDEV_MONITOR(driver)))
-        goto error;
+    /* sanity check that the monitor socket hasn't changed */
+    real_fd = udev_monitor_get_fd(udev_monitor);
 
-    udev_fd = udev_monitor_get_fd(udev_monitor);
-    if (fd != udev_fd) {
+    if (real_fd != fd) {
         udevPrivate *priv = driver->privateData;
 
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("File descriptor returned by udev %d does not "
                          "match node device file descriptor %d"),
-                       fd, udev_fd);
+                       real_fd, fd);
 
-        /* this is a non-recoverable error, let's remove the handle, so that we
-         * don't get in here again because of some spurious behaviour and report
-         * the same error multiple times
+        /* this is a non-recoverable error, thus the event handle has to be
+         * removed, so that we don't get into the handler again because of some
+         * spurious behaviour
          */
         virEventRemoveHandle(priv->watch);
-        goto error;
+        return false;
     }
 
-    device = udev_monitor_receive_device(udev_monitor);
-    if (device == NULL) {
+    return true;
+}
+
+
+static void
+udevEventHandleCallback(int watch ATTRIBUTE_UNUSED,
+                        int fd,
+                        int events ATTRIBUTE_UNUSED,
+                        void *data ATTRIBUTE_UNUSED)
+{
+    struct udev_device *device = NULL;
+    struct udev_monitor *udev_monitor = NULL;
+
+    nodeDeviceLock();
+    udev_monitor = DRV_STATE_UDEV_MONITOR(driver);
+
+    if (!udevCheckMonitorFD(udev_monitor, fd))
+        goto unlock;
+
+    if (!(device = udev_monitor_receive_device(udev_monitor))) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("udev_monitor_receive_device returned NULL"));
-        goto error;
+        goto unlock;
     }
 
     nodeDeviceUnlock();
@@ -1661,7 +1673,7 @@ udevEventHandleCallback(int watch ATTRIBUTE_UNUSED,
     udev_device_unref(device);
     return;
 
- error:
+ unlock:
     nodeDeviceUnlock();
     goto cleanup;
 }
-- 
2.13.3

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list