From ddfa14f982e2956881d69ff082cd6aaadc662f38 Mon Sep 17 00:00:00 2001 From: Joseph Marrero Corchado Date: Thu, 23 Apr 2026 15:32:43 -0400 Subject: [PATCH] loader-entries: Add composefs support for set-options-for-source The ostree path for set-options-for-source (merged in fb8c6682) stages a new deployment via ostree APIs. On composefs-booted systems this approach cannot work because the status detection logic treats any entry whose verity matches the booted deployment as "booted" rather than "staged", making same-image kargs-only changes invisible to finalization. Instead, the composefs path directly modifies BLS entry files on /boot. This is architecturally correct because bootc already manages BLS entries directly on composefs systems, and the BLSConfig.extra HashMap already preserves x-options-source-* extension keys through parse/write roundtrips. No GVariant serialization, ostree version gating, or finalization service is needed. Changes: - Add composefs implementation in bootc_composefs/loader_entries.rs that reads the booted BLS entry, computes the merged options via the shared parsing functions, and writes the updated entry back atomically. When staged entries exist (e.g. from bootc upgrade), they are also updated so finalization does not overwrite the kargs changes. - Refactor CLI dispatch to use match storage.kind() for routing between the ostree and composefs backends. - Make SourceName, OPTIONS_SOURCE_KEY_PREFIX, extract_source_options_from_bls, and compute_merged_options pub(crate) for reuse by the composefs module. - Bail with a clear error on UKI boot type since kargs are embedded in the PE binary. - Add composefs-specific TMT test (test-43) with 4 phases / 3 reboots covering input validation, kargs persistence, source replacement, multiple sources, idempotency, and source removal. - Add missing fixme_skip_if_composefs metadata to the existing ostree test file header so the TMT generator preserves the skip flag. - 8 new unit tests for the composefs path, all existing tests unchanged. Closes: https://github.com/bootc-dev/bootc/issues/899 Assisted-by: OpenCode (Claude Opus 4.6) --- .../lib/src/bootc_composefs/loader_entries.rs | 496 ++++++++++++++++++ crates/lib/src/bootc_composefs/mod.rs | 1 + crates/lib/src/cli.rs | 24 +- crates/lib/src/loader_entries.rs | 12 +- tmt/plans/integration.fmf | 7 + .../test-composefs-loader-entries-source.nu | 209 ++++++++ .../booted/test-loader-entries-source.nu | 2 + tmt/tests/tests.fmf | 5 + 8 files changed, 744 insertions(+), 12 deletions(-) create mode 100644 crates/lib/src/bootc_composefs/loader_entries.rs create mode 100644 tmt/tests/booted/test-composefs-loader-entries-source.nu diff --git a/crates/lib/src/bootc_composefs/loader_entries.rs b/crates/lib/src/bootc_composefs/loader_entries.rs new file mode 100644 index 000000000..5b2ea429e --- /dev/null +++ b/crates/lib/src/bootc_composefs/loader_entries.rs @@ -0,0 +1,496 @@ +//! # Composefs backend for source-tracked kernel arguments +//! +//! This module implements `set-options-for-source` for composefs-booted systems. +//! Unlike the ostree path (which stages a new deployment via ostree APIs), the +//! composefs path directly modifies BLS entry files on /boot. This is both +//! simpler and more efficient: +//! +//! - `BLSConfig.extra` already preserves `x-options-source-*` keys through +//! parse/write roundtrips +//! - No GVariant serialization, no ostree version gating, no finalization needed +//! - Kargs take effect on next boot without requiring a finalization service +//! +//! When a staged deployment exists (e.g. from `bootc upgrade`), the staged BLS +//! entries are also updated so finalization doesn't overwrite the kargs changes. +//! +//! See + +use anyhow::{Context, Result}; +use bootc_kernel_cmdline::utf8::CmdlineOwned; +use cap_std_ext::cap_std::fs::Dir; +use cap_std_ext::dirext::CapStdExtDirExt; +use fn_error_context::context; +use std::collections::BTreeMap; + +use super::state::get_booted_bls; +use super::status::get_sorted_staged_type1_boot_entries; +use crate::composefs_consts::TYPE1_ENT_PATH_STAGED; +use crate::loader_entries::{OPTIONS_SOURCE_KEY_PREFIX, SourceName, compute_merged_options}; +use crate::parsers::bls_config::{BLSConfig, BLSConfigType, parse_bls_config}; +use crate::store::{BootedComposefs, Storage}; + +/// Extract source options from a parsed `BLSConfig`'s `extra` HashMap. +fn extract_source_options_from_extra(bls: &BLSConfig) -> BTreeMap { + let mut sources = BTreeMap::new(); + for (key, value) in &bls.extra { + if let Some(name) = key.strip_prefix(OPTIONS_SOURCE_KEY_PREFIX) { + if !name.is_empty() && !value.is_empty() { + sources.insert(name.to_string(), CmdlineOwned::from(value.clone())); + } + } + } + sources +} + +/// Get the current options string from a BLS config. +fn get_options_str(bls: &BLSConfig) -> Result { + match &bls.cfg_type { + BLSConfigType::NonEFI { options, .. } => { + Ok(options.as_ref().map(|o| o.to_string()).unwrap_or_default()) + } + _ => anyhow::bail!( + "BLS entry is not a NonEFI (BLS) type; \ + source-tracked kargs are only supported with BLS boot entries" + ), + } +} + +/// Update a BLS config's options line and source keys in place. +fn update_bls_config( + bls: &mut BLSConfig, + merged_options: &CmdlineOwned, + source: &SourceName, + new_options: Option<&str>, + source_options: &BTreeMap, +) -> Result<()> { + // Update the options line + match &mut bls.cfg_type { + BLSConfigType::NonEFI { options, .. } => { + *options = Some(merged_options.clone()); + } + _ => anyhow::bail!("BLS entry is not a NonEFI (BLS) type"), + } + + // Clear all known source keys, then re-set the ones we want to keep. + for name in source_options.keys() { + let key = format!("{OPTIONS_SOURCE_KEY_PREFIX}{name}"); + bls.extra.remove(&key); + } + // Re-set the keys we want to keep (all except the one being modified) + for (name, value) in source_options { + if name != &**source { + let key = format!("{OPTIONS_SOURCE_KEY_PREFIX}{name}"); + bls.extra.insert(key, value.to_string()); + } + } + // Set the new/updated source key + let source_key = source.bls_key(); + match new_options { + Some(opts) => { + bls.extra.insert(source_key, opts.to_string()); + } + None => { + // Removal: ensure the key is not present + bls.extra.remove(&source_key); + } + } + + Ok(()) +} + +/// Read, update, and write back a BLS entry file in a directory. +/// +/// Finds the entry matching the given version, parses it, applies the +/// source kargs change, and writes it back atomically. +fn update_bls_entry_in_dir( + entries_dir: &Dir, + target_version: &str, + source: &SourceName, + new_options: Option<&str>, +) -> Result { + for entry in entries_dir.entries_utf8()? { + let entry = entry?; + let file_name = entry.file_name()?; + if !file_name.ends_with(".conf") { + continue; + } + let content = entries_dir + .read_to_string(&file_name) + .with_context(|| format!("Reading BLS entry {file_name}"))?; + let mut bls = + parse_bls_config(&content).with_context(|| format!("Parsing BLS entry {file_name}"))?; + + if bls.version().to_string() != target_version { + continue; + } + + // Skip EFI/UKI entries — can't modify their options + if !matches!(bls.cfg_type, BLSConfigType::NonEFI { .. }) { + continue; + } + + let current_options = get_options_str(&bls)?; + let source_options = extract_source_options_from_extra(&bls); + let merged = compute_merged_options(¤t_options, &source_options, source, new_options); + + update_bls_config(&mut bls, &merged, source, new_options, &source_options)?; + + entries_dir + .atomic_write(&file_name, bls.to_string().as_bytes()) + .with_context(|| format!("Writing updated BLS entry {file_name}"))?; + + tracing::info!("Updated BLS entry '{file_name}' with kargs for source '{source}'"); + return Ok(true); + } + Ok(false) +} + +/// Set the kernel arguments for a specific source on a composefs-booted system. +/// +/// This directly modifies the BLS entry files on /boot rather than staging a +/// new deployment. The `x-options-source-*` keys are stored in the BLS +/// `extra` HashMap, which round-trips through parse/Display automatically. +/// +/// If a staged deployment exists (e.g. from `bootc upgrade`), the staged BLS +/// entries are also updated so finalization doesn't overwrite the changes. +#[context("Setting options for source '{source}' (composefs)")] +pub(crate) fn set_options_for_source( + storage: &Storage, + booted_cfs: &BootedComposefs, + source: &str, + new_options: Option<&str>, +) -> Result<()> { + let source = SourceName::parse(source)?; + let boot_dir = storage.require_boot_dir()?; + + // Get the booted BLS entry to determine boot type and read source keys + let booted_bls = get_booted_bls(boot_dir, booted_cfs)?; + + // Bail on UKI/EFI boot type — kargs are embedded in the PE binary + if matches!(booted_bls.cfg_type, BLSConfigType::EFI { .. }) { + anyhow::bail!( + "Source-tracked kargs are not supported with UKI boot entries; \ + kernel arguments are embedded in the UKI PE binary" + ); + } + + // Check for existing staged entries — if present, we work from those + // so we don't lose changes from a pending upgrade. + let staged_entries = get_sorted_staged_type1_boot_entries(boot_dir, true)?; + let has_staged = !staged_entries.is_empty(); + + // Determine the "current" BLS config to compute idempotency from. + // If staged entries exist, use the primary staged entry (highest priority). + // Otherwise, use the booted entry. + let current_bls = if has_staged { + staged_entries.last().expect("staged_entries is non-empty") + } else { + &booted_bls + }; + + // Read current options and source keys for idempotency check + let current_options = get_options_str(current_bls)?; + let source_options = extract_source_options_from_extra(current_bls); + + // Compute merged options + let merged = compute_merged_options(¤t_options, &source_options, &source, new_options); + + // Check for idempotency + let merged_str = merged.to_string(); + let is_options_unchanged = merged_str == current_options; + let is_source_unchanged = match (source_options.get(&*source), new_options) { + (Some(old), Some(new)) => &**old == new, + (None, None) | (None, Some("")) => true, + _ => false, + }; + + if is_options_unchanged && is_source_unchanged { + tracing::info!("No changes needed for source '{source}'"); + return Ok(()); + } + + let booted_version = booted_bls.version().to_string(); + + // Update the booted BLS entry in loader/entries/ + let entries_dir = boot_dir + .open_dir("loader/entries") + .context("Opening loader/entries")?; + + if !update_bls_entry_in_dir(&entries_dir, &booted_version, &source, new_options)? { + anyhow::bail!( + "Could not find BLS entry for booted deployment (version '{booted_version}')" + ); + } + + // If staged entries exist, also update them so finalization doesn't + // overwrite our changes. + if has_staged { + let staged_dir = boot_dir + .open_dir(TYPE1_ENT_PATH_STAGED) + .context("Opening staged entries directory")?; + + for staged_bls in &staged_entries { + let staged_version = staged_bls.version().to_string(); + // Update each staged entry (best effort — some may be rollback + // entries that share our version) + let _ = update_bls_entry_in_dir(&staged_dir, &staged_version, &source, new_options); + } + } + + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::loader_entries::extract_source_options_from_bls; + + /// Helper to create a BLS config string with source keys + fn make_bls(options: &str, source_keys: &[(&str, &str)]) -> String { + let mut s = format!( + "title Test OS\n\ + version 42.0\n\ + linux /vmlinuz\n\ + initrd /initramfs.img\n\ + options {options}\n" + ); + for (key, value) in source_keys { + s.push_str(&format!("x-options-source-{key} {value}\n")); + } + s + } + + #[test] + fn test_extract_source_options_from_extra() { + let bls_text = make_bls( + "root=UUID=abc rw nohz=full isolcpus=1-3", + &[("tuned", "nohz=full isolcpus=1-3"), ("admin", "quiet")], + ); + let bls = parse_bls_config(&bls_text).unwrap(); + let sources = extract_source_options_from_extra(&bls); + assert_eq!(sources.len(), 2); + assert_eq!(&*sources["tuned"], "nohz=full isolcpus=1-3"); + assert_eq!(&*sources["admin"], "quiet"); + } + + #[test] + fn test_extract_source_options_from_extra_empty_value() { + let bls_text = "title Test\nversion 1\nlinux /vmlinuz\noptions root=UUID=abc\n\ + x-options-source-tuned \n"; + let bls = parse_bls_config(bls_text).unwrap(); + let sources = extract_source_options_from_extra(&bls); + assert!(sources.is_empty(), "empty value should be filtered out"); + } + + #[test] + fn test_extract_source_options_from_extra_no_sources() { + let bls_text = make_bls("root=UUID=abc rw", &[]); + let bls = parse_bls_config(&bls_text).unwrap(); + let sources = extract_source_options_from_extra(&bls); + assert!(sources.is_empty()); + } + + #[test] + fn test_update_bls_config_add_source() { + let bls_text = make_bls("root=UUID=abc rw composefs=digest123", &[]); + let mut bls = parse_bls_config(&bls_text).unwrap(); + let source = SourceName::parse("tuned").unwrap(); + let source_options = BTreeMap::new(); + let merged = compute_merged_options( + "root=UUID=abc rw composefs=digest123", + &source_options, + &source, + Some("nohz=full isolcpus=1-3"), + ); + + update_bls_config( + &mut bls, + &merged, + &source, + Some("nohz=full isolcpus=1-3"), + &source_options, + ) + .unwrap(); + + // Verify options updated + let opts = get_options_str(&bls).unwrap(); + assert!(opts.contains("nohz=full"), "should contain nohz=full"); + assert!(opts.contains("isolcpus=1-3"), "should contain isolcpus=1-3"); + assert!( + opts.contains("root=UUID=abc"), + "should preserve system kargs" + ); + + // Verify source key set + assert_eq!( + bls.extra.get("x-options-source-tuned").unwrap(), + "nohz=full isolcpus=1-3" + ); + } + + #[test] + fn test_update_bls_config_replace_source() { + let bls_text = make_bls( + "root=UUID=abc rw nohz=full isolcpus=1-3", + &[("tuned", "nohz=full isolcpus=1-3")], + ); + let mut bls = parse_bls_config(&bls_text).unwrap(); + let source = SourceName::parse("tuned").unwrap(); + let mut source_options = BTreeMap::new(); + source_options.insert( + "tuned".to_string(), + CmdlineOwned::from("nohz=full isolcpus=1-3".to_string()), + ); + let merged = compute_merged_options( + "root=UUID=abc rw nohz=full isolcpus=1-3", + &source_options, + &source, + Some("nohz=on rcu_nocbs=2-7"), + ); + + update_bls_config( + &mut bls, + &merged, + &source, + Some("nohz=on rcu_nocbs=2-7"), + &source_options, + ) + .unwrap(); + + let opts = get_options_str(&bls).unwrap(); + assert!(!opts.contains("nohz=full"), "old nohz=full should be gone"); + assert!( + !opts.contains("isolcpus=1-3"), + "old isolcpus=1-3 should be gone" + ); + assert!(opts.contains("nohz=on"), "new nohz=on should be present"); + assert!( + opts.contains("rcu_nocbs=2-7"), + "new rcu_nocbs=2-7 should be present" + ); + assert_eq!( + bls.extra.get("x-options-source-tuned").unwrap(), + "nohz=on rcu_nocbs=2-7" + ); + } + + #[test] + fn test_update_bls_config_remove_source() { + let bls_text = make_bls("root=UUID=abc rw nohz=full", &[("tuned", "nohz=full")]); + let mut bls = parse_bls_config(&bls_text).unwrap(); + let source = SourceName::parse("tuned").unwrap(); + let mut source_options = BTreeMap::new(); + source_options.insert( + "tuned".to_string(), + CmdlineOwned::from("nohz=full".to_string()), + ); + let merged = + compute_merged_options("root=UUID=abc rw nohz=full", &source_options, &source, None); + + update_bls_config(&mut bls, &merged, &source, None, &source_options).unwrap(); + + let opts = get_options_str(&bls).unwrap(); + assert!(!opts.contains("nohz=full"), "source karg should be removed"); + assert!( + opts.contains("root=UUID=abc"), + "system kargs should be preserved" + ); + assert!( + !bls.extra.contains_key("x-options-source-tuned"), + "source key should be removed" + ); + } + + #[test] + fn test_update_bls_config_multiple_sources() { + let bls_text = make_bls( + "root=UUID=abc rw nohz=full rd.driver.pre=vfio-pci", + &[("tuned", "nohz=full"), ("dracut", "rd.driver.pre=vfio-pci")], + ); + let mut bls = parse_bls_config(&bls_text).unwrap(); + let source = SourceName::parse("tuned").unwrap(); + let mut source_options = BTreeMap::new(); + source_options.insert( + "tuned".to_string(), + CmdlineOwned::from("nohz=full".to_string()), + ); + source_options.insert( + "dracut".to_string(), + CmdlineOwned::from("rd.driver.pre=vfio-pci".to_string()), + ); + let merged = compute_merged_options( + "root=UUID=abc rw nohz=full rd.driver.pre=vfio-pci", + &source_options, + &source, + Some("isolcpus=1-3"), + ); + + update_bls_config( + &mut bls, + &merged, + &source, + Some("isolcpus=1-3"), + &source_options, + ) + .unwrap(); + + let opts = get_options_str(&bls).unwrap(); + assert!( + !opts.contains("nohz=full"), + "old tuned karg should be removed" + ); + assert!( + opts.contains("isolcpus=1-3"), + "new tuned karg should be present" + ); + assert!( + opts.contains("rd.driver.pre=vfio-pci"), + "dracut karg should be preserved" + ); + assert_eq!( + bls.extra.get("x-options-source-tuned").unwrap(), + "isolcpus=1-3" + ); + assert_eq!( + bls.extra.get("x-options-source-dracut").unwrap(), + "rd.driver.pre=vfio-pci" + ); + } + + #[test] + fn test_bls_roundtrip_preserves_source_keys() { + let bls_text = make_bls("root=UUID=abc rw nohz=full", &[("tuned", "nohz=full")]); + let bls = parse_bls_config(&bls_text).unwrap(); + + // Roundtrip: serialize and re-parse + let serialized = bls.to_string(); + let reparsed = parse_bls_config(&serialized).unwrap(); + + assert_eq!( + reparsed.extra.get("x-options-source-tuned").unwrap(), + "nohz=full" + ); + let sources = extract_source_options_from_extra(&reparsed); + assert_eq!(sources.len(), 1); + assert_eq!(&*sources["tuned"], "nohz=full"); + } + + #[test] + fn test_extract_matches_bls_text_parser() { + // Verify that extracting from BLSConfig.extra gives the same + // result as extracting from raw BLS text + let bls_text = make_bls( + "root=UUID=abc rw nohz=full isolcpus=1-3", + &[("tuned", "nohz=full isolcpus=1-3"), ("admin", "quiet")], + ); + let bls = parse_bls_config(&bls_text).unwrap(); + + let from_extra = extract_source_options_from_extra(&bls); + let from_text = extract_source_options_from_bls(&bls_text); + + assert_eq!(from_extra.len(), from_text.len()); + for (name, value) in &from_extra { + assert_eq!(&**value, &*from_text[name], "mismatch for source '{name}'"); + } + } +} diff --git a/crates/lib/src/bootc_composefs/mod.rs b/crates/lib/src/bootc_composefs/mod.rs index 0660cdcd7..9467750e8 100644 --- a/crates/lib/src/bootc_composefs/mod.rs +++ b/crates/lib/src/bootc_composefs/mod.rs @@ -4,6 +4,7 @@ pub(crate) mod digest; pub(crate) mod export; pub(crate) mod finalize; pub(crate) mod gc; +pub(crate) mod loader_entries; pub(crate) mod repo; pub(crate) mod rollback; pub(crate) mod selinux; diff --git a/crates/lib/src/cli.rs b/crates/lib/src/cli.rs index e4fa2a70c..39a41e5e4 100644 --- a/crates/lib/src/cli.rs +++ b/crates/lib/src/cli.rs @@ -1955,12 +1955,24 @@ async fn run_from_opt(opt: Opt) -> Result<()> { Opt::LoaderEntries(opts) => match opts { LoaderEntriesOpts::SetOptionsForSource(opts) => { let storage = get_storage().await?; - let sysroot = storage.get_ostree()?; - crate::loader_entries::set_options_for_source_staged( - sysroot, - &opts.source, - opts.options.as_deref(), - )?; + match storage.kind()? { + BootedStorageKind::Ostree(_) => { + let sysroot = storage.get_ostree()?; + crate::loader_entries::set_options_for_source_staged( + sysroot, + &opts.source, + opts.options.as_deref(), + )?; + } + BootedStorageKind::Composefs(booted_cfs) => { + crate::bootc_composefs::loader_entries::set_options_for_source( + &storage, + &booted_cfs, + &opts.source, + opts.options.as_deref(), + )?; + } + } Ok(()) } }, diff --git a/crates/lib/src/loader_entries.rs b/crates/lib/src/loader_entries.rs index 0db9f8756..708244b3c 100644 --- a/crates/lib/src/loader_entries.rs +++ b/crates/lib/src/loader_entries.rs @@ -17,17 +17,17 @@ use ostree_ext::ostree; use std::collections::BTreeMap; /// The BLS extension key prefix for source-tracked options. -const OPTIONS_SOURCE_KEY_PREFIX: &str = "x-options-source-"; +pub(crate) const OPTIONS_SOURCE_KEY_PREFIX: &str = "x-options-source-"; /// A validated source name (alphanumeric + hyphens + underscores, non-empty). /// /// This is a newtype wrapper around `String` that enforces validation at /// construction time. See . -struct SourceName(String); +pub(crate) struct SourceName(String); impl SourceName { /// Parse and validate a source name. - fn parse(source: &str) -> Result { + pub(crate) fn parse(source: &str) -> Result { ensure!(!source.is_empty(), "Source name must not be empty"); ensure!( source @@ -39,7 +39,7 @@ impl SourceName { } /// The BLS key for this source (e.g., `x-options-source-tuned`). - fn bls_key(&self) -> String { + pub(crate) fn bls_key(&self) -> String { format!("{OPTIONS_SOURCE_KEY_PREFIX}{}", self.0) } } @@ -59,7 +59,7 @@ impl std::fmt::Display for SourceName { /// Extract source options from BLS entry content. Parses `x-options-source-*` keys /// from the raw BLS text since the ostree BootconfigParser doesn't expose key iteration. -fn extract_source_options_from_bls(content: &str) -> BTreeMap { +pub(crate) fn extract_source_options_from_bls(content: &str) -> BTreeMap { let mut sources = BTreeMap::new(); for line in content.lines() { let line = line.trim(); @@ -89,7 +89,7 @@ fn extract_source_options_from_bls(content: &str) -> BTreeMap, target_source: &SourceName, diff --git a/tmt/plans/integration.fmf b/tmt/plans/integration.fmf index 8a4850974..2ca496092 100644 --- a/tmt/plans/integration.fmf +++ b/tmt/plans/integration.fmf @@ -254,4 +254,11 @@ execute: test: - /tmt/tests/tests/test-42-loader-entries-source extra-fixme_skip_if_composefs: true + +/plan-43-composefs-loader-entries-source: + summary: Test bootc loader-entries set-options-for-source on composefs + discover: + how: fmf + test: + - /tmt/tests/tests/test-43-composefs-loader-entries-source # END GENERATED PLANS diff --git a/tmt/tests/booted/test-composefs-loader-entries-source.nu b/tmt/tests/booted/test-composefs-loader-entries-source.nu new file mode 100644 index 000000000..0171fae34 --- /dev/null +++ b/tmt/tests/booted/test-composefs-loader-entries-source.nu @@ -0,0 +1,209 @@ +# number: 43 +# tmt: +# summary: Test bootc loader-entries set-options-for-source on composefs +# duration: 30m +# +# This test verifies source-tracked kernel argument management on composefs- +# booted systems. The composefs path directly modifies BLS entry files on +# /boot rather than staging a new ostree deployment. It covers: +# 1. Input validation (invalid/empty source names) +# 2. Adding source-tracked kargs and verifying they appear in /proc/cmdline +# 3. Source keys (x-options-source-*) in BLS entry files +# 4. Source replacement semantics (old kargs removed, new ones added) +# 5. Multiple sources coexisting independently +# 6. Source removal (--source without --options clears all owned kargs) +# 7. Idempotent operation (no changes when kargs already match) +# 8. Existing system kargs preserved through changes +# +# This test is composefs-specific. It exits 0 (skip) on ostree-booted systems. +# The UKI boot type is also skipped since kargs are embedded in the PE binary. +# +# See: https://github.com/bootc-dev/bootc/issues/899 +use std assert +use tap.nu + +# Skip if not composefs-booted +if not (tap is_composefs) { + print "Not a composefs system, skipping" + exit 0 +} + +# Skip if UKI boot type — kargs are embedded in the PE binary +let st = bootc status --json | from json +let boot_type = $st.status.booted.composefs?.bootType? | default "bls" +if ($boot_type | str downcase) == "uki" { + print "UKI boot type, skipping (kargs embedded in PE binary)" + exit 0 +} + +def parse_cmdline [] { + open /proc/cmdline | str trim | split row " " +} + +# Read x-options-source-* keys from the booted BLS entry. +# On composefs, entries are named bootc_*.conf (not ostree-*.conf). +def read_bls_source_keys [] { + let entries = glob /boot/loader/entries/bootc_*.conf | sort + if ($entries | length) == 0 { + error make { msg: "No composefs BLS entries found" } + } + let entry = open ($entries | last) + $entry | lines | where { |line| $line starts-with "x-options-source-" } +} + +# Save the current system kargs for later comparison +def save_system_kargs [] { + let cmdline = parse_cmdline + let system_kargs = $cmdline | where { |k| + (($k starts-with "root=") or ($k == "rw") or ($k starts-with "console=")) + } + $system_kargs | to json | save -f /var/bootc-test-system-kargs.json +} + +def load_system_kargs [] { + open /var/bootc-test-system-kargs.json +} + +def first_boot [] { + tap begin "composefs loader-entries set-options-for-source" + + save_system_kargs + + # -- Input validation (same as ostree test) -- + + let r = do -i { bootc loader-entries set-options-for-source --source "bad name" --options "foo=bar" } | complete + assert ($r.exit_code != 0) "spaces in source name should fail" + + let r = do -i { bootc loader-entries set-options-for-source --source "foo@bar" --options "foo=bar" } | complete + assert ($r.exit_code != 0) "special chars in source name should fail" + + let r = do -i { bootc loader-entries set-options-for-source --source "" --options "foo=bar" } | complete + assert ($r.exit_code != 0) "empty source name should fail" + + # Valid name with underscores/dashes + bootc loader-entries set-options-for-source --source "my_custom-src" --options "testvalid=1" + # Clear it immediately + bootc loader-entries set-options-for-source --source "my_custom-src" + + # -- Add source kargs -- + # On composefs, this directly modifies the BLS entry (no staging) + bootc loader-entries set-options-for-source --source tuned --options "nohz=full isolcpus=1-3" + + # Verify the BLS entry was updated immediately (composefs writes directly) + let source_keys = read_bls_source_keys + let tuned_key = $source_keys | where { |line| $line starts-with "x-options-source-tuned" } + assert (($tuned_key | length) > 0) "x-options-source-tuned should be in BLS entry immediately" + print "ok: source key written to BLS entry" + + # Add a second source + bootc loader-entries set-options-for-source --source admin --options "quiet" + + # Verify both source keys present + let source_keys = read_bls_source_keys + let admin_key = $source_keys | where { |line| $line starts-with "x-options-source-admin" } + assert (($admin_key | length) > 0) "x-options-source-admin should be in BLS entry" + print "ok: multiple sources written" + + print "ok: validation and initial BLS update" + tmt-reboot +} + +def second_boot [] { + # Verify kargs survived reboot + let cmdline = parse_cmdline + assert ("nohz=full" in $cmdline) "nohz=full should be in cmdline after reboot" + assert ("isolcpus=1-3" in $cmdline) "isolcpus=1-3 should be in cmdline after reboot" + assert ("quiet" in $cmdline) "admin quiet karg should be in cmdline after reboot" + print "ok: kargs survived reboot" + + # Verify system kargs preserved + let system_kargs = load_system_kargs + for karg in $system_kargs { + assert ($karg in $cmdline) $"system karg '($karg)' must be preserved" + } + print "ok: system kargs preserved" + + # Verify source keys in BLS entry + let source_keys = read_bls_source_keys + let tuned_key = $source_keys | where { |line| $line starts-with "x-options-source-tuned" } + assert (($tuned_key | length) > 0) "x-options-source-tuned should be in BLS entry" + let tuned_line = $tuned_key | first + assert ($tuned_line | str contains "nohz=full") "tuned source key should contain nohz=full" + assert ($tuned_line | str contains "isolcpus=1-3") "tuned source key should contain isolcpus=1-3" + print "ok: source keys persisted across reboot" + + # -- Source replacement: new kargs replace old ones -- + # Clean up admin source first + bootc loader-entries set-options-for-source --source admin + + # Replace tuned kargs + bootc loader-entries set-options-for-source --source tuned --options "nohz=on rcu_nocbs=2-7" + + tmt-reboot +} + +def third_boot [] { + # Verify replacement worked + let cmdline = parse_cmdline + assert ("nohz=full" not-in $cmdline) "old nohz=full should be gone" + assert ("isolcpus=1-3" not-in $cmdline) "old isolcpus=1-3 should be gone" + assert ("nohz=on" in $cmdline) "new nohz=on should be present" + assert ("rcu_nocbs=2-7" in $cmdline) "new rcu_nocbs=2-7 should be present" + assert ("quiet" not-in $cmdline) "admin quiet should be gone after removal" + + # Verify system kargs still preserved + let system_kargs = load_system_kargs + for karg in $system_kargs { + assert ($karg in $cmdline) $"system karg '($karg)' must survive replacement" + } + print "ok: source replacement persisted, system kargs preserved" + + # -- Multiple sources coexist -- + bootc loader-entries set-options-for-source --source dracut --options "rd.driver.pre=vfio-pci" + + # -- Idempotent: same kargs again should be a no-op -- + # On composefs, idempotency means the BLS file is not rewritten + bootc loader-entries set-options-for-source --source tuned --options "nohz=on rcu_nocbs=2-7" + # (No easy way to detect no-write on composefs, but the command should succeed silently) + print "ok: idempotent operation succeeded" + + # -- Source removal -- + bootc loader-entries set-options-for-source --source dracut + + # Verify dracut removed, tuned preserved (check BLS immediately) + let source_keys = read_bls_source_keys + let dracut_keys = $source_keys | where { |line| $line starts-with "x-options-source-dracut" } + assert (($dracut_keys | length) == 0) "dracut source key should be gone after removal" + let tuned_keys = $source_keys | where { |line| $line starts-with "x-options-source-tuned" } + assert (($tuned_keys | length) > 0) "tuned source key should still exist" + print "ok: source removal and coexistence verified" + + tmt-reboot +} + +def fourth_boot [] { + # Final verification after reboot + let cmdline = parse_cmdline + assert ("rd.driver.pre=vfio-pci" not-in $cmdline) "dracut karg should be gone" + assert ("nohz=on" in $cmdline) "tuned nohz=on should still be present" + assert ("rcu_nocbs=2-7" in $cmdline) "tuned rcu_nocbs=2-7 should still be present" + + # Verify system kargs intact through all phases + let system_kargs = load_system_kargs + for karg in $system_kargs { + assert ($karg in $cmdline) $"system karg '($karg)' must survive all phases" + } + print "ok: all phases completed, system kargs preserved" + + tap ok +} + +def main [] { + match $env.TMT_REBOOT_COUNT? { + null | "0" => first_boot, + "1" => second_boot, + "2" => third_boot, + "3" => fourth_boot, + $o => { error make { msg: $"Unexpected TMT_REBOOT_COUNT ($o)" } }, + } +} diff --git a/tmt/tests/booted/test-loader-entries-source.nu b/tmt/tests/booted/test-loader-entries-source.nu index 6a6223bc5..b52656798 100644 --- a/tmt/tests/booted/test-loader-entries-source.nu +++ b/tmt/tests/booted/test-loader-entries-source.nu @@ -1,4 +1,6 @@ # number: 42 +# extra: +# fixme_skip_if_composefs: true # tmt: # summary: Test bootc loader-entries set-options-for-source # duration: 30m diff --git a/tmt/tests/tests.fmf b/tmt/tests/tests.fmf index bce8daddc..9475c54ab 100644 --- a/tmt/tests/tests.fmf +++ b/tmt/tests/tests.fmf @@ -158,3 +158,8 @@ check: summary: Test bootc loader-entries set-options-for-source duration: 30m test: nu booted/test-loader-entries-source.nu + +/test-43-composefs-loader-entries-source: + summary: Test bootc loader-entries set-options-for-source on composefs + duration: 30m + test: nu booted/test-composefs-loader-entries-source.nu