On Mon, Aug 28, 2017 at 11:04:35AM -0400, John Ferlan wrote:
>
>
> 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 <eskul...@redhat.com>
> > ---
> > 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!
Right, good catch, thanks.
>
> > +
> > + 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.
Okay, let's continue with the discussion in patch 3 comments :).
Erik
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
