From c47204dcbbff33a17f2c9a52ddf414af482576bd Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Tue, 31 Mar 2026 10:38:22 +0530 Subject: [PATCH 1/4] boot/config: Add method to get boot artifact name Add a method in BLSConfig and Grub Menuconfig to get the boot artifact name, i.e. get the name of the UKI or the name of the directory containing the Kernel + Initrd. The names are stripped of all our custom prefixes and suffixes, so basically they return the verity digest part of the name. This is useful for GC-ing Kernel + Initrd that are shared among multiple deployments since we can't rely on the composefs= parameter in the options as the cmdline verity digest might be different than the verity digest of the shared Kernel + Initrd. Tests written by Claude Code (Opus) Signed-off-by: Pragyan Poudyal --- crates/lib/src/parsers/bls_config.rs | 152 +++++++++++++++++++++- crates/lib/src/parsers/grub_menuconfig.rs | 98 +++++++++++++- 2 files changed, 246 insertions(+), 4 deletions(-) diff --git a/crates/lib/src/parsers/bls_config.rs b/crates/lib/src/parsers/bls_config.rs index 93dee848e..a7eaaeff2 100644 --- a/crates/lib/src/parsers/bls_config.rs +++ b/crates/lib/src/parsers/bls_config.rs @@ -2,8 +2,6 @@ //! //! This module parses the config files for the spec. -#![allow(dead_code)] - use anyhow::{Result, anyhow}; use bootc_kernel_cmdline::utf8::{Cmdline, CmdlineOwned}; use camino::Utf8PathBuf; @@ -15,7 +13,7 @@ use std::fmt::Display; use uapi_version::Version; use crate::bootc_composefs::status::ComposefsCmdline; -use crate::composefs_consts::UKI_NAME_PREFIX; +use crate::composefs_consts::{TYPE1_BOOT_DIR_PREFIX, UKI_NAME_PREFIX}; #[derive(Debug, PartialEq, Eq, Default)] pub enum BLSConfigType { @@ -173,6 +171,9 @@ impl BLSConfig { self } + /// Get the fs-verity digest from a BLS config + /// For EFI BLS entries, this returns the name of the UKI + /// For Non-EFI BLS entries, this returns the fs-verity digest in the "options" field pub(crate) fn get_verity(&self) -> Result { match &self.cfg_type { BLSConfigType::EFI { efi } => { @@ -205,6 +206,56 @@ impl BLSConfig { } } + /// Returns name of UKI in case of EFI config + /// Returns name of the directory containing Kernel + Initrd in case of Non-EFI config + /// + /// The names are stripped of our custom prefix and suffixes, so this returns the + /// verity digest part of the name + #[allow(dead_code)] + pub(crate) fn boot_artifact_name(&self) -> Result<&str> { + match &self.cfg_type { + BLSConfigType::EFI { efi } => { + let file_name = efi + .file_name() + .ok_or_else(|| anyhow::anyhow!("EFI path missing file name: {}", efi))?; + + let without_suffix = file_name.strip_suffix(EFI_EXT).ok_or_else(|| { + anyhow::anyhow!( + "EFI file name missing expected suffix '{}': {}", + EFI_EXT, + file_name + ) + })?; + + // For backwards compatibility, we don't make this prefix mandatory + match without_suffix.strip_prefix(UKI_NAME_PREFIX) { + Some(no_prefix) => Ok(no_prefix), + None => Ok(without_suffix), + } + } + + BLSConfigType::NonEFI { linux, .. } => { + let parent_dir = linux.parent().ok_or_else(|| { + anyhow::anyhow!("Linux kernel path has no parent directory: {}", linux) + })?; + + let dir_name = parent_dir.file_name().ok_or_else(|| { + anyhow::anyhow!("Parent directory has no file name: {}", parent_dir) + })?; + + // For backwards compatibility, we don't make this prefix mandatory + match dir_name.strip_prefix(TYPE1_BOOT_DIR_PREFIX) { + Some(dir_name_no_prefix) => Ok(dir_name_no_prefix), + None => Ok(dir_name), + } + } + + BLSConfigType::Unknown => { + anyhow::bail!("Cannot extract boot artifact name from unknown config type") + } + } + } + /// Gets the `options` field from the config /// Returns an error if the field doesn't exist /// or if the config is of type `EFI` @@ -585,4 +636,99 @@ mod tests { assert!(config_final < config_rc1); Ok(()) } + + #[test] + fn test_boot_artifact_name_efi_success() -> Result<()> { + use camino::Utf8PathBuf; + + let efi_path = Utf8PathBuf::from("bootc_composefs-abcd1234.efi"); + let config = BLSConfig { + cfg_type: BLSConfigType::EFI { efi: efi_path }, + version: "1".to_string(), + ..Default::default() + }; + + let artifact_name = config.boot_artifact_name()?; + assert_eq!(artifact_name, "abcd1234"); + Ok(()) + } + + #[test] + fn test_boot_artifact_name_non_efi_success() -> Result<()> { + use camino::Utf8PathBuf; + + let linux_path = Utf8PathBuf::from("/boot/bootc_composefs-xyz5678/vmlinuz"); + let config = BLSConfig { + cfg_type: BLSConfigType::NonEFI { + linux: linux_path, + initrd: vec![], + options: None, + }, + version: "1".to_string(), + ..Default::default() + }; + + let artifact_name = config.boot_artifact_name()?; + assert_eq!(artifact_name, "xyz5678"); + Ok(()) + } + + #[test] + fn test_boot_artifact_name_efi_missing_prefix() { + use camino::Utf8PathBuf; + + let efi_path = Utf8PathBuf::from("/EFI/Linux/abcd1234.efi"); + let config = BLSConfig { + cfg_type: BLSConfigType::EFI { efi: efi_path }, + version: "1".to_string(), + ..Default::default() + }; + + let artifact_name = config + .boot_artifact_name() + .expect("Should extract artifact name"); + assert_eq!(artifact_name, "abcd1234"); + } + + #[test] + fn test_boot_artifact_name_efi_missing_suffix() { + use camino::Utf8PathBuf; + + let efi_path = Utf8PathBuf::from("bootc_composefs-abcd1234"); + let config = BLSConfig { + cfg_type: BLSConfigType::EFI { efi: efi_path }, + version: "1".to_string(), + ..Default::default() + }; + + let result = config.boot_artifact_name(); + assert!(result.is_err()); + assert!( + result + .unwrap_err() + .to_string() + .contains("missing expected suffix") + ); + } + + #[test] + fn test_boot_artifact_name_efi_no_filename() { + use camino::Utf8PathBuf; + + let efi_path = Utf8PathBuf::from("/"); + let config = BLSConfig { + cfg_type: BLSConfigType::EFI { efi: efi_path }, + version: "1".to_string(), + ..Default::default() + }; + + let result = config.boot_artifact_name(); + assert!(result.is_err()); + assert!( + result + .unwrap_err() + .to_string() + .contains("missing file name") + ); + } } diff --git a/crates/lib/src/parsers/grub_menuconfig.rs b/crates/lib/src/parsers/grub_menuconfig.rs index 6ff3c6447..f2b551bbc 100644 --- a/crates/lib/src/parsers/grub_menuconfig.rs +++ b/crates/lib/src/parsers/grub_menuconfig.rs @@ -114,7 +114,7 @@ impl<'a> MenuEntry<'a> { let name = to_path .components() .last() - .ok_or(anyhow::anyhow!("Empty efi field"))? + .ok_or_else(|| anyhow::anyhow!("Empty efi field"))? .to_string() .strip_prefix(UKI_NAME_PREFIX) .ok_or_else(|| anyhow::anyhow!("efi does not start with custom prefix"))? @@ -124,6 +124,35 @@ impl<'a> MenuEntry<'a> { Ok(name) } + + /// Returns name of UKI in case of EFI config + /// + /// The names are stripped of our custom prefix and suffixes, so this returns + /// the verity digest part of the name + pub(crate) fn boot_artifact_name(&self) -> Result { + let chainloader_path = Utf8PathBuf::from(&self.body.chainloader); + + let file_name = chainloader_path.file_name().ok_or_else(|| { + anyhow::anyhow!( + "Chainloader path missing file name: {}", + &self.body.chainloader + ) + })?; + + let without_suffix = file_name.strip_suffix(EFI_EXT).ok_or_else(|| { + anyhow::anyhow!( + "EFI file name missing expected suffix '{}': {}", + EFI_EXT, + file_name + ) + })?; + + // For backwards compatibility, we don't make this prefix mandatory + match without_suffix.strip_prefix(UKI_NAME_PREFIX) { + Some(no_prefix) => Ok(no_prefix.into()), + None => Ok(without_suffix.into()), + } + } } /// Parser that takes content until balanced brackets, handling nested brackets and escapes. @@ -547,4 +576,71 @@ mod test { assert_eq!(result[1].body.chainloader, "/EFI/Linux/second.efi"); assert_eq!(result[1].body.search, "--set=root --fs-uuid \"some-uuid\""); } + + #[test] + fn test_menuentry_boot_artifact_name_success() { + let body = MenuentryBody { + insmod: vec!["fat", "chain"], + chainloader: "/EFI/bootc_composefs/bootc_composefs-abcd1234.efi".to_string(), + search: "--no-floppy --set=root --fs-uuid test", + version: 0, + extra: vec![], + }; + + let entry = MenuEntry { + title: "Test Entry".to_string(), + body, + }; + + let artifact_name = entry + .boot_artifact_name() + .expect("Should extract artifact name"); + assert_eq!(artifact_name, "abcd1234"); + } + + #[test] + fn test_menuentry_boot_artifact_name_missing_prefix() { + let body = MenuentryBody { + insmod: vec!["fat", "chain"], + chainloader: "/EFI/Linux/abcd1234.efi".to_string(), + search: "--no-floppy --set=root --fs-uuid test", + version: 0, + extra: vec![], + }; + + let entry = MenuEntry { + title: "Test Entry".to_string(), + body, + }; + + let artifact_name = entry + .boot_artifact_name() + .expect("Should extract artifact name"); + assert_eq!(artifact_name, "abcd1234"); + } + + #[test] + fn test_menuentry_boot_artifact_name_missing_suffix() { + let body = MenuentryBody { + insmod: vec!["fat", "chain"], + chainloader: "/EFI/bootc_composefs/bootc_composefs-abcd1234".to_string(), + search: "--no-floppy --set=root --fs-uuid test", + version: 0, + extra: vec![], + }; + + let entry = MenuEntry { + title: "Test Entry".to_string(), + body, + }; + + let result = entry.boot_artifact_name(); + assert!(result.is_err()); + assert!( + result + .unwrap_err() + .to_string() + .contains("missing expected suffix") + ); + } } From cfe6941cbe6cc08f319acfc3276fe6c2aa4e7edf Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Tue, 31 Mar 2026 13:04:44 +0530 Subject: [PATCH 2/4] composefs/boot: Change the way we find sharable vmlinuz + initrd Instead of looking in the ".origin" files and trying to match the boot_digest to digests in other origin files, we now simply re-compute the sha256sum for vmlinuz + initrd for all boot entries present. This fixes the bug that arises after mutiple upgrades where the original deployment that created the boot entry has been garbage collected, so we end up linking to another deployment that does have the same boot digest, but the verity digest doesn't match the verity digest used for the name of the directory where we store the kernel + initrd pair. Signed-off-by: Pragyan Poudyal --- crates/lib/src/bootc_composefs/boot.rs | 130 ++++++++++--------------- 1 file changed, 54 insertions(+), 76 deletions(-) diff --git a/crates/lib/src/bootc_composefs/boot.rs b/crates/lib/src/bootc_composefs/boot.rs index 646df9ff0..52f71feea 100644 --- a/crates/lib/src/bootc_composefs/boot.rs +++ b/crates/lib/src/bootc_composefs/boot.rs @@ -116,8 +116,7 @@ use crate::{ }; use crate::{ composefs_consts::{ - BOOT_LOADER_ENTRIES, ORIGIN_KEY_BOOT, ORIGIN_KEY_BOOT_DIGEST, STAGED_BOOT_LOADER_ENTRIES, - STATE_DIR_ABS, USER_CFG, USER_CFG_STAGED, + BOOT_LOADER_ENTRIES, STAGED_BOOT_LOADER_ENTRIES, USER_CFG, USER_CFG_STAGED, }, spec::{Bootloader, Host}, }; @@ -328,6 +327,27 @@ fn compute_boot_digest( Ok(hex::encode(digest)) } +#[context("Computing boot digest for Type1 entries")] +fn compute_boot_digest_type1(dir: &Dir) -> Result { + let mut vmlinuz = dir + .open(VMLINUZ) + .with_context(|| format!("Opening {VMLINUZ}"))?; + + let mut initrd = dir + .open(INITRD) + .with_context(|| format!("Opening {INITRD}"))?; + + let mut hasher = openssl::hash::Hasher::new(openssl::hash::MessageDigest::sha256()) + .context("Creating hasher")?; + + std::io::copy(&mut vmlinuz, &mut hasher)?; + std::io::copy(&mut initrd, &mut hasher)?; + + let digest: &[u8] = &hasher.finish().context("Finishing digest")?; + + Ok(hex::encode(digest)) +} + /// Compute SHA256Sum of .linux + .initrd section of the UKI /// /// # Arguments @@ -355,52 +375,35 @@ pub(crate) fn compute_boot_digest_uki(uki: &[u8]) -> Result { /// Given the SHA256 sum of current VMlinuz + Initrd combo, find boot entry with the same SHA256Sum /// /// # Returns -/// Returns the verity of all deployments that have a boot digest same as the one passed in +/// Returns the directory name that has the same sha256 digest for vmlinuz + initrd as the one +/// that's passed in #[context("Checking boot entry duplicates")] -pub(crate) fn find_vmlinuz_initrd_duplicates(digest: &str) -> Result>> { - let deployments = Dir::open_ambient_dir(STATE_DIR_ABS, ambient_authority()); - - let deployments = match deployments { - Ok(d) => d, - // The first ever deployment - Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(None), - Err(e) => anyhow::bail!(e), - }; - - let mut symlink_to: Option> = None; - - for depl in deployments.entries()? { - let depl = depl?; - - let depl_file_name = depl.file_name(); - let depl_file_name = depl_file_name.as_str()?; - - let config = depl - .open_dir() - .with_context(|| format!("Opening {depl_file_name}"))? - .read_to_string(format!("{depl_file_name}.origin")) - .context("Reading origin file")?; +pub(crate) fn find_vmlinuz_initrd_duplicate( + storage: &Storage, + digest: &str, +) -> Result> { + let boot_dir = storage.bls_boot_binaries_dir()?; + + for entry in boot_dir.entries_utf8()? { + let entry = entry?; + let dir_name = entry.file_name()?; + + if !entry.file_type()?.is_dir() { + continue; + } - let ini = tini::Ini::from_string(&config) - .with_context(|| format!("Failed to parse file {depl_file_name}.origin as ini"))?; + let Some(..) = dir_name.strip_prefix(TYPE1_BOOT_DIR_PREFIX) else { + continue; + }; - match ini.get::(ORIGIN_KEY_BOOT, ORIGIN_KEY_BOOT_DIGEST) { - Some(hash) => { - if hash == digest { - match symlink_to { - Some(ref mut prev) => prev.push(depl_file_name.to_string()), - None => symlink_to = Some(vec![depl_file_name.to_string()]), - } - } - } + let entry_digest = compute_boot_digest_type1(&boot_dir.open_dir(&dir_name)?)?; - // No SHASum recorded in origin file - // `symlink_to` is already none, but being explicit here - None => symlink_to = None, - }; + if entry_digest == digest { + return Ok(Some(dir_name)); + } } - Ok(symlink_to) + Ok(None) } #[context("Writing BLS entries to disk")] @@ -687,45 +690,20 @@ pub(crate) fn setup_composefs_bls_boot( options: Some(cmdline_refs), }); - match find_vmlinuz_initrd_duplicates(&boot_digest)? { - Some(shared_entries) => { + let shared_entry = match setup_type { + BootSetupType::Setup(_) => None, + BootSetupType::Upgrade((storage, ..)) => { + find_vmlinuz_initrd_duplicate(storage, &boot_digest)? + } + }; + + match shared_entry { + Some(shared_entry) => { // Multiple deployments could be using the same kernel + initrd, but there // would be only one available // // Symlinking directories themselves would be better, but vfat does not support // symlinks - - let mut shared_entry: Option = None; - - let entries = - Dir::open_ambient_dir(entry_paths.entries_path, ambient_authority()) - .context("Opening entries path")? - .entries_utf8() - .context("Getting dir entries")?; - - for ent in entries { - let ent = ent?; - // We shouldn't error here as all our file names are UTF-8 compatible - let ent_name = ent.file_name()?; - - let Some(entry_verity_part) = ent_name.strip_prefix(TYPE1_BOOT_DIR_PREFIX) - else { - // Not our directory - continue; - }; - - if shared_entries - .iter() - .any(|shared_ent| shared_ent == entry_verity_part) - { - shared_entry = Some(ent_name); - break; - } - } - - let shared_entry = shared_entry - .ok_or_else(|| anyhow::anyhow!("Shared boot binaries not found"))?; - match bls_config.cfg_type { BLSConfigType::NonEFI { ref mut linux, From a759090d9a7b8b33c723a958ce698d63e22f17a5 Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Tue, 31 Mar 2026 13:18:59 +0530 Subject: [PATCH 3/4] composefs/gc: Fix shared entry GC even when they're in use This fixes a bug where a shared Type1 entry would get GCd even when it's in use due to the original image that created it being deleted. Combined with the fact that we were comparing the fsverity digest in the options field of the BLS config (which will be different than the name of the directory containing the vmlinuz + initrd pair). Now, we compare against the directory name when GC-ing boot binaries Fixes: https://github.com/bootc-dev/bootc/issues/2102 Also, remove `allow(dead_code)` from BLS and Grub Menuconfig parsers as now we use `boot_artifact_name` method Signed-off-by: Pragyan Poudyal --- crates/lib/src/bootc_composefs/gc.rs | 35 +++++++++++++---- crates/lib/src/bootc_composefs/status.rs | 47 ++++++++++++++++++++--- crates/lib/src/parsers/bls_config.rs | 1 - crates/lib/src/parsers/grub_menuconfig.rs | 2 - 4 files changed, 69 insertions(+), 16 deletions(-) diff --git a/crates/lib/src/bootc_composefs/gc.rs b/crates/lib/src/bootc_composefs/gc.rs index f67d4becc..0aa0f9edc 100644 --- a/crates/lib/src/bootc_composefs/gc.rs +++ b/crates/lib/src/bootc_composefs/gc.rs @@ -91,11 +91,11 @@ fn collect_uki_binaries(boot_dir: &Dir, boot_binaries: &mut Vec) -> let entry = entry?; let name = entry.file_name()?; - let Some(verity) = name.strip_prefix(UKI_NAME_PREFIX) else { + let Some(efi_name_no_prefix) = name.strip_prefix(UKI_NAME_PREFIX) else { continue; }; - if name.ends_with(EFI_EXT) { + if let Some(verity) = efi_name_no_prefix.strip_suffix(EFI_EXT) { boot_binaries.push((BootType::Uki, verity.into())); } } @@ -235,7 +235,11 @@ pub(crate) async fn composefs_gc( // filter the ones that are not referenced by any bootloader entry !bootloader_entries .iter() - .any(|boot_entry| bin_path.1 == *boot_entry) + // We compare the name of directory containing the binary instead of comparing the + // fsverity digest. This is because a shared entry might differing directory + // name and fsverity digest in the cmdline. And since we want to GC the actual + // binaries, we compare with the directory name + .any(|boot_entry| boot_entry.boot_artifact_name == bin_path.1) }) .collect::>(); @@ -263,11 +267,28 @@ pub(crate) async fn composefs_gc( // Collect the deployments that have an image but no bootloader entry // and vice versa - let img_bootloader_diff = images + // + // Images without corresponding bootloader entries + let orphaned_images: Vec<&String> = images .iter() - .filter(|i| !bootloader_entries.contains(i)) - .chain(bootloader_entries.iter().filter(|b| !images.contains(b))) - .collect::>(); + .filter(|image| { + !bootloader_entries + .iter() + .any(|entry| &entry.fsverity == *image) + }) + .collect(); + + // Bootloader entries without corresponding images + let orphaned_bootloader_entries: Vec<&String> = bootloader_entries + .iter() + .map(|entry| &entry.fsverity) + .filter(|verity| !images.contains(verity)) + .collect(); + + let img_bootloader_diff: Vec<&String> = orphaned_images + .into_iter() + .chain(orphaned_bootloader_entries) + .collect(); tracing::debug!("img_bootloader_diff: {img_bootloader_diff:#?}"); diff --git a/crates/lib/src/bootc_composefs/status.rs b/crates/lib/src/bootc_composefs/status.rs index bde6a8aa3..58a93638a 100644 --- a/crates/lib/src/bootc_composefs/status.rs +++ b/crates/lib/src/bootc_composefs/status.rs @@ -121,6 +121,26 @@ pub(crate) struct StagedDeployment { pub(crate) finalization_locked: bool, } +#[derive(Debug, PartialEq)] +pub(crate) struct BootloaderEntry { + /// The fsverity digest associated with the bootloader entry + /// This is the value of composefs= param + pub(crate) fsverity: String, + /// The name of the (UKI/Kernel+Initrd directory) related to the entry + /// + /// For UKI, this is the name of the UKI stripped of our custom + /// prefix and .efi suffix + /// + /// For Type1 entries, this is the name to the directory containing + /// Kernel+Initrd, stripped of our custom prefix + /// + /// Since this is stripped of all our custom prefixes + file extensions + /// this is basically the verity digest part of the name + /// + /// We mainly need this in order to GC shared Type1 entries + pub(crate) boot_artifact_name: String, +} + /// Detect if we have `composefs=` in `/proc/cmdline` pub(crate) fn composefs_booted() -> Result> { static CACHED_DIGEST_VALUE: OnceLock> = OnceLock::new(); @@ -263,7 +283,7 @@ fn get_sorted_type1_boot_entries_helper( Ok(all_configs) } -fn list_type1_entries(boot_dir: &Dir) -> Result> { +fn list_type1_entries(boot_dir: &Dir) -> Result> { // Type1 Entry let boot_entries = get_sorted_type1_boot_entries(boot_dir, true)?; @@ -274,7 +294,12 @@ fn list_type1_entries(boot_dir: &Dir) -> Result> { boot_entries .into_iter() .chain(staged_boot_entries) - .map(|entry| entry.get_verity()) + .map(|entry| { + Ok(BootloaderEntry { + fsverity: entry.get_verity()?, + boot_artifact_name: entry.boot_artifact_name()?.to_string(), + }) + }) .collect::, _>>() } @@ -283,7 +308,7 @@ fn list_type1_entries(boot_dir: &Dir) -> Result> { /// # Returns /// The fsverity of EROFS images corresponding to boot entries #[fn_error_context::context("Listing bootloader entries")] -pub(crate) fn list_bootloader_entries(storage: &Storage) -> Result> { +pub(crate) fn list_bootloader_entries(storage: &Storage) -> Result> { let bootloader = get_bootloader()?; let boot_dir = storage.require_boot_dir()?; @@ -304,8 +329,13 @@ pub(crate) fn list_bootloader_entries(storage: &Storage) -> Result> boot_entries .into_iter() .chain(boot_entries_staged) - .map(|entry| entry.get_verity()) - .collect::, _>>()? + .map(|entry| { + Ok(BootloaderEntry { + fsverity: entry.get_verity()?, + boot_artifact_name: entry.boot_artifact_name()?, + }) + }) + .collect::, anyhow::Error>>()? } else { list_type1_entries(boot_dir)? } @@ -739,7 +769,11 @@ async fn composefs_deployment_status_from( // Rollback deployment is in here, but may also contain stale deployment entries let mut extra_deployment_boot_entries: Vec = Vec::new(); - for verity_digest in bootloader_entry_verity { + for BootloaderEntry { + fsverity: verity_digest, + .. + } in bootloader_entry_verity + { // read the origin file let config = state_dir .open_dir(&verity_digest) @@ -877,6 +911,7 @@ async fn composefs_deployment_status_from( .map(|menu| menu.get_verity()), ) .collect::>>()?; + let rollback_candidates: Vec<_> = extra_deployment_boot_entries .into_iter() .filter(|entry| { diff --git a/crates/lib/src/parsers/bls_config.rs b/crates/lib/src/parsers/bls_config.rs index a7eaaeff2..0988b0d61 100644 --- a/crates/lib/src/parsers/bls_config.rs +++ b/crates/lib/src/parsers/bls_config.rs @@ -211,7 +211,6 @@ impl BLSConfig { /// /// The names are stripped of our custom prefix and suffixes, so this returns the /// verity digest part of the name - #[allow(dead_code)] pub(crate) fn boot_artifact_name(&self) -> Result<&str> { match &self.cfg_type { BLSConfigType::EFI { efi } => { diff --git a/crates/lib/src/parsers/grub_menuconfig.rs b/crates/lib/src/parsers/grub_menuconfig.rs index f2b551bbc..90d7e2ee2 100644 --- a/crates/lib/src/parsers/grub_menuconfig.rs +++ b/crates/lib/src/parsers/grub_menuconfig.rs @@ -1,7 +1,5 @@ //! Parser for GRUB menuentry configuration files using nom combinators. -#![allow(dead_code)] - use std::fmt::Display; use anyhow::Result; From 7d8fee0973a879241fed82045ca71db546013da8 Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Tue, 31 Mar 2026 14:24:27 +0530 Subject: [PATCH 4/4] test: Add test for GC-ing shared Type1 entries Add a test to make sure we do not GC shared Type1 entries when they're still referenced Remove openh264 from repos to speed up installs Signed-off-by: Pragyan Poudyal --- hack/provision-derived.sh | 3 ++ tmt/tests/booted/test-composefs-gc.nu | 60 ++++++++++++++++++++++----- 2 files changed, 53 insertions(+), 10 deletions(-) diff --git a/hack/provision-derived.sh b/hack/provision-derived.sh index 59d9d15c6..531115129 100755 --- a/hack/provision-derived.sh +++ b/hack/provision-derived.sh @@ -17,6 +17,9 @@ mkdir -p ~/.config/nushell echo '$env.config = { show_banner: false, }' > ~/.config/nushell/config.nu touch ~/.config/nushell/env.nu +# We don't want openh264 +rm -f "/etc/yum.repos.d/fedora-cisco-openh264.repo" + . /usr/lib/os-release case "${ID}-${VERSION_ID}" in "centos-9") diff --git a/tmt/tests/booted/test-composefs-gc.nu b/tmt/tests/booted/test-composefs-gc.nu index 1288e9965..c8f93fea9 100644 --- a/tmt/tests/booted/test-composefs-gc.nu +++ b/tmt/tests/booted/test-composefs-gc.nu @@ -53,9 +53,6 @@ def second_boot [] { let path = cat /var/large-file-marker-objpath - echo "\$path" - echo $path - assert ($path | path exists) # Create another image with a different initrd so we can test kernel + initrd cleanup @@ -107,14 +104,12 @@ def third_boot [] { let boot_dir = if ($bootloader | str downcase) == "systemd" { # TODO: Some concrete API for this would be great mkdir /var/tmp/efi - mount /dev/vda2 /var/tmp/efi + mount /dev/disk/by-partlabel/EFI-SYSTEM /var/tmp/efi "/var/tmp/efi/EFI/Linux" } else { "/sysroot/boot" } - print $"bootdir ($boot_dir)" - assert ($"($boot_dir)/($dir_prefix)($booted_verity)" | path exists) # This is for the rollback, but since the rollback and the very @@ -125,11 +120,27 @@ def third_boot [] { echo $"($boot_dir)/($dir_prefix)(cat /var/first-verity)" | save /var/to-be-deleted-kernel + # Switching and rebooting here won't delete the old kernel because we still + # have it as the rollback deployment + echo " + FROM localhost/bootc-derived-initrd + RUN echo 'another file' > /usr/share/another-one + " | podman build -t localhost/bootc-prefinal . -f - + + + bootc switch --transport containers-storage localhost/bootc-prefinal + + tmt-reboot +} + +def fourth_boot [] { + assert equal $booted.image.image "localhost/bootc-prefinal" + # Now we create a new image derived from the current kernel + initrd # Switching to this and rebooting should remove the old kernel + initrd echo " FROM localhost/bootc-derived-initrd - RUN echo 'another file' > /usr/share/another-one + RUN echo 'another file 1' > /usr/share/another-one-1 " | podman build -t localhost/bootc-final . -f - @@ -138,19 +149,44 @@ def third_boot [] { tmt-reboot } -def fourth_boot [] { +def fifth_boot [] { let bootloader = (bootc status --json | from json).status.booted.composefs.bootloader if ($bootloader | str downcase) == "systemd" { # TODO: Some concrete API for this would be great mkdir /var/tmp/efi - mount /dev/vda2 /var/tmp/efi + mount /dev/disk/by-partlabel/EFI-SYSTEM /var/tmp/efi } assert equal $booted.image.image "localhost/bootc-final" assert (not ((cat /var/to-be-deleted-kernel | path exists))) - tap ok + # Now we want to test preservation of shared BLS binaries + # This takes at least 3 reboots + 1..3 | each { |i| + echo $" + FROM localhost/bootc-derived-initrd + RUN echo '($i)' > /usr/share/($i) + " | podman build -t $"localhost/bootc-shared-($i)" . -f - + } + + bootc switch --transport containers-storage localhost/bootc-shared-1 + + tmt-reboot +} + +def sixth_boot [i: int] { + assert equal $booted.image.image $"localhost/bootc-shared-($i)" + + # Just this being booted counts as success + if $i == 3 { + tap ok + return + } + + bootc switch --transport containers-storage $"localhost/bootc-shared-($i + 1)" + + tmt-reboot } def main [] { @@ -159,6 +195,10 @@ def main [] { "1" => second_boot, "2" => third_boot, "3" => fourth_boot, + "4" => fifth_boot, + "5" => { sixth_boot 1 }, + "6" => { sixth_boot 2 }, + "7" => { sixth_boot 3 }, $o => { error make { msg: $"Invalid TMT_REBOOT_COUNT ($o)" } }, } }