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.
This commit is contained in:
Franck Bui 2018-04-06 15:02:10 +02:00
parent 201b26a344
commit 918e6f1c01
2 changed files with 38 additions and 5 deletions

View File

@ -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 =

View File

@ -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_;