From 2d47c5f7f5b29b8147df5a01c9d798df7c07bf79 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 1 Apr 2026 21:47:43 +0000 Subject: [PATCH 1/9] composefs: Open deployments dir from physical root, not ambient path find_vmlinuz_initrd_duplicates() previously opened the absolute path /sysroot/state/deploy via STATE_DIR_ABS, which during install belongs to the host, not the target. Fix by passing a Dir opened relative to the target's physical_root. During fresh install the state dir naturally does not exist yet, so open_dir_optional returns None and the check is skipped -- no special-case guard needed. Also plumb physical_root through the setup_composefs_bls_boot match arms, and replace the hardcoded "/sysroot" in the Upgrade arm with storage.physical_root_path. Assisted-by: OpenCode (Claude Opus 4) Signed-off-by: Colin Walters --- crates/lib/src/bootc_composefs/boot.rs | 42 +++++++++++++++++--------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/crates/lib/src/bootc_composefs/boot.rs b/crates/lib/src/bootc_composefs/boot.rs index 646df9ff0..6e4cc25ce 100644 --- a/crates/lib/src/bootc_composefs/boot.rs +++ b/crates/lib/src/bootc_composefs/boot.rs @@ -117,7 +117,7 @@ use crate::{ use crate::{ composefs_consts::{ BOOT_LOADER_ENTRIES, ORIGIN_KEY_BOOT, ORIGIN_KEY_BOOT_DIGEST, STAGED_BOOT_LOADER_ENTRIES, - STATE_DIR_ABS, USER_CFG, USER_CFG_STAGED, + STATE_DIR_RELATIVE, USER_CFG, USER_CFG_STAGED, }, spec::{Bootloader, Host}, }; @@ -354,22 +354,21 @@ pub(crate) fn compute_boot_digest_uki(uki: &[u8]) -> Result { /// Given the SHA256 sum of current VMlinuz + Initrd combo, find boot entry with the same SHA256Sum /// +/// `deployments_dir` should be the composefs state/deploy directory opened +/// relative to the target physical root. This avoids using ambient absolute +/// paths, which would be wrong during install (where `/sysroot/state/deploy` +/// belongs to the host, not the target). +/// /// # Returns /// Returns the verity of all deployments that have a boot digest same as the one passed in #[context("Checking boot entry duplicates")] -pub(crate) fn find_vmlinuz_initrd_duplicates(digest: &str) -> Result>> { - let deployments = Dir::open_ambient_dir(STATE_DIR_ABS, ambient_authority()); - - let deployments = match deployments { - Ok(d) => d, - // The first ever deployment - Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(None), - Err(e) => anyhow::bail!(e), - }; - +pub(crate) fn find_vmlinuz_initrd_duplicates( + deployments_dir: &Dir, + digest: &str, +) -> Result>> { let mut symlink_to: Option> = None; - for depl in deployments.entries()? { + for depl in deployments_dir.entries()? { let depl = depl?; let depl_file_name = depl.file_name(); @@ -518,6 +517,11 @@ pub(crate) fn setup_composefs_bls_boot( ) -> Result { let id_hex = id.to_hex(); + let physical_root = match &setup_type { + BootSetupType::Setup((root_setup, ..)) => &root_setup.physical_root, + BootSetupType::Upgrade((storage, ..)) => &storage.physical_root, + }; + let (root_path, esp_device, mut cmdline_refs, fs, bootloader) = match setup_type { BootSetupType::Setup((root_setup, state, postfetch, fs)) => { // root_setup.kargs has [root=UUID=, "rw"] @@ -573,7 +577,7 @@ pub(crate) fn setup_composefs_bls_boot( let esp_dev = root_dev.find_partition_of_esp()?; ( - Utf8PathBuf::from("/sysroot"), + storage.physical_root_path.clone(), esp_dev.path(), cmdline, fs, @@ -687,7 +691,17 @@ pub(crate) fn setup_composefs_bls_boot( options: Some(cmdline_refs), }); - match find_vmlinuz_initrd_duplicates(&boot_digest)? { + // Check for shared boot binaries with existing deployments. + // On fresh install the state dir won't exist yet, so this is + // naturally a no-op. + let shared_boot_binaries = + match physical_root.open_dir_optional(STATE_DIR_RELATIVE)? { + Some(deploy_dir) => { + find_vmlinuz_initrd_duplicates(&deploy_dir, &boot_digest)? + } + None => None, + }; + match shared_boot_binaries { Some(shared_entries) => { // Multiple deployments could be using the same kernel + initrd, but there // would be only one available From fc6e9b73ce0f1c97b2eb96f43942d316108a15c3 Mon Sep 17 00:00:00 2001 From: ckyrouac Date: Wed, 11 Mar 2026 15:39:03 -0400 Subject: [PATCH 2/9] install: Enable installing to devices with multiple parents Assisted-by: Claude Code (Opus 4) Signed-off-by: ckyrouac Signed-off-by: Colin Walters --- crates/lib/src/bootloader.rs | 109 +++++- crates/lib/src/install.rs | 5 +- crates/utils/src/bwrap.rs | 21 +- hack/provision-derived.sh | 38 ++ tmt/plans/integration.fmf | 11 + tmt/tests/booted/test-multi-device-esp.nu | 441 ++++++++++++++++++++++ tmt/tests/test-39-multi-device-esp.fmf | 7 + 7 files changed, 614 insertions(+), 18 deletions(-) create mode 100644 tmt/tests/booted/test-multi-device-esp.nu create mode 100644 tmt/tests/test-39-multi-device-esp.fmf diff --git a/crates/lib/src/bootloader.rs b/crates/lib/src/bootloader.rs index 0c19cc04f..841af4d10 100644 --- a/crates/lib/src/bootloader.rs +++ b/crates/lib/src/bootloader.rs @@ -23,7 +23,13 @@ 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"; -/// Mount ESP part at /boot/efi +/// Mount the first ESP found among backing devices at /boot/efi. +/// +/// This is used by the install-alongside path to clean stale bootloader +/// files before reinstallation. On multi-device setups only the first +/// ESP is mounted and cleaned; stale files on additional ESPs are left +/// 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<()> { let efi_path = Utf8Path::new("boot").join(crate::bootloader::EFI_DIR); let Some(esp_fd) = root @@ -45,11 +51,14 @@ pub(crate) fn mount_esp_part(root: &Dir, root_path: &Utf8Path, is_ostree: bool) root }; - let dev = bootc_blockdev::list_dev_by_dir(physical_root)?.require_single_root()?; - if let Some(esp_dev) = dev.find_partition_of_type(bootc_blockdev::ESP) { - let esp_path = esp_dev.path(); - bootc_mount::mount(&esp_path, &root_path.join(&efi_path))?; - tracing::debug!("Mounted {esp_path} at /boot/efi"); + let roots = bootc_blockdev::list_dev_by_dir(physical_root)?.find_all_roots()?; + for dev in &roots { + if let Some(esp_dev) = dev.find_partition_of_type(bootc_blockdev::ESP) { + let esp_path = esp_dev.path(); + bootc_mount::mount(&esp_path, &root_path.join(&efi_path))?; + tracing::debug!("Mounted {esp_path} at /boot/efi"); + return Ok(()); + } } Ok(()) } @@ -67,6 +76,48 @@ pub(crate) fn supports_bootupd(root: &Dir) -> Result { Ok(r) } +/// Check whether the target bootupd supports `--filesystem`. +/// +/// Runs `bootupctl backend install --help` and looks for `--filesystem` in the +/// output. When `deployment_path` is set the command runs inside a bwrap +/// container so we probe the binary from the target image. +fn bootupd_supports_filesystem(rootfs: &Utf8Path, deployment_path: Option<&str>) -> Result { + let help_args = ["bootupctl", "backend", "install", "--help"]; + let output = if let Some(deploy) = deployment_path { + let target_root = rootfs.join(deploy); + BwrapCmd::new(&target_root) + .setenv( + "PATH", + "/bin:/usr/bin:/sbin:/usr/sbin:/usr/local/bin:/usr/local/sbin", + ) + .run_get_string(help_args)? + } else { + Command::new("bootupctl") + .args(&help_args[1..]) + .log_debug() + .run_get_string()? + }; + + let use_filesystem = output.contains("--filesystem"); + + if use_filesystem { + tracing::debug!("bootupd supports --filesystem"); + } else { + tracing::debug!("bootupd does not support --filesystem, falling back to --device"); + } + + Ok(use_filesystem) +} + +/// Install the bootloader via bootupd. +/// +/// When the target bootupd supports `--filesystem` we pass it pointing at a +/// block-backed mount so that bootupd can resolve the backing device(s) itself +/// via `lsblk`. In the bwrap path we bind-mount the physical root at +/// `/sysroot` to give `lsblk` a real block-backed path. +/// +/// For older bootupd versions that lack `--filesystem` we fall back to the +/// legacy `--device ` invocation. #[context("Installing bootloader")] pub(crate) fn install_via_bootupd( device: &bootc_blockdev::Device, @@ -91,8 +142,6 @@ pub(crate) fn install_via_bootupd( println!("Installing bootloader via bootupd"); - let device_path = device.path(); - // Build the bootupctl arguments let mut bootupd_args: Vec<&str> = vec!["backend", "install"]; if configopts.bootupd_skip_boot_uuid { @@ -107,7 +156,29 @@ pub(crate) fn install_via_bootupd( if let Some(ref opts) = bootupd_opts { bootupd_args.extend(opts.iter().copied()); } - bootupd_args.extend(["--device", &device_path, rootfs_mount]); + + // When the target bootupd lacks --filesystem support, fall back to the + // legacy --device flag. For --device we need the whole-disk device path + // (e.g. /dev/vda), not a partition (e.g. /dev/vda3), so resolve the + // parent via require_single_root(). (Older bootupd doesn't support + // multiple backing devices anyway.) + // Computed before building bootupd_args so the String lives long enough. + let root_device_path = if bootupd_supports_filesystem(rootfs, deployment_path) + .context("Probing bootupd --filesystem support")? + { + None + } else { + Some(device.require_single_root()?.path()) + }; + if let Some(ref dev) = root_device_path { + tracing::debug!("bootupd does not support --filesystem, falling back to --device {dev}"); + bootupd_args.extend(["--device", dev]); + bootupd_args.push(rootfs_mount); + } else { + tracing::debug!("bootupd supports --filesystem"); + bootupd_args.extend(["--filesystem", rootfs_mount]); + bootupd_args.push(rootfs_mount); + } // Run inside a bwrap container. It takes care of mounting and creating // the necessary API filesystems in the target deployment and acts as @@ -115,6 +186,7 @@ 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 rootfs_path = rootfs.to_path_buf(); tracing::debug!("Running bootupctl via bwrap in {}", target_root); @@ -122,11 +194,17 @@ pub(crate) fn install_via_bootupd( let mut bwrap_args = vec!["bootupctl"]; bwrap_args.extend(bootupd_args); - let cmd = BwrapCmd::new(&target_root) + let mut cmd = BwrapCmd::new(&target_root) // Bind mount /boot from the physical target root so bootupctl can find // the boot partition and install the bootloader there .bind(&boot_path, &"/boot"); + // Only bind mount the physical root at /sysroot when using --filesystem; + // bootupd needs it to resolve backing block devices via lsblk. + if root_device_path.is_none() { + cmd = cmd.bind(&rootfs_path, &"/sysroot"); + } + // The $PATH in the bwrap env is not complete enough for some images // so we inject a reasonnable default. // This is causing bootupctl and/or sfdisk binaries @@ -145,6 +223,11 @@ pub(crate) fn install_via_bootupd( } } +/// Install systemd-boot to the first ESP found among backing devices. +/// +/// On multi-device setups only the first ESP is installed to; additional +/// ESPs on other backing devices are left untouched. +// TODO: install to all ESPs on multi-device setups #[context("Installing bootloader")] pub(crate) fn install_systemd_boot( device: &bootc_blockdev::Device, @@ -153,8 +236,10 @@ pub(crate) fn install_systemd_boot( _deployment_path: Option<&str>, autoenroll: Option, ) -> Result<()> { - let esp_part = device - .find_partition_of_type(discoverable_partition_specification::ESP) + let roots = device.find_all_roots()?; + let esp_part = roots + .iter() + .find_map(|root| root.find_partition_of_type(discoverable_partition_specification::ESP)) .ok_or_else(|| anyhow::anyhow!("ESP partition not found"))?; let esp_mount = mount_esp(&esp_part.path()).context("Mounting ESP")?; diff --git a/crates/lib/src/install.rs b/crates/lib/src/install.rs index 6d0dcc607..84123eb06 100644 --- a/crates/lib/src/install.rs +++ b/crates/lib/src/install.rs @@ -2570,9 +2570,8 @@ pub(crate) async fn install_to_filesystem( // Find the real underlying backing device for the root. This is currently just required // for GRUB (BIOS) and in the future zipl (I think). let device_info = { - let dev = - bootc_blockdev::list_dev(Utf8Path::new(&inspect.source))?.require_single_root()?; - tracing::debug!("Backing device: {}", dev.path()); + let dev = bootc_blockdev::list_dev(Utf8Path::new(&inspect.source))?; + tracing::debug!("Target filesystem backing device: {}", dev.path()); dev }; diff --git a/crates/utils/src/bwrap.rs b/crates/utils/src/bwrap.rs index 353edb10d..3d3069ead 100644 --- a/crates/utils/src/bwrap.rs +++ b/crates/utils/src/bwrap.rs @@ -59,8 +59,8 @@ impl<'a> BwrapCmd<'a> { self } - /// Run the specified command inside the container. - pub fn run>(self, args: impl IntoIterator) -> Result<()> { + /// Build the bwrap `Command` with all bind mounts, env vars, and args. + fn build_command>(&self, args: impl IntoIterator) -> Command { let mut cmd = Command::new("bwrap"); // Bind the root filesystem @@ -92,6 +92,21 @@ impl<'a> BwrapCmd<'a> { cmd.arg("--"); cmd.args(args); - cmd.log_debug().run_inherited_with_cmd_context() + cmd + } + + /// Run the specified command inside the container. + pub fn run>(self, args: impl IntoIterator) -> Result<()> { + self.build_command(args) + .log_debug() + .run_inherited_with_cmd_context() + } + + /// Run the specified command inside the container and capture stdout as a string. + pub fn run_get_string>( + self, + args: impl IntoIterator, + ) -> Result { + self.build_command(args).log_debug().run_get_string() } } diff --git a/hack/provision-derived.sh b/hack/provision-derived.sh index 59d9d15c6..1968ada61 100755 --- a/hack/provision-derived.sh +++ b/hack/provision-derived.sh @@ -69,6 +69,44 @@ resize_rootfs: false CLOUDEOF fi +# Temporary: update bootupd from @CoreOS/continuous copr until +# base images include a version supporting --filesystem +. /usr/lib/os-release +case $ID in + fedora) copr_distro="fedora" ;; + *) copr_distro="centos-stream" ;; +esac +# Update bootc from rhcontainerbot copr; the new bootupd +# requires a newer bootc than what ships in some base images. +cat >/etc/yum.repos.d/rhcontainerbot-bootc.repo </etc/yum.repos.d/coreos-continuous.repo <, mountpoint: string] { + # Unmount if mounted + do { umount $mountpoint } | complete | ignore + do { rmdir $mountpoint } | complete | ignore + + # Deactivate and remove LVM + do { lvchange -an $"($vg_name)/test_lv" } | complete | ignore + do { lvremove -f $"($vg_name)/test_lv" } | complete | ignore + do { vgchange -an $vg_name } | complete | ignore + do { vgremove -f $vg_name } | complete | ignore + + # Remove PVs and detach loop devices + for loop in $loops { + if ($loop | path exists) { + do { pvremove -f $loop } | complete | ignore + do { losetup -d $loop } | complete | ignore + } + } +} + +# Create a disk with GPT, optional ESP, and LVM partition +# Returns the loop device path +def setup_disk_with_partitions [ + disk_path: string, + with_esp: bool, + disk_size: string = "5G" +] { + # Create disk image + truncate -s $disk_size $disk_path + + # Setup loop device + let loop = (losetup -f --show $disk_path | str trim) + + # Create partition table + if $with_esp { + # GPT with ESP (512MB) + LVM partition + $"label: gpt\nsize=512M, type=($ESP_TYPE)\ntype=($LVM_TYPE)\n" | sfdisk $loop + + # Reload partition table (partx is part of util-linux) + partx -u $loop + sleep 1sec + + # Format ESP + mkfs.vfat -F 32 $"($loop)p1" + } else { + # GPT with only LVM partition (full disk) + $"label: gpt\ntype=($LVM_TYPE)\n" | sfdisk $loop + + # Reload partition table (partx is part of util-linux) + partx -u $loop + sleep 1sec + } + + $loop +} + +# Create a disk with GPT, ESP, and a root partition (no LVM) +# Returns the loop device path +def setup_disk_with_root [ + disk_path: string, + disk_size: string = "5G" +] { + truncate -s $disk_size $disk_path + let loop = (losetup -f --show $disk_path | str trim) + + # GPT with ESP (512MB) + root partition + $"label: gpt\nsize=512M, type=($ESP_TYPE)\ntype=($ROOT_TYPE)\n" | sfdisk $loop + partx -u $loop + sleep 1sec + + mkfs.vfat -F 32 $"($loop)p1" + mkfs.ext4 -q $"($loop)p2" + + $loop +} + +# Simple cleanup for non-LVM scenarios (single loop device, no VG) +def cleanup_simple [loop: string, mountpoint: string] { + do { umount $mountpoint } | complete | ignore + do { rmdir $mountpoint } | complete | ignore + + if ($loop | path exists) { + do { losetup -d $loop } | complete | ignore + } +} + +# Validate that an ESP partition has bootloader files installed +def validate_esp [esp_partition: string] { + let esp_mount = "/var/mnt/esp_check" + mkdir $esp_mount + mount $esp_partition $esp_mount + + # Check for EFI directory with bootloader files + let efi_dir = $"($esp_mount)/EFI" + if not ($efi_dir | path exists) { + umount $esp_mount + rmdir $esp_mount + error make {msg: $"ESP validation failed: EFI directory not found on ($esp_partition)"} + } + + # Verify there's actual content in EFI (not just empty) + let efi_contents = (ls $efi_dir | length) + umount $esp_mount + rmdir $esp_mount + + if $efi_contents == 0 { + error make {msg: $"ESP validation failed: EFI directory is empty on ($esp_partition)"} + } +} + +# Run bootc install to-existing-root from within the container image under test +def run_install [mountpoint: string] { + (podman run + --rm + --privileged + -v $"($mountpoint):/target" + -v /dev:/dev + -v /run/udev:/run/udev:ro + -v /usr/share/empty:/usr/lib/bootc/bound-images.d + --pid=host + --security-opt label=type:unconfined_t + --env BOOTC_BOOTLOADER_DEBUG=1 + $target_image + bootc install to-existing-root + --disable-selinux + --acknowledge-destructive + --target-no-signature-verification + /target) +} + +# Test scenario 1: Single ESP on first device +def test_single_esp [] { + tap begin "multi-device ESP detection tests" + + bootc image copy-to-storage + + print "Starting single ESP test" + + let vg_name = "test_single_esp_vg" + let mountpoint = "/var/mnt/test_single_esp" + let disk1 = "/var/tmp/disk1_single.img" + let disk2 = "/var/tmp/disk2_single.img" + + # Setup disks + # DISK1: ESP + LVM partition + # DISK2: Full LVM partition (no ESP) + let loop1 = (setup_disk_with_partitions $disk1 true) + let loop2 = (setup_disk_with_partitions $disk2 false) + + try { + # Create LVM spanning both devices + # Use partition 2 from disk1 (after ESP) and partition 1 from disk2 (full disk) + pvcreate $"($loop1)p2" $"($loop2)p1" + vgcreate $vg_name $"($loop1)p2" $"($loop2)p1" + lvcreate -l "100%FREE" -n test_lv $vg_name + + let lv_path = $"/dev/($vg_name)/test_lv" + + # Create filesystem and mount + mkfs.ext4 -q $lv_path + mkdir $mountpoint + mount $lv_path $mountpoint + + # Create boot directory + mkdir $"($mountpoint)/boot" + + # Show block device hierarchy + lsblk --pairs --paths --inverse --output NAME,TYPE $lv_path + + run_install $mountpoint + + # Validate ESP was installed correctly + validate_esp $"($loop1)p1" + } catch {|e| + cleanup $vg_name [$loop1, $loop2] $mountpoint + rm -f $disk1 $disk2 + error make {msg: $"Single ESP test failed: ($e)"} + } + + # Cleanup + cleanup $vg_name [$loop1, $loop2] $mountpoint + rm -f $disk1 $disk2 + + print "Single ESP test completed successfully" + tmt-reboot +} + +# Test scenario 2: ESP on both devices +def test_dual_esp [] { + print "Starting dual ESP test" + + let vg_name = "test_dual_esp_vg" + let mountpoint = "/var/mnt/test_dual_esp" + let disk1 = "/var/tmp/disk1_dual.img" + let disk2 = "/var/tmp/disk2_dual.img" + + # Setup disks + # DISK1: ESP + LVM partition + # DISK2: ESP + LVM partition + let loop1 = (setup_disk_with_partitions $disk1 true) + let loop2 = (setup_disk_with_partitions $disk2 true) + + try { + # Create LVM spanning both devices + # Use partition 2 from both disks (after ESP) + pvcreate $"($loop1)p2" $"($loop2)p2" + vgcreate $vg_name $"($loop1)p2" $"($loop2)p2" + lvcreate -l "100%FREE" -n test_lv $vg_name + + let lv_path = $"/dev/($vg_name)/test_lv" + + # Create filesystem and mount + mkfs.ext4 -q $lv_path + mkdir $mountpoint + mount $lv_path $mountpoint + + # Create boot directory + mkdir $"($mountpoint)/boot" + + # Show block device hierarchy + lsblk --pairs --paths --inverse --output NAME,TYPE $lv_path + + run_install $mountpoint + + # Validate both ESPs were installed correctly + validate_esp $"($loop1)p1" + validate_esp $"($loop2)p1" + } catch {|e| + cleanup $vg_name [$loop1, $loop2] $mountpoint + rm -f $disk1 $disk2 + error make {msg: $"Dual ESP test failed: ($e)"} + } + + # Cleanup + cleanup $vg_name [$loop1, $loop2] $mountpoint + rm -f $disk1 $disk2 + + print "Dual ESP test completed successfully" +} + +# Test scenario 3: Three devices, ESP on disk1 and disk3 only +def test_three_devices_partial_esp [] { + print "Starting three devices partial ESP test" + + let vg_name = "test_three_dev_vg" + let mountpoint = "/var/mnt/test_three_dev" + let disk1 = "/var/tmp/disk1_three.img" + let disk2 = "/var/tmp/disk2_three.img" + let disk3 = "/var/tmp/disk3_three.img" + + # Setup disks + # DISK1: ESP + LVM partition + # DISK2: Full LVM partition (no ESP) + # DISK3: ESP + LVM partition + let loop1 = (setup_disk_with_partitions $disk1 true) + let loop2 = (setup_disk_with_partitions $disk2 false) + let loop3 = (setup_disk_with_partitions $disk3 true) + + try { + # Create LVM spanning all three devices + pvcreate $"($loop1)p2" $"($loop2)p1" $"($loop3)p2" + vgcreate $vg_name $"($loop1)p2" $"($loop2)p1" $"($loop3)p2" + lvcreate -l "100%FREE" -n test_lv $vg_name + + let lv_path = $"/dev/($vg_name)/test_lv" + + # Create filesystem and mount + mkfs.ext4 -q $lv_path + mkdir $mountpoint + mount $lv_path $mountpoint + + # Create boot directory + mkdir $"($mountpoint)/boot" + + # Show block device hierarchy + lsblk --pairs --paths --inverse --output NAME,TYPE $lv_path + + run_install $mountpoint + + # Validate ESP installed on disk1 and disk3, disk2 has no ESP + validate_esp $"($loop1)p1" + validate_esp $"($loop3)p1" + } catch {|e| + cleanup $vg_name [$loop1, $loop2, $loop3] $mountpoint + rm -f $disk1 $disk2 $disk3 + error make {msg: $"Three devices partial ESP test failed: ($e)"} + } + + # Cleanup + cleanup $vg_name [$loop1, $loop2, $loop3] $mountpoint + rm -f $disk1 $disk2 $disk3 + + print "Three devices partial ESP test completed successfully" +} + +# Test scenario 4: Single device with ESP + root partition (no LVM) +def test_single_device_no_lvm [] { + print "Starting single device no LVM test" + + let mountpoint = "/var/mnt/test_no_lvm" + let disk1 = "/var/tmp/disk1_nolvm.img" + + let loop1 = (setup_disk_with_root $disk1 "10G") + + try { + # Mount root partition directly (no LVM) + mkdir $mountpoint + mount $"($loop1)p2" $mountpoint + + # Create boot directory + mkdir $"($mountpoint)/boot" + + # Show block device hierarchy + lsblk --pairs --paths --inverse --output NAME,TYPE $"($loop1)p2" + + run_install $mountpoint + + # Validate ESP was installed correctly + validate_esp $"($loop1)p1" + } catch {|e| + cleanup_simple $loop1 $mountpoint + rm -f $disk1 + error make {msg: $"Single device no LVM test failed: ($e)"} + } + + # Cleanup + cleanup_simple $loop1 $mountpoint + rm -f $disk1 + + print "Single device no LVM test completed successfully" +} + +# Test scenario 5: No ESP on any device (install should fail gracefully) +def test_no_esp_failure [] { + print "Starting no ESP failure test" + + let vg_name = "test_no_esp_vg" + let mountpoint = "/var/mnt/test_no_esp" + let disk1 = "/var/tmp/disk1_noesp.img" + let disk2 = "/var/tmp/disk2_noesp.img" + + # Setup disks - neither has ESP + let loop1 = (setup_disk_with_partitions $disk1 false) + let loop2 = (setup_disk_with_partitions $disk2 false) + + try { + # Create LVM spanning both devices + pvcreate $"($loop1)p1" $"($loop2)p1" + vgcreate $vg_name $"($loop1)p1" $"($loop2)p1" + lvcreate -l "100%FREE" -n test_lv $vg_name + + let lv_path = $"/dev/($vg_name)/test_lv" + + # Create filesystem and mount + mkfs.ext4 -q $lv_path + mkdir $mountpoint + mount $lv_path $mountpoint + + # Create boot directory + mkdir $"($mountpoint)/boot" + + # Show block device hierarchy + lsblk --pairs --paths --inverse --output NAME,TYPE $lv_path + + # Run install and expect it to fail + let result = (do { + run_install $mountpoint + } | complete) + + assert ($result.exit_code != 0) "Expected install to fail with no ESP partitions" + print $"Install failed as expected with exit code ($result.exit_code)" + } catch {|e| + cleanup $vg_name [$loop1, $loop2] $mountpoint + rm -f $disk1 $disk2 + error make {msg: $"No ESP failure test failed: ($e)"} + } + + # Cleanup + cleanup $vg_name [$loop1, $loop2] $mountpoint + rm -f $disk1 $disk2 + + print "No ESP failure test completed successfully" + tap ok +} + +def main [] { + # This test exercises bootupd-based bootloader installation which only + # supports GRUB today. Skip when the image uses systemd-boot. + if (tap is_composefs) { + let st = bootc status --json | from json + if ($st.status.booted.composefs.bootloader | str downcase) == "systemd" { + print "SKIP: multi-device ESP test not supported with systemd-boot" + tap ok + return + } + } + + # See https://tmt.readthedocs.io/en/stable/stories/features.html#reboot-during-test + match $env.TMT_REBOOT_COUNT? { + null | "0" => test_single_esp, + "1" => { test_dual_esp; test_three_devices_partial_esp; tmt-reboot }, + "2" => { test_single_device_no_lvm; test_no_esp_failure }, + $o => { error make { msg: $"Invalid TMT_REBOOT_COUNT ($o)" } }, + } +} diff --git a/tmt/tests/test-39-multi-device-esp.fmf b/tmt/tests/test-39-multi-device-esp.fmf new file mode 100644 index 000000000..415a2a537 --- /dev/null +++ b/tmt/tests/test-39-multi-device-esp.fmf @@ -0,0 +1,7 @@ +summary: Test multi-device ESP detection for to-existing-root +test: nu booted/test-multi-device-esp.nu +duration: 60m +require: + - lvm2 + - dosfstools + - e2fsprogs From 218184db5ec1fb69dcfe99f29a47d4977ae56e5b Mon Sep 17 00:00:00 2001 From: ckyrouac Date: Wed, 25 Mar 2026 15:41:13 -0400 Subject: [PATCH 3/9] composefs: Walk parent devices to find ESP partition The composefs BLS and UKI boot setup paths called find_partition_of_esp() directly on the device, which fails when the root filesystem is on an LVM logical volume (the ESP is on the parent disk, not the LV). The store module had the same issue via require_single_root() + find_partition_of_esp(). Switch all call sites to find_colocated_esps() which walks up to the physical disk(s) via find_all_roots() before searching for the ESP, consistent with what install_systemd_boot and mount_esp_part already do. Assisted-by: Claude Code (Opus 4) Signed-off-by: ckyrouac Signed-off-by: Colin Walters --- crates/lib/src/bootc_composefs/boot.rs | 59 +++++++++++++++++++++----- crates/lib/src/store/mod.rs | 16 +++++-- 2 files changed, 60 insertions(+), 15 deletions(-) diff --git a/crates/lib/src/bootc_composefs/boot.rs b/crates/lib/src/bootc_composefs/boot.rs index 6e4cc25ce..178390265 100644 --- a/crates/lib/src/bootc_composefs/boot.rs +++ b/crates/lib/src/bootc_composefs/boot.rs @@ -533,8 +533,18 @@ pub(crate) fn setup_composefs_bls_boot( ComposefsCmdline::build(&id_hex, state.composefs_options.allow_missing_verity); cmdline_options.extend(&Cmdline::from(&composefs_cmdline.to_string())); - // Locate ESP partition device - let esp_part = root_setup.device_info.find_partition_of_esp()?; + // Locate ESP partition device by walking up to the root disk(s) + let esp_part = root_setup + .device_info + .find_colocated_esps()? + .and_then(|mut v| { + if v.is_empty() { + None + } else { + Some(v.remove(0)) + } + }) + .ok_or_else(|| anyhow::anyhow!("ESP partition not found"))?; ( root_setup.physical_root_path.clone(), @@ -571,10 +581,18 @@ pub(crate) fn setup_composefs_bls_boot( .context("Failed to create 'composefs=' parameter")?; cmdline.add_or_modify(¶m); - // Locate ESP partition device - let root_dev = - bootc_blockdev::list_dev_by_dir(&storage.physical_root)?.require_single_root()?; - let esp_dev = root_dev.find_partition_of_esp()?; + // Locate ESP partition device by walking up to the root disk(s) + let root_dev = bootc_blockdev::list_dev_by_dir(&storage.physical_root)?; + let esp_dev = root_dev + .find_colocated_esps()? + .and_then(|mut v| { + if v.is_empty() { + None + } else { + Some(v.remove(0)) + } + }) + .ok_or_else(|| anyhow::anyhow!("ESP partition not found"))?; ( storage.physical_root_path.clone(), @@ -1117,7 +1135,18 @@ pub(crate) fn setup_composefs_uki_boot( BootSetupType::Setup((root_setup, state, postfetch, ..)) => { state.require_no_kargs_for_uki()?; - let esp_part = root_setup.device_info.find_partition_of_esp()?; + // Locate ESP partition device by walking up to the root disk(s) + let esp_part = root_setup + .device_info + .find_colocated_esps()? + .and_then(|mut v| { + if v.is_empty() { + None + } else { + Some(v.remove(0)) + } + }) + .ok_or_else(|| anyhow::anyhow!("ESP partition not found"))?; ( root_setup.physical_root_path.clone(), @@ -1132,10 +1161,18 @@ pub(crate) fn setup_composefs_uki_boot( let sysroot = Utf8PathBuf::from("/sysroot"); // Still needed for root_path let bootloader = host.require_composefs_booted()?.bootloader.clone(); - // Locate ESP partition device - let root_dev = - bootc_blockdev::list_dev_by_dir(&storage.physical_root)?.require_single_root()?; - let esp_dev = root_dev.find_partition_of_esp()?; + // Locate ESP partition device by walking up to the root disk(s) + let root_dev = bootc_blockdev::list_dev_by_dir(&storage.physical_root)?; + let esp_dev = root_dev + .find_colocated_esps()? + .and_then(|mut v| { + if v.is_empty() { + None + } else { + Some(v.remove(0)) + } + }) + .ok_or_else(|| anyhow::anyhow!("ESP partition not found"))?; ( sysroot, diff --git a/crates/lib/src/store/mod.rs b/crates/lib/src/store/mod.rs index 8fd09d826..ce715d59a 100644 --- a/crates/lib/src/store/mod.rs +++ b/crates/lib/src/store/mod.rs @@ -198,10 +198,18 @@ impl BootedStorage { } let composefs = Arc::new(composefs); - //TODO: this assumes a single ESP on the root device - let root_dev = - bootc_blockdev::list_dev_by_dir(&physical_root)?.require_single_root()?; - let esp_dev = root_dev.find_partition_of_esp()?; + // Locate ESP by walking up to the root disk(s) + let root_dev = bootc_blockdev::list_dev_by_dir(&physical_root)?; + let esp_dev = root_dev + .find_colocated_esps()? + .and_then(|mut v| { + if v.is_empty() { + None + } else { + Some(v.remove(0)) + } + }) + .ok_or_else(|| anyhow::anyhow!("ESP partition not found"))?; let esp_mount = mount_esp(&esp_dev.path())?; let boot_dir = match get_bootloader()? { From 1a1be806f039058bb0d35b1a6820907625ffc943 Mon Sep 17 00:00:00 2001 From: ckyrouac Date: Wed, 25 Mar 2026 20:14:02 -0400 Subject: [PATCH 4/9] tests: Use locally-built image in test-install-outside-container The test was using `get_target_image` which returns the upstream `docker://quay.io/centos-bootc/centos-bootc:stream10`. On composefs+grub variants provisioned with an updated bootupd from copr, the upstream image has stock bootupd with incompatible EFI update metadata, causing the install to fail with "Failed to find EFI update metadata". Switch to using `containers-storage:localhost/bootc` (the locally-built image), matching the pattern used by test-32, test-37, and test-38. The locally-built image has the updated bootupd with compatible metadata. Assisted-by: Claude Code (Opus 4) Signed-off-by: ckyrouac Signed-off-by: Colin Walters --- tmt/tests/booted/test-install-outside-container.nu | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tmt/tests/booted/test-install-outside-container.nu b/tmt/tests/booted/test-install-outside-container.nu index 45f66372c..d4eb0b6ac 100644 --- a/tmt/tests/booted/test-install-outside-container.nu +++ b/tmt/tests/booted/test-install-outside-container.nu @@ -6,9 +6,10 @@ use std assert use tap.nu -# Use an OS-matched target image to avoid version mismatches -# (e.g., XFS features created by newer mkfs.xfs not recognized by older grub2) -let target_image = (tap get_target_image) +# Use the locally-built image which has updated bootupd with compatible +# EFI update metadata, matching the pattern used by test-32/37/38. +bootc image copy-to-storage +let target_image = "containers-storage:localhost/bootc" # setup filesystem mkdir /var/mnt From 417a42fea1951a73a1ab152f3d20dc3fb3fc9d0e Mon Sep 17 00:00:00 2001 From: ckyrouac Date: Wed, 25 Mar 2026 20:53:46 -0400 Subject: [PATCH 5/9] tests: Fix install-outside-container for composefs+grub The initial change to use locally-built images had two additional issues on composefs: 1. containers-storage: transport fails on composefs's read-only root with "mkdir /.local: read-only file system". Fix by exporting the image to an OCI layout directory on writable /var/tmp instead. 2. run_install() was masking /sysroot/ostree and removing bootupd update metadata, which composefs needs for bootloader installation and boot binaries. Fix by making run_install() skip these ostree-specific workarounds on composefs systems. Note: the composefs install-outside-container code path still has a separate bug ("Shared boot binaries not found" in boot.rs:745) that needs fixing in the Rust code. Assisted-by: Claude Code (Opus 4) Signed-off-by: ckyrouac Signed-off-by: Colin Walters --- tmt/tests/booted/tap.nu | 13 +++++++++++-- tmt/tests/booted/test-install-outside-container.nu | 10 +++++----- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/tmt/tests/booted/tap.nu b/tmt/tests/booted/tap.nu index 686131079..34a5fc085 100644 --- a/tmt/tests/booted/tap.nu +++ b/tmt/tests/booted/tap.nu @@ -55,12 +55,21 @@ export def get_target_image [] { # Run a bootc install command in an isolated mount namespace. # This handles the common setup needed for install tests run outside a container. +# For ostree: masks off bootupd updates and /sysroot/ostree to reproduce +# https://github.com/bootc-dev/bootc/issues/1778 +# For composefs: only removes bound images (bootupd metadata and boot +# binaries under /sysroot/ostree are needed for installation). export def run_install [cmd: string] { + let is_cfs = (is_composefs) + let mask_cmds = if $is_cfs { + "true" + } else { + "if test -d /sysroot/ostree; then mount --bind /usr/share/empty /sysroot/ostree; fi\nrm -vrf /usr/lib/bootupd/updates" + } systemd-run -p MountFlags=slave -qdPG -- /bin/sh -c $" set -xeuo pipefail bootc usr-overlay -if test -d /sysroot/ostree; then mount --bind /usr/share/empty /sysroot/ostree; fi -rm -vrf /usr/lib/bootupd/updates +($mask_cmds) rm -vrf /usr/lib/bootc/bound-images.d ($cmd) " diff --git a/tmt/tests/booted/test-install-outside-container.nu b/tmt/tests/booted/test-install-outside-container.nu index d4eb0b6ac..8c88cf17a 100644 --- a/tmt/tests/booted/test-install-outside-container.nu +++ b/tmt/tests/booted/test-install-outside-container.nu @@ -7,9 +7,12 @@ use std assert use tap.nu # Use the locally-built image which has updated bootupd with compatible -# EFI update metadata, matching the pattern used by test-32/37/38. +# EFI update metadata. Export to OCI layout on a writable path since +# containers-storage: transport can't work when the root fs is read-only +# (composefs), and install-outside-container tests run directly on the host. bootc image copy-to-storage -let target_image = "containers-storage:localhost/bootc" +skopeo copy containers-storage:localhost/bootc oci:/var/tmp/bootc-oci +let target_image = "oci:/var/tmp/bootc-oci" # setup filesystem mkdir /var/mnt @@ -22,9 +25,6 @@ let result = bootc install to-filesystem /var/mnt e>| find "--source-imgref must assert not equal $result null umount /var/mnt -# Mask off the bootupd state to reproduce https://github.com/bootc-dev/bootc/issues/1778 -# Also it turns out that installation outside of containers dies due to `error: Multiple commit objects found` -# so we mask off /sysroot/ostree # And using systemd-run here breaks our install_t so we disable SELinux enforcement setenforce 0 From 8df28232f9194a5c5eacc46a56692b1a17298cf8 Mon Sep 17 00:00:00 2001 From: ckyrouac Date: Thu, 26 Mar 2026 13:33:55 +0000 Subject: [PATCH 6/9] tests: Skip multi-device ESP test on non-UEFI systems The multi-device ESP test creates ESP partitions and expects bootupd to install a UEFI bootloader. On BIOS-booted systems, bootupd instead tries to install GRUB for i386-pc, which requires a BIOS Boot Partition and fails. The test plan already requests UEFI provisioning via the hardware hint, but Testing Farm does not always honor this on CentOS Stream x86_64. Add a runtime check for /sys/firmware/efi so the test skips gracefully on BIOS hosts rather than failing. Assisted-by: Claude Code (Opus 4.6) Signed-off-by: ckyrouac Signed-off-by: Colin Walters --- tmt/tests/booted/test-multi-device-esp.nu | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tmt/tests/booted/test-multi-device-esp.nu b/tmt/tests/booted/test-multi-device-esp.nu index d75f430ad..95f18e2fd 100644 --- a/tmt/tests/booted/test-multi-device-esp.nu +++ b/tmt/tests/booted/test-multi-device-esp.nu @@ -420,6 +420,16 @@ def test_no_esp_failure [] { } def main [] { + # This test requires a UEFI-booted host because it creates ESP partitions + # and expects bootupd to install a UEFI bootloader. On BIOS systems, + # bootupd would try to install GRUB for i386-pc which needs a BIOS Boot + # Partition instead of an ESP. + if not ("/sys/firmware/efi" | path exists) { + print "SKIP: multi-device ESP test requires UEFI boot" + tap ok + return + } + # This test exercises bootupd-based bootloader installation which only # supports GRUB today. Skip when the image uses systemd-boot. if (tap is_composefs) { From 808e6966adfd4aacb7a059b4da2fe5620431bec8 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 1 Apr 2026 17:55:14 +0000 Subject: [PATCH 7/9] bwrap: Add set_default_path() helper for standard PATH Extract the repeated PATH environment variable string into a set_default_path() method on BwrapCmd. The bwrap environment may not have a complete PATH, causing tools like bootupctl or sfdisk to not be found. This consolidates the workaround into one place. Assisted-by: OpenCode (Claude Opus 4) Signed-off-by: Colin Walters --- crates/lib/src/bootloader.rs | 19 +++++-------------- crates/utils/src/bwrap.rs | 12 ++++++++++++ ...e-esp.fmf => test-40-multi-device-esp.fmf} | 0 3 files changed, 17 insertions(+), 14 deletions(-) rename tmt/tests/{test-39-multi-device-esp.fmf => test-40-multi-device-esp.fmf} (100%) diff --git a/crates/lib/src/bootloader.rs b/crates/lib/src/bootloader.rs index 841af4d10..6544d643e 100644 --- a/crates/lib/src/bootloader.rs +++ b/crates/lib/src/bootloader.rs @@ -1,7 +1,7 @@ use std::fs::create_dir_all; use std::process::Command; -use anyhow::{Context, Result, anyhow, bail}; +use anyhow::{anyhow, bail, Context, Result}; use bootc_utils::{BwrapCmd, CommandRunExt}; use camino::Utf8Path; use cap_std_ext::cap_std::fs::Dir; @@ -10,7 +10,7 @@ use fn_error_context::context; use bootc_mount as mount; -use crate::bootc_composefs::boot::{SecurebootKeys, mount_esp}; +use crate::bootc_composefs::boot::{mount_esp, SecurebootKeys}; use crate::{discoverable_partition_specification, utils}; /// The name of the mountpoint for efi (as a subdirectory of /boot, or at the toplevel) @@ -86,10 +86,7 @@ fn bootupd_supports_filesystem(rootfs: &Utf8Path, deployment_path: Option<&str>) let output = if let Some(deploy) = deployment_path { let target_root = rootfs.join(deploy); BwrapCmd::new(&target_root) - .setenv( - "PATH", - "/bin:/usr/bin:/sbin:/usr/sbin:/usr/local/bin:/usr/local/sbin", - ) + .set_default_path() .run_get_string(help_args)? } else { Command::new("bootupctl") @@ -206,14 +203,8 @@ pub(crate) fn install_via_bootupd( } // The $PATH in the bwrap env is not complete enough for some images - // so we inject a reasonnable default. - // This is causing bootupctl and/or sfdisk binaries - // to be not found with fedora 43. - cmd.setenv( - "PATH", - "/bin:/usr/bin:/sbin:/usr/sbin:/usr/local/bin:/usr/local/sbin", - ) - .run(bwrap_args) + // so we inject a reasonable default. + cmd.set_default_path().run(bwrap_args) } else { // Running directly without chroot Command::new("bootupctl") diff --git a/crates/utils/src/bwrap.rs b/crates/utils/src/bwrap.rs index 3d3069ead..1f0d0a07c 100644 --- a/crates/utils/src/bwrap.rs +++ b/crates/utils/src/bwrap.rs @@ -59,6 +59,18 @@ impl<'a> BwrapCmd<'a> { self } + /// Set $PATH to a reasonable default for finding system binaries. + /// + /// The bwrap environment may not have a complete $PATH, causing + /// tools like bootupctl or sfdisk to not be found. This sets a + /// default that covers the standard binary directories. + pub fn set_default_path(self) -> Self { + self.setenv( + "PATH", + "/bin:/usr/bin:/sbin:/usr/sbin:/usr/local/bin:/usr/local/sbin", + ) + } + /// Build the bwrap `Command` with all bind mounts, env vars, and args. fn build_command>(&self, args: impl IntoIterator) -> Command { let mut cmd = Command::new("bwrap"); diff --git a/tmt/tests/test-39-multi-device-esp.fmf b/tmt/tests/test-40-multi-device-esp.fmf similarity index 100% rename from tmt/tests/test-39-multi-device-esp.fmf rename to tmt/tests/test-40-multi-device-esp.fmf From 9d5f8d7ea91fe9825ec24ead866b5940539105cb Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 1 Apr 2026 19:23:02 +0000 Subject: [PATCH 8/9] blockdev: Clean up ESP discovery code Several improvements to ESP partition discovery: Add find_partition_of_esp_optional() returning Result> to cleanly separate three outcomes: found, absent, and genuinely unexpected errors (like unsupported partition table types). The existing find_partition_of_esp() is now a thin wrapper that converts None to Err. Add find_first_colocated_esp() helper to replace a 10-line pattern that was repeated verbatim 5 times across boot.rs and store/mod.rs. Deduplicate roots in find_all_roots() using a seen-set: in complex topologies like multipath, multiple parent branches can converge on the same physical disk. find_colocated_esps() now uses the optional variant to properly propagate real errors while treating absence normally. Also extract the match-on-if-else in setup_composefs_bls_boot into a let binding for readability. Assisted-by: OpenCode (Claude Opus 4) Signed-off-by: Colin Walters --- crates/blockdev/src/blockdev.rs | 83 +++++++++++++++++--------- crates/lib/src/bootc_composefs/boot.rs | 46 ++------------ crates/lib/src/bootloader.rs | 20 +++++-- crates/lib/src/store/mod.rs | 11 +--- 4 files changed, 73 insertions(+), 87 deletions(-) diff --git a/crates/blockdev/src/blockdev.rs b/crates/blockdev/src/blockdev.rs index f544aa0be..6fd46af88 100644 --- a/crates/blockdev/src/blockdev.rs +++ b/crates/blockdev/src/blockdev.rs @@ -1,3 +1,4 @@ +use std::collections::HashSet; use std::env; use std::path::Path; use std::process::{Command, Stdio}; @@ -123,15 +124,26 @@ impl Device { /// Calls find_all_roots() to discover physical disks, then searches each for an ESP. /// Returns None if no ESPs are found. pub fn find_colocated_esps(&self) -> Result>> { - let esps: Vec<_> = self - .find_all_roots()? - .iter() - .flat_map(|root| root.find_partition_of_esp().ok()) - .cloned() - .collect(); + let mut esps = Vec::new(); + for root in &self.find_all_roots()? { + if let Some(esp) = root.find_partition_of_esp_optional()? { + esps.push(esp.clone()); + } + } Ok((!esps.is_empty()).then_some(esps)) } + /// Find a single ESP partition among all root devices backing this device. + /// + /// Walks the parent chain to find all backing disks, then looks for ESP + /// partitions on each. Returns the first ESP found. This is the common + /// case for composefs/UKI boot paths where exactly one ESP is expected. + pub fn find_first_colocated_esp(&self) -> Result { + self.find_colocated_esps()? + .and_then(|mut v| Some(v.remove(0))) + .ok_or_else(|| anyhow!("No ESP partition found among backing devices")) + } + /// Find all BIOS boot partitions across all root devices backing this device. /// Calls find_all_roots() to discover physical disks, then searches each for a BIOS boot partition. /// Returns None if no BIOS boot partitions are found. @@ -159,34 +171,41 @@ impl Device { /// /// For GPT disks, this matches by the ESP partition type GUID. /// For MBR (dos) disks, this matches by the MBR partition type IDs (0x06 or 0xEF). - pub fn find_partition_of_esp(&self) -> Result<&Device> { - let children = self - .children - .as_ref() - .ok_or_else(|| anyhow!("Device has no children"))?; + /// + /// Returns `Ok(None)` when there are no children or no ESP partition + /// is present. Returns `Err` only for genuinely unexpected conditions + /// (e.g. an unsupported partition table type). + pub fn find_partition_of_esp_optional(&self) -> Result> { + let Some(children) = self.children.as_ref() else { + return Ok(None); + }; match self.pttype.as_deref() { - Some("dos") => children - .iter() - .find(|child| { - child - .parttype - .as_ref() - .and_then(|pt| { - let pt = pt.strip_prefix("0x").unwrap_or(pt); - u8::from_str_radix(pt, 16).ok() - }) - .is_some_and(|pt| ESP_ID_MBR.contains(&pt)) - }) - .ok_or_else(|| anyhow!("ESP not found in MBR partition table")), + Some("dos") => Ok(children.iter().find(|child| { + child + .parttype + .as_ref() + .and_then(|pt| { + let pt = pt.strip_prefix("0x").unwrap_or(pt); + u8::from_str_radix(pt, 16).ok() + }) + .is_some_and(|pt| ESP_ID_MBR.contains(&pt)) + })), // When pttype is None (e.g. older lsblk or partition devices), default // to GPT UUID matching which will simply not match MBR hex types. - Some("gpt") | None => self - .find_partition_of_type(ESP) - .ok_or_else(|| anyhow!("ESP not found in GPT partition table")), + Some("gpt") | None => Ok(self.find_partition_of_type(ESP)), Some(other) => Err(anyhow!("Unsupported partition table type: {other}")), } } + /// Find the EFI System Partition (ESP) among children, or error if absent. + /// + /// This is a convenience wrapper around [`find_partition_of_esp_optional`] + /// for callers that require an ESP to be present. + pub fn find_partition_of_esp(&self) -> Result<&Device> { + self.find_partition_of_esp_optional()? + .ok_or_else(|| anyhow!("ESP partition not found on {}", self.path())) + } + /// Find a child partition by partition number (1-indexed). pub fn find_device_by_partno(&self, partno: u32) -> Result<&Device> { self.children @@ -308,6 +327,7 @@ impl Device { }; let mut roots = Vec::new(); + let mut seen = HashSet::new(); let mut queue = parents; while let Some(mut device) = queue.pop() { match device.children.take() { @@ -315,8 +335,13 @@ impl Device { queue.extend(grandparents); } _ => { - // Found a root; re-query to populate its actual children - roots.push(list_dev(Utf8Path::new(&device.path()))?); + // Deduplicate: in complex topologies (e.g. multipath) + // multiple branches can converge on the same physical disk. + let name = device.name.clone(); + if seen.insert(name) { + // Found a new root; re-query to populate its actual children + roots.push(list_dev(Utf8Path::new(&device.path()))?); + } } } } diff --git a/crates/lib/src/bootc_composefs/boot.rs b/crates/lib/src/bootc_composefs/boot.rs index 178390265..9d87cfbc0 100644 --- a/crates/lib/src/bootc_composefs/boot.rs +++ b/crates/lib/src/bootc_composefs/boot.rs @@ -534,17 +534,7 @@ pub(crate) fn setup_composefs_bls_boot( cmdline_options.extend(&Cmdline::from(&composefs_cmdline.to_string())); // Locate ESP partition device by walking up to the root disk(s) - let esp_part = root_setup - .device_info - .find_colocated_esps()? - .and_then(|mut v| { - if v.is_empty() { - None - } else { - Some(v.remove(0)) - } - }) - .ok_or_else(|| anyhow::anyhow!("ESP partition not found"))?; + let esp_part = root_setup.device_info.find_first_colocated_esp()?; ( root_setup.physical_root_path.clone(), @@ -583,16 +573,7 @@ pub(crate) fn setup_composefs_bls_boot( // Locate ESP partition device by walking up to the root disk(s) let root_dev = bootc_blockdev::list_dev_by_dir(&storage.physical_root)?; - let esp_dev = root_dev - .find_colocated_esps()? - .and_then(|mut v| { - if v.is_empty() { - None - } else { - Some(v.remove(0)) - } - }) - .ok_or_else(|| anyhow::anyhow!("ESP partition not found"))?; + let esp_dev = root_dev.find_first_colocated_esp()?; ( storage.physical_root_path.clone(), @@ -1136,17 +1117,7 @@ pub(crate) fn setup_composefs_uki_boot( state.require_no_kargs_for_uki()?; // Locate ESP partition device by walking up to the root disk(s) - let esp_part = root_setup - .device_info - .find_colocated_esps()? - .and_then(|mut v| { - if v.is_empty() { - None - } else { - Some(v.remove(0)) - } - }) - .ok_or_else(|| anyhow::anyhow!("ESP partition not found"))?; + let esp_part = root_setup.device_info.find_first_colocated_esp()?; ( root_setup.physical_root_path.clone(), @@ -1163,16 +1134,7 @@ pub(crate) fn setup_composefs_uki_boot( // Locate ESP partition device by walking up to the root disk(s) let root_dev = bootc_blockdev::list_dev_by_dir(&storage.physical_root)?; - let esp_dev = root_dev - .find_colocated_esps()? - .and_then(|mut v| { - if v.is_empty() { - None - } else { - Some(v.remove(0)) - } - }) - .ok_or_else(|| anyhow::anyhow!("ESP partition not found"))?; + let esp_dev = root_dev.find_first_colocated_esp()?; ( sysroot, diff --git a/crates/lib/src/bootloader.rs b/crates/lib/src/bootloader.rs index 6544d643e..a384b1e1a 100644 --- a/crates/lib/src/bootloader.rs +++ b/crates/lib/src/bootloader.rs @@ -11,7 +11,7 @@ use fn_error_context::context; use bootc_mount as mount; use crate::bootc_composefs::boot::{mount_esp, SecurebootKeys}; -use crate::{discoverable_partition_specification, utils}; +use crate::utils; /// The name of the mountpoint for efi (as a subdirectory of /boot, or at the toplevel) pub(crate) const EFI_DIR: &str = "efi"; @@ -53,13 +53,17 @@ pub(crate) fn mount_esp_part(root: &Dir, root_path: &Utf8Path, is_ostree: bool) let roots = bootc_blockdev::list_dev_by_dir(physical_root)?.find_all_roots()?; for dev in &roots { - if let Some(esp_dev) = dev.find_partition_of_type(bootc_blockdev::ESP) { + if let Some(esp_dev) = dev.find_partition_of_esp_optional()? { let esp_path = esp_dev.path(); bootc_mount::mount(&esp_path, &root_path.join(&efi_path))?; tracing::debug!("Mounted {esp_path} at /boot/efi"); return Ok(()); } } + tracing::debug!( + "No ESP partition found among {} root device(s)", + roots.len() + ); Ok(()) } @@ -228,10 +232,14 @@ pub(crate) fn install_systemd_boot( autoenroll: Option, ) -> Result<()> { let roots = device.find_all_roots()?; - let esp_part = roots - .iter() - .find_map(|root| root.find_partition_of_type(discoverable_partition_specification::ESP)) - .ok_or_else(|| anyhow::anyhow!("ESP partition not found"))?; + let mut esp_part = None; + for root in &roots { + if let Some(esp) = root.find_partition_of_esp_optional()? { + esp_part = Some(esp); + break; + } + } + let esp_part = esp_part.ok_or_else(|| anyhow::anyhow!("ESP partition not found"))?; let esp_mount = mount_esp(&esp_part.path()).context("Mounting ESP")?; let esp_path = Utf8Path::from_path(esp_mount.dir.path()) diff --git a/crates/lib/src/store/mod.rs b/crates/lib/src/store/mod.rs index ce715d59a..4f0cf4190 100644 --- a/crates/lib/src/store/mod.rs +++ b/crates/lib/src/store/mod.rs @@ -200,16 +200,7 @@ impl BootedStorage { // Locate ESP by walking up to the root disk(s) let root_dev = bootc_blockdev::list_dev_by_dir(&physical_root)?; - let esp_dev = root_dev - .find_colocated_esps()? - .and_then(|mut v| { - if v.is_empty() { - None - } else { - Some(v.remove(0)) - } - }) - .ok_or_else(|| anyhow::anyhow!("ESP partition not found"))?; + let esp_dev = root_dev.find_first_colocated_esp()?; let esp_mount = mount_esp(&esp_dev.path())?; let boot_dir = match get_bootloader()? { From 52602b4ab44430586034952cfe8ca19bf7d013f4 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 1 Apr 2026 21:21:35 +0000 Subject: [PATCH 9/9] tests: Validate ESP-related error in no-ESP failure test The no-ESP test only checked for a non-zero exit code, which would also pass if podman itself failed for unrelated reasons. Check that the output contains "ESP" to confirm the right failure mode. Assisted-by: OpenCode (Claude Opus 4) Signed-off-by: Colin Walters --- tmt/tests/booted/test-multi-device-esp.nu | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tmt/tests/booted/test-multi-device-esp.nu b/tmt/tests/booted/test-multi-device-esp.nu index 95f18e2fd..b3f69fcf3 100644 --- a/tmt/tests/booted/test-multi-device-esp.nu +++ b/tmt/tests/booted/test-multi-device-esp.nu @@ -404,6 +404,9 @@ def test_no_esp_failure [] { } | complete) assert ($result.exit_code != 0) "Expected install to fail with no ESP partitions" + # Verify the failure is ESP-related, not an unrelated podman/runtime error + let combined = $"($result.stdout)\n($result.stderr)" + assert ($combined | str contains "ESP") $"Expected ESP-related error message, got: ($combined | str substring 0..200)" print $"Install failed as expected with exit code ($result.exit_code)" } catch {|e| cleanup $vg_name [$loop1, $loop2] $mountpoint