From a7ad9dedb9fc46807e3f80a9f6e50c561b010072 Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Fri, 7 Aug 2026 11:17:10 +0530 Subject: [PATCH 1/2] uki: Fix dumpfile diffing The dumpfile diff was not being performed in the update/switch operations which was causing the dumpfile test to fail. Refactor out the diffing code to also run in the update/switch operations. Another major issue was `/boot` being masked in the EROFS which caused us to not find the dumpfile when reading from the EROFS. Update to read from the filesystem, create tmpfiles and run diff on the tmpfiles Signed-off-by: Pragyan Poudyal --- crates/lib/src/bootc_composefs/boot.rs | 156 ++++++++++++++--------- crates/lib/src/bootc_composefs/repo.rs | 4 + crates/lib/src/bootc_composefs/update.rs | 31 ++++- 3 files changed, 123 insertions(+), 68 deletions(-) diff --git a/crates/lib/src/bootc_composefs/boot.rs b/crates/lib/src/bootc_composefs/boot.rs index 9909aa0c0..232d863d8 100644 --- a/crates/lib/src/bootc_composefs/boot.rs +++ b/crates/lib/src/bootc_composefs/boot.rs @@ -64,7 +64,8 @@ use std::cell::Cell; use std::fs::create_dir_all; use std::io::{Read, Seek, SeekFrom, Write}; -use std::path::{Path, PathBuf}; +use std::os::fd::AsFd; +use std::path::Path; use std::sync::Arc; use anyhow::{Context, Result, anyhow, bail}; @@ -77,7 +78,7 @@ use cap_std_ext::{ use clap::ValueEnum; use composefs::fs::read_file; use composefs::fsverity::{FsVerityHashValue, Sha512HashValue}; -use composefs::tree::RegularFile; +use composefs::tree::{FileSystem, RegularFile}; use composefs_boot::bootloader::{ BootEntry as ComposefsBootEntry, EFI_ADDON_DIR_EXT, EFI_ADDON_FILE_EXT, EFI_EXT, PEType, UsrLibModulesVmlinuz, get_boot_resources, @@ -102,7 +103,10 @@ use crate::composefs_consts::{TYPE1_BOOT_DIR_PREFIX, TYPE1_ENT_PATH, TYPE1_ENT_P use crate::parsers::bls_config::{BLSConfig, BLSConfigType, EFIKey}; use crate::spec::BootloaderKind; use crate::task::Task; -use crate::{bootc_composefs::repo::open_composefs_repo, store::Storage}; +use crate::{ + bootc_composefs::repo::open_composefs_repo, + store::{ComposefsRepository, Storage}, +}; use crate::{bootc_composefs::status::get_sorted_grub_uki_boot_entries, install::PostFetchState}; use crate::{ composefs_consts::{ @@ -144,6 +148,93 @@ pub(crate) struct UKIDigestMismatch { pub uki_name: Option, } +pub(crate) fn print_uki_dumpfile_diff( + mismatch: &UKIDigestMismatch, + repo: &ComposefsRepository, + fs: &FileSystem, +) { + let dumpfile_name = mismatch + .uki_name + .as_ref() + .and_then(|x| x.strip_suffix(EFI_EXT).map(|x| format!("{x}.dump"))); + + let Some(dumpfile_name) = &dumpfile_name else { + return; + }; + + let Some(stored_content) = read_dumpfile_from_fs(fs, dumpfile_name, repo) else { + tracing::debug!("Dumpfile {dumpfile_name} not found in filesystem"); + return; + }; + + let Ok(tempdir) = tempfile::tempdir() else { + return; + }; + let path = tempdir.path(); + let Ok(tempdir_cap) = Dir::open_ambient_dir(path, ambient_authority()) else { + return; + }; + + let Ok(mut stored_file) = tempdir_cap.create("stored") else { + return; + }; + if stored_file.write_all(&stored_content).is_err() { + return; + } + + let Ok(mut current_file) = tempdir_cap.create("current") else { + return; + }; + if let Err(e) = dumpfile::write_dumpfile(&mut current_file, fs) { + tracing::debug!("Writing dumpfile failed: {e}"); + return; + } + + let mut cmd = std::process::Command::new("diff"); + cmd.arg("--color=auto") + .arg(format!("{}/stored", path.display())) + .arg(format!("{}/current", path.display())); + + // Redirect stdout to stderr since this is diagnostic output + if let Ok(fd) = std::io::stderr().as_fd().try_clone_to_owned() { + cmd.stdout(fd); + } + + if let Err(e) = cmd.status() { + tracing::warn!("diffing dumpfiles failed with Err: {e:?}"); + } +} + +fn read_regular_file( + file: &RegularFile, + repo: &ComposefsRepository, +) -> Option> { + match file { + RegularFile::External(object_id, _) | RegularFile::ExternalNoVerity(object_id, _) => { + repo.read_object(object_id).ok() + } + RegularFile::Inline(data) => Some(data.to_vec()), + RegularFile::Sparse(_) => None, + } +} + +fn read_dumpfile_from_fs( + fs: &FileSystem, + dumpfile_name: &str, + repo: &ComposefsRepository, +) -> Option> { + let root = fs.as_dir(); + let dumpfile_os = std::ffi::OsStr::new(dumpfile_name); + + if let Ok(boot_dir) = root.get_directory_ref("boot".as_ref()) { + if let Ok(file) = boot_dir.get_file(dumpfile_os) { + return read_regular_file(file, repo); + } + } + + None +} + pub(crate) enum BootSetupType<'a> { /// For initial setup, i.e. install to-disk Setup((&'a RootSetup, &'a State, &'a PostFetchState)), @@ -1618,64 +1709,7 @@ pub(crate) async fn setup_composefs_boot( Ok(boot_digest) => boot_digest, Err(e) => match e.downcast::() { Ok(mismatch) => { - // We expect the dumpfile to be named the same as the UKI - // Ex. UKI - 6.19.14-108.fc42.x86_64.efi - // Dumpfile - 6.19.14-108.fc42.x86_64.dump - let dumpfile_name = mismatch - .uki_name - .as_ref() - .and_then(|x| x.strip_suffix(EFI_EXT).map(|x| format!("{x}.dump"))); - - let Some(dumpfile_name) = &dumpfile_name else { - return Err(mismatch.into()); - }; - - let dump = composefs_ctl::dump_files( - &repo, - &id.to_hex(), - &vec![PathBuf::from(dumpfile_name)], - true, - ); - - let Ok(dump) = dump else { - tracing::debug!("Dumpfile not found for diff"); - return Err(mismatch.into()); - }; - - // SAFETY: This output is always UTF-8 compatible as it's of the form - // - let text = std::str::from_utf8(&dump)?; - let obj_path = text.split_whitespace().nth(1); - - let Some(obj_path) = obj_path else { - return Err(mismatch.into()); - }; - - let tempdir = tempfile::tempdir()?; - let path = tempdir.path(); - let tempdir = Dir::open_ambient_dir(path, ambient_authority())?; - - let mut tmpfile = tempdir.create("current")?; - dumpfile::write_dumpfile(&mut tmpfile, &fs).context("Writing dumpfile")?; - - let mut cmd = std::process::Command::new("diff"); - let out = cmd - .arg("--color=auto") - .arg( - root_setup - .physical_root_path - .join("sysroot/composefs/objects") - .join(obj_path), - ) - .arg(format!("{}/current", path.display())) - .status(); - - // Intentionally not short-circuiting here as the real error is digest - // mismtach - if let Err(e) = out { - tracing::warn!("diffing dumpfiles failed with Err: {e:?}"); - }; - + print_uki_dumpfile_diff(&mismatch, &repo, &fs); return Err(mismatch.into()); } Err(e) => Err(e)?, diff --git a/crates/lib/src/bootc_composefs/repo.rs b/crates/lib/src/bootc_composefs/repo.rs index 4d9fe068d..9454e61c5 100644 --- a/crates/lib/src/bootc_composefs/repo.rs +++ b/crates/lib/src/bootc_composefs/repo.rs @@ -42,6 +42,7 @@ use anyhow::{Context, Result}; use composefs::fsverity::{FsVerityHashValue, Sha512HashValue}; use composefs::repository::RepositoryConfig; +use composefs::tree::FileSystem; use composefs_boot::bootloader::{BootEntry as ComposefsBootEntry, get_boot_resources}; use composefs_ctl::composefs; use composefs_ctl::composefs_boot; @@ -188,6 +189,8 @@ pub(crate) struct PullRepoResult { pub(crate) id: Sha512HashValue, /// The OCI manifest content digest (e.g. "sha256:abc...") pub(crate) manifest_digest: String, + /// The untransformed OCI filesystem (still has /boot, /sysroot, etc.) + pub(crate) fs: FileSystem, } /// Pull an image directly into the composefs repository via skopeo. @@ -424,6 +427,7 @@ pub(crate) async fn pull_composefs_repo( entries, id, manifest_digest: pull_result.manifest_digest.to_string(), + fs, }) } diff --git a/crates/lib/src/bootc_composefs/update.rs b/crates/lib/src/bootc_composefs/update.rs index 05cb989b5..7c97c353a 100644 --- a/crates/lib/src/bootc_composefs/update.rs +++ b/crates/lib/src/bootc_composefs/update.rs @@ -17,7 +17,10 @@ use crate::bootc_composefs::gc::GCOpts; use crate::spec::BootloaderKind; use crate::{ bootc_composefs::{ - boot::{BootSetupType, BootType, setup_composefs_bls_boot, setup_composefs_uki_boot}, + boot::{ + BootSetupType, BootType, UKIDigestMismatch, print_uki_dumpfile_diff, + setup_composefs_bls_boot, setup_composefs_uki_boot, + }, gc::composefs_gc, repo::pull_composefs_repo, service::start_finalize_stated_svc, @@ -263,6 +266,7 @@ pub(crate) async fn do_upgrade( entries, id, manifest_digest, + fs: oci_fs, } = pull_composefs_repo( imgref, booted_cfs.cmdline.allow_missing_fsverity, @@ -320,12 +324,25 @@ pub(crate) async fn do_upgrade( &mounted_fs, )?, - BootType::Uki => setup_composefs_uki_boot( - BootSetupType::Upgrade((storage, booted_cfs, &host)), - &repo, - &id, - entries, - )?, + BootType::Uki => { + let uki_setup_result = setup_composefs_uki_boot( + BootSetupType::Upgrade((storage, booted_cfs, &host)), + &repo, + &id, + entries, + ); + + match uki_setup_result { + Ok(boot_digest) => boot_digest, + Err(e) => match e.downcast::() { + Ok(mismatch) => { + print_uki_dumpfile_diff(&mismatch, &repo, &oci_fs); + return Err(mismatch.into()); + } + Err(e) => Err(e)?, + }, + } + } }; // `repo` holds its own flock(LOCK_SH) on /sysroot/composefs, taken out by From 629ed65b20e81e04ee3ae0d8a99fd79173387047 Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Fri, 7 Aug 2026 11:19:26 +0530 Subject: [PATCH 2/2] tmt: Fix and re-enable uki-dumpfile test We were not computing the bootable digest which caused assertions to fail Signed-off-by: Pragyan Poudyal --- tmt/plans/integration.fmf | 2 +- tmt/tests/booted/test-composefs-uki-dumpfile.nu | 9 +++++---- tmt/tests/tests.fmf | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/tmt/plans/integration.fmf b/tmt/plans/integration.fmf index 271d862ef..20a1ce693 100644 --- a/tmt/plans/integration.fmf +++ b/tmt/plans/integration.fmf @@ -295,7 +295,7 @@ execute: - /tmt/tests/tests/test-46-etc-merge-conflict /plan-48-composefs-uki-dumpfile: - summary: Test composefs garbage collection for UKI + summary: Test composefs UKI dumpfile diff print discover: how: fmf test: diff --git a/tmt/tests/booted/test-composefs-uki-dumpfile.nu b/tmt/tests/booted/test-composefs-uki-dumpfile.nu index 8ba286364..e238eee7b 100644 --- a/tmt/tests/booted/test-composefs-uki-dumpfile.nu +++ b/tmt/tests/booted/test-composefs-uki-dumpfile.nu @@ -1,13 +1,14 @@ # number: 48 # tmt: -# summary: Test composefs garbage collection for UKI +# summary: Test composefs UKI dumpfile diff print # duration: 30m use std assert use tap.nu -# FIXME(Johan-Liebert1): This job is disabled for now -exit 0 +if not (tap is_composefs) { + exit 0 +} # bootc status let st = bootc status --json | from json @@ -37,7 +38,7 @@ def first_boot [] { let result = do { bootc switch --transport containers-storage localhost/dump-diff } | complete - let actual_digest = bootc internals cfs oci compute-id $"@(podman images --no-trunc | grep dump-diff | awk '{print $3}')" + let actual_digest = bootc internals cfs oci compute-id --bootable $"@(podman images --no-trunc | grep dump-diff | awk '{print $3}')" assert ($result.exit_code != 0) "bootc switch should fail" diff --git a/tmt/tests/tests.fmf b/tmt/tests/tests.fmf index 486e96cfa..2cae2db9e 100644 --- a/tmt/tests/tests.fmf +++ b/tmt/tests/tests.fmf @@ -185,6 +185,6 @@ check: test: nu booted/test-etc-merge-conflict.nu /test-48-composefs-uki-dumpfile: - summary: Test composefs garbage collection for UKI + summary: Test composefs UKI dumpfile diff print duration: 30m test: nu booted/test-composefs-uki-dumpfile.nu