From 3399a30ee02d0d31ba2d43d0ce0a2fd90d5c575d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=BD=D0=B0=D0=B1?= Date: Fri, 31 Mar 2023 18:47:48 +0200 Subject: [PATCH] contrib: dracut: fix race with root=zfs:dset when necessities required MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This had always worked in my testing, but a user on hardware reported this to happen 100%, and I reproduced it once with cold VM host caches. dracut-zfs-generator runs as a systemd generator, i.e. at Some Relatively Early Time; if root= is a fixed dataset, it tries to "solve [necessities] statically at generation time". If by that point zfs-import.target hasn't popped (because the import is taking a non-negligible amount of time for whatever reason), it'll see no children for the root datase, and as such generate no mounts. This has never had any right to work. No-one caught this earlier because it's just that much more convenient to have root=zfs:AUTO, which orders itself properly. To fix this, always run zfs-nonroot-necessities.service; this additionally simplifies the implementation by: * making BOOTFS from zfs-env-bootfs.service be the real, canonical, root dataset name, not just "whatever the first bootfs is", and only set it if we're ZFS-booting * zfs-{rollback,snapshot}-bootfs.service can use this instead of re-implementing it * having zfs-env-bootfs.service also set BOOTFSFLAGS * this means the sysroot.mount drop-in can be fixed text * zfs-nonroot-necessities.service can also be constant and always enabled, because it's conditioned on BOOTFS being set There is no longer any code generated at run-time (the sysroot.mount drop-in is an unavoidable gratuitous cp). The flow of BOOTFS{,FLAGS} from zfs-env-bootfs.service to sysroot.mount is not noted explicitly in dracut.zfs(7), because (a) at some point it's just visual noise and (b) it's already ordered via d-p-m.s from z-i.t. Reviewed-by: Brian Behlendorf Signed-off-by: Ahelenia Ziemiańska Closes #14690 --- contrib/dracut/90zfs/module-setup.sh.in | 3 + .../dracut/90zfs/zfs-env-bootfs.service.in | 15 ++++- contrib/dracut/90zfs/zfs-generator.sh.in | 67 ++----------------- contrib/dracut/90zfs/zfs-lib.sh.in | 2 +- .../90zfs/zfs-nonroot-necessities.service.in | 20 ++++++ .../90zfs/zfs-rollback-bootfs.service.in | 3 +- .../90zfs/zfs-snapshot-bootfs.service.in | 3 +- contrib/dracut/Makefile.am | 1 + man/man7/dracut.zfs.7 | 16 ++--- 9 files changed, 54 insertions(+), 76 deletions(-) create mode 100644 contrib/dracut/90zfs/zfs-nonroot-necessities.service.in diff --git a/contrib/dracut/90zfs/module-setup.sh.in b/contrib/dracut/90zfs/module-setup.sh.in index 78c74e7423bb..e55cb60e1612 100755 --- a/contrib/dracut/90zfs/module-setup.sh.in +++ b/contrib/dracut/90zfs/module-setup.sh.in @@ -81,6 +81,9 @@ install() { inst_simple "${moddir}/zfs-env-bootfs.service" "${systemdsystemunitdir}/zfs-env-bootfs.service" systemctl -q --root "${initdir}" add-wants zfs-import.target zfs-env-bootfs.service + inst_simple "${moddir}/zfs-nonroot-necessities.service" "${systemdsystemunitdir}/zfs-nonroot-necessities.service" + systemctl -q --root "${initdir}" add-requires initrd-root-fs.target zfs-nonroot-necessities.service + # Add user-provided unit overrides: # - /etc/systemd/system/${_service} # - /etc/systemd/system/${_service}.d/overrides.conf diff --git a/contrib/dracut/90zfs/zfs-env-bootfs.service.in b/contrib/dracut/90zfs/zfs-env-bootfs.service.in index 34c88037cac2..7ebab4c1a58d 100644 --- a/contrib/dracut/90zfs/zfs-env-bootfs.service.in +++ b/contrib/dracut/90zfs/zfs-env-bootfs.service.in @@ -1,6 +1,5 @@ [Unit] -Description=Set BOOTFS environment for dracut -Documentation=man:zpool(8) +Description=Set BOOTFS and BOOTFSFLAGS environment variables for dracut DefaultDependencies=no After=zfs-import-cache.service After=zfs-import-scan.service @@ -8,7 +7,17 @@ Before=zfs-import.target [Service] Type=oneshot -ExecStart=/bin/sh -c "exec systemctl set-environment BOOTFS=$(@sbindir@/zpool list -H -o bootfs | grep -m1 -vFx -)" +ExecStart=/bin/sh -c ' \ + . /lib/dracut-zfs-lib.sh; \ + decode_root_args || exit 0; \ + [ "$root" = "zfs:AUTO" ] && root="$(@sbindir@/zpool list -H -o bootfs | grep -m1 -vFx -)"; \ + rootflags="$(getarg rootflags=)"; \ + case ",$rootflags," in \ + *,zfsutil,*) ;; \ + ,,) rootflags=zfsutil ;; \ + *) rootflags="zfsutil,$rootflags" ;; \ + esac; \ + exec systemctl set-environment BOOTFS="$root" BOOTFSFLAGS="$rootflags"' [Install] WantedBy=zfs-import.target diff --git a/contrib/dracut/90zfs/zfs-generator.sh.in b/contrib/dracut/90zfs/zfs-generator.sh.in index 56f7ca9785ba..4e1eb7490e0d 100755 --- a/contrib/dracut/90zfs/zfs-generator.sh.in +++ b/contrib/dracut/90zfs/zfs-generator.sh.in @@ -14,81 +14,24 @@ GENERATOR_DIR="$1" . /lib/dracut-zfs-lib.sh decode_root_args || exit 0 -[ -z "${rootflags}" ] && rootflags=$(getarg rootflags=) -case ",${rootflags}," in - *,zfsutil,*) ;; - ,,) rootflags=zfsutil ;; - *) rootflags="zfsutil,${rootflags}" ;; -esac - [ -n "$debug" ] && echo "zfs-generator: writing extension for sysroot.mount to $GENERATOR_DIR/sysroot.mount.d/zfs-enhancement.conf" >> /dev/kmsg -mkdir -p "$GENERATOR_DIR"/sysroot.mount.d "$GENERATOR_DIR"/initrd-root-fs.target.requires "$GENERATOR_DIR"/dracut-pre-mount.service.d +mkdir -p "$GENERATOR_DIR"/sysroot.mount.d "$GENERATOR_DIR"/dracut-pre-mount.service.d + { echo "[Unit]" echo "Before=initrd-root-fs.target" echo "After=zfs-import.target" echo echo "[Mount]" - if [ "${root}" = "zfs:AUTO" ]; then - echo "PassEnvironment=BOOTFS" - echo 'What=${BOOTFS}' - else - echo "What=${root}" - fi + echo "PassEnvironment=BOOTFS BOOTFSFLAGS" + echo 'What=${BOOTFS}' echo "Type=zfs" - echo "Options=${rootflags}" + echo 'Options=${BOOTFSFLAGS}' } > "$GENERATOR_DIR"/sysroot.mount.d/zfs-enhancement.conf ln -fs ../sysroot.mount "$GENERATOR_DIR"/initrd-root-fs.target.requires/sysroot.mount - -if [ "${root}" = "zfs:AUTO" ]; then - { - echo "[Unit]" - echo "Before=initrd-root-fs.target" - echo "After=sysroot.mount" - echo "DefaultDependencies=no" - echo - echo "[Service]" - echo "Type=oneshot" - echo "PassEnvironment=BOOTFS" - echo "ExecStart=/bin/sh -c '" ' \ - . /lib/dracut-zfs-lib.sh; \ - _zfs_nonroot_necessities_cb() { \ - zfs mount | grep -m1 -q "^$1 " && return 0; \ - echo "Mounting $1 on /sysroot$2"; \ - mount -o zfsutil -t zfs "$1" "/sysroot$2"; \ - }; \ - for_relevant_root_children "${BOOTFS}" _zfs_nonroot_necessities_cb;' \ - "'" - } > "$GENERATOR_DIR"/zfs-nonroot-necessities.service - ln -fs ../zfs-nonroot-necessities.service "$GENERATOR_DIR"/initrd-root-fs.target.requires/zfs-nonroot-necessities.service -else - # We can solve this statically at generation time, so do! - _zfs_generator_cb() { - dset="${1}" - mpnt="${2}" - unit="$(systemd-escape --suffix=mount -p "/sysroot${mpnt}")" - - { - echo "[Unit]" - echo "Before=initrd-root-fs.target" - echo "After=sysroot.mount" - echo - echo "[Mount]" - echo "Where=/sysroot${mpnt}" - echo "What=${dset}" - echo "Type=zfs" - echo "Options=zfsutil" - } > "$GENERATOR_DIR/${unit}" - ln -fs ../"${unit}" "$GENERATOR_DIR"/initrd-root-fs.target.requires/"${unit}" - } - - for_relevant_root_children "${root}" _zfs_generator_cb -fi - - { echo "[Unit]" echo "After=zfs-import.target" diff --git a/contrib/dracut/90zfs/zfs-lib.sh.in b/contrib/dracut/90zfs/zfs-lib.sh.in index 3a43e514d6f9..7139e2e6fe4b 100755 --- a/contrib/dracut/90zfs/zfs-lib.sh.in +++ b/contrib/dracut/90zfs/zfs-lib.sh.in @@ -39,7 +39,7 @@ mount_dataset() { # for_relevant_root_children DATASET EXEC # Runs "EXEC dataset mountpoint" for all children of DATASET that are needed for system bringup -# Used by zfs-generator.sh and friends, too! +# Used by zfs-nonroot-necessities.service and friends, too! for_relevant_root_children() { dataset="${1}" exec="${2}" diff --git a/contrib/dracut/90zfs/zfs-nonroot-necessities.service.in b/contrib/dracut/90zfs/zfs-nonroot-necessities.service.in new file mode 100644 index 000000000000..8f420c737c72 --- /dev/null +++ b/contrib/dracut/90zfs/zfs-nonroot-necessities.service.in @@ -0,0 +1,20 @@ +[Unit] +Before=initrd-root-fs.target +After=sysroot.mount +DefaultDependencies=no +ConditionEnvironment=BOOTFS + +[Service] +Type=oneshot +PassEnvironment=BOOTFS +ExecStart=/bin/sh -c ' \ + . /lib/dracut-zfs-lib.sh; \ + _zfs_nonroot_necessities_cb() { \ + @sbindir@/zfs mount | grep -m1 -q "^$1 " && return 0; \ + echo "Mounting $1 on /sysroot$2"; \ + mount -o zfsutil -t zfs "$1" "/sysroot$2"; \ + }; \ + for_relevant_root_children "${BOOTFS}" _zfs_nonroot_necessities_cb' + +[Install] +RequiredBy=initrd-root-fs.target diff --git a/contrib/dracut/90zfs/zfs-rollback-bootfs.service.in b/contrib/dracut/90zfs/zfs-rollback-bootfs.service.in index a29cf3a3dd81..68fdcb1f323e 100644 --- a/contrib/dracut/90zfs/zfs-rollback-bootfs.service.in +++ b/contrib/dracut/90zfs/zfs-rollback-bootfs.service.in @@ -5,8 +5,9 @@ After=zfs-import.target dracut-pre-mount.service zfs-snapshot-bootfs.service Before=dracut-mount.service DefaultDependencies=no ConditionKernelCommandLine=bootfs.rollback +ConditionEnvironment=BOOTFS [Service] Type=oneshot -ExecStart=/bin/sh -c '. /lib/dracut-zfs-lib.sh; decode_root_args || exit; [ "$root" = "zfs:AUTO" ] && root="$BOOTFS"; SNAPNAME="$(getarg bootfs.rollback)"; exec @sbindir@/zfs rollback -Rf "$root@${SNAPNAME:-%v}"' +ExecStart=/bin/sh -c '. /lib/dracut-lib.sh; SNAPNAME="$(getarg bootfs.rollback)"; exec @sbindir@/zfs rollback -Rf "$BOOTFS@${SNAPNAME:-%v}"' RemainAfterExit=yes diff --git a/contrib/dracut/90zfs/zfs-snapshot-bootfs.service.in b/contrib/dracut/90zfs/zfs-snapshot-bootfs.service.in index 9e73d1a78724..a675b5b2ea98 100644 --- a/contrib/dracut/90zfs/zfs-snapshot-bootfs.service.in +++ b/contrib/dracut/90zfs/zfs-snapshot-bootfs.service.in @@ -5,8 +5,9 @@ After=zfs-import.target dracut-pre-mount.service Before=dracut-mount.service DefaultDependencies=no ConditionKernelCommandLine=bootfs.snapshot +ConditionEnvironment=BOOTFS [Service] Type=oneshot -ExecStart=-/bin/sh -c '. /lib/dracut-zfs-lib.sh; decode_root_args || exit; [ "$root" = "zfs:AUTO" ] && root="$BOOTFS"; SNAPNAME="$(getarg bootfs.snapshot)"; exec @sbindir@/zfs snapshot "$root@${SNAPNAME:-%v}"' +ExecStart=-/bin/sh -c '. /lib/dracut-lib.sh; SNAPNAME="$(getarg bootfs.snapshot)"; exec @sbindir@/zfs snapshot "$BOOTFS@${SNAPNAME:-%v}"' RemainAfterExit=yes diff --git a/contrib/dracut/Makefile.am b/contrib/dracut/Makefile.am index 73ca52b66316..b432ab76a6d8 100644 --- a/contrib/dracut/Makefile.am +++ b/contrib/dracut/Makefile.am @@ -16,6 +16,7 @@ pkgdracut_90_SCRIPTS = \ pkgdracut_90_DATA = \ %D%/90zfs/zfs-env-bootfs.service \ + %D%/90zfs/zfs-nonroot-necessities.service \ %D%/90zfs/zfs-rollback-bootfs.service \ %D%/90zfs/zfs-snapshot-bootfs.service diff --git a/man/man7/dracut.zfs.7 b/man/man7/dracut.zfs.7 index 2db2f6639eaf..c1475c695e83 100644 --- a/man/man7/dracut.zfs.7 +++ b/man/man7/dracut.zfs.7 @@ -1,6 +1,6 @@ .\" SPDX-License-Identifier: 0BSD .\" -.Dd April 4, 2022 +.Dd March 28, 2023 .Dt DRACUT.ZFS 7 .Os . @@ -28,13 +28,13 @@ zfs-import-scan.service \(da \(da | zfs-import-c zfs-import.target \(-> dracut-pre-mount.service | \(ua | | dracut-zfs-generator | - | ____________________/| + | _____________________/| |/ \(da - | sysroot.mount \(<-\(em\(em\(em\(em\(em\(em\(em\(em\(em\(em\(em\(em\(em\(em dracut-zfs-generator - | | \(da | - | \(da sysroot-{usr,etc,lib,&c.}.mount | - | initrd-root-fs.target \(<-\(em\(em\(em\(em\(em\(em\(em\(em\(em\(em\(em\(em or \(da - | | zfs-nonroot-necessities.service + | sysroot.mount \(<-\(em\(em\(em dracut-zfs-generator + | | + | \(da + | initrd-root-fs.target \(<-\(em zfs-nonroot-necessities.service + | | | | \(da | \(da dracut-mount.service | zfs-snapshot-bootfs.service | | @@ -42,7 +42,7 @@ zfs-import-scan.service \(da \(da | zfs-import-c \(da … | zfs-rollback-bootfs.service | | | \(da | - | sysroot-usr.mount \(<-\(em\(em\(em\(em\(em\(em\(em\(em\(em\(em\(em\(em\(em\(em\(em\(em\(em\(em\(em\(em\(em\(em\(em/ + | /sysroot/{usr,etc,lib,&c.} \(<-\(em\(em\(em\(em\(em\(em\(em\(em\(em\(em\(em\(em\(em\(em\(em\(em\(em\(em\(em/ | | | \(da | initrd-fs.target