Skip to content

Commit 4ed589c

Browse files
committed
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 <guarascio.daniele@gmail.com>
1 parent e499b77 commit 4ed589c

File tree

3 files changed

+109
-50
lines changed

3 files changed

+109
-50
lines changed

crates/lib/src/bootc_composefs/boot.rs

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -106,10 +106,8 @@ use crate::{
106106
};
107107
use crate::{
108108
bootc_composefs::state::{get_booted_bls, write_composefs_state},
109-
bootloader::esp_in,
110-
};
111-
use crate::{
112-
bootc_composefs::status::get_container_manifest_and_config, bootc_kargs::compute_new_kargs,
109+
bootc_composefs::status::get_container_manifest_and_config,
110+
bootc_kargs::compute_new_kargs,
113111
};
114112
use crate::{bootc_composefs::status::get_sorted_grub_uki_boot_entries, install::PostFetchState};
115113
use crate::{
@@ -214,6 +212,17 @@ fi
214212
)
215213
}
216214

215+
fn open_target_root(root_setup: &RootSetup) -> Result<Dir> {
216+
if let Some(target_root) = root_setup.target_root_path.as_ref() {
217+
Dir::open_ambient_dir(target_root, ambient_authority()).context("Opening target root path")
218+
} else {
219+
root_setup
220+
.physical_root
221+
.try_clone()
222+
.context("Cloning target root handle")
223+
}
224+
}
225+
217226
pub fn get_esp_partition(device: &str) -> Result<(String, Option<String>)> {
218227
let device_info = bootc_blockdev::partitions_of(Utf8Path::new(device))?;
219228
let esp = crate::bootloader::esp_in(&device_info)?;
@@ -521,11 +530,12 @@ pub(crate) fn setup_composefs_bls_boot(
521530
cmdline_options.extend(&Cmdline::from(&composefs_cmdline));
522531

523532
// Locate ESP partition device
524-
let esp_part = esp_in(&root_setup.device_info)?;
533+
let esp_root = open_target_root(root_setup)?;
534+
let esp_part = crate::bootloader::require_boot_efi_mount(&esp_root)?;
525535

526536
(
527537
root_setup.physical_root_path.clone(),
528-
esp_part.node.clone(),
538+
esp_part,
529539
cmdline_options,
530540
fs,
531541
postfetch.detected_bootloader.clone(),
@@ -1063,11 +1073,12 @@ pub(crate) fn setup_composefs_uki_boot(
10631073
BootSetupType::Setup((root_setup, state, postfetch, ..)) => {
10641074
state.require_no_kargs_for_uki()?;
10651075

1066-
let esp_part = esp_in(&root_setup.device_info)?;
1076+
let esp_root = open_target_root(root_setup)?;
1077+
let esp_part = crate::bootloader::require_boot_efi_mount(&esp_root)?;
10671078

10681079
(
10691080
root_setup.physical_root_path.clone(),
1070-
esp_part.node.clone(),
1081+
esp_part,
10711082
postfetch.detected_bootloader.clone(),
10721083
state.composefs_options.insecure,
10731084
state.composefs_options.uki_addon.as_ref(),
@@ -1231,18 +1242,22 @@ pub(crate) async fn setup_composefs_boot(
12311242
.or(root_setup.rootfs_uuid.as_deref())
12321243
.ok_or_else(|| anyhow!("No uuid for boot/root"))?;
12331244

1245+
let target_root = open_target_root(root_setup)?;
1246+
12341247
if cfg!(target_arch = "s390x") {
12351248
// TODO: Integrate s390x support into install_via_bootupd
12361249
crate::bootloader::install_via_zipl(&root_setup.device_info, boot_uuid)?;
12371250
} else if postfetch.detected_bootloader == Bootloader::Grub {
12381251
crate::bootloader::install_via_bootupd(
12391252
&root_setup.device_info,
1253+
&target_root,
12401254
&root_setup.physical_root_path,
12411255
&state.config_opts,
12421256
None,
12431257
)?;
12441258
} else {
12451259
crate::bootloader::install_systemd_boot(
1260+
&target_root,
12461261
&root_setup.device_info,
12471262
&root_setup.physical_root_path,
12481263
&state.config_opts,
@@ -1406,4 +1421,11 @@ mod tests {
14061421
"RHEL should sort before Fedora in descending order"
14071422
);
14081423
}
1424+
1425+
#[test]
1426+
fn test_efi_uuid_source_formatting() {
1427+
let source = get_efi_uuid_source();
1428+
assert!(source.contains("${config_directory}/"));
1429+
assert!(source.contains(EFI_UUID_FILE));
1430+
}
14091431
}

crates/lib/src/bootloader.rs

Lines changed: 66 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use fn_error_context::context;
1111
use bootc_blockdev::{Partition, PartitionTable};
1212
use bootc_mount as mount;
1313

14-
use crate::bootc_composefs::boot::{SecurebootKeys, get_sysroot_parent_dev, mount_esp};
14+
use crate::bootc_composefs::boot::{SecurebootKeys, mount_esp};
1515
use crate::{discoverable_partition_specification, utils};
1616

1717
/// 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> {
3131
.ok_or(anyhow::anyhow!("ESP not found in partition table"))
3232
}
3333

34-
/// Get esp partition node based on the root dir
35-
pub(crate) fn get_esp_partition_node(root: &Dir) -> Result<Option<String>> {
36-
let device = get_sysroot_parent_dev(&root)?;
37-
let base_partitions = bootc_blockdev::partitions_of(Utf8Path::new(&device))?;
38-
let esp = base_partitions.find_partition_of_esp()?;
39-
Ok(esp.map(|v| v.node.clone()))
34+
fn normalize_esp_source(source: String) -> String {
35+
if let Some(uuid) = source.strip_prefix("UUID=") {
36+
format!("/dev/disk/by-uuid/{uuid}")
37+
} else if let Some(label) = source.strip_prefix("LABEL=") {
38+
format!("/dev/disk/by-label/{label}")
39+
} else {
40+
source
41+
}
4042
}
4143

42-
/// Mount ESP part at /boot/efi
43-
pub(crate) fn mount_esp_part(root: &Dir, root_path: &Utf8Path, is_ostree: bool) -> Result<()> {
44-
let efi_path = Utf8Path::new("boot").join(crate::bootloader::EFI_DIR);
45-
let Some(esp_fd) = root
44+
pub(crate) fn require_boot_efi_mount(root: &Dir) -> Result<String> {
45+
let efi_path = Utf8Path::new("boot").join(EFI_DIR);
46+
let esp_fd = root
4647
.open_dir_optional(&efi_path)
4748
.context("Opening /boot/efi")?
48-
else {
49-
return Ok(());
50-
};
49+
.ok_or_else(|| anyhow::anyhow!("/boot/efi does not exist"))?;
5150

52-
let Some(false) = esp_fd.is_mountpoint(".")? else {
53-
return Ok(());
54-
};
51+
if !esp_fd.is_mountpoint(".")?.unwrap_or(false) {
52+
anyhow::bail!("/boot/efi is not a mountpoint. This is required for installation.");
53+
}
5554

56-
tracing::debug!("Not a mountpoint: /boot/efi");
57-
// On ostree env with enabled composefs, should be /target/sysroot
58-
let physical_root = if is_ostree {
59-
&root.open_dir("sysroot").context("Opening /sysroot")?
60-
} else {
61-
root
62-
};
63-
if let Some(esp_part) = get_esp_partition_node(physical_root)? {
64-
bootc_mount::mount(&esp_part, &root_path.join(&efi_path))?;
65-
tracing::debug!("Mounted {esp_part} at /boot/efi");
55+
let fs = bootc_mount::inspect_filesystem_of_dir(&esp_fd)?;
56+
if fs.fstype != "vfat" && fs.fstype != "fat" {
57+
anyhow::bail!(
58+
"/boot/efi is mounted but is not vfat/fat (found {}). This is required for installation.",
59+
fs.fstype
60+
);
6661
}
67-
Ok(())
62+
63+
let source = normalize_esp_source(fs.source);
64+
65+
let source_path = Utf8Path::new(&source);
66+
if !source_path.try_exists()? {
67+
anyhow::bail!(
68+
"/boot/efi source device does not exist: {source}. This is required for installation."
69+
);
70+
}
71+
72+
Ok(source)
6873
}
6974

7075
/// 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<bool> {
8388
#[context("Installing bootloader")]
8489
pub(crate) fn install_via_bootupd(
8590
device: &PartitionTable,
91+
root: &Dir,
8692
rootfs: &Utf8Path,
8793
configopts: &crate::install::InstallConfigOpts,
8894
deployment_path: Option<&str>,
8995
) -> Result<()> {
96+
// We require /boot/efi to be mounted; finding the device is just a sanity check that it is.
97+
// The device argument is currently used by bootupctl as a fallback if it can't find the ESP.
98+
// But we want to fail fast if our own check fails.
99+
let _esp_device = require_boot_efi_mount(root)?;
100+
90101
let verbose = std::env::var_os("BOOTC_BOOTLOADER_DEBUG").map(|_| "-vvvv");
91102
// bootc defaults to only targeting the platform boot method.
92103
let bootupd_opts = (!configopts.generic_image).then_some(["--update-firmware", "--auto"]);
@@ -111,17 +122,16 @@ pub(crate) fn install_via_bootupd(
111122

112123
#[context("Installing bootloader")]
113124
pub(crate) fn install_systemd_boot(
114-
device: &PartitionTable,
125+
root: &Dir,
126+
_device: &PartitionTable,
115127
_rootfs: &Utf8Path,
116128
_configopts: &crate::install::InstallConfigOpts,
117129
_deployment_path: Option<&str>,
118130
autoenroll: Option<SecurebootKeys>,
119131
) -> Result<()> {
120-
let esp_part = device
121-
.find_partition_of_type(discoverable_partition_specification::ESP)
122-
.ok_or_else(|| anyhow::anyhow!("ESP partition not found"))?;
132+
let esp_device = require_boot_efi_mount(root)?;
123133

124-
let esp_mount = mount_esp(&esp_part.node).context("Mounting ESP")?;
134+
let esp_mount = mount_esp(&esp_device).context("Mounting ESP")?;
125135
let esp_path = Utf8Path::from_path(esp_mount.dir.path())
126136
.ok_or_else(|| anyhow::anyhow!("Failed to convert ESP mount path to UTF-8"))?;
127137

@@ -237,3 +247,26 @@ pub(crate) fn install_via_zipl(device: &PartitionTable, boot_uuid: &str) -> Resu
237247
.log_debug()
238248
.run_inherited_with_cmd_context()
239249
}
250+
251+
#[cfg(test)]
252+
mod tests {
253+
use super::*;
254+
255+
#[test]
256+
fn normalize_esp_source_uuid() {
257+
let normalized = normalize_esp_source("UUID=abcd-1234".to_string());
258+
assert_eq!(normalized, "/dev/disk/by-uuid/abcd-1234");
259+
}
260+
261+
#[test]
262+
fn normalize_esp_source_label() {
263+
let normalized = normalize_esp_source("LABEL=EFI".to_string());
264+
assert_eq!(normalized, "/dev/disk/by-label/EFI");
265+
}
266+
267+
#[test]
268+
fn normalize_esp_source_passthrough() {
269+
let normalized = normalize_esp_source("/dev/sda1".to_string());
270+
assert_eq!(normalized, "/dev/sda1");
271+
}
272+
}

crates/lib/src/install.rs

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1606,6 +1606,13 @@ async fn install_with_sysroot(
16061606
.context("Opening deployment dir")?;
16071607
let postfetch = PostFetchState::new(state, &deployment_dir)?;
16081608

1609+
let target_root_path = rootfs
1610+
.target_root_path
1611+
.clone()
1612+
.unwrap_or(rootfs.physical_root_path.clone());
1613+
let target_root = Dir::open_ambient_dir(&target_root_path, cap_std::ambient_authority())
1614+
.with_context(|| format!("Opening target root directory {target_root_path}"))?;
1615+
16091616
if cfg!(target_arch = "s390x") {
16101617
// TODO: Integrate s390x support into install_via_bootupd
16111618
crate::bootloader::install_via_zipl(&rootfs.device_info, boot_uuid)?;
@@ -1614,10 +1621,8 @@ async fn install_with_sysroot(
16141621
Bootloader::Grub => {
16151622
crate::bootloader::install_via_bootupd(
16161623
&rootfs.device_info,
1617-
&rootfs
1618-
.target_root_path
1619-
.clone()
1620-
.unwrap_or(rootfs.physical_root_path.clone()),
1624+
&target_root,
1625+
&target_root_path,
16211626
&state.config_opts,
16221627
Some(&deployment_path.as_str()),
16231628
)?;
@@ -2007,14 +2012,13 @@ fn remove_all_except_loader_dirs(bootdir: &Dir, is_ostree: bool) -> Result<()> {
20072012
}
20082013

20092014
#[context("Removing boot directory content")]
2010-
fn clean_boot_directories(rootfs: &Dir, rootfs_path: &Utf8Path, is_ostree: bool) -> Result<()> {
2015+
fn clean_boot_directories(rootfs: &Dir, is_ostree: bool) -> Result<()> {
20112016
let bootdir =
20122017
crate::utils::open_dir_remount_rw(rootfs, BOOT.into()).context("Opening /boot")?;
20132018

20142019
if ARCH_USES_EFI {
2015-
// On booted FCOS, esp is not mounted by default
2016-
// Mount ESP part at /boot/efi before clean
2017-
crate::bootloader::mount_esp_part(&rootfs, &rootfs_path, is_ostree)?;
2020+
// Require an explicit /boot/efi mount to avoid cleaning the wrong ESP.
2021+
crate::bootloader::require_boot_efi_mount(rootfs)?;
20182022
}
20192023

20202024
// This should not remove /boot/efi note.
@@ -2228,7 +2232,7 @@ pub(crate) async fn install_to_filesystem(
22282232
.await??;
22292233
}
22302234
Some(ReplaceMode::Alongside) => {
2231-
clean_boot_directories(&target_rootfs_fd, &target_root_path, is_already_ostree)?
2235+
clean_boot_directories(&target_rootfs_fd, is_already_ostree)?
22322236
}
22332237
None => require_empty_rootdir(&rootfs_fd)?,
22342238
}

0 commit comments

Comments
 (0)