Skip to content

Commit b6decd9

Browse files
committed
cli: Refactor switch/rollback/edit to use BootedOstree
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 <[email protected]>
1 parent e6d8d0b commit b6decd9

File tree

4 files changed

+108
-73
lines changed

4 files changed

+108
-73
lines changed

crates/lib/src/cli.rs

Lines changed: 94 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -827,15 +827,15 @@ fn soft_reboot_staged(sysroot: &SysrootLock) -> Result<()> {
827827

828828
/// Perform a soft reboot for a rollback deployment
829829
#[context("Soft reboot rollback deployment")]
830-
fn soft_reboot_rollback(sysroot: &SysrootLock) -> Result<()> {
830+
fn soft_reboot_rollback(booted_ostree: &BootedOstree<'_>) -> Result<()> {
831831
println!("Rollback deployment is soft-reboot capable, preparing for soft-reboot...");
832832

833-
let deployments_list = sysroot.deployments();
833+
let deployments_list = booted_ostree.sysroot.deployments();
834834
let target_deployment = deployments_list
835835
.first()
836836
.ok_or_else(|| anyhow::anyhow!("No rollback deployment found!"))?;
837837

838-
prepare_soft_reboot(sysroot, target_deployment)
838+
prepare_soft_reboot(booted_ostree.sysroot, target_deployment)
839839
}
840840

841841
/// A few process changes that need to be made for writing.
@@ -996,34 +996,19 @@ pub(crate) fn imgref_for_switch(opts: &SwitchOpts) -> Result<ImageReference> {
996996
return Ok(target);
997997
}
998998

999-
/// Implementation of the `bootc switch` CLI command.
1000-
#[context("Switching")]
1001-
async fn switch(opts: SwitchOpts) -> Result<()> {
999+
/// Implementation of the `bootc switch` CLI command for ostree backend.
1000+
#[context("Switching (ostree)")]
1001+
async fn switch_ostree(
1002+
opts: SwitchOpts,
1003+
storage: &Storage,
1004+
booted_ostree: &BootedOstree<'_>,
1005+
) -> Result<()> {
10021006
let target = imgref_for_switch(&opts)?;
1003-
10041007
let prog: ProgressWriter = opts.progress.try_into()?;
1005-
1006-
// If we're doing an in-place mutation, we shortcut most of the rest of the work here
1007-
if opts.mutate_in_place {
1008-
let deployid = {
1009-
// Clone to pass into helper thread
1010-
let target = target.clone();
1011-
let root = cap_std::fs::Dir::open_ambient_dir("/", cap_std::ambient_authority())?;
1012-
tokio::task::spawn_blocking(move || {
1013-
crate::deploy::switch_origin_inplace(&root, &target)
1014-
})
1015-
.await??
1016-
};
1017-
println!("Updated {deployid} to pull from {target}");
1018-
return Ok(());
1019-
}
1020-
10211008
let cancellable = gio::Cancellable::NONE;
10221009

1023-
let storage = &get_storage().await?;
1024-
let ostree = storage.get_ostree()?;
1025-
let repo = &ostree.repo();
1026-
let (booted_deployment, _deployments, host) = crate::status::get_status_require_booted(ostree)?;
1010+
let repo = &booted_ostree.repo();
1011+
let (_, host) = crate::status::get_status(booted_ostree)?;
10271012

10281013
let new_spec = {
10291014
let mut new_spec = host.spec.clone();
@@ -1061,7 +1046,7 @@ async fn switch(opts: SwitchOpts) -> Result<()> {
10611046

10621047
if !opts.retain {
10631048
// By default, we prune the previous ostree ref so it will go away after later upgrades
1064-
if let Some(booted_origin) = booted_deployment.origin() {
1049+
if let Some(booted_origin) = booted_ostree.deployment.origin() {
10651050
if let Some(ostree_ref) = booted_origin.optional_string("origin", "refspec")? {
10661051
let (remote, ostree_ref) =
10671052
ostree::parse_refspec(&ostree_ref).context("Failed to parse ostree ref")?;
@@ -1070,7 +1055,7 @@ async fn switch(opts: SwitchOpts) -> Result<()> {
10701055
}
10711056
}
10721057

1073-
let stateroot = booted_deployment.osname();
1058+
let stateroot = booted_ostree.stateroot();
10741059
let from = MergeState::from_stateroot(storage, &stateroot)?;
10751060
crate::deploy::stage(storage, from, &fetched, &new_spec, prog.clone()).await?;
10761061

@@ -1079,12 +1064,8 @@ async fn switch(opts: SwitchOpts) -> Result<()> {
10791064
if opts.soft_reboot.is_some() {
10801065
// At this point we have staged the deployment and the host definition has changed.
10811066
// We need the updated host status before we check if we can prepare the soft-reboot.
1082-
let booted_ostree = BootedOstree {
1083-
sysroot: ostree,
1084-
deployment: booted_deployment.clone(),
1085-
};
1086-
let updated_host = crate::status::get_status(&booted_ostree)?.1;
1087-
handle_staged_soft_reboot(&booted_ostree, opts.soft_reboot, &updated_host)?;
1067+
let updated_host = crate::status::get_status(booted_ostree)?.1;
1068+
handle_staged_soft_reboot(booted_ostree, opts.soft_reboot, &updated_host)?;
10881069
}
10891070

10901071
if opts.apply {
@@ -1094,36 +1075,85 @@ async fn switch(opts: SwitchOpts) -> Result<()> {
10941075
Ok(())
10951076
}
10961077

1097-
/// Implementation of the `bootc rollback` CLI command.
1098-
#[context("Rollback")]
1099-
async fn rollback(opts: &RollbackOpts) -> Result<()> {
1078+
/// Implementation of the `bootc switch` CLI command.
1079+
#[context("Switching")]
1080+
async fn switch(opts: SwitchOpts) -> Result<()> {
11001081
let storage = &get_storage().await?;
1101-
let ostree = storage.get_ostree()?;
1082+
match storage.kind()? {
1083+
BootedStorageKind::Ostree(booted_ostree) => {
1084+
// If we're doing an in-place mutation, we shortcut most of the rest of the work here
1085+
if opts.mutate_in_place {
1086+
let target = imgref_for_switch(&opts)?;
1087+
let deployid = {
1088+
// Clone to pass into helper thread
1089+
let target = target.clone();
1090+
let root =
1091+
cap_std::fs::Dir::open_ambient_dir("/", cap_std::ambient_authority())?;
1092+
tokio::task::spawn_blocking(move || {
1093+
crate::deploy::switch_origin_inplace(&root, &target)
1094+
})
1095+
.await??
1096+
};
1097+
println!("Updated {deployid} to pull from {target}");
1098+
return Ok(());
1099+
}
1100+
switch_ostree(opts, storage, &booted_ostree).await
1101+
}
1102+
BootedStorageKind::Composefs(_) => {
1103+
if opts.mutate_in_place {
1104+
anyhow::bail!("--mutate-in-place is not yet supported for composefs backend");
1105+
}
1106+
switch_composefs(opts).await
1107+
}
1108+
}
1109+
}
1110+
1111+
/// Implementation of the `bootc rollback` CLI command for ostree backend.
1112+
#[context("Rollback (ostree)")]
1113+
async fn rollback_ostree(
1114+
opts: &RollbackOpts,
1115+
storage: &Storage,
1116+
booted_ostree: &BootedOstree<'_>,
1117+
) -> Result<()> {
11021118
crate::deploy::rollback(storage).await?;
11031119

11041120
if opts.soft_reboot.is_some() {
11051121
// Get status of rollback deployment to check soft-reboot capability
1106-
let host = crate::status::get_status_require_booted(ostree)?.2;
1122+
let host = crate::status::get_status(booted_ostree)?.1;
11071123

11081124
handle_soft_reboot(
11091125
opts.soft_reboot,
11101126
host.status.rollback.as_ref(),
11111127
"rollback",
1112-
|| soft_reboot_rollback(ostree),
1128+
|| soft_reboot_rollback(booted_ostree),
11131129
)?;
11141130
}
11151131

11161132
Ok(())
11171133
}
11181134

1119-
/// Implementation of the `bootc edit` CLI command.
1120-
#[context("Editing spec")]
1121-
async fn edit(opts: EditOpts) -> Result<()> {
1135+
/// Implementation of the `bootc rollback` CLI command.
1136+
#[context("Rollback")]
1137+
async fn rollback(opts: &RollbackOpts) -> Result<()> {
11221138
let storage = &get_storage().await?;
1123-
let ostree = storage.get_ostree()?;
1124-
let repo = &ostree.repo();
1139+
match storage.kind()? {
1140+
BootedStorageKind::Ostree(booted_ostree) => {
1141+
rollback_ostree(opts, storage, &booted_ostree).await
1142+
}
1143+
BootedStorageKind::Composefs(_) => composefs_rollback().await,
1144+
}
1145+
}
1146+
1147+
/// Implementation of the `bootc edit` CLI command for ostree backend.
1148+
#[context("Editing spec (ostree)")]
1149+
async fn edit_ostree(
1150+
opts: EditOpts,
1151+
storage: &Storage,
1152+
booted_ostree: &BootedOstree<'_>,
1153+
) -> Result<()> {
1154+
let repo = &booted_ostree.repo();
1155+
let (_, host) = crate::status::get_status(booted_ostree)?;
11251156

1126-
let (booted_deployment, _deployments, host) = crate::status::get_status_require_booted(ostree)?;
11271157
let new_host: Host = if let Some(filename) = opts.filename {
11281158
let mut r = std::io::BufReader::new(std::fs::File::open(filename)?);
11291159
serde_yaml::from_reader(&mut r)?
@@ -1154,7 +1184,7 @@ async fn edit(opts: EditOpts) -> Result<()> {
11541184

11551185
// TODO gc old layers here
11561186

1157-
let stateroot = booted_deployment.osname();
1187+
let stateroot = booted_ostree.stateroot();
11581188
let from = MergeState::from_stateroot(storage, &stateroot)?;
11591189
crate::deploy::stage(storage, from, &fetched, &new_spec, prog.clone()).await?;
11601190

@@ -1163,6 +1193,20 @@ async fn edit(opts: EditOpts) -> Result<()> {
11631193
Ok(())
11641194
}
11651195

1196+
/// Implementation of the `bootc edit` CLI command.
1197+
#[context("Editing spec")]
1198+
async fn edit(opts: EditOpts) -> Result<()> {
1199+
let storage = &get_storage().await?;
1200+
match storage.kind()? {
1201+
BootedStorageKind::Ostree(booted_ostree) => {
1202+
edit_ostree(opts, storage, &booted_ostree).await
1203+
}
1204+
BootedStorageKind::Composefs(_) => {
1205+
anyhow::bail!("Edit is not yet supported for composefs backend")
1206+
}
1207+
}
1208+
}
1209+
11661210
/// Implementation of `bootc usroverlay`
11671211
async fn usroverlay() -> Result<()> {
11681212
// 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<()> {
12681312
}
12691313
}
12701314
}
1271-
Opt::Switch(opts) => {
1272-
let storage = &get_storage().await?;
1273-
match storage.kind()? {
1274-
BootedStorageKind::Ostree(_) => switch(opts).await,
1275-
BootedStorageKind::Composefs(_) => switch_composefs(opts).await,
1276-
}
1277-
}
1315+
Opt::Switch(opts) => switch(opts).await,
12781316
Opt::Rollback(opts) => {
1279-
let storage = &get_storage().await?;
1280-
match storage.kind()? {
1281-
BootedStorageKind::Ostree(_) => rollback(&opts).await?,
1282-
BootedStorageKind::Composefs(_) => composefs_rollback().await?,
1283-
}
1284-
1317+
rollback(&opts).await?;
12851318
if opts.apply {
12861319
crate::reboot::reboot()?;
12871320
}
1288-
12891321
Ok(())
12901322
}
12911323
Opt::Edit(opts) => edit(opts).await,

crates/lib/src/deploy.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -835,15 +835,15 @@ fn write_reboot_required(image: &str) -> Result<()> {
835835
pub(crate) async fn rollback(sysroot: &Storage) -> Result<()> {
836836
const ROLLBACK_JOURNAL_ID: &str = "26f3b1eb24464d12aa5e7b544a6b5468";
837837
let ostree = sysroot.get_ostree()?;
838-
let (booted_deployment, deployments, host) = crate::status::get_status_require_booted(ostree)?;
838+
let (booted_ostree, deployments, host) = crate::status::get_status_require_booted(ostree)?;
839839

840840
let new_spec = {
841841
let mut new_spec = host.spec.clone();
842842
new_spec.boot_order = new_spec.boot_order.swap();
843843
new_spec
844844
};
845845

846-
let repo = &ostree.repo();
846+
let repo = &booted_ostree.repo();
847847

848848
// Just to be sure
849849
host.spec.verify_transition(&new_spec)?;
@@ -882,16 +882,18 @@ pub(crate) async fn rollback(sysroot: &Storage) -> Result<()> {
882882
// SAFETY: If there's a rollback status, then there's a deployment
883883
let rollback_deployment = deployments.rollback.expect("rollback deployment");
884884
let new_deployments = if reverting {
885-
[booted_deployment, rollback_deployment]
885+
[booted_ostree.deployment, rollback_deployment]
886886
} else {
887-
[rollback_deployment, booted_deployment]
887+
[rollback_deployment, booted_ostree.deployment]
888888
};
889889
let new_deployments = new_deployments
890890
.into_iter()
891891
.chain(deployments.other)
892892
.collect::<Vec<_>>();
893893
tracing::debug!("Writing new deployments: {new_deployments:?}");
894-
ostree.write_deployments(&new_deployments, gio::Cancellable::NONE)?;
894+
booted_ostree
895+
.sysroot
896+
.write_deployments(&new_deployments, gio::Cancellable::NONE)?;
895897
if reverting {
896898
println!("Next boot: current deployment");
897899
} else {

crates/lib/src/install.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2265,7 +2265,7 @@ pub(crate) async fn install_reset(opts: InstallResetOpts) -> Result<()> {
22652265
let sysroot = &crate::cli::get_storage().await?;
22662266
let ostree = sysroot.get_ostree()?;
22672267
let repo = &ostree.repo();
2268-
let (booted_deployment, _deployments, host) = crate::status::get_status_require_booted(ostree)?;
2268+
let (booted_ostree, _deployments, host) = crate::status::get_status_require_booted(ostree)?;
22692269

22702270
let stateroots = list_stateroots(ostree)?;
22712271
let target_stateroot = if let Some(s) = opts.stateroot {
@@ -2276,7 +2276,7 @@ pub(crate) async fn install_reset(opts: InstallResetOpts) -> Result<()> {
22762276
r.name
22772277
};
22782278

2279-
let booted_stateroot = booted_deployment.osname();
2279+
let booted_stateroot = booted_ostree.stateroot();
22802280
assert!(booted_stateroot.as_str() != target_stateroot);
22812281
let (fetched, spec) = if let Some(target) = opts.target_opts.imageref()? {
22822282
let mut new_spec = host.spec;
@@ -2307,7 +2307,8 @@ pub(crate) async fn install_reset(opts: InstallResetOpts) -> Result<()> {
23072307
let root_kargs = if opts.no_root_kargs {
23082308
Vec::new()
23092309
} else {
2310-
let bootcfg = booted_deployment
2310+
let bootcfg = booted_ostree
2311+
.deployment
23112312
.bootconfig()
23122313
.ok_or_else(|| anyhow!("Missing bootcfg for booted deployment"))?;
23132314
if let Some(options) = bootcfg.get("options") {

crates/lib/src/status.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -259,14 +259,14 @@ impl BootEntry {
259259
/// A variant of [`get_status`] that requires a booted deployment.
260260
pub(crate) fn get_status_require_booted(
261261
sysroot: &SysrootLock,
262-
) -> Result<(ostree::Deployment, Deployments, Host)> {
262+
) -> Result<(crate::store::BootedOstree<'_>, Deployments, Host)> {
263263
let booted_deployment = sysroot.require_booted_deployment()?;
264264
let booted_ostree = crate::store::BootedOstree {
265265
sysroot,
266-
deployment: booted_deployment.clone(),
266+
deployment: booted_deployment,
267267
};
268268
let (deployments, host) = get_status(&booted_ostree)?;
269-
Ok((booted_deployment, deployments, host))
269+
Ok((booted_ostree, deployments, host))
270270
}
271271

272272
/// Gather the ostree deployment objects, but also extract metadata from them into

0 commit comments

Comments
 (0)