Merge pull request #15265 from fbuihuu/mount-fixes

Mount fixes

9 files changed, 120 insertions, 78 deletions

diff --git a/src/core/automount.c b/src/core/automount.c
index 54711e2b7f..566b56eb34 100644
--- a/src/core/automount.c
+++ b/src/core/automount.c
@@ -152,6 +152,10 @@ static int automount_add_default_dependencies(Automount *a) {
         if (!MANAGER_IS_SYSTEM(UNIT(a)->manager))
                 return 0;
 
+        r = unit_add_dependency_by_name(UNIT(a), UNIT_BEFORE, SPECIAL_LOCAL_FS_TARGET, true, UNIT_DEPENDENCY_DEFAULT);
+        if (r < 0)
+                return r;
+
         r = unit_add_dependency_by_name(UNIT(a), UNIT_AFTER, SPECIAL_LOCAL_FS_PRE_TARGET, true, UNIT_DEPENDENCY_DEFAULT);
         if (r < 0)
                 return r;
diff --git a/src/core/device.c b/src/core/device.c
index a24fea04d0..50d55289fa 100644
--- a/src/core/device.c
+++ b/src/core/device.c
@@ -1063,7 +1063,6 @@ const UnitVTable device_vtable = {
                 "Device\0"
                 "Install\0",
 
-        .refuse_after = true,
         .gc_jobs = true,
 
         .init = device_init,
diff --git a/src/core/mount.c b/src/core/mount.c
index 016c63bb2e..5463196f69 100644
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -66,6 +66,14 @@ static bool MOUNT_STATE_WITH_PROCESS(MountState state) {
                       MOUNT_CLEANING);
 }
 
+static bool mount_is_automount(const MountParameters *p) {
+        assert(p);
+
+        return fstab_test_option(p->options,
+                                 "comment=systemd.automount\0"
+                                 "x-systemd.automount\0");
+}
+
 static bool mount_is_network(const MountParameters *p) {
         assert(p);
 
@@ -78,6 +86,15 @@ static bool mount_is_network(const MountParameters *p) {
         return false;
 }
 
+static bool mount_is_nofail(const Mount *m) {
+        assert(m);
+
+        if (!m->from_fragment)
+                return false;
+
+        return fstab_test_yes_no_option(m->parameters_fragment.options, "nofail\0" "fail\0");
+}
+
 static bool mount_is_loop(const MountParameters *p) {
         assert(p);
 
@@ -400,42 +417,72 @@ static bool mount_is_extrinsic(Mount *m) {
         MountParameters *p;
         assert(m);
 
-        /* Returns true for all units that are "magic" and should be excluded from the usual start-up and shutdown
-         * dependencies. We call them "extrinsic" here, as they are generally mounted outside of the systemd dependency
-         * logic. We shouldn't attempt to manage them ourselves but it's fine if the user operates on them with us. */
+        /* Returns true for all units that are "magic" and should be excluded from the usual
+         * start-up and shutdown dependencies. We call them "extrinsic" here, as they are generally
+         * mounted outside of the systemd dependency logic. We shouldn't attempt to manage them
+         * ourselves but it's fine if the user operates on them with us. */
 
-        if (!MANAGER_IS_SYSTEM(UNIT(m)->manager)) /* We only automatically manage mounts if we are in system mode */
+        /* We only automatically manage mounts if we are in system mode */
+        if (!MANAGER_IS_SYSTEM(UNIT(m)->manager))
                 return true;
 
         if (UNIT(m)->perpetual) /* All perpetual units never change state */
                 return true;
 
-        if (PATH_IN_SET(m->where,  /* Don't bother with the OS data itself */
-                        "/",       /* (strictly speaking redundant: should already be covered by the perpetual flag check above) */
-                        "/usr",
-                        "/etc"))
-                return true;
-
-        if (PATH_STARTSWITH_SET(m->where,
-                                "/run/initramfs",    /* This should stay around from before we boot until after we shutdown */
-                                "/proc",             /* All of this is API VFS */
-                                "/sys",              /* … dito … */
-                                "/dev"))             /* … dito … */
-                return true;
-
-        /* If this is an initrd mount, and we are not in the initrd, then leave this around forever, too. */
         p = get_mount_parameters(m);
-        if (p && fstab_test_option(p->options, "x-initrd.mount\0") && !in_initrd())
+        if (p && fstab_is_extrinsic(m->where, p->options))
                 return true;
 
         return false;
 }
 
+static int mount_add_default_ordering_dependencies(
+                Mount *m,
+                MountParameters *p,
+                UnitDependencyMask mask) {
+
+        const char *after, *before, *e;
+        int r;
+
+        assert(m);
+
+        e = path_startswith(m->where, "/sysroot");
+        if (e && in_initrd()) {
+                /* All mounts under /sysroot need to happen later, at initrd-fs.target time. IOW,
+                 * it's not technically part of the basic initrd filesystem itself, and so
+                 * shouldn't inherit the default Before=local-fs.target dependency. */
+
+                after = NULL;
+                before = isempty(e) ? SPECIAL_INITRD_ROOT_FS_TARGET : SPECIAL_INITRD_FS_TARGET;
+
+        } else if (mount_is_network(p)) {
+                after = SPECIAL_REMOTE_FS_PRE_TARGET;
+                before = SPECIAL_REMOTE_FS_TARGET;
+
+        } else {
+                after = SPECIAL_LOCAL_FS_PRE_TARGET;
+                before = SPECIAL_LOCAL_FS_TARGET;
+        }
+
+        if (!mount_is_nofail(m) && !mount_is_automount(p)) {
+                r = unit_add_dependency_by_name(UNIT(m), UNIT_BEFORE, before, true, mask);
+                if (r < 0)
+                        return r;
+        }
+
+        if (after) {
+                r = unit_add_dependency_by_name(UNIT(m), UNIT_AFTER, after, true, mask);
+                if (r < 0)
+                        return r;
+        }
+
+        return unit_add_two_dependencies_by_name(UNIT(m), UNIT_BEFORE, UNIT_CONFLICTS,
+                                                 SPECIAL_UMOUNT_TARGET, true, mask);
+}
+
 static int mount_add_default_dependencies(Mount *m) {
-        const char *after, *before;
         UnitDependencyMask mask;
         MountParameters *p;
-        bool nofail;
         int r;
 
         assert(m);
@@ -443,9 +490,10 @@ static int mount_add_default_dependencies(Mount *m) {
         if (!UNIT(m)->default_dependencies)
                 return 0;
 
-        /* We do not add any default dependencies to /, /usr or /run/initramfs/, since they are guaranteed to stay
-         * mounted the whole time, since our system is on it.  Also, don't bother with anything mounted below virtual
-         * file systems, it's also going to be virtual, and hence not worth the effort. */
+        /* We do not add any default dependencies to /, /usr or /run/initramfs/, since they are
+         * guaranteed to stay mounted the whole time, since our system is on it.  Also, don't
+         * bother with anything mounted below virtual file systems, it's also going to be virtual,
+         * and hence not worth the effort. */
         if (mount_is_extrinsic(m))
                 return 0;
 
@@ -454,51 +502,31 @@ static int mount_add_default_dependencies(Mount *m) {
                 return 0;
 
         mask = m->from_fragment ? UNIT_DEPENDENCY_FILE : UNIT_DEPENDENCY_MOUNTINFO_DEFAULT;
-        nofail = m->from_fragment ? fstab_test_yes_no_option(m->parameters_fragment.options, "nofail\0" "fail\0") : false;
+
+        r = mount_add_default_ordering_dependencies(m, p, mask);
+        if (r < 0)
+                return r;
 
         if (mount_is_network(p)) {
-                /* We order ourselves after network.target. This is
-                 * primarily useful at shutdown: services that take
-                 * down the network should order themselves before
-                 * network.target, so that they are shut down only
-                 * after this mount unit is stopped. */
+                /* We order ourselves after network.target. This is primarily useful at shutdown:
+                 * services that take down the network should order themselves before
+                 * network.target, so that they are shut down only after this mount unit is
+                 * stopped. */
 
                 r = unit_add_dependency_by_name(UNIT(m), UNIT_AFTER, SPECIAL_NETWORK_TARGET, true, mask);
                 if (r < 0)
                         return r;
 
-                /* We pull in network-online.target, and order
-                 * ourselves after it. This is useful at start-up to
-                 * actively pull in tools that want to be started
-                 * before we start mounting network file systems, and
-                 * whose purpose it is to delay this until the network
-                 * is "up". */
+                /* We pull in network-online.target, and order ourselves after it. This is useful
+                 * at start-up to actively pull in tools that want to be started before we start
+                 * mounting network file systems, and whose purpose it is to delay this until the
+                 * network is "up". */
 
                 r = unit_add_two_dependencies_by_name(UNIT(m), UNIT_WANTS, UNIT_AFTER, SPECIAL_NETWORK_ONLINE_TARGET, true, mask);
                 if (r < 0)
                         return r;
-
-                after = SPECIAL_REMOTE_FS_PRE_TARGET;
-                before = SPECIAL_REMOTE_FS_TARGET;
-        } else {
-                after = SPECIAL_LOCAL_FS_PRE_TARGET;
-                before = SPECIAL_LOCAL_FS_TARGET;
         }
 
-        if (!nofail) {
-                r = unit_add_dependency_by_name(UNIT(m), UNIT_BEFORE, before, true, mask);
-                if (r < 0)
-                        return r;
-        }
-
-        r = unit_add_dependency_by_name(UNIT(m), UNIT_AFTER, after, true, mask);
-        if (r < 0)
-                return r;
-
-        r = unit_add_two_dependencies_by_name(UNIT(m), UNIT_BEFORE, UNIT_CONFLICTS, SPECIAL_UMOUNT_TARGET, true, mask);
-        if (r < 0)
-                return r;
-
         /* If this is a tmpfs mount then we have to unmount it before we try to deactivate swaps */
         if (streq_ptr(p->fstype, "tmpfs")) {
                 r = unit_add_dependency_by_name(UNIT(m), UNIT_AFTER, SPECIAL_SWAP_TARGET, true, mask);
diff --git a/src/core/unit.c b/src/core/unit.c
index 84a5e5a948..6f65ff7d05 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -3028,13 +3028,10 @@ int unit_add_dependency(
                 return 0;
         }
 
-        if (d == UNIT_AFTER && UNIT_VTABLE(u)->refuse_after) {
-                log_unit_warning(u, "Requested dependency After=%s ignored (%s units cannot be delayed).", other->id, unit_type_to_string(u->type));
-                return 0;
-        }
-
-        if (d == UNIT_BEFORE && UNIT_VTABLE(other)->refuse_after) {
-                log_unit_warning(u, "Requested dependency Before=%s ignored (%s units cannot be delayed).", other->id, unit_type_to_string(other->type));
+        /* Note that ordering a device unit after a unit is permitted since it
+         * allows to start its job running timeout at a specific time. */
+        if (d == UNIT_BEFORE && other->type == UNIT_DEVICE) {
+                log_unit_warning(u, "Dependency Before=%s ignored (.device units cannot be delayed)", other->id);
                 return 0;
         }
 
diff --git a/src/core/unit.h b/src/core/unit.h
index 4ad429bf30..bee68607fe 100644
--- a/src/core/unit.h
+++ b/src/core/unit.h
@@ -615,9 +615,6 @@ typedef struct UnitVTable {
         /* True if the unit type knows a failure state, and thus can be source of an OnFailure= dependency */
         bool can_fail:1;
 
-        /* True if After= dependencies should be refused */
-        bool refuse_after:1;
-
         /* True if units of this type shall be startable only once and then never again */
         bool once_only:1;
 
diff --git a/src/fstab-generator/fstab-generator.c b/src/fstab-generator/fstab-generator.c
index bef2f27d3a..2b3e9ec7cb 100644
--- a/src/fstab-generator/fstab-generator.c
+++ b/src/fstab-generator/fstab-generator.c
@@ -391,12 +391,6 @@ static int add_mount(
                 "SourcePath=%s\n",
                 source);
 
-        /* All mounts under /sysroot need to happen later, at initrd-fs.target time. IOW, it's not
-         * technically part of the basic initrd filesystem itself, and so shouldn't inherit the default
-         * Before=local-fs.target dependency. */
-        if (in_initrd() && path_startswith(where, "/sysroot"))
-                fprintf(f, "DefaultDependencies=no\n");
-
         if (STRPTR_IN_SET(fstype, "nfs", "nfs4") && !(flags & AUTOMOUNT) &&
             fstab_test_yes_no_option(opts, "bg\0" "fg\0")) {
                 /* The default retry timeout that mount.nfs uses for 'bg' mounts
@@ -411,9 +405,6 @@ static int add_mount(
                 SET_FLAG(flags, NOFAIL, true);
         }
 
-        if (!(flags & NOFAIL) && !(flags & AUTOMOUNT))
-                fprintf(f, "Before=%s\n", post);
-
         if (!(flags & AUTOMOUNT) && opts) {
                  r = write_after(f, opts);
                  if (r < 0)
@@ -535,8 +526,6 @@ static int add_mount(
                         "Documentation=man:fstab(5) man:systemd-fstab-generator(8)\n",
                         source);
 
-                fprintf(f, "Before=%s\n", post);
-
                 if (opts) {
                         r = write_after(f, opts);
                         if (r < 0)
diff --git a/src/shared/fstab-util.c b/src/shared/fstab-util.c
index 86a57e6b2c..b19127be09 100644
--- a/src/shared/fstab-util.c
+++ b/src/shared/fstab-util.c
@@ -35,6 +35,30 @@ int fstab_has_fstype(const char *fstype) {
         return false;
 }
 
+bool fstab_is_extrinsic(const char *mount, const char *opts) {
+
+        /* Don't bother with the OS data itself */
+        if (PATH_IN_SET(mount,
+                        "/",
+                        "/usr",
+                        "/etc"))
+                return true;
+
+        if (PATH_STARTSWITH_SET(mount,
+                                "/run/initramfs",    /* This should stay around from before we boot until after we shutdown */
+                                "/proc",             /* All of this is API VFS */
+                                "/sys",              /* … dito … */
+                                "/dev"))             /* … dito … */
+                return true;
+
+        /* If this is an initrd mount, and we are not in the initrd, then leave
+         * this around forever, too. */
+        if (opts && fstab_test_option(opts, "x-initrd.mount\0") && !in_initrd())
+                return true;
+
+        return false;
+}
+
 int fstab_is_mount_point(const char *mount) {
         _cleanup_endmntent_ FILE *f = NULL;
         struct mntent *m;
diff --git a/src/shared/fstab-util.h b/src/shared/fstab-util.h
index f575ed0bb2..a73575e95c 100644
--- a/src/shared/fstab-util.h
+++ b/src/shared/fstab-util.h
@@ -6,6 +6,7 @@
 
 #include "macro.h"
 
+bool fstab_is_extrinsic(const char *mount, const char *opts);
 int fstab_is_mount_point(const char *mount);
 int fstab_has_fstype(const char *fstype);
 
diff --git a/src/shared/generator.c b/src/shared/generator.c
index acdd0096f1..04d2f86a4a 100644
--- a/src/shared/generator.c
+++ b/src/shared/generator.c
@@ -260,6 +260,9 @@ int generator_write_device_deps(
         _cleanup_free_ char *node = NULL, *unit = NULL;
         int r;
 
+        if (fstab_is_extrinsic(where, opts))
+                return 0;
+
         if (!fstab_test_option(opts, "_netdev\0"))
                 return 0;