switch-root: rework SWITCH_ROOT_SKIP_RECURSIVE_RUN flag

Negative flags always raise eyebrows. Let's normalize
SWITCH_ROOT_SKIP_RECURSIVE_RUN to become SWITCH_ROOT_RECURSIVE_RUN, i.e.
make recursive behaviour opt-in, rather than opt-out. We only want it
for the initrd→host transition, and in all other cases we'd prefer to
avoid it.

This allows us to simplify some code. Also, normalize the mount point
table in switch_root() a bit, to be static const, and then just select
between two mount flag sets via SWITCH_ROOT_RECURSIVE_RUN, where the
mount flag without MS_BIND (i.e. zero) just means "skip this entry".

This does not actually change anything in behaviour, it's just
refactoring.

Follow-up for: b12d41a8bb7c99f7d7a1c7821a886d98b42d9ce0

diff --git a/src/core/main.c b/src/core/main.c
index c54558f23b9ba0e0757f887be559e33ec64c0a02..f8b0c69f5aa5d0759f98881b70fd48aeeeb23741 100644 (file)
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -1891,7 +1891,7 @@ static int do_reexecute(
                 r = switch_root(/* new_root= */ switch_root_dir,
                                 /* old_root_after= */ NULL,
                                 /* flags= */ (objective == MANAGER_SWITCH_ROOT ? SWITCH_ROOT_DESTROY_OLD_ROOT : 0) |
-                                             (objective == MANAGER_SOFT_REBOOT ? SWITCH_ROOT_SKIP_RECURSIVE_RUN : 0));
+                                             (objective == MANAGER_SOFT_REBOOT ? 0 : SWITCH_ROOT_RECURSIVE_RUN));
                 if (r < 0)
                         log_error_errno(r, "Failed to switch root, trying to continue: %m");
         }

