Skip to content

Commit 6adf1db

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 835b4c5 commit 6adf1db

File tree

4 files changed

+103
-73
lines changed

4 files changed

+103
-73
lines changed

crates/lib/src/cli.rs

Lines changed: 89 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.
@@ -1005,34 +1005,19 @@ pub(crate) fn imgref_for_switch(opts: &SwitchOpts) -> Result<ImageReference> {
10051005
return Ok(target);
10061006
}
10071007

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

1032-
let storage = &get_storage().await?;
1033-
let ostree = storage.get_ostree()?;
1034-
let repo = &ostree.repo();
1035-
let (booted_deployment, _deployments, host) = crate::status::get_status_require_booted(ostree)?;
1019+
let repo = &booted_ostree.repo();
1020+
let (_, host) = crate::status::get_status(booted_ostree)?;
10361021

10371022
let new_spec = {
10381023
let mut new_spec = host.spec.clone();
@@ -1070,7 +1055,7 @@ async fn switch(opts: SwitchOpts) -> Result<()> {
10701055

10711056
if !opts.retain {
10721057
// By default, we prune the previous ostree ref so it will go away after later upgrades
1073-
if let Some(booted_origin) = booted_deployment.origin() {
1058+
if let Some(booted_origin) = booted_ostree.deployment.origin() {
10741059
if let Some(ostree_ref) = booted_origin.optional_string("origin", "refspec")? {
10751060
let (remote, ostree_ref) =
10761061
ostree::parse_refspec(&ostree_ref).context("Failed to parse ostree ref")?;
@@ -1079,7 +1064,7 @@ async fn switch(opts: SwitchOpts) -> Result<()> {
10791064
}
10801065
}
10811066

1082-
let stateroot = booted_deployment.osname();
1067+
let stateroot = booted_ostree.stateroot();
10831068
let from = MergeState::from_stateroot(storage, &stateroot)?;
10841069
crate::deploy::stage(storage, from, &fetched, &new_spec, prog.clone()).await?;
10851070

@@ -1088,12 +1073,8 @@ async fn switch(opts: SwitchOpts) -> Result<()> {
10881073
if opts.soft_reboot.is_some() {
10891074
// At this point we have staged the deployment and the host definition has changed.
10901075
// We need the updated host status before we check if we can prepare the soft-reboot.
1091-
let booted_ostree = BootedOstree {
1092-
sysroot: ostree,
1093-
deployment: booted_deployment.clone(),
1094-
};
1095-
let updated_host = crate::status::get_status(&booted_ostree)?.1;
1096-
handle_staged_soft_reboot(&booted_ostree, opts.soft_reboot, &updated_host)?;
1076+
let updated_host = crate::status::get_status(booted_ostree)?.1;
1077+
handle_staged_soft_reboot(booted_ostree, opts.soft_reboot, &updated_host)?;
10971078
}
10981079

10991080
if opts.apply {
@@ -1103,36 +1084,80 @@ async fn switch(opts: SwitchOpts) -> Result<()> {
11031084
Ok(())
11041085
}
11051086

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

11131124
if opts.soft_reboot.is_some() {
11141125
// Get status of rollback deployment to check soft-reboot capability
1115-
let host = crate::status::get_status_require_booted(ostree)?.2;
1126+
let host = crate::status::get_status(booted_ostree)?.1;
11161127

11171128
handle_soft_reboot(
11181129
opts.soft_reboot,
11191130
host.status.rollback.as_ref(),
11201131
"rollback",
1121-
|| soft_reboot_rollback(ostree),
1132+
|| soft_reboot_rollback(booted_ostree),
11221133
)?;
11231134
}
11241135

11251136
Ok(())
11261137
}
11271138

1128-
/// Implementation of the `bootc edit` CLI command.
1129-
#[context("Editing spec")]
1130-
async fn edit(opts: EditOpts) -> Result<()> {
1139+
/// Implementation of the `bootc rollback` CLI command.
1140+
#[context("Rollback")]
1141+
async fn rollback(opts: &RollbackOpts) -> Result<()> {
11311142
let storage = &get_storage().await?;
1132-
let ostree = storage.get_ostree()?;
1133-
let repo = &ostree.repo();
1143+
match storage.kind()? {
1144+
BootedStorageKind::Ostree(booted_ostree) => {
1145+
rollback_ostree(opts, storage, &booted_ostree).await
1146+
}
1147+
BootedStorageKind::Composefs(_) => composefs_rollback().await,
1148+
}
1149+
}
1150+
1151+
/// Implementation of the `bootc edit` CLI command for ostree backend.
1152+
#[context("Editing spec (ostree)")]
1153+
async fn edit_ostree(
1154+
opts: EditOpts,
1155+
storage: &Storage,
1156+
booted_ostree: &BootedOstree<'_>,
1157+
) -> Result<()> {
1158+
let repo = &booted_ostree.repo();
1159+
let (_, host) = crate::status::get_status(booted_ostree)?;
11341160

1135-
let (booted_deployment, _deployments, host) = crate::status::get_status_require_booted(ostree)?;
11361161
let new_host: Host = if let Some(filename) = opts.filename {
11371162
let mut r = std::io::BufReader::new(std::fs::File::open(filename)?);
11381163
serde_yaml::from_reader(&mut r)?
@@ -1163,7 +1188,7 @@ async fn edit(opts: EditOpts) -> Result<()> {
11631188

11641189
// TODO gc old layers here
11651190

1166-
let stateroot = booted_deployment.osname();
1191+
let stateroot = booted_ostree.stateroot();
11671192
let from = MergeState::from_stateroot(storage, &stateroot)?;
11681193
crate::deploy::stage(storage, from, &fetched, &new_spec, prog.clone()).await?;
11691194

@@ -1172,6 +1197,20 @@ async fn edit(opts: EditOpts) -> Result<()> {
11721197
Ok(())
11731198
}
11741199

1200+
/// Implementation of the `bootc edit` CLI command.
1201+
#[context("Editing spec")]
1202+
async fn edit(opts: EditOpts) -> Result<()> {
1203+
let storage = &get_storage().await?;
1204+
match storage.kind()? {
1205+
BootedStorageKind::Ostree(booted_ostree) => {
1206+
edit_ostree(opts, storage, &booted_ostree).await
1207+
}
1208+
BootedStorageKind::Composefs(_) => {
1209+
anyhow::bail!("Edit is not yet supported for composefs backend")
1210+
}
1211+
}
1212+
}
1213+
11751214
/// Implementation of `bootc usroverlay`
11761215
async fn usroverlay() -> Result<()> {
11771216
// This is just a pass-through today. At some point we may make this a libostree API
@@ -1277,24 +1316,12 @@ async fn run_from_opt(opt: Opt) -> Result<()> {
12771316
}
12781317
}
12791318
}
1280-
Opt::Switch(opts) => {
1281-
let storage = &get_storage().await?;
1282-
match storage.kind()? {
1283-
BootedStorageKind::Ostree(_) => switch(opts).await,
1284-
BootedStorageKind::Composefs(_) => switch_composefs(opts).await,
1285-
}
1286-
}
1319+
Opt::Switch(opts) => switch(opts).await,
12871320
Opt::Rollback(opts) => {
1288-
let storage = &get_storage().await?;
1289-
match storage.kind()? {
1290-
BootedStorageKind::Ostree(_) => rollback(&opts).await?,
1291-
BootedStorageKind::Composefs(_) => composefs_rollback().await?,
1292-
}
1293-
1321+
rollback(&opts).await?;
12941322
if opts.apply {
12951323
crate::reboot::reboot()?;
12961324
}
1297-
12981325
Ok(())
12991326
}
13001327
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)