lib/install: Use mounted ESP for install to-filesystem by Guara92 · Pull Request #1953 · bootc-dev/bootc

Guara92 · 2026-01-25T16:41:03Z

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)

Fixes: #1929

gemini-code-assist

Code Review

This pull request refactors the handling of the EFI System Partition (ESP) across the codebase. Key changes include introducing a new open_target_root function to abstract opening the target root directory and a require_boot_efi_mount function to validate and retrieve the mounted ESP's source. The require_boot_efi_mount function replaces previous logic for finding and mounting the ESP, ensuring /boot/efi is a valid, mounted vfat/fat filesystem. Consequently, functions like setup_composefs_bls_boot, setup_composefs_uki_boot, install_via_bootupd, install_systemd_boot, and clean_boot_directories are updated to utilize these new helper functions and pass Dir objects for root access. A review comment specifically points out that several parameters (_device, _rootfs, _configopts, _deployment_path) in the install_systemd_boot function are now unused and should be removed for clarity, along with updating their call sites.

gemini-code-assist · 2026-01-25T16:44:21Z

crates/lib/src/bootloader.rs

+    _device: &PartitionTable,
    _rootfs: &Utf8Path,
    _configopts: &crate::install::InstallConfigOpts,
    _deployment_path: Option<&str>,


These parameters (_device, _rootfs, _configopts, _deployment_path) are no longer used in this function. To improve code clarity and maintainability, they should be removed from the function signature. The call site in crates/lib/src/bootc_composefs/boot.rs will also need to be updated.

cgwalters · 2026-01-26T19:08:19Z

Thanks for this! I haven't tried reviewing the code yet, but there's also #1911 which touches things in this space. I also saw #1929 which needs some analysis and may be related too.

cgwalters

Thank you so much for your contribution!

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.

Can you explain in what circumstances this happens?

cgwalters · 2026-02-03T23:27:35Z

crates/lib/src/bootc_composefs/boot.rs

    )
 }

+fn open_target_root(root_setup: &RootSetup) -> Result<Dir> {


Slightly cleaner as a method on impl RootSetup I'd say

moved 79b8874 thanks

cgwalters · 2026-02-03T23:28:30Z

crates/lib/src/install/baseline.rs

        boot,
        kargs,
        skip_finalize: false,
+        require_esp_mount: true,


Hmm...I would say arguably to-disk should not mount the ESP and we should ensure that it's autodiscovered properly.

Very reasonable, thanks for the feedback, implemented 8d870fc

cgwalters · 2026-02-03T23:29:37Z

crates/lib/src/bootloader.rs

-    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 {


"normalize" here could use a definition.

Also if we're trying to micro-optimize this it can return a Cow<str>

Cow used and added some rustdoc on the function f3def47, is it clear enough now?

cgwalters · 2026-02-03T23:30:35Z

crates/lib/src/bootloader.rs

-        return Ok(());
-    };
+    if !esp_fd.is_mountpoint(".")?.unwrap_or(false) {
+        anyhow::bail!("/boot/efi is not a mountpoint. This is required for installation.");


I don't want to make /boot/efi strictly hard required - among other things the DPS suggests mounting at /efi which makes sense to me and we should handle.

Thanks for pointing me to the spec, those low-level stuff are kinda new to me, I implemented a lookup by priority based on DPS f3def47.
Am I still missing something?

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 <guarascio.daniele@gmail.com>

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 <guarascio.daniele@gmail.com>

Guara92 · 2026-02-08T12:06:39Z

Thank you so much for your contribution!

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.

Can you explain in what circumstances this happens?

Basically whenever the active mount (/boot/efi) disagreed with the first physical candidate on the disk.

Some of the circumstances where the old "scan-the-disk" logic would fail:

Multi-Boot: If a disk has multiple partitions flagged as an EFI System Partition (ESP), the old logic would always pick the first one it found in the partition table.
Cross-Disk Configurations: The old logic assumed the ESP must reside on the same physical disk as the root filesystem (/), i.e. if you want to fully use your fastest drive for your root filesystem (/) saving space moving your bootloader and ESP (/boot/efi) to an older drive.
LVM or Software RAID configuration: Your root is on a logical volume, the previous logic might fail to find the "parent disk" might return the wrong device or fail.

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<str>' 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 <guarascio.daniele@gmail.com>

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 <guarascio.daniele@gmail.com>

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 <guarascio.daniele@gmail.com>

github-actions bot added the area/install Issues related to `bootc install` label Jan 25, 2026

bootc-bot bot requested a review from jeckersb January 25, 2026 16:41

gemini-code-assist bot reviewed Jan 25, 2026

View reviewed changes

Guara92 force-pushed the fix-esp-selection branch from 22641b1 to 4ed589c Compare January 25, 2026 16:54

cgwalters reviewed Feb 3, 2026

View reviewed changes

Guara92 added 2 commits February 8, 2026 12:52

Guara92 force-pushed the fix-esp-selection branch from bae675d to 67361c4 Compare February 8, 2026 11:52

Guara92 force-pushed the fix-esp-selection branch from 9ebefc1 to 8d870fc Compare February 8, 2026 12:32

Guara92 added 2 commits February 8, 2026 13:40

lib: Refactor open_target_root as method on RootSetup

79b8874

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 <guarascio.daniele@gmail.com>

Guara92 requested a review from cgwalters February 8, 2026 14:11

Conversation

Guara92 commented Jan 25, 2026

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

cgwalters commented Jan 26, 2026

Uh oh!

cgwalters left a comment

Choose a reason for hiding this comment

Uh oh!

cgwalters Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Guara92 Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

cgwalters Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Guara92 Feb 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cgwalters Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Guara92 Feb 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cgwalters Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Guara92 Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Guara92 commented Feb 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Guara92 Feb 8, 2026 •

edited

Loading

Guara92 Feb 8, 2026 •

edited

Loading