From 87a19bfedcd132822974fa096f03e0e0b6de1099 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 15 Mar 2019 18:59:31 +0900 Subject: [PATCH 1/4] core: use _cleanup_free_ attribute and free_and_replace() macro in method_switch_root() --- src/core/dbus-manager.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 88e4c6bb95..c52f94c8f2 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -1490,7 +1490,7 @@ static int method_kexec(sd_bus_message *message, void *userdata, sd_bus_error *e } static int method_switch_root(sd_bus_message *message, void *userdata, sd_bus_error *error) { - char *ri = NULL, *rt = NULL; + _cleanup_free_ char *ri = NULL, *rt = NULL; const char *root, *init; Manager *m = userdata; struct statvfs svfs; @@ -1562,17 +1562,12 @@ static int method_switch_root(sd_bus_message *message, void *userdata, sd_bus_er if (!isempty(init)) { ri = strdup(init); - if (!ri) { - free(rt); + if (!ri) return -ENOMEM; - } } - free(m->switch_root); - m->switch_root = rt; - - free(m->switch_root_init); - m->switch_root_init = ri; + free_and_replace(m->switch_root, rt); + free_and_replace(m->switch_root_init, ri); m->objective = MANAGER_SWITCH_ROOT; From 49052946c91d37c6537de81180f10e662c7c7872 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 15 Mar 2019 19:01:12 +0900 Subject: [PATCH 2/4] core: use TAKE_PTR() at few more places --- src/core/main.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/core/main.c b/src/core/main.c index 981513bbc3..1617b23767 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -1908,9 +1908,8 @@ static int invoke_main_loop( *ret_shutdown_verb = NULL; /* Steal the switch root parameters */ - *ret_switch_root_dir = m->switch_root; - *ret_switch_root_init = m->switch_root_init; - m->switch_root = m->switch_root_init = NULL; + *ret_switch_root_dir = TAKE_PTR(m->switch_root); + *ret_switch_root_init = TAKE_PTR(m->switch_root_init); return 0; From c6e892bc0eebe1d42c282bd2d8bae149fbeba85f Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 15 Mar 2019 19:37:43 +0900 Subject: [PATCH 3/4] core: add Manager::honor_device_enumeration flag When system manager is started first time or after switching root, then the udev's device tag data do not exist yet. So, let's not honor the enumeration results. Fixes #11997. --- src/core/device.c | 6 +++++- src/core/manager.c | 19 +++++++++++++++++++ src/core/manager.h | 2 ++ 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/core/device.c b/src/core/device.c index 506bf74478..382c380498 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -666,9 +666,13 @@ static void device_found_changed(Device *d, DeviceFound previous, DeviceFound no } static void device_update_found_one(Device *d, DeviceFound found, DeviceFound mask) { + Manager *m; + assert(d); - if (MANAGER_IS_RUNNING(UNIT(d)->manager)) { + m = UNIT(d)->manager; + + if (MANAGER_IS_RUNNING(m) && (m->honor_device_enumeration || MANAGER_IS_USER(m))) { DeviceFound n, previous; /* When we are already running, then apply the new mask right-away, and trigger state changes diff --git a/src/core/manager.c b/src/core/manager.c index eecf48dea5..3f4dbe64e3 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -1619,6 +1619,8 @@ 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) { @@ -3131,6 +3133,9 @@ 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); @@ -3359,6 +3364,15 @@ 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; @@ -3549,6 +3563,11 @@ 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; diff --git a/src/core/manager.h b/src/core/manager.h index bce8020cfd..86b9ec202d 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -395,6 +395,8 @@ struct Manager { * multiple times on the same unit. */ unsigned sigchldgen; unsigned notifygen; + + bool honor_device_enumeration; }; #define MANAGER_IS_SYSTEM(m) ((m)->unit_file_scope == UNIT_FILE_SYSTEM) From 8cc6727a5a126266eb5582ccf85833e2acfeed62 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 15 Mar 2019 20:26:45 +0900 Subject: [PATCH 4/4] test: add a testcase for device plugged -> dead -> plugged bug --- test/TEST-31-DEVICE-ENUMERATION/Makefile | 1 + test/TEST-31-DEVICE-ENUMERATION/test.sh | 49 ++++++++++++++++++++ test/TEST-31-DEVICE-ENUMERATION/testsuite.sh | 12 +++++ 3 files changed, 62 insertions(+) create mode 120000 test/TEST-31-DEVICE-ENUMERATION/Makefile create mode 100755 test/TEST-31-DEVICE-ENUMERATION/test.sh create mode 100755 test/TEST-31-DEVICE-ENUMERATION/testsuite.sh diff --git a/test/TEST-31-DEVICE-ENUMERATION/Makefile b/test/TEST-31-DEVICE-ENUMERATION/Makefile new file mode 120000 index 0000000000..e9f93b1104 --- /dev/null +++ b/test/TEST-31-DEVICE-ENUMERATION/Makefile @@ -0,0 +1 @@ +../TEST-01-BASIC/Makefile \ No newline at end of file diff --git a/test/TEST-31-DEVICE-ENUMERATION/test.sh b/test/TEST-31-DEVICE-ENUMERATION/test.sh new file mode 100755 index 0000000000..d7cea73361 --- /dev/null +++ b/test/TEST-31-DEVICE-ENUMERATION/test.sh @@ -0,0 +1,49 @@ +#!/bin/bash +# -*- mode: shell-script; indent-tabs-mode: nil; sh-basic-offset: 4; -*- +# ex: ts=8 sw=4 sts=4 et filetype=sh +set -e +TEST_DESCRIPTION="plugged -> dead -> plugged issue #11997" +TEST_NO_NSPAWN=1 + +. $TEST_BASE_DIR/test-functions +QEMU_TIMEOUT=300 + +test_setup() { + create_empty_image + mkdir -p $TESTDIR/root + mount ${LOOPDEV}p1 $TESTDIR/root + + ( + LOG_LEVEL=5 + eval $(udevadm info --export --query=env --name=${LOOPDEV}p2) + + setup_basic_environment + + # mask some services that we do not want to run in these tests + ln -s /dev/null $initdir/etc/systemd/system/systemd-hwdb-update.service + ln -s /dev/null $initdir/etc/systemd/system/systemd-journal-catalog-update.service + ln -s /dev/null $initdir/etc/systemd/system/systemd-networkd.service + ln -s /dev/null $initdir/etc/systemd/system/systemd-networkd.socket + ln -s /dev/null $initdir/etc/systemd/system/systemd-resolved.service + + # setup the testsuite service + cat >$initdir/etc/systemd/system/testsuite.service < dead'; then + exit 1 +fi + +echo OK > /testok +exit 0