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

[John Ferlan]

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.

Reviewed-by: John Ferlan <jferlan at 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;
>  }
>  
>  
>