src/core/device.c

@@ -9,7 +9,6 @@
 #include "bus-error.h"
 #include "dbus-device.h"
 #include "dbus-unit.h"
-#include "device-private.h"
 #include "device-util.h"
 #include "device.h"
 #include "log.h"
@@ -25,9 +24,9 @@
 #include "unit.h"
 
 static const UnitActiveState state_translation_table[_DEVICE_STATE_MAX] = {
-        [DEVICE_DEAD]      = UNIT_INACTIVE,
+        [DEVICE_DEAD] = UNIT_INACTIVE,
         [DEVICE_TENTATIVE] = UNIT_ACTIVATING,
-        [DEVICE_PLUGGED]   = UNIT_ACTIVE,
+        [DEVICE_PLUGGED] = UNIT_ACTIVE,
 };
 
 static int device_dispatch_io(sd_device_monitor *monitor, sd_device *dev, void *userdata);
@@ -138,7 +137,6 @@ static int device_load(Unit *u) {
 
 static void device_set_state(Device *d, DeviceState state) {
         DeviceState old_state;
-
         assert(d);
 
         if (d->state != state)
@@ -410,9 +408,11 @@ static int device_add_udev_wants(Unit *u, sd_device *dev) {
                 if (r < 0)
                         return log_unit_error_errno(u, r, "Failed to add Wants= dependency: %m");
 
-                r = strv_consume(&added, TAKE_PTR(k));
+                r = strv_push(&added, k);
                 if (r < 0)
                         return log_oom();
+
+                k = NULL;
         }
 
         if (d->state != DEVICE_DEAD)
@@ -440,16 +440,20 @@ static int device_add_udev_wants(Unit *u, sd_device *dev) {
 }
 
 static bool device_is_bound_by_mounts(Device *d, sd_device *dev) {
+        const char *bound_by;
         int r;
 
         assert(d);
         assert(dev);
 
-        r = device_get_property_bool(dev, "SYSTEMD_MOUNT_DEVICE_BOUND");
+        if (sd_device_get_property_value(dev, "SYSTEMD_MOUNT_DEVICE_BOUND", &bound_by) >= 0) {
-        if (r < 0 && r != -ENOENT)
+                r = parse_boolean(bound_by);
-                log_device_warning_errno(dev, r, "Failed to parse SYSTEMD_MOUNT_DEVICE_BOUND= udev property, ignoring: %m");
+                if (r < 0)
+                        log_device_warning_errno(dev, r, "Failed to parse SYSTEMD_MOUNT_DEVICE_BOUND='%s' udev property, ignoring: %m", bound_by);
 
-        d->bind_mounts = r > 0;
+                d->bind_mounts = r > 0;
+        } else
+                d->bind_mounts = false;
 
         return d->bind_mounts;
 }
@@ -472,10 +476,10 @@ static void device_upgrade_mount_deps(Unit *u) {
 }
 
 static int device_setup_unit(Manager *m, sd_device *dev, const char *path, bool main) {
-        _cleanup_(unit_freep) Unit *new_unit = NULL;
         _cleanup_free_ char *e = NULL;
         const char *sysfs = NULL;
-        Unit *u;
+        Unit *u = NULL;
+        bool delete;
         int r;
 
         assert(m);
@@ -488,12 +492,32 @@ static int device_setup_unit(Manager *m, sd_device *dev, const char *path, bool
         }
 
         r = unit_name_from_path(path, ".device", &e);
-        if (r < 0)
+        if (r < 0) {
+                /* Let's complain about overly long device names only at most once every 5s or so. This is
+                 * something we should mention, since relevant devices are not manageable by systemd, but not
+                 * flood the log about. */
+                static RateLimit rate_limit = {
+                        .interval = 5 * USEC_PER_SEC,
+                        .burst = 1,
+                };
+
+                /* If we cannot convert a device name to a unit name then let's ignore the device. So far,
+                 * devices with such long names weren't really the kind you want to manage with systemd
+                 * anyway, hence this shouldn't be a problem. */
+
+                if (r == -ENAMETOOLONG)
+                        return log_struct_errno(
+                                        ratelimit_below(&rate_limit) ? LOG_WARNING : LOG_DEBUG, r,
+                                        "MESSAGE_ID=" SD_MESSAGE_DEVICE_PATH_NOT_SUITABLE_STR,
+                                        "DEVICE=%s", path,
+                                        LOG_MESSAGE("Device path '%s' too long to fit into unit name, ignoring device.", path));
+
                 return log_struct_errno(
-                                LOG_WARNING, r,
+                                ratelimit_below(&rate_limit) ? LOG_WARNING : LOG_DEBUG, r,
                                 "MESSAGE_ID=" SD_MESSAGE_DEVICE_PATH_NOT_SUITABLE_STR,
                                 "DEVICE=%s", path,
                                 LOG_MESSAGE("Failed to generate valid unit name from device path '%s', ignoring device: %m", path));
+        }
 
         u = manager_get_unit(m, e);
         if (u) {
@@ -513,16 +537,19 @@ static int device_setup_unit(Manager *m, sd_device *dev, const char *path, bool
                                                     "Device %s appeared twice with different sysfs paths %s and %s, ignoring the latter.",
                                                     e, DEVICE(u)->sysfs, sysfs);
 
+                delete = false;
+
                 /* Let's remove all dependencies generated due to udev properties. We'll re-add whatever is configured
                  * now below. */
                 unit_remove_dependencies(u, UNIT_DEPENDENCY_UDEV);
-
         } else {
-                r = unit_new_for_name(m, sizeof(Device), e, &new_unit);
+                delete = true;
-                if (r < 0)
-                        return log_device_error_errno(dev, r, "Failed to allocate device unit %s: %m", e);
 
-                u = new_unit;
+                r = unit_new_for_name(m, sizeof(Device), e, &u);
+                if (r < 0) {
+                        log_device_error_errno(dev, r, "Failed to allocate device unit %s: %m", e);
+                        goto fail;
+                }
 
                 unit_add_to_load_queue(u);
         }
@@ -531,8 +558,10 @@ static int device_setup_unit(Manager *m, sd_device *dev, const char *path, bool
          * initialized. Hence initialize it if necessary. */
         if (sysfs) {
                 r = device_set_sysfs(DEVICE(u), sysfs);
-                if (r < 0)
+                if (r < 0) {
-                        return log_unit_error_errno(u, r, "Failed to set sysfs path %s: %m", sysfs);
+                        log_unit_error_errno(u, r, "Failed to set sysfs path %s: %m", sysfs);
+                        goto fail;
+                }
 
                 /* The additional systemd udev properties we only interpret for the main object */
                 if (main)
@@ -548,8 +577,13 @@ static int device_setup_unit(Manager *m, sd_device *dev, const char *path, bool
         if (dev && device_is_bound_by_mounts(DEVICE(u), dev))
                 device_upgrade_mount_deps(u);
 
-        TAKE_PTR(new_unit);
         return 0;
+
+fail:
+        if (delete)
+                unit_free(u);
+
+        return r;
 }
 
 static void device_process_new(Manager *m, sd_device *dev) {
@@ -558,7 +592,6 @@ static void device_process_new(Manager *m, sd_device *dev) {
         int r;
 
         assert(m);
-        assert(dev);
 
         if (sd_device_get_syspath(dev, &sysfs) < 0)
                 return;
@@ -705,6 +738,8 @@ static void device_update_found_by_name(Manager *m, const char *path, DeviceFoun
 }
 
 static bool device_is_ready(sd_device *dev) {
+        const char *ready;
+
         assert(dev);
 
         if (device_is_renaming(dev) > 0)
@@ -714,7 +749,10 @@ static bool device_is_ready(sd_device *dev) {
         if (sd_device_has_current_tag(dev, "systemd") <= 0)
                 return false;
 
-        return device_get_property_bool(dev, "SYSTEMD_READY") != 0;
+        if (sd_device_get_property_value(dev, "SYSTEMD_READY", &ready) < 0)
+                return true;
+
+        return parse_boolean(ready) != 0;
 }
 
 static Unit *device_following(Unit *u) {
@@ -870,13 +908,10 @@ static void device_propagate_reload_by_sysfs(Manager *m, const char *sysfs) {
 }
 
 static void device_remove_old_on_move(Manager *m, sd_device *dev) {
-        _cleanup_free_ char *syspath_old = NULL;
+        _cleanup_free_ char *syspath_old = NULL, *e = NULL;
         const char *devpath_old;
         int r;
 
-        assert(m);
-        assert(dev);
-
         r = sd_device_get_property_value(dev, "DEVPATH_OLD", &devpath_old);
         if (r < 0)
                 return (void) log_device_debug_errno(dev, r, "Failed to get DEVPATH_OLD= property on 'move' uevent, ignoring: %m");
@@ -885,15 +920,20 @@ static void device_remove_old_on_move(Manager *m, sd_device *dev) {
         if (!syspath_old)
                 return (void) log_oom();
 
-        device_update_found_by_sysfs(m, syspath_old, 0, DEVICE_FOUND_MASK);
+        r = unit_name_from_path(syspath_old, ".device", &e);
+        if (r < 0)
+                return (void) log_device_debug_errno(dev, r, "Failed to generate unit name from old device path, ignoring: %m");
+
+        device_update_found_by_sysfs(m, syspath_old, 0, DEVICE_FOUND_UDEV|DEVICE_FOUND_MOUNT|DEVICE_FOUND_SWAP);
 }
 
 static int device_dispatch_io(sd_device_monitor *monitor, sd_device *dev, void *userdata) {
-        Manager *m = ASSERT_PTR(userdata);
         sd_device_action_t action;
+        Manager *m = userdata;
         const char *sysfs;
         int r;
 
+        assert(m);
         assert(dev);
 
         r = sd_device_get_syspath(dev, &sysfs);
@@ -924,7 +964,7 @@ static int device_dispatch_io(sd_device_monitor *monitor, sd_device *dev, void *
 
                 /* If we get notified that a device was removed by udev, then it's completely gone, hence
                  * unset all found bits */
-                device_update_found_by_sysfs(m, sysfs, 0, DEVICE_FOUND_MASK);
+                device_update_found_by_sysfs(m, sysfs, 0, DEVICE_FOUND_UDEV|DEVICE_FOUND_MOUNT|DEVICE_FOUND_SWAP);
 
         } else if (device_is_ready(dev)) {
 
@@ -946,34 +986,64 @@ static int device_dispatch_io(sd_device_monitor *monitor, sd_device *dev, void *
         return 0;
 }
 
-static int validate_node(const char *node, sd_device **ret) {
+static bool device_supported(void) {
-        _cleanup_(sd_device_unrefp) sd_device *dev = NULL;
+        static int read_only = -1;
+
+        /* If /sys is read-only we don't support device units, and any
+         * attempts to start one should fail immediately. */
+
+        if (read_only < 0)
+                read_only = path_is_read_only_fs("/sys");
+
+        return read_only <= 0;
+}
+
+static int validate_node(Manager *m, const char *node, sd_device **ret) {
+        struct stat st;
         int r;
 
+        assert(m);
         assert(node);
         assert(ret);
 
         /* Validates a device node that showed up in /proc/swaps or /proc/self/mountinfo if it makes sense for us to
          * track. Note that this validator is fine within missing device nodes, but not with badly set up ones! */
 
-        r = sd_device_new_from_devname(&dev, node);
+        if (!path_startswith(node, "/dev")) {
-        if (r == -ENODEV) {
                 *ret = NULL;
-                return 0; /* good! (though missing) */
+                return 0; /* bad! */
         }
-        if (r < 0)
-                return r; /* bad! */
 
-        *ret = TAKE_PTR(dev);
+        if (stat(node, &st) < 0) {
-        return 1; /* good! */
+                if (errno != ENOENT)
+                        return log_error_errno(errno, "Failed to stat() device node file %s: %m", node);
+
+                *ret = NULL;
+                return 1; /* good! (though missing) */
+
+        } else {
+                _cleanup_(sd_device_unrefp) sd_device *dev = NULL;
+
+                r = sd_device_new_from_stat_rdev(&dev, &st);
+                if (r == -ENOENT) {
+                        *ret = NULL;
+                        return 1; /* good! (though missing) */
+                } else if (r == -ENOTTY) {
+                        *ret = NULL;
+                        return 0; /* bad! (not a device node but some other kind of file system node) */
+                } else if (r < 0)
+                        return log_error_errno(r, "Failed to get udev device from devnum %u:%u: %m", major(st.st_rdev), minor(st.st_rdev));
+
+                *ret = TAKE_PTR(dev);
+                return 1; /* good! */
+        }
 }
 
 void device_found_node(Manager *m, const char *node, DeviceFound found, DeviceFound mask) {
         assert(m);
         assert(node);
-        assert(!FLAGS_SET(mask, DEVICE_FOUND_UDEV));
 
-        if (!udev_available())
+        if (!device_supported())
                 return;
 
         if (mask == 0)
@@ -994,10 +1064,10 @@ void device_found_node(Manager *m, const char *node, DeviceFound found, DeviceFo
                  * under the name referenced in /proc/swaps or /proc/self/mountinfo. But first, let's validate if
                  * everything is alright with the device node. */
 
-                if (validate_node(node, &dev) < 0)
+                if (validate_node(m, node, &dev) <= 0)
                         return; /* Don't create a device unit for this if the device node is borked. */
 
-                (void) device_setup_unit(m, dev, node, false); /* 'dev' may be NULL. */
+                (void) device_setup_unit(m, dev, node, false);
         }
 
         /* Update the device unit's state, should it exist */
@@ -1043,7 +1113,7 @@ const UnitVTable device_vtable = {
 
         .enumerate = device_enumerate,
         .shutdown = device_shutdown,
-        .supported = udev_available,
+        .supported = device_supported,
 
         .status_message_formats = {
                 .starting_stopping = {

src/libsystemd/sd-device/device-private.h

@@ -17,7 +17,6 @@ static inline int device_new_from_watch_handle(sd_device **ret, int wd) {
         return device_new_from_watch_handle_at(ret, -1, wd);
 }
 
-int device_get_property_bool(sd_device *device, const char *key);
 int device_get_device_id(sd_device *device, const char **ret);
 int device_get_devlink_priority(sd_device *device, int *ret);
 int device_get_watch_handle(sd_device *device);

src/libsystemd/sd-device/sd-device.c

@@ -2007,20 +2007,6 @@ _public_ int sd_device_get_property_value(sd_device *device, const char *key, co
         return 0;
 }
 
-int device_get_property_bool(sd_device *device, const char *key) {
-        const char *value;
-        int r;
-
-        assert(device);
-        assert(key);
-
-        r = sd_device_get_property_value(device, key, &value);
-        if (r < 0)
-                return r;
-
-        return parse_boolean(value);
-}
-
 _public_ int sd_device_get_trigger_uuid(sd_device *device, sd_id128_t *ret) {
         const char *s;
         sd_id128_t id;
@@ -2310,7 +2296,7 @@ _public_ int sd_device_trigger_with_uuid(
 
 _public_ int sd_device_open(sd_device *device, int flags) {
         _cleanup_close_ int fd = -1, fd2 = -1;
-        const char *devname, *subsystem = NULL;
+        const char *devname, *subsystem = NULL, *val = NULL;
         uint64_t q, diskseq = 0;
         struct stat st;
         dev_t devnum;
@@ -2352,10 +2338,11 @@ _public_ int sd_device_open(sd_device *device, int flags) {
         if (FLAGS_SET(flags, O_PATH))
                 return TAKE_FD(fd);
 
-        r = device_get_property_bool(device, "ID_IGNORE_DISKSEQ");
+        r = sd_device_get_property_value(device, "ID_IGNORE_DISKSEQ", &val);
         if (r < 0 && r != -ENOENT)
                 return r;
-        if (r <= 0) {
+
+        if (!val || parse_boolean(val) <= 0) {
                 r = sd_device_get_diskseq(device, &diskseq);
                 if (r < 0 && r != -ENOENT)
                         return r;