fstab-generator: Chase symlinks where possible (#6293)

This has a long history; see see 5261ba901845c084de5a8fd06500ed09bfb0bd80
which originally introduced the behavior.  Unfortunately that commit
doesn't include any rationale, but IIRC the basic issue is that
systemd wants to model the real mount state as units, and symlinks
make canonicalization much more difficult.

At the same time, on a RHEL6 system (upstart), one can make e.g. `/home` a
symlink, and things work as well as they always did; but one doesn't have
access to the sophistication of mount units (dependencies, introspection, etc.)
Supporting symlinks here will hence make it easier for people to do upgrades to
RHEL7 and beyond.

The `/home` as symlink case also appears prominently for OSTree; see
https://ostree.readthedocs.io/en/latest/manual/adapting-existing/

Further work has landed in the nspawn case for this; see e.g.
d944dc9553009822deaddec76814f5642a6a8176

A basic limitation with doing this in the fstab generator (and that I hit while
doing some testing) is that we obviously can't chase symlinks into mounts,
since the generator runs early before mounts. Or at least - doing so would
require multiple passes over the fstab data (as well as looking at existing
mount units), and potentially doing multi-phase generation. I'm not sure it's
worth doing that without a real world use case. For now, this will fix at least
the OSTree + `/home` <https://bugzilla.redhat.com/show_bug.cgi?id=1382873> case
mentioned above, and in general anyone who for whatever reason has symlinks in
their `/etc/fstab`.

diff --git a/man/systemd-fstab-generator.xml b/man/systemd-fstab-generator.xml
index 50d24d67f06edeae038a5d99469e1956d2cb5a45..b898d719f2213365748353da53cb52475cd4e8a5 100644 (file)
--- a/man/systemd-fstab-generator.xml
+++ b/man/systemd-fstab-generator.xml
@@ -71,6 +71,14 @@
     for more information about special <filename>/etc/fstab</filename>
     mount options this generator understands.</para>
 
+    <para>One special topic is handling of symbolic links.  Historical init
+    implementations supported symlinks in <filename>/etc/fstab</filename>.
+    Because mount units will refuse mounts where the target is a symbolic link,
+    this generator will resolve any symlinks as far as possible when processing
+    <filename>/etc/fstab</filename> in order to enhance backwards compatibility.
+    If a symlink target does not exist at the time that this generator runs, it
+    is assumed that the symlink target is the final target of the mount.</para>
+
     <para><filename>systemd-fstab-generator</filename> implements
     <citerefentry><refentrytitle>systemd.generator</refentrytitle><manvolnum>7</manvolnum></citerefentry>.</para>
   </refsect1>

diff --git a/man/systemd.mount.xml b/man/systemd.mount.xml
index 98f71d2e7e51bf1e3ffa55c99bee40eab983a85e..1bed7d17f162e0a8b06248b8efbdb6ab09c18af9 100644 (file)
--- a/man/systemd.mount.xml
+++ b/man/systemd.mount.xml
@@ -373,8 +373,9 @@
 
       <varlistentry>
         <term><varname>Where=</varname></term>
-        <listitem><para>Takes an absolute path of a directory of the
-        mount point. If the mount point does not exist at the time of
+        <listitem><para>Takes an absolute path of a directory for the
+        mount point; in particular, the destination cannot be a symbolic
+        link.  If the mount point does not exist at the time of
         mounting, it is created. This string must be reflected in the
         unit filename. (See above.) This option is
         mandatory.</para></listitem>

diff --git a/src/fstab-generator/fstab-generator.c b/src/fstab-generator/fstab-generator.c
index ce9dccfe68c6689577ba128a928205c7502e15da..7f23b9fd7424813bc7bac9c5263319f47820fda3 100644 (file)
--- a/src/fstab-generator/fstab-generator.c
+++ b/src/fstab-generator/fstab-generator.c
@@ -25,6 +25,7 @@
 
 #include "alloc-util.h"
 #include "fd-util.h"
+#include "fs-util.h"
 #include "fileio.h"
 #include "fstab-util.h"
 #include "generator.h"
@@ -290,6 +291,7 @@ static int add_mount(
                 const char *dest,
                 const char *what,
                 const char *where,
+                const char *original_where,
                 const char *fstype,
                 const char *opts,
                 int passno,
@@ -396,11 +398,10 @@ static int add_mount(
                         return r;
         }
 
-        fprintf(f,
-                "\n"
-                "[Mount]\n"
-                "Where=%s\n",
-                where);
+        fprintf(f, "\n[Mount]\n");
+        if (original_where)
+                fprintf(f, "# Canonicalized from %s\n", original_where);
+        fprintf(f, "Where=%s\n", where);
 
         r = write_what(f, what);
         if (r < 0)
@@ -520,7 +521,7 @@ static int parse_fstab(bool initrd) {
         }
 
         while ((me = getmntent(f))) {
-                _cleanup_free_ char *where = NULL, *what = NULL;
+                _cleanup_free_ char *where = NULL, *what = NULL, *canonical_where = NULL;
                 bool noauto, nofail;
                 int k;
 
@@ -540,8 +541,28 @@ static int parse_fstab(bool initrd) {
                 if (!where)
                         return log_oom();
 
-                if (is_path(where))
+                if (is_path(where)) {
                         path_kill_slashes(where);
+                        /* Follow symlinks here; see 5261ba901845c084de5a8fd06500ed09bfb0bd80 which makes sense for
+                         * mount units, but causes problems since it historically worked to have symlinks in e.g.
+                         * /etc/fstab. So we canonicalize here. Note that we use CHASE_NONEXISTENT to handle the case
+                         * where a symlink refers to another mount target; this works assuming the sub-mountpoint
+                         * target is the final directory.
+                         */
+                        r = chase_symlinks(where, initrd ? "/sysroot" : NULL,
+                                           CHASE_PREFIX_ROOT | CHASE_NONEXISTENT,
+                                           &canonical_where);
+                        if (r < 0)
+                                /* In this case for now we continue on as if it wasn't a symlink */
+                                log_warning_errno(r, "Failed to read symlink target for %s: %m", where);
+                        else {
+                                if (streq(canonical_where, where))
+                                        canonical_where = mfree(canonical_where);
+                                else
+                                        log_debug("Canonicalized what=%s where=%s to %s",
+                                                  what, where, canonical_where);
+                        }
+                }
 
                 noauto = fstab_test_yes_no_option(me->mnt_opts, "noauto\0" "auto\0");
                 nofail = fstab_test_yes_no_option(me->mnt_opts, "nofail\0" "fail\0");
@@ -567,7 +588,8 @@ static int parse_fstab(bool initrd) {
 
                         k = add_mount(arg_dest,
                                       what,
-                                      where,
+                                      canonical_where ?: where,
+                                      canonical_where ? where: NULL,
                                       me->mnt_type,
                                       me->mnt_opts,
                                       me->mnt_passno,
@@ -630,6 +652,7 @@ static int add_sysroot_mount(void) {
         return add_mount(arg_dest,
                          what,
                          "/sysroot",
+                         NULL,
                          arg_root_fstype,
                          opts,
                          is_device_path(what) ? 1 : 0, /* passno */
@@ -684,6 +707,7 @@ static int add_sysroot_usr_mount(void) {
         return add_mount(arg_dest,
                          what,
                          "/sysroot/usr",
+                         NULL,
                          arg_usr_fstype,
                          opts,
                          is_device_path(what) ? 1 : 0, /* passno */
@@ -724,6 +748,7 @@ static int add_volatile_var(void) {
         return add_mount(arg_dest_late,
                          "tmpfs",
                          "/var",
+                         NULL,
                          "tmpfs",
                          "mode=0755",
                          0,