diff --git a/src/shared/switch-root.c b/src/shared/switch-root.c
index 9fb9a3e3768ef426745e2ab733c8ea8444b42a08..34cd2ad0b5fe2664de5114ebfbefbfd9d2402eea 100644 (file)
--- a/src/shared/switch-root.c
+++ b/src/shared/switch-root.c
@@ -30,23 +30,22 @@ int switch_root(const char *new_root,
                 const char *old_root_after,   /* path below the new root, where to place the old root after the transition; may be NULL to unmount it */
                 SwitchRootFlags flags) {
 
-        /* Stuff mounted below /run we don't save on soft reboot, as it might have lost its relevance, i.e.
-         * credentials, removable media and such, we rather want that the new boot mounts this fresh.
-         * But on the switch from initrd we do use MS_REC, as it is expected that mounts set up in /run
-         * are maintained. */
-        unsigned long run_mount_flags = MS_BIND|(!FLAGS_SET(flags, SWITCH_ROOT_SKIP_RECURSIVE_RUN) ? MS_REC : 0);
-        struct {
+        /* Stuff mounted below /run/ we don't save on soft reboot, as it might have lost its relevance, i.e.
+         * credentials, removable media and such, we rather want that the new boot mounts this fresh.  But on
+         * the switch from initrd we do use MS_REC, as it is expected that mounts set up in /run/ are
+         * maintained. */
+        static const struct {
                 const char *path;
-                unsigned long mount_flags;
-                bool skip_if_run_is_rec; /* For child mounts of /run, if it's moved recursively no need to handle */
+                unsigned long mount_flags;                 /* Flags to apply if SWITCH_ROOT_RECURSIVE_RUN is unset */
+                unsigned long mount_flags_recursive_run;   /* Flags to apply if SWITCH_ROOT_RECURSIVE_RUN is set (0 if shall be skipped) */
         } transfer_table[] = {
-                { "/dev",                                 MS_BIND|MS_REC,  false }, /* Recursive, because we want to save the original /dev/shm + /dev/pts and similar */
-                { "/sys",                                 MS_BIND|MS_REC,  false }, /* Similar, we want to retain various API VFS, or the cgroupv1 /sys/fs/cgroup/ tree */
-                { "/proc",                                MS_BIND|MS_REC,  false }, /* Similar */
-                { "/run",                                 run_mount_flags, false }, /* Recursive except on soft reboot, see above */
-                { SYSTEM_CREDENTIALS_DIRECTORY,           MS_BIND,         true },  /* Credentials passed into the system should survive */
-                { ENCRYPTED_SYSTEM_CREDENTIALS_DIRECTORY, MS_BIND,         true },  /* Similar */
-                { "/run/host",                            MS_BIND|MS_REC,  true },  /* Host supplied hierarchy should also survive */
+                { "/dev",                                 MS_BIND|MS_REC,  MS_BIND|MS_REC }, /* Recursive, because we want to save the original /dev/shm/ + /dev/pts/ and similar */
+                { "/sys",                                 MS_BIND|MS_REC,  MS_BIND|MS_REC }, /* Similar, we want to retain various API VFS, or the cgroupv1 /sys/fs/cgroup/ tree */
+                { "/proc",                                MS_BIND|MS_REC,  MS_BIND|MS_REC }, /* Similar */
+                { "/run",                                 MS_BIND,         MS_BIND|MS_REC }, /* Recursive except on soft reboot, see above */
+                { SYSTEM_CREDENTIALS_DIRECTORY,           MS_BIND,         0 /* skip! */  }, /* Credentials passed into the system should survive */
+                { ENCRYPTED_SYSTEM_CREDENTIALS_DIRECTORY, MS_BIND,         0 /* skip! */  },  /* Similar */
+                { "/run/host",                            MS_BIND|MS_REC,  0 /* skip! */  },  /* Host supplied hierarchy should also survive */
         };
 
         _cleanup_close_ int old_root_fd = -EBADF, new_root_fd = -EBADF;
@@ -129,8 +128,10 @@ int switch_root(const char *new_root,
 
         FOREACH_ARRAY(transfer, transfer_table, ELEMENTSOF(transfer_table)) {
                 _cleanup_free_ char *chased = NULL;
+                unsigned long mount_flags;
 
-                if (transfer->skip_if_run_is_rec && !FLAGS_SET(flags, SWITCH_ROOT_SKIP_RECURSIVE_RUN))
+                mount_flags = FLAGS_SET(flags, SWITCH_ROOT_RECURSIVE_RUN) ? transfer->mount_flags_recursive_run : transfer->mount_flags;
+                if (mount_flags == 0) /* skip if zero */
                         continue;
 
                 if (access(transfer->path, F_OK) < 0) {
@@ -149,7 +150,7 @@ int switch_root(const char *new_root,
                 if (r > 0) /* If it is already mounted, then do nothing */
                         continue;
 
-                r = mount_nofollow_verbose(LOG_ERR, transfer->path, chased, NULL, transfer->mount_flags, NULL);
+                r = mount_nofollow_verbose(LOG_ERR, transfer->path, chased, NULL, mount_flags, NULL);
                 if (r < 0)
                         return r;
         }

diff --git a/src/shared/switch-root.h b/src/shared/switch-root.h
index 20561fcee8bdd9b4c61757de952b1b49f5cb7c0b..357e17955608e874e67fd3a01bcf7b2fe83a3993 100644 (file)
--- a/src/shared/switch-root.h
+++ b/src/shared/switch-root.h
@@ -7,7 +7,7 @@ typedef enum SwitchRootFlags {
         SWITCH_ROOT_DESTROY_OLD_ROOT      = 1 << 0, /* rm -rf old root when switching – under the condition
                                                      * that it is backed by non-persistent tmpfs/ramfs/… */
         SWITCH_ROOT_DONT_SYNC             = 1 << 1, /* don't call sync() immediately before switching root */
-        SWITCH_ROOT_SKIP_RECURSIVE_RUN    = 1 << 2, /* move /run without MS_REC */
+        SWITCH_ROOT_RECURSIVE_RUN         = 1 << 2, /* move /run/ with MS_REC from old to new root */
         SWITCH_ROOT_SKIP_RECURSIVE_UMOUNT = 1 << 3, /* do not umount recursively on move */
 } SwitchRootFlags;
 

diff --git a/src/shutdown/shutdown.c b/src/shutdown/shutdown.c
index 263c444ac20d6698008eae35a274401744e7b931..f4776099010ca438a3798b5b92095b553aeb77f9 100644 (file)
--- a/src/shutdown/shutdown.c
+++ b/src/shutdown/shutdown.c
@@ -174,7 +174,6 @@ static int switch_root_initramfs(void) {
                         /* new_root= */ "/run/initramfs",
                         /* old_root_after= */ "/oldroot",
                         /* flags= */ SWITCH_ROOT_DONT_SYNC |
-                                     SWITCH_ROOT_SKIP_RECURSIVE_RUN |
                                      SWITCH_ROOT_SKIP_RECURSIVE_UMOUNT);
 }