[libvirt] [PATCH v4 1/7] nodedev: Introduce udevEventCheckMonitorFD helper function

[Erik Skultety]

On Wed, Sep 20, 2017 at 08:52:26AM -0400, John Ferlan wrote:
>
>
> 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 at 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.

Do-oh! Right, I'll move it - that's just a result of 4+ rebases since I
couldn't make up my mind on how to split it in the least disruptive way so that
I'd make the reviewer's job much easier.

Thanks,
Erik