Skip to content

Commit

Permalink
device: rework the logic use to trust device enumeration
Browse files Browse the repository at this point in the history
Rather than assuming that the device enumeration can be trusted based on the
manager state (reloading vs reexecuting), we now simply look at the udev DB
itself - ie if no single devices couldn't have been enumerated then we assume
that the DB is not yet initialized [1] and thereby don't trust the enumeration
phase.

In this case we restore the serialized state and rely on events retriggering to
update the view on device PID1 had before it reexecuted.

[1] Maybe there's a better way to detect this state.

Fixes: systemd#12953
  • Loading branch information
fbuihuu committed Jul 10, 2019
1 parent ff029ac commit 6fa8516
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 22 deletions.
16 changes: 13 additions & 3 deletions src/core/device.c
Expand Up @@ -176,6 +176,7 @@ static int device_coldplug(Unit *u) {
}

static void device_catchup(Unit *u) {
Manager *m = u->manager;
Device *d = DEVICE(u);

assert(d);
Expand All @@ -184,6 +185,9 @@ static void device_catchup(Unit *u) {
* "catchup" phase, the manager is up and running thus the enumerated state will
* pull new deps if any */

if (!m->honor_device_enumeration)
return;

if (d->enumerated_found == d->found)
return;

Expand Down Expand Up @@ -682,11 +686,11 @@ static void device_update_found_one(Device *d, DeviceFound found, DeviceFound ma

m = UNIT(d)->manager;

if (MANAGER_IS_RUNNING(m) && (m->honor_device_enumeration || MANAGER_IS_USER(m))) {
if (MANAGER_IS_RUNNING(m)) {
DeviceFound n, previous;

/* When we are already running, then apply the new mask right-away, and trigger state changes
* right-away */
/* When we are already running, then apply the new mask right-away, and
* trigger state changes right-away */

n = (d->found & ~mask) | (found & mask);
if (n == d->found)
Expand Down Expand Up @@ -864,10 +868,16 @@ static void device_enumerate(Manager *m) {
goto fail;
}

m->honor_device_enumeration = false;

FOREACH_DEVICE(e, dev) {
Device *d, *n;
const char *sysfs;

/* Looks like udev's DB is consistent so let's trust the device
* enumeration phase */
m->honor_device_enumeration = true;

if (!device_is_ready(dev))
continue;

Expand Down
19 changes: 0 additions & 19 deletions src/core/manager.c
Expand Up @@ -1625,8 +1625,6 @@ static void manager_ready(Manager *m) {

/* Let's finally catch up with any changes that took place while we were reloading/reexecing */
manager_catchup(m);

m->honor_device_enumeration = true;
}

static Manager* manager_reloading_start(Manager *m) {
Expand Down Expand Up @@ -3160,9 +3158,6 @@ int manager_serialize(
(void) serialize_bool(f, "taint-logged", m->taint_logged);
(void) serialize_bool(f, "service-watchdogs", m->service_watchdogs);

/* After switching root, udevd has not been started yet. So, enumeration results should not be emitted. */
(void) serialize_bool(f, "honor-device-enumeration", !switching_root);

t = show_status_to_string(m->show_status);
if (t)
(void) serialize_item(f, "show-status", t);
Expand Down Expand Up @@ -3391,15 +3386,6 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
else
m->service_watchdogs = b;

} else if ((val = startswith(l, "honor-device-enumeration="))) {
int b;

b = parse_boolean(val);
if (b < 0)
log_notice("Failed to parse honor-device-enumeration flag '%s', ignoring.", val);
else
m->honor_device_enumeration = b;

} else if ((val = startswith(l, "show-status="))) {
ShowStatus s;

Expand Down Expand Up @@ -3590,11 +3576,6 @@ int manager_reload(Manager *m) {
assert(m->n_reloading > 0);
m->n_reloading--;

/* On manager reloading, device tag data should exists, thus, we should honor the results of device
* enumeration. The flag should be always set correctly by the serialized data, but it may fail. So,
* let's always set the flag here for safety. */
m->honor_device_enumeration = true;

manager_ready(m);

m->send_reloading_done = true;
Expand Down

0 comments on commit 6fa8516

Please sign in to comment.