Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from Sep 25, 2023
Merged

Conversation

brozs
Copy link
Collaborator

@brozs brozs commented Sep 22, 2023

This has a long history; see see 5261ba9 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. d944dc9

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.

(cherry picked from commit 634735b)

Resolves: RHEL-6223

This has a long history; see see 5261ba9
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.
d944dc9

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`.

(cherry picked from commit 634735b)

Resolves: RHEL-6223
@jamacku jamacku changed the base branch from rhel-7.9 to master September 22, 2023 10:18
@jamacku jamacku changed the title fstab-generator: Chase symlinks where possible (#6293) (RHEL-6223) fstab-generator: Chase symlinks where possible (#6293) Sep 22, 2023
@jamacku jamacku added pr/needs-review Formerly needs-review pr/needs-ci Formerly needs-ci labels Sep 22, 2023
Copy link
Member

@dtardon dtardon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jamacku jamacku removed pr/needs-review Formerly needs-review pr/needs-ci Formerly needs-ci labels Sep 25, 2023
@jamacku jamacku merged commit a4c51f1 into redhat-plumbers:master Sep 25, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants