[composefs-backend]: install to-existing-root fixes

crates/lib/src/bootc_composefs/repo.rs

@@ -74,6 +74,7 @@ pub(crate) fn open_composefs_repo(rootfs_dir: &Dir) -> Result<crate::store::Comp
         .context("Failed to open composefs repository")
 }
 
+#[context("Initializing composefs repository")]
 pub(crate) async fn initialize_composefs_repository(
     state: &State,
     root_setup: &RootSetup,

crates/lib/src/bootloader.rs

@@ -10,6 +10,7 @@ use cap_std_ext::dirext::CapStdExtDirExt;
 use fn_error_context::context;
 
 use bootc_mount as mount;
+use rustix::fs::statfs;
 
 use crate::bootc_composefs::boot::{MountedImageRoot, SecurebootKeys};
 use crate::utils;
@@ -46,6 +47,18 @@ const BOOTCTL_RANDOM_SEED_MIN_VERSION: u32 = 257;
 /// in place (bootupd will overwrite them during installation).
 // TODO: clean all ESPs on multi-device setups
 pub(crate) fn mount_esp_part(root: &Dir, root_path: &Utf8Path, is_ostree: bool) -> Result<()> {
+    // systemd-gpt-auto-generator automounts ESP at /boot
+    let is_boot_mountpoint = root.is_mountpoint("boot")?;
+
+    if matches!(is_boot_mountpoint, Some(true)) {
+        let statfs = statfs(root_path.join("boot").as_std_path())?;
+
+        // We probably don't need to be this thorough, but no harm done
+        if statfs.f_type == libc::MSDOS_SUPER_MAGIC {
+            return Ok(());
+        }
+    }
+
     let efi_path = Utf8Path::new("boot").join(crate::bootloader::EFI_DIR);
     let Some(esp_fd) = root
         .open_dir_optional(&efi_path)

crates/lib/src/install.rs

@@ -187,7 +187,7 @@ use serde::{Deserialize, Serialize};
 
 #[cfg(feature = "install-to-disk")]
 use self::baseline::InstallBlockDeviceOpts;
-use crate::bootc_composefs::status::ComposefsCmdline;
+use crate::bootc_composefs::status::{ComposefsCmdline, get_bootloader};
 use crate::bootc_composefs::{
     boot::setup_composefs_boot, repo::initialize_composefs_repository,
     status::get_container_manifest_and_config,
@@ -642,8 +642,12 @@ pub(crate) struct State {
     #[allow(dead_code)]
     pub(crate) composefs_required: bool,
 
-    // If Some, then --composefs_native is passed
+    /// If Some, then --composefs-backend is passed
     pub(crate) composefs_options: InstallComposefsOpts,
+
+    /// The bootloader on the host (determined by reading LoaderInfo from efivars)
+    /// Only Some when bootc is invoked with `install to-existing-root`
+    pub(crate) host_bootloader: Option<Bootloader>,
 }
 
 // Shared read-only global state
@@ -1542,12 +1546,21 @@ async fn verify_target_fetch(
 }
 
 /// Preparation for an install; validates and prepares some (thereafter immutable) global state.
+///
+/// # Parameters
+/// - `config_opts`: Installation configuration options (root user setup, generic image, etc.)
+/// - `source_opts`: Source image reference; if `None`, assumes running inside a container
+/// - `target_opts`: Target image reference and root path options
+/// - `composefs_options`: composefs-related settings for the installation
+/// - `target_fs`: Target filesystem type; used for `install to-filesystem`
+/// - `replace_mode`: If `Some`, indicates an `install to-filesystem` or `install to-existing-root` with the given replacement mode
 async fn prepare_install(
     mut config_opts: InstallConfigOpts,
     source_opts: InstallSourceOpts,
     mut target_opts: InstallTargetOpts,
     mut composefs_options: InstallComposefsOpts,
     target_fs: Option<FilesystemEnum>,
+    replace_mode: Option<ReplaceMode>,
 ) -> Result<Arc<State>> {
     tracing::trace!("Preparing install");
     let rootfs = cap_std::fs::Dir::open_ambient_dir("/", cap_std::ambient_authority())
@@ -1698,6 +1711,17 @@ async fn prepare_install(
 
     setup_sys_mount("efivarfs", EFIVARFS)?;
 
+    // Read efivars to get the bootloader
+    // Only if the operation is to replace the existing installation
+    let host_bootloader = match replace_mode {
+        Some(..) => {
+            let host_bootloader = get_bootloader().context("Determining existing bootloader")?;
+            println!("Detected bootloader on host: {host_bootloader}");
+            Some(host_bootloader)
+        }
+        None => None,
+    };
+
     // Now, deal with SELinux state.
     let selinux_state = reexecute_self_for_selinux_if_needed(&source, config_opts.disable_selinux)?;
     tracing::debug!("SELinux state: {selinux_state:?}");
@@ -1804,26 +1828,60 @@ async fn prepare_install(
         host_is_container,
         composefs_required,
         composefs_options,
+        host_bootloader,
     });
 
     Ok(state)
 }
 
 impl PostFetchState {
     pub(crate) fn new(state: &State, d: &Dir) -> Result<Self> {
+        let supports_bootupd = crate::bootloader::supports_bootupd(d)?;
+
         // Determine bootloader type for the target system
         // Priority: user-specified > bootupd availability > systemd-boot fallback
         let detected_bootloader = {
             if let Some(bootloader) = state.config_opts.bootloader.clone() {
                 bootloader
             } else {
-                if crate::bootloader::supports_bootupd(d)? {
+                // TODO(Johan-Liebert1): The new release of bootupd would support all
+                if supports_bootupd {
                     crate::spec::Bootloader::Grub
                 } else {
                     crate::spec::Bootloader::Systemd
                 }
             }
         };
+
+        // If this exists it means we're replacing the current root
+        // or installing alongside it. The new image may or may not have
+        // the same bootloader as the host
+        //
+        // We could simply throw an error here, but at this point we'd have
+        // already nuked the boot or ESP so we should try our best to figure
+        // out what to install
+        let detected_bootloader = match state.host_bootloader {
+            Some(b) => match b {
+                Bootloader::Grub | Bootloader::GrubCC => {
+                    if supports_bootupd {
+                        b
+                    } else {
+                        Bootloader::Systemd
+                    }
+                }
+                Bootloader::Systemd => match crate::bootloader::bootctl_systemd_version() {
+                    Ok(_) => Bootloader::Systemd,
+                    Err(_) => {
+                        println!("Could not find bootctl, defaulting to Grub");
+                        Bootloader::Grub
+                    }
+                },
+                Bootloader::None => Bootloader::None,
+            },
+
+            None => detected_bootloader,
+        };
+
         println!("Bootloader: {detected_bootloader}");
         let r = Self {
             detected_bootloader,
@@ -2171,6 +2229,7 @@ pub(crate) async fn install_to_disk(mut opts: InstallToDiskOpts) -> Result<()> {
         opts.target_opts,
         opts.composefs_opts,
         block_opts.filesystem,
+        None,
     )
     .await?;
 
@@ -2297,12 +2356,14 @@ fn remove_all_in_dir_no_xdev(d: &Dir, mount_err: bool) -> Result<()> {
         if etype == FileType::dir() {
             if let Some(subdir) = d.open_dir_noxdev(&name)? {
                 remove_all_in_dir_no_xdev(&subdir, mount_err)?;
-                d.remove_dir(&name)?;
+                d.remove_dir(&name)
+                    .with_context(|| format!("Removing dir {name:?}"))?;
             } else if mount_err {
                 anyhow::bail!("Found unexpected mount point {name:?}");
             }
         } else {
-            d.remove_file_optional(&name)?;
+            d.remove_file_optional(&name)
+                .with_context(|| format!("Removing {name:?}"))?;
         }
     }
     anyhow::Ok(())
@@ -2558,6 +2619,7 @@ pub(crate) async fn install_to_filesystem(
         opts.target_opts,
         opts.composefs_opts,
         Some(inspect.fstype.as_str().try_into()?),
+        fsopts.replace,
     )
     .await?;
 
@@ -2631,18 +2693,22 @@ pub(crate) async fn install_to_filesystem(
             false
         }
     };
+
     // Find the UUID of /boot because we need it for GRUB.
-    let boot_uuid = if boot_is_mount {
-        let boot_path = target_root_path.join(BOOT);
-        tracing::debug!("boot_path={boot_path}");
-        let u = bootc_mount::inspect_filesystem(&boot_path)
-            .with_context(|| format!("Inspecting /{BOOT}"))?
-            .uuid
-            .ok_or_else(|| anyhow!("No UUID found for /{BOOT}"))?;
-        Some(u)
-    } else {
-        None
+    let boot_uuid = match state.config_opts.bootloader {
+        Some(Bootloader::Grub) | None if boot_is_mount => {
+            let boot_path = target_root_path.join(BOOT);
+            tracing::debug!("boot_path={boot_path}");
+            let u = bootc_mount::inspect_filesystem(&boot_path)
+                .with_context(|| format!("Inspecting /{BOOT}"))?
+                .uuid
+                .ok_or_else(|| anyhow!("No UUID found for /{BOOT}"))?;
+            Some(u)
+        }
+
+        _ => None,
     };
+
     tracing::debug!("boot UUID: {boot_uuid:?}");
 
     // Find the real underlying backing device for the root.  This is currently just required

crates/lib/src/store/mod.rs

@@ -137,7 +137,13 @@ pub(crate) const COMPOSEFS_MODE: Mode = Mode::from_raw_mode(0o700);
 
 /// Ensure the composefs directory exists in the given physical root
 /// with the correct permissions (mode 0700).
+#[context("Ensuring composefs directory")]
 pub(crate) fn ensure_composefs_dir(physical_root: &Dir) -> Result<()> {
+    // Usually the case with bootc install to-existing-root
+    if matches!(physical_root.is_mountpoint("."), Ok(Some(true))) {
+        crate::utils::open_dir_remount_rw(physical_root, ".".into())?;
+    }
+
     let mut db = DirBuilder::new();
     db.mode(COMPOSEFS_MODE.as_raw_mode());
     physical_root
@@ -608,6 +614,7 @@ impl Storage {
     ///
     /// This lazily opens the composefs repository, creating the directory if needed
     /// and bootstrapping verity settings from the ostree configuration.
+    #[context("Ensuring composefs")]
     pub(crate) fn get_ensure_composefs(&self) -> Result<Arc<ComposefsRepository>> {
         if let Some(composefs) = self.composefs.get() {
             return Ok(Arc::clone(composefs));

crates/lib/src/utils.rs

@@ -1,6 +1,6 @@
 use std::future::Future;
 use std::io::Write;
-use std::os::fd::BorrowedFd;
+use std::os::fd::{AsFd, BorrowedFd};
 use std::path::{Component, Path, PathBuf};
 use std::process::Command;
 use std::time::Duration;
@@ -17,6 +17,7 @@ use libsystemd::logging::journal_print;
 use ostree::glib;
 use ostree_ext::container::SignatureSource;
 use ostree_ext::ostree;
+use rustix::fs::StatVfsMountFlags;
 
 /// Try to look for keys injected by e.g. rpm-ostree requesting machine-local
 /// changes; if any are present, return `true`.
@@ -83,7 +84,18 @@ pub fn have_executable(name: &str) -> Result<bool> {
 /// Given a target directory, if it's a read-only mount, then remount it writable
 #[context("Opening {target} with writable mount")]
 pub(crate) fn open_dir_remount_rw(root: &Dir, target: &Utf8Path) -> Result<Dir> {
-    if matches!(root.is_mountpoint(target), Ok(Some(true))) {
+    let target_dir = root
+        .open_dir(target)
+        .with_context(|| format!("Opening {target}"))?;
+
+    if matches!(target_dir.is_mountpoint("."), Ok(Some(true))) {
+        let st = rustix::fs::fstatvfs(target_dir.as_fd())
+            .with_context(|| format!("Getting filesystem info for {target}"))?;
+
+        if !st.f_flag.contains(StatVfsMountFlags::RDONLY) {
+            return Ok(target_dir);
+        };
+
         tracing::debug!("Target {target} is a mountpoint, remounting rw");
         let st = Command::new("mount")
             .args(["-o", "remount,rw", target.as_str()])
@@ -92,7 +104,8 @@ pub(crate) fn open_dir_remount_rw(root: &Dir, target: &Utf8Path) -> Result<Dir>
 
         anyhow::ensure!(st.success(), "Failed to remount: {st:?}");
     }
-    root.open_dir(target).map_err(anyhow::Error::new)
+
+    Ok(target_dir)
 }
 
 /// Given a target path, remove its immutability if present

crates/system-reinstall-bootc/src/podman.rs

@@ -30,6 +30,8 @@ pub(crate) fn reinstall_command(
     ssh_key_file: &str,
     has_clean: bool,
 ) -> Result<Command> {
+    // NOTE: We mount /sys/firmware/efi/efivars during installation setup in
+    // [`bootc_lib::prepare_install`] so we don't explicitly need it here
     let mut podman_command_and_args = [
         // We use podman to run the bootc container. This might change in the future to remove the
         // podman dependency.
@@ -50,7 +52,9 @@ pub(crate) fn reinstall_command(
         "--security-opt",
         "label=type:unconfined_t",
         "-v",
-        "/:/target",
+        // systemd gpt-auto-generator has boot.mount autofs mounted on /boot
+        // We want to propage that inside the container
+        "/:/target:rslave",
     ]
     .map(String::from)
     .to_vec();

tmt/plans/integration.fmf

@@ -292,4 +292,11 @@ execute:
     how: fmf
     test:
       - /tmt/tests/tests/test-46-etc-merge-conflict
+
+/plan-47-system-reinstall-composefs:
+  summary: Test system-reinstall with composefs backend and bootloader compatibility
+  discover:
+    how: fmf
+    test:
+      - /tmt/tests/tests/test-47-system-reinstall-composefs
 # END GENERATED PLANS