From 2c20ee91cb513b1e6764e70b5faaa46b6f05eb42 Mon Sep 17 00:00:00 2001 From: Daniele Guarascio Date: Sun, 25 Jan 2026 17:36:44 +0100 Subject: [PATCH 1/5] lib/install: Use mounted ESP for install to-filesystem Previously, `bootc install to-filesystem` determined the ESP by scanning the partition table of the disk hosting the target root. This logic failed to respect an explicit `/boot/efi` mount if it was different from the first ESP on the disk. This change updates the installation logic (both for systemd-boot and bootupd paths) to strictly require that `/boot/efi` is mounted under the target root. It inspects this mountpoint to determine the correct backing device, ensuring the installation targets the intended ESP. Assisted-by: Zed Agent (GPT-5.2-Codex) Signed-off-by: Daniele Guarascio --- crates/lib/src/bootc_composefs/boot.rs | 38 +++++++--- crates/lib/src/bootloader.rs | 99 +++++++++++++++++--------- crates/lib/src/install.rs | 22 +++--- 3 files changed, 109 insertions(+), 50 deletions(-) diff --git a/crates/lib/src/bootc_composefs/boot.rs b/crates/lib/src/bootc_composefs/boot.rs index 801a019bd..dea1df293 100644 --- a/crates/lib/src/bootc_composefs/boot.rs +++ b/crates/lib/src/bootc_composefs/boot.rs @@ -106,10 +106,8 @@ use crate::{ }; use crate::{ bootc_composefs::state::{get_booted_bls, write_composefs_state}, - bootloader::esp_in, -}; -use crate::{ - bootc_composefs::status::get_container_manifest_and_config, bootc_kargs::compute_new_kargs, + bootc_composefs::status::get_container_manifest_and_config, + bootc_kargs::compute_new_kargs, }; use crate::{bootc_composefs::status::get_sorted_grub_uki_boot_entries, install::PostFetchState}; use crate::{ @@ -214,6 +212,17 @@ fi ) } +fn open_target_root(root_setup: &RootSetup) -> Result { + if let Some(target_root) = root_setup.target_root_path.as_ref() { + Dir::open_ambient_dir(target_root, ambient_authority()).context("Opening target root path") + } else { + root_setup + .physical_root + .try_clone() + .context("Cloning target root handle") + } +} + pub fn get_esp_partition(device: &str) -> Result<(String, Option)> { let device_info = bootc_blockdev::partitions_of(Utf8Path::new(device))?; let esp = crate::bootloader::esp_in(&device_info)?; @@ -521,11 +530,12 @@ pub(crate) fn setup_composefs_bls_boot( cmdline_options.extend(&Cmdline::from(&composefs_cmdline)); // Locate ESP partition device - let esp_part = esp_in(&root_setup.device_info)?; + let esp_root = open_target_root(root_setup)?; + let esp_part = crate::bootloader::require_boot_efi_mount(&esp_root)?; ( root_setup.physical_root_path.clone(), - esp_part.node.clone(), + esp_part, cmdline_options, fs, postfetch.detected_bootloader.clone(), @@ -1063,11 +1073,12 @@ pub(crate) fn setup_composefs_uki_boot( BootSetupType::Setup((root_setup, state, postfetch, ..)) => { state.require_no_kargs_for_uki()?; - let esp_part = esp_in(&root_setup.device_info)?; + let esp_root = open_target_root(root_setup)?; + let esp_part = crate::bootloader::require_boot_efi_mount(&esp_root)?; ( root_setup.physical_root_path.clone(), - esp_part.node.clone(), + esp_part, postfetch.detected_bootloader.clone(), state.composefs_options.insecure, state.composefs_options.uki_addon.as_ref(), @@ -1231,18 +1242,22 @@ pub(crate) async fn setup_composefs_boot( .or(root_setup.rootfs_uuid.as_deref()) .ok_or_else(|| anyhow!("No uuid for boot/root"))?; + let target_root = open_target_root(root_setup)?; + if cfg!(target_arch = "s390x") { // TODO: Integrate s390x support into install_via_bootupd crate::bootloader::install_via_zipl(&root_setup.device_info, boot_uuid)?; } else if postfetch.detected_bootloader == Bootloader::Grub { crate::bootloader::install_via_bootupd( &root_setup.device_info, + &target_root, &root_setup.physical_root_path, &state.config_opts, None, )?; } else { crate::bootloader::install_systemd_boot( + &target_root, &root_setup.device_info, &root_setup.physical_root_path, &state.config_opts, @@ -1406,4 +1421,11 @@ mod tests { "RHEL should sort before Fedora in descending order" ); } + + #[test] + fn test_efi_uuid_source_formatting() { + let source = get_efi_uuid_source(); + assert!(source.contains("${config_directory}/")); + assert!(source.contains(EFI_UUID_FILE)); + } } diff --git a/crates/lib/src/bootloader.rs b/crates/lib/src/bootloader.rs index 1e1157ef8..4a0337ae2 100644 --- a/crates/lib/src/bootloader.rs +++ b/crates/lib/src/bootloader.rs @@ -11,7 +11,7 @@ use fn_error_context::context; use bootc_blockdev::{Partition, PartitionTable}; use bootc_mount as mount; -use crate::bootc_composefs::boot::{SecurebootKeys, get_sysroot_parent_dev, mount_esp}; +use crate::bootc_composefs::boot::{SecurebootKeys, mount_esp}; use crate::{discoverable_partition_specification, utils}; /// The name of the mountpoint for efi (as a subdirectory of /boot, or at the toplevel) @@ -31,40 +31,45 @@ pub(crate) fn esp_in(device: &PartitionTable) -> Result<&Partition> { .ok_or(anyhow::anyhow!("ESP not found in partition table")) } -/// Get esp partition node based on the root dir -pub(crate) fn get_esp_partition_node(root: &Dir) -> Result> { - let device = get_sysroot_parent_dev(&root)?; - let base_partitions = bootc_blockdev::partitions_of(Utf8Path::new(&device))?; - let esp = base_partitions.find_partition_of_esp()?; - Ok(esp.map(|v| v.node.clone())) +fn normalize_esp_source(source: String) -> String { + if let Some(uuid) = source.strip_prefix("UUID=") { + format!("/dev/disk/by-uuid/{uuid}") + } else if let Some(label) = source.strip_prefix("LABEL=") { + format!("/dev/disk/by-label/{label}") + } else { + source + } } -/// Mount ESP part at /boot/efi -pub(crate) fn mount_esp_part(root: &Dir, root_path: &Utf8Path, is_ostree: bool) -> Result<()> { - let efi_path = Utf8Path::new("boot").join(crate::bootloader::EFI_DIR); - let Some(esp_fd) = root +pub(crate) fn require_boot_efi_mount(root: &Dir) -> Result { + let efi_path = Utf8Path::new("boot").join(EFI_DIR); + let esp_fd = root .open_dir_optional(&efi_path) .context("Opening /boot/efi")? - else { - return Ok(()); - }; + .ok_or_else(|| anyhow::anyhow!("/boot/efi does not exist"))?; - let Some(false) = esp_fd.is_mountpoint(".")? else { - return Ok(()); - }; + if !esp_fd.is_mountpoint(".")?.unwrap_or(false) { + anyhow::bail!("/boot/efi is not a mountpoint. This is required for installation."); + } - tracing::debug!("Not a mountpoint: /boot/efi"); - // On ostree env with enabled composefs, should be /target/sysroot - let physical_root = if is_ostree { - &root.open_dir("sysroot").context("Opening /sysroot")? - } else { - root - }; - if let Some(esp_part) = get_esp_partition_node(physical_root)? { - bootc_mount::mount(&esp_part, &root_path.join(&efi_path))?; - tracing::debug!("Mounted {esp_part} at /boot/efi"); + let fs = bootc_mount::inspect_filesystem_of_dir(&esp_fd)?; + if fs.fstype != "vfat" && fs.fstype != "fat" { + anyhow::bail!( + "/boot/efi is mounted but is not vfat/fat (found {}). This is required for installation.", + fs.fstype + ); } - Ok(()) + + let source = normalize_esp_source(fs.source); + + let source_path = Utf8Path::new(&source); + if !source_path.try_exists()? { + anyhow::bail!( + "/boot/efi source device does not exist: {source}. This is required for installation." + ); + } + + Ok(source) } /// Determine if the invoking environment contains bootupd, and if there are bootupd-based @@ -83,10 +88,16 @@ pub(crate) fn supports_bootupd(root: &Dir) -> Result { #[context("Installing bootloader")] pub(crate) fn install_via_bootupd( device: &PartitionTable, + root: &Dir, rootfs: &Utf8Path, configopts: &crate::install::InstallConfigOpts, deployment_path: Option<&str>, ) -> Result<()> { + // We require /boot/efi to be mounted; finding the device is just a sanity check that it is. + // The device argument is currently used by bootupctl as a fallback if it can't find the ESP. + // But we want to fail fast if our own check fails. + let _esp_device = require_boot_efi_mount(root)?; + let verbose = std::env::var_os("BOOTC_BOOTLOADER_DEBUG").map(|_| "-vvvv"); // bootc defaults to only targeting the platform boot method. let bootupd_opts = (!configopts.generic_image).then_some(["--update-firmware", "--auto"]); @@ -165,17 +176,16 @@ pub(crate) fn install_via_bootupd( #[context("Installing bootloader")] pub(crate) fn install_systemd_boot( - device: &PartitionTable, + root: &Dir, + _device: &PartitionTable, _rootfs: &Utf8Path, _configopts: &crate::install::InstallConfigOpts, _deployment_path: Option<&str>, autoenroll: Option, ) -> Result<()> { - let esp_part = device - .find_partition_of_type(discoverable_partition_specification::ESP) - .ok_or_else(|| anyhow::anyhow!("ESP partition not found"))?; + let esp_device = require_boot_efi_mount(root)?; - let esp_mount = mount_esp(&esp_part.node).context("Mounting ESP")?; + let esp_mount = mount_esp(&esp_device).context("Mounting ESP")?; let esp_path = Utf8Path::from_path(esp_mount.dir.path()) .ok_or_else(|| anyhow::anyhow!("Failed to convert ESP mount path to UTF-8"))?; @@ -291,3 +301,26 @@ pub(crate) fn install_via_zipl(device: &PartitionTable, boot_uuid: &str) -> Resu .log_debug() .run_inherited_with_cmd_context() } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn normalize_esp_source_uuid() { + let normalized = normalize_esp_source("UUID=abcd-1234".to_string()); + assert_eq!(normalized, "/dev/disk/by-uuid/abcd-1234"); + } + + #[test] + fn normalize_esp_source_label() { + let normalized = normalize_esp_source("LABEL=EFI".to_string()); + assert_eq!(normalized, "/dev/disk/by-label/EFI"); + } + + #[test] + fn normalize_esp_source_passthrough() { + let normalized = normalize_esp_source("/dev/sda1".to_string()); + assert_eq!(normalized, "/dev/sda1"); + } +} diff --git a/crates/lib/src/install.rs b/crates/lib/src/install.rs index 75c151e0d..5be4c5c01 100644 --- a/crates/lib/src/install.rs +++ b/crates/lib/src/install.rs @@ -1753,6 +1753,13 @@ async fn install_with_sysroot( .context("Opening deployment dir")?; let postfetch = PostFetchState::new(state, &deployment_dir)?; + let target_root_path = rootfs + .target_root_path + .clone() + .unwrap_or(rootfs.physical_root_path.clone()); + let target_root = Dir::open_ambient_dir(&target_root_path, cap_std::ambient_authority()) + .with_context(|| format!("Opening target root directory {target_root_path}"))?; + if cfg!(target_arch = "s390x") { // TODO: Integrate s390x support into install_via_bootupd crate::bootloader::install_via_zipl(&rootfs.device_info, boot_uuid)?; @@ -1761,10 +1768,8 @@ async fn install_with_sysroot( Bootloader::Grub => { crate::bootloader::install_via_bootupd( &rootfs.device_info, - &rootfs - .target_root_path - .clone() - .unwrap_or(rootfs.physical_root_path.clone()), + &target_root, + &target_root_path, &state.config_opts, Some(&deployment_path.as_str()), )?; @@ -2154,14 +2159,13 @@ fn remove_all_except_loader_dirs(bootdir: &Dir, is_ostree: bool) -> Result<()> { } #[context("Removing boot directory content")] -fn clean_boot_directories(rootfs: &Dir, rootfs_path: &Utf8Path, is_ostree: bool) -> Result<()> { +fn clean_boot_directories(rootfs: &Dir, is_ostree: bool) -> Result<()> { let bootdir = crate::utils::open_dir_remount_rw(rootfs, BOOT.into()).context("Opening /boot")?; if ARCH_USES_EFI { - // On booted FCOS, esp is not mounted by default - // Mount ESP part at /boot/efi before clean - crate::bootloader::mount_esp_part(&rootfs, &rootfs_path, is_ostree)?; + // Require an explicit /boot/efi mount to avoid cleaning the wrong ESP. + crate::bootloader::require_boot_efi_mount(rootfs)?; } // This should not remove /boot/efi note. @@ -2375,7 +2379,7 @@ pub(crate) async fn install_to_filesystem( .await??; } Some(ReplaceMode::Alongside) => { - clean_boot_directories(&target_rootfs_fd, &target_root_path, is_already_ostree)? + clean_boot_directories(&target_rootfs_fd, is_already_ostree)? } None => require_empty_rootdir(&rootfs_fd)?, } From 67361c40f9402052b5dd6db1325774497453138f Mon Sep 17 00:00:00 2001 From: Daniele Guarascio Date: Sun, 25 Jan 2026 19:02:06 +0100 Subject: [PATCH 2/5] install: Allow permissive ESP detection for existing root The strict ESP mount enforcement previously introduced caused regressions in scenarios, specifically in CI environments running inside containers (tmt/podman). In these contexts, bind mounts often mask `/boot/efi`, causing `is_mountpoint` checks to fail even when the configuration is valid. This patch introduces a `require_esp_mount` field to `RootSetup`. When targeting an existing root (host), we now utilize a permissive mode: if the explicit mount check fails, logic falls back to scanning the partition table. This restores compatibility with containerized installs while maintaining strict safety checks for `to-filesystem` and `to-disk` modes. Signed-off-by: Daniele Guarascio --- crates/lib/src/bootc_composefs/boot.rs | 32 ++++++++++++++++++++++++-- crates/lib/src/bootloader.rs | 11 +++++---- crates/lib/src/install.rs | 9 +++++--- crates/lib/src/install/baseline.rs | 1 + 4 files changed, 44 insertions(+), 9 deletions(-) diff --git a/crates/lib/src/bootc_composefs/boot.rs b/crates/lib/src/bootc_composefs/boot.rs index dea1df293..18c1b2c98 100644 --- a/crates/lib/src/bootc_composefs/boot.rs +++ b/crates/lib/src/bootc_composefs/boot.rs @@ -531,7 +531,20 @@ pub(crate) fn setup_composefs_bls_boot( // Locate ESP partition device let esp_root = open_target_root(root_setup)?; - let esp_part = crate::bootloader::require_boot_efi_mount(&esp_root)?; + let esp_part = if root_setup.require_esp_mount { + crate::bootloader::require_boot_efi_mount(&esp_root)? + } else { + match crate::bootloader::require_boot_efi_mount(&esp_root) { + Ok(p) => p, + Err(e) => { + tracing::debug!( + "ESP mount check failed in permissive mode: {e}; falling back to partition table scan" + ); + let esp = crate::bootloader::esp_in(&root_setup.device_info)?; + esp.node.clone() + } + } + }; ( root_setup.physical_root_path.clone(), @@ -1074,7 +1087,21 @@ pub(crate) fn setup_composefs_uki_boot( state.require_no_kargs_for_uki()?; let esp_root = open_target_root(root_setup)?; - let esp_part = crate::bootloader::require_boot_efi_mount(&esp_root)?; + + let esp_part = if root_setup.require_esp_mount { + crate::bootloader::require_boot_efi_mount(&esp_root)? + } else { + match crate::bootloader::require_boot_efi_mount(&esp_root) { + Ok(p) => p, + Err(e) => { + tracing::debug!( + "ESP mount check failed in permissive mode: {e}; falling back to partition table scan" + ); + let esp = crate::bootloader::esp_in(&root_setup.device_info)?; + esp.node.clone() + } + } + }; ( root_setup.physical_root_path.clone(), @@ -1254,6 +1281,7 @@ pub(crate) async fn setup_composefs_boot( &root_setup.physical_root_path, &state.config_opts, None, + root_setup.require_esp_mount, )?; } else { crate::bootloader::install_systemd_boot( diff --git a/crates/lib/src/bootloader.rs b/crates/lib/src/bootloader.rs index 4a0337ae2..e6f939609 100644 --- a/crates/lib/src/bootloader.rs +++ b/crates/lib/src/bootloader.rs @@ -92,11 +92,14 @@ pub(crate) fn install_via_bootupd( rootfs: &Utf8Path, configopts: &crate::install::InstallConfigOpts, deployment_path: Option<&str>, + require_mount: bool, ) -> Result<()> { - // We require /boot/efi to be mounted; finding the device is just a sanity check that it is. - // The device argument is currently used by bootupctl as a fallback if it can't find the ESP. - // But we want to fail fast if our own check fails. - let _esp_device = require_boot_efi_mount(root)?; + if require_mount { + // We require /boot/efi to be mounted; finding the device is just a sanity check that it is. + // The device argument is currently used by bootupctl as a fallback if it can't find the ESP. + // But we want to fail fast if our own check fails. + let _esp_device = require_boot_efi_mount(root)?; + } let verbose = std::env::var_os("BOOTC_BOOTLOADER_DEBUG").map(|_| "-vvvv"); // bootc defaults to only targeting the platform boot method. diff --git a/crates/lib/src/install.rs b/crates/lib/src/install.rs index 5be4c5c01..833f80c6a 100644 --- a/crates/lib/src/install.rs +++ b/crates/lib/src/install.rs @@ -1298,6 +1298,7 @@ pub(crate) struct RootSetup { skip_finalize: bool, boot: Option, pub(crate) kargs: CmdlineOwned, + pub(crate) require_esp_mount: bool, } fn require_boot_uuid(spec: &MountSpec) -> Result<&str> { @@ -1772,6 +1773,7 @@ async fn install_with_sysroot( &target_root_path, &state.config_opts, Some(&deployment_path.as_str()), + rootfs.require_esp_mount, )?; } Bootloader::Systemd => { @@ -2159,11 +2161,11 @@ fn remove_all_except_loader_dirs(bootdir: &Dir, is_ostree: bool) -> Result<()> { } #[context("Removing boot directory content")] -fn clean_boot_directories(rootfs: &Dir, is_ostree: bool) -> Result<()> { +fn clean_boot_directories(rootfs: &Dir, is_ostree: bool, strict_esp: bool) -> Result<()> { let bootdir = crate::utils::open_dir_remount_rw(rootfs, BOOT.into()).context("Opening /boot")?; - if ARCH_USES_EFI { + if ARCH_USES_EFI && strict_esp { // Require an explicit /boot/efi mount to avoid cleaning the wrong ESP. crate::bootloader::require_boot_efi_mount(rootfs)?; } @@ -2379,7 +2381,7 @@ pub(crate) async fn install_to_filesystem( .await??; } Some(ReplaceMode::Alongside) => { - clean_boot_directories(&target_rootfs_fd, is_already_ostree)? + clean_boot_directories(&target_rootfs_fd, is_already_ostree, !targeting_host_root)? } None => require_empty_rootdir(&rootfs_fd)?, } @@ -2541,6 +2543,7 @@ pub(crate) async fn install_to_filesystem( boot, kargs, skip_finalize, + require_esp_mount: !targeting_host_root, }; install_to_filesystem_impl(&state, &mut rootfs, cleanup).await?; diff --git a/crates/lib/src/install/baseline.rs b/crates/lib/src/install/baseline.rs index d05604ed5..e8c9f9a94 100644 --- a/crates/lib/src/install/baseline.rs +++ b/crates/lib/src/install/baseline.rs @@ -496,5 +496,6 @@ pub(crate) fn install_create_rootfs( boot, kargs, skip_finalize: false, + require_esp_mount: true, }) } From 8d870fc814dcf22d526c101223961268ddf9114d Mon Sep 17 00:00:00 2001 From: Daniele Guarascio Date: Sun, 8 Feb 2026 13:25:32 +0100 Subject: [PATCH 3/5] lib: Fallback to partition table for ESP Currently, the installation requires the ESP to be explicitly mounted. This change allows the bootloader logic to locate the ESP by scanning the partition table if the mount check fails. This change also updates 'get_esp_device' to return 'Cow' to avoid unnecessary allocations when returning a reference to the partition node. The baseline installation path is updated to allow this fallback, improving robustness when the filesystem is not fully mounted. Signed-off-by: Daniele Guarascio --- crates/lib/src/bootc_composefs/boot.rs | 1 + crates/lib/src/bootloader.rs | 28 ++++++++++++++++++++++++-- crates/lib/src/install/baseline.rs | 2 +- 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/crates/lib/src/bootc_composefs/boot.rs b/crates/lib/src/bootc_composefs/boot.rs index 18c1b2c98..e7f3d4e40 100644 --- a/crates/lib/src/bootc_composefs/boot.rs +++ b/crates/lib/src/bootc_composefs/boot.rs @@ -1291,6 +1291,7 @@ pub(crate) async fn setup_composefs_boot( &state.config_opts, None, get_secureboot_keys(&mounted_fs, BOOTC_AUTOENROLL_PATH)?, + root_setup.require_esp_mount, )?; } diff --git a/crates/lib/src/bootloader.rs b/crates/lib/src/bootloader.rs index e6f939609..4919390ba 100644 --- a/crates/lib/src/bootloader.rs +++ b/crates/lib/src/bootloader.rs @@ -1,3 +1,4 @@ +use std::borrow::Cow; use std::fs::create_dir_all; use std::process::Command; @@ -72,6 +73,28 @@ pub(crate) fn require_boot_efi_mount(root: &Dir) -> Result { Ok(source) } +/// Helper to find the ESP device, either via mount or partition table scan +pub(crate) fn get_esp_device<'a>( + root: &Dir, + device_info: &'a PartitionTable, + require_mount: bool, +) -> Result> { + if require_mount { + Ok(Cow::Owned(require_boot_efi_mount(root)?)) + } else { + match require_boot_efi_mount(root) { + Ok(p) => Ok(Cow::Owned(p)), + Err(e) => { + tracing::debug!( + "ESP mount check failed in permissive mode: {e}; falling back to partition table scan" + ); + let esp = esp_in(device_info)?; + Ok(Cow::Borrowed(&esp.node)) + } + } + } +} + /// Determine if the invoking environment contains bootupd, and if there are bootupd-based /// updates in the target root. #[context("Querying for bootupd")] @@ -180,13 +203,14 @@ pub(crate) fn install_via_bootupd( #[context("Installing bootloader")] pub(crate) fn install_systemd_boot( root: &Dir, - _device: &PartitionTable, + device: &PartitionTable, _rootfs: &Utf8Path, _configopts: &crate::install::InstallConfigOpts, _deployment_path: Option<&str>, autoenroll: Option, + require_mount: bool, ) -> Result<()> { - let esp_device = require_boot_efi_mount(root)?; + let esp_device = get_esp_device(root, device, require_mount)?; let esp_mount = mount_esp(&esp_device).context("Mounting ESP")?; let esp_path = Utf8Path::from_path(esp_mount.dir.path()) diff --git a/crates/lib/src/install/baseline.rs b/crates/lib/src/install/baseline.rs index e8c9f9a94..0d7280e1d 100644 --- a/crates/lib/src/install/baseline.rs +++ b/crates/lib/src/install/baseline.rs @@ -496,6 +496,6 @@ pub(crate) fn install_create_rootfs( boot, kargs, skip_finalize: false, - require_esp_mount: true, + require_esp_mount: false, }) } From 79b8874bf14d6f23880175205a5f90905696224a Mon Sep 17 00:00:00 2001 From: Daniele Guarascio Date: Sun, 8 Feb 2026 13:40:38 +0100 Subject: [PATCH 4/5] lib: Refactor open_target_root as method on RootSetup Move the open_target_root helper from bootc_composefs/boot.rs to install.rs as a method on the RootSetup struct. Signed-off-by: Daniele Guarascio --- crates/lib/src/bootc_composefs/boot.rs | 17 +++-------------- crates/lib/src/install.rs | 11 +++++++++++ 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/crates/lib/src/bootc_composefs/boot.rs b/crates/lib/src/bootc_composefs/boot.rs index e7f3d4e40..3341d34c7 100644 --- a/crates/lib/src/bootc_composefs/boot.rs +++ b/crates/lib/src/bootc_composefs/boot.rs @@ -212,17 +212,6 @@ fi ) } -fn open_target_root(root_setup: &RootSetup) -> Result { - if let Some(target_root) = root_setup.target_root_path.as_ref() { - Dir::open_ambient_dir(target_root, ambient_authority()).context("Opening target root path") - } else { - root_setup - .physical_root - .try_clone() - .context("Cloning target root handle") - } -} - pub fn get_esp_partition(device: &str) -> Result<(String, Option)> { let device_info = bootc_blockdev::partitions_of(Utf8Path::new(device))?; let esp = crate::bootloader::esp_in(&device_info)?; @@ -530,7 +519,7 @@ pub(crate) fn setup_composefs_bls_boot( cmdline_options.extend(&Cmdline::from(&composefs_cmdline)); // Locate ESP partition device - let esp_root = open_target_root(root_setup)?; + let esp_root = root_setup.open_target_root()?; let esp_part = if root_setup.require_esp_mount { crate::bootloader::require_boot_efi_mount(&esp_root)? } else { @@ -1086,7 +1075,7 @@ pub(crate) fn setup_composefs_uki_boot( BootSetupType::Setup((root_setup, state, postfetch, ..)) => { state.require_no_kargs_for_uki()?; - let esp_root = open_target_root(root_setup)?; + let esp_root = root_setup.open_target_root()?; let esp_part = if root_setup.require_esp_mount { crate::bootloader::require_boot_efi_mount(&esp_root)? @@ -1269,7 +1258,7 @@ pub(crate) async fn setup_composefs_boot( .or(root_setup.rootfs_uuid.as_deref()) .ok_or_else(|| anyhow!("No uuid for boot/root"))?; - let target_root = open_target_root(root_setup)?; + let target_root = root_setup.open_target_root()?; if cfg!(target_arch = "s390x") { // TODO: Integrate s390x support into install_via_bootupd diff --git a/crates/lib/src/install.rs b/crates/lib/src/install.rs index 833f80c6a..8862537ca 100644 --- a/crates/lib/src/install.rs +++ b/crates/lib/src/install.rs @@ -1307,6 +1307,17 @@ fn require_boot_uuid(spec: &MountSpec) -> Result<&str> { } impl RootSetup { + pub(crate) fn open_target_root(&self) -> Result { + if let Some(target_root) = self.target_root_path.as_ref() { + Dir::open_ambient_dir(target_root, cap_std::ambient_authority()) + .context("Opening target root path") + } else { + self.physical_root + .try_clone() + .context("Cloning target root handle") + } + } + /// Get the UUID= mount specifier for the /boot filesystem; if there isn't one, the root UUID will /// be returned. pub(crate) fn get_boot_uuid(&self) -> Result> { From f3def476eab45fae1d3035a201629745d43d3470 Mon Sep 17 00:00:00 2001 From: Daniele Guarascio Date: Sun, 8 Feb 2026 15:06:02 +0100 Subject: [PATCH 5/5] lib: Support ESP mounted at /efi and /boot Update the ESP discovery logic to explicitly check for mounts at /efi (per DPS) and /boot (shared ESP), in addition to the traditional /boot/efi. This ensures correct handling of cleanup, bootloader installation, and composefs setup when the ESP is mounted in these alternative locations. Also updates unit tests to verify Cow usage in ESP source normalization. Signed-off-by: Daniele Guarascio --- crates/lib/src/bootc_composefs/boot.rs | 20 ++-- crates/lib/src/bootloader.rs | 131 ++++++++++++++++++------- crates/lib/src/install.rs | 32 +++--- 3 files changed, 124 insertions(+), 59 deletions(-) diff --git a/crates/lib/src/bootc_composefs/boot.rs b/crates/lib/src/bootc_composefs/boot.rs index 3341d34c7..479ed4d19 100644 --- a/crates/lib/src/bootc_composefs/boot.rs +++ b/crates/lib/src/bootc_composefs/boot.rs @@ -520,11 +520,11 @@ pub(crate) fn setup_composefs_bls_boot( // Locate ESP partition device let esp_root = root_setup.open_target_root()?; - let esp_part = if root_setup.require_esp_mount { - crate::bootloader::require_boot_efi_mount(&esp_root)? + let esp_device = if root_setup.require_esp_mount { + crate::bootloader::find_esp_mount(&esp_root)?.device } else { - match crate::bootloader::require_boot_efi_mount(&esp_root) { - Ok(p) => p, + match crate::bootloader::find_esp_mount(&esp_root) { + Ok(esp) => esp.device, Err(e) => { tracing::debug!( "ESP mount check failed in permissive mode: {e}; falling back to partition table scan" @@ -537,7 +537,7 @@ pub(crate) fn setup_composefs_bls_boot( ( root_setup.physical_root_path.clone(), - esp_part, + esp_device, cmdline_options, fs, postfetch.detected_bootloader.clone(), @@ -1077,11 +1077,11 @@ pub(crate) fn setup_composefs_uki_boot( let esp_root = root_setup.open_target_root()?; - let esp_part = if root_setup.require_esp_mount { - crate::bootloader::require_boot_efi_mount(&esp_root)? + let esp_device = if root_setup.require_esp_mount { + crate::bootloader::find_esp_mount(&esp_root)?.device } else { - match crate::bootloader::require_boot_efi_mount(&esp_root) { - Ok(p) => p, + match crate::bootloader::find_esp_mount(&esp_root) { + Ok(esp) => esp.device, Err(e) => { tracing::debug!( "ESP mount check failed in permissive mode: {e}; falling back to partition table scan" @@ -1094,7 +1094,7 @@ pub(crate) fn setup_composefs_uki_boot( ( root_setup.physical_root_path.clone(), - esp_part, + esp_device, postfetch.detected_bootloader.clone(), state.composefs_options.insecure, state.composefs_options.uki_addon.as_ref(), diff --git a/crates/lib/src/bootloader.rs b/crates/lib/src/bootloader.rs index 4919390ba..2188a5203 100644 --- a/crates/lib/src/bootloader.rs +++ b/crates/lib/src/bootloader.rs @@ -4,7 +4,7 @@ use std::process::Command; use anyhow::{Context, Result, anyhow, bail}; use bootc_utils::{BwrapCmd, CommandRunExt}; -use camino::Utf8Path; +use camino::{Utf8Path, Utf8PathBuf}; use cap_std_ext::cap_std::fs::Dir; use cap_std_ext::dirext::CapStdExtDirExt; use fn_error_context::context; @@ -17,6 +17,7 @@ use crate::{discoverable_partition_specification, utils}; /// The name of the mountpoint for efi (as a subdirectory of /boot, or at the toplevel) pub(crate) const EFI_DIR: &str = "efi"; + /// The EFI system partition GUID /// Path to the bootupd update payload #[allow(dead_code)] @@ -25,6 +26,14 @@ const BOOTUPD_UPDATES: &str = "usr/lib/bootupd/updates"; // from: https://github.com/systemd/systemd/blob/26b2085d54ebbfca8637362eafcb4a8e3faf832f/man/systemd-boot.xml#L392 const SYSTEMD_KEY_DIR: &str = "loader/keys"; +/// Represents a found EFI System Partition (ESP) mount +pub(crate) struct Esp { + /// The backing device path + pub(crate) device: String, + /// The mountpoint path relative to the root + pub(crate) path: Utf8PathBuf, +} + #[allow(dead_code)] pub(crate) fn esp_in(device: &PartitionTable) -> Result<&Partition> { device @@ -32,45 +41,77 @@ pub(crate) fn esp_in(device: &PartitionTable) -> Result<&Partition> { .ok_or(anyhow::anyhow!("ESP not found in partition table")) } -fn normalize_esp_source(source: String) -> String { +/// Convert a source specification like UUID=... or LABEL=... into a +/// canonical /dev/disk/by-... path. If the source is already a +/// path, it is returned unchanged. +fn normalize_esp_source(source: &str) -> Cow<'_, str> { if let Some(uuid) = source.strip_prefix("UUID=") { - format!("/dev/disk/by-uuid/{uuid}") + Cow::Owned(format!("/dev/disk/by-uuid/{uuid}")) } else if let Some(label) = source.strip_prefix("LABEL=") { - format!("/dev/disk/by-label/{label}") + Cow::Owned(format!("/dev/disk/by-label/{label}")) } else { - source + Cow::Borrowed(source) } } -pub(crate) fn require_boot_efi_mount(root: &Dir) -> Result { - let efi_path = Utf8Path::new("boot").join(EFI_DIR); - let esp_fd = root - .open_dir_optional(&efi_path) - .context("Opening /boot/efi")? - .ok_or_else(|| anyhow::anyhow!("/boot/efi does not exist"))?; +/// Helper to check if a path is a valid ESP mount +fn inspect_esp_at(root: &Dir, path: impl AsRef) -> Result> { + let path = path.as_ref(); + let esp_fd = match root.open_dir_optional(path)? { + Some(fd) => fd, + None => return Ok(None), + }; if !esp_fd.is_mountpoint(".")?.unwrap_or(false) { - anyhow::bail!("/boot/efi is not a mountpoint. This is required for installation."); + return Ok(None); } let fs = bootc_mount::inspect_filesystem_of_dir(&esp_fd)?; if fs.fstype != "vfat" && fs.fstype != "fat" { - anyhow::bail!( - "/boot/efi is mounted but is not vfat/fat (found {}). This is required for installation.", - fs.fstype - ); + return Ok(None); } - let source = normalize_esp_source(fs.source); + let source = normalize_esp_source(&fs.source); - let source_path = Utf8Path::new(&source); + let source_path = Utf8Path::new(source.as_ref()); if !source_path.try_exists()? { - anyhow::bail!( - "/boot/efi source device does not exist: {source}. This is required for installation." - ); + return Ok(None); } - Ok(source) + Ok(Some(source.into_owned())) +} + +/// Find the ESP mountpoint by searching common locations (/efi, /boot/efi, /boot) +pub(crate) fn find_esp_mount(root: &Dir) -> Result { + // Possible paths for ESP, in order of preference. + // We check /efi first as per DPS, then /boot/efi (distro standard), + // and finally /boot itself (if it is the ESP). + if let Some(device) = inspect_esp_at(root, EFI_DIR)? { + tracing::debug!("Found ESP at {EFI_DIR} (device: {device})"); + return Ok(Esp { + device, + path: Utf8PathBuf::from(EFI_DIR), + }); + } + + let boot_efi = Utf8Path::new("boot").join(EFI_DIR); + if let Some(device) = inspect_esp_at(root, &boot_efi)? { + tracing::debug!("Found ESP at {boot_efi} (device: {device})"); + return Ok(Esp { + device, + path: boot_efi, + }); + } + + if let Some(device) = inspect_esp_at(root, "boot")? { + tracing::debug!("Found ESP at boot (device: {device})"); + return Ok(Esp { + device, + path: Utf8PathBuf::from("boot"), + }); + } + + anyhow::bail!("No ESP found at /efi, /boot/efi or /boot. This is required for installation.") } /// Helper to find the ESP device, either via mount or partition table scan @@ -80,10 +121,10 @@ pub(crate) fn get_esp_device<'a>( require_mount: bool, ) -> Result> { if require_mount { - Ok(Cow::Owned(require_boot_efi_mount(root)?)) + Ok(Cow::Owned(find_esp_mount(root)?.device)) } else { - match require_boot_efi_mount(root) { - Ok(p) => Ok(Cow::Owned(p)), + match find_esp_mount(root) { + Ok(esp) => Ok(Cow::Owned(esp.device)), Err(e) => { tracing::debug!( "ESP mount check failed in permissive mode: {e}; falling back to partition table scan" @@ -117,12 +158,11 @@ pub(crate) fn install_via_bootupd( deployment_path: Option<&str>, require_mount: bool, ) -> Result<()> { - if require_mount { - // We require /boot/efi to be mounted; finding the device is just a sanity check that it is. - // The device argument is currently used by bootupctl as a fallback if it can't find the ESP. - // But we want to fail fast if our own check fails. - let _esp_device = require_boot_efi_mount(root)?; - } + let esp = if require_mount { + Some(find_esp_mount(root)?) + } else { + find_esp_mount(root).ok() + }; let verbose = std::env::var_os("BOOTC_BOOTLOADER_DEBUG").map(|_| "-vvvv"); // bootc defaults to only targeting the platform boot method. @@ -163,6 +203,8 @@ pub(crate) fn install_via_bootupd( if let Some(deploy) = deployment_path { let target_root = rootfs.join(deploy); let boot_path = rootfs.join("boot"); + let efi_path = rootfs.join(EFI_DIR); + let efi_target = format!("/{EFI_DIR}"); tracing::debug!("Running bootupctl via bwrap in {}", target_root); @@ -177,6 +219,15 @@ pub(crate) fn install_via_bootupd( // Bind the target block device inside the bwrap container so bootupctl can access it .bind_device(device.path().as_str()); + // If we found an ESP, ensure it's bound. + // If it's at /boot or /boot/efi, it's already bound via /boot. + // If it's at /efi, we need to bind it explicitly. + if let Some(esp) = esp { + if esp.path == EFI_DIR { + cmd = cmd.bind(&efi_path, &efi_target); + } + } + // Also bind all partitions of the tafet block device for partition in &device.partitions { cmd = cmd.bind_device(&partition.node); @@ -334,20 +385,24 @@ mod tests { use super::*; #[test] - fn normalize_esp_source_uuid() { - let normalized = normalize_esp_source("UUID=abcd-1234".to_string()); + fn test_normalize_esp_source_uuid() { + let normalized = normalize_esp_source("UUID=abcd-1234"); assert_eq!(normalized, "/dev/disk/by-uuid/abcd-1234"); + assert!(matches!(normalized, Cow::Owned(_))); } #[test] - fn normalize_esp_source_label() { - let normalized = normalize_esp_source("LABEL=EFI".to_string()); + fn test_normalize_esp_source_label() { + let normalized = normalize_esp_source("LABEL=EFI"); assert_eq!(normalized, "/dev/disk/by-label/EFI"); + assert!(matches!(normalized, Cow::Owned(_))); } #[test] - fn normalize_esp_source_passthrough() { - let normalized = normalize_esp_source("/dev/sda1".to_string()); - assert_eq!(normalized, "/dev/sda1"); + fn test_normalize_esp_source_passthrough() { + let path = "/dev/sda1"; + let normalized = normalize_esp_source(path); + assert_eq!(normalized, path); + assert!(matches!(normalized, Cow::Borrowed(_))); } } diff --git a/crates/lib/src/install.rs b/crates/lib/src/install.rs index 8862537ca..3e06fb728 100644 --- a/crates/lib/src/install.rs +++ b/crates/lib/src/install.rs @@ -2176,21 +2176,31 @@ fn clean_boot_directories(rootfs: &Dir, is_ostree: bool, strict_esp: bool) -> Re let bootdir = crate::utils::open_dir_remount_rw(rootfs, BOOT.into()).context("Opening /boot")?; - if ARCH_USES_EFI && strict_esp { - // Require an explicit /boot/efi mount to avoid cleaning the wrong ESP. - crate::bootloader::require_boot_efi_mount(rootfs)?; - } + let esp = if ARCH_USES_EFI { + if strict_esp { + Some(crate::bootloader::find_esp_mount(rootfs)?) + } else { + crate::bootloader::find_esp_mount(rootfs).ok() + } + } else { + None + }; - // This should not remove /boot/efi note. + // This should not remove the ESP if it is a mount point. remove_all_except_loader_dirs(&bootdir, is_ostree).context("Emptying /boot")?; + // If we have an ESP that is NOT /boot, we empty it. + // If it is /boot, we already did a selective empty above. // TODO: we should also support not wiping the ESP. - if ARCH_USES_EFI { - if let Some(efidir) = bootdir - .open_dir_optional(crate::bootloader::EFI_DIR) - .context("Opening /boot/efi")? - { - remove_all_in_dir_no_xdev(&efidir, false).context("Emptying EFI system partition")?; + if let Some(esp) = esp { + if esp.path != BOOT { + if let Some(efidir) = rootfs + .open_dir_optional(&esp.path) + .with_context(|| format!("Opening ESP at {}", esp.path))? + { + remove_all_in_dir_no_xdev(&efidir, false) + .context("Emptying EFI system partition")?; + } } }