On 09/18/2017 12:34 PM, Erik Skultety wrote:
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@redhat.com> --- src/node_device/node_device_udev.c | 40 +++++++++++++++++++++++++------------- 1 file changed, 27 insertions(+), 13 deletions(-)
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index f4177455c..fe21ad7df 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1615,14 +1615,10 @@ udevHandleOneDevice(struct udev_device *device) }
-static void -udevEventHandleCallback(int watch ATTRIBUTE_UNUSED, - int fd, - int events ATTRIBUTE_UNUSED, - void *data ATTRIBUTE_UNUSED) +static bool +udevEventCheckMonitorFD(struct udev_monitor *udev_monitor, + int fd) { - struct udev_device *device = NULL; - struct udev_monitor *udev_monitor = DRV_STATE_UDEV_MONITOR(driver); int udev_fd = -1;
udev_fd = udev_monitor_get_fd(udev_monitor); @@ -1641,21 +1637,39 @@ udevEventHandleCallback(int watch ATTRIBUTE_UNUSED, virEventRemoveHandle(priv->watch); priv->watch = -1;
- goto cleanup; + return false; + } + + 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; + + udev_monitor = DRV_STATE_UDEV_MONITOR(driver); + + if (!udevEventCheckMonitorFD(udev_monitor, fd)) { + nodeDeviceUnlock();
Me thinks this particular line belongs in the next patch... of course that means the { } won't be necessary here. Reviewed-by: John Ferlan <jferlan@redhat.com> John
+ return; }
device = udev_monitor_receive_device(udev_monitor); - if (device == NULL) { + + if (!device) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("udev_monitor_receive_device returned NULL")); - goto cleanup; + return; }
udevHandleOneDevice(device); - - cleanup: udev_device_unref(device); - return; }