From b05cb6818d1562cba7866fdfcbf915976cdb2ec5 Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Tue, 9 Jun 2026 09:22:32 +0530 Subject: [PATCH 01/21] Add Bootloader enum Now that we are going to support multiple bootloaders, we need a way to keep track of which one we're operating on Signed-off-by: Pragyan Poudyal --- src/bootloader.rs | 73 +++++++++++++++++++++++++++++++++++++++++++++++ src/efi.rs | 2 +- src/main.rs | 1 + 3 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 src/bootloader.rs diff --git a/src/bootloader.rs b/src/bootloader.rs new file mode 100644 index 00000000..7b2d3fbf --- /dev/null +++ b/src/bootloader.rs @@ -0,0 +1,73 @@ +use anyhow::Result; +use fn_error_context::context; +use std::{fmt::Display, sync::OnceLock}; + +use crate::efi::get_loader_info; + +#[derive(Debug, Default, Copy, Clone, clap::ValueEnum, PartialEq, Eq)] +pub enum Bootloader { + #[default] + Grub, + GrubCC, +} + +impl Display for Bootloader { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Bootloader::Grub => f.write_str("grub"), + Bootloader::GrubCC => f.write_str("grub-cc"), + } + } +} + +impl Bootloader { + fn next(self) -> Option { + match self { + Self::Grub => Some(Self::GrubCC), + Self::GrubCC => None, + } + } + + pub(crate) fn iter() -> impl Iterator { + std::iter::successors(Some(Self::Grub), |v| v.next()) + } + + /// Returns the name of the EFI component for this particular bootloader + /// We use directories inside /usr/lib/efi as values of EFI component + /// + /// Example + /// /usr/lib/efi/ + /// |-- grub-cc + /// |-- grub2 + /// `-- shim + pub(crate) fn efi_component_name(&self) -> &'static str { + match self { + Bootloader::Grub => "grub2", + Bootloader::GrubCC => "grub-cc", + } + } +} + +#[context("Getting bootloader")] +pub(crate) fn get_bootloader() -> Result { + static BOOTLOADER: OnceLock = OnceLock::new(); + + if let Some(bootloader) = BOOTLOADER.get() { + return Ok(*bootloader); + } + + let bootloader = match get_loader_info() { + Some(info) => { + if info.to_lowercase().contains("grub cc") { + Bootloader::GrubCC + } else { + Bootloader::Grub + } + } + None => Bootloader::Grub, + }; + + BOOTLOADER.get_or_init(|| bootloader); + + return Ok(bootloader); +} diff --git a/src/efi.rs b/src/efi.rs index cb5c1b0c..c8fd5432 100644 --- a/src/efi.rs +++ b/src/efi.rs @@ -286,7 +286,7 @@ fn read_efi_var_utf16_string(name: &str) -> Option { } /// Read the LoaderInfo EFI variable if it exists. -fn get_loader_info() -> Option { +pub(crate) fn get_loader_info() -> Option { read_efi_var_utf16_string(LOADER_INFO_VAR_STR) } diff --git a/src/main.rs b/src/main.rs index 56e9ef99..53371753 100644 --- a/src/main.rs +++ b/src/main.rs @@ -19,6 +19,7 @@ Refs: mod backend; #[cfg(any(target_arch = "x86_64", target_arch = "powerpc64"))] mod bios; +mod bootloader; mod bootupd; mod cli; mod component; From 1b9b19e24f722571136dd54d068b8090726d3a3b Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Tue, 9 Jun 2026 10:18:48 +0530 Subject: [PATCH 02/21] efi: Filter components by bootloader Generate metadata for only a particular bootloader, either Grub or GrubCC Signed-off-by: Pragyan Poudyal --- src/bios.rs | 3 +- src/bootupd.rs | 5 +- src/cli/bootupd.rs | 10 +++- src/component.rs | 7 ++- src/efi.rs | 132 +++++++++++++++++++++++++++++++++++++-------- 5 files changed, 131 insertions(+), 26 deletions(-) diff --git a/src/bios.rs b/src/bios.rs index 1ee0b51a..dd8e0c0b 100644 --- a/src/bios.rs +++ b/src/bios.rs @@ -9,6 +9,7 @@ use std::process::Command; use bootc_internal_blockdev::Device; +use crate::bootloader::Bootloader; use crate::bootupd::RootContext; use crate::component::*; use crate::freezethaw::fsfreeze_thaw_cycle; @@ -127,7 +128,7 @@ impl Component for Bios { }) } - fn generate_update_metadata(&self, sysroot_path: &str) -> Result> { + fn generate_update_metadata(&self, sysroot_path: &str, bootloader: Bootloader) -> Result> { let grub_install = Path::new(sysroot_path).join(GRUB_BIN); if !grub_install.exists() { println!("Failed to find {:?}", grub_install); diff --git a/src/bootupd.rs b/src/bootupd.rs index 16a49138..885425b3 100644 --- a/src/bootupd.rs +++ b/src/bootupd.rs @@ -1,5 +1,6 @@ #[cfg(any(target_arch = "x86_64", target_arch = "powerpc64"))] use crate::bios; +use crate::bootloader::Bootloader; use crate::component; use crate::component::{Component, ValidationResult}; use crate::coreos; @@ -270,13 +271,13 @@ pub(crate) fn get_components() -> Components { get_components_impl(false) } -pub(crate) fn generate_update_metadata(sysroot_path: &str) -> Result<()> { +pub(crate) fn generate_update_metadata(sysroot_path: &str, bootloader: Bootloader) -> Result<()> { // create bootupd update dir which will save component metadata files for both components let updates_dir = Path::new(sysroot_path).join(crate::model::BOOTUPD_UPDATES_DIR); std::fs::create_dir_all(&updates_dir) .with_context(|| format!("Failed to create updates dir {:?}", &updates_dir))?; for component in get_components().values() { - if let Some(v) = component.generate_update_metadata(sysroot_path)? { + if let Some(v) = component.generate_update_metadata(sysroot_path, bootloader)? { println!( "Generated update layout for {}: {}", component.name(), diff --git a/src/cli/bootupd.rs b/src/cli/bootupd.rs index b560910f..61210bf2 100644 --- a/src/cli/bootupd.rs +++ b/src/cli/bootupd.rs @@ -1,3 +1,4 @@ +use crate::bootloader::Bootloader; use crate::bootupd::{self, ConfigMode}; use anyhow::{Context, Result}; use camino::Utf8Path; @@ -90,6 +91,12 @@ pub struct GenerateOpts { /// Physical root mountpoint #[clap(value_parser)] sysroot: Option, + + /// The bootloader to generate metadata for + // + // We have a default to not break older systems + #[clap(long, default_value_t = Bootloader::Grub)] + bootloader: Bootloader, } impl DCommand { @@ -107,7 +114,8 @@ impl DCommand { if sysroot != "/" { anyhow::bail!("Using a non-default sysroot is not supported: {}", sysroot); } - bootupd::generate_update_metadata(sysroot).context("generating metadata failed")?; + bootupd::generate_update_metadata(sysroot, opts.bootloader) + .context("generating metadata failed")?; Ok(()) } diff --git a/src/component.rs b/src/component.rs index 403ffeec..3af934fc 100644 --- a/src/component.rs +++ b/src/component.rs @@ -14,6 +14,7 @@ use std::path::{Path, PathBuf}; use bootc_internal_blockdev::Device; +use crate::bootloader::Bootloader; use crate::{bootupd::RootContext, model::*}; #[derive(Serialize, Deserialize, Debug)] @@ -70,7 +71,11 @@ pub(crate) trait Component { /// this is an `rpm-ostree compose tree` for example. For a dual-partition /// style updater, this would be run as part of a postprocessing step /// while the filesystem for the partition is mounted. - fn generate_update_metadata(&self, sysroot: &str) -> Result>; + fn generate_update_metadata( + &self, + sysroot: &str, + bootloader: Bootloader, + ) -> Result>; /// Used on the client to query for an update cached in the current booted OS. fn query_update(&self, sysroot: &Dir) -> Result>; diff --git a/src/efi.rs b/src/efi.rs index c8fd5432..59b2181a 100644 --- a/src/efi.rs +++ b/src/efi.rs @@ -24,6 +24,7 @@ use widestring::U16CString; use bootc_internal_blockdev::Device; +use crate::bootloader::Bootloader; use crate::bootupd::RootContext; use crate::freezethaw::fsfreeze_thaw_cycle; use crate::model::*; @@ -375,7 +376,8 @@ impl Component for Efi { let updated_path = { let efilib_path = rootcxt.path.join(EFILIB); - if efilib_path.exists() && get_efi_component_from_usr(&rootcxt.path, EFILIB)?.is_some() + if efilib_path.exists() + && get_efi_component_from_usr(&rootcxt.path, EFILIB, None)?.is_some() { PathBuf::from(EFILIB) } else { @@ -468,7 +470,7 @@ impl Component for Efi { let src_path = Utf8Path::new(src_root); let efi_comps = if src_path.join(EFILIB).exists() { - get_efi_component_from_usr(&src_path, EFILIB)? + get_efi_component_from_usr(&src_path, EFILIB, None)? } else { None }; @@ -524,7 +526,8 @@ impl Component for Efi { let updatemeta = self.query_update(sysroot_dir)?.expect("update available"); let updated_path = { let efilib_path = rootcxt.path.join(EFILIB); - if efilib_path.exists() && get_efi_component_from_usr(&rootcxt.path, EFILIB)?.is_some() + if efilib_path.exists() + && get_efi_component_from_usr(&rootcxt.path, EFILIB, None)?.is_some() { PathBuf::from(EFILIB) } else { @@ -567,7 +570,11 @@ impl Component for Efi { }) } - fn generate_update_metadata(&self, sysroot: &str) -> Result> { + fn generate_update_metadata( + &self, + sysroot: &str, + bootloader: Bootloader, + ) -> Result> { let sysroot_path = Utf8Path::new(sysroot); let sysroot_dir = Dir::open_ambient_dir(sysroot_path, cap_std::ambient_authority())?; @@ -579,7 +586,7 @@ impl Component for Efi { // have them in /usr/lib/ostree-boot, which we move to /usr/lib/bootupd/updates/EFI let metadata = if sysroot_path.join(EFILIB).exists() { println!("Generating metadata from {EFILIB}"); - generate_meta_from_usr_efi(sysroot_path)? + generate_meta_from_usr_efi(sysroot_path, bootloader)? } else { match &ostreeboot { Some(..) => { @@ -804,8 +811,12 @@ fn find_file_recursive>(dir: P, target_file: &str) -> Result Result { - let Some(efi_components) = get_efi_component_from_usr(sysroot_path, EFILIB)? else { +fn generate_meta_from_usr_efi( + sysroot_path: &Utf8Path, + bootloader: Bootloader, +) -> Result { + let Some(efi_components) = get_efi_component_from_usr(sysroot_path, EFILIB, Some(bootloader))? + else { anyhow::bail!("Failed to find EFI components"); }; @@ -837,9 +848,11 @@ pub struct EFIComponent { } /// Get EFIComponents from e.g. usr/lib/efi, like "usr/lib/efi///EFI" +/// Filter the components by Bootloader, if Bootloader is None, no filtering is performed fn get_efi_component_from_usr<'a>( sysroot: &'a Utf8Path, usr_path: &'a str, + bootloader: Option, ) -> Result>> { let efilib_path = sysroot.join(usr_path); let skip_count = Utf8Path::new(usr_path).components().count(); @@ -876,7 +889,22 @@ fn get_efi_component_from_usr<'a>( } components.sort_by(|a, b| a.name.cmp(&b.name)); - Ok(Some(components)) + let Some(bootloader) = bootloader else { + return Ok(Some(components)); + }; + + // Remove all EFI Components not associated with the bootloader + let to_remove = Bootloader::iter() + .filter(|b| *b != bootloader) + .map(|b| b.efi_component_name()) + .collect::>(); + + let efi_comps = components + .into_iter() + .filter(|comp| !to_remove.contains(&comp.name.as_str())) + .collect::>(); + + Ok(Some(efi_comps)) } /// Copies usr/lib/ostree-boot/EFI to usr/lib/bootupd/updates @@ -1044,30 +1072,92 @@ Boot0003* test"; let tmpdir: &tempfile::TempDir = &tempfile::tempdir()?; let tpath = tmpdir.path(); let efi_path = tpath.join("usr/lib/efi"); - std::fs::create_dir_all(efi_path.join("BAR/1.1/EFI"))?; - std::fs::create_dir_all(efi_path.join("FOO/1.1/EFI"))?; - std::fs::create_dir_all(efi_path.join("FOOBAR/1.1/test"))?; + + // Create realistic directory structure for both bootloaders + // grub-cc structure + std::fs::create_dir_all(efi_path.join("grub-cc/1:2.12-59.fc45/EFI/fedora"))?; + std::fs::File::create(efi_path.join("grub-cc/1:2.12-59.fc45/EFI/fedora/grubx64-cc.efi"))?; + + // grub2 structure + std::fs::create_dir_all(efi_path.join("grub2/1:2.12-58.fc44/EFI/fedora"))?; + std::fs::File::create(efi_path.join("grub2/1:2.12-58.fc44/EFI/fedora/grubx64.efi"))?; + + // shim structure + std::fs::create_dir_all(efi_path.join("shim/16.1-5/EFI/BOOT"))?; + std::fs::create_dir_all(efi_path.join("shim/16.1-5/EFI/fedora"))?; + std::fs::File::create(efi_path.join("shim/16.1-5/EFI/BOOT/BOOTX64.EFI"))?; + std::fs::File::create(efi_path.join("shim/16.1-5/EFI/BOOT/fbx64.efi"))?; + std::fs::File::create(efi_path.join("shim/16.1-5/EFI/fedora/BOOTX64.CSV"))?; + std::fs::File::create(efi_path.join("shim/16.1-5/EFI/fedora/mmx64.efi"))?; + std::fs::File::create(efi_path.join("shim/16.1-5/EFI/fedora/shim.efi"))?; + std::fs::File::create(efi_path.join("shim/16.1-5/EFI/fedora/shimx64.efi"))?; + let utf8_tpath = Utf8Path::from_path(tpath).ok_or_else(|| anyhow::anyhow!("Path is not valid UTF-8"))?; - let efi_comps = get_efi_component_from_usr(utf8_tpath, EFILIB)?; + + // Test with no filtering - should return all EFI components + let efi_comps = get_efi_component_from_usr(utf8_tpath, EFILIB, None)?; assert_eq!( efi_comps, Some(vec![ EFIComponent { - name: "BAR".to_string(), - version: "1.1".to_string(), - path: Utf8PathBuf::from("usr/lib/efi/BAR/1.1/EFI"), + name: "grub-cc".to_string(), + version: "1:2.12-59.fc45".to_string(), + path: Utf8PathBuf::from("usr/lib/efi/grub-cc/1:2.12-59.fc45/EFI"), + }, + EFIComponent { + name: "grub2".to_string(), + version: "1:2.12-58.fc44".to_string(), + path: Utf8PathBuf::from("usr/lib/efi/grub2/1:2.12-58.fc44/EFI"), }, EFIComponent { - name: "FOO".to_string(), - version: "1.1".to_string(), - path: Utf8PathBuf::from("usr/lib/efi/FOO/1.1/EFI"), + name: "shim".to_string(), + version: "16.1-5".to_string(), + path: Utf8PathBuf::from("usr/lib/efi/shim/16.1-5/EFI"), }, ]) ); - std::fs::remove_dir_all(efi_path.join("BAR/1.1/EFI"))?; - std::fs::remove_dir_all(efi_path.join("FOO/1.1/EFI"))?; - let efi_comps = get_efi_component_from_usr(utf8_tpath, EFILIB)?; + + // Test with filtering for Grub - should only return grub2 and shim + let efi_comps = get_efi_component_from_usr(utf8_tpath, EFILIB, Some(Bootloader::Grub))?; + assert_eq!( + efi_comps, + Some(vec![ + EFIComponent { + name: "grub2".to_string(), + version: "1:2.12-58.fc44".to_string(), + path: Utf8PathBuf::from("usr/lib/efi/grub2/1:2.12-58.fc44/EFI"), + }, + EFIComponent { + name: "shim".to_string(), + version: "16.1-5".to_string(), + path: Utf8PathBuf::from("usr/lib/efi/shim/16.1-5/EFI"), + }, + ]) + ); + + // Test with filtering for GrubCC - should only return grub-cc and shim + let efi_comps = get_efi_component_from_usr(utf8_tpath, EFILIB, Some(Bootloader::GrubCC))?; + assert_eq!( + efi_comps, + Some(vec![ + EFIComponent { + name: "grub-cc".to_string(), + version: "1:2.12-59.fc45".to_string(), + path: Utf8PathBuf::from("usr/lib/efi/grub-cc/1:2.12-59.fc45/EFI"), + }, + EFIComponent { + name: "shim".to_string(), + version: "16.1-5".to_string(), + path: Utf8PathBuf::from("usr/lib/efi/shim/16.1-5/EFI"), + }, + ]) + ); + + // Test with empty directory - should return None + std::fs::remove_dir_all(&efi_path)?; + std::fs::create_dir_all(&efi_path)?; + let efi_comps = get_efi_component_from_usr(utf8_tpath, EFILIB, None)?; assert_eq!(efi_comps, None); Ok(()) } From a8be4e9160456dc48d24119936b253e41be7266e Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Tue, 9 Jun 2026 10:28:40 +0530 Subject: [PATCH 03/21] bios/efi: Handle bootloader support Add `is_bootloader_supported` function to figure out if bootloader is supported by the component or not. Skip installs and metadata generation for the component if it does not support the bootloader Signed-off-by: Pragyan Poudyal --- src/bios.rs | 14 +++++++++++++- src/bootupd.rs | 17 +++++++++++++++++ src/component.rs | 2 ++ src/efi.rs | 4 ++++ 4 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/bios.rs b/src/bios.rs index dd8e0c0b..58931556 100644 --- a/src/bios.rs +++ b/src/bios.rs @@ -103,6 +103,10 @@ impl Component for Bios { "BIOS" } + fn is_bootloader_supported(&self, bootloader: Bootloader) -> bool { + matches!(bootloader, Bootloader::Grub) + } + fn install( &self, src_root: &str, @@ -128,7 +132,15 @@ impl Component for Bios { }) } - fn generate_update_metadata(&self, sysroot_path: &str, bootloader: Bootloader) -> Result> { + fn generate_update_metadata( + &self, + sysroot_path: &str, + bootloader: Bootloader, + ) -> Result> { + if bootloader != Bootloader::Grub { + anyhow::bail!("{bootloader} cannot be installed for bios"); + } + let grub_install = Path::new(sysroot_path).join(GRUB_BIN); if !grub_install.exists() { println!("Failed to find {:?}", grub_install); diff --git a/src/bootupd.rs b/src/bootupd.rs index 885425b3..8fb6912d 100644 --- a/src/bootupd.rs +++ b/src/bootupd.rs @@ -112,6 +112,14 @@ pub(crate) fn install( continue; } + if !component.is_bootloader_supported(bootloader) { + println!( + "Skip installing component {} as it does not support bootloader {bootloader}", + component.name() + ); + continue; + } + // Determine which devices to install to. For EFI, filter to only // devices that have an ESP partition. let devices_to_install: Vec> = if devices.is_empty() { @@ -277,6 +285,15 @@ pub(crate) fn generate_update_metadata(sysroot_path: &str, bootloader: Bootloade std::fs::create_dir_all(&updates_dir) .with_context(|| format!("Failed to create updates dir {:?}", &updates_dir))?; for component in get_components().values() { + if !component.is_bootloader_supported(bootloader) { + println!( + "Bootloader {bootloader} not supported for {}. Skipping metadata generation", + component.name(), + ); + + continue; + } + if let Some(v) = component.generate_update_metadata(sysroot_path, bootloader)? { println!( "Generated update layout for {}: {}", diff --git a/src/component.rs b/src/component.rs index 3af934fc..29e28244 100644 --- a/src/component.rs +++ b/src/component.rs @@ -97,6 +97,8 @@ pub(crate) trait Component { /// Locating efi vendor dir fn get_efi_vendor(&self, sysroot: &Path) -> Result>; + + fn is_bootloader_supported(&self, bootloader: Bootloader) -> bool; } /// Given a component name, create an implementation. diff --git a/src/efi.rs b/src/efi.rs index 59b2181a..fe97ab95 100644 --- a/src/efi.rs +++ b/src/efi.rs @@ -316,6 +316,10 @@ impl Component for Efi { "EFI" } + fn is_bootloader_supported(&self, bootloader: Bootloader) -> bool { + matches!(bootloader, Bootloader::Grub | Bootloader::GrubCC) + } + fn query_adopt(&self, devices: &Option>) -> Result> { if devices.is_none() { log::trace!("No ESP detected"); From e6133409b1f76d06813cf42b04b71687a15df087 Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Tue, 9 Jun 2026 10:46:47 +0530 Subject: [PATCH 04/21] Handle installs for bootloaders Signed-off-by: Pragyan Poudyal --- src/bios.rs | 7 ++++++- src/bootupd.rs | 27 +++++++++++++++------------ src/cli/bootupd.rs | 6 ++++++ src/component.rs | 4 ++-- src/efi.rs | 3 ++- 5 files changed, 31 insertions(+), 16 deletions(-) diff --git a/src/bios.rs b/src/bios.rs index 58931556..28a5b93c 100644 --- a/src/bios.rs +++ b/src/bios.rs @@ -113,7 +113,12 @@ impl Component for Bios { dest_root: &str, device: Option<&Device>, _update_firmware: bool, + bootloader: Bootloader, ) -> Result { + if !self.is_bootloader_supported(bootloader) { + anyhow::bail!("{bootloader} cannot be installed for bios"); + } + let device = device.ok_or_else(|| anyhow::anyhow!("BIOS component requires a target device"))?; let src_dir = Dir::open_ambient_dir(src_root, ambient_authority()) @@ -137,7 +142,7 @@ impl Component for Bios { sysroot_path: &str, bootloader: Bootloader, ) -> Result> { - if bootloader != Bootloader::Grub { + if !self.is_bootloader_supported(bootloader) { anyhow::bail!("{bootloader} cannot be installed for bios"); } diff --git a/src/bootupd.rs b/src/bootupd.rs index 8fb6912d..1ee925dd 100644 --- a/src/bootupd.rs +++ b/src/bootupd.rs @@ -61,6 +61,7 @@ pub(crate) fn install( update_firmware: bool, target_components: Option<&[String]>, auto_components: bool, + bootloader: Bootloader, ) -> Result<()> { let source_root_dir = Dir::open_ambient_dir(source_root, ambient_authority()).context("Opening source root")?; @@ -157,7 +158,7 @@ pub(crate) fn install( for device in &devices_to_install { let device_desc = device.map_or("(auto)".to_string(), |d| d.path()); let meta = component - .install(source_root, dest_root, *device, update_firmware) + .install(source_root, dest_root, *device, update_firmware, bootloader) .with_context(|| { format!( "installing component {} to device {}", @@ -194,19 +195,21 @@ pub(crate) fn install( target_arch = "powerpc64", target_arch = "riscv64" ))] - match configs.enabled_with_uuid() { - Some(uuid) => { - let meta = get_static_config_meta()?; - state.static_configs = Some(meta); - crate::grubconfigs::install( - sysroot, - Some(&source_root_dir), - installed_efi_vendor.as_deref(), - uuid, - )?; + if bootloader == Bootloader::Grub { + match configs.enabled_with_uuid() { + Some(uuid) => { + let meta = get_static_config_meta()?; + state.static_configs = Some(meta); + crate::grubconfigs::install( + sysroot, + Some(&source_root_dir), + installed_efi_vendor.as_deref(), + uuid, + )?; + } // On other architectures, assume that there's nothing to do. + None => {} } - None => {} } // Unmount the ESP, etc. diff --git a/src/cli/bootupd.rs b/src/cli/bootupd.rs index 61210bf2..f379bdf0 100644 --- a/src/cli/bootupd.rs +++ b/src/cli/bootupd.rs @@ -84,6 +84,11 @@ pub struct InstallOpts { /// then only enable installation to the ESP. #[clap(long)] auto: bool, + + /// The bootloader to use + /// Defaults to Grub + #[clap(long, default_value_t = Bootloader::Grub)] + bootloader: Bootloader, } #[derive(Debug, Parser)] @@ -151,6 +156,7 @@ impl DCommand { opts.update_firmware, opts.components.as_deref(), opts.auto, + opts.bootloader, ) .context("boot data installation failed")?; Ok(()) diff --git a/src/component.rs b/src/component.rs index 29e28244..ea9df238 100644 --- a/src/component.rs +++ b/src/component.rs @@ -14,8 +14,7 @@ use std::path::{Path, PathBuf}; use bootc_internal_blockdev::Device; -use crate::bootloader::Bootloader; -use crate::{bootupd::RootContext, model::*}; +use crate::{bootloader::Bootloader, bootupd::RootContext, model::*}; #[derive(Serialize, Deserialize, Debug)] #[serde(rename_all = "kebab-case")] @@ -64,6 +63,7 @@ pub(crate) trait Component { dest_root: &str, device: Option<&Device>, update_firmware: bool, + bootloader: Bootloader, ) -> Result; /// Implementation of `bootupd generate-update-metadata` for a given component. diff --git a/src/efi.rs b/src/efi.rs index fe97ab95..f049452e 100644 --- a/src/efi.rs +++ b/src/efi.rs @@ -440,6 +440,7 @@ impl Component for Efi { dest_root: &str, device: Option<&Device>, update_firmware: bool, + bootloader: Bootloader, ) -> Result { let src_dir = Dir::open_ambient_dir(src_root, ambient_authority()) .with_context(|| format!("opening source directory {src_root}"))?; @@ -474,7 +475,7 @@ impl Component for Efi { let src_path = Utf8Path::new(src_root); let efi_comps = if src_path.join(EFILIB).exists() { - get_efi_component_from_usr(&src_path, EFILIB, None)? + get_efi_component_from_usr(src_path, EFILIB, Some(bootloader))? } else { None }; From 40fc00f38155640a721ea577ce90804f6a4c11e7 Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Wed, 10 Jun 2026 10:42:19 +0530 Subject: [PATCH 05/21] grub-cc: Handle statefile For grub we store the statefile in `/sysroot/boot/bootupd.json`. For grub-cc, and in future systemd-boot, we won't have a `/boot`, so we now store the state pre bootloader. Grub2: Statefile is still in `/sysroot/boot/bootupd.json` GrubCC: Statefile is stored in all the ESPs Signed-off-by: Pragyan Poudyal --- src/backend/statefile.rs | 143 +++++++++++++++++++++++++++++++++------ src/bootupd.rs | 33 ++++++--- src/efi.rs | 2 +- 3 files changed, 147 insertions(+), 31 deletions(-) diff --git a/src/backend/statefile.rs b/src/backend/statefile.rs index 4f7bef26..287fc012 100644 --- a/src/backend/statefile.rs +++ b/src/backend/statefile.rs @@ -1,5 +1,8 @@ //! On-disk saved state. +use crate::bootloader::Bootloader; +use crate::efi::Efi; +use crate::freezethaw::fsfreeze_thaw_cycle; use crate::model::SavedState; use crate::util::SignalTerminationGuard; use anyhow::{bail, Context, Result}; @@ -10,7 +13,9 @@ use fn_error_context::context; use fs2::FileExt; use std::fs::File; use std::io::prelude::*; +use std::os::fd::{AsRawFd, FromRawFd}; use std::path::Path; +use tempfile::tempdir; impl SavedState { /// System-wide bootupd write lock (relative to sysroot). @@ -57,13 +62,52 @@ impl SavedState { /// Load the JSON file containing on-disk state. #[context("Loading saved state")] - pub(crate) fn load_from_disk(root_path: impl AsRef) -> Result> { + pub(crate) fn load_from_disk( + root_path: impl AsRef, + bootloader: Option, + ) -> Result> { let root_path = root_path.as_ref(); let sysroot = Dir::open_ambient_dir(root_path, ambient_authority()) .with_context(|| format!("opening sysroot '{}'", root_path.display()))?; - let statefile_path = Path::new(Self::STATEFILE_DIR).join(Self::STATEFILE_NAME); - let saved_state = if let Some(statusf) = sysroot.open_optional(&statefile_path)? { + let (statefile, _esp_guard) = match bootloader { + Some(b) => match b { + Bootloader::Grub => { + let path = Path::new(Self::STATEFILE_DIR).join(Self::STATEFILE_NAME); + (sysroot.open_optional(&path)?, None) + } + + Bootloader::GrubCC => { + let efi = Efi::default(); + + let dir = Dir::open_ambient_dir(&root_path, ambient_authority()) + .with_context(|| format!("Opening filesystem path {root_path:?}"))?; + let device = bootc_internal_blockdev::list_dev_by_dir(&dir)?; + + // Since we write the state file to all ESPs, it should be enough to get it + // from the first one. Though, we could check the integrity by getting from + // all the ESPs and making sure they're all the same... + let esp = device.find_first_colocated_esp()?; + + let tmpdir = tempdir()?; + std::fs::create_dir_all(tmpdir.path().join("efi")) + .context("Creating efi inside tmpdir")?; + + let mounted = efi + .ensure_mounted_esp(tmpdir.path(), &Path::new(&esp.path())) + .context("Mounting ESP")?; + + let dir = Dir::open_ambient_dir(&mounted, ambient_authority())?; + + (dir.open_optional(Self::STATEFILE_NAME)?, Some(efi)) + } + }, + + // No bootloader, we're probably running inside a container + None => (None, None), + }; + + let saved_state = if let Some(statusf) = statefile { let mut bufr = std::io::BufReader::new(statusf); let mut s = String::new(); bufr.read_to_string(&mut s)?; @@ -85,18 +129,34 @@ impl SavedState { } else { None }; + Ok(saved_state) } /// Check whether statefile exists. - pub(crate) fn ensure_not_present(root_path: impl AsRef) -> Result<()> { - let statepath = Path::new(root_path.as_ref()) - .join(Self::STATEFILE_DIR) - .join(Self::STATEFILE_NAME); - if statepath.exists() { - bail!("{} already exists", statepath.display()); + pub(crate) fn ensure_not_present( + root_path: impl AsRef, + bootloader: Bootloader, + ) -> Result<()> { + let saved_state = SavedState::load_from_disk(&root_path, Some(bootloader))?; + + if saved_state.is_none() { + return Ok(()); + } + + match bootloader { + Bootloader::Grub => { + let statepath = Path::new(root_path.as_ref()) + .join(Self::STATEFILE_DIR) + .join(Self::STATEFILE_NAME); + + bail!("{} already exists", statepath.display()); + } + + Bootloader::GrubCC => { + bail!("{} already exists in the ESP", Self::STATEFILE_NAME); + } } - Ok(()) } } @@ -112,16 +172,59 @@ pub(crate) struct StateLockGuard { impl StateLockGuard { /// Atomically replace the on-disk state with a new version. - pub(crate) fn update_state(&mut self, state: &SavedState) -> Result<()> { - let subdir = self.sysroot.open_dir(SavedState::STATEFILE_DIR)?; - - subdir - .atomic_write_with_perms( - SavedState::STATEFILE_NAME, - serde_json::to_vec(state).context("Serializing state")?, - Permissions::from_mode(0o644), - ) - .context("Writing state file")?; + #[context("Updating state")] + pub(crate) fn update_state( + &mut self, + state: &SavedState, + bootloader: Bootloader, + ) -> Result<()> { + if bootloader == Bootloader::Grub { + let subdir = self.sysroot.open_dir(SavedState::STATEFILE_DIR)?; + + subdir + .atomic_write_with_perms( + SavedState::STATEFILE_NAME, + serde_json::to_vec(state).context("Serializing state")?, + Permissions::from_mode(0o644), + ) + .context("Writing state file")?; + + return Ok(()); + } + + let dir = unsafe { Dir::from_raw_fd(self.sysroot.as_raw_fd()) }; + let device = bootc_internal_blockdev::list_dev_by_dir(&dir)?; + let all_esps = device + .find_colocated_esps() + .context("Searching for ESP")? + .ok_or_else(|| anyhow::anyhow!("ESP not found"))?; + + let efi = Efi::default(); + + let tmpdir = tempdir()?; + + // [`ensure_mounted_esp`] needs this + std::fs::create_dir_all(tmpdir.path().join("efi")).context("Creating efi inside tmpdir")?; + + for esp in all_esps { + let mounted = efi + .ensure_mounted_esp(tmpdir.path(), &Path::new(&esp.path())) + .context("Mounting ESP")?; + + let dir = Dir::open_ambient_dir(&mounted, ambient_authority())?; + + dir.atomic_replace_with(SavedState::STATEFILE_NAME, |w| -> std::io::Result<()> { + serde_json::to_writer(w, state)?; + Ok(()) + })?; + + // dir.set_permissions(SavedState::STATEFILE_NAME, 0o644)?; + + // Do the sync before unmount + fsfreeze_thaw_cycle(dir.reopen_as_ownedfd()?)?; + drop(dir); + efi.unmount().context("unmount after update")?; + } Ok(()) } diff --git a/src/bootupd.rs b/src/bootupd.rs index 1ee925dd..a50a098b 100644 --- a/src/bootupd.rs +++ b/src/bootupd.rs @@ -1,6 +1,6 @@ #[cfg(any(target_arch = "x86_64", target_arch = "powerpc64"))] use crate::bios; -use crate::bootloader::Bootloader; +use crate::bootloader::{get_bootloader, Bootloader}; use crate::component; use crate::component::{Component, ValidationResult}; use crate::coreos; @@ -19,6 +19,7 @@ use crate::freezethaw::fsfreeze_thaw_cycle; ))] use crate::grubconfigs::{ensure_grub_permissions, GRUB2DIR}; use crate::model::{ComponentStatus, ComponentUpdatable, ContentMetadata, SavedState, Status}; +use crate::util::running_in_container; use crate::{ostreeutil, util}; use anyhow::{anyhow, Context, Result}; use camino::{Utf8Path, Utf8PathBuf}; @@ -65,7 +66,7 @@ pub(crate) fn install( ) -> Result<()> { let source_root_dir = Dir::open_ambient_dir(source_root, ambient_authority()).context("Opening source root")?; - SavedState::ensure_not_present(dest_root) + SavedState::ensure_not_present(dest_root, bootloader) .context("failed to install, invalid re-install attempted")?; let all_components = get_components_impl(auto_components); @@ -218,7 +219,7 @@ pub(crate) fn install( let mut state_guard = SavedState::unlocked(sysroot.try_clone()?).context("failed to acquire write lock")?; state_guard - .update_state(&state) + .update_state(&state, bootloader) .context("failed to update state")?; Ok(()) @@ -332,7 +333,9 @@ fn ensure_writable_boot() -> Result<()> { /// daemon implementation of component update pub(crate) fn update(name: &str, rootcxt: &RootContext) -> Result { - let mut state = SavedState::load_from_disk("/")?.unwrap_or_default(); + let bootloader = get_bootloader()?; + + let mut state = SavedState::load_from_disk("/", Some(bootloader))?.unwrap_or_default(); let component = component::new_from_name(name)?; let inst = if let Some(inst) = state.installed.get(name) { inst.clone() @@ -374,7 +377,7 @@ pub(crate) fn update(name: &str, rootcxt: &RootContext) -> Result Result Result> { + let bootloader = get_bootloader()?; let sysroot = &rootcxt.sysroot; - let mut state = SavedState::load_from_disk("/")?.unwrap_or_default(); + let mut state = SavedState::load_from_disk("/", Some(bootloader))?.unwrap_or_default(); let component = component::new_from_name(name)?; if state.installed.contains_key(name) { anyhow::bail!("Component {} is already installed", name); @@ -441,7 +445,7 @@ pub(crate) fn adopt_and_update( println!("Static GRUB configuration has been adopted successfully."); } - state_guard.update_state(&state)?; + state_guard.update_state(&state, bootloader)?; return Ok(Some(update)); } else { // Nothing adopted, skip @@ -468,7 +472,7 @@ fn list_dev_current_root() -> Result { /// daemon implementation of component validate pub(crate) fn validate(name: &str) -> Result { - let state = SavedState::load_from_disk("/")?.unwrap_or_default(); + let state = SavedState::load_from_disk("/", Some(get_bootloader()?))?.unwrap_or_default(); let component = component::new_from_name(name)?; let Some(inst) = state.installed.get(name) else { anyhow::bail!("Component {} is not installed", name); @@ -477,11 +481,20 @@ pub(crate) fn validate(name: &str) -> Result { component.validate(inst, &device) } +/// Impl for bootupctl status +/// This function assumes we're not running in a container pub(crate) fn status() -> Result { let mut ret: Status = Default::default(); let mut known_components = get_components(); let sysroot = Dir::open_ambient_dir("/", ambient_authority())?; - let state = SavedState::load_from_disk("/")?; + + let bootloader = if running_in_container() { + None + } else { + Some(get_bootloader()?) + }; + + let state = SavedState::load_from_disk("/", bootloader)?; if let Some(state) = state { for (name, ic) in state.installed.iter() { log::trace!("Gathering status for installed component: {}", name); diff --git a/src/efi.rs b/src/efi.rs index f049452e..38a95c7a 100644 --- a/src/efi.rs +++ b/src/efi.rs @@ -179,7 +179,7 @@ impl Efi { Ok(destdir) } - fn unmount(&self) -> Result<()> { + pub(crate) fn unmount(&self) -> Result<()> { if let Some(mount) = self.mountpoint.borrow_mut().take() { Command::new("umount") .arg(&mount) From 3a6875471ff89b490fa8bd291dd2b1b3c9c54c5c Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Thu, 11 Jun 2026 12:31:07 +0530 Subject: [PATCH 06/21] efi: Do not unmount if mountpoint was already mounted Signed-off-by: Pragyan Poudyal --- src/efi.rs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/efi.rs b/src/efi.rs index 38a95c7a..f5661c8f 100644 --- a/src/efi.rs +++ b/src/efi.rs @@ -87,9 +87,12 @@ pub(crate) fn is_efi_booted() -> Result { .map_err(Into::into) } -#[derive(Default)] +#[derive(Default, Debug)] pub(crate) struct Efi { mountpoint: RefCell>, + /// Whether the above mountpoint was already mounted or not + /// Won't unmount if it was already mounted + was_mounted: RefCell, } impl Efi { @@ -116,6 +119,7 @@ impl Efi { continue; } util::ensure_writable_mount(&path)?; + *self.was_mounted.borrow_mut() = true; found_mount = Some(path); break; } @@ -148,6 +152,7 @@ impl Efi { if is_mount_point(&mnt)? { log::debug!("ESP already mounted at {mnt:?}, reusing"); mountpoint = Some(mnt); + *self.was_mounted.borrow_mut() = true; break; } } @@ -180,6 +185,7 @@ impl Efi { } pub(crate) fn unmount(&self) -> Result<()> { + *self.was_mounted.borrow_mut() = false; if let Some(mount) = self.mountpoint.borrow_mut().take() { Command::new("umount") .arg(&mount) @@ -704,7 +710,12 @@ impl Component for Efi { impl Drop for Efi { fn drop(&mut self) { - log::debug!("Unmounting"); + if *self.was_mounted.borrow() { + log::debug!("mountpoint was already mounted. Won't unmount",); + return; + } + + log::debug!("Unmounting {:?}", self.mountpoint); let _ = self.unmount(); } } From 484a06cc1f3970dae3ad0d80cbfb7e5d998435e5 Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Fri, 12 Jun 2026 12:02:58 +0530 Subject: [PATCH 07/21] state: Handle state file for GrubCC Store the sysroot_path in StateLockGuard which we require for mounting the ESP. Also, prevent mounting ESP at a tempdir, instead just mount it at `/boot` which is what the BLS spec suggest Fix issue with composefs systems where lsblk would fail to find the backing device for "/" due to it being mounted as a virtual filesystem. Instead we now use `/sysroot` or `/boot` to find the baking device Signed-off-by: Pragyan Poudyal --- src/backend/statefile.rs | 141 +++++++++++++++++++++++---------------- src/bootupd.rs | 22 +++--- 2 files changed, 93 insertions(+), 70 deletions(-) diff --git a/src/backend/statefile.rs b/src/backend/statefile.rs index 287fc012..fc2339b6 100644 --- a/src/backend/statefile.rs +++ b/src/backend/statefile.rs @@ -1,11 +1,14 @@ //! On-disk saved state. use crate::bootloader::Bootloader; +use crate::bootupd::list_dev_current_root; use crate::efi::Efi; use crate::freezethaw::fsfreeze_thaw_cycle; use crate::model::SavedState; use crate::util::SignalTerminationGuard; use anyhow::{bail, Context, Result}; +use bootc_internal_blockdev::Device; +use camino::Utf8PathBuf; use cap_std::ambient_authority; use cap_std::fs::{Dir, Permissions, PermissionsExt}; use cap_std_ext::dirext::CapStdExtDirExt; @@ -13,16 +16,59 @@ use fn_error_context::context; use fs2::FileExt; use std::fs::File; use std::io::prelude::*; -use std::os::fd::{AsRawFd, FromRawFd}; use std::path::Path; -use tempfile::tempdir; + +fn parse_statefile(statusf: cap_std::fs::File) -> Result> { + let mut bufr = std::io::BufReader::new(statusf); + let mut s = String::new(); + bufr.read_to_string(&mut s)?; + let state: serde_json::Result = serde_json::from_str(s.as_str()); + + let r = match state { + Ok(s) => s, + Err(orig_err) => { + let state: serde_json::Result = + serde_json::from_str(s.as_str()); + match state { + Ok(s) => s.upconvert(), + Err(_) => { + return Err(orig_err.into()); + } + } + } + }; + + Ok(Some(r)) +} + +/// lsblk: composefs:abc123..: not a block device +/// is what lsblk throws on composefs booted systems if we try to +/// get block devices using "/" +/// +/// First, try to get the device from the `root` which is necessary +/// during installs as we don't want (or can't) to open up /sysroot or /boot +/// +/// If that fails, it means we're not on the install path so we get the +/// device from checking mount point from /boot or /sysroot +#[context("Getting parent device")] +fn get_parent_device(root: &Dir) -> Result { + match bootc_internal_blockdev::list_dev_by_dir(root) { + Ok(d) => Ok(d), + Err(e) => { + // Not really an error just yet + log::debug!("{e:?}"); + list_dev_current_root() + } + } +} impl SavedState { /// System-wide bootupd write lock (relative to sysroot). const WRITE_LOCK_PATH: &'static str = "run/bootupd-lock"; /// Top-level directory for statefile (relative to sysroot). pub(crate) const STATEFILE_DIR: &'static str = "boot"; - /// On-disk bootloader statefile, akin to a tiny rpm/dpkg database, stored in `/boot`. + /// On-disk bootloader statefile, akin to a tiny rpm/dpkg database, + /// stored in `/boot` for Grub and in `ESP` for GrubCC pub(crate) const STATEFILE_NAME: &'static str = "bootupd-state.json"; /// Try to acquire a system-wide lock to ensure non-conflicting state updates. @@ -30,7 +76,10 @@ impl SavedState { /// While ordinarily the daemon runs as a systemd unit (which implicitly /// ensures a single instance) this is a double check against other /// execution paths. - pub(crate) fn acquire_write_lock(sysroot: Dir) -> Result { + pub(crate) fn acquire_write_lock( + sysroot_path: Utf8PathBuf, + sysroot: Dir, + ) -> Result { sysroot .atomic_write_with_perms(Self::WRITE_LOCK_PATH, "", Permissions::from_mode(0o644)) .context("Creating lock file")?; @@ -43,6 +92,7 @@ impl SavedState { lockfile.lock_exclusive().context("Acquiring lock")?; let guard = StateLockGuard { + sysroot_path, sysroot, termguard: Some(SignalTerminationGuard::new()?), lockfile: Some(lockfile), @@ -52,8 +102,9 @@ impl SavedState { /// Use this for cases when the target root isn't booted, which is /// offline installs. - pub(crate) fn unlocked(sysroot: Dir) -> Result { + pub(crate) fn unlocked(sysroot_path: Utf8PathBuf, sysroot: Dir) -> Result { Ok(StateLockGuard { + sysroot_path, sysroot, termguard: None, lockfile: None, @@ -67,70 +118,49 @@ impl SavedState { bootloader: Option, ) -> Result> { let root_path = root_path.as_ref(); - let sysroot = Dir::open_ambient_dir(root_path, ambient_authority()) + + let root = Dir::open_ambient_dir(root_path, ambient_authority()) .with_context(|| format!("opening sysroot '{}'", root_path.display()))?; - let (statefile, _esp_guard) = match bootloader { + match bootloader { Some(b) => match b { Bootloader::Grub => { let path = Path::new(Self::STATEFILE_DIR).join(Self::STATEFILE_NAME); - (sysroot.open_optional(&path)?, None) + + match root.open_optional(&path)? { + Some(f) => parse_statefile(f), + None => Ok(None), + } } Bootloader::GrubCC => { let efi = Efi::default(); - let dir = Dir::open_ambient_dir(&root_path, ambient_authority()) - .with_context(|| format!("Opening filesystem path {root_path:?}"))?; - let device = bootc_internal_blockdev::list_dev_by_dir(&dir)?; + let device = get_parent_device(&root)?; // Since we write the state file to all ESPs, it should be enough to get it // from the first one. Though, we could check the integrity by getting from // all the ESPs and making sure they're all the same... let esp = device.find_first_colocated_esp()?; - let tmpdir = tempdir()?; - std::fs::create_dir_all(tmpdir.path().join("efi")) - .context("Creating efi inside tmpdir")?; - + // According to BLS, the ESP should be mounted at /boot or /boot/efi + // which the following method already checks let mounted = efi - .ensure_mounted_esp(tmpdir.path(), &Path::new(&esp.path())) + .ensure_mounted_esp(root_path, Path::new(&esp.path())) .context("Mounting ESP")?; let dir = Dir::open_ambient_dir(&mounted, ambient_authority())?; - (dir.open_optional(Self::STATEFILE_NAME)?, Some(efi)) + match dir.open_optional(Self::STATEFILE_NAME)? { + Some(f) => parse_statefile(f), + None => Ok(None), + } } }, // No bootloader, we're probably running inside a container - None => (None, None), - }; - - let saved_state = if let Some(statusf) = statefile { - let mut bufr = std::io::BufReader::new(statusf); - let mut s = String::new(); - bufr.read_to_string(&mut s)?; - let state: serde_json::Result = serde_json::from_str(s.as_str()); - let r = match state { - Ok(s) => s, - Err(orig_err) => { - let state: serde_json::Result = - serde_json::from_str(s.as_str()); - match state { - Ok(s) => s.upconvert(), - Err(_) => { - return Err(orig_err.into()); - } - } - } - }; - Some(r) - } else { - None - }; - - Ok(saved_state) + None => Ok(None), + } } /// Check whether statefile exists. @@ -163,6 +193,7 @@ impl SavedState { /// Write-lock guard for statefile, protecting against concurrent state updates. #[derive(Debug)] pub(crate) struct StateLockGuard { + pub(crate) sysroot_path: Utf8PathBuf, pub(crate) sysroot: Dir, #[allow(dead_code)] termguard: Option, @@ -192,8 +223,7 @@ impl StateLockGuard { return Ok(()); } - let dir = unsafe { Dir::from_raw_fd(self.sysroot.as_raw_fd()) }; - let device = bootc_internal_blockdev::list_dev_by_dir(&dir)?; + let device = get_parent_device(&self.sysroot)?; let all_esps = device .find_colocated_esps() .context("Searching for ESP")? @@ -201,24 +231,21 @@ impl StateLockGuard { let efi = Efi::default(); - let tmpdir = tempdir()?; - - // [`ensure_mounted_esp`] needs this - std::fs::create_dir_all(tmpdir.path().join("efi")).context("Creating efi inside tmpdir")?; + let serialized_state = serde_json::to_vec(state).context("Serializing state")?; for esp in all_esps { let mounted = efi - .ensure_mounted_esp(tmpdir.path(), &Path::new(&esp.path())) + .ensure_mounted_esp(self.sysroot_path.as_std_path(), Path::new(&esp.path())) .context("Mounting ESP")?; let dir = Dir::open_ambient_dir(&mounted, ambient_authority())?; - dir.atomic_replace_with(SavedState::STATEFILE_NAME, |w| -> std::io::Result<()> { - serde_json::to_writer(w, state)?; - Ok(()) - })?; - - // dir.set_permissions(SavedState::STATEFILE_NAME, 0o644)?; + dir.atomic_write_with_perms( + SavedState::STATEFILE_NAME, + &serialized_state, + Permissions::from_mode(0o644), + ) + .context("Writing state file")?; // Do the sync before unmount fsfreeze_thaw_cycle(dir.reopen_as_ownedfd()?)?; diff --git a/src/bootupd.rs b/src/bootupd.rs index a50a098b..3814af97 100644 --- a/src/bootupd.rs +++ b/src/bootupd.rs @@ -216,8 +216,8 @@ pub(crate) fn install( // Unmount the ESP, etc. drop(target_components); - let mut state_guard = - SavedState::unlocked(sysroot.try_clone()?).context("failed to acquire write lock")?; + let mut state_guard = SavedState::unlocked(dest_root.into(), sysroot.try_clone()?) + .context("failed to acquire write lock")?; state_guard .update_state(&state, bootloader) .context("failed to update state")?; @@ -374,8 +374,8 @@ pub(crate) fn update(name: &str, rootcxt: &RootContext) -> Result Result { +pub(crate) fn list_dev_current_root() -> Result { let auth = cap_std::ambient_authority(); for path in ["/boot", "/sysroot"] { if let Ok(dir) = Dir::open_ambient_dir(path, auth) { @@ -488,13 +488,9 @@ pub(crate) fn status() -> Result { let mut known_components = get_components(); let sysroot = Dir::open_ambient_dir("/", ambient_authority())?; - let bootloader = if running_in_container() { - None - } else { - Some(get_bootloader()?) - }; + let bootloader = get_bootloader()?; + let state = SavedState::load_from_disk("/", Some(bootloader))?; - let state = SavedState::load_from_disk("/", bootloader)?; if let Some(state) = state { for (name, ic) in state.installed.iter() { log::trace!("Gathering status for installed component: {}", name); From 2cee598f2cb3a21a99b8771e71651dda76ceadaf Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Fri, 12 Jun 2026 12:35:56 +0530 Subject: [PATCH 08/21] Handle generate metadata and Bootloader Updates For `bootupd generate-update-metadata` we now generate metadata for all found bootloaders. The final json will look like ```json { "timestamp": "2026-06-10T09:52:58.107743026Z", "version": "grub-cc-1:2.12-59.fc45,grub2-1:2.12-58.fc44,shim-16.1-5", "versions": [ { "name": "grub-cc", "rpm_evr": "1:2.12-59.fc45" }, { "name": "grub2", "rpm_evr": "1:2.12-58.fc44" }, { "name": "shim", "rpm_evr": "16.1-5" } ] } ``` This allows us to have multiple bootloaders in a single image and install/update based on preference. During install/update, we check to bootloader and filter out any and all entries from the update metadata that do not match or are not required for the bootloader Signed-off-by: Pragyan Poudyal --- src/bios.rs | 28 +++++++++++----------- src/bootupd.rs | 60 +++++++++++++++++++++++----------------------- src/cli/bootupd.rs | 9 +------ src/component.rs | 44 +++++++++++++++++++++++----------- src/efi.rs | 35 ++++++++++++++------------- src/model.rs | 23 ++++++++++++++++++ 6 files changed, 116 insertions(+), 83 deletions(-) diff --git a/src/bios.rs b/src/bios.rs index 28a5b93c..19cdbcc5 100644 --- a/src/bios.rs +++ b/src/bios.rs @@ -116,14 +116,14 @@ impl Component for Bios { bootloader: Bootloader, ) -> Result { if !self.is_bootloader_supported(bootloader) { - anyhow::bail!("{bootloader} cannot be installed for bios"); + anyhow::bail!("{bootloader} cannot be installed for {}", self.name()); } let device = device.ok_or_else(|| anyhow::anyhow!("BIOS component requires a target device"))?; let src_dir = Dir::open_ambient_dir(src_root, ambient_authority()) .with_context(|| format!("opening source directory {src_root}"))?; - let Some(meta) = get_component_update(&src_dir, self)? else { + let Some(meta) = get_component_update(&src_dir, self, Some(bootloader))? else { anyhow::bail!("No update metadata for component {} found", self.name()); }; @@ -137,15 +137,7 @@ impl Component for Bios { }) } - fn generate_update_metadata( - &self, - sysroot_path: &str, - bootloader: Bootloader, - ) -> Result> { - if !self.is_bootloader_supported(bootloader) { - anyhow::bail!("{bootloader} cannot be installed for bios"); - } - + fn generate_update_metadata(&self, sysroot_path: &str) -> Result> { let grub_install = Path::new(sysroot_path).join(GRUB_BIN); if !grub_install.exists() { println!("Failed to find {:?}", grub_install); @@ -263,8 +255,16 @@ impl Component for Bios { })) } - fn query_update(&self, sysroot: &Dir) -> Result> { - get_component_update(sysroot, self) + fn query_update( + &self, + sysroot: &Dir, + bootloader: Bootloader, + ) -> Result> { + if !self.is_bootloader_supported(bootloader) { + return Ok(None); + } + + get_component_update(sysroot, self, Some(bootloader)) } fn query_requires_update(&self, sysroot: &Dir) -> Result<()> { @@ -277,7 +277,7 @@ impl Component for Bios { fn run_update(&self, rootcxt: &RootContext, _: &InstalledContent) -> Result { let updatemeta = self - .query_update(&rootcxt.sysroot)? + .query_update(&rootcxt.sysroot, Bootloader::Grub)? .expect("update available"); for parent in rootcxt.device.find_all_roots()? { diff --git a/src/bootupd.rs b/src/bootupd.rs index 3814af97..40c998be 100644 --- a/src/bootupd.rs +++ b/src/bootupd.rs @@ -19,7 +19,6 @@ use crate::freezethaw::fsfreeze_thaw_cycle; ))] use crate::grubconfigs::{ensure_grub_permissions, GRUB2DIR}; use crate::model::{ComponentStatus, ComponentUpdatable, ContentMetadata, SavedState, Status}; -use crate::util::running_in_container; use crate::{ostreeutil, util}; use anyhow::{anyhow, Context, Result}; use camino::{Utf8Path, Utf8PathBuf}; @@ -54,6 +53,7 @@ impl ConfigMode { } } +#[allow(clippy::too_many_arguments)] pub(crate) fn install( source_root: &str, dest_root: &str, @@ -105,18 +105,21 @@ pub(crate) fn install( continue; } - // skip components that don't have an update metadata - if component.query_update(&source_root_dir)?.is_none() { + if !component.is_bootloader_supported(bootloader) { println!( - "Skip installing component {} without update metadata", + "Skip installing component {} as it does not support bootloader {bootloader}", component.name() ); continue; } - if !component.is_bootloader_supported(bootloader) { + // skip components that don't have an update metadata + if component + .query_update(&source_root_dir, bootloader)? + .is_none() + { println!( - "Skip installing component {} as it does not support bootloader {bootloader}", + "Skip installing component {} without update metadata", component.name() ); continue; @@ -283,22 +286,15 @@ pub(crate) fn get_components() -> Components { get_components_impl(false) } -pub(crate) fn generate_update_metadata(sysroot_path: &str, bootloader: Bootloader) -> Result<()> { +pub(crate) fn generate_update_metadata(sysroot_path: &str) -> Result<()> { // create bootupd update dir which will save component metadata files for both components let updates_dir = Path::new(sysroot_path).join(crate::model::BOOTUPD_UPDATES_DIR); + std::fs::create_dir_all(&updates_dir) .with_context(|| format!("Failed to create updates dir {:?}", &updates_dir))?; - for component in get_components().values() { - if !component.is_bootloader_supported(bootloader) { - println!( - "Bootloader {bootloader} not supported for {}. Skipping metadata generation", - component.name(), - ); - - continue; - } - if let Some(v) = component.generate_update_metadata(sysroot_path, bootloader)? { + for component in get_components().values() { + if let Some(v) = component.generate_update_metadata(sysroot_path)? { println!( "Generated update layout for {}: {}", component.name(), @@ -343,7 +339,7 @@ pub(crate) fn update(name: &str, rootcxt: &RootContext) -> Result match inst.meta.can_upgrade_to(p) { std::cmp::Ordering::Less => p, // current < available -> upgrade @@ -364,10 +360,12 @@ pub(crate) fn update(name: &str, rootcxt: &RootContext) -> Result Result { pub(crate) fn status() -> Result { let mut ret: Status = Default::default(); let mut known_components = get_components(); - let sysroot = Dir::open_ambient_dir("/", ambient_authority())?; + let root = Dir::open_ambient_dir("/", ambient_authority())?; let bootloader = get_bootloader()?; let state = SavedState::load_from_disk("/", Some(bootloader))?; @@ -499,7 +499,7 @@ pub(crate) fn status() -> Result { .ok_or_else(|| anyhow!("Unknown component installed: {}", name))?; let component = component.as_ref(); let interrupted = state.pending.as_ref().and_then(|p| p.get(name.as_str())); - let update = component.query_update(&sysroot)?; + let update = component.query_update(&root, bootloader)?; let updatable = ComponentUpdatable::from_metadata(&ic.meta, update.as_ref()); let adopted_from = ic.adopted_from.clone(); ret.components.insert( @@ -538,7 +538,7 @@ pub(crate) fn status() -> Result { if let Some(adopt_ver) = component::query_adopt_state()? { let component = component::new_from_name(&name)?; // Skip if the update metadata could not be found - if component.query_update(&sysroot)?.is_none() { + if component.query_update(&root, bootloader)?.is_none() { continue; }; ret.adoptable.insert(name.to_string(), adopt_ver); diff --git a/src/cli/bootupd.rs b/src/cli/bootupd.rs index f379bdf0..e96bd098 100644 --- a/src/cli/bootupd.rs +++ b/src/cli/bootupd.rs @@ -96,12 +96,6 @@ pub struct GenerateOpts { /// Physical root mountpoint #[clap(value_parser)] sysroot: Option, - - /// The bootloader to generate metadata for - // - // We have a default to not break older systems - #[clap(long, default_value_t = Bootloader::Grub)] - bootloader: Bootloader, } impl DCommand { @@ -119,8 +113,7 @@ impl DCommand { if sysroot != "/" { anyhow::bail!("Using a non-default sysroot is not supported: {}", sysroot); } - bootupd::generate_update_metadata(sysroot, opts.bootloader) - .context("generating metadata failed")?; + bootupd::generate_update_metadata(sysroot).context("generating metadata failed")?; Ok(()) } diff --git a/src/component.rs b/src/component.rs index ea9df238..a6fad7e3 100644 --- a/src/component.rs +++ b/src/component.rs @@ -71,15 +71,15 @@ pub(crate) trait Component { /// this is an `rpm-ostree compose tree` for example. For a dual-partition /// style updater, this would be run as part of a postprocessing step /// while the filesystem for the partition is mounted. - fn generate_update_metadata( + fn generate_update_metadata(&self, sysroot: &str) -> Result>; + + /// Used on the client to query for an update cached in the current booted OS. + fn query_update( &self, - sysroot: &str, + sysroot: &Dir, bootloader: Bootloader, ) -> Result>; - /// Used on the client to query for an update cached in the current booted OS. - fn query_update(&self, sysroot: &Dir) -> Result>; - /// This is called in the update code if query_update() returned no metadata. /// It should return an error if the current booted system should expect some /// metadata for this component. @@ -167,21 +167,37 @@ pub(crate) fn write_update_metadata( } /// Given a component, return metadata on the available update (if any) +// +/// If bootloader is Some, all metadata not pertaining to the specified bootloader +/// is filtered +/// +/// If bootloader is None, no filtering is performed #[context("Loading update for component {}", component.name())] pub(crate) fn get_component_update( sysroot: &Dir, component: &dyn Component, + bootloader: Option, ) -> Result> { let name = component_update_data_name(component); - let path = Path::new(BOOTUPD_UPDATES_DIR).join(name); - if let Some(f) = sysroot.open_optional(&path)? { - let mut f = std::io::BufReader::new(f); - let u = serde_json::from_reader(&mut f) - .with_context(|| format!("failed to parse {:?}", &path))?; - Ok(Some(u)) - } else { - Ok(None) - } + let path = Path::new(BOOTUPD_UPDATES_DIR).join(&name); + + let Some(f) = sysroot.open_optional(&path)? else { + return Ok(None); + }; + + let mut f = std::io::BufReader::new(f); + let mut u = + serde_json::from_reader(&mut f).with_context(|| format!("failed to parse {:?}", &path))?; + + let Some(bootloader) = bootloader else { + return Ok(Some(u)); + }; + + // We store metadata of all bootloaders present in the image + // So here, we will now filter out the bootloaders + u.filter_bootloader(bootloader); + + Ok(Some(u)) } #[context("Querying adoptable state")] diff --git a/src/efi.rs b/src/efi.rs index f5661c8f..5cc035ee 100644 --- a/src/efi.rs +++ b/src/efi.rs @@ -24,7 +24,7 @@ use widestring::U16CString; use bootc_internal_blockdev::Device; -use crate::bootloader::Bootloader; +use crate::bootloader::{get_bootloader, Bootloader}; use crate::bootupd::RootContext; use crate::freezethaw::fsfreeze_thaw_cycle; use crate::model::*; @@ -450,7 +450,7 @@ impl Component for Efi { ) -> Result { let src_dir = Dir::open_ambient_dir(src_root, ambient_authority()) .with_context(|| format!("opening source directory {src_root}"))?; - let Some(meta) = get_component_update(&src_dir, self)? else { + let Some(meta) = get_component_update(&src_dir, self, Some(bootloader))? else { anyhow::bail!("No update metadata for component {} found", self.name()); }; log::debug!("Found metadata {}", meta.version); @@ -534,7 +534,9 @@ impl Component for Efi { .as_ref() .ok_or_else(|| anyhow::anyhow!("No filetree for installed EFI found!"))?; let sysroot_dir = &rootcxt.sysroot; - let updatemeta = self.query_update(sysroot_dir)?.expect("update available"); + let updatemeta = self + .query_update(sysroot_dir, get_bootloader()?)? + .expect("update available"); let updated_path = { let efilib_path = rootcxt.path.join(EFILIB); if efilib_path.exists() @@ -581,11 +583,7 @@ impl Component for Efi { }) } - fn generate_update_metadata( - &self, - sysroot: &str, - bootloader: Bootloader, - ) -> Result> { + fn generate_update_metadata(&self, sysroot: &str) -> Result> { let sysroot_path = Utf8Path::new(sysroot); let sysroot_dir = Dir::open_ambient_dir(sysroot_path, cap_std::ambient_authority())?; @@ -597,7 +595,7 @@ impl Component for Efi { // have them in /usr/lib/ostree-boot, which we move to /usr/lib/bootupd/updates/EFI let metadata = if sysroot_path.join(EFILIB).exists() { println!("Generating metadata from {EFILIB}"); - generate_meta_from_usr_efi(sysroot_path, bootloader)? + generate_meta_from_usr_efi(sysroot_path)? } else { match &ostreeboot { Some(..) => { @@ -631,8 +629,12 @@ impl Component for Efi { Ok(Some(metadata)) } - fn query_update(&self, sysroot: &Dir) -> Result> { - get_component_update(sysroot, self) + fn query_update( + &self, + sysroot: &Dir, + bootloader: Bootloader, + ) -> Result> { + get_component_update(sysroot, self, Some(bootloader)) } fn query_requires_update(&self, _sysroot: &Dir) -> Result<()> { @@ -827,12 +829,11 @@ fn find_file_recursive>(dir: P, target_file: &str) -> Result Result { - let Some(efi_components) = get_efi_component_from_usr(sysroot_path, EFILIB, Some(bootloader))? - else { +fn generate_meta_from_usr_efi(sysroot_path: &Utf8Path) -> Result { + // We DO NOT want to filter while generating metadata + // We want to have metadata for multiple bootloaders + // Later on, while installing, we'll filter out the stuff we don't need + let Some(efi_components) = get_efi_component_from_usr(sysroot_path, EFILIB, None)? else { anyhow::bail!("Failed to find EFI components"); }; diff --git a/src/model.rs b/src/model.rs index e899a015..31fe6cad 100644 --- a/src/model.rs +++ b/src/model.rs @@ -9,6 +9,7 @@ use serde::{Deserialize, Serialize}; use std::cmp::Ordering; use std::collections::BTreeMap; +use crate::bootloader::Bootloader; use crate::packagesystem::*; /// The directory where updates are stored @@ -33,6 +34,28 @@ impl ContentMetadata { compare_package_versions(&self.version, &target.version) } } + + pub(crate) fn filter_bootloader(&mut self, bootloader: Bootloader) { + let to_remove = Bootloader::iter() + .filter(|b| *b != bootloader) + .map(|b| b.efi_component_name()) + .collect::>(); + + // Version is of type "-,-" + self.version = self + .version + .split(",") + .filter(|v| { + // Keep everything that is NOT in to_remove + !to_remove.iter().any(|b| v.starts_with(b)) + }) + .collect::>() + .join(","); + + if let Some(versions) = &mut self.versions { + versions.retain(|v| !to_remove.contains(&v.name.as_str())); + } + } } #[derive(Serialize, Deserialize, Clone, Debug)] From 9bdfa592876617eaeaca91c37b7899c7848cc1bc Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Fri, 12 Jun 2026 19:11:22 +0530 Subject: [PATCH 09/21] Fix builds for powerpc and s390x Only allow Grub as the bootloader for powerpc and s390x Signed-off-by: Pragyan Poudyal --- src/backend/statefile.rs | 67 +++++++++++++++++++++++++++++++++------- src/bootloader.rs | 11 +++++-- src/cli/bootupd.rs | 6 ++++ 3 files changed, 70 insertions(+), 14 deletions(-) diff --git a/src/backend/statefile.rs b/src/backend/statefile.rs index fc2339b6..c7cbce5d 100644 --- a/src/backend/statefile.rs +++ b/src/backend/statefile.rs @@ -2,7 +2,6 @@ use crate::bootloader::Bootloader; use crate::bootupd::list_dev_current_root; -use crate::efi::Efi; use crate::freezethaw::fsfreeze_thaw_cycle; use crate::model::SavedState; use crate::util::SignalTerminationGuard; @@ -133,7 +132,24 @@ impl SavedState { } } + #[cfg(any(target_arch = "powerpc64", target_arch = "s390x"))] Bootloader::GrubCC => { + let arch = if cfg!(target_arch = "powerpc64") { + "powerpc64" + } else { + "s390x" + }; + + anyhow::bail!("Only Grub is supported for {arch}"); + } + + #[cfg(any( + target_arch = "x86_64", + target_arch = "aarch64", + target_arch = "riscv64" + ))] + Bootloader::GrubCC => { + use crate::efi::Efi; let efi = Efi::default(); let device = get_parent_device(&root)?; @@ -202,27 +218,54 @@ pub(crate) struct StateLockGuard { } impl StateLockGuard { - /// Atomically replace the on-disk state with a new version. + fn write_grub_statefile(&self, state: &SavedState) -> Result<()> { + let subdir = self.sysroot.open_dir(SavedState::STATEFILE_DIR)?; + + subdir + .atomic_write_with_perms( + SavedState::STATEFILE_NAME, + serde_json::to_vec(state).context("Serializing state")?, + Permissions::from_mode(0o644), + ) + .context("Writing state file")?; + + return Ok(()); + } + + #[cfg(any(target_arch = "powerpc64", target_arch = "s390x"))] #[context("Updating state")] pub(crate) fn update_state( &mut self, state: &SavedState, bootloader: Bootloader, ) -> Result<()> { - if bootloader == Bootloader::Grub { - let subdir = self.sysroot.open_dir(SavedState::STATEFILE_DIR)?; + let arch = if cfg!(target_arch = "powerpc64") { + "powerpc64" + } else { + "s390x" + }; - subdir - .atomic_write_with_perms( - SavedState::STATEFILE_NAME, - serde_json::to_vec(state).context("Serializing state")?, - Permissions::from_mode(0o644), - ) - .context("Writing state file")?; + if bootloader != Bootloader::Grub { + anyhow::bail!("Found bootloader: {bootloader}. Only Grub is supported for {arch}"); + } - return Ok(()); + self.write_grub_statefile(state) + } + + /// Atomically replace the on-disk state with a new version. + #[cfg(not(any(target_arch = "powerpc64", target_arch = "s390x")))] + #[context("Updating state")] + pub(crate) fn update_state( + &mut self, + state: &SavedState, + bootloader: Bootloader, + ) -> Result<()> { + if bootloader == Bootloader::Grub { + return self.write_grub_statefile(state); } + use crate::efi::Efi; + let device = get_parent_device(&self.sysroot)?; let all_esps = device .find_colocated_esps() diff --git a/src/bootloader.rs b/src/bootloader.rs index 7b2d3fbf..6ec8d4d7 100644 --- a/src/bootloader.rs +++ b/src/bootloader.rs @@ -2,8 +2,6 @@ use anyhow::Result; use fn_error_context::context; use std::{fmt::Display, sync::OnceLock}; -use crate::efi::get_loader_info; - #[derive(Debug, Default, Copy, Clone, clap::ValueEnum, PartialEq, Eq)] pub enum Bootloader { #[default] @@ -48,8 +46,17 @@ impl Bootloader { } } +#[cfg(any(target_arch = "powerpc64", target_arch = "s390x"))] +#[context("Getting bootloader")] +pub(crate) fn get_bootloader() -> Result { + Ok(Bootloader::Grub) +} + +#[cfg(not(any(target_arch = "powerpc64", target_arch = "s390x")))] #[context("Getting bootloader")] pub(crate) fn get_bootloader() -> Result { + use crate::efi::get_loader_info; + static BOOTLOADER: OnceLock = OnceLock::new(); if let Some(bootloader) = BOOTLOADER.get() { diff --git a/src/cli/bootupd.rs b/src/cli/bootupd.rs index e96bd098..73050087 100644 --- a/src/cli/bootupd.rs +++ b/src/cli/bootupd.rs @@ -119,6 +119,12 @@ impl DCommand { /// Runner for `install` verb. pub(crate) fn run_install(opts: InstallOpts) -> Result<()> { + if opts.bootloader != Bootloader::Grub + && cfg!(any(target_arch = "powerpc64", target_arch = "s390x")) + { + anyhow::bail!("Only Grub is supported for powerpc64 and s390x"); + } + let configmode = if opts.write_uuid { ConfigMode::WithUUID } else if opts.with_static_configs { From b4019aa08c318df5d2d958a5d679ce5a9e04d873 Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Fri, 19 Jun 2026 14:56:20 +0530 Subject: [PATCH 10/21] component: Reduce use of `dyn Component` Move functions `get_component_update` and `component_update_data_name` inside Component trait with default implementations. This reduces use of `dyn Component` which is much cleaner and allows us to have default implementations on the trait Signed-off-by: Pragyan Poudyal --- src/bios.rs | 6 ++-- src/component.rs | 85 ++++++++++++++++++++++++------------------------ src/efi.rs | 10 ++++-- 3 files changed, 52 insertions(+), 49 deletions(-) diff --git a/src/bios.rs b/src/bios.rs index 19cdbcc5..1ea5593c 100644 --- a/src/bios.rs +++ b/src/bios.rs @@ -123,7 +123,7 @@ impl Component for Bios { device.ok_or_else(|| anyhow::anyhow!("BIOS component requires a target device"))?; let src_dir = Dir::open_ambient_dir(src_root, ambient_authority()) .with_context(|| format!("opening source directory {src_root}"))?; - let Some(meta) = get_component_update(&src_dir, self, Some(bootloader))? else { + let Some(meta) = self.get_component_update(&src_dir, Some(bootloader))? else { anyhow::bail!("No update metadata for component {} found", self.name()); }; @@ -146,7 +146,7 @@ impl Component for Bios { // Query the rpm database and list the package and build times for /usr/sbin/grub2-install let meta = packagesystem::query_files(sysroot_path, [&grub_install])?; - write_update_metadata(sysroot_path, self, &meta)?; + write_update_metadata(sysroot_path, self.component_update_data_name(), &meta)?; Ok(Some(meta)) } @@ -264,7 +264,7 @@ impl Component for Bios { return Ok(None); } - get_component_update(sysroot, self, Some(bootloader)) + self.get_component_update(sysroot, Some(bootloader)) } fn query_requires_update(&self, sysroot: &Dir) -> Result<()> { diff --git a/src/component.rs b/src/component.rs index a6fad7e3..376ee8d5 100644 --- a/src/component.rs +++ b/src/component.rs @@ -99,6 +99,46 @@ pub(crate) trait Component { fn get_efi_vendor(&self, sysroot: &Path) -> Result>; fn is_bootloader_supported(&self, bootloader: Bootloader) -> bool; + + /// Given a component, return metadata on the available update (if any) + // + /// If bootloader is Some, all metadata not pertaining to the specified bootloader + /// is filtered + /// + /// If bootloader is None, no filtering is performed + #[context("Loading update for component {}", self.name())] + fn get_component_update( + &self, + sysroot: &Dir, + bootloader: Option, + ) -> Result> { + let name = self.component_update_data_name(); + let path = Path::new(BOOTUPD_UPDATES_DIR).join(&name); + + let Some(f) = sysroot.open_optional(&path)? else { + return Ok(None); + }; + + let mut f = std::io::BufReader::new(f); + let mut u = serde_json::from_reader(&mut f) + .with_context(|| format!("failed to parse {:?}", &path))?; + + let Some(bootloader) = bootloader else { + return Ok(Some(u)); + }; + + // We store metadata of all bootloaders present in the image + // So here, we will now filter out the bootloaders + u.filter_bootloader(bootloader); + + Ok(Some(u)) + } + + /// Returns the name of the JSON file containing a component's available update metadata installed + /// into the booted operating system root. + fn component_update_data_name(&self) -> PathBuf { + Path::new(&format!("{}.json", self.name())).into() + } } /// Given a component name, create an implementation. @@ -141,24 +181,17 @@ pub(crate) fn component_updatedir(sysroot: &str, component: &dyn Component) -> P Path::new(sysroot).join(component_updatedirname(component)) } -/// Returns the name of the JSON file containing a component's available update metadata installed -/// into the booted operating system root. -fn component_update_data_name(component: &dyn Component) -> PathBuf { - Path::new(&format!("{}.json", component.name())).into() -} - /// Helper method for writing an update file pub(crate) fn write_update_metadata( sysroot: &str, - component: &dyn Component, + file_path: PathBuf, meta: &ContentMetadata, ) -> Result<()> { let sysroot = Dir::open_ambient_dir(sysroot, ambient_authority())?; let dir = sysroot.open_dir(BOOTUPD_UPDATES_DIR)?; - let name = component_update_data_name(component); dir.atomic_write_with_perms( - name, + file_path, serde_json::to_vec(&meta).context("Serializing metadata")?, Permissions::from_mode(0o644), )?; @@ -166,40 +199,6 @@ pub(crate) fn write_update_metadata( Ok(()) } -/// Given a component, return metadata on the available update (if any) -// -/// If bootloader is Some, all metadata not pertaining to the specified bootloader -/// is filtered -/// -/// If bootloader is None, no filtering is performed -#[context("Loading update for component {}", component.name())] -pub(crate) fn get_component_update( - sysroot: &Dir, - component: &dyn Component, - bootloader: Option, -) -> Result> { - let name = component_update_data_name(component); - let path = Path::new(BOOTUPD_UPDATES_DIR).join(&name); - - let Some(f) = sysroot.open_optional(&path)? else { - return Ok(None); - }; - - let mut f = std::io::BufReader::new(f); - let mut u = - serde_json::from_reader(&mut f).with_context(|| format!("failed to parse {:?}", &path))?; - - let Some(bootloader) = bootloader else { - return Ok(Some(u)); - }; - - // We store metadata of all bootloaders present in the image - // So here, we will now filter out the bootloaders - u.filter_bootloader(bootloader); - - Ok(Some(u)) -} - #[context("Querying adoptable state")] pub(crate) fn query_adopt_state() -> Result> { // This would be extended with support for other operating systems later diff --git a/src/efi.rs b/src/efi.rs index 5cc035ee..cbdd7e8a 100644 --- a/src/efi.rs +++ b/src/efi.rs @@ -450,7 +450,7 @@ impl Component for Efi { ) -> Result { let src_dir = Dir::open_ambient_dir(src_root, ambient_authority()) .with_context(|| format!("opening source directory {src_root}"))?; - let Some(meta) = get_component_update(&src_dir, self, Some(bootloader))? else { + let Some(meta) = self.get_component_update(&src_dir, Some(bootloader))? else { anyhow::bail!("No update metadata for component {} found", self.name()); }; log::debug!("Found metadata {}", meta.version); @@ -624,7 +624,11 @@ impl Component for Efi { ostreeboot.remove_all_optional("efi/EFI")?; }; - write_update_metadata(sysroot_path.as_str(), self, &metadata)?; + write_update_metadata( + sysroot_path.as_str(), + self.component_update_data_name(), + &metadata, + )?; Ok(Some(metadata)) } @@ -634,7 +638,7 @@ impl Component for Efi { sysroot: &Dir, bootloader: Bootloader, ) -> Result> { - get_component_update(sysroot, self, Some(bootloader)) + self.get_component_update(sysroot, Some(bootloader)) } fn query_requires_update(&self, _sysroot: &Dir) -> Result<()> { From 6a0f8ba0337b8f85bc268d1b45b8558d6e0fe36e Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Fri, 19 Jun 2026 15:12:58 +0530 Subject: [PATCH 11/21] metadata: Set default bootloader Add a cmd to set default bootloader after generating content metadata. This is the bootloader we'll use at install time if no bootloader option is provided. ContentMetadata now has an `defautl_bootloader: Option` field Signed-off-by: Pragyan Poudyal --- src/bootloader.rs | 5 ++- src/bootupd.rs | 1 + src/cli/bootupctl.rs | 5 +++ src/cli/bootupd.rs | 32 ++++++++++++++ src/cli/mod.rs | 2 +- src/component.rs | 103 +++++++++++++++++++++++++++++++++++++++++++ src/efi.rs | 1 + src/model.rs | 13 ++++++ src/model_legacy.rs | 1 + src/packagesystem.rs | 1 + 10 files changed, 162 insertions(+), 2 deletions(-) diff --git a/src/bootloader.rs b/src/bootloader.rs index 6ec8d4d7..1cd12af5 100644 --- a/src/bootloader.rs +++ b/src/bootloader.rs @@ -1,8 +1,11 @@ use anyhow::Result; use fn_error_context::context; +use serde::{Deserialize, Serialize}; use std::{fmt::Display, sync::OnceLock}; -#[derive(Debug, Default, Copy, Clone, clap::ValueEnum, PartialEq, Eq)] +#[derive( + Debug, Default, Copy, Clone, clap::ValueEnum, PartialEq, Eq, Hash, Serialize, Deserialize, +)] pub enum Bootloader { #[default] Grub, diff --git a/src/bootupd.rs b/src/bootupd.rs index 40c998be..40fe3d53 100644 --- a/src/bootupd.rs +++ b/src/bootupd.rs @@ -235,6 +235,7 @@ fn get_static_config_meta() -> Result { timestamp: self_bin_meta.modified()?.into(), version: crate_version!().into(), versions: None, + default_bootloader: None, }; Ok(self_meta) } diff --git a/src/cli/bootupctl.rs b/src/cli/bootupctl.rs index 1c30b24e..7e045032 100644 --- a/src/cli/bootupctl.rs +++ b/src/cli/bootupctl.rs @@ -73,6 +73,8 @@ pub enum CtlBackend { Generate(super::bootupd::GenerateOpts), #[clap(name = "install", hide = true)] Install(super::bootupd::InstallOpts), + #[clap(hide = true)] + SetDefaultBootloader(super::bootupd::DefaultBootloaderOpts), } #[derive(Debug, Parser)] @@ -109,6 +111,9 @@ impl CtlCommand { CtlVerb::Backend(CtlBackend::Install(opts)) => { super::bootupd::DCommand::run_install(opts) } + CtlVerb::Backend(CtlBackend::SetDefaultBootloader(opts)) => { + super::bootupd::DCommand::set_default_bootloader(opts) + } CtlVerb::MigrateStaticGrubConfig => Self::run_migrate_static_grub_config(), } } diff --git a/src/cli/bootupd.rs b/src/cli/bootupd.rs index 73050087..623bfb26 100644 --- a/src/cli/bootupd.rs +++ b/src/cli/bootupd.rs @@ -40,6 +40,7 @@ pub enum DVerb { GenerateUpdateMetadata(GenerateOpts), #[clap(name = "install", about = "Install components")] Install(InstallOpts), + SetDefaultBootloader(DefaultBootloaderOpts), } #[derive(Debug, Parser)] @@ -98,12 +99,22 @@ pub struct GenerateOpts { sysroot: Option, } +#[derive(Debug, Parser)] +pub struct DefaultBootloaderOpts { + /// Physical root mountpoint + #[clap(long)] + pub(crate) sysroot: Option, + /// The bootloader to be set as the default + pub(crate) bootloader: Bootloader, +} + impl DCommand { /// Run CLI application. pub fn run(self) -> Result<()> { match self.cmd { DVerb::Install(opts) => Self::run_install(opts), DVerb::GenerateUpdateMetadata(opts) => Self::run_generate_meta(opts), + DVerb::SetDefaultBootloader(opts) => Self::set_default_bootloader(opts), } } @@ -160,4 +171,25 @@ impl DCommand { .context("boot data installation failed")?; Ok(()) } + + pub(crate) fn set_default_bootloader(opts: DefaultBootloaderOpts) -> Result<()> { + let all_components = crate::bootupd::get_components(); + let target_components: Vec<_> = all_components.values().collect(); + + for &component in target_components.iter() { + if !component.is_bootloader_supported(opts.bootloader) { + log::info!( + "{} is not supported for {}. Skipping...", + opts.bootloader, + component.name() + ); + + continue; + } + + component.set_default_bootloader(&opts)?; + } + + Ok(()) + } } diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 285e1ea5..02212b56 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -4,7 +4,7 @@ use anyhow::Result; use clap::Parser; use log::LevelFilter; mod bootupctl; -mod bootupd; +pub(crate) mod bootupd; /// Top-level multicall CLI. #[derive(Debug, Parser)] diff --git a/src/component.rs b/src/component.rs index 376ee8d5..9bd4703d 100644 --- a/src/component.rs +++ b/src/component.rs @@ -14,6 +14,7 @@ use std::path::{Path, PathBuf}; use bootc_internal_blockdev::Device; +use crate::cli::bootupd::DefaultBootloaderOpts; use crate::{bootloader::Bootloader, bootupd::RootContext, model::*}; #[derive(Serialize, Deserialize, Debug)] @@ -139,6 +140,32 @@ pub(crate) trait Component { fn component_update_data_name(&self) -> PathBuf { Path::new(&format!("{}.json", self.name())).into() } + + fn set_default_bootloader(&self, opts: &DefaultBootloaderOpts) -> Result<()> { + if !self.is_bootloader_supported(opts.bootloader) { + anyhow::bail!("{} not supported for {}", opts.bootloader, self.name()); + } + + let root_path = opts.sysroot.as_deref().unwrap_or("/"); + + let root = Dir::open_ambient_dir(root_path, ambient_authority()) + .with_context(|| format!("Opening {root_path}"))?; + + // This command expects bootupd.json to be present + let mut update_meta = self + .get_component_update(&root, None)? + .ok_or_else(|| anyhow::anyhow!("Expected to get update metadata"))?; + + if !update_meta.bootloader_available(opts.bootloader) { + anyhow::bail!("{} is not present in metadata", opts.bootloader); + } + + update_meta.default_bootloader = Some(opts.bootloader); + + write_update_metadata(root_path, self.component_update_data_name(), &update_meta)?; + + Ok(()) + } } /// Given a component name, create an implementation. @@ -207,6 +234,7 @@ pub(crate) fn query_adopt_state() -> Result> { timestamp: coreos_aleph.ts, version: coreos_aleph.aleph.version, versions: None, + default_bootloader: None, }; log::trace!("Adoptable: {:?}", &meta); return Ok(Some(Adoptable { @@ -224,6 +252,7 @@ pub(crate) fn query_adopt_state() -> Result> { timestamp, version: "unknown".to_string(), versions: None, + default_bootloader: None, }; return Ok(Some(Adoptable { version: meta, @@ -236,6 +265,7 @@ pub(crate) fn query_adopt_state() -> Result> { #[cfg(test)] mod tests { use cap_std::fs::{DirBuilder, DirBuilderExt, Permissions, PermissionsExt}; + use chrono::Utc; use super::*; @@ -313,4 +343,77 @@ mod tests { } Ok(()) } + + #[test] + fn test_set_default_bootloader() -> Result<()> { + let td = tempfile::tempdir()?; + let sysroot = td.path().to_str().unwrap().to_string(); + let tdir = Dir::open_ambient_dir(&sysroot, ambient_authority())?; + + // Create the updates directory + let mut dir_builder = DirBuilder::new(); + dir_builder.mode(0o755); + dir_builder.recursive(true); + tdir.create_dir_with(BOOTUPD_UPDATES_DIR, &dir_builder)?; + + // Create test metadata without the target bootloader + let meta = ContentMetadata { + timestamp: Utc::now(), + version: "grub2-efi-x64-1:2.12-21.fc41.x86_64".into(), // Only has grub2-efi, not grub-cc + versions: None, + default_bootloader: None, + }; + + let all_components = crate::bootupd::get_components(); + let efi_component = all_components.get("EFI").unwrap(); + + // Write metadata file + write_update_metadata(&sysroot, efi_component.component_update_data_name(), &meta)?; + + let opts = DefaultBootloaderOpts { + sysroot: Some(sysroot.clone()), + bootloader: Bootloader::GrubCC, // This bootloader is not in the metadata version string + }; + + if efi_component.is_bootloader_supported(opts.bootloader) { + let result = efi_component.set_default_bootloader(&opts); + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("is not present in metadata")); + } + + // Now create test metadata with both bootloaders available + let meta = ContentMetadata { + timestamp: Utc::now(), + version: "grub2-efi-x64-1:2.12-21.fc41.x86_64,grub-cc-efi-x64-1:2.12-21.fc41.x86_64" + .into(), + versions: None, + default_bootloader: None, + }; + + // Write initial metadata file + write_update_metadata(&sysroot, efi_component.component_update_data_name(), &meta)?; + + let opts = DefaultBootloaderOpts { + sysroot: Some(sysroot.clone()), + bootloader: Bootloader::GrubCC, + }; + + if efi_component.is_bootloader_supported(opts.bootloader) { + // Should succeed + let result = efi_component.set_default_bootloader(&opts); + assert!(result.is_ok()); + + // Verify the metadata was updated + let root = Dir::open_ambient_dir(&sysroot, ambient_authority())?; + let updated_meta = efi_component.get_component_update(&root, None)?; + assert!(updated_meta.is_some()); + let updated_meta = updated_meta.unwrap(); + assert_eq!(updated_meta.default_bootloader, Some(Bootloader::GrubCC)); + } + + Ok(()) + } } diff --git a/src/efi.rs b/src/efi.rs index cbdd7e8a..13b0726c 100644 --- a/src/efi.rs +++ b/src/efi.rs @@ -856,6 +856,7 @@ fn generate_meta_from_usr_efi(sysroot_path: &Utf8Path) -> Result>, + /// The default bootloader to install if at install time no bootloader option is + /// provided + pub(crate) default_bootloader: Option, } impl ContentMetadata { @@ -35,6 +38,12 @@ impl ContentMetadata { } } + pub(crate) fn bootloader_available(&mut self, bootloader: Bootloader) -> bool { + self.version + .split(",") + .any(|v| v.starts_with(bootloader.efi_component_name())) + } + pub(crate) fn filter_bootloader(&mut self, bootloader: Bootloader) { let to_remove = Bootloader::iter() .filter(|b| *b != bootloader) @@ -160,11 +169,13 @@ mod test { timestamp: t, version: "grub2-efi-ia32-1:2.12-21.fc41.x86_64,grub2-efi-x64-1:2.12-21.fc41.x86_64,shim-ia32-15.8-3.x86_64,shim-x64-15.8-3.x86_64".into(), versions: None, + default_bootloader: None, }; let b = ContentMetadata { timestamp: t + Duration::try_seconds(1).unwrap(), version: "grub2-efi-ia32-1:2.12-28.fc41.x86_64,grub2-efi-x64-1:2.12-28.fc41.x86_64,shim-ia32-15.8-3.x86_64,shim-x64-15.8-3.x86_64".into(), versions: None, + default_bootloader: None, }; assert_eq!(a.can_upgrade_to(&b), Ordering::Less); // means upgradable assert_eq!(b.can_upgrade_to(&a), Ordering::Greater); @@ -183,6 +194,7 @@ mod test { rpm_evr: "15.8-3".into(), }, ]), + default_bootloader: None, }; let b = ContentMetadata { timestamp: t + Duration::try_seconds(1).unwrap(), @@ -197,6 +209,7 @@ mod test { rpm_evr: "15.8-3".into(), }, ]), + default_bootloader: None, }; assert_eq!(a.can_upgrade_to(&b), Ordering::Less); // means upgradable assert_eq!(b.can_upgrade_to(&a), Ordering::Greater); diff --git a/src/model_legacy.rs b/src/model_legacy.rs index a0991922..47b8cad0 100644 --- a/src/model_legacy.rs +++ b/src/model_legacy.rs @@ -50,6 +50,7 @@ impl ContentMetadata01 { timestamp, version: self.version, versions: None, + default_bootloader: None, } } } diff --git a/src/packagesystem.rs b/src/packagesystem.rs index f20e5e81..d566abb8 100644 --- a/src/packagesystem.rs +++ b/src/packagesystem.rs @@ -75,6 +75,7 @@ fn rpm_parse_metadata(stdout: &[u8]) -> Result { timestamp: **largest_timestamp, version, versions: Some(modules_vec), + default_bootloader: None, }) } From 615a6b3d07dc4d5db105e6ba8de3d740353e5c12 Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Mon, 22 Jun 2026 11:27:54 +0530 Subject: [PATCH 12/21] install: Make `--bootloader` argument optional Now that we can set default bootloader option, we don't require a bootloader to be passed in at install time as we can just get it from the set default Signed-off-by: Pragyan Poudyal --- src/bootloader.rs | 8 ++++ src/bootupd.rs | 111 +++++++++++++++++++++++++++++++++++---------- src/cli/bootupd.rs | 38 ++++++---------- src/component.rs | 8 ++++ src/model.rs | 15 ++++++ 5 files changed, 133 insertions(+), 47 deletions(-) diff --git a/src/bootloader.rs b/src/bootloader.rs index 1cd12af5..f4455a2a 100644 --- a/src/bootloader.rs +++ b/src/bootloader.rs @@ -47,6 +47,14 @@ impl Bootloader { Bootloader::GrubCC => "grub-cc", } } + + pub(crate) fn try_from_efi_component_name(component_name: &str) -> Result { + match component_name { + "grub2" => Ok(Self::Grub), + "grub-cc" => Ok(Self::GrubCC), + _ => anyhow::bail!("Not a valid bootloader: {component_name}"), + } + } } #[cfg(any(target_arch = "powerpc64", target_arch = "s390x"))] diff --git a/src/bootupd.rs b/src/bootupd.rs index 40fe3d53..a0ff7c8d 100644 --- a/src/bootupd.rs +++ b/src/bootupd.rs @@ -1,6 +1,7 @@ #[cfg(any(target_arch = "x86_64", target_arch = "powerpc64"))] use crate::bios; use crate::bootloader::{get_bootloader, Bootloader}; +use crate::cli::bootupd::InstallOpts; use crate::component; use crate::component::{Component, ValidationResult}; use crate::coreos; @@ -53,30 +54,30 @@ impl ConfigMode { } } -#[allow(clippy::too_many_arguments)] -pub(crate) fn install( - source_root: &str, - dest_root: &str, - devices: &[Device], - configs: ConfigMode, - update_firmware: bool, - target_components: Option<&[String]>, - auto_components: bool, - bootloader: Bootloader, -) -> Result<()> { - let source_root_dir = - Dir::open_ambient_dir(source_root, ambient_authority()).context("Opening source root")?; - SavedState::ensure_not_present(dest_root, bootloader) - .context("failed to install, invalid re-install attempted")?; +pub(crate) fn install(opts: &InstallOpts, devices: &[Device], configs: ConfigMode) -> Result<()> { + // SavedState needs to be per component + // Consider this scenario: + // - Grub installed (statefile in /sysroot/boot) + // - Re-install attempted with GrubCC + // + // So we can't just check statefile based on the determined bootloader + // We need to check all cases + for b in Bootloader::iter() { + SavedState::ensure_not_present(&opts.dest_root, b) + .context("failed to install, invalid re-install attempted")?; + } + + let source_root_dir = Dir::open_ambient_dir(&opts.src_root, ambient_authority()) + .context("Opening source root")?; - let all_components = get_components_impl(auto_components); + let all_components = get_components_impl(opts.auto); if all_components.is_empty() { println!("No components available for this platform."); return Ok(()); } - let target_components = if let Some(target_components) = target_components { + let target_components = if let Some(target_components) = &opts.components { // Checked by CLI parser - assert!(!auto_components); + assert!(!opts.auto); target_components .iter() .map(|name| { @@ -89,12 +90,70 @@ pub(crate) fn install( all_components.values().collect() }; - if target_components.is_empty() && !auto_components { + if target_components.is_empty() && !opts.auto { anyhow::bail!("No components specified"); } + let bootloader = match opts.bootloader { + // CLI overrides anything else + Some(b) => b, + None => { + let mut efi_default = None; + let mut efi_component_update = None; + let mut bios_default = None; + + for c in &target_components { + if c.name() == "EFI" { + efi_default = c.get_default_bootloader(&source_root_dir)?; + + if efi_default.is_none() { + // We don't want to filter any bootloader + efi_component_update = c.get_component_update(&source_root_dir, None)?; + } + } + + if c.name() == "BIOS" { + bios_default = c.get_default_bootloader(&source_root_dir)?; + } + } + + match (bios_default, efi_default) { + // EFI bootloader takes precedence + // Take the following example + // - BIOS default = Grub (always) + // - EFI default = GrubCC + // + // We can't install GrubCC for BIOS as it's not supported + // So we just default to installing GrubCC + (Some(_), Some(eb)) => eb, + (None, Some(eb)) => eb, + (Some(bb), None) => bb, + + // We still can get the bootloader by reading in the EFI component update + // If there's only ONE bootloader in the metadata, then that's the one to + // be installed + (None, None) => { + let Some(efi_component_update) = efi_component_update else { + anyhow::bail!("Could not determine bootloader. Default bootloader not set") + }; + + let available_bootloaders = efi_component_update.num_bootloader_available(); + + if available_bootloaders.len() != 1 { + anyhow::bail!( + "Could not determine bootloader. Default bootloader not set. Multiple bootloaders found as install candidates" + ) + } + + available_bootloaders[0] + } + } + } + }; + let mut state = SavedState::default(); let mut installed_efi_vendor = None; + for &component in target_components.iter() { // skip for BIOS if no devices specified if component.name() == "BIOS" && devices.is_empty() { @@ -162,7 +221,13 @@ pub(crate) fn install( for device in &devices_to_install { let device_desc = device.map_or("(auto)".to_string(), |d| d.path()); let meta = component - .install(source_root, dest_root, *device, update_firmware, bootloader) + .install( + &opts.src_root, + &opts.dest_root, + *device, + opts.update_firmware, + bootloader, + ) .with_context(|| { format!( "installing component {} to device {}", @@ -185,13 +250,13 @@ pub(crate) fn install( } // Yes this is a hack...the Component thing just turns out to be too generic. if installed_efi_vendor.is_none() { - if let Some(vendor) = component.get_efi_vendor(Path::new(source_root))? { + if let Some(vendor) = component.get_efi_vendor(Path::new(&opts.src_root))? { installed_efi_vendor = Some(vendor); } } } } - let sysroot = &Dir::open_ambient_dir(dest_root, ambient_authority())?; + let sysroot = &Dir::open_ambient_dir(&opts.dest_root, ambient_authority())?; #[cfg(any( target_arch = "x86_64", @@ -219,7 +284,7 @@ pub(crate) fn install( // Unmount the ESP, etc. drop(target_components); - let mut state_guard = SavedState::unlocked(dest_root.into(), sysroot.try_clone()?) + let mut state_guard = SavedState::unlocked(opts.dest_root.clone().into(), sysroot.try_clone()?) .context("failed to acquire write lock")?; state_guard .update_state(&state, bootloader) diff --git a/src/cli/bootupd.rs b/src/cli/bootupd.rs index 623bfb26..df87501e 100644 --- a/src/cli/bootupd.rs +++ b/src/cli/bootupd.rs @@ -44,52 +44,52 @@ pub enum DVerb { } #[derive(Debug, Parser)] -pub struct InstallOpts { +pub(crate) struct InstallOpts { /// Source root #[clap(long, value_parser, default_value_t = String::from("/"))] - src_root: String, + pub(crate) src_root: String, /// Target root #[clap(value_parser)] - dest_root: String, + pub(crate) dest_root: String, /// Target device(s) for bootloader installation. Can be specified multiple /// times to install to multiple devices (e.g., for multi-disk RAID/LVM setups). #[clap(long, action = clap::ArgAction::Append, conflicts_with = "filesystem")] - device: Vec, + pub(crate) device: Vec, /// Filesystem path to inspect for backing devices. Bootupd will walk up the /// device hierarchy to find physical disks and install to all ESPs found. #[clap(long)] - filesystem: Option, + pub(crate) filesystem: Option, /// Enable installation of the built-in static config files #[clap(long)] - with_static_configs: bool, + pub(crate) with_static_configs: bool, /// Implies `--with-static-configs`. When present, this also writes a /// file with the UUID of the target filesystems. #[clap(long)] - write_uuid: bool, + pub(crate) write_uuid: bool, /// On EFI systems, invoke `efibootmgr` to update the firmware. #[clap(long)] - update_firmware: bool, + pub(crate) update_firmware: bool, #[clap(long = "component", conflicts_with = "auto")] /// Only install these components - components: Option>, + pub(crate) components: Option>, /// Automatically choose components based on booted host state. /// /// For example on x86_64, if the host system is booted via EFI, /// then only enable installation to the ESP. #[clap(long)] - auto: bool, + pub(crate) auto: bool, /// The bootloader to use /// Defaults to Grub - #[clap(long, default_value_t = Bootloader::Grub)] - bootloader: Bootloader, + #[clap(long)] + pub(crate) bootloader: Option, } #[derive(Debug, Parser)] @@ -130,7 +130,7 @@ impl DCommand { /// Runner for `install` verb. pub(crate) fn run_install(opts: InstallOpts) -> Result<()> { - if opts.bootloader != Bootloader::Grub + if !matches!(opts.bootloader, Some(Bootloader::Grub) | None) && cfg!(any(target_arch = "powerpc64", target_arch = "s390x")) { anyhow::bail!("Only Grub is supported for powerpc64 and s390x"); @@ -158,17 +158,7 @@ impl DCommand { .collect::>>()? }; - bootupd::install( - &opts.src_root, - &opts.dest_root, - &devices, - configmode, - opts.update_firmware, - opts.components.as_deref(), - opts.auto, - opts.bootloader, - ) - .context("boot data installation failed")?; + bootupd::install(&opts, &devices, configmode).context("boot data installation failed")?; Ok(()) } diff --git a/src/component.rs b/src/component.rs index 9bd4703d..226269a4 100644 --- a/src/component.rs +++ b/src/component.rs @@ -166,6 +166,14 @@ pub(crate) trait Component { Ok(()) } + + fn get_default_bootloader(&self, root: &Dir) -> Result> { + let update_meta = self + .get_component_update(&root, None)? + .ok_or_else(|| anyhow::anyhow!("Expected to get update metadata"))?; + + Ok(update_meta.default_bootloader) + } } /// Given a component name, create an implementation. diff --git a/src/model.rs b/src/model.rs index 575795fd..48164a50 100644 --- a/src/model.rs +++ b/src/model.rs @@ -38,6 +38,21 @@ impl ContentMetadata { } } + /// Returns bootloaders are available for install + pub(crate) fn num_bootloader_available(&self) -> Vec { + let mut available = vec![]; + + if let Some(versions) = &self.versions { + for version in versions { + if let Ok(b) = Bootloader::try_from_efi_component_name(&version.name) { + available.push(b); + } + } + } + + return available; + } + pub(crate) fn bootloader_available(&mut self, bootloader: Bootloader) -> bool { self.version .split(",") From 2e05ff35496bd45f96c9640f86e8eb9cf2e4910b Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Mon, 22 Jun 2026 12:22:13 +0530 Subject: [PATCH 13/21] jenkinsfile: Don't install grub2-minimal Installation is failing with ``` 12:16:20 >>> Non-critical error in %posttrans scriptlet: grub2-common-1:2.12-61.fc44.noarch 12:16:20 >>> Scriptlet output: 12:16:20 >>> /var/tmp/rpm-tmp.JMDpbL: line 7: grub2-mkconfig: command not found 12:16:20 >>> 12:16:20 >>> [RPM] %posttrans(grub2-common-1:2.12-61.fc44.noarch) scriptlet failed, exit status 127 12:16:20 Transaction failed: Rpm transaction failed. ``` and I don't think we're using grub tools in the stage Signed-off-by: Pragyan Poudyal --- .cci.jenkinsfile | 1 - 1 file changed, 1 deletion(-) diff --git a/.cci.jenkinsfile b/.cci.jenkinsfile index 0f717f7a..6407d948 100644 --- a/.cci.jenkinsfile +++ b/.cci.jenkinsfile @@ -17,7 +17,6 @@ parallel build: { } stage("Unit tests") { shwrap(""" - dnf install -y grub2-tools-minimal cargo test --features rpm cargo test """) From fa2b4f050ee58507bd68648c09caeaac2f0cb1b5 Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Tue, 23 Jun 2026 09:34:30 +0530 Subject: [PATCH 14/21] Explicitly handle composefs root While searching for backing device for root, we were simply assuming error thrown by lsblk would be because of composefs and implicity looking for mountpoints for /sysroot and /boot underneath. We still have the same logic, but insted of relying on some unknown error, we explicitly check for a composefs mount Signed-off-by: Pragyan Poudyal --- Cargo.lock | 1 + Cargo.toml | 1 + src/backend/statefile.rs | 16 +++++++++------- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d3e4f035..1d8ebe35 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -170,6 +170,7 @@ version = "0.2.35" dependencies = [ "anyhow", "bootc-internal-blockdev", + "bootc-internal-mount", "bootc-internal-utils", "camino", "cap-std", diff --git a/Cargo.toml b/Cargo.toml index ee4b1ee4..b6e24721 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,6 +23,7 @@ path = "src/main.rs" anyhow = "1.0" bootc-internal-blockdev = "1.16.0" bootc-internal-utils = "1.16.0" +bootc-internal-mount = "1.16.0" cap-std-ext = "5.0.0" camino = "1.2.2" chrono = { version = "0.4.44", features = ["serde"] } diff --git a/src/backend/statefile.rs b/src/backend/statefile.rs index c7cbce5d..3106a5c0 100644 --- a/src/backend/statefile.rs +++ b/src/backend/statefile.rs @@ -51,14 +51,16 @@ fn parse_statefile(statusf: cap_std::fs::File) -> Result> { /// device from checking mount point from /boot or /sysroot #[context("Getting parent device")] fn get_parent_device(root: &Dir) -> Result { - match bootc_internal_blockdev::list_dev_by_dir(root) { - Ok(d) => Ok(d), - Err(e) => { - // Not really an error just yet - log::debug!("{e:?}"); - list_dev_current_root() - } + let root_fs = bootc_internal_mount::inspect_filesystem_of_dir(root) + .context("Inspecting root filesystem")?; + + if root_fs.fstype == "overlay" && root_fs.source.contains("composefs") { + // Root is mounted as overlay composefs, lsblk will throw an error + // Ergo, find backing device by looking at mountpoints for /sysroot | /boot + return list_dev_current_root(); } + + return bootc_internal_blockdev::list_dev_by_dir(root); } impl SavedState { From 14467cca47f5667eb3e701d0821e445dbb2272f2 Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Wed, 24 Jun 2026 11:01:25 +0530 Subject: [PATCH 15/21] ci: Test grubcc Create a new Dockerfile with base "fedora-bootc:44" for testing images with both grub and grub-cc present. Currently we don't have a bootc release that allows us to test with grub-cc, so we only test grub installations. This mainly tests backwards compatibility for the following cases - Only one bootloader present and no "default" in metadata - Two bootloaders present with grub as the "default" Signed-off-by: Pragyan Poudyal --- .github/workflows/ci.yml | 45 +++++++++++++---- Dockerfile.grubcc | 101 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 138 insertions(+), 8 deletions(-) create mode 100644 Dockerfile.grubcc diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 15ecf24c..38d0d628 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,16 +18,33 @@ env: CARGO_TERM_COLOR: always jobs: - c9s-bootc-e2e: + bootc-e2e: strategy: matrix: runner: - ubuntu-24.04 - ubuntu-24.04-arm + grubcc: + - 0 + - 1 runs-on: [ "${{ matrix.runner }}" ] steps: + - name: Setup env + run: | + IMG_NAME=localhost/bootupd-grubcc-${{ matrix.grubcc }} + echo "IMG_NAME=${IMG_NAME}" >> $GITHUB_ENV + + if [[ "${{ matrix.grubcc }}" == "1" ]]; then + # we only have grub-cc in feodra (for now) + OS_ID="fedora" + else + OS_ID="centos" + fi + + echo "OS_ID=${OS_ID}" >> $GITHUB_ENV + - name: Get a newer podman for heredoc support (from debian testing) run: | set -eux @@ -51,14 +68,26 @@ jobs: sudo apt update -y sudo apt install -y podman - - name: build - run: sudo podman build -t localhost/bootupd:latest -f Dockerfile . + - name: build with grubcc=${{ matrix.grubcc }} + run: | + dockerfile=Dockerfile + build_args=() + + if [[ "${{ matrix.grubcc }}" == "1" ]]; then + dockerfile=Dockerfile.grubcc + build_args=(--build-arg "grubcc=${{ matrix.grubcc }}") + fi + + sudo podman build "${build_args[@]}" . -t "$IMG_NAME" -f "$dockerfile" - name: bootupctl status in container run: | set -xeuo pipefail - sudo podman run --rm -v $PWD:/run/src -w /run/src --privileged localhost/bootupd:latest tests/tests/bootupctl-status-in-bootc.sh + sudo podman run --rm -v $PWD:/run/src -w /run/src --privileged "$IMG_NAME" tests/tests/bootupctl-status-in-bootc.sh + # Make sure grub stuff works with and without grubcc in the container image + # TODO: Test GrubCC installation with composefs after a bootc release includes + # https://github.com/bootc-dev/bootc/pull/2223 - name: bootc install to disk run: | set -xeuo pipefail @@ -66,7 +95,7 @@ jobs: sudo podman run --rm --privileged -v .:/target --pid=host --security-opt label=disable \ -v /var/lib/containers:/var/lib/containers \ -v /dev:/dev \ - localhost/bootupd:latest bootc install to-disk --skip-fetch-check \ + "$IMG_NAME" bootc install to-disk --filesystem ext4 --skip-fetch-check \ --disable-selinux --generic-image --via-loopback /target/myimage.raw # Verify we installed grub.cfg and shim on the disk sudo losetup -P -f myimage.raw @@ -80,7 +109,7 @@ jobs: # Assume aarch64 for now shim="shimaa64.efi" fi - sudo ls /mnt/EFI/centos/{grub.cfg,${shim}} + sudo ls /mnt/EFI/$OS_ID/{grub.cfg,${shim}} sudo umount /mnt # check /boot/grub2/grub.cfg permission root_part=$(sudo sfdisk -l -J "${device}" | jq -r '.partitiontable.partitions[] | select(.name == "root").node') @@ -99,7 +128,7 @@ jobs: set -xeuo pipefail sudo podman run --rm -ti --privileged -v /:/target --pid=host --security-opt label=disable \ -v /dev:/dev -v /var/lib/containers:/var/lib/containers \ - localhost/bootupd:latest env BOOTC_BOOTLOADER_DEBUG=1 \ + "$IMG_NAME" env BOOTC_BOOTLOADER_DEBUG=1 \ bootc install to-filesystem --skip-fetch-check \ --acknowledge-destructive \ --disable-selinux --replace=alongside /target @@ -113,4 +142,4 @@ jobs: - name: bootupctl generate-update-metadata run: | set -xeuo pipefail - sudo podman run --rm -v $PWD:/run/src -w /run/src --privileged localhost/bootupd:latest tests/tests/generate-update-metadata.sh + sudo podman run --rm -v $PWD:/run/src -w /run/src --privileged "$IMG_NAME" tests/tests/generate-update-metadata.sh diff --git a/Dockerfile.grubcc b/Dockerfile.grubcc new file mode 100644 index 00000000..44849d7e --- /dev/null +++ b/Dockerfile.grubcc @@ -0,0 +1,101 @@ +# Build from the current git into a fedora44 container image. + +ARG base=quay.io/fedora/fedora-bootc:44 + +FROM $base as build +# This installs our package dependencies, and we want to cache it independently of the rest. +# Basically we don't want changing a .rs file to blow out the cache of packages. +RUN <&2 + exit 1 + ;; +esac + +mv ./*.rpm grub-cc.rpm + +EOF + +FROM $base +ARG grubcc +# Clean out the default to ensure we're using our updated content +RUN rpm -e bootupd +COPY --from=build /out/ / +COPY --from=grub-cc-download /grub-cc.rpm /var/grub-cc.rpm +# Install bootc from copr +RUN --mount=type=tmpfs,target=/run --mount=type=tmpfs,target=/tmp </EFI/fedora/ +mkdir -p "/usr/lib/efi/grub-cc/${evr}/EFI/$ID/" +cp "$file" "/usr/lib/efi/grub-cc/${evr}/EFI/$ID/grubx64.efi" + +cleanup + +# Regenerate metadata +bootupctl backend generate-update-metadata -vvv + +# Set grub as default +bootupctl backend set-default-bootloader grub + +EOF + +# Sanity check this too +RUN bootc container lint --fatal-warnings From 00ec302bc41056505cab77e48f3aeccaecfcf739 Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Wed, 24 Jun 2026 18:13:30 +0530 Subject: [PATCH 16/21] bootloader: Add SystemdBoot enum variant This works exactly like GrubCC We expect systemd-boot efi to be in `/usr/lib/efi/systemd-boot//fedora/grubx64.cc` Signed-off-by: Pragyan Poudyal --- src/backend/statefile.rs | 6 +++--- src/bootloader.rs | 9 ++++++++- src/efi.rs | 5 ++++- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/backend/statefile.rs b/src/backend/statefile.rs index 3106a5c0..0ba4679e 100644 --- a/src/backend/statefile.rs +++ b/src/backend/statefile.rs @@ -135,7 +135,7 @@ impl SavedState { } #[cfg(any(target_arch = "powerpc64", target_arch = "s390x"))] - Bootloader::GrubCC => { + Bootloader::GrubCC | Bootloader::Systemd => { let arch = if cfg!(target_arch = "powerpc64") { "powerpc64" } else { @@ -150,7 +150,7 @@ impl SavedState { target_arch = "aarch64", target_arch = "riscv64" ))] - Bootloader::GrubCC => { + Bootloader::GrubCC | Bootloader::Systemd => { use crate::efi::Efi; let efi = Efi::default(); @@ -201,7 +201,7 @@ impl SavedState { bail!("{} already exists", statepath.display()); } - Bootloader::GrubCC => { + Bootloader::GrubCC | Bootloader::Systemd => { bail!("{} already exists in the ESP", Self::STATEFILE_NAME); } } diff --git a/src/bootloader.rs b/src/bootloader.rs index f4455a2a..c470ab30 100644 --- a/src/bootloader.rs +++ b/src/bootloader.rs @@ -10,6 +10,7 @@ pub enum Bootloader { #[default] Grub, GrubCC, + Systemd, } impl Display for Bootloader { @@ -17,6 +18,7 @@ impl Display for Bootloader { match self { Bootloader::Grub => f.write_str("grub"), Bootloader::GrubCC => f.write_str("grub-cc"), + Bootloader::Systemd => f.write_str("systemd"), } } } @@ -25,7 +27,8 @@ impl Bootloader { fn next(self) -> Option { match self { Self::Grub => Some(Self::GrubCC), - Self::GrubCC => None, + Self::GrubCC => Some(Self::Systemd), + Self::Systemd => None, } } @@ -45,6 +48,7 @@ impl Bootloader { match self { Bootloader::Grub => "grub2", Bootloader::GrubCC => "grub-cc", + Bootloader::Systemd => "systemd-boot", } } @@ -52,6 +56,7 @@ impl Bootloader { match component_name { "grub2" => Ok(Self::Grub), "grub-cc" => Ok(Self::GrubCC), + "systemd-boot" => Ok(Self::Systemd), _ => anyhow::bail!("Not a valid bootloader: {component_name}"), } } @@ -78,6 +83,8 @@ pub(crate) fn get_bootloader() -> Result { Some(info) => { if info.to_lowercase().contains("grub cc") { Bootloader::GrubCC + } else if info.to_lowercase().contains("systemd-boot") { + Bootloader::Systemd } else { Bootloader::Grub } diff --git a/src/efi.rs b/src/efi.rs index 13b0726c..8a81aab7 100644 --- a/src/efi.rs +++ b/src/efi.rs @@ -323,7 +323,10 @@ impl Component for Efi { } fn is_bootloader_supported(&self, bootloader: Bootloader) -> bool { - matches!(bootloader, Bootloader::Grub | Bootloader::GrubCC) + matches!( + bootloader, + Bootloader::Grub | Bootloader::GrubCC | Bootloader::Systemd + ) } fn query_adopt(&self, devices: &Option>) -> Result> { From 9763156321605ffd2796784da8c4f9eab1c61f48 Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Thu, 25 Jun 2026 11:45:05 +0530 Subject: [PATCH 17/21] ci: Add tests for grub-cc and systemd-boot We don't have e2e upgrade tests for now, but we can atleast test if the installations are working well Signed-off-by: Pragyan Poudyal --- .github/workflows/ci.yml | 12 +++- Dockerfile.grubcc => Dockerfile.bls | 40 ++++++++++--- scripts/test-bootloader.sh | 88 +++++++++++++++++++++++++++++ 3 files changed, 131 insertions(+), 9 deletions(-) rename Dockerfile.grubcc => Dockerfile.bls (73%) create mode 100755 scripts/test-bootloader.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 38d0d628..83c9f90f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -74,7 +74,7 @@ jobs: build_args=() if [[ "${{ matrix.grubcc }}" == "1" ]]; then - dockerfile=Dockerfile.grubcc + dockerfile=Dockerfile.bls build_args=(--build-arg "grubcc=${{ matrix.grubcc }}") fi @@ -143,3 +143,13 @@ jobs: run: | set -xeuo pipefail sudo podman run --rm -v $PWD:/run/src -w /run/src --privileged "$IMG_NAME" tests/tests/generate-update-metadata.sh + + - name: Test GrubCC and SystemdBoot + run: | + if [[ "${{ matrix.grubcc }}" == "0" ]]; then + exit 0 + fi + + set -xeuo pipefail + sudo ./scripts/test-bootloader.sh "$IMG_NAME" "grub-cc" + sudo ./scripts/test-bootloader.sh "$IMG_NAME" "systemd" diff --git a/Dockerfile.grubcc b/Dockerfile.bls similarity index 73% rename from Dockerfile.grubcc rename to Dockerfile.bls index 44849d7e..2402d01c 100644 --- a/Dockerfile.grubcc +++ b/Dockerfile.bls @@ -1,3 +1,4 @@ +# Dockerfile to test GrubCC and systemd-boot # Build from the current git into a fedora44 container image. ARG base=quay.io/fedora/fedora-bootc:44 @@ -42,7 +43,6 @@ mv ./*.rpm grub-cc.rpm EOF FROM $base -ARG grubcc # Clean out the default to ensure we're using our updated content RUN rpm -e bootupd COPY --from=build /out/ / @@ -50,7 +50,7 @@ COPY --from=grub-cc-download /grub-cc.rpm /var/grub-cc.rpm # Install bootc from copr RUN --mount=type=tmpfs,target=/run --mount=type=tmpfs,target=/tmp </EFI/fedora/ +mkdir -p "/usr/lib/efi/systemd-boot/${evr}/EFI/$ID" + +if [[ $(uname -m) == x86_64 ]]; then + grubName="grubx64.efi" +else + grubName="grubaa64.efi" +fi + +cp /usr/lib/systemd/boot/efi/systemd-boot*.efi "/usr/lib/efi/systemd-boot/${evr}/EFI/$ID/$grubName" + +EOF + +# Install grub-cc RUN --mount=type=tmpfs,target=/run --mount=type=tmpfs,target=/tmp </EFI/fedora/ mkdir -p "/usr/lib/efi/grub-cc/${evr}/EFI/$ID/" -cp "$file" "/usr/lib/efi/grub-cc/${evr}/EFI/$ID/grubx64.efi" +cp "$file" "/usr/lib/efi/grub-cc/${evr}/EFI/$ID/$grubName" cleanup diff --git a/scripts/test-bootloader.sh b/scripts/test-bootloader.sh new file mode 100755 index 00000000..11b20c59 --- /dev/null +++ b/scripts/test-bootloader.sh @@ -0,0 +1,88 @@ +#!/bin/bash + +set -eux + +IMG_NAME=$1 +BOOTLOADER=$2 + +case $BOOTLOADER in + systemd) + EFI_DIR_NAME=systemd-boot + ;; + grub-cc) + EFI_DIR_NAME=grub-cc + ;; + grub) + EFI_DIR_NAME=grub2 + ;; +esac + +cat <<-EOF > sfdisk-buf +label: gpt +label-id: 65be9332-59ba-11f1-9b26-6a8e2ab625e4 +size=1Gib, type=C12A7328-F81F-11D2-BA4B-00A0C93EC93B, name="EFI-SYSTEM" + type=4F68BCE3-E8CD-4DB1-96E7-FBCAF984B709, name="root" +EOF + +truncate -s4G "${BOOTLOADER}-test.img" + +cat sfdisk-buf | sfdisk --wipe=always "${BOOTLOADER}-test.img" + +mkdir -p /var/mnt + +# Also update kernel partition tables +loopdev=$(losetup --find --show --partscan "${BOOTLOADER}-test.img") +sleep 1 + +mkfs.vfat "${loopdev}p1" +mkfs.ext4 "${loopdev}p2" + +mount "${loopdev}p2" /var/mnt + +ESP="/var/mnt/efi" + +mkdir -p $ESP +mount "${loopdev}p1" $ESP + + +# Test installing the bootloader +podman run --rm --net=host --privileged --pid=host \ + --privileged \ + --security-opt label=type:unconfined_t \ + --env RUST_LOG=trace \ + -v /dev:/dev \ + -v /var/mnt:/var/mnt \ + "$IMG_NAME" \ + bootupctl backend install --bootloader "$BOOTLOADER" /var/mnt -vvvv + +# Make sure bootupd-state.json is in the esp +test -f "$ESP/bootupd-state.json" + +cat "$ESP/bootupd-state.json" | jq + +version=$(cat "$ESP/bootupd-state.json" | jq -r ".installed.EFI.meta.version") + +if [[ $version != *shim* ]]; then echo "shim not found in version"; exit 1; fi +if [[ $version != *"$EFI_DIR_NAME"* ]]; then echo "$BOOTLOADER not found in version"; exit 1; fi + +# Test if the correct binary has been installed +actualShasum=$(podman run --rm "$IMG_NAME" find "/usr/lib/efi/$EFI_DIR_NAME" -type f -exec sha512sum {} + | awk '{print $1}') +actualShasum="sha512:$actualShasum" + +if [[ $(uname -m) == "x86_64" ]]; then + grubName="grubx64.efi" +else + grubName="grubaa64.efi" +fi + +# TODO: Remove hardcoded "fedora" once we have support in centos +storedShasum=$(cat "$ESP/bootupd-state.json" | jq -r --arg grub "$grubName" '.installed.EFI.filetree.children["fedora/\($grub)"].sha512') + +test "$actualShasum" == "$storedShasum" + +efiBinShasum=$(find "$ESP" -type f -name "$grubName" -exec sha512sum {} + | awk '{print $1}') +efiBinShasum="sha512:$efiBinShasum" + +test "$efiBinShasum" == "$actualShasum" + +umount -Rl /var/mnt From fcfd22c0932ee1e18a500eb9a8cd93b139d412fc Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Thu, 25 Jun 2026 15:28:17 +0530 Subject: [PATCH 18/21] filetree: Skip extra bootloaders We generate the final filetree from `/usr/lib/efi` which worked well with only one bootloader, but now that we have three which are stored in the following places Grub2 - /usr/lib/efi/grub2//EFI/fedora/grubx64.efi GrubCC - /usr/lib/efi/grub-cc//EFI/fedora/grubx64.efi SystemdBoot - /usr/lib/efi/systemd-boot//EFI/fedora/grubx64.efi The problem arises as all the distinct efi binaries have the same name (grubx64.efi - due to the particular name being baked into shim), we cannot blindly create a filesystem tree out of this as later values will override the previous ones. To mitigate this, introduce a param `skip_dirs` which will filter directories not required for the bootloader being installed Signed-off-by: Pragyan Poudyal --- src/efi.rs | 32 ++++++++++++++++++++++++++++---- src/filetree.rs | 40 ++++++++++++++++++++++++---------------- 2 files changed, 52 insertions(+), 20 deletions(-) diff --git a/src/efi.rs b/src/efi.rs index 8a81aab7..e75ba223 100644 --- a/src/efi.rs +++ b/src/efi.rs @@ -401,7 +401,9 @@ impl Component for Efi { .sysroot .open_dir(&updated_path) .with_context(|| format!("opening update dir {}", updated_path.display()))?; - let updatef = filetree::FileTree::new_from_dir(&updated).context("reading update dir")?; + // TODO(Johan-Liebert1): Handle adpot + let updatef = + filetree::FileTree::new_from_dir(&updated, None).context("reading update dir")?; let esp_devices = esp_devices.unwrap_or_default(); for esp in esp_devices { @@ -509,8 +511,21 @@ impl Component for Efi { &src.to_owned() }; + // Now that we have multiple bootloaders (all named grubx64.efi/grubaa64.efi + // due to that name being baked into the shim), we can't blindly use the key + // "fedora/grubx64.efi" to refer to the installed bootloader, hence we ask for + // any extra bootloaders to be not included in the FileTree + let dirs_to_skip = Bootloader::iter() + .filter(|b| *b != bootloader) + .map(|b| b.efi_component_name()) + .collect::>(); + // Get filetree from efi path - let ft = crate::filetree::FileTree::new_from_dir(&src_dir.open_dir(efi_path)?)?; + let ft = crate::filetree::FileTree::new_from_dir( + &src_dir.open_dir(efi_path)?, + Some(dirs_to_skip), + )?; + if update_firmware { if let Some(dev) = device { if let Some(vendordir) = @@ -532,13 +547,15 @@ impl Component for Efi { rootcxt: &RootContext, current: &InstalledContent, ) -> Result { + let bootloader = get_bootloader()?; + let currentf = current .filetree .as_ref() .ok_or_else(|| anyhow::anyhow!("No filetree for installed EFI found!"))?; let sysroot_dir = &rootcxt.sysroot; let updatemeta = self - .query_update(sysroot_dir, get_bootloader()?)? + .query_update(sysroot_dir, bootloader)? .expect("update available"); let updated_path = { let efilib_path = rootcxt.path.join(EFILIB); @@ -555,7 +572,14 @@ impl Component for Efi { .sysroot .open_dir(&updated_path) .with_context(|| format!("opening update dir {}", updated_path.display()))?; - let updatef = filetree::FileTree::new_from_dir(&updated).context("reading update dir")?; + + let dirs_to_skip = Bootloader::iter() + .filter(|b| *b != bootloader) + .map(|b| b.efi_component_name()) + .collect::>(); + + let updatef = filetree::FileTree::new_from_dir(&updated, Some(dirs_to_skip)) + .context("reading update dir")?; let diff = currentf.diff(&updatef)?; let Some(esp_devices) = rootcxt.device.find_colocated_esps()? else { diff --git a/src/filetree.rs b/src/filetree.rs index 31d0916d..a640070d 100644 --- a/src/filetree.rs +++ b/src/filetree.rs @@ -191,12 +191,20 @@ impl FileTree { target_arch = "aarch64", target_arch = "riscv64" ))] - pub(crate) fn new_from_dir(dir: &Dir) -> Result { + pub(crate) fn new_from_dir(dir: &Dir, dirs_to_skip: Option>) -> Result { let mut children = BTreeMap::new(); + let dirs_to_skip = dirs_to_skip.unwrap_or([].into()); + for (k, mut v) in Self::unsorted_from_dir(dir)?.drain() { - let k_path = get_dest_efi_path(Utf8Path::new(&k)).to_string(); + let src = Utf8Path::new(&k); + let k_path = get_dest_efi_path(src); + + if src.components().any(|c| dirs_to_skip.contains(&c.as_str())) { + continue; + } + v.source = Some(k); - children.insert(k_path, v); + children.insert(k_path.to_string(), v); } Ok(Self { children }) @@ -532,8 +540,8 @@ mod tests { use std::path::Path; fn run_diff(a: &Dir, b: &Dir) -> Result { - let ta = FileTree::new_from_dir(a)?; - let tb = FileTree::new_from_dir(b)?; + let ta = FileTree::new_from_dir(a, None)?; + let tb = FileTree::new_from_dir(b, None)?; let diff = ta.diff(&tb)?; Ok(diff) } @@ -557,15 +565,15 @@ mod tests { let c = Dir::open_ambient_dir(&c, ambient_authority())?; let da = Dir::open_ambient_dir(a, ambient_authority())?; let db = Dir::open_ambient_dir(b, ambient_authority())?; - let ta = FileTree::new_from_dir(&da)?; - let tb = FileTree::new_from_dir(&db)?; + let ta = FileTree::new_from_dir(&da, None)?; + let tb = FileTree::new_from_dir(&db, None)?; let diff = ta.diff(&tb)?; let rdiff = tb.diff(&ta)?; assert_eq!(diff.count(), rdiff.count()); assert_eq!(diff.additions.len(), rdiff.removals.len()); assert_eq!(diff.changes.len(), rdiff.changes.len()); apply_diff(&db, &c, &diff, opts)?; - let tc = FileTree::new_from_dir(&c)?; + let tc = FileTree::new_from_dir(&c, None)?; let newdiff = tb.diff(&tc)?; let skip_removals = opts.map(|o| o.skip_removals).unwrap_or(false); if skip_removals { @@ -622,8 +630,8 @@ mod tests { let diff = run_diff(&a, &b)?; assert_eq!(diff.count(), 1); assert_eq!(diff.removals.len(), 1); - let ta = FileTree::new_from_dir(&a)?; - let tb = FileTree::new_from_dir(&b)?; + let ta = FileTree::new_from_dir(&a, None)?; + let tb = FileTree::new_from_dir(&b, None)?; let cdiff = ta.changes(&tb)?; assert_eq!(cdiff.count(), 1); assert_eq!(cdiff.removals.len(), 1); @@ -654,7 +662,7 @@ mod tests { let diff = run_diff(&a, &b)?; assert_eq!(diff.count(), 1); assert_eq!(diff.changes.len(), 1); - let ta = FileTree::new_from_dir(&a)?; + let ta = FileTree::new_from_dir(&a, None)?; let rdiff = ta.relative_diff_to(&b)?; assert_eq!(rdiff.count(), diff.count()); assert_eq!(rdiff.changes.len(), diff.changes.len()); @@ -685,8 +693,8 @@ mod tests { { let a = Dir::open_ambient_dir(&a, ambient_authority())?; let b = Dir::open_ambient_dir(&b, ambient_authority())?; - let ta = FileTree::new_from_dir(&a)?; - let tb = FileTree::new_from_dir(&b)?; + let ta = FileTree::new_from_dir(&a, None)?; + let tb = FileTree::new_from_dir(&b, None)?; let diff = ta.diff(&tb)?; assert_eq!(diff.changes.len(), 1); assert_eq!(diff.additions.len(), 1); @@ -716,8 +724,8 @@ mod tests { fs::write(c.join(bar).join("newfile"), "filedata")?; let a = Dir::open_ambient_dir(&a.join("EFI"), ambient_authority())?; let c = Dir::open_ambient_dir(&c, ambient_authority())?; - let ta = FileTree::new_from_dir(&a)?; - let tc = FileTree::new_from_dir(&c)?; + let ta = FileTree::new_from_dir(&a, None)?; + let tc = FileTree::new_from_dir(&c, None)?; let diff = ta.diff(&tc)?; assert_eq!(diff.changes.len(), 1); assert_eq!(diff.additions.len(), 1); @@ -895,7 +903,7 @@ mod tests { } { b.remove_file(testfile)?; - let ta = FileTree::new_from_dir(&a)?; + let ta = FileTree::new_from_dir(&a, None)?; let diff = ta.relative_diff_to(&b)?; assert_eq!(diff.count(), 1); assert_eq!(diff.removals.len(), 1); From a6c877a3d250f601714af65e0ee8b1200f4bc0b2 Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Mon, 29 Jun 2026 15:50:52 +0530 Subject: [PATCH 19/21] Introduce `#[cfg(efi_arch)]` A utility cfg macro defined in build.rs so that we can deny EFI only bootloaders - GrubCC and SystemdBoot, on non-EFI systems at compile time Also, only allow setting default bootloaders for EFI systems Signed-off-by: Pragyan Poudyal --- build.rs | 11 +++++++++++ src/backend/statefile.rs | 40 +++++++++------------------------------- src/bootloader.rs | 24 +++++++++++++++++++++--- src/bootupd.rs | 6 ++++++ src/cli/bootupctl.rs | 2 ++ src/cli/bootupd.rs | 9 +++------ src/component.rs | 9 ++++++++- src/efi.rs | 6 ++++++ src/model.rs | 7 +++++++ src/model_legacy.rs | 1 + src/packagesystem.rs | 1 + 11 files changed, 75 insertions(+), 41 deletions(-) create mode 100644 build.rs diff --git a/build.rs b/build.rs new file mode 100644 index 00000000..be5cb350 --- /dev/null +++ b/build.rs @@ -0,0 +1,11 @@ +fn main() { + println!("cargo::rustc-check-cfg=cfg(efi_arch)"); + + if cfg!(any( + target_arch = "x86_64", + target_arch = "aarch64", + target_arch = "riscv64" + )) { + println!("cargo:rustc-cfg=efi_arch"); + } +} diff --git a/src/backend/statefile.rs b/src/backend/statefile.rs index 0ba4679e..e9d81581 100644 --- a/src/backend/statefile.rs +++ b/src/backend/statefile.rs @@ -2,7 +2,7 @@ use crate::bootloader::Bootloader; use crate::bootupd::list_dev_current_root; -use crate::freezethaw::fsfreeze_thaw_cycle; + use crate::model::SavedState; use crate::util::SignalTerminationGuard; use anyhow::{bail, Context, Result}; @@ -134,22 +134,7 @@ impl SavedState { } } - #[cfg(any(target_arch = "powerpc64", target_arch = "s390x"))] - Bootloader::GrubCC | Bootloader::Systemd => { - let arch = if cfg!(target_arch = "powerpc64") { - "powerpc64" - } else { - "s390x" - }; - - anyhow::bail!("Only Grub is supported for {arch}"); - } - - #[cfg(any( - target_arch = "x86_64", - target_arch = "aarch64", - target_arch = "riscv64" - ))] + #[cfg(efi_arch)] Bootloader::GrubCC | Bootloader::Systemd => { use crate::efi::Efi; let efi = Efi::default(); @@ -201,6 +186,7 @@ impl SavedState { bail!("{} already exists", statepath.display()); } + #[cfg(efi_arch)] Bootloader::GrubCC | Bootloader::Systemd => { bail!("{} already exists in the ESP", Self::STATEFILE_NAME); } @@ -211,6 +197,7 @@ impl SavedState { /// Write-lock guard for statefile, protecting against concurrent state updates. #[derive(Debug)] pub(crate) struct StateLockGuard { + #[allow(dead_code)] pub(crate) sysroot_path: Utf8PathBuf, pub(crate) sysroot: Dir, #[allow(dead_code)] @@ -234,29 +221,19 @@ impl StateLockGuard { return Ok(()); } - #[cfg(any(target_arch = "powerpc64", target_arch = "s390x"))] #[context("Updating state")] + #[cfg(not(efi_arch))] pub(crate) fn update_state( &mut self, state: &SavedState, - bootloader: Bootloader, + _bootloader: Bootloader, ) -> Result<()> { - let arch = if cfg!(target_arch = "powerpc64") { - "powerpc64" - } else { - "s390x" - }; - - if bootloader != Bootloader::Grub { - anyhow::bail!("Found bootloader: {bootloader}. Only Grub is supported for {arch}"); - } - - self.write_grub_statefile(state) + return self.write_grub_statefile(state); } /// Atomically replace the on-disk state with a new version. - #[cfg(not(any(target_arch = "powerpc64", target_arch = "s390x")))] #[context("Updating state")] + #[cfg(efi_arch)] pub(crate) fn update_state( &mut self, state: &SavedState, @@ -267,6 +244,7 @@ impl StateLockGuard { } use crate::efi::Efi; + use crate::freezethaw::fsfreeze_thaw_cycle; let device = get_parent_device(&self.sysroot)?; let all_esps = device diff --git a/src/bootloader.rs b/src/bootloader.rs index c470ab30..2b30551c 100644 --- a/src/bootloader.rs +++ b/src/bootloader.rs @@ -1,7 +1,7 @@ use anyhow::Result; use fn_error_context::context; use serde::{Deserialize, Serialize}; -use std::{fmt::Display, sync::OnceLock}; +use std::fmt::Display; #[derive( Debug, Default, Copy, Clone, clap::ValueEnum, PartialEq, Eq, Hash, Serialize, Deserialize, @@ -9,7 +9,9 @@ use std::{fmt::Display, sync::OnceLock}; pub enum Bootloader { #[default] Grub, + #[cfg(efi_arch)] GrubCC, + #[cfg(efi_arch)] Systemd, } @@ -17,13 +19,16 @@ impl Display for Bootloader { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { Bootloader::Grub => f.write_str("grub"), + #[cfg(efi_arch)] Bootloader::GrubCC => f.write_str("grub-cc"), + #[cfg(efi_arch)] Bootloader::Systemd => f.write_str("systemd"), } } } impl Bootloader { + #[cfg(efi_arch)] fn next(self) -> Option { match self { Self::Grub => Some(Self::GrubCC), @@ -32,6 +37,13 @@ impl Bootloader { } } + #[cfg(not(efi_arch))] + fn next(self) -> Option { + match self { + Self::Grub => None, + } + } + pub(crate) fn iter() -> impl Iterator { std::iter::successors(Some(Self::Grub), |v| v.next()) } @@ -47,31 +59,37 @@ impl Bootloader { pub(crate) fn efi_component_name(&self) -> &'static str { match self { Bootloader::Grub => "grub2", + #[cfg(efi_arch)] Bootloader::GrubCC => "grub-cc", + #[cfg(efi_arch)] Bootloader::Systemd => "systemd-boot", } } + #[cfg(efi_arch)] pub(crate) fn try_from_efi_component_name(component_name: &str) -> Result { match component_name { "grub2" => Ok(Self::Grub), + #[cfg(efi_arch)] "grub-cc" => Ok(Self::GrubCC), + #[cfg(efi_arch)] "systemd-boot" => Ok(Self::Systemd), _ => anyhow::bail!("Not a valid bootloader: {component_name}"), } } } -#[cfg(any(target_arch = "powerpc64", target_arch = "s390x"))] +#[cfg(not(efi_arch))] #[context("Getting bootloader")] pub(crate) fn get_bootloader() -> Result { Ok(Bootloader::Grub) } -#[cfg(not(any(target_arch = "powerpc64", target_arch = "s390x")))] +#[cfg(efi_arch)] #[context("Getting bootloader")] pub(crate) fn get_bootloader() -> Result { use crate::efi::get_loader_info; + use std::sync::OnceLock; static BOOTLOADER: OnceLock = OnceLock::new(); diff --git a/src/bootupd.rs b/src/bootupd.rs index a0ff7c8d..b23c0b14 100644 --- a/src/bootupd.rs +++ b/src/bootupd.rs @@ -94,6 +94,7 @@ pub(crate) fn install(opts: &InstallOpts, devices: &[Device], configs: ConfigMod anyhow::bail!("No components specified"); } + #[cfg(efi_arch)] let bootloader = match opts.bootloader { // CLI overrides anything else Some(b) => b, @@ -151,6 +152,9 @@ pub(crate) fn install(opts: &InstallOpts, devices: &[Device], configs: ConfigMod } }; + #[cfg(not(efi_arch))] + let bootloader = Bootloader::Grub; + let mut state = SavedState::default(); let mut installed_efi_vendor = None; @@ -300,6 +304,8 @@ fn get_static_config_meta() -> Result { timestamp: self_bin_meta.modified()?.into(), version: crate_version!().into(), versions: None, + + #[cfg(efi_arch)] default_bootloader: None, }; Ok(self_meta) diff --git a/src/cli/bootupctl.rs b/src/cli/bootupctl.rs index 7e045032..38bb9a54 100644 --- a/src/cli/bootupctl.rs +++ b/src/cli/bootupctl.rs @@ -74,6 +74,7 @@ pub enum CtlBackend { #[clap(name = "install", hide = true)] Install(super::bootupd::InstallOpts), #[clap(hide = true)] + #[cfg(efi_arch)] SetDefaultBootloader(super::bootupd::DefaultBootloaderOpts), } @@ -111,6 +112,7 @@ impl CtlCommand { CtlVerb::Backend(CtlBackend::Install(opts)) => { super::bootupd::DCommand::run_install(opts) } + #[cfg(efi_arch)] CtlVerb::Backend(CtlBackend::SetDefaultBootloader(opts)) => { super::bootupd::DCommand::set_default_bootloader(opts) } diff --git a/src/cli/bootupd.rs b/src/cli/bootupd.rs index df87501e..d496b611 100644 --- a/src/cli/bootupd.rs +++ b/src/cli/bootupd.rs @@ -40,6 +40,7 @@ pub enum DVerb { GenerateUpdateMetadata(GenerateOpts), #[clap(name = "install", about = "Install components")] Install(InstallOpts), + #[cfg(efi_arch)] SetDefaultBootloader(DefaultBootloaderOpts), } @@ -114,6 +115,7 @@ impl DCommand { match self.cmd { DVerb::Install(opts) => Self::run_install(opts), DVerb::GenerateUpdateMetadata(opts) => Self::run_generate_meta(opts), + #[cfg(efi_arch)] DVerb::SetDefaultBootloader(opts) => Self::set_default_bootloader(opts), } } @@ -130,12 +132,6 @@ impl DCommand { /// Runner for `install` verb. pub(crate) fn run_install(opts: InstallOpts) -> Result<()> { - if !matches!(opts.bootloader, Some(Bootloader::Grub) | None) - && cfg!(any(target_arch = "powerpc64", target_arch = "s390x")) - { - anyhow::bail!("Only Grub is supported for powerpc64 and s390x"); - } - let configmode = if opts.write_uuid { ConfigMode::WithUUID } else if opts.with_static_configs { @@ -162,6 +158,7 @@ impl DCommand { Ok(()) } + #[cfg(efi_arch)] pub(crate) fn set_default_bootloader(opts: DefaultBootloaderOpts) -> Result<()> { let all_components = crate::bootupd::get_components(); let target_components: Vec<_> = all_components.values().collect(); diff --git a/src/component.rs b/src/component.rs index 226269a4..83183352 100644 --- a/src/component.rs +++ b/src/component.rs @@ -14,6 +14,7 @@ use std::path::{Path, PathBuf}; use bootc_internal_blockdev::Device; +#[cfg(efi_arch)] use crate::cli::bootupd::DefaultBootloaderOpts; use crate::{bootloader::Bootloader, bootupd::RootContext, model::*}; @@ -141,6 +142,7 @@ pub(crate) trait Component { Path::new(&format!("{}.json", self.name())).into() } + #[cfg(efi_arch)] fn set_default_bootloader(&self, opts: &DefaultBootloaderOpts) -> Result<()> { if !self.is_bootloader_supported(opts.bootloader) { anyhow::bail!("{} not supported for {}", opts.bootloader, self.name()); @@ -167,6 +169,7 @@ pub(crate) trait Component { Ok(()) } + #[cfg(efi_arch)] fn get_default_bootloader(&self, root: &Dir) -> Result> { let update_meta = self .get_component_update(&root, None)? @@ -242,6 +245,7 @@ pub(crate) fn query_adopt_state() -> Result> { timestamp: coreos_aleph.ts, version: coreos_aleph.aleph.version, versions: None, + #[cfg(efi_arch)] default_bootloader: None, }; log::trace!("Adoptable: {:?}", &meta); @@ -260,6 +264,7 @@ pub(crate) fn query_adopt_state() -> Result> { timestamp, version: "unknown".to_string(), versions: None, + #[cfg(efi_arch)] default_bootloader: None, }; return Ok(Some(Adoptable { @@ -273,7 +278,6 @@ pub(crate) fn query_adopt_state() -> Result> { #[cfg(test)] mod tests { use cap_std::fs::{DirBuilder, DirBuilderExt, Permissions, PermissionsExt}; - use chrono::Utc; use super::*; @@ -353,7 +357,10 @@ mod tests { } #[test] + #[cfg(efi_arch)] fn test_set_default_bootloader() -> Result<()> { + use chrono::Utc; + let td = tempfile::tempdir()?; let sysroot = td.path().to_str().unwrap().to_string(); let tdir = Dir::open_ambient_dir(&sysroot, ambient_authority())?; diff --git a/src/efi.rs b/src/efi.rs index e75ba223..c211e98c 100644 --- a/src/efi.rs +++ b/src/efi.rs @@ -323,6 +323,10 @@ impl Component for Efi { } fn is_bootloader_supported(&self, bootloader: Bootloader) -> bool { + #[cfg(not(efi_arch))] + return matches!(bootloader, Bootloader::Grub); + + #[cfg(efi_arch)] matches!( bootloader, Bootloader::Grub | Bootloader::GrubCC | Bootloader::Systemd @@ -883,6 +887,7 @@ fn generate_meta_from_usr_efi(sysroot_path: &Utf8Path) -> Result Result<()> { let tmpdir: &tempfile::TempDir = &tempfile::tempdir()?; let tpath = tmpdir.path(); diff --git a/src/model.rs b/src/model.rs index 48164a50..0e171e28 100644 --- a/src/model.rs +++ b/src/model.rs @@ -26,6 +26,7 @@ pub(crate) struct ContentMetadata { pub(crate) versions: Option>, /// The default bootloader to install if at install time no bootloader option is /// provided + #[cfg(efi_arch)] pub(crate) default_bootloader: Option, } @@ -39,6 +40,7 @@ impl ContentMetadata { } /// Returns bootloaders are available for install + #[cfg(efi_arch)] pub(crate) fn num_bootloader_available(&self) -> Vec { let mut available = vec![]; @@ -53,6 +55,7 @@ impl ContentMetadata { return available; } + #[cfg(efi_arch)] pub(crate) fn bootloader_available(&mut self, bootloader: Bootloader) -> bool { self.version .split(",") @@ -184,12 +187,14 @@ mod test { timestamp: t, version: "grub2-efi-ia32-1:2.12-21.fc41.x86_64,grub2-efi-x64-1:2.12-21.fc41.x86_64,shim-ia32-15.8-3.x86_64,shim-x64-15.8-3.x86_64".into(), versions: None, + #[cfg(efi_arch)] default_bootloader: None, }; let b = ContentMetadata { timestamp: t + Duration::try_seconds(1).unwrap(), version: "grub2-efi-ia32-1:2.12-28.fc41.x86_64,grub2-efi-x64-1:2.12-28.fc41.x86_64,shim-ia32-15.8-3.x86_64,shim-x64-15.8-3.x86_64".into(), versions: None, + #[cfg(efi_arch)] default_bootloader: None, }; assert_eq!(a.can_upgrade_to(&b), Ordering::Less); // means upgradable @@ -209,6 +214,7 @@ mod test { rpm_evr: "15.8-3".into(), }, ]), + #[cfg(efi_arch)] default_bootloader: None, }; let b = ContentMetadata { @@ -224,6 +230,7 @@ mod test { rpm_evr: "15.8-3".into(), }, ]), + #[cfg(efi_arch)] default_bootloader: None, }; assert_eq!(a.can_upgrade_to(&b), Ordering::Less); // means upgradable diff --git a/src/model_legacy.rs b/src/model_legacy.rs index 47b8cad0..86f219b4 100644 --- a/src/model_legacy.rs +++ b/src/model_legacy.rs @@ -50,6 +50,7 @@ impl ContentMetadata01 { timestamp, version: self.version, versions: None, + #[cfg(efi_arch)] default_bootloader: None, } } diff --git a/src/packagesystem.rs b/src/packagesystem.rs index d566abb8..c3ee154a 100644 --- a/src/packagesystem.rs +++ b/src/packagesystem.rs @@ -75,6 +75,7 @@ fn rpm_parse_metadata(stdout: &[u8]) -> Result { timestamp: **largest_timestamp, version, versions: Some(modules_vec), + #[cfg(efi_arch)] default_bootloader: None, }) } From 0447372d170948f29ac9c15e899150a01857d222 Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Wed, 8 Jul 2026 14:27:17 +0530 Subject: [PATCH 20/21] Introduce `ComponentType` enum Use this enum in match statements so that if we add a new component in future, we'll get compile time errors in places where we need to handle the new component. This also gets rid of some hardcoded strings Also remove `#[default]` from Bootloader enum. Having a "default" in the enum doesn't make sense as we never construct this enum Signed-off-by: Pragyan Poudyal --- src/bios.rs | 6 +++++- src/bootloader.rs | 5 +---- src/bootupd.rs | 28 ++++++++++++++++------------ src/cli/bootupd.rs | 1 - src/component.rs | 29 +++++++++++++++++++++++++++-- src/efi.rs | 6 +++++- 6 files changed, 54 insertions(+), 21 deletions(-) diff --git a/src/bios.rs b/src/bios.rs index 1ea5593c..03a34085 100644 --- a/src/bios.rs +++ b/src/bios.rs @@ -100,7 +100,11 @@ impl Bios { impl Component for Bios { fn name(&self) -> &'static str { - "BIOS" + self.component_type().into() + } + + fn component_type(&self) -> ComponentType { + ComponentType::Bios } fn is_bootloader_supported(&self, bootloader: Bootloader) -> bool { diff --git a/src/bootloader.rs b/src/bootloader.rs index 2b30551c..9b3094fe 100644 --- a/src/bootloader.rs +++ b/src/bootloader.rs @@ -3,11 +3,8 @@ use fn_error_context::context; use serde::{Deserialize, Serialize}; use std::fmt::Display; -#[derive( - Debug, Default, Copy, Clone, clap::ValueEnum, PartialEq, Eq, Hash, Serialize, Deserialize, -)] +#[derive(Debug, Copy, Clone, clap::ValueEnum, PartialEq, Eq, Hash, Serialize, Deserialize)] pub enum Bootloader { - #[default] Grub, #[cfg(efi_arch)] GrubCC, diff --git a/src/bootupd.rs b/src/bootupd.rs index b23c0b14..84044402 100644 --- a/src/bootupd.rs +++ b/src/bootupd.rs @@ -2,7 +2,7 @@ use crate::bios; use crate::bootloader::{get_bootloader, Bootloader}; use crate::cli::bootupd::InstallOpts; -use crate::component; +use crate::component::{self, ComponentType}; use crate::component::{Component, ValidationResult}; use crate::coreos; #[cfg(any( @@ -104,17 +104,21 @@ pub(crate) fn install(opts: &InstallOpts, devices: &[Device], configs: ConfigMod let mut bios_default = None; for c in &target_components { - if c.name() == "EFI" { - efi_default = c.get_default_bootloader(&source_root_dir)?; + use crate::component::ComponentType; - if efi_default.is_none() { - // We don't want to filter any bootloader - efi_component_update = c.get_component_update(&source_root_dir, None)?; + match c.component_type() { + ComponentType::Bios => { + bios_default = Some(Bootloader::Grub); + } + ComponentType::Efi => { + efi_default = c.get_default_bootloader(&source_root_dir)?; + + if efi_default.is_none() { + // We don't want to filter any bootloader + efi_component_update = + c.get_component_update(&source_root_dir, None)?; + } } - } - - if c.name() == "BIOS" { - bios_default = c.get_default_bootloader(&source_root_dir)?; } } @@ -160,7 +164,7 @@ pub(crate) fn install(opts: &InstallOpts, devices: &[Device], configs: ConfigMod for &component in target_components.iter() { // skip for BIOS if no devices specified - if component.name() == "BIOS" && devices.is_empty() { + if component.component_type() == ComponentType::Bios && devices.is_empty() { println!( "Skip installing component {} without target device", component.name() @@ -193,7 +197,7 @@ pub(crate) fn install(opts: &InstallOpts, devices: &[Device], configs: ConfigMod let devices_to_install: Vec> = if devices.is_empty() { // No devices specified: install once with auto-detection (None). vec![None] - } else if component.name() == "EFI" { + } else if component.component_type() == ComponentType::Efi { // For EFI, only install to devices that have an ESP partition. let esp_devices: Vec<&Device> = devices .iter() diff --git a/src/cli/bootupd.rs b/src/cli/bootupd.rs index d496b611..a9b8951b 100644 --- a/src/cli/bootupd.rs +++ b/src/cli/bootupd.rs @@ -88,7 +88,6 @@ pub(crate) struct InstallOpts { pub(crate) auto: bool, /// The bootloader to use - /// Defaults to Grub #[clap(long)] pub(crate) bootloader: Option, } diff --git a/src/component.rs b/src/component.rs index 83183352..1b14889b 100644 --- a/src/component.rs +++ b/src/component.rs @@ -26,6 +26,27 @@ pub(crate) enum ValidationResult { Errors(Vec), } +#[derive(PartialEq, Eq, Clone, Copy)] +pub(crate) enum ComponentType { + Bios, + Efi, +} + +impl From for &'static str { + fn from(val: ComponentType) -> Self { + match val { + ComponentType::Bios => "BIOS", + ComponentType::Efi => "EFI", + } + } +} + +impl std::fmt::Display for ComponentType { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str((*self).into()) + } +} + /// A bootloader subsystem (EFI or BIOS) that can be installed, updated, and validated. /// /// Components encapsulate platform-specific bootloader management. Each implementation @@ -36,6 +57,10 @@ pub(crate) trait Component { /// and should remain stable. fn name(&self) -> &'static str; + /// Returns the type of the component as an enum + /// Prefer this over [`Component::name`] + fn component_type(&self) -> ComponentType; + /// In an operating system whose initially booted disk image is not /// using bootupd, detect whether it looks like the component exists /// and "synthesize" content metadata from it. @@ -313,10 +338,10 @@ mod tests { let all_components = crate::bootupd::get_components(); let target_components: Vec<_> = all_components.values().collect(); for &component in target_components.iter() { - if component.name() == "BIOS" { + if component.component_type() == ComponentType::Bios { assert_eq!(component.get_efi_vendor(tdp)?, None); } - if component.name() == "EFI" { + if component.component_type() == ComponentType::Efi { let x = component.get_efi_vendor(tdp); assert_eq!(x.is_err(), true); efi.remove_dir_all("centos")?; diff --git a/src/efi.rs b/src/efi.rs index c211e98c..fceecd98 100644 --- a/src/efi.rs +++ b/src/efi.rs @@ -319,7 +319,11 @@ fn skip_systemd_bootloaders() -> bool { impl Component for Efi { fn name(&self) -> &'static str { - "EFI" + self.component_type().into() + } + + fn component_type(&self) -> ComponentType { + ComponentType::Efi } fn is_bootloader_supported(&self, bootloader: Bootloader) -> bool { From e93be0f084bd90cc90d079db97b6fb0cab9cb428 Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Fri, 10 Jul 2026 15:51:40 +0530 Subject: [PATCH 21/21] Minor refactors Renaming some functions, using match instead of if-else for cleaner code. Slightly major change here is the `was_mounted` field was not being accounted for in `update_state` and the path was being unmounted regardless. Updated to call `drop` on `Efi` which handles it well Signed-off-by: Pragyan Poudyal --- Dockerfile.bls | 2 +- scripts/test-bootloader.sh | 4 ++++ src/backend/statefile.rs | 7 ++++--- src/bootloader.rs | 14 +++++--------- src/bootupd.rs | 5 ++--- src/component.rs | 2 +- src/model.rs | 4 ++-- 7 files changed, 19 insertions(+), 19 deletions(-) diff --git a/Dockerfile.bls b/Dockerfile.bls index 2402d01c..a1693562 100644 --- a/Dockerfile.bls +++ b/Dockerfile.bls @@ -68,7 +68,7 @@ rpm -ql systemd-boot-unsigned . /etc/os-release -# Create the expected directory structure at /usr/lib/efi/grub-cc//EFI/fedora/ +# Create the expected directory structure at /usr/lib/efi/systemd-boot//EFI/fedora/ mkdir -p "/usr/lib/efi/systemd-boot/${evr}/EFI/$ID" if [[ $(uname -m) == x86_64 ]]; then diff --git a/scripts/test-bootloader.sh b/scripts/test-bootloader.sh index 11b20c59..1fe645ff 100755 --- a/scripts/test-bootloader.sh +++ b/scripts/test-bootloader.sh @@ -15,6 +15,10 @@ case $BOOTLOADER in grub) EFI_DIR_NAME=grub2 ;; + *) + echo "Unknown bootloader $BOOTLOADER" + exit 1 + ;; esac cat <<-EOF > sfdisk-buf diff --git a/src/backend/statefile.rs b/src/backend/statefile.rs index e9d81581..a81dba58 100644 --- a/src/backend/statefile.rs +++ b/src/backend/statefile.rs @@ -252,11 +252,11 @@ impl StateLockGuard { .context("Searching for ESP")? .ok_or_else(|| anyhow::anyhow!("ESP not found"))?; - let efi = Efi::default(); - let serialized_state = serde_json::to_vec(state).context("Serializing state")?; for esp in all_esps { + let efi = Efi::default(); + let mounted = efi .ensure_mounted_esp(self.sysroot_path.as_std_path(), Path::new(&esp.path())) .context("Mounting ESP")?; @@ -273,7 +273,8 @@ impl StateLockGuard { // Do the sync before unmount fsfreeze_thaw_cycle(dir.reopen_as_ownedfd()?)?; drop(dir); - efi.unmount().context("unmount after update")?; + // This takes care of not unmounting ESP if it was already mounted + drop(efi); } Ok(()) diff --git a/src/bootloader.rs b/src/bootloader.rs index 9b3094fe..f5a4b97b 100644 --- a/src/bootloader.rs +++ b/src/bootloader.rs @@ -95,15 +95,11 @@ pub(crate) fn get_bootloader() -> Result { } let bootloader = match get_loader_info() { - Some(info) => { - if info.to_lowercase().contains("grub cc") { - Bootloader::GrubCC - } else if info.to_lowercase().contains("systemd-boot") { - Bootloader::Systemd - } else { - Bootloader::Grub - } - } + Some(info) => match info.to_lowercase() { + i if i.contains("grub cc") => Bootloader::GrubCC, + i if i.contains("systemd-boot") => Bootloader::Systemd, + _ => Bootloader::Grub, + }, None => Bootloader::Grub, }; diff --git a/src/bootupd.rs b/src/bootupd.rs index 84044402..733148e3 100644 --- a/src/bootupd.rs +++ b/src/bootupd.rs @@ -130,8 +130,7 @@ pub(crate) fn install(opts: &InstallOpts, devices: &[Device], configs: ConfigMod // // We can't install GrubCC for BIOS as it's not supported // So we just default to installing GrubCC - (Some(_), Some(eb)) => eb, - (None, Some(eb)) => eb, + (_, Some(eb)) => eb, (Some(bb), None) => bb, // We still can get the bootloader by reading in the EFI component update @@ -142,7 +141,7 @@ pub(crate) fn install(opts: &InstallOpts, devices: &[Device], configs: ConfigMod anyhow::bail!("Could not determine bootloader. Default bootloader not set") }; - let available_bootloaders = efi_component_update.num_bootloader_available(); + let available_bootloaders = efi_component_update.available_bootloaders(); if available_bootloaders.len() != 1 { anyhow::bail!( diff --git a/src/component.rs b/src/component.rs index 1b14889b..094d5172 100644 --- a/src/component.rs +++ b/src/component.rs @@ -183,7 +183,7 @@ pub(crate) trait Component { .get_component_update(&root, None)? .ok_or_else(|| anyhow::anyhow!("Expected to get update metadata"))?; - if !update_meta.bootloader_available(opts.bootloader) { + if !update_meta.is_bootloader_available(opts.bootloader) { anyhow::bail!("{} is not present in metadata", opts.bootloader); } diff --git a/src/model.rs b/src/model.rs index 0e171e28..9ad02967 100644 --- a/src/model.rs +++ b/src/model.rs @@ -41,7 +41,7 @@ impl ContentMetadata { /// Returns bootloaders are available for install #[cfg(efi_arch)] - pub(crate) fn num_bootloader_available(&self) -> Vec { + pub(crate) fn available_bootloaders(&self) -> Vec { let mut available = vec![]; if let Some(versions) = &self.versions { @@ -56,7 +56,7 @@ impl ContentMetadata { } #[cfg(efi_arch)] - pub(crate) fn bootloader_available(&mut self, bootloader: Bootloader) -> bool { + pub(crate) fn is_bootloader_available(&mut self, bootloader: Bootloader) -> bool { self.version .split(",") .any(|v| v.starts_with(bootloader.efi_component_name()))