Skip to content

Commit e5249d1

Browse files
committed
storage: Refactor BootedStorage::new to return Option
Change `BootedStorage::new()` to return `Result<Option<Self>>` 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 <[email protected]>
1 parent a1d072f commit e5249d1

File tree

3 files changed

+35
-40
lines changed

3 files changed

+35
-40
lines changed

crates/lib/src/cli.rs

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use std::os::unix::process::CommandExt;
88
use std::process::Command;
99
use std::sync::Arc;
1010

11-
use anyhow::{ensure, Context, Result};
11+
use anyhow::{anyhow, ensure, Context, Result};
1212
use camino::{Utf8Path, Utf8PathBuf};
1313
use cap_std_ext::cap_std;
1414
use cap_std_ext::cap_std::fs::Dir;
@@ -24,7 +24,6 @@ use ostree_ext::composefs::fsverity;
2424
use ostree_ext::composefs::fsverity::FsVerityHashValue;
2525
use ostree_ext::composefs::splitstream::SplitStreamWriter;
2626
use ostree_ext::container as ostree_container;
27-
use ostree_ext::container_utils::ostree_booted;
2827
use ostree_ext::containers_image_proxy::ImageProxyConfig;
2928
use ostree_ext::keyfileext::KeyFileExt;
3029
use ostree_ext::ostree;
@@ -713,17 +712,14 @@ pub(crate) fn ensure_self_unshared_mount_namespace() -> Result<()> {
713712
}
714713

715714
/// Load global storage state, expecting that we're booted into a bootc system.
715+
/// This prepares the process for write operations (re-exec, mount namespace, etc).
716716
#[context("Initializing storage")]
717717
pub(crate) async fn get_storage() -> Result<crate::store::BootedStorage> {
718-
BootedStorage::new(true).await
719-
}
720-
721-
/// Load global storage state, but do not internally call `prepare_for_write` for ostree booted
722-
/// systems
723-
/// We do this to keep the `bootc status` output unchanged
724-
#[context("Initializing locked storage")]
725-
pub(crate) async fn get_storage_unlocked() -> Result<crate::store::BootedStorage> {
726-
BootedStorage::new(false).await
718+
prepare_for_write()?;
719+
let r = BootedStorage::new()
720+
.await?
721+
.ok_or_else(|| anyhow!("System not booted via bootc"))?;
722+
Ok(r)
727723
}
728724

729725
#[context("Querying root privilege")]

crates/lib/src/status.rs

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,12 @@ use ostree_ext::sysroot::SysrootLock;
1818

1919
use ostree_ext::ostree;
2020

21-
use crate::cli::get_storage_unlocked;
2221
use crate::cli::OutputFormat;
2322
use crate::spec::BootEntryComposefs;
2423
use crate::spec::ImageStatus;
2524
use crate::spec::{BootEntry, BootOrder, Host, HostSpec, HostStatus, HostType};
2625
use crate::spec::{ImageReference, ImageSignature};
26+
use crate::store::BootedStorage;
2727
use crate::store::BootedStorageKind;
2828
use crate::store::CachedImageStatus;
2929

@@ -369,13 +369,9 @@ pub(crate) fn get_status(
369369
}
370370

371371
async fn get_host() -> Result<Host> {
372-
let storage = match get_storage_unlocked().await {
373-
Ok(storage) => storage,
374-
Err(_) => {
375-
// If storage initialization fails (e.g., in container environments),
376-
// return a default host indicating the system is not deployed via bootc
377-
return Ok(Host::default());
378-
}
372+
let Some(storage) = BootedStorage::new().await? else {
373+
// If we're not booted, then return a default.
374+
return Ok(Host::default());
379375
};
380376

381377
let host = match storage.kind() {

crates/lib/src/store/mod.rs

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,12 @@ use cap_std_ext::cap_std::fs::{Dir, DirBuilder, DirBuilderExt as _};
2626
use cap_std_ext::dirext::CapStdExtDirExt;
2727
use fn_error_context::context;
2828

29+
use ostree_ext::container_utils::ostree_booted;
2930
use ostree_ext::sysroot::SysrootLock;
3031
use ostree_ext::{gio, ostree};
3132
use rustix::fs::Mode;
3233

3334
use crate::bootc_composefs::status::{composefs_booted, ComposefsCmdline};
34-
use crate::cli::prepare_for_write;
3535
use crate::lsm;
3636
use crate::podstorage::CStorage;
3737
use crate::spec::ImageStatus;
@@ -109,14 +109,18 @@ impl BootedStorage {
109109
///
110110
/// This detects whether the system is booted via composefs or ostree
111111
/// and initializes the appropriate storage backend.
112-
pub(crate) async fn new(prep_for_write: bool) -> Result<Self> {
112+
///
113+
/// Note: For write operations, callers should call `prepare_for_write()`
114+
/// before calling this function to ensure the process is in the correct
115+
/// mount namespace.
116+
pub(crate) async fn new() -> Result<Option<Self>> {
113117
let physical_root = Dir::open_ambient_dir("/sysroot", cap_std::ambient_authority())
114118
.context("Opening /sysroot")?;
115119

116120
let run =
117121
Dir::open_ambient_dir("/run", cap_std::ambient_authority()).context("Opening /run")?;
118122

119-
if let Some(cmdline) = composefs_booted()? {
123+
let r = if let Some(cmdline) = composefs_booted()? {
120124
let mut composefs = ComposefsRepository::open_path(&physical_root, COMPOSEFS)?;
121125
if cmdline.insecure {
122126
composefs.set_insecure(true);
@@ -131,27 +135,26 @@ impl BootedStorage {
131135
imgstore: Default::default(),
132136
};
133137

134-
return Ok(Self { storage });
135-
}
136-
137-
if prep_for_write {
138-
prepare_for_write()?;
139-
}
138+
Some(Self { storage })
139+
} else if ostree_booted()? {
140+
let sysroot = ostree::Sysroot::new_default();
141+
sysroot.set_mount_namespace_in_use();
142+
let sysroot = ostree_ext::sysroot::SysrootLock::new_from_sysroot(&sysroot).await?;
143+
sysroot.load(gio::Cancellable::NONE)?;
140144

141-
let sysroot = ostree::Sysroot::new_default();
142-
sysroot.set_mount_namespace_in_use();
143-
let sysroot = ostree_ext::sysroot::SysrootLock::new_from_sysroot(&sysroot).await?;
144-
sysroot.load(gio::Cancellable::NONE)?;
145+
let storage = Storage {
146+
physical_root,
147+
run,
148+
ostree: OnceCell::from(sysroot),
149+
composefs: Default::default(),
150+
imgstore: Default::default(),
151+
};
145152

146-
let storage = Storage {
147-
physical_root,
148-
run,
149-
ostree: OnceCell::from(sysroot),
150-
composefs: Default::default(),
151-
imgstore: Default::default(),
153+
Some(Self { storage })
154+
} else {
155+
None
152156
};
153-
154-
Ok(Self { storage })
157+
Ok(r)
155158
}
156159

157160
/// Determine the boot storage backend kind.

0 commit comments

Comments
 (0)