Skip to content

Commit 5f41d0d

Browse files
committed
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 <[email protected]>
1 parent eb23dc6 commit 5f41d0d

File tree

10 files changed

+49
-46
lines changed

10 files changed

+49
-46
lines changed

crates/lib/src/bootc_composefs/boot.rs

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use std::path::Path;
55

66
use anyhow::{anyhow, Context, Result};
77
use bootc_blockdev::find_parent_devices;
8-
use bootc_mount::inspect_filesystem;
8+
use bootc_mount::inspect_filesystem_of_dir;
99
use bootc_mount::tempmount::TempMount;
1010
use camino::{Utf8Path, Utf8PathBuf};
1111
use cap_std_ext::{
@@ -37,7 +37,10 @@ use crate::composefs_consts::{TYPE1_ENT_PATH, TYPE1_ENT_PATH_STAGED};
3737
use crate::parsers::bls_config::{BLSConfig, BLSConfigType};
3838
use crate::parsers::grub_menuconfig::MenuEntry;
3939
use crate::task::Task;
40-
use crate::{bootc_composefs::repo::open_composefs_repo, store::ComposefsFilesystem};
40+
use crate::{
41+
bootc_composefs::repo::open_composefs_repo,
42+
store::{ComposefsFilesystem, Storage},
43+
};
4144
use crate::{
4245
bootc_composefs::state::{get_booted_bls, write_composefs_state},
4346
bootloader::esp_in,
@@ -76,7 +79,7 @@ pub(crate) enum BootSetupType<'a> {
7679
/// For initial setup, i.e. install to-disk
7780
Setup((&'a RootSetup, &'a State, &'a ComposefsFilesystem)),
7881
/// For `bootc upgrade`
79-
Upgrade((&'a ComposefsFilesystem, &'a Host)),
82+
Upgrade((&'a Storage, &'a ComposefsFilesystem, &'a Host)),
8083
}
8184

8285
#[derive(
@@ -168,14 +171,12 @@ pub fn mount_esp(device: &str) -> Result<TempMount> {
168171
TempMount::mount_dev(device, "vfat", flags, Some(c"fmask=0177,dmask=0077"))
169172
}
170173

171-
pub fn get_sysroot_parent_dev() -> Result<String> {
172-
let sysroot = Utf8PathBuf::from("/sysroot");
173-
174-
let fsinfo = inspect_filesystem(&sysroot)?;
174+
pub fn get_sysroot_parent_dev(physical_root: &Dir) -> Result<String> {
175+
let fsinfo = inspect_filesystem_of_dir(physical_root)?;
175176
let parent_devices = find_parent_devices(&fsinfo.source)?;
176177

177178
let Some(parent) = parent_devices.into_iter().next() else {
178-
anyhow::bail!("Could not find parent device for mountpoint /sysroot");
179+
anyhow::bail!("Could not find parent device of system root");
179180
};
180181

181182
Ok(parent)
@@ -399,8 +400,8 @@ pub(crate) fn setup_composefs_bls_boot(
399400
)
400401
}
401402

402-
BootSetupType::Upgrade((fs, host)) => {
403-
let sysroot_parent = get_sysroot_parent_dev()?;
403+
BootSetupType::Upgrade((storage, fs, host)) => {
404+
let sysroot_parent = get_sysroot_parent_dev(&storage.physical_root)?;
404405
let bootloader = host.require_composefs_booted()?.bootloader.clone();
405406

406407
(
@@ -872,9 +873,9 @@ pub(crate) fn setup_composefs_uki_boot(
872873
)
873874
}
874875

875-
BootSetupType::Upgrade((_, host)) => {
876-
let sysroot = Utf8PathBuf::from("/sysroot");
877-
let sysroot_parent = get_sysroot_parent_dev()?;
876+
BootSetupType::Upgrade((storage, _, host)) => {
877+
let sysroot = Utf8PathBuf::from("/sysroot"); // Still needed for root_path
878+
let sysroot_parent = get_sysroot_parent_dev(&storage.physical_root)?;
878879
let bootloader = host.require_composefs_booted()?.bootloader.clone();
879880

880881
(

crates/lib/src/bootc_composefs/delete.rs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
11
use std::{collections::HashSet, io::Write, path::Path};
22

33
use anyhow::{Context, Result};
4-
use cap_std_ext::{
5-
cap_std::{ambient_authority, fs::Dir},
6-
dirext::CapStdExtDirExt,
7-
};
4+
use cap_std_ext::{cap_std::fs::Dir, dirext::CapStdExtDirExt};
85
use composefs::fsverity::Sha512HashValue;
96
use composefs_boot::bootloader::{EFI_ADDON_DIR_EXT, EFI_EXT};
107

@@ -216,17 +213,20 @@ fn remove_grub_menucfg_entry(id: &str, boot_dir: &Dir, deleting_staged: bool) ->
216213
}
217214

218215
#[fn_error_context::context("Deleting boot entries for deployment {}", deployment.deployment.verity)]
219-
fn delete_depl_boot_entries(deployment: &DeploymentEntry, deleting_staged: bool) -> Result<()> {
216+
fn delete_depl_boot_entries(
217+
deployment: &DeploymentEntry,
218+
physical_root: &Dir,
219+
deleting_staged: bool,
220+
) -> Result<()> {
220221
match deployment.deployment.bootloader {
221222
Bootloader::Grub => {
222-
let boot_dir = Dir::open_ambient_dir("/sysroot/boot", ambient_authority())
223-
.context("Opening boot dir")?;
223+
let boot_dir = physical_root.open_dir("boot").context("Opening boot dir")?;
224224

225225
match deployment.deployment.boot_type {
226226
BootType::Bls => delete_type1_entry(deployment, &boot_dir, deleting_staged),
227227

228228
BootType::Uki => {
229-
let device = get_sysroot_parent_dev()?;
229+
let device = get_sysroot_parent_dev(physical_root)?;
230230
let (esp_part, ..) = get_esp_partition(&device)?;
231231
let esp_mount = mount_esp(&esp_part)?;
232232

@@ -242,7 +242,7 @@ fn delete_depl_boot_entries(deployment: &DeploymentEntry, deleting_staged: bool)
242242
}
243243

244244
Bootloader::Systemd => {
245-
let device = get_sysroot_parent_dev()?;
245+
let device = get_sysroot_parent_dev(physical_root)?;
246246
let (esp_part, ..) = get_esp_partition(&device)?;
247247

248248
let esp_mount = mount_esp(&esp_part)?;
@@ -362,7 +362,7 @@ pub(crate) async fn delete_composefs_deployment(
362362

363363
tracing::info!("Deleting {kind}deployment '{deployment_id}'");
364364

365-
delete_depl_boot_entries(&depl_to_del, deleting_staged)?;
365+
delete_depl_boot_entries(&depl_to_del, &storage.physical_root, deleting_staged)?;
366366

367367
composefs_gc(storage, booted_cfs).await?;
368368

crates/lib/src/bootc_composefs/finalize.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ pub(crate) async fn composefs_backend_finalize(
8686
// Unmount EROFS
8787
drop(erofs_tmp_mnt);
8888

89-
let sysroot_parent = get_sysroot_parent_dev()?;
89+
let sysroot_parent = get_sysroot_parent_dev(&storage.physical_root)?;
9090
// NOTE: Assumption here that ESP will always be present
9191
let (esp_part, ..) = get_esp_partition(&sysroot_parent)?;
9292

crates/lib/src/bootc_composefs/gc.rs

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,7 @@
55
//! - We delete bootloader + image but fail to delete the state/unrefenced objects etc
66
77
use anyhow::{Context, Result};
8-
use cap_std_ext::{
9-
cap_std::{ambient_authority, fs::Dir},
10-
dirext::CapStdExtDirExt,
11-
};
8+
use cap_std_ext::{cap_std::fs::Dir, dirext::CapStdExtDirExt};
129
use composefs::fsverity::{FsVerityHashValue, Sha512HashValue};
1310

1411
use crate::{
@@ -47,13 +44,12 @@ fn list_erofs_images(sysroot: &Dir) -> Result<Vec<String>> {
4744
/// # Returns
4845
/// The fsverity of EROFS images corresponding to boot entries
4946
#[fn_error_context::context("Listing bootloader entries")]
50-
fn list_bootloader_entries() -> Result<Vec<String>> {
47+
fn list_bootloader_entries(physical_root: &Dir) -> Result<Vec<String>> {
5148
let bootloader = get_bootloader()?;
5249

5350
let entries = match bootloader {
5451
Bootloader::Grub => {
55-
let boot_dir = Dir::open_ambient_dir("/sysroot/boot", ambient_authority())
56-
.context("Opening boot dir")?;
52+
let boot_dir = physical_root.open_dir("boot").context("Opening boot dir")?;
5753

5854
// Grub entries are always in boot
5955
let grub_dir = boot_dir.open_dir("grub2").context("Opening grub dir")?;
@@ -79,7 +75,7 @@ fn list_bootloader_entries() -> Result<Vec<String>> {
7975
}
8076

8177
Bootloader::Systemd => {
82-
let device = get_sysroot_parent_dev()?;
78+
let device = get_sysroot_parent_dev(physical_root)?;
8379
let (esp_part, ..) = get_esp_partition(&device)?;
8480
let esp_mount = mount_esp(&esp_part)?;
8581

@@ -179,7 +175,7 @@ pub(crate) async fn composefs_gc(storage: &Storage, booted_cfs: &BootedComposefs
179175

180176
let sysroot = &storage.physical_root;
181177

182-
let bootloader_entries = list_bootloader_entries()?;
178+
let bootloader_entries = list_bootloader_entries(&storage.physical_root)?;
183179
let images = list_erofs_images(&sysroot)?;
184180

185181
// Collect the deployments that have an image but no bootloader entry

crates/lib/src/bootc_composefs/rollback.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ pub(crate) async fn composefs_rollback(
215215
}
216216

217217
Bootloader::Systemd => {
218-
let parent = get_sysroot_parent_dev()?;
218+
let parent = get_sysroot_parent_dev(&storage.physical_root)?;
219219
let (esp_part, ..) = get_esp_partition(&parent)?;
220220
let esp_mount = mount_esp(&esp_part)?;
221221

crates/lib/src/bootc_composefs/status.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,7 @@ pub(crate) async fn composefs_deployment_status_from(
355355
//
356356
// See: https://uapi-group.org/specifications/specs/boot_loader_specification/#mount-points
357357
Bootloader::Systemd => {
358-
let parent = get_sysroot_parent_dev()?;
358+
let parent = get_sysroot_parent_dev(sysroot)?;
359359
let (esp_part, ..) = get_esp_partition(&parent)?;
360360

361361
let esp_mount = mount_esp(&esp_part)?;

crates/lib/src/bootc_composefs/switch.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,15 +57,18 @@ pub(crate) async fn switch_composefs(
5757
match boot_type {
5858
BootType::Bls => {
5959
boot_digest = Some(setup_composefs_bls_boot(
60-
BootSetupType::Upgrade((&fs, &host)),
60+
BootSetupType::Upgrade((storage, &fs, &host)),
6161
repo,
6262
&id,
6363
entry,
6464
)?)
6565
}
66-
BootType::Uki => {
67-
setup_composefs_uki_boot(BootSetupType::Upgrade((&fs, &host)), repo, &id, entries)?
68-
}
66+
BootType::Uki => setup_composefs_uki_boot(
67+
BootSetupType::Upgrade((storage, &fs, &host)),
68+
repo,
69+
&id,
70+
entries,
71+
)?,
6972
};
7073

7174
// TODO: Remove this hardcoded path when write_composefs_state accepts a Dir

crates/lib/src/bootc_composefs/update.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -160,16 +160,19 @@ pub(crate) async fn upgrade_composefs(
160160
match boot_type {
161161
BootType::Bls => {
162162
boot_digest = Some(setup_composefs_bls_boot(
163-
BootSetupType::Upgrade((&fs, &host)),
163+
BootSetupType::Upgrade((storage, &fs, &host)),
164164
repo,
165165
&id,
166166
entry,
167167
)?)
168168
}
169169

170-
BootType::Uki => {
171-
setup_composefs_uki_boot(BootSetupType::Upgrade((&fs, &host)), repo, &id, entries)?
172-
}
170+
BootType::Uki => setup_composefs_uki_boot(
171+
BootSetupType::Upgrade((storage, &fs, &host)),
172+
repo,
173+
&id,
174+
entries,
175+
)?,
173176
};
174177

175178
write_composefs_state(

crates/system-reinstall-bootc/src/btrfs.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use fn_error_context::context;
44

55
#[context("check_root_siblings")]
66
pub(crate) fn check_root_siblings() -> Result<Vec<String>> {
7-
let mounts = bootc_mount::run_findmnt(&[], None)?;
7+
let mounts = bootc_mount::run_findmnt(&[], None, None)?;
88
let problem_filesystems: Vec<String> = mounts
99
.filesystems
1010
.iter()

crates/system-reinstall-bootc/src/lvm.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ pub(crate) fn check_root_siblings() -> Result<Vec<String>> {
5757
let siblings: Vec<String> = all_volumes
5858
.iter()
5959
.filter(|lv| {
60-
let mount = run_findmnt(&["-S", &lv.lv_path], None).log_err_default();
60+
let mount = run_findmnt(&["-S", &lv.lv_path], None, None).log_err_default();
6161
if let Some(fs) = mount.filesystems.first() {
6262
&fs.target == "/"
6363
} else {
@@ -66,7 +66,7 @@ pub(crate) fn check_root_siblings() -> Result<Vec<String>> {
6666
})
6767
.flat_map(|root_lv| parse_volumes(Some(root_lv.vg_name.as_str())).unwrap_or_default())
6868
.try_fold(Vec::new(), |mut acc, r| -> anyhow::Result<_> {
69-
let mount = run_findmnt(&["-S", &r.lv_path], None).log_err_default();
69+
let mount = run_findmnt(&["-S", &r.lv_path], None, None).log_err_default();
7070
let mount_path = if let Some(fs) = mount.filesystems.first() {
7171
&fs.target
7272
} else {

0 commit comments

Comments
 (0)