path: root/0001-core-rework-device-state-logic.patch

From 628c89cc68ab96fce2de7ebba5933725d147aecc Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Fri, 27 Feb 2015 21:55:08 +0100
Subject: [PATCH] core: rework device state logic

This change introduces a new state "tentative" for device units. Device
units are considered "plugged" when udev announced them, "dead" when
they are not available in the kernel, and "tentative" when they are
referenced in /proc/self/mountinfo or /proc/swaps but not (yet)
announced via udev.

This should fix a race when device nodes (like loop devices) are created
and immediately mounted. Previously, systemd might end up seeing the
mount unit before the device, and would thus pull down the mount because
its BindTo dependency on the device would not be fulfilled.
---
 src/core/device.c | 368 +++++++++++++++++++++++++++++++++---------------------
 src/core/device.h |  14 ++-
 src/core/mount.c  |  46 ++++---
 src/core/swap.c   |  32 +++--
 src/core/swap.h   |   4 +-
 src/core/unit.c   |   1 -
 6 files changed, 285 insertions(+), 180 deletions(-)

diff --git a/src/core/device.c b/src/core/device.c
index 2d983cc..e41ed41 100644
--- a/src/core/device.c
+++ b/src/core/device.c
@@ -34,7 +34,8 @@
 
 static const UnitActiveState state_translation_table[_DEVICE_STATE_MAX] = {
         [DEVICE_DEAD] = UNIT_INACTIVE,
-        [DEVICE_PLUGGED] = UNIT_ACTIVE
+        [DEVICE_TENTATIVE] = UNIT_ACTIVATING,
+        [DEVICE_PLUGGED] = UNIT_ACTIVE,
 };
 
 static int device_dispatch_io(sd_event_source *source, int fd, uint32_t revents, void *userdata);
@@ -63,6 +64,41 @@ static void device_unset_sysfs(Device *d) {
         d->sysfs = NULL;
 }
 
+static int device_set_sysfs(Device *d, const char *sysfs) {
+        Device *first;
+        char *copy;
+        int r;
+
+        assert(d);
+
+        if (streq_ptr(d->sysfs, sysfs))
+                return 0;
+
+        r = hashmap_ensure_allocated(&UNIT(d)->manager->devices_by_sysfs, &string_hash_ops);
+        if (r < 0)
+                return r;
+
+        copy = strdup(sysfs);
+        if (!copy)
+                return -ENOMEM;
+
+        device_unset_sysfs(d);
+
+        first = hashmap_get(UNIT(d)->manager->devices_by_sysfs, sysfs);
+        LIST_PREPEND(same_sysfs, first, d);
+
+        r = hashmap_replace(UNIT(d)->manager->devices_by_sysfs, copy, first);
+        if (r < 0) {
+                LIST_REMOVE(same_sysfs, first, d);
+                free(copy);
+                return r;
+        }
+
+        d->sysfs = copy;
+
+        return 0;
+}
+
 static void device_init(Unit *u) {
         Device *d = DEVICE(u);
 
@@ -110,8 +146,13 @@ static int device_coldplug(Unit *u) {
         assert(d);
         assert(d->state == DEVICE_DEAD);
 
-        if (d->sysfs)
+        if (d->found & DEVICE_FOUND_UDEV)
+                /* If udev says the device is around, it's around */
                 device_set_state(d, DEVICE_PLUGGED);
+        else if (d->found != DEVICE_NOT_FOUND)
+                /* If a device is found in /proc/self/mountinfo or
+                 * /proc/swaps, it's "tentatively" around. */
+                device_set_state(d, DEVICE_TENTATIVE);
 
         return 0;
 }
@@ -140,49 +181,9 @@ _pure_ static const char *device_sub_state_to_string(Unit *u) {
         return device_state_to_string(DEVICE(u)->state);
 }
 
-static int device_add_escaped_name(Unit *u, const char *dn) {
-        _cleanup_free_ char *e = NULL;
-        int r;
-
-        assert(u);
-        assert(dn);
-        assert(dn[0] == '/');
-
-        e = unit_name_from_path(dn, ".device");
-        if (!e)
-                return -ENOMEM;
-
-        r = unit_add_name(u, e);
-        if (r < 0 && r != -EEXIST)
-                return r;
-
-        return 0;
-}
-
-static int device_find_escape_name(Manager *m, const char *dn, Unit **_u) {
-        _cleanup_free_ char *e = NULL;
-        Unit *u;
-
-        assert(m);
-        assert(dn);
-        assert(dn[0] == '/');
-        assert(_u);
-
-        e = unit_name_from_path(dn, ".device");
-        if (!e)
-                return -ENOMEM;
-
-        u = manager_get_unit(m, e);
-        if (u) {
-                *_u = u;
-                return 1;
-        }
-
-        return 0;
-}
-
-static int device_make_description(Unit *u, struct udev_device *dev, const char *path) {
+static int device_update_description(Unit *u, struct udev_device *dev, const char *path) {
         const char *model;
+        int r;
 
         assert(u);
         assert(dev);
@@ -207,13 +208,16 @@ static int device_make_description(Unit *u, struct udev_device *dev, const char
 
                         j = strjoin(model, " ", label, NULL);
                         if (j)
-                                return unit_set_description(u, j);
-                }
+                                r = unit_set_description(u, j);
+                } else
+                        r = unit_set_description(u, model);
+        } else
+                r = unit_set_description(u, path);
 
-                return unit_set_description(u, model);
-        }
+        if (r < 0)
+                log_unit_error_errno(u->id, r, "Failed to set device description: %m");
 
-        return unit_set_description(u, path);
+        return r;
 }
 
 static int device_add_udev_wants(Unit *u, struct udev_device *dev) {
@@ -240,20 +244,20 @@ static int device_add_udev_wants(Unit *u, struct udev_device *dev) {
 
                 n = unit_name_mangle(e, MANGLE_NOGLOB);
                 if (!n)
-                        return -ENOMEM;
+                        return log_oom();
 
                 r = unit_add_dependency_by_name(u, UNIT_WANTS, n, NULL, true);
                 if (r < 0)
-                        return r;
+                        return log_unit_error_errno(u->id, r, "Failed to add wants dependency: %m");
         }
         if (!isempty(state))
-                log_unit_warning(u->id, "Property %s on %s has trailing garbage, ignoring.",
-                                 property, strna(udev_device_get_syspath(dev)));
+                log_unit_warning(u->id, "Property %s on %s has trailing garbage, ignoring.", property, strna(udev_device_get_syspath(dev)));
 
         return 0;
 }
 
-static int device_update_unit(Manager *m, struct udev_device *dev, const char *path, bool main) {
+static int device_setup_unit(Manager *m, struct udev_device *dev, const char *path, bool main) {
+        _cleanup_free_ char *e = NULL;
         const char *sysfs;
         Unit *u = NULL;
         bool delete;
@@ -267,12 +271,18 @@ static int device_update_unit(Manager *m, struct udev_device *dev, const char *p
         if (!sysfs)
                 return 0;
 
-        r = device_find_escape_name(m, path, &u);
-        if (r < 0)
-                return r;
+        e = unit_name_from_path(path, ".device");
+        if (!e)
+                return log_oom();
+
+        u = manager_get_unit(m, e);
 
-        if (u && DEVICE(u)->sysfs && !path_equal(DEVICE(u)->sysfs, sysfs))
+        if (u &&
+            DEVICE(u)->sysfs &&
+            !path_equal(DEVICE(u)->sysfs, sysfs)) {
+                log_unit_error(u->id, "Device %s appeared twice with different sysfs paths %s and %s", e, DEVICE(u)->sysfs, sysfs);
                 return -EEXIST;
+        }
 
         if (!u) {
                 delete = true;
@@ -281,7 +291,7 @@ static int device_update_unit(Manager *m, struct udev_device *dev, const char *p
                 if (!u)
                         return log_oom();
 
-                r = device_add_escaped_name(u, path);
+                r = unit_add_name(u, e);
                 if (r < 0)
                         goto fail;
 
@@ -293,37 +303,16 @@ static int device_update_unit(Manager *m, struct udev_device *dev, const char *p
          * actually been seen yet ->sysfs will not be
          * initialized. Hence initialize it if necessary. */
 
-        if (!DEVICE(u)->sysfs) {
-                Device *first;
-
-                DEVICE(u)->sysfs = strdup(sysfs);
-                if (!DEVICE(u)->sysfs) {
-                        r = -ENOMEM;
-                        goto fail;
-                }
-
-                r = hashmap_ensure_allocated(&m->devices_by_sysfs, &string_hash_ops);
-                if (r < 0)
-                        goto fail;
-
-                first = hashmap_get(m->devices_by_sysfs, sysfs);
-                LIST_PREPEND(same_sysfs, first, DEVICE(u));
-
-                r = hashmap_replace(m->devices_by_sysfs, DEVICE(u)->sysfs, first);
-                if (r < 0)
-                        goto fail;
-        }
-
-        device_make_description(u, dev, path);
+        r = device_set_sysfs(DEVICE(u), sysfs);
+        if (r < 0)
+                goto fail;
 
-        if (main) {
-                /* The additional systemd udev properties we only
-                 * interpret for the main object */
+        (void) device_update_description(u, dev, path);
 
-                r = device_add_udev_wants(u, dev);
-                if (r < 0)
-                        goto fail;
-        }
+        /* The additional systemd udev properties we only interpret
+         * for the main object */
+        if (main)
+                (void) device_add_udev_wants(u, dev);
 
         /* Note that this won't dispatch the load queue, the caller
          * has to do that if needed and appropriate */
@@ -332,7 +321,7 @@ static int device_update_unit(Manager *m, struct udev_device *dev, const char *p
         return 0;
 
 fail:
-        log_warning_errno(r, "Failed to load device unit: %m");
+        log_unit_warning_errno(u->id, r, "Failed to set up device unit: %m");
 
         if (delete && u)
                 unit_free(u);
@@ -340,7 +329,7 @@ fail:
         return r;
 }
 
-static int device_process_new_device(Manager *m, struct udev_device *dev) {
+static int device_process_new(Manager *m, struct udev_device *dev) {
         const char *sysfs, *dn, *alias;
         struct udev_list_entry *item = NULL, *first = NULL;
         int r;
@@ -352,14 +341,14 @@ static int device_process_new_device(Manager *m, struct udev_device *dev) {
                 return 0;
 
         /* Add the main unit named after the sysfs path */
-        r = device_update_unit(m, dev, sysfs, true);
+        r = device_setup_unit(m, dev, sysfs, true);
         if (r < 0)
                 return r;
 
         /* Add an additional unit for the device node */
         dn = udev_device_get_devnode(dev);
         if (dn)
-                device_update_unit(m, dev, dn, false);
+                (void) device_setup_unit(m, dev, dn, false);
 
         /* Add additional units for all symlinks */
         first = udev_device_get_devlinks_list_entry(dev);
@@ -386,7 +375,7 @@ static int device_process_new_device(Manager *m, struct udev_device *dev) {
                             st.st_rdev != udev_device_get_devnum(dev))
                                 continue;
 
-                device_update_unit(m, dev, p, false);
+                (void) device_setup_unit(m, dev, p, false);
         }
 
         /* Add additional units for all explicitly configured
@@ -403,7 +392,7 @@ static int device_process_new_device(Manager *m, struct udev_device *dev) {
                         e[l] = 0;
 
                         if (path_is_absolute(e))
-                                device_update_unit(m, dev, e, false);
+                                (void) device_setup_unit(m, dev, e, false);
                         else
                                 log_warning("SYSTEMD_ALIAS for %s is not an absolute path, ignoring: %s", sysfs, e);
                 }
@@ -414,39 +403,62 @@ static int device_process_new_device(Manager *m, struct udev_device *dev) {
         return 0;
 }
 
-static void device_set_path_plugged(Manager *m, struct udev_device *dev) {
-        const char *sysfs;
+static void device_update_found_one(Device *d, bool add, DeviceFound found, bool now) {
+        DeviceFound n;
+
+        assert(d);
+
+        n = add ? (d->found | found) : (d->found & ~found);
+        if (n == d->found)
+                return;
+
+        d->found = n;
+
+        if (now) {
+                if (d->found & DEVICE_FOUND_UDEV)
+                        device_set_state(d, DEVICE_PLUGGED);
+                else if (d->found != DEVICE_NOT_FOUND)
+                        device_set_state(d, DEVICE_TENTATIVE);
+                else
+                        device_set_state(d, DEVICE_DEAD);
+        }
+}
+
+static int device_update_found_by_sysfs(Manager *m, const char *sysfs, bool add, DeviceFound found, bool now) {
         Device *d, *l;
 
         assert(m);
-        assert(dev);
+        assert(sysfs);
 
-        sysfs = udev_device_get_syspath(dev);
-        if (!sysfs)
-                return;
+        if (found == DEVICE_NOT_FOUND)
+                return 0;
 
         l = hashmap_get(m->devices_by_sysfs, sysfs);
         LIST_FOREACH(same_sysfs, d, l)
-                device_set_state(d, DEVICE_PLUGGED);
+                device_update_found_one(d, add, found, now);
+
+        return 0;
 }
 
-static int device_process_removed_device(Manager *m, struct udev_device *dev) {
-        const char *sysfs;
-        Device *d;
+static int device_update_found_by_name(Manager *m, const char *path, bool add, DeviceFound found, bool now) {
+        _cleanup_free_ char *e = NULL;
+        Unit *u;
 
         assert(m);
-        assert(dev);
+        assert(path);
 
-        sysfs = udev_device_get_syspath(dev);
-        if (!sysfs)
-                return -ENOMEM;
+        if (found == DEVICE_NOT_FOUND)
+                return 0;
 
-        /* Remove all units of this sysfs path */
-        while ((d = hashmap_get(m->devices_by_sysfs, sysfs))) {
-                device_unset_sysfs(d);
-                device_set_state(d, DEVICE_DEAD);
-        }
+        e = unit_name_from_path(path, ".device");
+        if (!e)
+                return log_oom();
 
+        u = manager_get_unit(m, e);
+        if (!u)
+                return 0;
+
+        device_update_found_one(DEVICE(u), add, found, now);
         return 0;
 }
 
@@ -462,22 +474,6 @@ static bool device_is_ready(struct udev_device *dev) {
         return parse_boolean(ready) != 0;
 }
 
-static int device_process_new_path(Manager *m, const char *path) {
-        _cleanup_udev_device_unref_ struct udev_device *dev = NULL;
-
-        assert(m);
-        assert(path);
-
-        dev = udev_device_new_from_syspath(m->udev, path);
-        if (!dev)
-                return log_oom();
-
-        if (!device_is_ready(dev))
-                return 0;
-
-        return device_process_new_device(m, dev);
-}
-
 static Unit *device_following(Unit *u) {
         Device *d = DEVICE(u);
         Device *other, *first = NULL;
@@ -604,12 +600,31 @@ static int device_enumerate(Manager *m) {
                 goto fail;
 
         first = udev_enumerate_get_list_entry(e);
-        udev_list_entry_foreach(item, first)
-                device_process_new_path(m, udev_list_entry_get_name(item));
+        udev_list_entry_foreach(item, first) {
+                _cleanup_udev_device_unref_ struct udev_device *dev = NULL;
+                const char *sysfs;
+
+                sysfs = udev_list_entry_get_name(item);
+
+                dev = udev_device_new_from_syspath(m->udev, sysfs);
+                if (!dev) {
+                        log_oom();
+                        continue;
+                }
+
+                if (!device_is_ready(dev))
+                        continue;
+
+                (void) device_process_new(m, dev);
+
+                device_update_found_by_sysfs(m, sysfs, true, DEVICE_FOUND_UDEV, false);
+        }
 
         return 0;
 
 fail:
+        log_error_errno(r, "Failed to enumerate devices: %m");
+
         device_shutdown(m);
         return r;
 }
@@ -617,7 +632,7 @@ fail:
 static int device_dispatch_io(sd_event_source *source, int fd, uint32_t revents, void *userdata) {
         _cleanup_udev_device_unref_ struct udev_device *dev = NULL;
         Manager *m = userdata;
-        const char *action;
+        const char *action, *sysfs;
         int r;
 
         assert(m);
@@ -639,33 +654,47 @@ static int device_dispatch_io(sd_event_source *source, int fd, uint32_t revents,
         if (!dev)
                 return 0;
 
+        sysfs = udev_device_get_syspath(dev);
+        if (!sysfs) {
+                log_error("Failed to get udev sys path.");
+                return 0;
+        }
+
         action = udev_device_get_action(dev);
         if (!action) {
                 log_error("Failed to get udev action string.");
                 return 0;
         }
 
-        if (streq(action, "remove") || !device_is_ready(dev))  {
-                r = device_process_removed_device(m, dev);
-                if (r < 0)
-                        log_error_errno(r, "Failed to process device remove event: %m");
-
-                r = swap_process_removed_device(m, dev);
+        if (streq(action, "remove"))  {
+                r = swap_process_device_remove(m, dev);
                 if (r < 0)
                         log_error_errno(r, "Failed to process swap device remove event: %m");
 
-        } else {
-                r = device_process_new_device(m, dev);
-                if (r < 0)
-                        log_error_errno(r, "Failed to process device new event: %m");
+                /* 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, false, DEVICE_FOUND_UDEV|DEVICE_FOUND_MOUNT|DEVICE_FOUND_SWAP, true);
 
-                r = swap_process_new_device(m, dev);
+        } else if (device_is_ready(dev)) {
+
+                (void) device_process_new(m, dev);
+
+                r = swap_process_device_new(m, dev);
                 if (r < 0)
                         log_error_errno(r, "Failed to process swap device new event: %m");
 
                 manager_dispatch_load_queue(m);
 
-                device_set_path_plugged(m, dev);
+                /* The device is found now, set the udev found bit */
+                device_update_found_by_sysfs(m, sysfs, true, DEVICE_FOUND_UDEV, true);
+
+        } else {
+                /* The device is nominally around, but not ready for
+                 * us. Hence unset the udev bit, but leave the rest
+                 * around. */
+
+                device_update_found_by_sysfs(m, sysfs, false, DEVICE_FOUND_UDEV, true);
         }
 
         return 0;
@@ -684,9 +713,58 @@ static bool device_supported(Manager *m) {
         return read_only <= 0;
 }
 
+int device_found_node(Manager *m, const char *node, bool add, DeviceFound found, bool now) {
+        _cleanup_udev_device_unref_ struct udev_device *dev = NULL;
+        struct stat st;
+
+        assert(m);
+        assert(node);
+
+        /* This is called whenever we find a device referenced in
+         * /proc/swaps or /proc/self/mounts. Such a device might be
+         * mounted/enabled at a time where udev has not finished
+         * probing it yet, and we thus haven't learned about it
+         * yet. In this case we will set the device unit to
+         * "tentative" state. */
+
+        if (add) {
+                if (!path_startswith(node, "/dev"))
+                        return 0;
+
+                if (stat(node, &st) < 0) {
+                        if (errno == ENOENT)
+                                return 0;
+
+                        return log_error_errno(errno, "Failed to stat device node file %s: %m", node);
+                }
+
+                if (!S_ISBLK(st.st_mode) && !S_ISCHR(st.st_mode))
+                        return 0;
+
+                dev = udev_device_new_from_devnum(m->udev, S_ISBLK(st.st_mode) ? 'b' : 'c', st.st_rdev);
+                if (!dev) {
+                        if (errno == ENOENT)
+                                return 0;
+
+                        return log_oom();
+                }
+
+                /* If the device is known in the kernel and newly
+                 * appeared, then we'll create a device unit for it,
+                 * under the name referenced in /proc/swaps or
+                 * /proc/self/mountinfo. */
+
+                (void) device_setup_unit(m, dev, node, false);
+        }
+
+        /* Update the device unit's state, should it exist */
+        return device_update_found_by_name(m, node, add, found, now);
+}
+
 static const char* const device_state_table[_DEVICE_STATE_MAX] = {
         [DEVICE_DEAD] = "dead",
-        [DEVICE_PLUGGED] = "plugged"
+        [DEVICE_TENTATIVE] = "tentative",
+        [DEVICE_PLUGGED] = "plugged",
 };
 
 DEFINE_STRING_TABLE_LOOKUP(device_state, DeviceState);
diff --git a/src/core/device.h b/src/core/device.h
index 9065085..9f46e08 100644
--- a/src/core/device.h
+++ b/src/core/device.h
@@ -28,20 +28,28 @@ typedef struct Device Device;
  * simplifies the state engine greatly */
 typedef enum DeviceState {
         DEVICE_DEAD,
-        DEVICE_PLUGGED,
+        DEVICE_TENTATIVE, /* mounted or swapped, but not (yet) announced by udev */
+        DEVICE_PLUGGED,   /* announced by udev */
         _DEVICE_STATE_MAX,
         _DEVICE_STATE_INVALID = -1
 } DeviceState;
 
+typedef enum DeviceFound {
+        DEVICE_NOT_FOUND = 0,
+        DEVICE_FOUND_UDEV = 1,
+        DEVICE_FOUND_MOUNT = 2,
+        DEVICE_FOUND_SWAP = 4,
+} DeviceFound;
+
 struct Device {
         Unit meta;
 
         char *sysfs;
+        DeviceFound found;
 
         /* In order to be able to distinguish dependencies on
         different device nodes we might end up creating multiple
         devices for the same sysfs path. We chain them up here. */
-
         LIST_FIELDS(struct Device, same_sysfs);
 
         DeviceState state;
@@ -51,3 +59,5 @@ extern const UnitVTable device_vtable;
 
 const char* device_state_to_string(DeviceState i) _const_;
 DeviceState device_state_from_string(const char *s) _pure_;
+
+int device_found_node(Manager *m, const char *node, bool add, DeviceFound found, bool now);
diff --git a/src/core/mount.c b/src/core/mount.c
index 40037e7..8e4a376 100644
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -1386,7 +1386,7 @@ static int mount_dispatch_timer(sd_event_source *source, usec_t usec, void *user
         return 0;
 }
 
-static int mount_add_one(
+static int mount_setup_unit(
                 Manager *m,
                 const char *what,
                 const char *where,
@@ -1429,7 +1429,7 @@ static int mount_add_one(
 
                 u = unit_new(m, sizeof(Mount));
                 if (!u)
-                        return -ENOMEM;
+                        return log_oom();
 
                 r = unit_add_name(u, e);
                 if (r < 0)
@@ -1542,6 +1542,8 @@ static int mount_add_one(
         return 0;
 
 fail:
+        log_warning_errno(r, "Failed to set up mount unit: %m");
+
         if (delete && u)
                 unit_free(u);
 
@@ -1549,33 +1551,36 @@ fail:
 }
 
 static int mount_load_proc_self_mountinfo(Manager *m, bool set_flags) {
-        _cleanup_(mnt_free_tablep) struct libmnt_table *tb = NULL;
-        _cleanup_(mnt_free_iterp) struct libmnt_iter *itr = NULL;
-        struct libmnt_fs *fs;
+        _cleanup_(mnt_free_tablep) struct libmnt_table *t = NULL;
+        _cleanup_(mnt_free_iterp) struct libmnt_iter *i = NULL;
         int r = 0;
 
         assert(m);
 
-        tb = mnt_new_table();
-        itr = mnt_new_iter(MNT_ITER_FORWARD);
-        if (!tb || !itr)
+        t = mnt_new_table();
+        if (!t)
                 return log_oom();
 
-        r = mnt_table_parse_mtab(tb, NULL);
+        i = mnt_new_iter(MNT_ITER_FORWARD);
+        if (!i)
+                return log_oom();
+
+        r = mnt_table_parse_mtab(t, NULL);
         if (r < 0)
-                return r;
+                return log_error_errno(r, "Failed to parse /proc/self/mountinfo: %m");
 
         r = 0;
         for (;;) {
                 const char *device, *path, *options, *fstype;
                 _cleanup_free_ const char *d = NULL, *p = NULL;
+                struct libmnt_fs *fs;
                 int k;
 
-                k = mnt_table_next_fs(tb, itr, &fs);
+                k = mnt_table_next_fs(t, i, &fs);
                 if (k == 1)
                         break;
-                else if (k < 0)
-                        return log_error_errno(k, "Failed to get next entry from /etc/fstab: %m");
+                if (k < 0)
+                        return log_error_errno(k, "Failed to get next entry from /proc/self/mountinfo: %m");
 
                 device = mnt_fs_get_source(fs);
                 path = mnt_fs_get_target(fs);
@@ -1583,11 +1588,16 @@ static int mount_load_proc_self_mountinfo(Manager *m, bool set_flags) {
                 fstype = mnt_fs_get_fstype(fs);
 
                 d = cunescape(device);
+                if (!d)
+                        return log_oom();
+
                 p = cunescape(path);
-                if (!d || !p)
+                if (!p)
                         return log_oom();
 
-                k = mount_add_one(m, d, p, options, fstype, set_flags);
+                (void) device_found_node(m, d, true, DEVICE_FOUND_MOUNT, set_flags);
+
+                k = mount_setup_unit(m, d, p, options, fstype, set_flags);
                 if (r == 0 && k < 0)
                         r = k;
         }
@@ -1731,8 +1741,6 @@ static int mount_dispatch_io(sd_event_source *source, int fd, uint32_t revents,
 
         r = mount_load_proc_self_mountinfo(m, true);
         if (r < 0) {
-                log_error_errno(r, "Failed to reread /proc/self/mountinfo: %m");
-
                 /* Reset flags, just in case, for later calls */
                 LIST_FOREACH(units_by_type, u, m->units_by_type[UNIT_MOUNT]) {
                         Mount *mount = MOUNT(u);
@@ -1765,6 +1773,10 @@ static int mount_dispatch_io(sd_event_source *source, int fd, uint32_t revents,
                                 break;
                         }
 
+                        if (mount->parameters_proc_self_mountinfo.what)
+                                (void) device_found_node(m, mount->parameters_proc_self_mountinfo.what, false, DEVICE_FOUND_MOUNT, true);
+
+
                 } else if (mount->just_mounted || mount->just_changed) {
 
                         /* New or changed mount entry */
diff --git a/src/core/swap.c b/src/core/swap.c
index f73a8e6..de3a5d8 100644
--- a/src/core/swap.c
+++ b/src/core/swap.c
@@ -331,7 +331,7 @@ static int swap_load(Unit *u) {
         return swap_verify(s);
 }
 
-static int swap_add_one(
+static int swap_setup_unit(
                 Manager *m,
                 const char *what,
                 const char *what_proc_swaps,
@@ -356,8 +356,10 @@ static int swap_add_one(
 
         if (u &&
             SWAP(u)->from_proc_swaps &&
-            !path_equal(SWAP(u)->parameters_proc_swaps.what, what_proc_swaps))
+            !path_equal(SWAP(u)->parameters_proc_swaps.what, what_proc_swaps)) {
+                log_error("Swap %s appeared twice with different device paths %s and %s", e, SWAP(u)->parameters_proc_swaps.what, what_proc_swaps);
                 return -EEXIST;
+        }
 
         if (!u) {
                 delete = true;
@@ -372,7 +374,7 @@ static int swap_add_one(
 
                 SWAP(u)->what = strdup(what);
                 if (!SWAP(u)->what) {
-                        r = log_oom();
+                        r = -ENOMEM;
                         goto fail;
                 }
 
@@ -400,7 +402,6 @@ static int swap_add_one(
         p->priority = priority;
 
         unit_add_to_dbus_queue(u);
-
         return 0;
 
 fail:
@@ -412,7 +413,7 @@ fail:
         return r;
 }
 
-static int swap_process_new_swap(Manager *m, const char *device, int prio, bool set_flags) {
+static int swap_process_new(Manager *m, const char *device, int prio, bool set_flags) {
         _cleanup_udev_device_unref_ struct udev_device *d = NULL;
         struct udev_list_entry *item = NULL, *first = NULL;
         const char *dn;
@@ -421,7 +422,7 @@ static int swap_process_new_swap(Manager *m, const char *device, int prio, bool
 
         assert(m);
 
-        r = swap_add_one(m, device, device, prio, set_flags);
+        r = swap_setup_unit(m, device, device, prio, set_flags);
         if (r < 0)
                 return r;
 
@@ -437,7 +438,7 @@ static int swap_process_new_swap(Manager *m, const char *device, int prio, bool
         /* Add the main device node */
         dn = udev_device_get_devnode(d);
         if (dn && !streq(dn, device))
-                swap_add_one(m, dn, device, prio, set_flags);
+                swap_setup_unit(m, dn, device, prio, set_flags);
 
         /* Add additional units for all symlinks */
         first = udev_device_get_devlinks_list_entry(d);
@@ -458,7 +459,7 @@ static int swap_process_new_swap(Manager *m, const char *device, int prio, bool
                             st.st_rdev != udev_device_get_devnum(d))
                                 continue;
 
-                swap_add_one(m, p, device, prio, set_flags);
+                swap_setup_unit(m, p, device, prio, set_flags);
         }
 
         return r;
@@ -1084,15 +1085,17 @@ static int swap_load_proc_swaps(Manager *m, bool set_flags) {
                         if (k == EOF)
                                 break;
 
-                        log_warning("Failed to parse /proc/swaps:%u", i);
+                        log_warning("Failed to parse /proc/swaps:%u.", i);
                         continue;
                 }
 
                 d = cunescape(dev);
                 if (!d)
-                        return -ENOMEM;
+                        return log_oom();
+
+                device_found_node(m, d, true, DEVICE_FOUND_SWAP, set_flags);
 
-                k = swap_process_new_swap(m, d, prio, set_flags);
+                k = swap_process_new(m, d, prio, set_flags);
                 if (k < 0)
                         r = k;
         }
@@ -1144,6 +1147,9 @@ static int swap_dispatch_io(sd_event_source *source, int fd, uint32_t revents, v
                                 break;
                         }
 
+                        if (swap->what)
+                                device_found_node(m, swap->what, false, DEVICE_FOUND_SWAP, true);
+
                 } else if (swap->just_activated) {
 
                         /* New swap entry */
@@ -1291,7 +1297,7 @@ fail:
         return r;
 }
 
-int swap_process_new_device(Manager *m, struct udev_device *dev) {
+int swap_process_device_new(Manager *m, struct udev_device *dev) {
         struct udev_list_entry *item = NULL, *first = NULL;
         _cleanup_free_ char *e = NULL;
         const char *dn;
@@ -1334,7 +1340,7 @@ int swap_process_new_device(Manager *m, struct udev_device *dev) {
         return r;
 }
 
-int swap_process_removed_device(Manager *m, struct udev_device *dev) {
+int swap_process_device_remove(Manager *m, struct udev_device *dev) {
         const char *dn;
         int r = 0;
         Swap *s;
diff --git a/src/core/swap.h b/src/core/swap.h
index c36c6f2..5de8c20 100644
--- a/src/core/swap.h
+++ b/src/core/swap.h
@@ -115,8 +115,8 @@ struct Swap {
 
 extern const UnitVTable swap_vtable;
 
-int swap_process_new_device(Manager *m, struct udev_device *dev);
-int swap_process_removed_device(Manager *m, struct udev_device *dev);
+int swap_process_device_new(Manager *m, struct udev_device *dev);
+int swap_process_device_remove(Manager *m, struct udev_device *dev);
 
 const char* swap_state_to_string(SwapState i) _const_;
 SwapState swap_state_from_string(const char *s) _pure_;
diff --git a/src/core/unit.c b/src/core/unit.c
index 63ccd67..7cd7043 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -2834,7 +2834,6 @@ int unit_add_node_link(Unit *u, const char *what, bool wants) {
                 return -ENOMEM;
 
         r = manager_load_unit(u->manager, e, NULL, NULL, &device);
-
         if (r < 0)
                 return r;
 
-- 
2.3.5