diff --git a/crates/lib/src/bootc_composefs/boot.rs b/crates/lib/src/bootc_composefs/boot.rs index 72ff812a8f..4e68738a87 100644 --- a/crates/lib/src/bootc_composefs/boot.rs +++ b/crates/lib/src/bootc_composefs/boot.rs @@ -97,7 +97,7 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use crate::bootc_composefs::state::{get_booted_bls, write_composefs_state}; -use crate::bootc_composefs::status::ComposefsCmdline; +use crate::bootc_composefs::status::{ComposefsCmdline, get_sorted_staged_type1_boot_entries}; use crate::bootc_kargs::compute_new_kargs; use crate::composefs_consts::{TYPE1_BOOT_DIR_PREFIX, TYPE1_ENT_PATH, TYPE1_ENT_PATH_STAGED}; use crate::parsers::bls_config::{BLSConfig, BLSConfigType, EFIKey}; @@ -445,6 +445,49 @@ pub(crate) fn secondary_sort_key(os_id: &str) -> String { format!("bootc-{os_id}-{SORTKEY_PRIORITY_SECONDARY}") } +/// The OS id that [`primary_sort_key`] or [`secondary_sort_key`] encoded in +/// an entry's sort key, or "bootc" for an entry without one. +pub(crate) fn os_id_from_sort_key(entry: &BLSConfig) -> &str { + entry + .sort_key + .as_deref() + .and_then(|key| key.strip_prefix("bootc-")) + .and_then(|key| { + key.strip_suffix(&format!("-{SORTKEY_PRIORITY_PRIMARY}")) + .or_else(|| key.strip_suffix(&format!("-{SORTKEY_PRIORITY_SECONDARY}"))) + }) + .unwrap_or("bootc") +} + +/// Write `primary` as the default entry and `secondary`, if any, as the one +/// after it into `entries_dir`, named for Grub's ordering, and sync the +/// directory. +pub(crate) fn write_type1_entries( + entries_dir: &Dir, + os_id: &str, + primary: &BLSConfig, + secondary: Option<&BLSConfig>, +) -> Result<()> { + entries_dir.atomic_write( + type1_entry_conf_file_name(os_id, &primary.version(), FILENAME_PRIORITY_PRIMARY), + primary.to_string().as_bytes(), + )?; + + if let Some(secondary) = secondary { + entries_dir.atomic_write( + type1_entry_conf_file_name(os_id, &secondary.version(), FILENAME_PRIORITY_SECONDARY), + secondary.to_string().as_bytes(), + )?; + } + + let owned_fd = entries_dir + .reopen_as_ownedfd() + .context("Reopening as owned fd")?; + rustix::fs::fsync(owned_fd).context("fsync")?; + + Ok(()) +} + /// Returns the name of the directory where we store Type1 boot entries pub(crate) fn get_type1_dir_name(depl_verity: &str) -> String { format!("{TYPE1_BOOT_DIR_PREFIX}{depl_verity}") @@ -671,7 +714,11 @@ pub(crate) fn setup_composefs_bls_boot( ) -> Result { let id_hex = id.to_hex(); - let (root_path, esp_device, mut cmdline_refs, bootloader) = match setup_type { + // The tree whose kargs.d the new one is diffed against; the booted root + // unless building on a staged deployment + let mut base_root: Option = None; + + let (root_path, esp_device, mut cmdline_refs, bootloader, inherited_extra) = match setup_type { BootSetupType::Setup((root_setup, state, postfetch)) => { // root_setup.kargs has [root=UUID=, "rw"] let mut cmdline_options = Cmdline::new(); @@ -712,6 +759,7 @@ pub(crate) fn setup_composefs_bls_boot( esp_part.path(), cmdline_options, postfetch.detected_bootloader.clone(), + std::collections::HashMap::new(), ) } @@ -719,7 +767,40 @@ pub(crate) fn setup_composefs_bls_boot( let bootloader = host.require_composefs_booted()?.bootloader.clone(); let boot_dir = storage.require_boot_dir()?; - let current_cfg = get_booted_bls(&boot_dir, booted_cfs)?; + + // Build on the pending entry when a deployment is staged, so a + // change staged earlier in this boot (`bootc loader-entries + // set-options-for-source`, a switch) is not lost when this one + // replaces it, as rpm-ostree and the ostree backend do. kargs.d + // is then diffed against that deployment's tree rather than the + // booted one. + let staged_cfg = match host.status.staged { + Some(_) => get_sorted_staged_type1_boot_entries(&boot_dir, true)? + .into_iter() + .next(), + None => None, + }; + base_root = Some( + match staged_cfg + .as_ref() + .map(|cfg| cfg.get_verity()) + .transpose()? + { + Some(verity) if *verity != *booted_cfs.cmdline.digest => Dir::reopen_dir( + &repo.mount(&verity).context("Mounting staged deployment")?, + )?, + _ => Dir::open_ambient_dir("/", ambient_authority()).context("Opening root")?, + }, + ); + let current_cfg = match staged_cfg { + Some(cfg) => cfg, + None => get_booted_bls(&boot_dir, booted_cfs)?, + }; + + // Extension keys (e.g. `x-options-source-*` written by + // `loader-entries set-options-for-source`) belong to the machine, + // not the image: carry them into the new entry like `options`. + let inherited_extra = current_cfg.extra.clone(); let mut cmdline = match current_cfg.cfg_type { BLSConfigType::NonEFI { options, .. } => { @@ -750,19 +831,14 @@ pub(crate) fn setup_composefs_bls_boot( esp_dev.path(), cmdline, bootloader, + inherited_extra, ) } }; let is_upgrade = matches!(setup_type, BootSetupType::Upgrade(..)); - let current_root = if is_upgrade { - Some(&Dir::open_ambient_dir("/", ambient_authority()).context("Opening root")? as &Dir) - } else { - None - }; - - compute_new_kargs(mounted_erofs, current_root, &mut cmdline_refs)?; + compute_new_kargs(mounted_erofs, base_root.as_ref(), &mut cmdline_refs)?; let (entry_paths, _tmpdir_guard) = match bootloader.kind()? { BootloaderKind::GRUBClassic => { @@ -842,6 +918,7 @@ pub(crate) fn setup_composefs_bls_boot( .with_title(title) .with_version(version) .with_sort_key(sort_key) + .with_extra(inherited_extra) .with_cfg(BLSConfigType::NonEFI { linux: entry_paths .abs_entries_path @@ -937,24 +1014,13 @@ pub(crate) fn setup_composefs_bls_boot( let loader_entries_dir = Dir::open_ambient_dir(&config_path, ambient_authority()) .with_context(|| format!("Opening {config_path:?}"))?; - loader_entries_dir.atomic_write( - type1_entry_conf_file_name(&os_id, &bls_config.version(), FILENAME_PRIORITY_PRIMARY), - bls_config.to_string().as_bytes(), + write_type1_entries( + &loader_entries_dir, + &os_id, + &bls_config, + booted_bls.as_ref(), )?; - if let Some(booted_bls) = booted_bls { - loader_entries_dir.atomic_write( - type1_entry_conf_file_name(&os_id, &booted_bls.version(), FILENAME_PRIORITY_SECONDARY), - booted_bls.to_string().as_bytes(), - )?; - } - - let owned_loader_entries_fd = loader_entries_dir - .reopen_as_ownedfd() - .context("Reopening as owned fd")?; - - rustix::fs::fsync(owned_loader_entries_fd).context("fsync")?; - Ok(boot_digest) } @@ -1928,4 +1994,57 @@ mod tests { "RHEL should sort before Fedora in descending order" ); } + #[test] + fn test_os_id_from_sort_key() { + let cases = [ + (Some(primary_sort_key("fedora")), "fedora"), + (Some(secondary_sort_key("fedora-coreos")), "fedora-coreos"), + (Some("something-else".to_string()), "bootc"), + (None, "bootc"), + ]; + for (sort_key, expected) in cases { + let mut entry = BLSConfig::default(); + entry.sort_key = sort_key.clone(); + assert_eq!(os_id_from_sort_key(&entry), expected, "{sort_key:?}"); + } + } + + #[test] + fn test_write_type1_entries() -> Result<()> { + let td = cap_std_ext::cap_tempfile::tempdir(ambient_authority())?; + let entry = |sort_key: String, options: &str| { + let mut e = BLSConfig::default(); + e.with_title("t".into()) + .with_version("43".into()) + .with_sort_key(sort_key) + .with_cfg(BLSConfigType::NonEFI { + linux: "/vmlinuz".into(), + initrd: vec!["/initrd".into()], + options: Some(linux_kernel_cmdline::utf8::CmdlineOwned::from( + options.to_string(), + )), + }); + e + }; + let primary = entry(primary_sort_key("fedora"), "root=/dev/a nohz=full"); + let secondary = entry(secondary_sort_key("fedora"), "root=/dev/a"); + + write_type1_entries(&td, "fedora", &primary, Some(&secondary))?; + + let mut names: Vec<_> = td + .entries()? + .map(|e| e.unwrap().file_name().to_str().unwrap().to_string()) + .collect(); + names.sort(); + assert_eq!(names, ["bootc_fedora-43-0.conf", "bootc_fedora-43-1.conf"]); + let written = crate::parsers::bls_config::parse_bls_config( + &td.read_to_string("bootc_fedora-43-1.conf")?, + )?; + assert_eq!(written, primary); + let written = crate::parsers::bls_config::parse_bls_config( + &td.read_to_string("bootc_fedora-43-0.conf")?, + )?; + assert_eq!(written, secondary); + Ok(()) + } } diff --git a/crates/lib/src/bootc_composefs/finalize.rs b/crates/lib/src/bootc_composefs/finalize.rs index 8864ca660b..05f527d98a 100644 --- a/crates/lib/src/bootc_composefs/finalize.rs +++ b/crates/lib/src/bootc_composefs/finalize.rs @@ -95,24 +95,73 @@ pub(crate) async fn composefs_backend_finalize( "Staged deployment is not a composefs deployment" ))?; + // A kernel-argument change stages the booted deployment itself with a new + // entry (`bootc loader-entries set-options-for-source`); it shares the + // state directory, so there is no /etc to merge, only entries to swap. + if staged_composefs.verity != booted_composefs.verity { + merge_etc( + storage, + booted_cfs, + &booted_composefs.verity, + &staged_composefs.verity, + )?; + } + + let boot_dir = storage.require_boot_dir()?; + + match booted_composefs.bootloader.kind()? { + BootloaderKind::GRUBClassic => match staged_composefs.boot_type { + BootType::Bls => { + let entries_dir = boot_dir.open_dir("loader")?; + rename_exchange_bls_entries(&entries_dir)?; + } + BootType::Uki => finalize_staged_grub_uki(boot_dir)?, + }, + + BootloaderKind::BLSCompatible => { + let entries_dir = boot_dir.open_dir("loader")?; + rename_exchange_bls_entries(&entries_dir)?; + } + }; + + // Now that we have successfully updated bootloader entires, we can GC the unreferenced ones + // We do not prune the composefs repository here though + composefs_gc( + storage, + booted_cfs, + GCOpts { + dry_run: false, + prune_repo: false, + }, + ) + .await?; + + Ok(()) +} + +/// Three-way merge the booted /etc into the staged deployment's state directory +#[context("Merging /etc into staged deployment")] +fn merge_etc( + storage: &Storage, + booted_cfs: &BootedComposefs, + booted_verity: &str, + staged_verity: &str, +) -> Result<()> { // Mount the booted EROFS image to get pristine etc let sysroot_fd = storage.physical_root.reopen_as_ownedfd()?; let composefs_fd = mount_composefs_image( &sysroot_fd, - &booted_composefs.verity, + booted_verity, booted_cfs.cmdline.allow_missing_fsverity, )?; let erofs_tmp_mnt = TempMount::mount_fd(&composefs_fd)?; - // Perform the /etc merge let pristine_etc = Dir::open_ambient_dir(erofs_tmp_mnt.dir.path().join("etc"), ambient_authority())?; let current_etc = Dir::open_ambient_dir("/etc", ambient_authority())?; - let new_etc_path = Path::new(STATE_DIR_ABS) - .join(&staged_composefs.verity) - .join("etc"); + let new_etc_path = Path::new(STATE_DIR_ABS).join(staged_verity).join("etc"); let new_etc = Dir::open_ambient_dir(new_etc_path, ambient_authority())?; @@ -132,38 +181,6 @@ pub(crate) async fn composefs_backend_finalize( .remove_file_optional(".updated") .context("Removing /etc/.updated from staged deployment")?; - // Unmount EROFS - drop(erofs_tmp_mnt); - - let boot_dir = storage.require_boot_dir()?; - - match booted_composefs.bootloader.kind()? { - BootloaderKind::GRUBClassic => match staged_composefs.boot_type { - BootType::Bls => { - let entries_dir = boot_dir.open_dir("loader")?; - rename_exchange_bls_entries(&entries_dir)?; - } - BootType::Uki => finalize_staged_grub_uki(boot_dir)?, - }, - - BootloaderKind::BLSCompatible => { - let entries_dir = boot_dir.open_dir("loader")?; - rename_exchange_bls_entries(&entries_dir)?; - } - }; - - // Now that we have successfully updated bootloader entires, we can GC the unreferenced ones - // We do not prune the composefs repository here though - composefs_gc( - storage, - booted_cfs, - GCOpts { - dry_run: false, - prune_repo: false, - }, - ) - .await?; - Ok(()) } 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 0000000000..bacd6f83b2 --- /dev/null +++ b/crates/lib/src/bootc_composefs/loader_entries.rs @@ -0,0 +1,392 @@ +//! # Composefs backend for source-tracked kernel arguments +//! +//! bootc owns the BLS entries on composefs systems, so there is no ostree +//! staging to go through. `set-options-for-source` still behaves like the +//! ostree backend: it stages the booted deployment again with an entry +//! carrying the merged `options` line, and the current entry becomes the +//! rollback. Finalization installs the pending entries at shutdown as for +//! any staged deployment; there is no new state directory since the +//! deployment is the same. `bootc rollback` then undoes the change. +//! +//! When an upgrade is already staged, its pending entry under +//! `loader/entries.staged/` is rewritten instead, so the change rides along +//! with it and is dropped with it. +//! +//! Removing a source deletes its `x-options-source-*` key; the ostree backend +//! leaves an empty tombstone because its parser cannot remove keys. +//! +//! See + +use anyhow::{Context, Result}; +use cap_std_ext::cap_std::ambient_authority; +use cap_std_ext::cap_std::fs::Dir; +use cap_std_ext::dirext::CapStdExtDirExt; +use fn_error_context::context; +use linux_kernel_cmdline::utf8::CmdlineOwned; +use std::collections::BTreeMap; + +use super::boot::{os_id_from_sort_key, primary_sort_key, secondary_sort_key, write_type1_entries}; +use super::service::start_finalize_stated_svc; +use super::state::{get_booted_bls, write_staged_deployment}; +use super::status::{StagedDeployment, get_composefs_status, get_sorted_type1_entries}; +use crate::composefs_consts::{ + COMPOSEFS_STAGED_DEPLOYMENT_FNAME, COMPOSEFS_TRANSIENT_STATE_DIR, TYPE1_ENT_PATH_STAGED, +}; +use crate::loader_entries::{OPTIONS_SOURCE_KEY_PREFIX, SourceName, compute_merged_options}; +use crate::parsers::bls_config::{BLSConfig, BLSConfigType}; +use crate::spec::Host; +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 +} + +/// 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>, +) -> Result<()> { + match &mut bls.cfg_type { + BLSConfigType::NonEFI { options, .. } => { + *options = Some(merged_options.clone()); + } + _ => anyhow::bail!("BLS entry is not a NonEFI (BLS) type"), + } + + let source_key = source.bls_key(); + match new_options { + Some(opts) => { + bls.extra.insert(source_key, opts.to_string()); + } + None => { + bls.extra.remove(&source_key); + } + } + + Ok(()) +} + +/// Whether setting `new_options` for `source` would change anything: the +/// options line and the source's own record are both already as requested. +fn is_unchanged( + current_options: &str, + merged: &CmdlineOwned, + source_options: &BTreeMap, + source: &SourceName, + new_options: Option<&str>, +) -> bool { + let source_unchanged = match (source_options.get(&**source), new_options) { + (Some(old), Some(new)) => &**old == new, + (None, None) | (None, Some("")) => true, + _ => false, + }; + source_unchanged && &**merged == current_options +} + +/// Whether the staged deployment is the booted one with other kernel arguments +fn is_kargs_only_staged(host: &Host, booted_cfs: &BootedComposefs) -> bool { + host.status + .staged + .as_ref() + .and_then(|s| s.composefs.as_ref()) + .is_some_and(|s| *s.verity == *booted_cfs.cmdline.digest) +} + +/// Drop the staged deployment record and its pending entries +#[context("Removing staged deployment")] +fn remove_staged_deployment(boot_dir: &Dir) -> Result<()> { + boot_dir + .remove_all_optional(TYPE1_ENT_PATH_STAGED) + .context("Removing staged entries")?; + if let Ok(transient_dir) = + Dir::open_ambient_dir(COMPOSEFS_TRANSIENT_STATE_DIR, ambient_authority()) + { + transient_dir + .remove_file_optional(COMPOSEFS_STAGED_DEPLOYMENT_FNAME) + .context("Removing staged deployment file")?; + } + Ok(()) +} + +/// Set the kernel arguments for a specific source on a composefs-booted system. +#[context("Setting options for source '{source}' (composefs)")] +pub(crate) async 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()?; + + 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" + ); + } + + // Build on the pending entry when a deployment is staged, so a pending + // upgrade keeps its kernel arguments and this change is not undone at + // finalization. + let host = get_composefs_status(storage, booted_cfs).await?; + let staged_primary = match host.status.staged { + Some(_) => get_sorted_type1_entries(boot_dir, true, true)? + .into_iter() + .next(), + None => None, + }; + let base = staged_primary + .as_ref() + .map(|entry| &entry.config) + .unwrap_or(&booted_bls); + + let current_options = base.get_cmdline()?.to_string(); + let source_options = extract_source_options_from_extra(base); + let merged = compute_merged_options(¤t_options, &source_options, &source, new_options); + + if is_unchanged( + ¤t_options, + &merged, + &source_options, + &source, + new_options, + ) { + tracing::info!("No changes needed for source '{source}'"); + return Ok(()); + } + + let mut updated = base.clone(); + update_bls_config(&mut updated, &merged, &source, new_options)?; + + match staged_primary { + // A pending kernel-argument change brought back to what is booted: + // nothing is left to finalize + Some(_) + if is_kargs_only_staged(&host, booted_cfs) + && updated.get_cmdline().ok() == booted_bls.get_cmdline().ok() + && updated.extra == booted_bls.extra => + { + remove_staged_deployment(boot_dir)?; + tracing::info!("Pending kargs change for source '{source}' reverted; nothing staged"); + } + Some(staged) => { + let staged_dir = boot_dir + .open_dir(TYPE1_ENT_PATH_STAGED) + .context("Opening staged entries directory")?; + staged_dir + .atomic_write(&staged.filename, updated.to_string().as_bytes()) + .with_context(|| format!("Writing staged BLS entry {}", staged.filename))?; + let owned = staged_dir + .reopen_as_ownedfd() + .context("Reopening as owned fd")?; + rustix::fs::fsync(owned).context("fsync")?; + tracing::info!( + "Updated staged BLS entry '{}' with kargs for source '{source}'", + staged.filename + ); + } + None => { + // Stage the booted deployment again: the new entry becomes the + // default and the current one its rollback, exactly as an upgrade + // keeps the booted entry. + let os_id = os_id_from_sort_key(&booted_bls).to_string(); + updated.sort_key = Some(primary_sort_key(&os_id)); + let mut rollback = booted_bls.clone(); + rollback.sort_key = Some(secondary_sort_key(&os_id)); + + boot_dir + .remove_all_optional(TYPE1_ENT_PATH_STAGED) + .context("Removing stale staged entries")?; + boot_dir + .create_dir_all(TYPE1_ENT_PATH_STAGED) + .context("Creating staged entries directory")?; + let staged_dir = boot_dir + .open_dir(TYPE1_ENT_PATH_STAGED) + .context("Opening staged entries directory")?; + write_type1_entries(&staged_dir, &os_id, &updated, Some(&rollback))?; + + start_finalize_stated_svc()?; + write_staged_deployment(&StagedDeployment { + depl_id: booted_cfs.cmdline.digest.to_string(), + finalization_locked: false, + })?; + tracing::info!("Staged kargs for source '{source}'; the current entry is the rollback"); + } + } + + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::loader_entries::extract_source_options_from_bls; + use crate::parsers::bls_config::parse_bls_config; + + /// 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 + } + + fn options_of(bls: &BLSConfig) -> String { + bls.get_cmdline().unwrap().to_string() + } + + #[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"); + + // Same result as the text parser used by the ostree backend + let from_text = extract_source_options_from_bls(&bls_text); + assert_eq!(sources.len(), from_text.len()); + for (name, value) in &sources { + assert_eq!(&**value, &*from_text[name]); + } + } + + #[test] + fn test_extract_source_options_from_extra_skips_empty() { + 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(); + assert!(extract_source_options_from_extra(&bls).is_empty()); + let bls = parse_bls_config(&make_bls("root=UUID=abc rw", &[])).unwrap(); + assert!(extract_source_options_from_extra(&bls).is_empty()); + } + + /// (options, existing source keys, new options for `tuned`) -> (expected + /// options, expected tuned key, keys that must survive untouched) + #[test] + fn test_update_bls_config() { + let cases: &[(&str, &[(&str, &str)], Option<&str>, &str, Option<&str>)] = &[ + ( + "root=UUID=abc rw composefs=digest123", + &[], + Some("nohz=full isolcpus=1-3"), + "root=UUID=abc rw composefs=digest123 nohz=full isolcpus=1-3", + Some("nohz=full isolcpus=1-3"), + ), + ( + "root=UUID=abc rw nohz=full isolcpus=1-3", + &[("tuned", "nohz=full isolcpus=1-3")], + Some("nohz=on rcu_nocbs=2-7"), + "root=UUID=abc rw nohz=on rcu_nocbs=2-7", + Some("nohz=on rcu_nocbs=2-7"), + ), + ( + "root=UUID=abc rw nohz=full", + &[("tuned", "nohz=full")], + None, + "root=UUID=abc rw", + None, + ), + ( + "root=UUID=abc rw nohz=full rd.driver.pre=vfio-pci", + &[("tuned", "nohz=full"), ("dracut", "rd.driver.pre=vfio-pci")], + Some("isolcpus=1-3"), + "root=UUID=abc rw isolcpus=1-3 rd.driver.pre=vfio-pci", + Some("isolcpus=1-3"), + ), + ]; + let source = SourceName::parse("tuned").unwrap(); + for (options, keys, new_options, expected_options, expected_key) in cases { + let mut bls = parse_bls_config(&make_bls(options, keys)).unwrap(); + let source_options = extract_source_options_from_extra(&bls); + let merged = compute_merged_options(options, &source_options, &source, *new_options); + update_bls_config(&mut bls, &merged, &source, *new_options).unwrap(); + + assert_eq!( + options_of(&bls), + *expected_options, + "options for {options:?}" + ); + assert_eq!( + bls.extra.get("x-options-source-tuned").map(String::as_str), + *expected_key, + "tuned key for {options:?}" + ); + for (name, value) in keys.iter().filter(|(name, _)| *name != "tuned") { + assert_eq!( + bls.extra.get(&format!("x-options-source-{name}")).unwrap(), + value, + "{name} must be untouched" + ); + } + // Survives serialization + let reparsed = parse_bls_config(&bls.to_string()).unwrap(); + assert_eq!(reparsed.extra, bls.extra); + } + } + + #[test] + fn test_is_unchanged() { + let source = SourceName::parse("tuned").unwrap(); + let cases: &[(&str, &[(&str, &str)], Option<&str>, bool)] = &[ + // Same value re-applied, even with other arguments after it + ( + "root=/dev/a nohz=full quiet", + &[("tuned", "nohz=full")], + Some("nohz=full"), + true, + ), + ("root=/dev/a", &[], None, true), + ("root=/dev/a", &[], Some(""), true), + ( + "root=/dev/a nohz=full", + &[("tuned", "nohz=full")], + Some("nohz=on"), + false, + ), + ( + "root=/dev/a nohz=full", + &[("tuned", "nohz=full")], + None, + false, + ), + ("root=/dev/a", &[], Some("nohz=full"), false), + ]; + for (options, keys, new_options, expected) in cases { + let bls = parse_bls_config(&make_bls(options, keys)).unwrap(); + let source_options = extract_source_options_from_extra(&bls); + let merged = compute_merged_options(options, &source_options, &source, *new_options); + assert_eq!( + is_unchanged(options, &merged, &source_options, &source, *new_options), + *expected, + "{options:?} -> {new_options:?}" + ); + } + } +} diff --git a/crates/lib/src/bootc_composefs/mod.rs b/crates/lib/src/bootc_composefs/mod.rs index 42d521150a..1d9d42900a 100644 --- a/crates/lib/src/bootc_composefs/mod.rs +++ b/crates/lib/src/bootc_composefs/mod.rs @@ -5,6 +5,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 progress; pub(crate) mod repo; pub(crate) mod rollback; diff --git a/crates/lib/src/bootc_composefs/rollback.rs b/crates/lib/src/bootc_composefs/rollback.rs index 307545463d..47d4d5548e 100644 --- a/crates/lib/src/bootc_composefs/rollback.rs +++ b/crates/lib/src/bootc_composefs/rollback.rs @@ -8,8 +8,7 @@ use ocidir::cap_std::ambient_authority; use rustix::fs::{AtFlags, RenameFlags, fsync, renameat_with}; use crate::bootc_composefs::boot::{ - BootType, FILENAME_PRIORITY_PRIMARY, FILENAME_PRIORITY_SECONDARY, primary_sort_key, - secondary_sort_key, type1_entry_conf_file_name, + BootType, os_id_from_sort_key, primary_sort_key, secondary_sort_key, write_type1_entries, }; use crate::bootc_composefs::status::{get_composefs_status, get_sorted_type1_boot_entries}; use crate::composefs_consts::{ @@ -136,16 +135,14 @@ fn rollback_composefs_entries(host: &Host, boot_dir: &Dir, bootloader: Bootloade assert!(all_configs.len() == 2); // For rollback: previous gets primary sort-key, booted gets secondary sort-key - // Use "bootc" as default os_id for rollback scenarios - // TODO: Extract actual os_id from deployment - let os_id = "bootc"; + let os_id = os_id_from_sort_key(&all_configs[0]).to_string(); // This is the currently booted deployment - it should become secondary // OR if rollback was queued, it would become primary - all_configs[0].sort_key = Some(primary_sort_key(os_id)); + all_configs[0].sort_key = Some(primary_sort_key(&os_id)); // This is the previous deployment - it should become primary (rollback target) // OR if rollback was queued, it would become secondary - all_configs[1].sort_key = Some(secondary_sort_key(os_id)); + all_configs[1].sort_key = Some(secondary_sort_key(&os_id)); // Ostree will drop any staged deployment on rollback // We follow the same approach for now @@ -182,28 +179,13 @@ fn rollback_composefs_entries(host: &Host, boot_dir: &Dir, bootloader: Bootloade .open_dir(TYPE1_ENT_PATH_STAGED) .context("Opening staged entries dir")?; - // Write the BLS configs in there - for cfg in all_configs { - // After rollback: previous deployment becomes primary, booted becomes secondary - let priority = if cfg.sort_key == Some(secondary_sort_key(os_id)) { - FILENAME_PRIORITY_SECONDARY - } else { - FILENAME_PRIORITY_PRIMARY - }; - - let file_name = type1_entry_conf_file_name(os_id, &cfg.version(), priority); - - rollback_entries_dir - .atomic_write(&file_name, cfg.to_string()) - .with_context(|| format!("Writing to {file_name}"))?; - } - - let rollback_entries_dir = rollback_entries_dir - .reopen_as_ownedfd() - .context("Reopening as owned fd")?; - - // Should we sync after every write? - fsync(rollback_entries_dir).context("fsync")?; + // After rollback: previous deployment becomes primary, booted becomes secondary + write_type1_entries( + &rollback_entries_dir, + &os_id, + &all_configs[0], + Some(&all_configs[1]), + )?; // Atomically exchange "entries" <-> "entries.rollback" let dir = boot_dir.open_dir("loader").context("Opening loader dir")?; diff --git a/crates/lib/src/bootc_composefs/state.rs b/crates/lib/src/bootc_composefs/state.rs index e43447a2f5..db752341e3 100644 --- a/crates/lib/src/bootc_composefs/state.rs +++ b/crates/lib/src/bootc_composefs/state.rs @@ -33,9 +33,9 @@ use crate::parsers::bls_config::{BLSConfigType, EFIKey}; use crate::store::{BootedComposefs, Storage}; use crate::{ composefs_consts::{ - COMPOSEFS_STAGED_DEPLOYMENT_FNAME, COMPOSEFS_TRANSIENT_STATE_DIR, ORIGIN_KEY_BOOT, - ORIGIN_KEY_BOOT_DIGEST, ORIGIN_KEY_BOOT_TYPE, ORIGIN_KEY_IMAGE, ORIGIN_KEY_MANIFEST_DIGEST, - SHARED_VAR_PATH, STATE_DIR_RELATIVE, + COMPOSEFS_CMDLINE, COMPOSEFS_STAGED_DEPLOYMENT_FNAME, COMPOSEFS_TRANSIENT_STATE_DIR, + ORIGIN_KEY_BOOT, ORIGIN_KEY_BOOT_DIGEST, ORIGIN_KEY_BOOT_TYPE, ORIGIN_KEY_IMAGE, + ORIGIN_KEY_MANIFEST_DIGEST, SHARED_VAR_PATH, STATE_DIR_RELATIVE, }, parsers::bls_config::BLSConfig, spec::ImageReference, @@ -64,38 +64,89 @@ pub(crate) fn read_origin(sysroot: &Dir, deployment_id: &str) -> Result Result { - let sorted_entries = get_sorted_type1_boot_entries(boot_dir, true)?; +/// Whether `entry` boots the deployment identified by `booted_cfs`. +fn entry_has_verity(entry: &BLSConfig, booted_cfs: &ComposefsCmdline) -> Result { + match &entry.cfg_type { + BLSConfigType::EFI { key } => { + let path = match key { + EFIKey::Efi(path) | EFIKey::Uki(path) => path, + }; + Ok(path.as_str().contains(&*booted_cfs.digest)) + } - for entry in sorted_entries { - match &entry.cfg_type { - BLSConfigType::EFI { key } => { - let path = match key { - EFIKey::Efi(path) | EFIKey::Uki(path) => path, - }; - if path.as_str().contains(&*booted_cfs.cmdline.digest) { - return Ok(entry); - } - } - - BLSConfigType::NonEFI { options, .. } => { - let Some(opts) = options else { - anyhow::bail!("options not found in bls config") - }; - - let cfs_cmdline = ComposefsCmdline::find_in_cmdline(&Cmdline::from(opts)) - .ok_or_else(|| anyhow::anyhow!("composefs param not found in cmdline"))?; - - if cfs_cmdline.digest == booted_cfs.cmdline.digest { - return Ok(entry); - } - } - - BLSConfigType::Unknown => anyhow::bail!("Unknown BLS Config type"), - }; + BLSConfigType::NonEFI { options, .. } => { + let Some(opts) = options else { + anyhow::bail!("options not found in bls config") + }; + + let cfs_cmdline = ComposefsCmdline::find_in_cmdline(&Cmdline::from(opts)) + .ok_or_else(|| anyhow::anyhow!("composefs param not found in cmdline"))?; + + Ok(cfs_cmdline.digest == booted_cfs.digest) + } + + BLSConfigType::Unknown => anyhow::bail!("Unknown BLS Config type"), } +} + +/// The number of `options` parameters when every one of them is on +/// `kernel_cmdline`, else `None`. `composefs=` is skipped: after a soft +/// reboot the command line still names the previous deployment. +fn options_on_cmdline(options: &Cmdline, kernel_cmdline: &Cmdline) -> Option { + let mut n = 0; + for param in options { + if param.key() == COMPOSEFS_CMDLINE.into() { + continue; + } + let present = kernel_cmdline + .iter() + .any(|k| k.key() == param.key() && k.value() == param.value()); + if !present { + return None; + } + n += 1; + } + Some(n) +} + +/// Pick the entry the running kernel was booted from. +/// +/// Entries are matched by their `composefs=` digest. A kernel-argument +/// change (`bootc loader-entries set-options-for-source`) keeps the previous +/// entry of the booted deployment as its rollback, so several entries can +/// share the digest; then the one whose `options` are all on the kernel +/// command line wins, and among those the largest set, so an entry that only +/// lacks an added argument is not mistaken for the booted one. When none +/// matches, the first candidate in `entries` order is returned. +pub(crate) fn select_booted_bls<'a>( + entries: impl IntoIterator, + booted_cfs: &ComposefsCmdline, + kernel_cmdline: &Cmdline, +) -> Result> { + let mut best: Option<(&BLSConfig, Option)> = None; + for entry in entries { + if !entry_has_verity(entry, booted_cfs)? { + continue; + } + let score = entry + .get_cmdline() + .ok() + .and_then(|options| options_on_cmdline(options, kernel_cmdline)); + match best { + Some((_, current)) if score <= current => {} + _ => best = Some((entry, score)), + } + } + Ok(best.map(|(entry, _)| entry)) +} + +pub(crate) fn get_booted_bls(boot_dir: &Dir, booted_cfs: &BootedComposefs) -> Result { + let sorted_entries = get_sorted_type1_boot_entries(boot_dir, true)?; + let kernel_cmdline = Cmdline::from_proc()?; - Err(anyhow::anyhow!("Booted BLS not found")) + select_booted_bls(&sorted_entries, booted_cfs.cmdline, &kernel_cmdline)? + .cloned() + .ok_or_else(|| anyhow::anyhow!("Booted BLS not found")) } /// Mounts an EROFS image and copies the pristine /etc and /var to the deployment's /etc and /var. @@ -214,6 +265,25 @@ pub(crate) fn update_boot_digest_in_origin( ) } +/// Record `staged` as the deployment to finalize at shutdown +#[context("Recording staged deployment")] +pub(crate) fn write_staged_deployment(staged: &StagedDeployment) -> Result<()> { + std::fs::create_dir_all(COMPOSEFS_TRANSIENT_STATE_DIR) + .with_context(|| format!("Creating {COMPOSEFS_TRANSIENT_STATE_DIR}"))?; + + let staged_depl_dir = Dir::open_ambient_dir(COMPOSEFS_TRANSIENT_STATE_DIR, ambient_authority()) + .with_context(|| format!("Opening {COMPOSEFS_TRANSIENT_STATE_DIR}"))?; + + staged_depl_dir + .atomic_write( + COMPOSEFS_STAGED_DEPLOYMENT_FNAME, + staged + .to_canon_json_vec() + .context("Failed to serialize staged deployment JSON")?, + ) + .with_context(|| format!("Writing to {COMPOSEFS_STAGED_DEPLOYMENT_FNAME}")) +} + /// Creates and populates the composefs state directory for a deployment. /// /// This function sets up the state directory structure and configuration files @@ -308,21 +378,7 @@ pub(crate) async fn write_composefs_state( .context("Failed to write to .origin file")?; if let Some(staged) = staged { - std::fs::create_dir_all(COMPOSEFS_TRANSIENT_STATE_DIR) - .with_context(|| format!("Creating {COMPOSEFS_TRANSIENT_STATE_DIR}"))?; - - let staged_depl_dir = - Dir::open_ambient_dir(COMPOSEFS_TRANSIENT_STATE_DIR, ambient_authority()) - .with_context(|| format!("Opening {COMPOSEFS_TRANSIENT_STATE_DIR}"))?; - - staged_depl_dir - .atomic_write( - COMPOSEFS_STAGED_DEPLOYMENT_FNAME, - staged - .to_canon_json_vec() - .context("Failed to serialize staged deployment JSON")?, - ) - .with_context(|| format!("Writing to {COMPOSEFS_STAGED_DEPLOYMENT_FNAME}"))?; + write_staged_deployment(&staged)?; } Ok(()) @@ -375,3 +431,51 @@ pub(crate) fn get_composefs_usr_overlay_status() -> Result BLSConfig { + parse_bls_config(&format!( + "title t\nversion 1\nlinux /vmlinuz\ninitrd /initrd\noptions {options}\n" + )) + .unwrap() + } + + #[test] + fn select_booted_bls_prefers_the_entry_matching_the_cmdline() -> Result<()> { + let booted = ComposefsCmdline::new(DIGEST); + let other = entry(&format!("root=/dev/a rw composefs={OTHER} nohz=full")); + let old = entry(&format!("root=/dev/a rw composefs={DIGEST}")); + let new = entry(&format!("root=/dev/a rw composefs={DIGEST} nohz=full")); + let entries = vec![other.clone(), old.clone(), new.clone()]; + + let cases = [ + // The added argument is on the cmdline: the larger set wins even + // though the smaller one is a subset too. + ( + format!("BOOT_IMAGE=/vmlinuz root=/dev/a rw composefs={DIGEST} nohz=full"), + &new, + ), + // Rolled back to the entry without it. + (format!("root=/dev/a rw composefs={DIGEST}"), &old), + // Soft reboot: composefs= still names the previous deployment. + (format!("root=/dev/a rw composefs={OTHER} nohz=full"), &new), + // Nothing matches: first candidate with the digest. + ("root=/dev/b rw".to_string(), &old), + ]; + for (cmdline, expected) in cases { + let picked = select_booted_bls(&entries, &booted, &Cmdline::from(&cmdline))? + .unwrap_or_else(|| panic!("no entry for {cmdline}")); + assert_eq!(picked, expected, "cmdline {cmdline}"); + } + + assert!(select_booted_bls(&[other], &booted, &Cmdline::from("x"))?.is_none()); + Ok(()) + } +} diff --git a/crates/lib/src/bootc_composefs/status.rs b/crates/lib/src/bootc_composefs/status.rs index 33c323bd07..665efd7c30 100644 --- a/crates/lib/src/bootc_composefs/status.rs +++ b/crates/lib/src/bootc_composefs/status.rs @@ -45,6 +45,7 @@ use ostree_ext::{container::deploy::ORIGIN_CONTAINER, oci_spec::image::ImageConf use ostree_ext::oci_spec::image::ImageManifest; use tokio::io::AsyncReadExt; +use crate::bootc_composefs::state::select_booted_bls; use crate::composefs_consts::{ COMPOSEFS_STAGED_DEPLOYMENT_FNAME, COMPOSEFS_TRANSIENT_STATE_DIR, ORIGIN_KEY_BOOT, ORIGIN_KEY_BOOT_TYPE, STATE_DIR_RELATIVE, @@ -148,6 +149,13 @@ pub(crate) struct BootloaderEntry { /// /// We mainly need this in order to GC shared Type1 entries pub(crate) boot_artifact_name: String, + /// The `options` line of a Type1 entry; `None` for UKI entries + /// + /// Entries of one deployment share the verity, so this is what tells the + /// booted entry from the rollback left behind by a kernel-argument change. + pub(crate) options: Option, + /// Whether the entry is pending under `loader/entries.staged` + pub(crate) staged: bool, } /// Detect if we have `composefs=` in `/proc/cmdline` @@ -253,8 +261,44 @@ pub(crate) fn get_sorted_type1_boot_entries( boot_dir: &Dir, ascending: bool, ) -> Result> { + Ok(get_sorted_type1_entries(boot_dir, ascending, false)? + .into_iter() + .map(|e| e.config) + .collect()) +} + +/// A Type1 boot entry together with the name of its `.conf` file +#[derive(Debug)] +pub(crate) struct Type1Entry { + pub(crate) filename: String, + pub(crate) config: BLSConfig, +} + +/// Same as [`get_sorted_type1_boot_entries`] but keeps each entry's file +/// name, for callers that rewrite an entry in place. `staged` selects the +/// entries under `loader/entries.staged` instead of `loader/entries`. +pub(crate) fn get_sorted_type1_entries( + boot_dir: &Dir, + ascending: bool, + staged: bool, +) -> Result> { let bootloader = get_bootloader()?; - get_sorted_type1_boot_entries_helper(boot_dir, ascending, false, bootloader) + get_sorted_type1_entries_helper(boot_dir, ascending, staged, bootloader) +} + +#[cfg(test)] +fn get_sorted_type1_boot_entries_helper( + boot_dir: &Dir, + ascending: bool, + get_staged_entries: bool, + bootloader: crate::spec::Bootloader, +) -> Result> { + Ok( + get_sorted_type1_entries_helper(boot_dir, ascending, get_staged_entries, bootloader)? + .into_iter() + .map(|e| e.config) + .collect(), + ) } /// Same as [`get_sorted_type1_boot_entries`], but returns staged entries @@ -263,23 +307,19 @@ pub(crate) fn get_sorted_staged_type1_boot_entries( boot_dir: &Dir, ascending: bool, ) -> Result> { - let bootloader = get_bootloader()?; - get_sorted_type1_boot_entries_helper(boot_dir, ascending, true, bootloader) + Ok(get_sorted_type1_entries(boot_dir, ascending, true)? + .into_iter() + .map(|e| e.config) + .collect()) } #[context("Getting sorted Type1 boot entries")] -fn get_sorted_type1_boot_entries_helper( +fn get_sorted_type1_entries_helper( boot_dir: &Dir, ascending: bool, get_staged_entries: bool, bootloader: crate::spec::Bootloader, -) -> Result> { - #[derive(Debug)] - struct ConfigWithFilename { - config: BLSConfig, - filename: String, - } - +) -> Result> { let dir = match get_staged_entries { true => { let dir = boot_dir.open_dir_optional(TYPE1_ENT_PATH_STAGED)?; @@ -319,7 +359,7 @@ fn get_sorted_type1_boot_entries_helper( let config = parse_bls_config(&contents).context("Parsing bls config")?; - configs_with_filenames.push(ConfigWithFilename { + configs_with_filenames.push(Type1Entry { config, filename: file_name.to_string(), }); @@ -341,10 +381,7 @@ fn get_sorted_type1_boot_entries_helper( if ascending { ord } else { ord.reverse() } }); - Ok(configs_with_filenames - .into_iter() - .map(|c| c.config) - .collect()) + Ok(configs_with_filenames) } pub(crate) fn list_type1_entries(boot_dir: &Dir) -> Result> { @@ -357,11 +394,14 @@ pub(crate) fn list_type1_entries(boot_dir: &Dir) -> Result> boot_entries .into_iter() - .chain(staged_boot_entries) - .map(|entry| { + .map(|entry| (entry, false)) + .chain(staged_boot_entries.into_iter().map(|entry| (entry, true))) + .map(|(entry, staged)| { Ok(BootloaderEntry { fsverity: entry.get_verity()?, boot_artifact_name: entry.boot_artifact_name()?.to_string(), + options: entry.get_cmdline().ok().map(|c| c.to_string()), + staged, }) }) .collect::, _>>() @@ -392,11 +432,14 @@ pub(crate) fn list_bootloader_entries(storage: &Storage) -> Result, anyhow::Error>>()? @@ -786,6 +829,13 @@ fn set_reboot_capable_type1_deployments( { let depl_verity = &depl.require_composefs()?.verity; + // Another entry of the booted deployment differs only in kernel + // arguments, which need a real reboot + if *depl_verity == *booted_cmdline.digest { + depl.soft_reboot_capable = false; + continue; + } + let entry = find_bls_entry(&depl_verity, &bls_entries)? .ok_or_else(|| anyhow::anyhow!("Entry not found"))?; @@ -897,9 +947,14 @@ fn set_reboot_capable_uki_deployments( /// Whether the bootloader will boot a deployment other than the booted one, /// i.e. whether the first (default) boot entry references some other deployment. #[context("Determining if rollback is queued")] +/// Whether the default entry is not the booted one. `booted_options` is the +/// booted entry's options line when known: entries of the same deployment +/// share the digest after a kernel-argument change, so the digest alone is +/// not enough to recognize the booted entry among them. fn rollback_queued_from_first_entry( bls_config: &BLSConfig, booted_composefs_digest: &str, + booted_options: Option<&str>, ) -> Result { match &bls_config.cfg_type { // For UKI boot @@ -911,15 +966,33 @@ fn rollback_queued_from_first_entry( } // For boot entry Type1 - BLSConfigType::NonEFI { options, .. } => Ok(!options - .as_ref() - .ok_or_else(|| anyhow::anyhow!("options key not found in bls config"))? - .contains(booted_composefs_digest)), + BLSConfigType::NonEFI { options, .. } => { + let options = options + .as_ref() + .ok_or_else(|| anyhow::anyhow!("options key not found in bls config"))?; + if !options.contains(booted_composefs_digest) { + return Ok(true); + } + Ok(booted_options.is_some_and(|booted| booted != &**options)) + } BLSConfigType::Unknown => anyhow::bail!("Unknown BLS Config Type"), } } +/// The options line of the Type1 entry the kernel was booted from, or `None` +/// when there are no Type1 entries (UKI boot). +fn booted_type1_options(boot_dir: &Dir, cmdline: &ComposefsCmdline) -> Result> { + if boot_dir.open_dir_optional(TYPE1_ENT_PATH)?.is_none() { + return Ok(None); + } + let entries = get_sorted_type1_boot_entries(boot_dir, true)?; + let kernel_cmdline = Cmdline::from_proc()?; + Ok(select_booted_bls(&entries, cmdline, &kernel_cmdline)? + .and_then(|entry| entry.get_cmdline().ok()) + .map(|options| options.to_string())) +} + #[context("Getting composefs deployment status")] async fn composefs_deployment_status_from( storage: &Storage, @@ -952,6 +1025,17 @@ async fn composefs_deployment_status_from( Err(e) => Err(e), }?; + let staged_deployment = staged_deployment + .as_deref() + .map(serde_json::from_str::) + .transpose()?; + + // A kernel-argument change (`bootc loader-entries set-options-for-source`) + // stages the booted deployment again with a new entry and keeps the current + // one as its rollback, so up to three entries share the booted verity and + // only the options line tells them apart. + let booted_options = booted_type1_options(boot_dir, cmdline)?; + let mut boot_type: Option = None; // Boot entries from deployments that are neither booted nor staged deployments @@ -960,6 +1044,8 @@ async fn composefs_deployment_status_from( for BootloaderEntry { fsverity: verity_digest, + options: entry_options, + staged: from_staged_dir, .. } in bootloader_entry_verity { @@ -1000,13 +1086,27 @@ async fn composefs_deployment_status_from( }; if verity_digest == booted_composefs_digest.as_ref() { - host.status.booted = Some(boot_entry); + let same_options = match (&booted_options, &entry_options) { + (Some(booted), Some(entry)) => booted == entry, + _ => true, + }; + if same_options { + host.status.booted = Some(boot_entry); + } else if !from_staged_dir { + // The entry the booted deployment had before its kernel + // arguments changed + extra_deployment_boot_entries.push(boot_entry); + } else if let Some(staged_depl) = staged_deployment + .as_ref() + .filter(|s| s.depl_id == verity_digest) + { + boot_entry.download_only = staged_depl.finalization_locked; + host.status.staged = Some(boot_entry); + } continue; } - if let Some(staged_deployment) = &staged_deployment { - let staged_depl = serde_json::from_str::(&staged_deployment)?; - + if let Some(staged_depl) = &staged_deployment { if verity_digest == staged_depl.depl_id { boot_entry.download_only = staged_depl.finalization_locked; host.status.staged = Some(boot_entry); @@ -1048,6 +1148,7 @@ async fn composefs_deployment_status_from( let is_rollback_queued = rollback_queued_from_first_entry( bls_config, booted_composefs_digest.as_ref(), + booted_options.as_deref(), )?; (is_rollback_queued, Some(bls_configs), None) @@ -1083,8 +1184,11 @@ async fn composefs_deployment_status_from( .first() .ok_or(anyhow::anyhow!("First boot entry not found"))?; - let is_rollback_queued = - rollback_queued_from_first_entry(bls_config, booted_composefs_digest.as_ref())?; + let is_rollback_queued = rollback_queued_from_first_entry( + bls_config, + booted_composefs_digest.as_ref(), + booted_options.as_deref(), + )?; (is_rollback_queued, Some(bls_configs), None) } @@ -1383,9 +1487,9 @@ mod tests { ); // The default entry references the booted deployment: nothing is queued - assert!(!rollback_queued_from_first_entry(first, BOOTED)?); + assert!(!rollback_queued_from_first_entry(first, BOOTED, None)?); // The default entry references another deployment: a rollback is queued - assert!(rollback_queued_from_first_entry(first, ROLLBACK)?); + assert!(rollback_queued_from_first_entry(first, ROLLBACK, None)?); Ok(()) } @@ -1432,8 +1536,8 @@ mod tests { &primary_sort_key("fedora") ); - assert!(!rollback_queued_from_first_entry(first, BOOTED)?); - assert!(rollback_queued_from_first_entry(first, ROLLBACK)?); + assert!(!rollback_queued_from_first_entry(first, BOOTED, None)?); + assert!(rollback_queued_from_first_entry(first, ROLLBACK, None)?); Ok(()) } @@ -1485,8 +1589,8 @@ mod tests { &primary_sort_key("fedora") ); - assert!(!rollback_queued_from_first_entry(first, BOOTED)?); - assert!(rollback_queued_from_first_entry(first, ROLLBACK)?); + assert!(!rollback_queued_from_first_entry(first, BOOTED, None)?); + assert!(rollback_queued_from_first_entry(first, ROLLBACK, None)?); Ok(()) } @@ -1828,4 +1932,35 @@ mod tests { Ok(()) } + /// Two entries of the booted deployment differ only in kernel arguments + /// after `loader-entries set-options-for-source`; the digest alone does + /// not say whether the default entry is the booted one. + #[test] + fn test_rollback_queued_from_first_entry_same_verity() -> Result<()> { + const DIGEST: &str = "abc123"; + let entry = |options: &str| { + crate::parsers::bls_config::parse_bls_config(&format!( + "title t\nversion 1\nlinux /vmlinuz\ninitrd /initrd\noptions {options}\n" + )) + .unwrap() + }; + let old = entry(&format!("root=/dev/a composefs={DIGEST}")); + let new = entry(&format!("root=/dev/a composefs={DIGEST} nohz=full")); + let new_options = new.get_cmdline()?.to_string(); + + // Booted into the new entry: queued only when the old one is first + assert!(!rollback_queued_from_first_entry( + &new, + DIGEST, + Some(&new_options) + )?); + assert!(rollback_queued_from_first_entry( + &old, + DIGEST, + Some(&new_options) + )?); + // Without the options line the digest is all there is + assert!(!rollback_queued_from_first_entry(&old, DIGEST, None)?); + Ok(()) + } } diff --git a/crates/lib/src/bootc_composefs/update.rs b/crates/lib/src/bootc_composefs/update.rs index c2b43c8b86..57169d02a7 100644 --- a/crates/lib/src/bootc_composefs/update.rs +++ b/crates/lib/src/bootc_composefs/update.rs @@ -518,7 +518,21 @@ pub(crate) async fn upgrade_composefs( // Check if we already have this update staged // Or if we have another staged deployment with a different image - let staged_image = host.status.staged.as_ref().and_then(|i| i.image.as_ref()); + // + // A kernel-argument change stages the booted deployment itself; that is + // not an update, and do_upgrade() builds on its pending entry. + let kargs_only_staged = host + .status + .staged + .as_ref() + .and_then(|s| s.composefs.as_ref()) + .is_some_and(|s| *s.verity == *composefs.cmdline.digest); + let staged_image = host + .status + .staged + .as_ref() + .filter(|_| !kargs_only_staged) + .and_then(|i| i.image.as_ref()); if let Some(staged_image) = staged_image { // We have a staged image and it has the same digest as the currently booted image's latest diff --git a/crates/lib/src/bootc_kargs.rs b/crates/lib/src/bootc_kargs.rs index 6d1883576f..884cac0aae 100644 --- a/crates/lib/src/bootc_kargs.rs +++ b/crates/lib/src/bootc_kargs.rs @@ -194,9 +194,10 @@ fn get_kargs_from_ostree( Ok(ret) } -/// Compute the kernel arguments for the new deployment. This starts from the booted -/// karg, but applies the diff between the bootc karg files in /usr/lib/bootc/kargs.d -/// between the booted deployment and the new one. +/// Compute the kernel arguments for the new deployment. This starts from the kargs +/// of the deployment being replaced -- the staged one if there is one, otherwise the +/// merge (booted) deployment -- and applies the diff between the bootc karg files in +/// /usr/lib/bootc/kargs.d of that deployment and the new one. pub(crate) fn get_kargs( sysroot: &Storage, merge_deployment: &Deployment, @@ -207,16 +208,25 @@ pub(crate) fn get_kargs( let repo = &ostree.repo(); let sys_arch = std::env::consts::ARCH; - // Get the kargs used for the merge in the bootloader config - let mut kargs = ostree::Deployment::bootconfig(merge_deployment) + // A staged deployment carries kargs changes that are not in the booted + // entry yet (e.g. from `bootc loader-entries set-options-for-source` or + // `rpm-ostree kargs`). Staging replaces it, so build on it rather than + // silently discarding those changes. + let staged = ostree + .staged_deployment() + .filter(|s| s.stateroot() == merge_deployment.stateroot()); + let base_deployment = staged.as_ref().unwrap_or(merge_deployment); + + // Get the kargs used for the base deployment in the bootloader config + let mut kargs = ostree::Deployment::bootconfig(base_deployment) .and_then(|bootconfig| { ostree::BootconfigParser::get(&bootconfig, "options") .map(|options| Cmdline::from(options.to_string())) }) .unwrap_or_default(); - // Get the kargs in kargs.d of the merge - let merge_root = &crate::utils::deployment_fd(ostree, merge_deployment)?; + // Get the kargs in kargs.d of the base deployment + let merge_root = &crate::utils::deployment_fd(ostree, base_deployment)?; let existing_kargs = get_kargs_in_root(merge_root, sys_arch)?; // Get the kargs in kargs.d of the pending image diff --git a/crates/lib/src/cli.rs b/crates/lib/src/cli.rs index 4a8ccea630..a7234711e0 100644 --- a/crates/lib/src/cli.rs +++ b/crates/lib/src/cli.rs @@ -2263,12 +2263,25 @@ 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(), + ) + .await?; + } + } Ok(()) } }, diff --git a/crates/lib/src/deploy.rs b/crates/lib/src/deploy.rs index 3346c6c972..ae33dee5c7 100644 --- a/crates/lib/src/deploy.rs +++ b/crates/lib/src/deploy.rs @@ -904,9 +904,8 @@ async fn deploy( lock_finalization: bool, ) -> Result { // Compute the kernel argument overrides. In practice today this API is always expecting - // a merge deployment. The kargs code also always looks at the booted root (which - // is a distinct minor issue, but not super important as right now the install path - // doesn't use this API). + // a merge deployment; the kargs code builds on the staged deployment instead when + // there is one, so that a pending kargs change is not lost. let (stateroot, override_kargs) = match &from { MergeState::MergeDeployment(deployment) => { let kargs = crate::bootc_kargs::get_kargs(sysroot, &deployment, image)?; diff --git a/crates/lib/src/loader_entries.rs b/crates/lib/src/loader_entries.rs index 3f193da392..f7be88117b 100644 --- a/crates/lib/src/loader_entries.rs +++ b/crates/lib/src/loader_entries.rs @@ -11,23 +11,23 @@ use anyhow::{Context, Result, ensure}; use fn_error_context::context; -use linux_kernel_cmdline::utf8::{Cmdline, CmdlineOwned}; +use linux_kernel_cmdline::utf8::{Cmdline, CmdlineOwned, Parameter}; use ostree::{gio, glib}; 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(); @@ -85,34 +85,50 @@ fn extract_source_options_from_bls(content: &str) -> BTreeMap, target_source: &SourceName, new_options: Option<&str>, ) -> CmdlineOwned { - let mut merged = CmdlineOwned::from(current_options.to_owned()); - - // Remove old options from the target source (if it was previously tracked) - if let Some(old_source_opts) = source_options.get(&**target_source) { - for param in old_source_opts.iter() { - merged.remove_exact(¶m); + let current = Cmdline::from(current_options); + let old_params: Vec = source_options + .get(&**target_source) + .map(|old| old.iter().collect()) + .unwrap_or_default(); + let new_cmdline = new_options.filter(|v| !v.is_empty()).map(Cmdline::from); + let new_params: Vec = new_cmdline + .iter() + .flat_map(|c| c.iter()) + .map(|p| p.to_string()) + .collect(); + + let mut merged: Vec = Vec::new(); + let mut inserted = false; + for param in current.iter() { + if old_params.contains(¶m) { + // First old parameter: emit the new ones here; drop the rest. + if !inserted { + merged.extend(new_params.iter().cloned()); + inserted = true; + } + continue; } + merged.push(param.to_string()); } - - // Add new options for the target source - if let Some(new_opts) = new_options.filter(|v| !v.is_empty()) { - let new_cmdline = Cmdline::from(new_opts); - for param in new_cmdline.iter() { - merged.add(¶m); - } + if !inserted { + merged.extend(new_params); } - merged + CmdlineOwned::from(merged.join(" ")) } /// Read x-options-source-* keys from the staged deployment data file. @@ -556,7 +572,40 @@ x-options-source-admin nohz=full ], "tuned", Some("nohz=full"), - "root=UUID=abc rw rd.driver.pre=vfio-pci nohz=full", + "root=UUID=abc rw nohz=full rd.driver.pre=vfio-pci", + ), + ( + "re-applying an unchanged source is a no-op even when followed by other options", + "root=UUID=abc rw nohz=on rcu_nocbs=2-7 cross1=a rpmarg=yes", + &[ + ("tuned", "nohz=on rcu_nocbs=2-7"), + ("crosstest", "cross1=a"), + ], + "tuned", + Some("nohz=on rcu_nocbs=2-7"), + "root=UUID=abc rw nohz=on rcu_nocbs=2-7 cross1=a rpmarg=yes", + ), + ( + "updating a source keeps its position", + "root=UUID=abc rw nohz=on rcu_nocbs=2-7 cross1=a rpmarg=yes", + &[ + ("tuned", "nohz=on rcu_nocbs=2-7"), + ("crosstest", "cross1=a"), + ], + "tuned", + Some("nohz=on skew_tick=1"), + "root=UUID=abc rw nohz=on skew_tick=1 cross1=a rpmarg=yes", + ), + ( + "removing a source keeps the order of the rest", + "root=UUID=abc rw nohz=on cross1=a rcu_nocbs=2-7 rpmarg=yes", + &[ + ("tuned", "nohz=on rcu_nocbs=2-7"), + ("crosstest", "cross1=a"), + ], + "tuned", + None, + "root=UUID=abc rw cross1=a rpmarg=yes", ), ]; diff --git a/crates/lib/src/parsers/bls_config.rs b/crates/lib/src/parsers/bls_config.rs index c796ffdab1..aff6552741 100644 --- a/crates/lib/src/parsers/bls_config.rs +++ b/crates/lib/src/parsers/bls_config.rs @@ -199,7 +199,6 @@ impl BLSConfig { self.sort_key = Some(new_val); self } - #[allow(dead_code)] pub(crate) fn with_extra(&mut self, new_val: HashMap) -> &mut Self { self.extra = new_val; self diff --git a/docs/src/building/kernel-arguments.md b/docs/src/building/kernel-arguments.md index 2d619fc491..f26828b907 100644 --- a/docs/src/building/kernel-arguments.md +++ b/docs/src/building/kernel-arguments.md @@ -78,22 +78,150 @@ mount -o remount,rw /boot # tool to edit /boot/loader/entries ``` -At the current time, `bootc` does not itself offer -an API to manipulate kernel arguments maintained per-machine. +`bootc loader-entries set-options-for-source` manages per-machine +kernel arguments on behalf of a named *source*; see +[Source-tracked kernel arguments](#source-tracked-kernel-arguments) below. -Other projects such as `rpm-ostree` do, via e.g. `rpm-ostree kargs`, +Other projects such as `rpm-ostree` do too, via e.g. `rpm-ostree kargs`, which is just a frontend for editing the bootloader configuration files. Note an important detail is that `rpm-ostree kargs` always creates a new deployment. -`rpm-ostree kargs` and bootc will interoperate as they both -use the ostree backend today, and any kernel arguments changed -via that mechanism will persist across upgrades. +`rpm-ostree kargs` and bootc interoperate as they both use the ostree +backend today: any kernel arguments changed via either mechanism persist +across upgrades, and each builds on the deployment it replaces, so a +change staged by one is not lost when the other stages next. It is currently undefined behavior to remove kernel arguments locally that are included in the base image via `/usr/lib/bootc/kargs.d`. +## Source-tracked kernel arguments + +A tool that adds kernel arguments — TuneD applying a profile is the +motivating case — needs to later replace or remove exactly the arguments +it added, without keeping state in `/etc` (which may be transient). +`bootc loader-entries set-options-for-source` records ownership next to +the arguments themselves, in the BLS entry: + +``` +options root=UUID=... rw ostree=/ostree/boot.0/... console=ttyS0 nohz=full isolcpus=1-3 rd.driver.pre=vfio-pci +x-options-source-tuned nohz=full isolcpus=1-3 +x-options-source-dracut rd.driver.pre=vfio-pci +``` + +* `options` is the kernel command line and the ground truth. +* `x-options-source-NAME` is bookkeeping: the arguments source `NAME` + currently owns. Bootloaders ignore unknown keys. Names are limited to + `[A-Za-z0-9_-]+`. +* A source key with an **empty value is a tombstone** ("this source owns + nothing"). bootc never deletes a key, it clears it. + +```bash +# set or replace TuneD's arguments +bootc loader-entries set-options-for-source --source tuned --options "nohz=full isolcpus=1-3" +# remove them +bootc loader-entries set-options-for-source --source tuned +``` + +Both stage a new deployment (unless nothing would change); the arguments +take effect on the next boot. See +`man bootc-loader-entries-set-options-for-source`. + +### How a call is processed + +```mermaid +flowchart TD + s([set-options-for-source --source S --options O]) --> v[validate S] + v --> base{Staged deployment exists?} + base -- yes --> b1[base = staged deployment
commit, origin, options from it] + base -- no --> b2[base = booted deployment] + b1 --> disc1[sources = names from the booted entry,
values from the staged bootconfig,
plus names only in the staged data] + b2 --> disc2[sources = parse the booted entry] + disc1 --> merge + disc2 --> merge[merged = options with S's old
arguments replaced in place by O] + merge --> idem{merged == options
and old S == O?} + idem -- yes --> noop([No changes needed]) + idem -- no --> set[On the booted deployment's bootconfig:
clear every known source key,
re-set all sources except S,
set S = O if given] + set --> stage[stage_tree_with_options
merge = booted, commit/origin = base,
override_kernel_argv = merged] +``` + +* **Base vs. merge deployment.** The commit, origin and current + `options` come from the *staged* deployment when one exists, so a + pending `bootc upgrade` is kept. The merge deployment (for the `/etc` + merge, and where the source keys are written) is always the booted one. +* **In-place replacement.** The source's arguments are replaced where + they were, so re-applying an unchanged source is byte-identical and + a no-op even when other arguments follow, and the relative order of + arguments is preserved (it matters where the last occurrence wins). +* **The full set is written on every call**, tombstones included. This + is what lets ostree carry the keys correctly through staging. + +### How the keys survive staging + +A staged deployment has no BLS entry until finalization at shutdown, and +any later staging in the same boot — `bootc upgrade`, `rpm-ostree kargs`, +another `set-options-for-source` — replaces it. ostree carries the +`x-options-source-*` keys across that gap in the staged deployment data +(`bootconfig-extra`) and decides which set to carry when a staging is +replaced; that mechanism, and the contract bootc follows as an "aware" +caller, are documented in +[Extension BLS keys and staged deployments](https://ostreedev.github.io/ostree/bootconfig-extra/) +on the ostree side. It requires ostree 2026.1 (2026.5 for the case where +another tool re-stages in the same boot); bootc checks the version at +runtime. + +On composefs-backed systems bootc writes the BLS entries itself, and +`set-options-for-source` keeps the same model without ostree: it stages +the booted deployment again with a new entry carrying the merged +`options` line, and the current entry becomes the rollback, so +`bootc rollback` undoes the change just as it does for a new +deployment. Finalization installs the pending entries at shutdown; there +is no new state directory since the deployment is the same. If an upgrade +is already staged, its pending entry is rewritten instead and the change +rides along with it (and is dropped with it). A removed source's key is +deleted rather than tombstoned. UKI boot is not supported, since the +arguments are embedded in the image. + +### Interaction with `bootc upgrade` / `switch` + +```mermaid +flowchart TD + s([upgrade / switch]) --> base{Staged deployment in the
same stateroot exists?} + base -- yes --> b1[base = staged deployment] + base -- no --> b2[base = booted deployment] + b1 --> k[kargs = base's options line] + b2 --> k + k --> old[old = kargs.d of base's tree] + old --> new{new image has
/usr/lib/bootc/kargs.d?} + new -- no --> add[kargs += old] --> stage + new -- yes --> diff[remove old - new from kargs,
add new - old to kargs] --> stage[stage_tree_with_options
override_kernel_argv = kargs] +``` + +Upgrading builds on the staged deployment's arguments when there is one, +so a source change (or an `rpm-ostree kargs` change) staged earlier in +the same boot survives the upgrade; only the `kargs.d` *diff* between the +two images is applied on top. The upgrade path never touches source keys; +ostree carries them forward, and on composefs bootc copies the extension +keys of the entry it builds on into the new one along with `options`. + +### Interaction with `rpm-ostree` and direct edits + +`rpm-ostree kargs` likewise builds on the pending deployment, so the two +can be interleaved in either order. It does not know about source keys; +ostree carries them. If something edits `options` directly (for example +`rpm-ostree kargs --delete` of an argument a source owns), the source's +record is stale until that source is next written — a subsequent +`set-options-for-source` for it simply finds nothing to remove. + +### Consumers + +TuneD (2.27+ with the bootc bootloader support) uses `--source tuned`, +declaring its profile's full argument set on every apply and clearing it +on unapply. On images with a transient `/etc`, set TuneD's +`profile_mode` to `manual` in the image so it does not auto-select a +different profile on each boot and clear the administrator's arguments. + ## Injecting default arguments into custom kernels The Linux kernel supports building in arguments into the kernel diff --git a/docs/src/man/bootc-loader-entries-set-options-for-source.8.md b/docs/src/man/bootc-loader-entries-set-options-for-source.8.md index 238e402042..c9699fbc70 100644 --- a/docs/src/man/bootc-loader-entries-set-options-for-source.8.md +++ b/docs/src/man/bootc-loader-entries-set-options-for-source.8.md @@ -21,6 +21,11 @@ When a staged deployment already exists (e.g. from `bootc upgrade`), it is replaced using the staged deployment's commit and origin, preserving the pending upgrade while layering the kargs change on top. +On composefs systems the booted deployment itself is staged again with +a new boot entry; the current entry becomes the rollback and the change +is finalized at shutdown like an upgrade. `bootc rollback` undoes it. +If an upgrade is already staged, its pending entry is updated instead. + # OPTIONS @@ -36,9 +41,16 @@ preserving the pending upgrade while layering the kargs change on top. # REQUIREMENTS -This command requires ostree >= 2026.1 with `bootconfig-extra` support -for preserving extension BLS keys through staged deployment roundtrips. -On older ostree versions, the command will exit with an error. +On the ostree backend this command requires ostree >= 2026.1 with +`bootconfig-extra` support for preserving extension BLS keys through +staged deployment roundtrips. On older ostree versions, the command +will exit with an error. ostree >= 2026.5 is needed for the source keys +to survive when another tool (e.g. `rpm-ostree kargs`) re-stages before +the reboot. + +On composefs systems the boot entries must be BLS Type #1 entries; with +a UKI the kernel arguments are embedded in the image and the command +exits with an error. # EXAMPLES @@ -65,8 +77,8 @@ Multiple sources can coexist independently: # KNOWN LIMITATIONS -Source keys set by prior calls in the same boot cycle (before any reboot) -are discovered by reading the staged deployment data file at +On the ostree backend, source keys set by prior calls in the same boot +cycle (before any reboot) are discovered by reading the staged deployment data file at `/run/ostree/staged-deployment`. If this file is missing or cannot be parsed, sources from prior calls may not be discovered, potentially orphaning their kargs. In practice this should not occur, as the file is diff --git a/tmt/plans/integration.fmf b/tmt/plans/integration.fmf index 7b52f4f02a..0ee88428d7 100644 --- a/tmt/plans/integration.fmf +++ b/tmt/plans/integration.fmf @@ -271,7 +271,6 @@ execute: how: fmf test: - /tmt/tests/tests/test-42-loader-entries-source - extra-fixme_skip_if_composefs: true /plan-43-switch-same-digest: summary: Error on bootc switch to image with identical fs-verity digest diff --git a/tmt/tests/booted/test-loader-entries-source.nu b/tmt/tests/booted/test-loader-entries-source.nu index 423f80a97c..cf707dbf56 100644 --- a/tmt/tests/booted/test-loader-entries-source.nu +++ b/tmt/tests/booted/test-loader-entries-source.nu @@ -1,135 +1,228 @@ # number: 42 -# extra: -# fixme_skip_if_composefs: true # tmt: # summary: Test bootc loader-entries set-options-for-source -# duration: 30m +# duration: 45m # # This test verifies the source-tracked kernel argument management via -# bootc loader-entries set-options-for-source. It covers: +# bootc loader-entries set-options-for-source on both the ostree and the +# composefs backend. It covers: # 1. Input validation (invalid/empty source names) # 2. Adding source-tracked kargs and verifying they appear in /proc/cmdline # 3. Kargs and x-options-source-* BLS keys surviving the staging roundtrip -# 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 (root=, ostree=, etc.) preserved through changes -# 9. --options "" (empty string) clears kargs without removing the source -# 10. Staged deployment interaction (bootc switch + set-options-for-source -# preserves the pending image switch) +# 4. `bootc rollback` boots without the change, and re-applying it works +# 5. Source replacement semantics (old kargs removed, new ones added) +# 6. Multiple sources coexisting independently +# 7. Source removal (--source without --options clears all owned kargs) +# 8. Idempotent operation (no changes when kargs already match) +# 9. Existing system kargs (root=, ostree=, composefs=, etc.) preserved +# 10. --options "" (empty string) clears kargs without removing the source +# 11. Staged deployment interaction (bootc switch + set-options-for-source +# preserves the pending image switch and the source's ownership key) +# 12. ostree only: cross-consumer staging (bootc stages source kargs, then +# rpm-ostree re-stages on the same boot via kargs --append; the +# x-options-source-* keys must survive in the replacement staged +# deployment, see ostreedev/ostree#3611) # -# Requires ostree with bootconfig-extra support (>= 2026.1). +# On ostree this requires bootconfig-extra support (ostree >= 2026.1). +# On composefs the UKI boot type is skipped since the kargs are embedded in +# the PE binary. # See: https://github.com/ostreedev/ostree/pull/3570 +# See: https://github.com/ostreedev/ostree/pull/3611 # See: https://github.com/bootc-dev/bootc/issues/899 use std assert use tap.nu -let is_bad_version = ostree --version | lines | any {|l| $l | str contains "2026.2" } +let composefs = tap is_composefs -if $is_bad_version { - print "Found Ostree v2026.2, skipping test" - exit 0 +if $composefs { + 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 + } +} else { + let is_bad_version = ostree --version | lines | any {|l| $l | str contains "2026.2" } + if $is_bad_version { + print "Found Ostree v2026.2, skipping test" + exit 0 + } } def parse_cmdline [] { open /proc/cmdline | str trim | split row " " } -# Read x-options-source-* keys from the booted BLS entry. -# The booted deployment always has the highest version number, -# so we pick the last entry when sorted by filename (ostree-N.conf). +# The BLS entry the kernel was booted from. Its options are all on the +# command line (the bootloader only adds to it); take the largest such set, +# because a kernel-argument change on composefs keeps the deployment's +# previous entry as its rollback and that one differs only by the arguments +# it lacks. The boot partition is not necessarily mounted at /boot on +# composefs systems, so look under /sysroot/boot as well. +def booted_bls_entry [] { + let cmdline = parse_cmdline + let entries = if (tap is_composefs) { + [/boot/loader/entries /sysroot/boot/loader/entries] + | each { |d| glob $"($d)/bootc_*.conf" } + | flatten + } else { + glob /boot/loader/entries/ostree-*.conf + } + let matching = $entries | each { |e| + let options = open $e + | lines + | where { |line| $line starts-with "options " } + | first + | str replace "options " "" + | str trim + | split row " " + if ($options | all { |o| $o in $cmdline }) { + { path: $e, score: ($options | length) } + } else { + null + } + } | compact + if ($matching | is-empty) { + error make { msg: "No BLS entry matches /proc/cmdline" } + } + $matching | sort-by score | last | get path +} + +# The x-options-source-* lines of the booted BLS entry def read_bls_source_keys [] { - let entries = glob /boot/loader/entries/ostree-*.conf | sort - if ($entries | length) == 0 { - error make { msg: "No BLS entries found" } + open (booted_bls_entry) | lines | where { |line| $line starts-with "x-options-source-" } +} + +# Value of an x-options-source-* line, or "" for a tombstoned source. +def source_key_value [line: string] { + $line | str replace --regex '^x-options-source-\S+\s*' '' | str trim +} + +# The x-options-source-* lines for one source +def source_keys [name: string] { + read_bls_source_keys | where { |line| $line starts-with $"x-options-source-($name)" } +} + +# Whether `name` owns nothing: removed on composefs, tombstoned on ostree +def assert_source_gone [name: string] { + let keys = source_keys $name + if (tap is_composefs) { + assert (($keys | length) == 0) $"($name) source key should be gone" + } else { + assert (($keys | length) == 1) $"($name) source key should remain as a tombstone" + assert ((source_key_value ($keys | first)) == "") $"($name) source key should be empty" } - let entry = open ($entries | last) - $entry | lines | where { |line| $line starts-with "x-options-source-" } } -# Save the current system kargs (root=, ostree=, rw, etc.) for later comparison +# Save the current system kargs (root=, rw, etc.) for later comparison. +# The ostree= / composefs= arguments are excluded because they change +# between deployments. def save_system_kargs [] { let cmdline = parse_cmdline - # Filter to well-known system kargs that must never be lost - # Note: ostree= is excluded because its value changes between deployments - # (boot version counter, bootcsum). It's managed by ostree's - # install_deployment_kernel() and always regenerated during finalization. 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 assert_system_kargs [phase: string] { + let cmdline = parse_cmdline + for karg in (open /var/bootc-test-system-kargs.json) { + assert ($karg in $cmdline) $"system karg '($karg)' must survive ($phase)" + } +} + +def assert_staged [expected: bool, msg: string] { + let st = bootc status --json | from json + assert (($st.status.staged != null) == $expected) $msg } def first_boot [] { tap begin "loader-entries set-options-for-source" - # Save system kargs for later verification save_system_kargs # -- Input validation -- - - # Invalid source name (spaces) 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" - # Invalid source name (special chars) 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" - # Empty source name 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 + # Valid name with underscores/dashes, then clear it (no --options = remove source) bootc loader-entries set-options-for-source --source "my_custom-src" --options "testvalid=1" - # Clear it immediately (no --options = remove source) bootc loader-entries set-options-for-source --source "my_custom-src" # -- Add source kargs (multiple sources before reboot) -- bootc loader-entries set-options-for-source --source tuned --options "nohz=full isolcpus=1-3" bootc loader-entries set-options-for-source --source admin --options "quiet" - # Verify deployment is staged - let st = bootc status --json | from json - assert ($st.status.staged != null) "deployment should be staged" + assert_staged true "deployment should be staged" print "ok: validation and initial staging" tmt-reboot } def second_boot [] { - # Verify kargs survived the staging roundtrip 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" - - # Verify both sources staged in first_boot survived assert ("quiet" in $cmdline) "admin quiet karg should be in cmdline after reboot" print "ok: multiple sources staged before reboot both survived" - # Verify system kargs were 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" + assert_system_kargs "the staging roundtrip" - # Verify x-options-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 + let tuned_keys = source_keys tuned + assert (($tuned_keys | length) > 0) "x-options-source-tuned should be in BLS entry" + let tuned_line = $tuned_keys | 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" - 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" + assert ((source_keys admin | length) > 0) "x-options-source-admin should be in BLS entry" print "ok: kargs and source keys survived reboot" + # -- Rollback: the previous entry, without the change, is the rollback -- + let st = bootc status --json | from json + assert ($st.status.rollback != null) "kargs change must leave a rollback" + bootc rollback + let st = bootc status --json | from json + assert ($st.status.rollbackQueued == true) "rollback should be queued" + print "ok: rollback queued" + + tmt-reboot +} + +def third_boot [] { + let cmdline = parse_cmdline + assert ("nohz=full" not-in $cmdline) "nohz=full must be gone after rollback" + assert ("isolcpus=1-3" not-in $cmdline) "isolcpus=1-3 must be gone after rollback" + assert ("quiet" not-in $cmdline) "quiet must be gone after rollback" + assert ((source_keys tuned | length) == 0) "rolled back entry must not have the tuned key" + assert ((source_keys admin | length) == 0) "rolled back entry must not have the admin key" + assert_system_kargs "rollback" + + let st = bootc status --json | from json + assert ($st.status.rollback != null) "the kargs entry should now be the rollback" + print "ok: rollback booted without the source kargs" + + # Re-apply and continue with the change in place + bootc loader-entries set-options-for-source --source tuned --options "nohz=full isolcpus=1-3" + bootc loader-entries set-options-for-source --source admin --options "quiet" + assert_staged true "re-applied kargs should be staged" + + tmt-reboot +} + +def fourth_boot [] { + let cmdline = parse_cmdline + assert ("nohz=full" in $cmdline) "nohz=full should be back after re-applying" + assert ("isolcpus=1-3" in $cmdline) "isolcpus=1-3 should be back after re-applying" + assert ("quiet" in $cmdline) "quiet should be back after re-applying" + assert ((source_keys tuned | length) > 0) "tuned source key should be back" + print "ok: re-applied kargs and keys survived reboot" + # Clean up admin source before continuing with replacement test bootc loader-entries set-options-for-source --source admin @@ -139,21 +232,14 @@ def second_boot [] { tmt-reboot } -def third_boot [] { - # Verify replacement worked +def fifth_boot [] { 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" - # Admin source was removed in second_boot assert ("quiet" not-in $cmdline) "admin quiet should be gone after removal" - - # Verify system kargs still preserved after replacement - let system_kargs = load_system_kargs - for karg in $system_kargs { - assert ($karg in $cmdline) $"system karg '($karg)' must survive replacement" - } + assert_system_kargs "replacement" print "ok: source replacement persisted, system kargs preserved" # -- Multiple sources coexist -- @@ -162,57 +248,85 @@ def third_boot [] { tmt-reboot } -def fourth_boot [] { - # Verify both sources persisted +def sixth_boot [] { let cmdline = parse_cmdline 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" assert ("rd.driver.pre=vfio-pci" in $cmdline) "dracut karg should be present" - - # Verify both source keys in BLS - let source_keys = read_bls_source_keys - let tuned_keys = $source_keys | where { |line| $line starts-with "x-options-source-tuned" } - let dracut_keys = $source_keys | where { |line| $line starts-with "x-options-source-dracut" } - assert (($tuned_keys | length) > 0) "tuned source key should exist" - assert (($dracut_keys | length) > 0) "dracut source key should exist" + assert ((source_keys tuned | length) > 0) "tuned source key should exist" + assert ((source_keys dracut | length) > 0) "dracut source key should exist" print "ok: multiple sources coexist" # -- Clear source with empty --options "" (different from no --options) -- - # --options "" should remove the kargs but the key can remain with empty value + # --options "" removes the kargs but the key can remain with an empty value bootc loader-entries set-options-for-source --source dracut --options "" - # dracut kargs should be removed from pending deployment - let st = bootc status --json | from json - assert ($st.status.staged != null) "empty options should still stage a deployment" + assert_staged true "empty options should still stage a deployment" print "ok: --options '' clears kargs" # Now also test no --options (remove the source entirely) - # First re-add dracut so we can test removal bootc loader-entries set-options-for-source --source dracut --options "rd.driver.pre=vfio-pci" - # Then remove it with no --options bootc loader-entries set-options-for-source --source dracut + if not (tap is_composefs) { + # -- Cross-consumer staging -- + # bootc stages source-tracked kargs and then rpm-ostree re-stages on + # the same boot, appending an unrelated karg. The replacement staged + # deployment must inherit the x-options-source-* keys from the + # previously staged one via ostree's fallback path. + bootc loader-entries set-options-for-source --source crosstest --options "cross1=a cross2=b" + assert_staged true "crosstest should stage a deployment" + + rpm-ostree kargs --append=rpmarg=yes + assert_staged true "deployment should still be staged after rpm-ostree kargs" + print "ok: cross-consumer staging set up (bootc then rpm-ostree)" + } + tmt-reboot } -def fifth_boot [] { - # Verify dracut cleared, tuned preserved +def seventh_boot [] { let cmdline = parse_cmdline - assert ("rd.driver.pre=vfio-pci" not-in $cmdline) "dracut karg should be gone" + + if not (tap is_composefs) { + assert ("cross1=a" in $cmdline) "crosstest cross1=a should survive rpm-ostree re-staging" + assert ("cross2=b" in $cmdline) "crosstest cross2=b should survive rpm-ostree re-staging" + assert ("rpmarg=yes" in $cmdline) "rpm-ostree rpmarg=yes should be present" + + let crosstest_keys = source_keys crosstest + assert (($crosstest_keys | length) > 0) "x-options-source-crosstest BLS key must survive rpm-ostree re-staging" + let crosstest_line = $crosstest_keys | first + assert ($crosstest_line | str contains "cross1=a") "crosstest source key should contain cross1=a" + assert ($crosstest_line | str contains "cross2=b") "crosstest source key should contain cross2=b" + print "ok: cross-consumer staging preserved all source kargs and rpm-ostree karg" + } + 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" + assert ("rd.driver.pre=vfio-pci" not-in $cmdline) "dracut karg should be gone" + assert_source_gone dracut print "ok: source clear persisted" # -- Idempotent: same kargs again should be a no-op -- + # tuned already has "nohz=on rcu_nocbs=2-7", so nothing is staged even + # though other arguments follow its own on the options line. bootc loader-entries set-options-for-source --source tuned --options "nohz=on rcu_nocbs=2-7" - # Should not stage a new deployment (idempotent) - let st = bootc status --json | from json - assert ($st.status.staged == null) "idempotent call should not stage a deployment" + assert_staged false "idempotent call should not stage a deployment" print "ok: idempotent operation" + if not (tap is_composefs) { + # Clean up the cross-consumer kargs. These stage a deployment, and + # the image switch below re-stages on top of it: the switch must + # build on the staged kargs, not the booted ones, or this removal + # would be silently undone (verified in eighth_boot). + bootc loader-entries set-options-for-source --source crosstest + rpm-ostree kargs --delete=rpmarg=yes + } + # -- Staged deployment interaction -- - # Build a derived image and switch to it (this stages a deployment). - # Then call set-options-for-source on top. The staged deployment should - # be replaced with one that has the new image AND the source kargs. + # Switch to a derived image (this stages a deployment), then change a + # source on top. The staged deployment must keep the new image and get + # the new kargs, and its entry must carry the x-options-source-tuned key; + # without the latter, tuned's kargs could never be removed again. bootc image copy-to-storage let td = mktemp -d @@ -222,43 +336,54 @@ RUN echo source-test-marker > /usr/share/source-test-marker.txt podman build -t localhost/bootc-source-test $"($td)" bootc switch --transport containers-storage localhost/bootc-source-test - let st = bootc status --json | from json - assert ($st.status.staged != null) "switch should stage a deployment" + assert_staged true "switch should stage a deployment" - # Now add source kargs on top of the staged switch bootc loader-entries set-options-for-source --source tuned --options "nohz=on rcu_nocbs=2-7 skew_tick=1" - - # Verify a deployment is still staged (it was replaced, not removed) - let st = bootc status --json | from json - assert ($st.status.staged != null) "deployment should still be staged after set-options-for-source" + assert_staged true "deployment should still be staged after set-options-for-source" tmt-reboot } -def sixth_boot [] { - # Verify the image switch landed (the derived image's marker file exists) - assert ("/usr/share/source-test-marker.txt" | path exists) "derived image marker should exist" +def eighth_boot [] { + let marker = open /usr/share/source-test-marker.txt | str trim + assert equal $marker "source-test-marker" print "ok: image switch preserved" - # Verify the source kargs also landed let cmdline = parse_cmdline assert ("nohz=on" in $cmdline) "tuned nohz=on should be present" assert ("rcu_nocbs=2-7" in $cmdline) "tuned rcu_nocbs=2-7 should be present" assert ("skew_tick=1" in $cmdline) "tuned skew_tick=1 should be present" - # Verify source key in BLS - 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) "tuned source key should exist after staged interaction" + if not (tap is_composefs) { + assert ("cross1=a" not-in $cmdline) "crosstest kargs should be gone after cleanup" + assert ("cross2=b" not-in $cmdline) "crosstest kargs should be gone after cleanup" + assert ("rpmarg=yes" not-in $cmdline) "rpm-ostree rpmarg should be gone after cleanup" + assert_source_gone crosstest + } + + let tuned_keys = source_keys tuned + assert (($tuned_keys | length) > 0) "x-options-source-tuned must be carried into the new entry" + let tuned_val = source_key_value ($tuned_keys | first) + assert ($tuned_val | str contains "skew_tick=1") $"tuned key should own skew_tick=1, got '($tuned_val)'" + assert_system_kargs "the staged interaction" print "ok: staged deployment interaction preserved both image and source kargs" - # Verify system kargs still intact - let system_kargs = load_system_kargs + # Remove the source: this only works if its key was carried into the + # new entry. + bootc loader-entries set-options-for-source --source tuned + assert_staged true "source removal should stage a deployment" + + tmt-reboot +} + +def ninth_boot [] { let cmdline = parse_cmdline - for karg in $system_kargs { - assert ($karg in $cmdline) $"system karg '($karg)' must survive staged interaction" - } - print "ok: system kargs preserved through all phases" + assert ("nohz=on" not-in $cmdline) "tuned nohz=on should be gone after removal" + assert ("rcu_nocbs=2-7" not-in $cmdline) "tuned rcu_nocbs=2-7 should be gone after removal" + assert ("skew_tick=1" not-in $cmdline) "tuned skew_tick=1 should be gone after removal" + assert_source_gone tuned + assert_system_kargs "all phases" + print "ok: source removal after switch persisted, system kargs preserved" tap ok } @@ -271,6 +396,9 @@ def main [] { "3" => fourth_boot, "4" => fifth_boot, "5" => sixth_boot, + "6" => seventh_boot, + "7" => eighth_boot, + "8" => ninth_boot, $o => { error make { msg: $"Unexpected TMT_REBOOT_COUNT ($o)" } }, } } diff --git a/tmt/tests/tests.fmf b/tmt/tests/tests.fmf index c2c91422d9..afa215518a 100644 --- a/tmt/tests/tests.fmf +++ b/tmt/tests/tests.fmf @@ -165,7 +165,7 @@ check: /test-42-loader-entries-source: summary: Test bootc loader-entries set-options-for-source - duration: 30m + duration: 45m test: nu booted/test-loader-entries-source.nu /test-43-switch-same-digest: