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 <[email protected]>
> ---
> 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 <[email protected]>
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;
> }
>
>
>
--
libvir-list mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libvir-list
