Re: [libvirt] [PATCH v3 2/6] udev: Split udevEventHandleCallback in two functions

On 08/24/2017 07:23 AM, Erik Skultety wrote: > This patch splits udevEventHandleCallback in two (introduces > udevEventHandleThread) in order to be later able to refactor the latter > to actually become a detached thread which will wait some time for the > kernel to create the whole sysfs tree for a device as we cannot do that > in the event loop directly. > > Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; > --- > src/node_device/node_device_udev.c | 54 ++++++++++++++++++++++++++------------ > 1 file changed, 37 insertions(+), 17 deletions(-) > > diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c > index 465d272ba..fe943c78b 100644 > --- a/src/node_device/node_device_udev.c > +++ b/src/node_device/node_device_udev.c > @@ -1647,36 +1647,56 @@ udevCheckMonitorFD(struct udev_monitor *udev_monitor, int fd) > > > static void > +udevEventHandleThread(void *opaque ATTRIBUTE_UNUSED) It's not ATTRIBUTE_UNUSED > +{ > + struct udev_device *device = NULL; > + struct udev_monitor *udev_monitor = NULL; > + int fd = (intptr_t) opaque; ^^^ opaque!

> + > + nodeDeviceLock(); > + udev_monitor = DRV_STATE_UDEV_MONITOR(driver); > + > + if (!udevCheckMonitorFD(udev_monitor, fd)) > + goto unlock; > + > + device = udev_monitor_receive_device(udev_monitor); > + if (device == NULL) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("udev_monitor_receive_device returned NULL")); > + goto unlock; > + } > + > + nodeDeviceUnlock(); > + udevHandleOneDevice(device); > + > + cleanup: > + udev_device_unref(device); > + return; > + > + unlock: > + nodeDeviceUnlock(); > + goto cleanup; > +} > + > + > +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 unlock; > + if (!udevCheckMonitorFD(udev_monitor, fd)) { > + nodeDeviceUnlock(); > + return; > } The above sequence will be done twice - once here and repeated again in HandleThread. I understand why, but as I get to patch 3 - I'm not sure things are going to work as expected.