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

[John Ferlan]

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 at 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!

> +
> +    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.

John

> -
>      nodeDeviceUnlock();
> -    udevHandleOneDevice(device);
> -
> - cleanup:
> -    udev_device_unref(device);
> -    return;
>  
> - unlock:
> -    nodeDeviceUnlock();
> -    goto cleanup;
> +    udevEventHandleThread((void *)(intptr_t) fd);
>  }
>  
>  
>