Merge pull request #12013 from yuwata/fix-switchroot-11997

core: on switching root do not emit device state change based on enumeration results

8 files changed, 94 insertions, 13 deletions

diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index 475c9194ac..1c1f4dcd06 100644
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -1492,7 +1492,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;
@@ -1564,17 +1564,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;
 
diff --git a/src/core/device.c b/src/core/device.c
index 771239f53b..9f7caa49ec 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/main.c b/src/core/main.c
index 0ba22f815d..46db47126c 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;
 
diff --git a/src/core/manager.c b/src/core/manager.c
index 6cb7d4d0d0..12ae911a38 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) {
@@ -3149,6 +3151,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);
@@ -3377,6 +3382,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;
 
@@ -3567,6 +3581,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 8c7bd7e231..cdd4882a6a 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)
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 <<EOF
+[Unit]
+Description=Testsuite service
+
+[Service]
+ExecStart=/bin/bash -x /testsuite.sh
+Type=oneshot
+StandardOutput=tty
+StandardError=tty
+EOF
+        cp testsuite.sh $initdir/
+
+        setup_testsuite
+    ) || return 1
+
+    ddebug "umount $TESTDIR/root"
+    umount $TESTDIR/root
+}
+
+do_test "$@"
diff --git a/test/TEST-31-DEVICE-ENUMERATION/testsuite.sh b/test/TEST-31-DEVICE-ENUMERATION/testsuite.sh
new file mode 100755
index 0000000000..cb12b51bd2
--- /dev/null
+++ b/test/TEST-31-DEVICE-ENUMERATION/testsuite.sh
@@ -0,0 +1,12 @@
+#!/bin/bash
+# -*- mode: shell-script; indent-tabs-mode: nil; sh-basic-offset: 4; -*-
+# ex: ts=8 sw=4 sts=4 et filetype=sh
+set -ex
+set -o pipefail
+
+if journalctl -b | grep -e '\.device: Changed plugged -> dead'; then
+    exit 1
+fi
+
+echo OK > /testok
+exit 0