From acfe29eda7025fb88b20563133468cecd29ca50d Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 5 Nov 2025 11:51:42 -0500 Subject: [PATCH 01/13] cli: Only detect containers, not ostree containers Distinguishing isn't useful and we want to get away from having ostree repos in images anyways. Signed-off-by: Colin Walters --- crates/lib/src/cli.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/crates/lib/src/cli.rs b/crates/lib/src/cli.rs index a1350c03b..a17ec3302 100644 --- a/crates/lib/src/cli.rs +++ b/crates/lib/src/cli.rs @@ -867,11 +867,6 @@ fn prepare_for_write() -> Result<()> { if ENTERED.load(Ordering::SeqCst) { return Ok(()); } - if ostree_ext::container_utils::is_ostree_container()? { - anyhow::bail!( - "Detected container (ostree base); this command requires a booted host system." - ); - } if ostree_ext::container_utils::running_in_container() { anyhow::bail!("Detected container; this command requires a booted host system."); } From 4658f845c8c4ac6b604b693984a0cbeef2fc8b7c Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 5 Nov 2025 11:52:46 -0500 Subject: [PATCH 02/13] cli: Remove ostree detection in prepare_for_write We detect things later regardless, and this is prep for composefs. Signed-off-by: Colin Walters --- crates/lib/src/cli.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/crates/lib/src/cli.rs b/crates/lib/src/cli.rs index a17ec3302..a933b37e0 100644 --- a/crates/lib/src/cli.rs +++ b/crates/lib/src/cli.rs @@ -870,10 +870,6 @@ fn prepare_for_write() -> Result<()> { if ostree_ext::container_utils::running_in_container() { anyhow::bail!("Detected container; this command requires a booted host system."); } - anyhow::ensure!( - ostree_booted()?, - "This command requires an ostree-booted host system" - ); crate::cli::require_root(false)?; ensure_self_unshared_mount_namespace()?; if crate::lsm::selinux_enabled()? && !crate::lsm::selinux_ensure_install()? { From 7ebd101c1ddc0a448b04149036b93761f4d8bed9 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 5 Nov 2025 11:48:50 -0500 Subject: [PATCH 03/13] style: Remove unnecessary explicit return statements Use implicit return expressions instead of explicit `return` statements for the final expression in functions, following Rust conventions. Assisted-by: Claude Code (Sonnet 4.5) Signed-off-by: Colin Walters --- crates/lib/src/bootc_composefs/boot.rs | 4 ++-- crates/lib/src/bootc_composefs/status.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/lib/src/bootc_composefs/boot.rs b/crates/lib/src/bootc_composefs/boot.rs index 43483a519..d95e0b2b6 100644 --- a/crates/lib/src/bootc_composefs/boot.rs +++ b/crates/lib/src/bootc_composefs/boot.rs @@ -178,7 +178,7 @@ pub fn get_sysroot_parent_dev() -> Result { anyhow::bail!("Could not find parent device for mountpoint /sysroot"); }; - return Ok(parent); + Ok(parent) } pub fn type1_entry_conf_file_name(sort_key: impl std::fmt::Display) -> String { @@ -211,7 +211,7 @@ fn compute_boot_digest( let digest: &[u8] = &hasher.finish().context("Finishing digest")?; - return Ok(hex::encode(digest)); + Ok(hex::encode(digest)) } /// Given the SHA256 sum of current VMlinuz + Initrd combo, find boot entry with the same SHA256Sum diff --git a/crates/lib/src/bootc_composefs/status.rs b/crates/lib/src/bootc_composefs/status.rs index 1f4caa3ef..c58bb99dc 100644 --- a/crates/lib/src/bootc_composefs/status.rs +++ b/crates/lib/src/bootc_composefs/status.rs @@ -126,7 +126,7 @@ pub(crate) fn get_sorted_type1_boot_entries( all_configs.sort_by(|a, b| if ascending { a.cmp(b) } else { b.cmp(a) }); - return Ok(all_configs); + Ok(all_configs) } /// imgref = transport:image_name @@ -244,7 +244,7 @@ async fn boot_entry_from_composefs_deployment( soft_reboot_capable: false, }; - return Ok(e); + Ok(e) } #[context("Getting composefs deployment status")] From e6d8d0bf39c8d6e2490b1437e61e48a281f4d83f Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 30 Oct 2025 07:20:44 -0400 Subject: [PATCH 04/13] store: Introduce BootedOstree and BootedComposefs types Refactor the storage layer to introduce new types that better encapsulate boot context: - Add BootedStorage wrapper that detects ostree vs composefs boot - Add BootedOstree struct holding sysroot reference and deployment - Add BootedComposefs struct holding repository and cmdline - Add BootedStorageKind enum to discriminate between backends This allows passing boot context as a cohesive unit rather than separate parameters. Update the upgrade code path to use these new types: - Change get_status() to accept &BootedOstree instead of separate params - Update handle_staged_soft_reboot() similarly - Update upgrade() to use structs throughout Add comprehensive documentation to all new types and methods in store/mod.rs. Assisted-by: Claude Code (Sonnet 4.5) Signed-off-by: Colin Walters --- crates/lib/src/bootc_composefs/status.rs | 13 +- crates/lib/src/bootc_composefs/update.rs | 8 +- crates/lib/src/cli.rs | 138 +++++++++-------- crates/lib/src/image.rs | 2 +- crates/lib/src/install.rs | 2 +- crates/lib/src/status.rs | 28 ++-- crates/lib/src/store/mod.rs | 180 ++++++++++++++++++++--- 7 files changed, 265 insertions(+), 106 deletions(-) diff --git a/crates/lib/src/bootc_composefs/status.rs b/crates/lib/src/bootc_composefs/status.rs index c58bb99dc..9da338d02 100644 --- a/crates/lib/src/bootc_composefs/status.rs +++ b/crates/lib/src/bootc_composefs/status.rs @@ -247,14 +247,23 @@ async fn boot_entry_from_composefs_deployment( Ok(e) } -#[context("Getting composefs deployment status")] pub(crate) async fn composefs_deployment_status() -> Result { let composefs_state = composefs_booted()? .ok_or_else(|| anyhow::anyhow!("Failed to find composefs parameter in kernel cmdline"))?; - let composefs_digest = &composefs_state.digest; let sysroot = Dir::open_ambient_dir("/sysroot", ambient_authority()).context("Opening sysroot")?; + + composefs_deployment_status_from(&sysroot, composefs_state).await +} + +#[context("Getting composefs deployment status")] +pub(crate) async fn composefs_deployment_status_from( + sysroot: &Dir, + cmdline: &ComposefsCmdline, +) -> Result { + let composefs_digest = &cmdline.digest; + let deployments = sysroot .read_dir(STATE_DIR_RELATIVE) .with_context(|| format!("Reading sysroot {STATE_DIR_RELATIVE}"))?; diff --git a/crates/lib/src/bootc_composefs/update.rs b/crates/lib/src/bootc_composefs/update.rs index 7ad09f82c..8d625bbf7 100644 --- a/crates/lib/src/bootc_composefs/update.rs +++ b/crates/lib/src/bootc_composefs/update.rs @@ -14,7 +14,7 @@ use crate::{ }, cli::UpgradeOpts, spec::ImageReference, - store::ComposefsRepository, + store::{BootedComposefs, ComposefsRepository, Storage}, }; use cap_std_ext::cap_std::{ambient_authority, fs::Dir}; @@ -61,7 +61,11 @@ async fn is_image_pulled( } #[context("Upgrading composefs")] -pub(crate) async fn upgrade_composefs(opts: UpgradeOpts) -> Result<()> { +pub(crate) async fn upgrade_composefs( + opts: UpgradeOpts, + _storage: &Storage, + _composefs: &BootedComposefs, +) -> Result<()> { let host = composefs_deployment_status() .await .context("Getting composefs deployment status")?; diff --git a/crates/lib/src/cli.rs b/crates/lib/src/cli.rs index a933b37e0..20d1bae98 100644 --- a/crates/lib/src/cli.rs +++ b/crates/lib/src/cli.rs @@ -38,7 +38,6 @@ use crate::bootc_composefs::{ finalize::{composefs_backend_finalize, get_etc_diff}, rollback::composefs_rollback, state::composefs_usr_overlay, - status::composefs_booted, switch::switch_composefs, update::upgrade_composefs, }; @@ -48,7 +47,8 @@ use crate::podstorage::set_additional_image_store; use crate::progress_jsonl::{ProgressWriter, RawProgressFd}; use crate::spec::Host; use crate::spec::ImageReference; -use crate::store::ComposefsRepository; +use crate::store::{BootedOstree, ComposefsRepository, Storage}; +use crate::store::{BootedStorage, BootedStorageKind}; use crate::utils::sigpolicy_from_opt; /// Shared progress options @@ -712,24 +712,11 @@ pub(crate) fn ensure_self_unshared_mount_namespace() -> Result<()> { bootc_utils::reexec::reexec_with_guardenv(recurse_env, &["unshare", "-m", "--"]) } -/// Acquire a locked sysroot. -/// TODO drain this and the above into SysrootLock -#[context("Acquiring sysroot")] -pub(crate) async fn get_locked_sysroot() -> Result { - prepare_for_write()?; - let sysroot = ostree::Sysroot::new_default(); - sysroot.set_mount_namespace_in_use(); - let sysroot = ostree_ext::sysroot::SysrootLock::new_from_sysroot(&sysroot).await?; - sysroot.load(gio::Cancellable::NONE)?; - Ok(sysroot) -} - /// Load global storage state, expecting that we're booted into a bootc system. #[context("Initializing storage")] -pub(crate) async fn get_storage() -> Result { - let global_run = Dir::open_ambient_dir("/run", cap_std::ambient_authority())?; - let sysroot = get_locked_sysroot().await?; - crate::store::Storage::new(sysroot, &global_run) +pub(crate) async fn get_storage() -> Result { + prepare_for_write()?; + BootedStorage::new().await } #[context("Querying root privilege")] @@ -811,7 +798,7 @@ where /// Handle soft reboot for staged deployments (used by upgrade and switch) #[context("Handling staged soft reboot")] fn handle_staged_soft_reboot( - sysroot: &SysrootLock, + booted_ostree: &BootedOstree<'_>, soft_reboot_mode: Option, host: &crate::spec::Host, ) -> Result<()> { @@ -819,7 +806,7 @@ fn handle_staged_soft_reboot( soft_reboot_mode, host.status.staged.as_ref(), "staged", - || soft_reboot_staged(sysroot), + || soft_reboot_staged(booted_ostree.sysroot), ) } @@ -881,11 +868,14 @@ fn prepare_for_write() -> Result<()> { /// Implementation of the `bootc upgrade` CLI command. #[context("Upgrading")] -async fn upgrade(opts: UpgradeOpts) -> Result<()> { - let sysroot = &get_storage().await?; - let ostree = sysroot.get_ostree()?; - let repo = &ostree.repo(); - let (booted_deployment, _deployments, host) = crate::status::get_status_require_booted(ostree)?; +async fn upgrade( + opts: UpgradeOpts, + storage: &Storage, + booted_ostree: &BootedOstree<'_>, +) -> Result<()> { + let repo = &booted_ostree.repo(); + + let host = crate::status::get_status(booted_ostree)?.1; let imgref = host.spec.image.as_ref(); let prog: ProgressWriter = opts.progress.try_into()?; @@ -953,16 +943,16 @@ async fn upgrade(opts: UpgradeOpts) -> Result<()> { .unwrap_or_default(); if staged_unchanged { println!("Staged update present, not changed."); - handle_staged_soft_reboot(ostree, opts.soft_reboot, &host)?; + handle_staged_soft_reboot(booted_ostree, opts.soft_reboot, &host)?; if opts.apply { crate::reboot::reboot()?; } } else if booted_unchanged { println!("No update available.") } else { - let stateroot = booted_deployment.osname(); - let from = MergeState::from_stateroot(sysroot, &stateroot)?; - crate::deploy::stage(sysroot, from, &fetched, &spec, prog.clone()).await?; + let stateroot = booted_ostree.stateroot(); + let from = MergeState::from_stateroot(storage, &stateroot)?; + crate::deploy::stage(storage, from, &fetched, &spec, prog.clone()).await?; changed = true; if let Some(prev) = booted_image.as_ref() { if let Some(fetched_manifest) = fetched.get_manifest(repo)? { @@ -974,13 +964,13 @@ async fn upgrade(opts: UpgradeOpts) -> Result<()> { } } if changed { - sysroot.update_mtime()?; + storage.update_mtime()?; if opts.soft_reboot.is_some() { // At this point we have new staged deployment and the host definition has changed. // We need the updated host status before we check if we can prepare the soft-reboot. - let updated_host = crate::status::get_status(ostree, Some(&booted_deployment))?.1; - handle_staged_soft_reboot(ostree, opts.soft_reboot, &updated_host)?; + let updated_host = crate::status::get_status(booted_ostree)?.1; + handle_staged_soft_reboot(booted_ostree, opts.soft_reboot, &updated_host)?; } if opts.apply { @@ -1030,8 +1020,8 @@ async fn switch(opts: SwitchOpts) -> Result<()> { let cancellable = gio::Cancellable::NONE; - let sysroot = &get_storage().await?; - let ostree = sysroot.get_ostree()?; + let storage = &get_storage().await?; + let ostree = storage.get_ostree()?; let repo = &ostree.repo(); let (booted_deployment, _deployments, host) = crate::status::get_status_require_booted(ostree)?; @@ -1081,16 +1071,20 @@ async fn switch(opts: SwitchOpts) -> Result<()> { } let stateroot = booted_deployment.osname(); - let from = MergeState::from_stateroot(sysroot, &stateroot)?; - crate::deploy::stage(sysroot, from, &fetched, &new_spec, prog.clone()).await?; + let from = MergeState::from_stateroot(storage, &stateroot)?; + crate::deploy::stage(storage, from, &fetched, &new_spec, prog.clone()).await?; - sysroot.update_mtime()?; + storage.update_mtime()?; if opts.soft_reboot.is_some() { // At this point we have staged the deployment and the host definition has changed. // We need the updated host status before we check if we can prepare the soft-reboot. - let updated_host = crate::status::get_status(ostree, Some(&booted_deployment))?.1; - handle_staged_soft_reboot(ostree, opts.soft_reboot, &updated_host)?; + let booted_ostree = BootedOstree { + sysroot: ostree, + deployment: booted_deployment.clone(), + }; + let updated_host = crate::status::get_status(&booted_ostree)?.1; + handle_staged_soft_reboot(&booted_ostree, opts.soft_reboot, &updated_host)?; } if opts.apply { @@ -1103,9 +1097,9 @@ async fn switch(opts: SwitchOpts) -> Result<()> { /// Implementation of the `bootc rollback` CLI command. #[context("Rollback")] async fn rollback(opts: &RollbackOpts) -> Result<()> { - let sysroot = &get_storage().await?; - let ostree = sysroot.get_ostree()?; - crate::deploy::rollback(sysroot).await?; + let storage = &get_storage().await?; + let ostree = storage.get_ostree()?; + crate::deploy::rollback(storage).await?; if opts.soft_reboot.is_some() { // Get status of rollback deployment to check soft-reboot capability @@ -1125,8 +1119,8 @@ async fn rollback(opts: &RollbackOpts) -> Result<()> { /// Implementation of the `bootc edit` CLI command. #[context("Editing spec")] async fn edit(opts: EditOpts) -> Result<()> { - let sysroot = &get_storage().await?; - let ostree = sysroot.get_ostree()?; + let storage = &get_storage().await?; + let ostree = storage.get_ostree()?; let repo = &ostree.repo(); let (booted_deployment, _deployments, host) = crate::status::get_status_require_booted(ostree)?; @@ -1153,7 +1147,7 @@ async fn edit(opts: EditOpts) -> Result<()> { // We only support two state transitions right now; switching the image, // or flipping the bootloader ordering. if host.spec.boot_order != new_host.spec.boot_order { - return crate::deploy::rollback(sysroot).await; + return crate::deploy::rollback(storage).await; } let fetched = crate::deploy::pull(repo, new_spec.image, None, opts.quiet, prog.clone()).await?; @@ -1161,10 +1155,10 @@ async fn edit(opts: EditOpts) -> Result<()> { // TODO gc old layers here let stateroot = booted_deployment.osname(); - let from = MergeState::from_stateroot(sysroot, &stateroot)?; - crate::deploy::stage(sysroot, from, &fetched, &new_spec, prog.clone()).await?; + let from = MergeState::from_stateroot(storage, &stateroot)?; + crate::deploy::stage(storage, from, &fetched, &new_spec, prog.clone()).await?; - sysroot.update_mtime()?; + storage.update_mtime()?; Ok(()) } @@ -1264,24 +1258,28 @@ async fn run_from_opt(opt: Opt) -> Result<()> { let root = &Dir::open_ambient_dir("/", cap_std::ambient_authority())?; match opt { Opt::Upgrade(opts) => { - if composefs_booted()?.is_some() { - upgrade_composefs(opts).await - } else { - upgrade(opts).await + let storage = &get_storage().await?; + match storage.kind()? { + BootedStorageKind::Ostree(booted_ostree) => { + upgrade(opts, storage, &booted_ostree).await + } + BootedStorageKind::Composefs(booted_cfs) => { + upgrade_composefs(opts, storage, &booted_cfs).await + } } } Opt::Switch(opts) => { - if composefs_booted()?.is_some() { - switch_composefs(opts).await - } else { - switch(opts).await + let storage = &get_storage().await?; + match storage.kind()? { + BootedStorageKind::Ostree(_) => switch(opts).await, + BootedStorageKind::Composefs(_) => switch_composefs(opts).await, } } Opt::Rollback(opts) => { - if composefs_booted()?.is_some() { - composefs_rollback().await? - } else { - rollback(&opts).await? + let storage = &get_storage().await?; + match storage.kind()? { + BootedStorageKind::Ostree(_) => rollback(&opts).await?, + BootedStorageKind::Composefs(_) => composefs_rollback().await?, } if opts.apply { @@ -1292,10 +1290,10 @@ async fn run_from_opt(opt: Opt) -> Result<()> { } Opt::Edit(opts) => edit(opts).await, Opt::UsrOverlay => { - if composefs_booted()?.is_some() { - composefs_usr_overlay() - } else { - usroverlay().await + let storage = &get_storage().await?; + match storage.kind()? { + BootedStorageKind::Ostree(_) => usroverlay().await, + BootedStorageKind::Composefs(_) => composefs_usr_overlay(), } } Opt::Container(opts) => match opts { @@ -1391,8 +1389,8 @@ async fn run_from_opt(opt: Opt) -> Result<()> { crate::image::push_entrypoint(source.as_deref(), target.as_deref()).await } ImageOpts::PullFromDefaultStorage { image } => { - let sysroot = get_storage().await?; - sysroot + let storage = get_storage().await?; + storage .get_ensure_imgstore()? .pull_from_host_storage(&image) .await @@ -1492,8 +1490,8 @@ async fn run_from_opt(opt: Opt) -> Result<()> { InternalsOpts::Cfs { args } => crate::cfsctl::run_from_iter(args.iter()).await, InternalsOpts::Reboot => crate::reboot::reboot(), InternalsOpts::Fsck => { - let sysroot = &get_storage().await?; - crate::fsck::fsck(&sysroot, std::io::stdout().lock()).await?; + let storage = &get_storage().await?; + crate::fsck::fsck(&storage, std::io::stdout().lock()).await?; Ok(()) } InternalsOpts::FixupEtcFstab => crate::deploy::fixup_etc_fstab(&root), @@ -1507,8 +1505,8 @@ async fn run_from_opt(opt: Opt) -> Result<()> { Ok(()) } InternalsOpts::Cleanup => { - let sysroot = get_storage().await?; - crate::deploy::cleanup(&sysroot).await + let storage = get_storage().await?; + crate::deploy::cleanup(&storage).await } InternalsOpts::Relabel { as_path, path } => { let root = &Dir::open_ambient_dir("/", cap_std::ambient_authority())?; diff --git a/crates/lib/src/image.rs b/crates/lib/src/image.rs index ad984ed6f..584f4c46d 100644 --- a/crates/lib/src/image.rs +++ b/crates/lib/src/image.rs @@ -72,7 +72,7 @@ async fn list_images(list_type: ImageListType) -> Result> { let rootfs = cap_std::fs::Dir::open_ambient_dir("/", cap_std::ambient_authority()) .context("Opening /")?; - let sysroot: Option = + let sysroot: Option = if ostree_ext::container_utils::running_in_container() { None } else { diff --git a/crates/lib/src/install.rs b/crates/lib/src/install.rs index 504a5f34d..815029774 100644 --- a/crates/lib/src/install.rs +++ b/crates/lib/src/install.rs @@ -811,7 +811,7 @@ async fn initialize_ostree_root(state: &State, root_setup: &RootSetup) -> Result sysroot.load(cancellable)?; let sysroot = SysrootLock::new_from_sysroot(&sysroot).await?; - let storage = Storage::new(sysroot, &temp_run)?; + let storage = Storage::new_ostree(sysroot, &temp_run)?; Ok((storage, has_ostree)) } diff --git a/crates/lib/src/status.rs b/crates/lib/src/status.rs index 8ed1e164e..8aa48413d 100644 --- a/crates/lib/src/status.rs +++ b/crates/lib/src/status.rs @@ -19,7 +19,7 @@ use ostree_ext::sysroot::SysrootLock; use ostree_ext::ostree; -use crate::bootc_composefs::status::{composefs_booted, composefs_deployment_status}; +use crate::bootc_composefs::status::composefs_deployment_status; use crate::cli::OutputFormat; use crate::spec::BootEntryComposefs; use crate::spec::ImageStatus; @@ -261,7 +261,11 @@ pub(crate) fn get_status_require_booted( sysroot: &SysrootLock, ) -> Result<(ostree::Deployment, Deployments, Host)> { let booted_deployment = sysroot.require_booted_deployment()?; - let (deployments, host) = get_status(sysroot, Some(&booted_deployment))?; + let booted_ostree = crate::store::BootedOstree { + sysroot, + deployment: booted_deployment.clone(), + }; + let (deployments, host) = get_status(&booted_ostree)?; Ok((booted_deployment, deployments, host)) } @@ -269,9 +273,10 @@ pub(crate) fn get_status_require_booted( /// a more native Rust structure. #[context("Computing status")] pub(crate) fn get_status( - sysroot: &SysrootLock, - booted_deployment: Option<&ostree::Deployment>, + booted_ostree: &crate::store::BootedOstree<'_>, ) -> Result<(Deployments, Host)> { + let sysroot = booted_ostree.sysroot; + let booted_deployment = Some(&booted_ostree.deployment); let stateroot = booted_deployment.as_ref().map(|d| d.osname()); let (mut related_deployments, other_deployments) = sysroot .deployments() @@ -365,13 +370,14 @@ pub(crate) fn get_status( async fn get_host() -> Result { let host = if ostree_booted()? { - let sysroot = super::cli::get_storage().await?; - let ostree = sysroot.get_ostree()?; - let booted_deployment = ostree.booted_deployment(); - let (_deployments, host) = get_status(&ostree, booted_deployment.as_ref())?; - host - } else if composefs_booted()?.is_some() { - composefs_deployment_status().await? + let storage = super::cli::get_storage().await?; + match storage.kind()? { + crate::store::BootedStorageKind::Ostree(booted_ostree) => { + let (_deployments, host) = get_status(&booted_ostree)?; + host + } + crate::store::BootedStorageKind::Composefs(_) => composefs_deployment_status().await?, + } } else { Default::default() }; diff --git a/crates/lib/src/store/mod.rs b/crates/lib/src/store/mod.rs index 8ce41f54a..5b9d2995f 100644 --- a/crates/lib/src/store/mod.rs +++ b/crates/lib/src/store/mod.rs @@ -17,6 +17,7 @@ //! This lives in `/composefs` in the physical root. use std::cell::OnceCell; +use std::ops::Deref; use std::sync::Arc; use anyhow::{Context, Result}; @@ -25,10 +26,11 @@ use cap_std_ext::cap_std::fs::{Dir, DirBuilder, DirBuilderExt as _}; use cap_std_ext::dirext::CapStdExtDirExt; use fn_error_context::context; -use ostree_ext::ostree; use ostree_ext::sysroot::SysrootLock; +use ostree_ext::{gio, ostree}; use rustix::fs::Mode; +use crate::bootc_composefs::status::{composefs_booted, ComposefsCmdline}; use crate::lsm; use crate::podstorage::CStorage; use crate::spec::ImageStatus; @@ -44,29 +46,152 @@ pub const SYSROOT: &str = "sysroot"; /// The toplevel composefs directory path pub const COMPOSEFS: &str = "composefs"; +#[allow(dead_code)] pub const COMPOSEFS_MODE: Mode = Mode::from_raw_mode(0o700); /// The path to the bootc root directory, relative to the physical /// system root pub(crate) const BOOTC_ROOT: &str = "ostree/bootc"; +/// Storage accessor for a booted system. +/// +/// This wraps [`Storage`] and can determine whether the system is booted +/// via ostree or composefs, providing a unified interface for both. +pub(crate) struct BootedStorage { + pub(crate) storage: Storage, +} + +impl Deref for BootedStorage { + type Target = Storage; + + fn deref(&self) -> &Self::Target { + &self.storage + } +} + +/// Represents an ostree-based boot environment +pub struct BootedOstree<'a> { + pub(crate) sysroot: &'a SysrootLock, + pub(crate) deployment: ostree::Deployment, +} + +impl<'a> BootedOstree<'a> { + /// Get the ostree repository + pub(crate) fn repo(&self) -> ostree::Repo { + self.sysroot.repo() + } + + /// Get the stateroot name + pub(crate) fn stateroot(&self) -> ostree::glib::GString { + self.deployment.osname() + } +} + +/// Represents a composefs-based boot environment +#[allow(dead_code)] +pub struct BootedComposefs { + pub repo: Arc, + pub cmdline: &'static ComposefsCmdline, +} + +/// Discriminated union representing the boot storage backend. +/// +/// A system can boot via either ostree or composefs; this enum +/// allows code to handle both cases while maintaining type safety. +pub(crate) enum BootedStorageKind<'a> { + Ostree(BootedOstree<'a>), + Composefs(BootedComposefs), +} + +impl BootedStorage { + /// Create a new booted storage accessor. + /// + /// This detects whether the system is booted via composefs or ostree + /// and initializes the appropriate storage backend. + pub(crate) async fn new() -> Result { + let physical_root = Dir::open_ambient_dir("/sysroot", cap_std::ambient_authority()) + .context("Opening /sysroot")?; + let run = + Dir::open_ambient_dir("/run", cap_std::ambient_authority()).context("Opening /run")?; + if let Some(cmdline) = composefs_booted()? { + let mut composefs = + ComposefsRepository::open_path(physical_root.open_dir(COMPOSEFS)?, ".")?; + if cmdline.insecure { + composefs.set_insecure(true); + } + let composefs = Arc::new(composefs); + + let storage = Storage { + physical_root, + run, + ostree: Default::default(), + composefs: OnceCell::from(composefs), + imgstore: Default::default(), + }; + Ok(Self { storage }) + } else { + let sysroot = ostree::Sysroot::new_default(); + sysroot.set_mount_namespace_in_use(); + let sysroot = ostree_ext::sysroot::SysrootLock::new_from_sysroot(&sysroot).await?; + sysroot.load(gio::Cancellable::NONE)?; + // Verify this is a booted system + let _ = sysroot.require_booted_deployment()?; + + let storage = Storage { + physical_root, + run, + ostree: OnceCell::from(sysroot), + composefs: Default::default(), + imgstore: Default::default(), + }; + Ok(Self { storage }) + } + } + + /// Determine the boot storage backend kind. + /// + /// Returns information about whether the system booted via ostree or composefs, + /// along with the relevant sysroot/deployment or repository/cmdline data. + pub(crate) fn kind(&self) -> Result> { + if let Some(cmdline) = composefs_booted()? { + // SAFETY: This must have been set above in new() + let repo = self.composefs.get().unwrap(); + Ok(BootedStorageKind::Composefs(BootedComposefs { + repo: Arc::clone(repo), + cmdline, + })) + } else { + // SAFETY: This must have been set above in new() + let sysroot = self.ostree.get().unwrap(); + let deployment = sysroot.require_booted_deployment()?; + Ok(BootedStorageKind::Ostree(BootedOstree { + sysroot, + deployment, + })) + } + } +} + /// A reference to a physical filesystem root, plus /// accessors for the different types of container storage. pub(crate) struct Storage { /// Directory holding the physical root pub physical_root: Dir, + /// Our runtime state + run: Dir, /// The OSTree storage - ostree: SysrootLock, + ostree: OnceCell, /// The composefs storage composefs: OnceCell>, /// The containers-image storage used foR LBIs imgstore: OnceCell, - - /// Our runtime state - run: Dir, } +/// Cached image status data used for optimization. +/// +/// This stores the current image status and any cached update information +/// to avoid redundant fetches during status operations. #[derive(Default)] pub(crate) struct CachedImageStatus { pub image: Option, @@ -74,7 +199,11 @@ pub(crate) struct CachedImageStatus { } impl Storage { - pub fn new(sysroot: SysrootLock, run: &Dir) -> Result { + /// Create a new storage accessor from an existing ostree sysroot. + /// + /// This is used for non-booted scenarios (e.g., `bootc install`) where + /// we're operating on a target filesystem rather than the running system. + pub fn new_ostree(sysroot: SysrootLock, run: &Dir) -> Result { let run = run.try_clone()?; // ostree has historically always relied on @@ -93,10 +222,13 @@ impl Storage { ostree_sysroot_dir }; + let ostree_cell = OnceCell::new(); + let _ = ostree_cell.set(sysroot); + Ok(Self { physical_root, - ostree: sysroot, run, + ostree: ostree_cell, composefs: Default::default(), imgstore: Default::default(), }) @@ -104,10 +236,15 @@ impl Storage { /// Access the underlying ostree repository pub(crate) fn get_ostree(&self) -> Result<&SysrootLock> { - Ok(&self.ostree) + self.ostree + .get() + .ok_or_else(|| anyhow::anyhow!("OSTree storage not initialized")) } - /// Access the underlying ostree repository + /// Get a cloned reference to the ostree sysroot. + /// + /// This is used when code needs an owned `ostree::Sysroot` rather than + /// a reference to the `SysrootLock`. pub(crate) fn get_ostree_cloned(&self) -> Result { let r = self.get_ostree()?; Ok((*r).clone()) @@ -118,9 +255,10 @@ impl Storage { if let Some(imgstore) = self.imgstore.get() { return Ok(imgstore); } - let sysroot_dir = crate::utils::sysroot_dir(&self.ostree)?; + let ostree = self.get_ostree()?; + let sysroot_dir = crate::utils::sysroot_dir(ostree)?; - let sepolicy = if self.ostree.booted_deployment().is_none() { + let sepolicy = if ostree.booted_deployment().is_none() { // fallback to policy from container root // this should only happen during cleanup of a broken install tracing::trace!("falling back to container root's selinux policy"); @@ -130,8 +268,8 @@ impl Storage { // load the sepolicy from the booted ostree deployment so the imgstorage can be // properly labeled with /var/lib/container/storage labels tracing::trace!("loading sepolicy from booted ostree deployment"); - let dep = self.ostree.booted_deployment().unwrap(); - let dep_fs = deployment_fd(&self.ostree, &dep)?; + let dep = ostree.booted_deployment().unwrap(); + let dep_fs = deployment_fd(ostree, &dep)?; lsm::new_sepolicy_at(&dep_fs)? }; @@ -141,6 +279,10 @@ impl Storage { Ok(self.imgstore.get_or_init(|| imgstore)) } + /// Access the composefs repository; will automatically initialize it if necessary. + /// + /// This lazily opens the composefs repository, creating the directory if needed + /// and bootstrapping verity settings from the ostree configuration. pub(crate) fn get_ensure_composefs(&self) -> Result> { if let Some(composefs) = self.composefs.get() { return Ok(Arc::clone(composefs)); @@ -150,13 +292,13 @@ impl Storage { db.mode(COMPOSEFS_MODE.as_raw_mode()); self.physical_root.ensure_dir_with(COMPOSEFS, &db)?; - let mut composefs = - ComposefsRepository::open_path(&self.physical_root.open_dir(COMPOSEFS)?, ".")?; - // Bootstrap verity off of the ostree state. In practice this means disabled by // default right now. - let ostree_repo = &self.ostree.repo(); + let ostree = self.get_ostree()?; + let ostree_repo = &ostree.repo(); let ostree_verity = ostree_ext::fsverity::is_verity_enabled(ostree_repo)?; + let mut composefs = + ComposefsRepository::open_path(self.physical_root.open_dir(COMPOSEFS)?, ".")?; if !ostree_verity.enabled { tracing::debug!("Setting insecure mode for composefs repo"); composefs.set_insecure(true); @@ -169,8 +311,8 @@ impl Storage { /// Update the mtime on the storage root directory #[context("Updating storage root mtime")] pub(crate) fn update_mtime(&self) -> Result<()> { - let sysroot_dir = - crate::utils::sysroot_dir(&self.ostree).context("Reopen sysroot directory")?; + let ostree = self.get_ostree()?; + let sysroot_dir = crate::utils::sysroot_dir(ostree).context("Reopen sysroot directory")?; sysroot_dir .update_timestamps(std::path::Path::new(BOOTC_ROOT)) From b6decd930ea5aa1dfcd050c5580096fc1672023d Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 30 Oct 2025 21:15:35 -0400 Subject: [PATCH 05/13] cli: Refactor switch/rollback/edit to use BootedOstree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Apply the same pattern used in upgrade() to switch, rollback, and edit commands. This eliminates redundant composefs_booted() calls from the main CLI code paths by consistently using storage.kind() for dispatch. Changes: - Split switch/rollback/edit into dispatcher + _ostree() helper functions - Update get_status_require_booted() to return BootedOstree instead of separate deployment/sysroot components - Update soft_reboot_rollback() to accept &BootedOstree - Update deploy::rollback() and install_reset() for new types - All command verbs now follow consistent pattern: storage.kind()? → match BootedStorageKind → call helper with &BootedOstree This change eliminates all composefs_booted() calls from crates/lib/src/cli.rs main code paths, passing boot context data down from BootedStorage instead. Assisted-by: Claude Code (Sonnet 4.5) Signed-off-by: Colin Walters --- crates/lib/src/cli.rs | 156 +++++++++++++++++++++++--------------- crates/lib/src/deploy.rs | 12 +-- crates/lib/src/install.rs | 7 +- crates/lib/src/status.rs | 6 +- 4 files changed, 108 insertions(+), 73 deletions(-) diff --git a/crates/lib/src/cli.rs b/crates/lib/src/cli.rs index 20d1bae98..691b7736a 100644 --- a/crates/lib/src/cli.rs +++ b/crates/lib/src/cli.rs @@ -827,15 +827,15 @@ fn soft_reboot_staged(sysroot: &SysrootLock) -> Result<()> { /// Perform a soft reboot for a rollback deployment #[context("Soft reboot rollback deployment")] -fn soft_reboot_rollback(sysroot: &SysrootLock) -> Result<()> { +fn soft_reboot_rollback(booted_ostree: &BootedOstree<'_>) -> Result<()> { println!("Rollback deployment is soft-reboot capable, preparing for soft-reboot..."); - let deployments_list = sysroot.deployments(); + let deployments_list = booted_ostree.sysroot.deployments(); let target_deployment = deployments_list .first() .ok_or_else(|| anyhow::anyhow!("No rollback deployment found!"))?; - prepare_soft_reboot(sysroot, target_deployment) + prepare_soft_reboot(booted_ostree.sysroot, target_deployment) } /// A few process changes that need to be made for writing. @@ -996,34 +996,19 @@ pub(crate) fn imgref_for_switch(opts: &SwitchOpts) -> Result { return Ok(target); } -/// Implementation of the `bootc switch` CLI command. -#[context("Switching")] -async fn switch(opts: SwitchOpts) -> Result<()> { +/// Implementation of the `bootc switch` CLI command for ostree backend. +#[context("Switching (ostree)")] +async fn switch_ostree( + opts: SwitchOpts, + storage: &Storage, + booted_ostree: &BootedOstree<'_>, +) -> Result<()> { let target = imgref_for_switch(&opts)?; - let prog: ProgressWriter = opts.progress.try_into()?; - - // If we're doing an in-place mutation, we shortcut most of the rest of the work here - if opts.mutate_in_place { - let deployid = { - // Clone to pass into helper thread - let target = target.clone(); - let root = cap_std::fs::Dir::open_ambient_dir("/", cap_std::ambient_authority())?; - tokio::task::spawn_blocking(move || { - crate::deploy::switch_origin_inplace(&root, &target) - }) - .await?? - }; - println!("Updated {deployid} to pull from {target}"); - return Ok(()); - } - let cancellable = gio::Cancellable::NONE; - let storage = &get_storage().await?; - let ostree = storage.get_ostree()?; - let repo = &ostree.repo(); - let (booted_deployment, _deployments, host) = crate::status::get_status_require_booted(ostree)?; + let repo = &booted_ostree.repo(); + let (_, host) = crate::status::get_status(booted_ostree)?; let new_spec = { let mut new_spec = host.spec.clone(); @@ -1061,7 +1046,7 @@ async fn switch(opts: SwitchOpts) -> Result<()> { if !opts.retain { // By default, we prune the previous ostree ref so it will go away after later upgrades - if let Some(booted_origin) = booted_deployment.origin() { + if let Some(booted_origin) = booted_ostree.deployment.origin() { if let Some(ostree_ref) = booted_origin.optional_string("origin", "refspec")? { let (remote, ostree_ref) = ostree::parse_refspec(&ostree_ref).context("Failed to parse ostree ref")?; @@ -1070,7 +1055,7 @@ async fn switch(opts: SwitchOpts) -> Result<()> { } } - let stateroot = booted_deployment.osname(); + let stateroot = booted_ostree.stateroot(); let from = MergeState::from_stateroot(storage, &stateroot)?; crate::deploy::stage(storage, from, &fetched, &new_spec, prog.clone()).await?; @@ -1079,12 +1064,8 @@ async fn switch(opts: SwitchOpts) -> Result<()> { if opts.soft_reboot.is_some() { // At this point we have staged the deployment and the host definition has changed. // We need the updated host status before we check if we can prepare the soft-reboot. - let booted_ostree = BootedOstree { - sysroot: ostree, - deployment: booted_deployment.clone(), - }; - let updated_host = crate::status::get_status(&booted_ostree)?.1; - handle_staged_soft_reboot(&booted_ostree, opts.soft_reboot, &updated_host)?; + let updated_host = crate::status::get_status(booted_ostree)?.1; + handle_staged_soft_reboot(booted_ostree, opts.soft_reboot, &updated_host)?; } if opts.apply { @@ -1094,36 +1075,85 @@ async fn switch(opts: SwitchOpts) -> Result<()> { Ok(()) } -/// Implementation of the `bootc rollback` CLI command. -#[context("Rollback")] -async fn rollback(opts: &RollbackOpts) -> Result<()> { +/// Implementation of the `bootc switch` CLI command. +#[context("Switching")] +async fn switch(opts: SwitchOpts) -> Result<()> { let storage = &get_storage().await?; - let ostree = storage.get_ostree()?; + match storage.kind()? { + BootedStorageKind::Ostree(booted_ostree) => { + // If we're doing an in-place mutation, we shortcut most of the rest of the work here + if opts.mutate_in_place { + let target = imgref_for_switch(&opts)?; + let deployid = { + // Clone to pass into helper thread + let target = target.clone(); + let root = + cap_std::fs::Dir::open_ambient_dir("/", cap_std::ambient_authority())?; + tokio::task::spawn_blocking(move || { + crate::deploy::switch_origin_inplace(&root, &target) + }) + .await?? + }; + println!("Updated {deployid} to pull from {target}"); + return Ok(()); + } + switch_ostree(opts, storage, &booted_ostree).await + } + BootedStorageKind::Composefs(_) => { + if opts.mutate_in_place { + anyhow::bail!("--mutate-in-place is not yet supported for composefs backend"); + } + switch_composefs(opts).await + } + } +} + +/// Implementation of the `bootc rollback` CLI command for ostree backend. +#[context("Rollback (ostree)")] +async fn rollback_ostree( + opts: &RollbackOpts, + storage: &Storage, + booted_ostree: &BootedOstree<'_>, +) -> Result<()> { crate::deploy::rollback(storage).await?; if opts.soft_reboot.is_some() { // Get status of rollback deployment to check soft-reboot capability - let host = crate::status::get_status_require_booted(ostree)?.2; + let host = crate::status::get_status(booted_ostree)?.1; handle_soft_reboot( opts.soft_reboot, host.status.rollback.as_ref(), "rollback", - || soft_reboot_rollback(ostree), + || soft_reboot_rollback(booted_ostree), )?; } Ok(()) } -/// Implementation of the `bootc edit` CLI command. -#[context("Editing spec")] -async fn edit(opts: EditOpts) -> Result<()> { +/// Implementation of the `bootc rollback` CLI command. +#[context("Rollback")] +async fn rollback(opts: &RollbackOpts) -> Result<()> { let storage = &get_storage().await?; - let ostree = storage.get_ostree()?; - let repo = &ostree.repo(); + match storage.kind()? { + BootedStorageKind::Ostree(booted_ostree) => { + rollback_ostree(opts, storage, &booted_ostree).await + } + BootedStorageKind::Composefs(_) => composefs_rollback().await, + } +} + +/// Implementation of the `bootc edit` CLI command for ostree backend. +#[context("Editing spec (ostree)")] +async fn edit_ostree( + opts: EditOpts, + storage: &Storage, + booted_ostree: &BootedOstree<'_>, +) -> Result<()> { + let repo = &booted_ostree.repo(); + let (_, host) = crate::status::get_status(booted_ostree)?; - let (booted_deployment, _deployments, host) = crate::status::get_status_require_booted(ostree)?; let new_host: Host = if let Some(filename) = opts.filename { let mut r = std::io::BufReader::new(std::fs::File::open(filename)?); serde_yaml::from_reader(&mut r)? @@ -1154,7 +1184,7 @@ async fn edit(opts: EditOpts) -> Result<()> { // TODO gc old layers here - let stateroot = booted_deployment.osname(); + let stateroot = booted_ostree.stateroot(); let from = MergeState::from_stateroot(storage, &stateroot)?; crate::deploy::stage(storage, from, &fetched, &new_spec, prog.clone()).await?; @@ -1163,6 +1193,20 @@ async fn edit(opts: EditOpts) -> Result<()> { Ok(()) } +/// Implementation of the `bootc edit` CLI command. +#[context("Editing spec")] +async fn edit(opts: EditOpts) -> Result<()> { + let storage = &get_storage().await?; + match storage.kind()? { + BootedStorageKind::Ostree(booted_ostree) => { + edit_ostree(opts, storage, &booted_ostree).await + } + BootedStorageKind::Composefs(_) => { + anyhow::bail!("Edit is not yet supported for composefs backend") + } + } +} + /// Implementation of `bootc usroverlay` async fn usroverlay() -> Result<()> { // This is just a pass-through today. At some point we may make this a libostree API @@ -1268,24 +1312,12 @@ async fn run_from_opt(opt: Opt) -> Result<()> { } } } - Opt::Switch(opts) => { - let storage = &get_storage().await?; - match storage.kind()? { - BootedStorageKind::Ostree(_) => switch(opts).await, - BootedStorageKind::Composefs(_) => switch_composefs(opts).await, - } - } + Opt::Switch(opts) => switch(opts).await, Opt::Rollback(opts) => { - let storage = &get_storage().await?; - match storage.kind()? { - BootedStorageKind::Ostree(_) => rollback(&opts).await?, - BootedStorageKind::Composefs(_) => composefs_rollback().await?, - } - + rollback(&opts).await?; if opts.apply { crate::reboot::reboot()?; } - Ok(()) } Opt::Edit(opts) => edit(opts).await, diff --git a/crates/lib/src/deploy.rs b/crates/lib/src/deploy.rs index d8cd9a999..abc22454d 100644 --- a/crates/lib/src/deploy.rs +++ b/crates/lib/src/deploy.rs @@ -835,7 +835,7 @@ fn write_reboot_required(image: &str) -> Result<()> { pub(crate) async fn rollback(sysroot: &Storage) -> Result<()> { const ROLLBACK_JOURNAL_ID: &str = "26f3b1eb24464d12aa5e7b544a6b5468"; let ostree = sysroot.get_ostree()?; - let (booted_deployment, deployments, host) = crate::status::get_status_require_booted(ostree)?; + let (booted_ostree, deployments, host) = crate::status::get_status_require_booted(ostree)?; let new_spec = { let mut new_spec = host.spec.clone(); @@ -843,7 +843,7 @@ pub(crate) async fn rollback(sysroot: &Storage) -> Result<()> { new_spec }; - let repo = &ostree.repo(); + let repo = &booted_ostree.repo(); // Just to be sure host.spec.verify_transition(&new_spec)?; @@ -882,16 +882,18 @@ pub(crate) async fn rollback(sysroot: &Storage) -> Result<()> { // SAFETY: If there's a rollback status, then there's a deployment let rollback_deployment = deployments.rollback.expect("rollback deployment"); let new_deployments = if reverting { - [booted_deployment, rollback_deployment] + [booted_ostree.deployment, rollback_deployment] } else { - [rollback_deployment, booted_deployment] + [rollback_deployment, booted_ostree.deployment] }; let new_deployments = new_deployments .into_iter() .chain(deployments.other) .collect::>(); tracing::debug!("Writing new deployments: {new_deployments:?}"); - ostree.write_deployments(&new_deployments, gio::Cancellable::NONE)?; + booted_ostree + .sysroot + .write_deployments(&new_deployments, gio::Cancellable::NONE)?; if reverting { println!("Next boot: current deployment"); } else { diff --git a/crates/lib/src/install.rs b/crates/lib/src/install.rs index 815029774..65a223b81 100644 --- a/crates/lib/src/install.rs +++ b/crates/lib/src/install.rs @@ -2265,7 +2265,7 @@ pub(crate) async fn install_reset(opts: InstallResetOpts) -> Result<()> { let sysroot = &crate::cli::get_storage().await?; let ostree = sysroot.get_ostree()?; let repo = &ostree.repo(); - let (booted_deployment, _deployments, host) = crate::status::get_status_require_booted(ostree)?; + let (booted_ostree, _deployments, host) = crate::status::get_status_require_booted(ostree)?; let stateroots = list_stateroots(ostree)?; let target_stateroot = if let Some(s) = opts.stateroot { @@ -2276,7 +2276,7 @@ pub(crate) async fn install_reset(opts: InstallResetOpts) -> Result<()> { r.name }; - let booted_stateroot = booted_deployment.osname(); + let booted_stateroot = booted_ostree.stateroot(); assert!(booted_stateroot.as_str() != target_stateroot); let (fetched, spec) = if let Some(target) = opts.target_opts.imageref()? { let mut new_spec = host.spec; @@ -2307,7 +2307,8 @@ pub(crate) async fn install_reset(opts: InstallResetOpts) -> Result<()> { let root_kargs = if opts.no_root_kargs { Vec::new() } else { - let bootcfg = booted_deployment + let bootcfg = booted_ostree + .deployment .bootconfig() .ok_or_else(|| anyhow!("Missing bootcfg for booted deployment"))?; if let Some(options) = bootcfg.get("options") { diff --git a/crates/lib/src/status.rs b/crates/lib/src/status.rs index 8aa48413d..69067d5e7 100644 --- a/crates/lib/src/status.rs +++ b/crates/lib/src/status.rs @@ -259,14 +259,14 @@ impl BootEntry { /// A variant of [`get_status`] that requires a booted deployment. pub(crate) fn get_status_require_booted( sysroot: &SysrootLock, -) -> Result<(ostree::Deployment, Deployments, Host)> { +) -> Result<(crate::store::BootedOstree<'_>, Deployments, Host)> { let booted_deployment = sysroot.require_booted_deployment()?; let booted_ostree = crate::store::BootedOstree { sysroot, - deployment: booted_deployment.clone(), + deployment: booted_deployment, }; let (deployments, host) = get_status(&booted_ostree)?; - Ok((booted_deployment, deployments, host)) + Ok((booted_ostree, deployments, host)) } /// Gather the ostree deployment objects, but also extract metadata from them into From 3b83f29b686d5d51989a8a672c16c3f76f8473c9 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 30 Oct 2025 21:23:54 -0400 Subject: [PATCH 06/13] composefs: Pass BootedComposefs data down instead of scraping global state Update composefs command functions to accept and use Storage and BootedComposefs parameters instead of calling composefs_deployment_status() which scrapes global state. Changes: - Add get_composefs_status(storage, booted_cfs) helper in status.rs - Update upgrade_composefs() to use passed composefs.repo and get_composefs_status() - Update switch_composefs() to accept and use storage/booted_cfs parameters - Update composefs_rollback() to use storage.physical_root for opening directories - Update delete_composefs_deployment() to accept and pass through parameters - Update CLI dispatchers to pass storage and booted_cfs to all composefs functions This eliminates composefs_deployment_status() calls from command functions, completing the pattern established with BootedOstree for ostree commands. Assisted-by: Claude Code (Sonnet 4.5) Signed-off-by: Colin Walters --- crates/lib/src/bootc_composefs/delete.rs | 13 +++++++++---- crates/lib/src/bootc_composefs/rollback.rs | 15 ++++++++++----- crates/lib/src/bootc_composefs/status.rs | 10 ++++++++++ crates/lib/src/bootc_composefs/switch.rs | 12 +++++++++--- crates/lib/src/bootc_composefs/update.rs | 16 ++++++---------- crates/lib/src/cli.rs | 20 ++++++++++++++++---- 6 files changed, 60 insertions(+), 26 deletions(-) diff --git a/crates/lib/src/bootc_composefs/delete.rs b/crates/lib/src/bootc_composefs/delete.rs index 54258de42..692ab1432 100644 --- a/crates/lib/src/bootc_composefs/delete.rs +++ b/crates/lib/src/bootc_composefs/delete.rs @@ -17,7 +17,7 @@ use crate::{ gc::composefs_gc, repo::open_composefs_repo, rollback::{composefs_rollback, rename_exchange_user_cfg}, - status::{composefs_deployment_status, get_sorted_grub_uki_boot_entries}, + status::{get_composefs_status, get_sorted_grub_uki_boot_entries}, }, composefs_consts::{ COMPOSEFS_STAGED_DEPLOYMENT_FNAME, COMPOSEFS_TRANSIENT_STATE_DIR, STATE_DIR_RELATIVE, @@ -26,6 +26,7 @@ use crate::{ parsers::bls_config::{parse_bls_config, BLSConfigType}, spec::{BootEntry, Bootloader, DeploymentEntry}, status::Slot, + store::{BootedComposefs, Storage}, }; #[fn_error_context::context("Deleting Type1 Entry {}", depl.deployment.verity)] @@ -316,8 +317,12 @@ pub(crate) fn delete_staged(staged: &Option) -> Result<()> { } #[fn_error_context::context("Deleting composefs deployment {}", deployment_id)] -pub(crate) async fn delete_composefs_deployment(deployment_id: &str) -> Result<()> { - let host = composefs_deployment_status().await?; +pub(crate) async fn delete_composefs_deployment( + deployment_id: &str, + storage: &Storage, + booted_cfs: &BootedComposefs, +) -> Result<()> { + let host = get_composefs_status(storage, booted_cfs).await?; let booted = host.require_composefs_booted()?; @@ -344,7 +349,7 @@ pub(crate) async fn delete_composefs_deployment(deployment_id: &str) -> Result<( // Unqueue rollback. This makes it easier to delete boot entries later on if matches!(depl_to_del.ty, Some(Slot::Rollback)) && host.status.rollback_queued { - composefs_rollback().await?; + composefs_rollback(storage, booted_cfs).await?; } let kind = if depl_to_del.pinned { diff --git a/crates/lib/src/bootc_composefs/rollback.rs b/crates/lib/src/bootc_composefs/rollback.rs index 0860f0489..2b9d05f43 100644 --- a/crates/lib/src/bootc_composefs/rollback.rs +++ b/crates/lib/src/bootc_composefs/rollback.rs @@ -1,7 +1,6 @@ use std::io::Write; use anyhow::{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; @@ -10,9 +9,10 @@ use rustix::fs::{fsync, renameat_with, AtFlags, RenameFlags}; use crate::bootc_composefs::boot::{ get_esp_partition, get_sysroot_parent_dev, mount_esp, type1_entry_conf_file_name, BootType, }; -use crate::bootc_composefs::status::{composefs_deployment_status, get_sorted_type1_boot_entries}; +use crate::bootc_composefs::status::{get_composefs_status, get_sorted_type1_boot_entries}; use crate::composefs_consts::TYPE1_ENT_PATH_STAGED; use crate::spec::Bootloader; +use crate::store::{BootedComposefs, Storage}; use crate::{ bootc_composefs::{boot::get_efi_uuid_source, status::get_sorted_grub_uki_boot_entries}, composefs_consts::{ @@ -164,8 +164,11 @@ fn rollback_composefs_entries(boot_dir: &Dir, bootloader: Bootloader) -> Result< } #[context("Rolling back composefs")] -pub(crate) async fn composefs_rollback() -> Result<()> { - let host = composefs_deployment_status().await?; +pub(crate) async fn composefs_rollback( + storage: &Storage, + booted_cfs: &BootedComposefs, +) -> Result<()> { + let host = get_composefs_status(storage, booted_cfs).await?; let new_spec = { let mut new_spec = host.spec.clone(); @@ -195,7 +198,9 @@ pub(crate) async fn composefs_rollback() -> Result<()> { match &rollback_entry.bootloader { Bootloader::Grub => { - let boot_dir = Dir::open_ambient_dir("/sysroot/boot", ambient_authority()) + let boot_dir = storage + .physical_root + .open_dir("boot") .context("Opening boot dir")?; match rollback_entry.boot_type { diff --git a/crates/lib/src/bootc_composefs/status.rs b/crates/lib/src/bootc_composefs/status.rs index 9da338d02..46a3ccd79 100644 --- a/crates/lib/src/bootc_composefs/status.rs +++ b/crates/lib/src/bootc_composefs/status.rs @@ -257,6 +257,16 @@ pub(crate) async fn composefs_deployment_status() -> Result { composefs_deployment_status_from(&sysroot, composefs_state).await } +/// Get composefs status using provided storage and booted composefs data +/// instead of scraping global state. +#[context("Getting composefs deployment status")] +pub(crate) async fn get_composefs_status( + storage: &crate::store::Storage, + booted_cfs: &crate::store::BootedComposefs, +) -> Result { + composefs_deployment_status_from(&storage.physical_root, booted_cfs.cmdline).await +} + #[context("Getting composefs deployment status")] pub(crate) async fn composefs_deployment_status_from( sysroot: &Dir, diff --git a/crates/lib/src/bootc_composefs/switch.rs b/crates/lib/src/bootc_composefs/switch.rs index 485fc59db..b45c66ebb 100644 --- a/crates/lib/src/bootc_composefs/switch.rs +++ b/crates/lib/src/bootc_composefs/switch.rs @@ -8,17 +8,22 @@ use crate::{ repo::pull_composefs_repo, service::start_finalize_stated_svc, state::write_composefs_state, - status::composefs_deployment_status, + status::get_composefs_status, }, cli::{imgref_for_switch, SwitchOpts}, + store::{BootedComposefs, Storage}, }; #[context("Composefs Switching")] -pub(crate) async fn switch_composefs(opts: SwitchOpts) -> Result<()> { +pub(crate) async fn switch_composefs( + opts: SwitchOpts, + storage: &Storage, + booted_cfs: &BootedComposefs, +) -> Result<()> { let target = imgref_for_switch(&opts)?; // TODO: Handle in-place - let host = composefs_deployment_status() + let host = get_composefs_status(storage, booted_cfs) .await .context("Getting composefs deployment status")?; @@ -63,6 +68,7 @@ pub(crate) async fn switch_composefs(opts: SwitchOpts) -> Result<()> { } }; + // TODO: Remove this hardcoded path when write_composefs_state accepts a Dir write_composefs_state( &Utf8PathBuf::from("/sysroot"), id, diff --git a/crates/lib/src/bootc_composefs/update.rs b/crates/lib/src/bootc_composefs/update.rs index 8d625bbf7..eaa27aa85 100644 --- a/crates/lib/src/bootc_composefs/update.rs +++ b/crates/lib/src/bootc_composefs/update.rs @@ -7,18 +7,16 @@ use ostree_ext::oci_spec::image::{ImageConfiguration, ImageManifest}; use crate::{ bootc_composefs::{ boot::{setup_composefs_bls_boot, setup_composefs_uki_boot, BootSetupType, BootType}, - repo::{get_imgref, open_composefs_repo, pull_composefs_repo}, + repo::{get_imgref, pull_composefs_repo}, service::start_finalize_stated_svc, state::write_composefs_state, - status::{composefs_deployment_status, get_container_manifest_and_config}, + status::{get_composefs_status, get_container_manifest_and_config}, }, cli::UpgradeOpts, spec::ImageReference, store::{BootedComposefs, ComposefsRepository, Storage}, }; -use cap_std_ext::cap_std::{ambient_authority, fs::Dir}; - #[context("Getting SHA256 Digest for {id}")] pub fn str_to_sha256digest(id: &str) -> Result { let id = id.strip_prefix("sha256:").unwrap_or(id); @@ -63,10 +61,10 @@ async fn is_image_pulled( #[context("Upgrading composefs")] pub(crate) async fn upgrade_composefs( opts: UpgradeOpts, - _storage: &Storage, - _composefs: &BootedComposefs, + storage: &Storage, + composefs: &BootedComposefs, ) -> Result<()> { - let host = composefs_deployment_status() + let host = get_composefs_status(storage, composefs) .await .context("Getting composefs deployment status")?; @@ -76,9 +74,7 @@ pub(crate) async fn upgrade_composefs( .as_ref() .ok_or_else(|| anyhow::anyhow!("No image source specified"))?; - let sysroot = - Dir::open_ambient_dir("/sysroot", ambient_authority()).context("Opening sysroot")?; - let repo = open_composefs_repo(&sysroot)?; + let repo = &*composefs.repo; let (img_pulled, mut manifest, mut config) = is_image_pulled(&repo, imgref).await?; let booted_img_digest = manifest.config().digest().digest(); diff --git a/crates/lib/src/cli.rs b/crates/lib/src/cli.rs index 691b7736a..53040c9f8 100644 --- a/crates/lib/src/cli.rs +++ b/crates/lib/src/cli.rs @@ -1099,11 +1099,11 @@ async fn switch(opts: SwitchOpts) -> Result<()> { } switch_ostree(opts, storage, &booted_ostree).await } - BootedStorageKind::Composefs(_) => { + BootedStorageKind::Composefs(booted_cfs) => { if opts.mutate_in_place { anyhow::bail!("--mutate-in-place is not yet supported for composefs backend"); } - switch_composefs(opts).await + switch_composefs(opts, storage, &booted_cfs).await } } } @@ -1140,7 +1140,9 @@ async fn rollback(opts: &RollbackOpts) -> Result<()> { BootedStorageKind::Ostree(booted_ostree) => { rollback_ostree(opts, storage, &booted_ostree).await } - BootedStorageKind::Composefs(_) => composefs_rollback().await, + BootedStorageKind::Composefs(booted_cfs) => { + composefs_rollback(storage, &booted_cfs).await + } } } @@ -1624,7 +1626,17 @@ async fn run_from_opt(opt: Opt) -> Result<()> { Opt::ConfigDiff => get_etc_diff().await, - Opt::DeleteDeployment { depl_id } => delete_composefs_deployment(&depl_id).await, + Opt::DeleteDeployment { depl_id } => { + let storage = &get_storage().await?; + match storage.kind()? { + BootedStorageKind::Ostree(_) => { + anyhow::bail!("DeleteDeployment is only supported for composefs backend") + } + BootedStorageKind::Composefs(booted_cfs) => { + delete_composefs_deployment(&depl_id, storage, &booted_cfs).await + } + } + } } } From 71233bceb7489b34b255eefbdcedcfae6b62abee Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 30 Oct 2025 21:29:48 -0400 Subject: [PATCH 07/13] composefs: Update gc and finalize functions to use passed Storage/BootedComposefs Update composefs_gc(), composefs_backend_finalize(), and get_etc_diff() to accept Storage and BootedComposefs parameters instead of calling composefs_deployment_status() and opening hardcoded paths. Changes: - composefs_gc() now accepts storage and booted_cfs parameters - composefs_backend_finalize() uses storage.physical_root instead of hardcoded "/sysroot" - get_etc_diff() uses storage.physical_root instead of hardcoded "/sysroot" - Update CLI dispatchers to use BootedStorageKind pattern for finalize and config-diff - Remove unused imports from finalize.rs (open_dir, CWD) This eliminates remaining composefs_deployment_status() calls from gc.rs and finalize.rs, and removes several hardcoded "/sysroot" paths. Assisted-by: Claude Code (Sonnet 4.5) Signed-off-by: Colin Walters --- crates/lib/src/bootc_composefs/delete.rs | 2 +- crates/lib/src/bootc_composefs/finalize.rs | 35 ++++++++++++---------- crates/lib/src/bootc_composefs/gc.rs | 16 +++++----- crates/lib/src/bootc_composefs/status.rs | 11 ------- crates/lib/src/cli.rs | 28 +++++++++++++---- crates/lib/src/status.rs | 5 ++-- 6 files changed, 55 insertions(+), 42 deletions(-) diff --git a/crates/lib/src/bootc_composefs/delete.rs b/crates/lib/src/bootc_composefs/delete.rs index 692ab1432..5d1afb8ab 100644 --- a/crates/lib/src/bootc_composefs/delete.rs +++ b/crates/lib/src/bootc_composefs/delete.rs @@ -364,7 +364,7 @@ pub(crate) async fn delete_composefs_deployment( delete_depl_boot_entries(&depl_to_del, deleting_staged)?; - composefs_gc().await?; + composefs_gc(storage, booted_cfs).await?; Ok(()) } diff --git a/crates/lib/src/bootc_composefs/finalize.rs b/crates/lib/src/bootc_composefs/finalize.rs index 7a0ac6399..48ca9eff8 100644 --- a/crates/lib/src/bootc_composefs/finalize.rs +++ b/crates/lib/src/bootc_composefs/finalize.rs @@ -4,28 +4,28 @@ use crate::bootc_composefs::boot::{ get_esp_partition, get_sysroot_parent_dev, mount_esp, BootType, }; use crate::bootc_composefs::rollback::{rename_exchange_bls_entries, rename_exchange_user_cfg}; +use crate::bootc_composefs::status::get_composefs_status; +use crate::composefs_consts::STATE_DIR_ABS; use crate::spec::Bootloader; -use crate::{ - bootc_composefs::status::composefs_deployment_status, composefs_consts::STATE_DIR_ABS, -}; +use crate::store::{BootedComposefs, Storage}; use anyhow::{Context, Result}; -use bootc_initramfs_setup::{mount_composefs_image, open_dir}; +use bootc_initramfs_setup::mount_composefs_image; use bootc_mount::tempmount::TempMount; use cap_std_ext::cap_std::{ambient_authority, fs::Dir}; use cap_std_ext::dirext::CapStdExtDirExt; use etc_merge::{compute_diff, merge, print_diff, traverse_etc}; -use rustix::fs::{fsync, renameat, CWD}; +use rustix::fs::{fsync, renameat}; use rustix::path::Arg; use fn_error_context::context; -pub(crate) async fn get_etc_diff() -> Result<()> { - let host = composefs_deployment_status().await?; +pub(crate) async fn get_etc_diff(storage: &Storage, booted_cfs: &BootedComposefs) -> Result<()> { + let host = get_composefs_status(storage, booted_cfs).await?; let booted_composefs = host.require_composefs_booted()?; // Mount the booted EROFS image to get pristine etc - let sysroot = open_dir(CWD, "/sysroot").context("Opening /sysroot")?; - let composefs_fd = mount_composefs_image(&sysroot, &booted_composefs.verity, false)?; + let sysroot_fd = storage.physical_root.reopen_as_ownedfd()?; + let composefs_fd = mount_composefs_image(&sysroot_fd, &booted_composefs.verity, false)?; let erofs_tmp_mnt = TempMount::mount_fd(&composefs_fd)?; @@ -41,8 +41,11 @@ pub(crate) async fn get_etc_diff() -> Result<()> { Ok(()) } -pub(crate) async fn composefs_backend_finalize() -> Result<()> { - let host = composefs_deployment_status().await?; +pub(crate) async fn composefs_backend_finalize( + storage: &Storage, + booted_cfs: &BootedComposefs, +) -> Result<()> { + let host = get_composefs_status(storage, booted_cfs).await?; let booted_composefs = host.require_composefs_booted()?; @@ -56,8 +59,8 @@ pub(crate) async fn composefs_backend_finalize() -> Result<()> { ))?; // Mount the booted EROFS image to get pristine etc - let sysroot = open_dir(CWD, "/sysroot")?; - let composefs_fd = mount_composefs_image(&sysroot, &booted_composefs.verity, false)?; + let sysroot_fd = storage.physical_root.reopen_as_ownedfd()?; + let composefs_fd = mount_composefs_image(&sysroot_fd, &booted_composefs.verity, false)?; let erofs_tmp_mnt = TempMount::mount_fd(&composefs_fd)?; @@ -88,8 +91,10 @@ pub(crate) async fn composefs_backend_finalize() -> Result<()> { let (esp_part, ..) = get_esp_partition(&sysroot_parent)?; let esp_mount = mount_esp(&esp_part)?; - let boot_dir = Dir::open_ambient_dir("/sysroot/boot", ambient_authority()) - .context("Opening sysroot/boot")?; + let boot_dir = storage + .physical_root + .open_dir("boot") + .context("Opening boot")?; // NOTE: Assuming here we won't have two bootloaders at the same time match booted_composefs.bootloader { diff --git a/crates/lib/src/bootc_composefs/gc.rs b/crates/lib/src/bootc_composefs/gc.rs index a6e0c31a4..2f04ee48c 100644 --- a/crates/lib/src/bootc_composefs/gc.rs +++ b/crates/lib/src/bootc_composefs/gc.rs @@ -16,12 +16,13 @@ use crate::{ boot::{get_esp_partition, get_sysroot_parent_dev, mount_esp}, delete::{delete_image, delete_staged, delete_state_dir, get_image_objects}, status::{ - composefs_deployment_status, get_bootloader, get_sorted_grub_uki_boot_entries, + get_bootloader, get_composefs_status, get_sorted_grub_uki_boot_entries, get_sorted_type1_boot_entries, }, }, composefs_consts::{STATE_DIR_RELATIVE, USER_CFG}, spec::Bootloader, + store::{BootedComposefs, Storage}, }; #[fn_error_context::context("Listing EROFS images")] @@ -172,12 +173,11 @@ pub(crate) fn gc_objects(sysroot: &Dir) -> Result<()> { /// Similarly if EROFS image B1 doesn't exist, but state dir does, then delete the state dir and /// perform GC #[fn_error_context::context("Running composefs garbage collection")] -pub(crate) async fn composefs_gc() -> Result<()> { - let host = composefs_deployment_status().await?; - let booted_cfs = host.require_composefs_booted()?; +pub(crate) async fn composefs_gc(storage: &Storage, booted_cfs: &BootedComposefs) -> Result<()> { + let host = get_composefs_status(storage, booted_cfs).await?; + let booted_cfs_status = host.require_composefs_booted()?; - let sysroot = - Dir::open_ambient_dir("/sysroot", ambient_authority()).context("Opening sysroot")?; + let sysroot = &storage.physical_root; let bootloader_entries = list_bootloader_entries()?; let images = list_erofs_images(&sysroot)?; @@ -190,10 +190,10 @@ pub(crate) async fn composefs_gc() -> Result<()> { let staged = &host.status.staged; - if img_bootloader_diff.contains(&&booted_cfs.verity) { + if img_bootloader_diff.contains(&&booted_cfs_status.verity) { anyhow::bail!( "Inconsistent state. Booted entry '{}' found for cleanup", - booted_cfs.verity + booted_cfs_status.verity ) } diff --git a/crates/lib/src/bootc_composefs/status.rs b/crates/lib/src/bootc_composefs/status.rs index 46a3ccd79..3204107d9 100644 --- a/crates/lib/src/bootc_composefs/status.rs +++ b/crates/lib/src/bootc_composefs/status.rs @@ -19,7 +19,6 @@ use crate::{ use std::str::FromStr; use bootc_utils::try_deserialize_timestamp; -use cap_std_ext::cap_std::ambient_authority; use cap_std_ext::cap_std::fs::Dir; use ostree_container::OstreeImageReference; use ostree_ext::container::deploy::ORIGIN_CONTAINER; @@ -247,16 +246,6 @@ async fn boot_entry_from_composefs_deployment( Ok(e) } -pub(crate) async fn composefs_deployment_status() -> Result { - let composefs_state = composefs_booted()? - .ok_or_else(|| anyhow::anyhow!("Failed to find composefs parameter in kernel cmdline"))?; - - let sysroot = - Dir::open_ambient_dir("/sysroot", ambient_authority()).context("Opening sysroot")?; - - composefs_deployment_status_from(&sysroot, composefs_state).await -} - /// Get composefs status using provided storage and booted composefs data /// instead of scraping global state. #[context("Getting composefs deployment status")] diff --git a/crates/lib/src/cli.rs b/crates/lib/src/cli.rs index 53040c9f8..e33ed1748 100644 --- a/crates/lib/src/cli.rs +++ b/crates/lib/src/cli.rs @@ -1140,9 +1140,7 @@ async fn rollback(opts: &RollbackOpts) -> Result<()> { BootedStorageKind::Ostree(booted_ostree) => { rollback_ostree(opts, storage, &booted_ostree).await } - BootedStorageKind::Composefs(booted_cfs) => { - composefs_rollback(storage, &booted_cfs).await - } + BootedStorageKind::Composefs(booted_cfs) => composefs_rollback(storage, &booted_cfs).await, } } @@ -1622,9 +1620,29 @@ async fn run_from_opt(opt: Opt) -> Result<()> { } }, - Opt::ComposefsFinalizeStaged => composefs_backend_finalize().await, + Opt::ComposefsFinalizeStaged => { + let storage = &get_storage().await?; + match storage.kind()? { + BootedStorageKind::Ostree(_) => { + anyhow::bail!("ComposefsFinalizeStaged is only supported for composefs backend") + } + BootedStorageKind::Composefs(booted_cfs) => { + composefs_backend_finalize(storage, &booted_cfs).await + } + } + } - Opt::ConfigDiff => get_etc_diff().await, + Opt::ConfigDiff => { + let storage = &get_storage().await?; + match storage.kind()? { + BootedStorageKind::Ostree(_) => { + anyhow::bail!("ConfigDiff is only supported for composefs backend") + } + BootedStorageKind::Composefs(booted_cfs) => { + get_etc_diff(storage, &booted_cfs).await + } + } + } Opt::DeleteDeployment { depl_id } => { let storage = &get_storage().await?; diff --git a/crates/lib/src/status.rs b/crates/lib/src/status.rs index 69067d5e7..220602b5a 100644 --- a/crates/lib/src/status.rs +++ b/crates/lib/src/status.rs @@ -19,7 +19,6 @@ use ostree_ext::sysroot::SysrootLock; use ostree_ext::ostree; -use crate::bootc_composefs::status::composefs_deployment_status; use crate::cli::OutputFormat; use crate::spec::BootEntryComposefs; use crate::spec::ImageStatus; @@ -376,7 +375,9 @@ async fn get_host() -> Result { let (_deployments, host) = get_status(&booted_ostree)?; host } - crate::store::BootedStorageKind::Composefs(_) => composefs_deployment_status().await?, + crate::store::BootedStorageKind::Composefs(booted_cfs) => { + crate::bootc_composefs::status::get_composefs_status(&storage, &booted_cfs).await? + } } } else { Default::default() From f23c594cbab00e0d73ef777cb4bc2cd31c1b894d Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 31 Oct 2025 10:01:25 -0400 Subject: [PATCH 08/13] mount: Add cwd parameter to run_findmnt and inspect_filesystem_of_dir helper Add a `cwd: Option<&Dir>` parameter to `run_findmnt()` to support running findmnt with a working directory context. This allows inspecting filesystems using a Dir fd rather than requiring absolute paths. Also add `inspect_filesystem_of_dir()` as a convenience wrapper that inspects the filesystem of a Dir by calling findmnt on "." with the directory as cwd. Assisted-by: Claude Code (Sonnet 4.5) Signed-off-by: Colin Walters --- crates/mount/src/mount.rs | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/crates/mount/src/mount.rs b/crates/mount/src/mount.rs index 1e5d56e47..ce77cac3a 100644 --- a/crates/mount/src/mount.rs +++ b/crates/mount/src/mount.rs @@ -10,6 +10,7 @@ use std::{ use anyhow::{anyhow, Context, Result}; use bootc_utils::CommandRunExt; use camino::Utf8Path; +use cap_std_ext::{cap_std::fs::Dir, cmdext::CapStdExtCommandExt}; use fn_error_context::context; use rustix::{ mount::{MoveMountFlags, OpenTreeFlags}, @@ -52,8 +53,11 @@ pub struct Findmnt { pub filesystems: Vec, } -pub fn run_findmnt(args: &[&str], path: Option<&str>) -> Result { +pub fn run_findmnt(args: &[&str], cwd: Option<&Dir>, path: Option<&str>) -> Result { let mut cmd = Command::new("findmnt"); + if let Some(cwd) = cwd { + cmd.cwd_dir(cwd.try_clone()?); + } cmd.args([ "-J", "-v", @@ -67,8 +71,8 @@ pub fn run_findmnt(args: &[&str], path: Option<&str>) -> Result { } // Retrieve a mounted filesystem from a device given a matching path -fn findmnt_filesystem(args: &[&str], path: &str) -> Result { - let o = run_findmnt(args, Some(path))?; +fn findmnt_filesystem(args: &[&str], cwd: Option<&Dir>, path: &str) -> Result { + let o = run_findmnt(args, cwd, Some(path))?; o.filesystems .into_iter() .next() @@ -79,19 +83,26 @@ fn findmnt_filesystem(args: &[&str], path: &str) -> Result { /// Inspect a target which must be a mountpoint root - it is an error /// if the target is not the mount root. pub fn inspect_filesystem(path: &Utf8Path) -> Result { - findmnt_filesystem(&["--mountpoint"], path.as_str()) + findmnt_filesystem(&["--mountpoint"], None, path.as_str()) +} + +#[context("Inspecting filesystem")] +/// Inspect a target which must be a mountpoint root - it is an error +/// if the target is not the mount root. +pub fn inspect_filesystem_of_dir(d: &Dir) -> Result { + findmnt_filesystem(&["--mountpoint"], Some(d), ".") } #[context("Inspecting filesystem by UUID {uuid}")] /// Inspect a filesystem by partition UUID pub fn inspect_filesystem_by_uuid(uuid: &str) -> Result { - findmnt_filesystem(&["--source"], &(format!("UUID={uuid}"))) + findmnt_filesystem(&["--source"], None, &(format!("UUID={uuid}"))) } // Check if a specified device contains an already mounted filesystem // in the root mount namespace pub fn is_mounted_in_pid1_mountns(path: &str) -> Result { - let o = run_findmnt(&["-N"], Some("1"))?; + let o = run_findmnt(&["-N"], None, Some("1"))?; let mounted = o.filesystems.iter().any(|fs| is_source_mounted(path, fs)); From a71e301a0209545d1d73f3db4f8415bc7a9f0303 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 31 Oct 2025 10:01:36 -0400 Subject: [PATCH 09/13] composefs: Pass physical_root down instead of accessing global /sysroot Refactor `get_sysroot_parent_dev()` to accept a `physical_root: &Dir` parameter instead of hardcoding access to `/sysroot`. This continues the ongoing work to eliminate global state access in the composefs backend, following the same pattern as the recent BootedComposefs and BootedOstree refactoring. The function now uses `inspect_filesystem_of_dir()` which operates on a Dir fd. All callers updated to pass `&storage.physical_root` or open a Dir for the upgrade code paths. Helper functions like `list_bootloader_entries()` and `delete_depl_boot_entries()` also updated to accept and pass down the physical_root parameter. Assisted-by: Claude Code (Sonnet 4.5) Signed-off-by: Colin Walters --- crates/lib/src/bootc_composefs/boot.rs | 27 +++++++++++----------- crates/lib/src/bootc_composefs/delete.rs | 20 ++++++++-------- crates/lib/src/bootc_composefs/finalize.rs | 2 +- crates/lib/src/bootc_composefs/gc.rs | 14 ++++------- crates/lib/src/bootc_composefs/rollback.rs | 2 +- crates/lib/src/bootc_composefs/status.rs | 2 +- crates/lib/src/bootc_composefs/switch.rs | 11 +++++---- crates/lib/src/bootc_composefs/update.rs | 11 +++++---- crates/system-reinstall-bootc/src/btrfs.rs | 2 +- crates/system-reinstall-bootc/src/lvm.rs | 4 ++-- 10 files changed, 49 insertions(+), 46 deletions(-) diff --git a/crates/lib/src/bootc_composefs/boot.rs b/crates/lib/src/bootc_composefs/boot.rs index d95e0b2b6..9db1fd8e0 100644 --- a/crates/lib/src/bootc_composefs/boot.rs +++ b/crates/lib/src/bootc_composefs/boot.rs @@ -5,7 +5,7 @@ use std::path::Path; use anyhow::{anyhow, Context, Result}; use bootc_blockdev::find_parent_devices; -use bootc_mount::inspect_filesystem; +use bootc_mount::inspect_filesystem_of_dir; use bootc_mount::tempmount::TempMount; use camino::{Utf8Path, Utf8PathBuf}; use cap_std_ext::{ @@ -37,7 +37,10 @@ use crate::composefs_consts::{TYPE1_ENT_PATH, TYPE1_ENT_PATH_STAGED}; use crate::parsers::bls_config::{BLSConfig, BLSConfigType}; use crate::parsers::grub_menuconfig::MenuEntry; use crate::task::Task; -use crate::{bootc_composefs::repo::open_composefs_repo, store::ComposefsFilesystem}; +use crate::{ + bootc_composefs::repo::open_composefs_repo, + store::{ComposefsFilesystem, Storage}, +}; use crate::{ bootc_composefs::state::{get_booted_bls, write_composefs_state}, bootloader::esp_in, @@ -76,7 +79,7 @@ pub(crate) enum BootSetupType<'a> { /// For initial setup, i.e. install to-disk Setup((&'a RootSetup, &'a State, &'a ComposefsFilesystem)), /// For `bootc upgrade` - Upgrade((&'a ComposefsFilesystem, &'a Host)), + Upgrade((&'a Storage, &'a ComposefsFilesystem, &'a Host)), } #[derive( @@ -168,14 +171,12 @@ pub fn mount_esp(device: &str) -> Result { TempMount::mount_dev(device, "vfat", flags, Some(c"fmask=0177,dmask=0077")) } -pub fn get_sysroot_parent_dev() -> Result { - let sysroot = Utf8PathBuf::from("/sysroot"); - - let fsinfo = inspect_filesystem(&sysroot)?; +pub fn get_sysroot_parent_dev(physical_root: &Dir) -> Result { + let fsinfo = inspect_filesystem_of_dir(physical_root)?; let parent_devices = find_parent_devices(&fsinfo.source)?; let Some(parent) = parent_devices.into_iter().next() else { - anyhow::bail!("Could not find parent device for mountpoint /sysroot"); + anyhow::bail!("Could not find parent device of system root"); }; Ok(parent) @@ -399,8 +400,8 @@ pub(crate) fn setup_composefs_bls_boot( ) } - BootSetupType::Upgrade((fs, host)) => { - let sysroot_parent = get_sysroot_parent_dev()?; + BootSetupType::Upgrade((storage, fs, host)) => { + let sysroot_parent = get_sysroot_parent_dev(&storage.physical_root)?; let bootloader = host.require_composefs_booted()?.bootloader.clone(); ( @@ -872,9 +873,9 @@ pub(crate) fn setup_composefs_uki_boot( ) } - BootSetupType::Upgrade((_, host)) => { - let sysroot = Utf8PathBuf::from("/sysroot"); - let sysroot_parent = get_sysroot_parent_dev()?; + BootSetupType::Upgrade((storage, _, host)) => { + let sysroot = Utf8PathBuf::from("/sysroot"); // Still needed for root_path + let sysroot_parent = get_sysroot_parent_dev(&storage.physical_root)?; let bootloader = host.require_composefs_booted()?.bootloader.clone(); ( diff --git a/crates/lib/src/bootc_composefs/delete.rs b/crates/lib/src/bootc_composefs/delete.rs index 5d1afb8ab..b16930e83 100644 --- a/crates/lib/src/bootc_composefs/delete.rs +++ b/crates/lib/src/bootc_composefs/delete.rs @@ -1,10 +1,7 @@ use std::{collections::HashSet, io::Write, path::Path}; use anyhow::{Context, Result}; -use cap_std_ext::{ - cap_std::{ambient_authority, fs::Dir}, - dirext::CapStdExtDirExt, -}; +use cap_std_ext::{cap_std::fs::Dir, dirext::CapStdExtDirExt}; use composefs::fsverity::Sha512HashValue; use composefs_boot::bootloader::{EFI_ADDON_DIR_EXT, EFI_EXT}; @@ -216,17 +213,20 @@ fn remove_grub_menucfg_entry(id: &str, boot_dir: &Dir, deleting_staged: bool) -> } #[fn_error_context::context("Deleting boot entries for deployment {}", deployment.deployment.verity)] -fn delete_depl_boot_entries(deployment: &DeploymentEntry, deleting_staged: bool) -> Result<()> { +fn delete_depl_boot_entries( + deployment: &DeploymentEntry, + physical_root: &Dir, + deleting_staged: bool, +) -> Result<()> { match deployment.deployment.bootloader { Bootloader::Grub => { - let boot_dir = Dir::open_ambient_dir("/sysroot/boot", ambient_authority()) - .context("Opening boot dir")?; + let boot_dir = physical_root.open_dir("boot").context("Opening boot dir")?; match deployment.deployment.boot_type { BootType::Bls => delete_type1_entry(deployment, &boot_dir, deleting_staged), BootType::Uki => { - let device = get_sysroot_parent_dev()?; + let device = get_sysroot_parent_dev(physical_root)?; let (esp_part, ..) = get_esp_partition(&device)?; let esp_mount = mount_esp(&esp_part)?; @@ -242,7 +242,7 @@ fn delete_depl_boot_entries(deployment: &DeploymentEntry, deleting_staged: bool) } Bootloader::Systemd => { - let device = get_sysroot_parent_dev()?; + let device = get_sysroot_parent_dev(physical_root)?; let (esp_part, ..) = get_esp_partition(&device)?; let esp_mount = mount_esp(&esp_part)?; @@ -362,7 +362,7 @@ pub(crate) async fn delete_composefs_deployment( tracing::info!("Deleting {kind}deployment '{deployment_id}'"); - delete_depl_boot_entries(&depl_to_del, deleting_staged)?; + delete_depl_boot_entries(&depl_to_del, &storage.physical_root, deleting_staged)?; composefs_gc(storage, booted_cfs).await?; diff --git a/crates/lib/src/bootc_composefs/finalize.rs b/crates/lib/src/bootc_composefs/finalize.rs index 48ca9eff8..d397c9f5c 100644 --- a/crates/lib/src/bootc_composefs/finalize.rs +++ b/crates/lib/src/bootc_composefs/finalize.rs @@ -86,7 +86,7 @@ pub(crate) async fn composefs_backend_finalize( // Unmount EROFS drop(erofs_tmp_mnt); - let sysroot_parent = get_sysroot_parent_dev()?; + let sysroot_parent = get_sysroot_parent_dev(&storage.physical_root)?; // NOTE: Assumption here that ESP will always be present let (esp_part, ..) = get_esp_partition(&sysroot_parent)?; diff --git a/crates/lib/src/bootc_composefs/gc.rs b/crates/lib/src/bootc_composefs/gc.rs index 2f04ee48c..8195cade2 100644 --- a/crates/lib/src/bootc_composefs/gc.rs +++ b/crates/lib/src/bootc_composefs/gc.rs @@ -5,10 +5,7 @@ //! - We delete bootloader + image but fail to delete the state/unrefenced objects etc use anyhow::{Context, Result}; -use cap_std_ext::{ - cap_std::{ambient_authority, fs::Dir}, - dirext::CapStdExtDirExt, -}; +use cap_std_ext::{cap_std::fs::Dir, dirext::CapStdExtDirExt}; use composefs::fsverity::{FsVerityHashValue, Sha512HashValue}; use crate::{ @@ -47,13 +44,12 @@ fn list_erofs_images(sysroot: &Dir) -> Result> { /// # Returns /// The fsverity of EROFS images corresponding to boot entries #[fn_error_context::context("Listing bootloader entries")] -fn list_bootloader_entries() -> Result> { +fn list_bootloader_entries(physical_root: &Dir) -> Result> { let bootloader = get_bootloader()?; let entries = match bootloader { Bootloader::Grub => { - let boot_dir = Dir::open_ambient_dir("/sysroot/boot", ambient_authority()) - .context("Opening boot dir")?; + let boot_dir = physical_root.open_dir("boot").context("Opening boot dir")?; // Grub entries are always in boot let grub_dir = boot_dir.open_dir("grub2").context("Opening grub dir")?; @@ -79,7 +75,7 @@ fn list_bootloader_entries() -> Result> { } Bootloader::Systemd => { - let device = get_sysroot_parent_dev()?; + let device = get_sysroot_parent_dev(physical_root)?; let (esp_part, ..) = get_esp_partition(&device)?; let esp_mount = mount_esp(&esp_part)?; @@ -179,7 +175,7 @@ pub(crate) async fn composefs_gc(storage: &Storage, booted_cfs: &BootedComposefs let sysroot = &storage.physical_root; - let bootloader_entries = list_bootloader_entries()?; + let bootloader_entries = list_bootloader_entries(&storage.physical_root)?; let images = list_erofs_images(&sysroot)?; // Collect the deployments that have an image but no bootloader entry diff --git a/crates/lib/src/bootc_composefs/rollback.rs b/crates/lib/src/bootc_composefs/rollback.rs index 2b9d05f43..6338bf9b5 100644 --- a/crates/lib/src/bootc_composefs/rollback.rs +++ b/crates/lib/src/bootc_composefs/rollback.rs @@ -215,7 +215,7 @@ pub(crate) async fn composefs_rollback( } Bootloader::Systemd => { - let parent = get_sysroot_parent_dev()?; + let parent = get_sysroot_parent_dev(&storage.physical_root)?; let (esp_part, ..) = get_esp_partition(&parent)?; let esp_mount = mount_esp(&esp_part)?; diff --git a/crates/lib/src/bootc_composefs/status.rs b/crates/lib/src/bootc_composefs/status.rs index 3204107d9..04624243b 100644 --- a/crates/lib/src/bootc_composefs/status.rs +++ b/crates/lib/src/bootc_composefs/status.rs @@ -355,7 +355,7 @@ pub(crate) async fn composefs_deployment_status_from( // // See: https://uapi-group.org/specifications/specs/boot_loader_specification/#mount-points Bootloader::Systemd => { - let parent = get_sysroot_parent_dev()?; + let parent = get_sysroot_parent_dev(sysroot)?; let (esp_part, ..) = get_esp_partition(&parent)?; let esp_mount = mount_esp(&esp_part)?; diff --git a/crates/lib/src/bootc_composefs/switch.rs b/crates/lib/src/bootc_composefs/switch.rs index b45c66ebb..5cb1e2c1f 100644 --- a/crates/lib/src/bootc_composefs/switch.rs +++ b/crates/lib/src/bootc_composefs/switch.rs @@ -57,15 +57,18 @@ pub(crate) async fn switch_composefs( match boot_type { BootType::Bls => { boot_digest = Some(setup_composefs_bls_boot( - BootSetupType::Upgrade((&fs, &host)), + BootSetupType::Upgrade((storage, &fs, &host)), repo, &id, entry, )?) } - BootType::Uki => { - setup_composefs_uki_boot(BootSetupType::Upgrade((&fs, &host)), repo, &id, entries)? - } + BootType::Uki => setup_composefs_uki_boot( + BootSetupType::Upgrade((storage, &fs, &host)), + repo, + &id, + entries, + )?, }; // TODO: Remove this hardcoded path when write_composefs_state accepts a Dir diff --git a/crates/lib/src/bootc_composefs/update.rs b/crates/lib/src/bootc_composefs/update.rs index eaa27aa85..eebfd3faa 100644 --- a/crates/lib/src/bootc_composefs/update.rs +++ b/crates/lib/src/bootc_composefs/update.rs @@ -160,16 +160,19 @@ pub(crate) async fn upgrade_composefs( match boot_type { BootType::Bls => { boot_digest = Some(setup_composefs_bls_boot( - BootSetupType::Upgrade((&fs, &host)), + BootSetupType::Upgrade((storage, &fs, &host)), repo, &id, entry, )?) } - BootType::Uki => { - setup_composefs_uki_boot(BootSetupType::Upgrade((&fs, &host)), repo, &id, entries)? - } + BootType::Uki => setup_composefs_uki_boot( + BootSetupType::Upgrade((storage, &fs, &host)), + repo, + &id, + entries, + )?, }; write_composefs_state( diff --git a/crates/system-reinstall-bootc/src/btrfs.rs b/crates/system-reinstall-bootc/src/btrfs.rs index 31df04d61..cfaa6eccb 100644 --- a/crates/system-reinstall-bootc/src/btrfs.rs +++ b/crates/system-reinstall-bootc/src/btrfs.rs @@ -4,7 +4,7 @@ use fn_error_context::context; #[context("check_root_siblings")] pub(crate) fn check_root_siblings() -> Result> { - let mounts = bootc_mount::run_findmnt(&[], None)?; + let mounts = bootc_mount::run_findmnt(&[], None, None)?; let problem_filesystems: Vec = mounts .filesystems .iter() diff --git a/crates/system-reinstall-bootc/src/lvm.rs b/crates/system-reinstall-bootc/src/lvm.rs index 5247ae9ed..3022ed6f7 100644 --- a/crates/system-reinstall-bootc/src/lvm.rs +++ b/crates/system-reinstall-bootc/src/lvm.rs @@ -57,7 +57,7 @@ pub(crate) fn check_root_siblings() -> Result> { let siblings: Vec = all_volumes .iter() .filter(|lv| { - let mount = run_findmnt(&["-S", &lv.lv_path], None).log_err_default(); + let mount = run_findmnt(&["-S", &lv.lv_path], None, None).log_err_default(); if let Some(fs) = mount.filesystems.first() { &fs.target == "/" } else { @@ -66,7 +66,7 @@ pub(crate) fn check_root_siblings() -> Result> { }) .flat_map(|root_lv| parse_volumes(Some(root_lv.vg_name.as_str())).unwrap_or_default()) .try_fold(Vec::new(), |mut acc, r| -> anyhow::Result<_> { - let mount = run_findmnt(&["-S", &r.lv_path], None).log_err_default(); + let mount = run_findmnt(&["-S", &r.lv_path], None, None).log_err_default(); let mount_path = if let Some(fs) = mount.filesystems.first() { &fs.target } else { From efa4bef628305e4033c8c6e98d0a1553ecd5131a Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Tue, 4 Nov 2025 11:49:00 +0530 Subject: [PATCH 10/13] storage: Fix premature ostree booted check There were a few checks that asserted ostree booted system even for systems booted via composefs. Move the checks in `Storage::new` Signed-off-by: Pragyan Poudyal Signed-off-by: Colin Walters --- crates/lib/src/cli.rs | 3 +-- crates/lib/src/status.rs | 22 +++++++++----------- crates/lib/src/store/mod.rs | 40 ++++++++++++++++++++----------------- 3 files changed, 32 insertions(+), 33 deletions(-) diff --git a/crates/lib/src/cli.rs b/crates/lib/src/cli.rs index e33ed1748..257967a93 100644 --- a/crates/lib/src/cli.rs +++ b/crates/lib/src/cli.rs @@ -715,7 +715,6 @@ pub(crate) fn ensure_self_unshared_mount_namespace() -> Result<()> { /// Load global storage state, expecting that we're booted into a bootc system. #[context("Initializing storage")] pub(crate) async fn get_storage() -> Result { - prepare_for_write()?; BootedStorage::new().await } @@ -842,7 +841,7 @@ fn soft_reboot_rollback(booted_ostree: &BootedOstree<'_>) -> Result<()> { /// IMPORTANT: This may end up re-executing the current process, /// so anything that happens before this should be idempotent. #[context("Preparing for write")] -fn prepare_for_write() -> Result<()> { +pub(crate) fn prepare_for_write() -> Result<()> { use std::sync::atomic::{AtomicBool, Ordering}; // This is intending to give "at most once" semantics to this diff --git a/crates/lib/src/status.rs b/crates/lib/src/status.rs index 220602b5a..d16075782 100644 --- a/crates/lib/src/status.rs +++ b/crates/lib/src/status.rs @@ -10,7 +10,6 @@ use fn_error_context::context; use ostree::glib; use ostree_container::OstreeImageReference; use ostree_ext::container as ostree_container; -use ostree_ext::container_utils::ostree_booted; use ostree_ext::keyfileext::KeyFileExt; use ostree_ext::oci_spec; use ostree_ext::oci_spec::image::Digest; @@ -368,19 +367,16 @@ pub(crate) fn get_status( } async fn get_host() -> Result { - let host = if ostree_booted()? { - let storage = super::cli::get_storage().await?; - match storage.kind()? { - crate::store::BootedStorageKind::Ostree(booted_ostree) => { - let (_deployments, host) = get_status(&booted_ostree)?; - host - } - crate::store::BootedStorageKind::Composefs(booted_cfs) => { - crate::bootc_composefs::status::get_composefs_status(&storage, &booted_cfs).await? - } + let storage = super::cli::get_storage().await?; + + let host = match storage.kind()? { + crate::store::BootedStorageKind::Ostree(booted_ostree) => { + let (_deployments, host) = get_status(&booted_ostree)?; + host + } + crate::store::BootedStorageKind::Composefs(booted_cfs) => { + crate::bootc_composefs::status::get_composefs_status(&storage, &booted_cfs).await? } - } else { - Default::default() }; Ok(host) diff --git a/crates/lib/src/store/mod.rs b/crates/lib/src/store/mod.rs index 5b9d2995f..82b99152b 100644 --- a/crates/lib/src/store/mod.rs +++ b/crates/lib/src/store/mod.rs @@ -31,6 +31,7 @@ use ostree_ext::{gio, ostree}; use rustix::fs::Mode; use crate::bootc_composefs::status::{composefs_booted, ComposefsCmdline}; +use crate::cli::prepare_for_write; use crate::lsm; use crate::podstorage::CStorage; use crate::spec::ImageStatus; @@ -111,11 +112,12 @@ impl BootedStorage { pub(crate) async fn new() -> Result { let physical_root = Dir::open_ambient_dir("/sysroot", cap_std::ambient_authority()) .context("Opening /sysroot")?; + let run = Dir::open_ambient_dir("/run", cap_std::ambient_authority()).context("Opening /run")?; + if let Some(cmdline) = composefs_booted()? { - let mut composefs = - ComposefsRepository::open_path(physical_root.open_dir(COMPOSEFS)?, ".")?; + let mut composefs = ComposefsRepository::open_path(&physical_root, COMPOSEFS)?; if cmdline.insecure { composefs.set_insecure(true); } @@ -128,24 +130,26 @@ impl BootedStorage { composefs: OnceCell::from(composefs), imgstore: Default::default(), }; - Ok(Self { storage }) - } else { - let sysroot = ostree::Sysroot::new_default(); - sysroot.set_mount_namespace_in_use(); - let sysroot = ostree_ext::sysroot::SysrootLock::new_from_sysroot(&sysroot).await?; - sysroot.load(gio::Cancellable::NONE)?; - // Verify this is a booted system - let _ = sysroot.require_booted_deployment()?; - let storage = Storage { - physical_root, - run, - ostree: OnceCell::from(sysroot), - composefs: Default::default(), - imgstore: Default::default(), - }; - Ok(Self { storage }) + return Ok(Self { storage }); } + + prepare_for_write()?; + + let sysroot = ostree::Sysroot::new_default(); + sysroot.set_mount_namespace_in_use(); + let sysroot = ostree_ext::sysroot::SysrootLock::new_from_sysroot(&sysroot).await?; + sysroot.load(gio::Cancellable::NONE)?; + + let storage = Storage { + physical_root, + run, + ostree: OnceCell::from(sysroot), + composefs: Default::default(), + imgstore: Default::default(), + }; + + Ok(Self { storage }) } /// Determine the boot storage backend kind. From 7c61342a2866fb436a2cae71a97c88c762932381 Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Tue, 4 Nov 2025 11:51:26 +0530 Subject: [PATCH 11/13] tests/integration: Update composefs booted status test Check for verity inside the json returned by `bootc status --json` and compare it with the compsefs digest from kernel cmdline Signed-off-by: Pragyan Poudyal Signed-off-by: Colin Walters --- Cargo.lock | 1 + crates/tests-integration/Cargo.toml | 1 + .../tests-integration/src/composefs_bcvk.rs | 23 +++++++++++++++++++ 3 files changed, 25 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index edf06b837..2674b9dd2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2624,6 +2624,7 @@ name = "tests-integration" version = "0.1.0" dependencies = [ "anyhow", + "bootc-kernel-cmdline", "camino", "cap-std-ext", "clap", diff --git a/crates/tests-integration/Cargo.toml b/crates/tests-integration/Cargo.toml index 538d575b9..eb1ca75d5 100644 --- a/crates/tests-integration/Cargo.toml +++ b/crates/tests-integration/Cargo.toml @@ -23,6 +23,7 @@ serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } tempfile = { workspace = true } xshell = { workspace = true } +bootc-kernel-cmdline = { path = "../kernel_cmdline", version = "0.0.0" } # Crate-specific dependencies libtest-mimic = "0.8.0" diff --git a/crates/tests-integration/src/composefs_bcvk.rs b/crates/tests-integration/src/composefs_bcvk.rs index 0dd385eff..41f46e596 100644 --- a/crates/tests-integration/src/composefs_bcvk.rs +++ b/crates/tests-integration/src/composefs_bcvk.rs @@ -1,4 +1,5 @@ use anyhow::Result; +use bootc_kernel_cmdline; use camino::Utf8Path; use libtest_mimic::Trial; use xshell::{cmd, Shell}; @@ -57,6 +58,28 @@ fn inner_tests() -> Vec { assert!(st.is_object()); assert!(Utf8Path::new("/sysroot/composefs").try_exists()?); assert!(!Utf8Path::new("/sysroot/ostree").try_exists()?); + + let cmdline = bootc_kernel_cmdline::utf8::Cmdline::from_proc()?; + + let cfs = cmdline.find("composefs"); + assert!(cfs.is_some()); + let cfs = cfs.unwrap(); + + let verity_from_cmdline = cfs.value(); + assert!(verity_from_cmdline.is_some()); + let verity_from_cmdline = verity_from_cmdline.unwrap(); + + let verity_from_status = st + .get("status") + .and_then(|s| s.get("booted")) + .and_then(|b| b.get("composefs")) + .and_then(|c| c.get("verity")) + .and_then(|v| v.as_str()); + + assert!(verity_from_status.is_some()); + + assert_eq!(verity_from_status.unwrap(), verity_from_cmdline); + Ok(()) })] .into_iter() From e159138a2a1f0982946602f46b0264443c58f56f Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Wed, 5 Nov 2025 13:06:11 +0530 Subject: [PATCH 12/13] storage: Conditionally call `prepare_for_write` For ostree systems, before making any changes to the repo/boot entries we call `prepare_for_write`. Before this we were calling it unconditionally even if the command was `bootc status`, which was causing test failures. We pass in a flag now to `BootedStorage::new` which will conditionally call `prepare_for_write`. We also need this to make sure we keep the API more or less similar to what we had before Signed-off-by: Pragyan Poudyal Signed-off-by: Colin Walters --- crates/lib/src/cli.rs | 10 +++++++++- crates/lib/src/status.rs | 32 ++++++++++++++++++++++++-------- crates/lib/src/store/mod.rs | 6 ++++-- 3 files changed, 37 insertions(+), 11 deletions(-) diff --git a/crates/lib/src/cli.rs b/crates/lib/src/cli.rs index 257967a93..963207687 100644 --- a/crates/lib/src/cli.rs +++ b/crates/lib/src/cli.rs @@ -715,7 +715,15 @@ pub(crate) fn ensure_self_unshared_mount_namespace() -> Result<()> { /// Load global storage state, expecting that we're booted into a bootc system. #[context("Initializing storage")] pub(crate) async fn get_storage() -> Result { - BootedStorage::new().await + BootedStorage::new(true).await +} + +/// Load global storage state, but do not internally call `prepare_for_write` for ostree booted +/// systems +/// We do this to keep the `bootc status` output unchanged +#[context("Initializing locked storage")] +pub(crate) async fn get_storage_unlocked() -> Result { + BootedStorage::new(false).await } #[context("Querying root privilege")] diff --git a/crates/lib/src/status.rs b/crates/lib/src/status.rs index d16075782..8502456ac 100644 --- a/crates/lib/src/status.rs +++ b/crates/lib/src/status.rs @@ -18,11 +18,13 @@ use ostree_ext::sysroot::SysrootLock; use ostree_ext::ostree; +use crate::cli::get_storage_unlocked; use crate::cli::OutputFormat; use crate::spec::BootEntryComposefs; use crate::spec::ImageStatus; use crate::spec::{BootEntry, BootOrder, Host, HostSpec, HostStatus, HostType}; use crate::spec::{ImageReference, ImageSignature}; +use crate::store::BootedStorageKind; use crate::store::CachedImageStatus; impl From for ImageSignature { @@ -367,15 +369,29 @@ pub(crate) fn get_status( } async fn get_host() -> Result { - let storage = super::cli::get_storage().await?; - - let host = match storage.kind()? { - crate::store::BootedStorageKind::Ostree(booted_ostree) => { - let (_deployments, host) = get_status(&booted_ostree)?; - host + let storage = match get_storage_unlocked().await { + Ok(storage) => storage, + Err(_) => { + // If storage initialization fails (e.g., in container environments), + // return a default host indicating the system is not deployed via bootc + return Ok(Host::default()); } - crate::store::BootedStorageKind::Composefs(booted_cfs) => { - crate::bootc_composefs::status::get_composefs_status(&storage, &booted_cfs).await? + }; + + let host = match storage.kind() { + Ok(kind) => match kind { + BootedStorageKind::Ostree(booted_ostree) => { + let (_deployments, host) = get_status(&booted_ostree)?; + host + } + BootedStorageKind::Composefs(booted_cfs) => { + crate::bootc_composefs::status::get_composefs_status(&storage, &booted_cfs).await? + } + }, + Err(_) => { + // If determining storage kind fails (e.g., no booted deployment), + // return a default host indicating the system is not deployed via bootc + Host::default() } }; diff --git a/crates/lib/src/store/mod.rs b/crates/lib/src/store/mod.rs index 82b99152b..c8031b60e 100644 --- a/crates/lib/src/store/mod.rs +++ b/crates/lib/src/store/mod.rs @@ -109,7 +109,7 @@ impl BootedStorage { /// /// This detects whether the system is booted via composefs or ostree /// and initializes the appropriate storage backend. - pub(crate) async fn new() -> Result { + pub(crate) async fn new(prep_for_write: bool) -> Result { let physical_root = Dir::open_ambient_dir("/sysroot", cap_std::ambient_authority()) .context("Opening /sysroot")?; @@ -134,7 +134,9 @@ impl BootedStorage { return Ok(Self { storage }); } - prepare_for_write()?; + if prep_for_write { + prepare_for_write()?; + } let sysroot = ostree::Sysroot::new_default(); sysroot.set_mount_namespace_in_use(); From 4e07f7400466e61389243f5fc7d5d874be79f5ab Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 5 Nov 2025 11:49:26 -0500 Subject: [PATCH 13/13] storage: Refactor BootedStorage::new to return Option Change `BootedStorage::new()` to return `Result>` instead of taking a `prep_for_write` boolean parameter. This makes the API more idiomatic and clearer about when the system is not booted via bootc. Assisted-by: Claude Code (Sonnet 4.5) Signed-off-by: Colin Walters --- crates/lib/src/cli.rs | 18 ++++++--------- crates/lib/src/status.rs | 12 ++++------ crates/lib/src/store/mod.rs | 45 ++++++++++++++++++++----------------- 3 files changed, 35 insertions(+), 40 deletions(-) diff --git a/crates/lib/src/cli.rs b/crates/lib/src/cli.rs index 963207687..75bc7871f 100644 --- a/crates/lib/src/cli.rs +++ b/crates/lib/src/cli.rs @@ -8,7 +8,7 @@ use std::os::unix::process::CommandExt; use std::process::Command; use std::sync::Arc; -use anyhow::{ensure, Context, Result}; +use anyhow::{anyhow, ensure, Context, Result}; use camino::{Utf8Path, Utf8PathBuf}; use cap_std_ext::cap_std; use cap_std_ext::cap_std::fs::Dir; @@ -24,7 +24,6 @@ use ostree_ext::composefs::fsverity; use ostree_ext::composefs::fsverity::FsVerityHashValue; use ostree_ext::composefs::splitstream::SplitStreamWriter; use ostree_ext::container as ostree_container; -use ostree_ext::container_utils::ostree_booted; use ostree_ext::containers_image_proxy::ImageProxyConfig; use ostree_ext::keyfileext::KeyFileExt; use ostree_ext::ostree; @@ -713,17 +712,14 @@ pub(crate) fn ensure_self_unshared_mount_namespace() -> Result<()> { } /// Load global storage state, expecting that we're booted into a bootc system. +/// This prepares the process for write operations (re-exec, mount namespace, etc). #[context("Initializing storage")] pub(crate) async fn get_storage() -> Result { - BootedStorage::new(true).await -} - -/// Load global storage state, but do not internally call `prepare_for_write` for ostree booted -/// systems -/// We do this to keep the `bootc status` output unchanged -#[context("Initializing locked storage")] -pub(crate) async fn get_storage_unlocked() -> Result { - BootedStorage::new(false).await + prepare_for_write()?; + let r = BootedStorage::new() + .await? + .ok_or_else(|| anyhow!("System not booted via bootc"))?; + Ok(r) } #[context("Querying root privilege")] diff --git a/crates/lib/src/status.rs b/crates/lib/src/status.rs index 8502456ac..55e7146b3 100644 --- a/crates/lib/src/status.rs +++ b/crates/lib/src/status.rs @@ -18,12 +18,12 @@ use ostree_ext::sysroot::SysrootLock; use ostree_ext::ostree; -use crate::cli::get_storage_unlocked; use crate::cli::OutputFormat; use crate::spec::BootEntryComposefs; use crate::spec::ImageStatus; use crate::spec::{BootEntry, BootOrder, Host, HostSpec, HostStatus, HostType}; use crate::spec::{ImageReference, ImageSignature}; +use crate::store::BootedStorage; use crate::store::BootedStorageKind; use crate::store::CachedImageStatus; @@ -369,13 +369,9 @@ pub(crate) fn get_status( } async fn get_host() -> Result { - let storage = match get_storage_unlocked().await { - Ok(storage) => storage, - Err(_) => { - // If storage initialization fails (e.g., in container environments), - // return a default host indicating the system is not deployed via bootc - return Ok(Host::default()); - } + let Some(storage) = BootedStorage::new().await? else { + // If we're not booted, then return a default. + return Ok(Host::default()); }; let host = match storage.kind() { diff --git a/crates/lib/src/store/mod.rs b/crates/lib/src/store/mod.rs index c8031b60e..38a91f585 100644 --- a/crates/lib/src/store/mod.rs +++ b/crates/lib/src/store/mod.rs @@ -26,12 +26,12 @@ use cap_std_ext::cap_std::fs::{Dir, DirBuilder, DirBuilderExt as _}; use cap_std_ext::dirext::CapStdExtDirExt; use fn_error_context::context; +use ostree_ext::container_utils::ostree_booted; use ostree_ext::sysroot::SysrootLock; use ostree_ext::{gio, ostree}; use rustix::fs::Mode; use crate::bootc_composefs::status::{composefs_booted, ComposefsCmdline}; -use crate::cli::prepare_for_write; use crate::lsm; use crate::podstorage::CStorage; use crate::spec::ImageStatus; @@ -109,14 +109,18 @@ impl BootedStorage { /// /// This detects whether the system is booted via composefs or ostree /// and initializes the appropriate storage backend. - pub(crate) async fn new(prep_for_write: bool) -> Result { + /// + /// Note: For write operations, callers should call `prepare_for_write()` + /// before calling this function to ensure the process is in the correct + /// mount namespace. + pub(crate) async fn new() -> Result> { let physical_root = Dir::open_ambient_dir("/sysroot", cap_std::ambient_authority()) .context("Opening /sysroot")?; let run = Dir::open_ambient_dir("/run", cap_std::ambient_authority()).context("Opening /run")?; - if let Some(cmdline) = composefs_booted()? { + let r = if let Some(cmdline) = composefs_booted()? { let mut composefs = ComposefsRepository::open_path(&physical_root, COMPOSEFS)?; if cmdline.insecure { composefs.set_insecure(true); @@ -131,27 +135,26 @@ impl BootedStorage { imgstore: Default::default(), }; - return Ok(Self { storage }); - } - - if prep_for_write { - prepare_for_write()?; - } + Some(Self { storage }) + } else if ostree_booted()? { + let sysroot = ostree::Sysroot::new_default(); + sysroot.set_mount_namespace_in_use(); + let sysroot = ostree_ext::sysroot::SysrootLock::new_from_sysroot(&sysroot).await?; + sysroot.load(gio::Cancellable::NONE)?; - let sysroot = ostree::Sysroot::new_default(); - sysroot.set_mount_namespace_in_use(); - let sysroot = ostree_ext::sysroot::SysrootLock::new_from_sysroot(&sysroot).await?; - sysroot.load(gio::Cancellable::NONE)?; + let storage = Storage { + physical_root, + run, + ostree: OnceCell::from(sysroot), + composefs: Default::default(), + imgstore: Default::default(), + }; - let storage = Storage { - physical_root, - run, - ostree: OnceCell::from(sysroot), - composefs: Default::default(), - imgstore: Default::default(), + Some(Self { storage }) + } else { + None }; - - Ok(Self { storage }) + Ok(r) } /// Determine the boot storage backend kind.