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

coreos-boot-mount-generator: Always use mpath for /boot if for root #1022

Merged
merged 2 commits into from
May 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -41,43 +41,44 @@ add_wants() {
# is backed by a device-mapper target the dev-mapper.*.device is used.
mk_mount() {
local mount_pt="${1}"; shift
local label="${1}"; shift
local path="${1}"; shift
local options="${1}"; shift

local path="/dev/disk/by-label/${label}"
local devservice=$(systemd-escape -p ${path} --suffix=service)
local unit_name=$(systemd-escape -p ${mount_pt} --suffix=mount)

eval $(udevadm info --query property --export "${path}")
device="$(systemd-escape ${path})"
if [ "${DM_NAME:-x}" != "x" ]; then
path="/dev/mapper/${DM_NAME}"
device="$(systemd-escape dev/mapper/${DM_NAME})"
fi
device="${device//-dev/dev}"
echo "coreos-boot-mount-generator: using ${device} for ${label} mount to ${mount_pt}"

cat > "${UNIT_DIR}/${unit_name}" <<EOF
# Automatically created by coreos-boot-mount-generator
[Unit]
Description=CoreOS Dynamic Mount for ${mount_pt}
Documentation=https://github.com/coreos/fedora-coreos-config

Before=local-fs.target
Requires=systemd-fsck@${device}.service
After=systemd-fsck@${device}.service
Requires=systemd-fsck@${devservice}
After=systemd-fsck@${devservice}

[Mount]
What=${path}
Where=${mount_pt}
Options=${options}
EOF

add_wants "${unit_name}"
}

# If the root device is multipath, hook up /boot to use that too,
# based on our custom udev rules in 90-coreos-device-mapper.rules
# that creates "label found on mpath" links.
# Otherwise, use the usual by-label symlink.
# See discussion in https://github.com/coreos/fedora-coreos-config/pull/1022
bootdev=/dev/disk/by-label/boot
# Yes this isn't a real karg parser but we're trapped in this shell script
if grep -q rd.multipath /proc/cmdline; then
Copy link
Member

Choose a reason for hiding this comment

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

Minor: this will incorrectly trigger on rd.multipath=0 (see https://github.com/dracutdevs/dracut/blob/9355cb8ea5024533210067373657dc337d63ecb9/modules.d/90multipath/multipathd.service#L12). I mean, a user really shouldn't have to do that, though still would be good to be resilient here.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is in the real root; I'm uncertain about pulling in dracut-lib.sh into the real root. We could try to ship rdcore in the real root too and move this there?

Or I could special case rd.multipath=default for now...

Or it looks like coreos-liveiso-autologin-generator has have_karg that we could try to factor out into a little shell library shared by our shell generators...

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's do this as is so I can cherry pick it, and figure out a cleaner karg handling later?

bootdev=/dev/disk/by-label/dm-mpath-boot
fi

# We mount read-only by default mostly to protect
# against accidental damage. Only a few things
# owned by CoreOS should be touching /boot or the ESP.
# Use nodev,nosuid because some hardening guides want
# that even though it's of minimal value.
mk_mount /boot boot ro,nodev,nosuid
mk_mount /boot "${bootdev}" ro,nodev,nosuid
Original file line number Diff line number Diff line change
@@ -1,24 +1,27 @@
# CoreOS-specific symlinks for dm-multipath filesystem labels,
# used for `label=boot` and `label=root`.

ACTION=="remove", GOTO="dm_label_end"
SUBSYSTEM!="block", GOTO="dm_label_end"
KERNEL!="dm-*", GOTO="dm_label_end"

# Ensure that the device mapper target is active
# And the required attributes exist.
ENV{DM_ACTIVATION}!="1", GOTO="dm_label_end"
ENV{ID_FS_LABEL_ENC}!="?*", GOTO="dm_label_end"
ENV{ID_FS_UUID_ENC}!="?*", GOTO="dm_label_end"
ENV{DM_SUSPENDED}=="1", GOTO="dm_label_end"
Copy link
Member

Choose a reason for hiding this comment

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

Nice! I think this will help with some of the udev issues I'm working around in #1011.

Copy link
Member

Choose a reason for hiding this comment

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

Related to this area of the code, I've also removed the DM_ACTIVATION line in #1011, which makes the symlinks even more solid!


# Only act on filesystems. This should prevent layered devices
# such as Raid on Multipath devices from appearing.
ENV{ID_FS_USAGE}!="filesystem", GOTO="dm_label_end"

# And if the filesystem doesn't have a label+uuid, we're done.
ENV{ID_FS_LABEL_ENC}!="?*", GOTO="dm_label_end"
ENV{ID_FS_UUID_ENC}!="?*", GOTO="dm_label_end"

# Setup up Multipath labels and UUID's. Match on DM_UUID which
# is stable regardless of whether friendly names are used or not.
# 66-kpartx.rules use DM_UUID to match for linear mappings on multipath
# targets.
ENV{DM_UUID}=="*mpath*" \
, ENV{DM_SUSPENDED}=="Active" \
, ENV{DM_TABLES_LOADED}=="Live" \
, SYMLINK+="disk/by-label/dm-mpath-$env{ID_FS_LABEL_ENC}" \
, SYMLINK+="disk/by-uuid/dm-mpath-$env{ID_FS_UUID_ENC}"

Expand Down