composefs: Hold /boot open while a deployment is staged

crates/lib/src/bootc_composefs/finalize.rs

@@ -5,6 +5,7 @@ use crate::bootc_composefs::gc::{GCOpts, composefs_gc};
 use crate::bootc_composefs::rollback::{rename_exchange_bls_entries, rename_exchange_user_cfg};
 use crate::bootc_composefs::status::get_composefs_status;
 use crate::composefs_consts::STATE_DIR_ABS;
+use crate::install::BOOT;
 use crate::spec::BootloaderKind;
 use crate::store::{BootedComposefs, Storage};
 use anyhow::{Context, Result};
@@ -15,7 +16,7 @@ use cap_std_ext::dirext::CapStdExtDirExt;
 use composefs::generic_tree::{FileSystem, Stat};
 use composefs_ctl::composefs;
 use etc_merge::{Diff, compute_diff, merge, traverse_etc};
-use rustix::fs::fsync;
+use rustix::fs::{Mode, OFlags, fsync};
 
 use fn_error_context::context;
 
@@ -167,6 +168,33 @@ pub(crate) async fn composefs_backend_finalize(
     Ok(())
 }
 
+/// Keep /boot open until we're killed, which systemd does with SIGTERM
+/// when `bootc-finalize-staged-hold.service` is stopped after
+/// `bootc-finalize-staged.service`.
+///
+/// When /boot is an automount (e.g. the ESP set up by
+/// systemd-gpt-auto-generator), an idle expire breaks finalization in two
+/// ways. If it races with shutdown, it deadlocks: the finalization (which
+/// looks up /boot) blocks on the expire, while the unmount is ordered after
+/// the finalization. If it completes before shutdown, systemd won't remount
+/// /boot once shutdown has begun, so finalization fails to open it (EHOSTDOWN)
+/// and the old deployment boots. An open file descriptor makes autofs treat
+/// the mount as busy, so it never expires. Note this only works from the
+/// root mount namespace.
+pub(crate) fn hold_boot() -> Result<()> {
+    let path = format!("/{BOOT}");
+    let _fd = rustix::fs::open(
+        path.as_str(),
+        OFlags::RDONLY | OFlags::DIRECTORY | OFlags::CLOEXEC,
+        Mode::empty(),
+    )
+    .with_context(|| format!("Opening {path}"))?;
+    tracing::debug!("Holding {path} open until terminated");
+    loop {
+        std::thread::park();
+    }
+}
+
 #[context("Grub: Finalizing staged UKI")]
 fn finalize_staged_grub_uki(boot_fd: &Dir) -> Result<()> {
     let entries_dir = boot_fd.open_dir("grub2")?;

crates/lib/src/cli.rs

@@ -212,6 +212,19 @@ pub(crate) struct SwitchOpts {
     pub(crate) progress: ProgressOptions,
 }
 
+/// Finalize a staged composefs deployment.
+///
+/// This is invoked at shutdown by `bootc-finalize-staged.service`.
+#[derive(Debug, Parser, PartialEq, Eq)]
+pub(crate) struct ComposefsFinalizeStagedOpts {
+    /// Hold /boot open until terminated, instead of finalizing.
+    ///
+    /// This is used by `bootc-finalize-staged-hold.service` to keep an
+    /// automounted /boot from expiring while a deployment is staged.
+    #[clap(long)]
+    pub(crate) hold: bool,
+}
+
 /// Options controlling rollback
 #[derive(Debug, Parser, PartialEq, Eq)]
 pub(crate) struct RollbackOpts {
@@ -1066,7 +1079,7 @@ pub(crate) enum Opt {
     #[clap(subcommand)]
     #[clap(hide = true)]
     Internals(InternalsOpts),
-    ComposefsFinalizeStaged,
+    ComposefsFinalizeStaged(ComposefsFinalizeStagedOpts),
     /// Diff current /etc configuration versus default
     #[clap(hide = true)]
     ConfigDiff,
@@ -2714,14 +2727,23 @@ async fn run_from_opt(opt: Opt) -> Result<CliExitStatus> {
             }
         },
 
-        Opt::ComposefsFinalizeStaged => {
-            let storage = &get_storage().await?;
-            match storage.kind()? {
-                BootedStorageKind::Ostree(_) => {
-                    anyhow::bail!("ComposefsFinalizeStaged is only supported for composefs backend")
-                }
-                BootedStorageKind::Composefs(booted_cfs) => {
-                    composefs_backend_finalize(storage, &booted_cfs).await
+        Opt::ComposefsFinalizeStaged(opts) => {
+            if opts.hold {
+                // Must happen before loading storage: the hold has to stay in
+                // the root mount namespace, and must not depend on anything
+                // but /boot.
+                crate::bootc_composefs::finalize::hold_boot()
+            } else {
+                let storage = &get_storage().await?;
+                match storage.kind()? {
+                    BootedStorageKind::Ostree(_) => {
+                        anyhow::bail!(
+                            "ComposefsFinalizeStaged is only supported for composefs backend"
+                        )
+                    }
+                    BootedStorageKind::Composefs(booted_cfs) => {
+                        composefs_backend_finalize(storage, &booted_cfs).await
+                    }
                 }
             }
         }
@@ -2870,6 +2892,17 @@ mod tests {
             Opt::parse_including_static(["bootc", "status", "-v"]),
             Opt::Status(StatusOpts { verbose: true, .. })
         ));
+
+        for (args, hold) in [
+            (&["bootc", "composefs-finalize-staged"][..], false),
+            (&["bootc", "composefs-finalize-staged", "--hold"][..], true),
+        ] {
+            assert_eq!(
+                Opt::parse_including_static(args),
+                Opt::ComposefsFinalizeStaged(ComposefsFinalizeStagedOpts { hold }),
+                "{args:?}"
+            );
+        }
     }
 
     #[test]

docs/src/man/bootc-composefs-finalize-staged.8.md

@@ -1,25 +1,52 @@
 # NAME
 
-bootc-composefs-finalize-staged - TODO: Add description
+bootc-composefs-finalize-staged - Finalize a staged composefs deployment
 
 # SYNOPSIS
 
-bootc composefs-finalize-staged
+**bootc composefs-finalize-staged** \[*OPTIONS...*\]
 
 # DESCRIPTION
 
-TODO: Add description
+Finalize a staged composefs deployment. This is an internal command
+invoked at shutdown by **bootc-finalize-staged.service**; it is not
+intended to be run directly.
+
+When `bootc upgrade` or `bootc switch` stages a new deployment on a
+composefs system, it starts **bootc-finalize-staged.service**, whose
+`ExecStop` runs this command as the system shuts down. It merges the
+current `/etc` into the staged deployment and updates the bootloader
+configuration so that the next boot uses it. If no deployment is
+staged, it does nothing. It fails on systems using the ostree
+backend, where **ostree-finalize-staged.service** does this instead.
+
+The finalize service also pulls in
+**bootc-finalize-staged-hold.service**, which runs this command with
+`--hold` to keep `/boot` open while a deployment is staged. Otherwise
+an automounted `/boot` (such as the ESP set up by
+**systemd-gpt-auto-generator**(8)) could expire while idle, and then
+either deadlock with shutdown or be unavailable when finalization
+runs.
+
+# OPTIONS
 
 <!-- BEGIN GENERATED OPTIONS -->
+**--hold**
+
+    Hold /boot open until terminated, instead of finalizing
+
 <!-- END GENERATED OPTIONS -->
 
 # EXAMPLES
 
-TODO: Add practical examples showing how to use this command.
+Check whether a staged deployment is waiting to be finalized at the
+next shutdown:
+
+    systemctl status bootc-finalize-staged.service bootc-finalize-staged-hold.service
 
 # SEE ALSO
 
-**bootc**(8)
+**bootc**(8), **bootc-upgrade**(8), **bootc-switch**(8)
 
 # VERSION
 

docs/src/man/bootc.8.md

@@ -34,7 +34,7 @@ pulled and `bootc upgrade`.
 | **bootc install** | Install the running container to a target |
 | **bootc container** | Operations which can be executed as part of a container build |
 | **bootc loader-entries** | Operations on Boot Loader Specification (BLS) entries |
-| **bootc composefs-finalize-staged** |  |
+| **bootc composefs-finalize-staged** | Finalize a staged composefs deployment |
 
 <!-- END GENERATED SUBCOMMANDS -->
 

systemd/bootc-finalize-staged-hold.service

@@ -0,0 +1,25 @@
+# Keeps /boot busy while a deployment is staged, so that an automounted
+# /boot (e.g. the ESP set up by systemd-gpt-auto-generator) can't idle-expire.
+# An expire racing with shutdown deadlocks bootc-finalize-staged.service's
+# ExecStop, and one that completes before shutdown leaves /boot unmounted
+# for good, since systemd won't trigger the automount again during shutdown,
+# so the ExecStop fails. This is a port of
+# ostree-finalize-staged-hold.service; see
+# https://github.com/ostreedev/ostree/pull/2543 for background.
+[Unit]
+Description=Hold /boot Open for Composefs Finalize Staged Deployment
+Documentation=man:bootc(1)
+DefaultDependencies=no
+
+RequiresMountsFor=/boot
+After=local-fs.target
+Before=basic.target final.target
+
+[Service]
+Type=exec
+
+# This is explicitly run in the root namespace to ensure an automounted
+# /boot doesn't time out since autofs doesn't handle mount namespaces.
+#
+# https://bugzilla.redhat.com/show_bug.cgi?id=2056090
+ExecStart=+/usr/bin/bootc composefs-finalize-staged --hold

systemd/bootc-finalize-staged.service

@@ -28,6 +28,11 @@ Before=basic.target final.target
 After=systemd-journal-flush.service
 Conflicts=final.target
 
+# Start the hold unit and ensure it stays active throughout this
+# service.
+Wants=bootc-finalize-staged-hold.service
+After=bootc-finalize-staged-hold.service
+
 [Service]
 Type=oneshot
 RemainAfterExit=yes

tmt/tests/booted/test-44-shadow-fixup.nu

@@ -62,6 +62,14 @@ def initial_build [] {
     podman build -t localhost/bootc-shadow-fixup-b .
 
     bootc switch --transport containers-storage localhost/bootc-shadow-fixup-b
+
+    # On composefs, staging must also start the hold unit that keeps an
+    # automounted /boot from expiring before finalization at shutdown.
+    if (tap is_composefs) {
+        let hold_state = (do { ^systemctl is-active bootc-finalize-staged-hold.service } | complete | get stdout | str trim)
+        assert ($hold_state == "active") $"bootc-finalize-staged-hold.service not active after staging: ($hold_state)"
+    }
+
     bootc_testlib reboot
 }