discover/udev: Register udev monitor before enumerating devices

Currently, we enumerate udev devices before setting up our monitor. This
means that we may lose devices that udev discovers after we start the
enumeration, but before the monitor is registered.

This change enables the monitor before enumeration, so we don't lose
devices. We add a filter to the enumeration code to only parse
completely initialised devices.

This means we may need to handle change events as the main source of
device notifications. We keep the existing CDROM event handler, but
check for new devices and handle those as an add.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>

diff --git a/discover/udev.c b/discover/udev.c
index e3495e99efe7f6f69ad33ba42dae26c07c2900d6..0b2ec2fd905d3da2bad341a26653c738f68da2a8 100644 (file)
--- a/discover/udev.c
+++ b/discover/udev.c
@@ -183,18 +183,16 @@ static int udev_handle_dev_remove(struct pb_udev *udev, struct udev_device *dev)
        return 0;
 }
 
        return 0;
 }
 

-static int udev_handle_dev_change(struct pb_udev *udev, struct udev_device *dev)
+/* returns true if further event processing should stop (eg., we've
+ * ejected the cdrom)
+ */
+static bool udev_handle_cdrom_events(struct pb_udev *udev,
+               struct udev_device *dev, struct discover_device *ddev)

 {
 {

-       struct discover_device *ddev;
-       const char *name, *node;
+       const char *node;

 
 

-       name = udev_device_get_sysname(dev);

        node = udev_device_get_devnode(dev);
 
        node = udev_device_get_devnode(dev);
 

-       /* we're only interested in CDROM change events at present */
-       if (!udev_device_get_property_value(dev, "ID_CDROM"))
-               return 0;
-

        /* handle CDROM eject requests */
        if (udev_device_get_property_value(dev, "DISK_EJECT_REQUEST")) {
                bool eject = false;
        /* handle CDROM eject requests */
        if (udev_device_get_property_value(dev, "DISK_EJECT_REQUEST")) {
                bool eject = false;


@@ -204,26 +202,51 @@ static int udev_handle_dev_change(struct pb_udev *udev, struct udev_device *dev)
                /* If the device is mounted, cdrom_id's own eject request may
                 * have failed. So, we'll need to do our own here.
                 */
                /* If the device is mounted, cdrom_id's own eject request may
                 * have failed. So, we'll need to do our own here.
                 */

-               ddev = device_lookup_by_id(udev->handler, name);

                if (ddev) {
                        eject = ddev->mounted;
                        udev_handle_dev_remove(udev, dev);
                if (ddev) {
                        eject = ddev->mounted;
                        udev_handle_dev_remove(udev, dev);

+                       return false;

                }
 
                if (eject)
                        cdrom_eject(node);
 
                }
 
                if (eject)
                        cdrom_eject(node);
 

-               return 0;
+               return true;

        }
 
        if (udev_device_get_property_value(dev, "DISK_MEDIA_CHANGE")) {
                if (cdrom_media_present(node))
        }
 
        if (udev_device_get_property_value(dev, "DISK_MEDIA_CHANGE")) {
                if (cdrom_media_present(node))

-                       return udev_handle_dev_add(udev, dev);
+                       udev_handle_dev_add(udev, dev);

                else
                else

-                       return udev_handle_dev_remove(udev, dev);
+                       udev_handle_dev_remove(udev, dev);
+               return true;

        }
 
        }
 

-       return 0;
+       return false;
+}
+
+static int udev_handle_dev_change(struct pb_udev *udev, struct udev_device *dev)
+{
+       struct discover_device *ddev;
+       const char *name;
+       int rc = 0;
+
+       name = udev_device_get_sysname(dev);
+
+       ddev = device_lookup_by_id(udev->handler, name);
+
+       /* if this is a CDROM device, process eject & media change requests;
+        * these may stop further processing */
+       if (!udev_device_get_property_value(dev, "ID_CDROM")) {
+               if (udev_handle_cdrom_events(udev, dev, ddev))
+                       return 0;
+       }
+
+       /* if this is a new device, treat it as an add */
+       if (!ddev)
+               rc = udev_handle_dev_add(udev, dev);
+
+       return rc;

 }
 
 static int udev_handle_dev_action(struct udev_device *dev, const char *action)
 }
 
 static int udev_handle_dev_action(struct udev_device *dev, const char *action)


@@ -273,12 +296,17 @@ static int udev_enumerate(struct udev *udev)
        }
 
        result = udev_enumerate_add_match_subsystem(enumerate, "block");
        }
 
        result = udev_enumerate_add_match_subsystem(enumerate, "block");

-

        if (result) {
                pb_log("udev_enumerate_add_match_subsystem failed\n");
                goto fail;
        }
 
        if (result) {
                pb_log("udev_enumerate_add_match_subsystem failed\n");
                goto fail;
        }
 

+       result = udev_enumerate_add_match_is_initialized(enumerate);
+       if (result) {
+               pb_log("udev_enumerate_add_match_is_initialised failed\n");
+               goto fail;
+       }
+

        udev_enumerate_scan_devices(enumerate);
 
        list = udev_enumerate_get_list_entry(enumerate);
        udev_enumerate_scan_devices(enumerate);
 
        list = udev_enumerate_get_list_entry(enumerate);


@@ -407,16 +435,14 @@ struct pb_udev *udev_init(struct waitset *waitset,
 
        udev_set_log_fn(udev->udev, udev_log_fn);
 
 
        udev_set_log_fn(udev->udev, udev_log_fn);
 

-       result = udev_enumerate(udev->udev);
-
-       if (result)
-               goto fail_enumerate;
-

        result = udev_setup_monitor(udev->udev, &udev->monitor);
        result = udev_setup_monitor(udev->udev, &udev->monitor);

-

        if (result)
                goto fail_monitor;
 
        if (result)
                goto fail_monitor;
 

+       result = udev_enumerate(udev->udev);
+       if (result)
+               goto fail_enumerate;
+

        waiter_register_io(waitset, udev_monitor_get_fd(udev->monitor), WAIT_IN,
                udev_process, udev->monitor);
 
        waiter_register_io(waitset, udev_monitor_get_fd(udev->monitor), WAIT_IN,
                udev_process, udev->monitor);
 


@@ -425,6 +451,7 @@ struct pb_udev *udev_init(struct waitset *waitset,
        return udev;
 
 fail_monitor:
        return udev;
 
 fail_monitor:

+       udev_monitor_unref(udev->monitor);

 fail_enumerate:
        udev_unref(udev->udev);
 fail_new:
 fail_enumerate:
        udev_unref(udev->udev);
 fail_new: