From 918e6f1c0151429f5095355f4f3f74f16e79724a Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Fri, 6 Apr 2018 15:02:10 +0200 Subject: [PATCH 1/2] device: make sure to always retroactively start device dependencies PID1 updates the state of device units upon 2 different events: - when it processes an event sent by udev and in this case the device deps are started if the device enters in the "plugged" state. - when it enumerates all devices during its startup or when it is asked to reload its configuration data but in this case the device deps (if any) are not retroactively started. When udev processes a new "add" kernel event, it first registers the new device in its databases then sends an event to systemd. If for any reason, systemd is asked to reload its configuration between the previous 2 steps, it might see for the first time the new device while scanning /sys for all devices. Only during a second step, udev will send the event for the new device. In this peculiar case the device deps wont be started (even though the device is first seen by PID1). Indeed when reloading its configurations, PID1 will put the device unit in the "plugged" state but without starting the device deps. Thereafter PID1 will get the event from udev for the new device but the device unit will be in "plugged" state already therefore it won't see any need to start the device dependencies. Rather than assuming that during the reloading of systemd manager configuration all devices listed in udev DBs have been already processed and should be put in the "plugged" state (done by device_coldplug()), this patch does that only for devices which have been processed via an udev event (device_dispatch_io()) previously. In this case we set "d->found" to "DEVICE_FOUND_UDEV" and we make also sure to no more initialize "d->found" while enumerating devices. Instead this field is now saved/restored while devices are serialized. --- src/core/device.c | 31 ++++++++++++++++++++++++++++++- src/core/device.h | 12 ++++++++---- 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/src/core/device.c b/src/core/device.c index 6bbbd9466f..aafc97ffd4 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -17,6 +17,7 @@ #include "parse-util.h" #include "path-util.h" #include "stat-util.h" +#include "string-table.h" #include "string-util.h" #include "swap.h" #include "udev-util.h" @@ -132,6 +133,13 @@ static int device_coldplug(Unit *u) { assert(d); assert(d->state == DEVICE_DEAD); + /* This should happen only when we reexecute PID1 from an old version + * which didn't serialize d->found. In this case simply assume that the + * device was in plugged state right before we started reexecuting which + * might be a wrong assumption. */ + if (d->found == DEVICE_FOUND_UDEV_DB) + d->found = DEVICE_FOUND_UDEV; + if (d->found & DEVICE_FOUND_UDEV) /* If udev says the device is around, it's around */ device_set_state(d, DEVICE_PLUGGED); @@ -152,6 +160,7 @@ static int device_serialize(Unit *u, FILE *f, FDSet *fds) { assert(fds); unit_serialize_item(u, f, "state", device_state_to_string(d->state)); + unit_serialize_item(u, f, "found", device_found_to_string(d->found)); return 0; } @@ -172,6 +181,16 @@ static int device_deserialize_item(Unit *u, const char *key, const char *value, log_unit_debug(u, "Failed to parse state value: %s", value); else d->deserialized_state = state; + + } else if (streq(key, "found")) { + DeviceFound found; + + found = device_found_from_string(value); + if (found < 0) + log_unit_debug(u, "Failed to parse found value: %s", value); + else + d->found = found; + } else log_unit_debug(u, "Unknown serialization key: %s", key); @@ -731,7 +750,7 @@ static void device_enumerate(Manager *m) { (void) device_process_new(m, dev); - device_update_found_by_sysfs(m, sysfs, true, DEVICE_FOUND_UDEV, false); + device_update_found_by_sysfs(m, sysfs, true, DEVICE_FOUND_UDEV_DB, false); } return; @@ -903,6 +922,16 @@ bool device_shall_be_bound_by(Unit *device, Unit *u) { return DEVICE(device)->bind_mounts; } +static const char* const device_found_table[] = { + [DEVICE_NOT_FOUND] = "not-found", + [DEVICE_FOUND_UDEV] = "found-udev", + [DEVICE_FOUND_UDEV_DB] = "found-udev-db", + [DEVICE_FOUND_MOUNT] = "found-mount", + [DEVICE_FOUND_SWAP] = "found-swap", +}; + +DEFINE_STRING_TABLE_LOOKUP(device_found, DeviceFound); + const UnitVTable device_vtable = { .object_size = sizeof(Device), .sections = diff --git a/src/core/device.h b/src/core/device.h index 4b970a0d6a..a96aa6d24d 100644 --- a/src/core/device.h +++ b/src/core/device.h @@ -10,10 +10,11 @@ typedef struct Device Device; typedef enum DeviceFound { - DEVICE_NOT_FOUND = 0, - DEVICE_FOUND_UDEV = 1, - DEVICE_FOUND_MOUNT = 2, - DEVICE_FOUND_SWAP = 4, + DEVICE_NOT_FOUND = 0, + DEVICE_FOUND_UDEV = 1 << 1, + DEVICE_FOUND_UDEV_DB = 1 << 2, + DEVICE_FOUND_MOUNT = 1 << 3, + DEVICE_FOUND_SWAP = 1 << 4, } DeviceFound; struct Device { @@ -36,3 +37,6 @@ extern const UnitVTable device_vtable; int device_found_node(Manager *m, const char *node, bool add, DeviceFound found, bool now); bool device_shall_be_bound_by(Unit *device, Unit *u); + +const char *device_found_to_string(DeviceFound f) _const_; +DeviceFound device_found_from_string(const char *s) _pure_; From 036d2eefaee2b64e7946ef19de9b0b94b60e8955 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Tue, 17 Apr 2018 15:12:06 +0200 Subject: [PATCH 2/2] device: skip deserialization of device units when udevd is not running Do not try to party initialize a device during deserialization if it's not known by udev (anymore) and therefore hasn't been seen during device enumeration. The device unit in this case has not been initialized properly and setting it in the "plugged" state can be confusing. Actually this happens during every boots when PID switches to the new rootfs: PID is reexecuted and enumerates devices but since udev is not running, the list of enumerated devices is empty. --- src/core/device.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/core/device.c b/src/core/device.c index aafc97ffd4..12dccdb975 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -173,6 +173,18 @@ static int device_deserialize_item(Unit *u, const char *key, const char *value, assert(value); assert(fds); + /* The device was known at the time units were serialized but it's not + * anymore at the time units are deserialized. This happens when PID1 is + * re-executed after having switched to the new rootfs: devices were + * enumerated but udevd wasn't running yet thus the list of devices + * (handled by systemd) to initialize was empty. In such case we wait + * for the device events to be re-triggered by udev so device units are + * properly re-initialized. */ + if (d->found == DEVICE_NOT_FOUND) { + assert(d->sysfs == NULL); + return 0; + } + if (streq(key, "state")) { DeviceState state;