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@xxxxxxxxxx>
> > ---
> > 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
