Skip to content

Commit 82db53e

Browse files
committed
fs: Allow reading filesystems without storing objects in repository
For the `cfsctl oci compute-id` case we were making full copies of the objects in a temporary repository - a completely unnecessary performance hit. Further that path expects `O_TMPFILE` which isn't supported everywhere. (In theory we should support non-O_TMPFILE filesystems too) Fixes: bootc-dev/bootc#1977 Assisted-by: OpenCode (Claude Opus 4) Signed-off-by: Colin Walters <walters@verbum.org>
1 parent 2d93817 commit 82db53e

4 files changed

Lines changed: 156 additions & 23 deletions

File tree

crates/cfsctl/src/lib.rs

Lines changed: 55 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,7 @@ use anyhow::{Context as _, Result};
3636
use clap::{Parser, Subcommand, ValueEnum};
3737
#[cfg(feature = "oci")]
3838
use comfy_table::{Table, presets::UTF8_FULL};
39-
use rustix::fs::{Mode, OFlags};
40-
41-
use rustix::fs::CWD;
39+
use rustix::fs::{CWD, Mode, OFlags};
4240
use serde::Serialize;
4341

4442
use composefs_boot::BootOps;
@@ -102,6 +100,11 @@ pub struct App {
102100
#[clap(long)]
103101
require_verity: bool,
104102

103+
/// Don't open a repository. Only valid for commands that don't need one
104+
/// (compute-id, create-dumpfile).
105+
#[clap(long)]
106+
pub no_repo: bool,
107+
105108
#[clap(subcommand)]
106109
cmd: Command,
107110
}
@@ -410,14 +413,15 @@ enum Command {
410413
/// optional reference name for the image, use as 'ref/<name>' elsewhere
411414
image_name: Option<String>,
412415
},
413-
/// Read rootfs located at a path, add all files to the repo, then compute the composefs image object id of the rootfs.
414-
/// Note that this does not create or commit the composefs image itself.
416+
/// Read rootfs located at a path and compute the composefs image object id of the rootfs.
417+
/// Note that this does not create or commit the composefs image itself, and does not
418+
/// store any file objects in the repository.
415419
ComputeId {
416420
#[clap(flatten)]
417421
fs_opts: FsReadOptions,
418422
},
419-
/// Read rootfs located at a path, add all files to the repo, then dump full content of the rootfs to a composefs dumpfile
420-
/// and write to stdout.
423+
/// Read rootfs located at a path and dump full content of the rootfs to a composefs dumpfile,
424+
/// writing to stdout. Does not store any file objects in the repository.
421425
CreateDumpfile {
422426
#[clap(flatten)]
423427
fs_opts: FsReadOptions,
@@ -569,6 +573,14 @@ pub async fn run_app(args: App) -> Result<()> {
569573
);
570574
}
571575

576+
if args.no_repo {
577+
let effective_hash = args.hash.unwrap_or(HashType::Sha512);
578+
return match effective_hash {
579+
HashType::Sha256 => run_cmd_without_repo::<Sha256HashValue>(args),
580+
HashType::Sha512 => run_cmd_without_repo::<Sha512HashValue>(args),
581+
};
582+
}
583+
572584
let repo_path = resolve_repo_path(&args)?;
573585
let effective_hash = resolve_hash_type(&repo_path, args.hash)?;
574586

@@ -704,15 +716,25 @@ fn load_filesystem_from_oci_image<ObjectID: FsVerityHashValue>(
704716

705717
fn load_filesystem_from_ondisk_fs<ObjectID: FsVerityHashValue>(
706718
fs_opts: &FsReadOptions,
707-
repo: &Repository<ObjectID>,
719+
repo: Option<&Repository<ObjectID>>,
708720
) -> Result<FileSystem<RegularFile<ObjectID>>> {
709721
let mut fs = if fs_opts.no_propagate_usr_to_root {
710-
composefs::fs::read_filesystem(CWD, &fs_opts.path, Some(repo))?
722+
composefs::fs::read_filesystem(CWD, &fs_opts.path, repo)?
711723
} else {
712-
composefs::fs::read_container_root(CWD, &fs_opts.path, Some(repo))?
724+
composefs::fs::read_container_root(CWD, &fs_opts.path, repo)?
713725
};
714726
if fs_opts.bootable {
715-
fs.transform_for_boot(repo)?;
727+
if let Some(repo) = repo {
728+
fs.transform_for_boot(repo)?;
729+
} else {
730+
let rootfd = rustix::fs::openat(
731+
CWD,
732+
&fs_opts.path,
733+
OFlags::RDONLY | OFlags::DIRECTORY | OFlags::CLOEXEC,
734+
Mode::empty(),
735+
)?;
736+
fs.transform_for_boot_from_dir(rootfd)?;
737+
}
716738
}
717739
Ok(fs)
718740
}
@@ -774,6 +796,25 @@ fn dump_file_impl(
774796
Ok(())
775797
}
776798

799+
/// Run commands that don't require a repository.
800+
pub fn run_cmd_without_repo<ObjectID: FsVerityHashValue>(args: App) -> Result<()> {
801+
match args.cmd {
802+
Command::ComputeId { fs_opts } => {
803+
let fs = load_filesystem_from_ondisk_fs::<ObjectID>(&fs_opts, None)?;
804+
let id = fs.compute_image_id();
805+
println!("{}", id.to_hex());
806+
}
807+
Command::CreateDumpfile { fs_opts } => {
808+
let fs = load_filesystem_from_ondisk_fs::<ObjectID>(&fs_opts, None)?;
809+
fs.print_dumpfile()?;
810+
}
811+
_ => {
812+
anyhow::bail!("--no-repo is only supported for compute-id and create-dumpfile");
813+
}
814+
}
815+
Ok(())
816+
}
817+
777818
/// Run with cmd
778819
pub async fn run_cmd_with_repo<ObjectID>(repo: Repository<ObjectID>, args: App) -> Result<()>
779820
where
@@ -1047,17 +1088,17 @@ where
10471088
fs_opts,
10481089
ref image_name,
10491090
} => {
1050-
let fs = load_filesystem_from_ondisk_fs(&fs_opts, &repo)?;
1091+
let fs = load_filesystem_from_ondisk_fs(&fs_opts, Some(&repo))?;
10511092
let id = fs.commit_image(&repo, image_name.as_deref())?;
10521093
println!("{}", id.to_id());
10531094
}
10541095
Command::ComputeId { fs_opts } => {
1055-
let fs = load_filesystem_from_ondisk_fs(&fs_opts, &repo)?;
1096+
let fs = load_filesystem_from_ondisk_fs(&fs_opts, Some(&repo))?;
10561097
let id = fs.compute_image_id();
10571098
println!("{}", id.to_hex());
10581099
}
10591100
Command::CreateDumpfile { fs_opts } => {
1060-
let fs = load_filesystem_from_ondisk_fs(&fs_opts, &repo)?;
1101+
let fs = load_filesystem_from_ondisk_fs::<ObjectID>(&fs_opts, None)?;
10611102
fs.print_dumpfile()?;
10621103
}
10631104
Command::Mount { name, mountpoint } => {

crates/composefs-boot/src/selabel.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use fn_error_context::context;
1919
use regex_automata::{Anchored, Input, hybrid::dfa, util::syntax};
2020
use rustix::{
2121
fd::AsFd,
22-
fs::{openat, Mode, OFlags},
22+
fs::{Mode, OFlags, openat},
2323
io::Errno,
2424
};
2525

crates/composefs/src/fs.rs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -162,8 +162,8 @@ pub fn write_to_path<ObjectID: FsVerityHashValue>(
162162

163163
/// Helper for reading filesystem trees from disk into composefs representation.
164164
///
165-
/// Tracks hardlinks via inode numbers and handles integration with repositories
166-
/// for storing large file content.
165+
/// Tracks hardlinks via inode numbers and optionally stores large file objects
166+
/// in a repository.
167167
#[derive(Debug)]
168168
pub struct FilesystemReader<'repo, ObjectID: FsVerityHashValue> {
169169
repo: Option<&'repo Repository<ObjectID>>,
@@ -295,8 +295,7 @@ impl<ObjectID: FsVerityHashValue> FilesystemReader<'_, ObjectID> {
295295
/// Reads a directory from disk into composefs representation.
296296
///
297297
/// Recursively reads directory contents, tracking hardlinks and optionally
298-
/// reading the directory's own metadata. Large files are stored in the repository
299-
/// if one was provided.
298+
/// storing large file objects in the repository.
300299
#[context("Reading directory {}", name.to_string_lossy())]
301300
fn read_directory(&mut self, dirfd: impl AsFd, name: &OsStr) -> Result<Directory<ObjectID>> {
302301
let fd = openat(
@@ -341,9 +340,12 @@ impl<ObjectID: FsVerityHashValue> FilesystemReader<'_, ObjectID> {
341340
}
342341
}
343342

344-
/// Load a filesystem tree from the given path. A repository may
345-
/// be provided; if it is, then all files found in the filesystem
346-
/// are copied in.
343+
/// Load a filesystem tree from the given path.
344+
///
345+
/// If a repository is provided, all files found in the filesystem are
346+
/// stored in it. If `None`, fsverity digests are computed in memory
347+
/// without writing to disk — suitable for computing image IDs and
348+
/// dumpfiles without requiring O_TMPFILE support.
347349
#[context("Reading filesystem from {}", path.display())]
348350
pub fn read_filesystem<ObjectID: FsVerityHashValue>(
349351
dirfd: impl AsFd,
@@ -373,7 +375,7 @@ pub fn read_filesystem<ObjectID: FsVerityHashValue>(
373375
/// use composefs::fs::{read_filesystem_filtered, CONTAINER_XATTR_ALLOWLIST};
374376
///
375377
/// // Filter to only allow security.capability
376-
/// let fs = read_filesystem_filtered(dirfd, path, repo, |name| {
378+
/// let fs = read_filesystem_filtered(dirfd, path, None, |name| {
377379
/// name.as_encoded_bytes() == b"security.capability"
378380
/// })?;
379381
/// ```

crates/integration-tests/src/tests/cli.rs

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1079,6 +1079,96 @@ fn test_oci_fsck_single_image() -> Result<()> {
10791079
}
10801080
integration_test!(test_oci_fsck_single_image);
10811081

1082+
fn test_compute_id_no_repo() -> Result<()> {
1083+
let sh = Shell::new()?;
1084+
let cfsctl = cfsctl()?;
1085+
let fixture_dir = tempfile::tempdir()?;
1086+
let rootfs = create_test_rootfs(fixture_dir.path())?;
1087+
1088+
let output = cmd!(
1089+
sh,
1090+
"{cfsctl} --no-repo compute-id --no-propagate-usr-to-root {rootfs}"
1091+
)
1092+
.read()?;
1093+
assert!(
1094+
!output.trim().is_empty(),
1095+
"expected image ID output, got nothing"
1096+
);
1097+
// Should be a valid hex digest
1098+
assert!(
1099+
output.trim().chars().all(|c| c.is_ascii_hexdigit()),
1100+
"expected hex digest, got: {output}"
1101+
);
1102+
Ok(())
1103+
}
1104+
integration_test!(test_compute_id_no_repo);
1105+
1106+
fn test_compute_id_no_repo_matches_repo() -> Result<()> {
1107+
let sh = Shell::new()?;
1108+
let cfsctl = cfsctl()?;
1109+
let fixture_dir = tempfile::tempdir()?;
1110+
let rootfs = create_test_rootfs(fixture_dir.path())?;
1111+
let repo_dir = tempfile::tempdir()?;
1112+
let repo = repo_dir.path();
1113+
1114+
let id_no_repo = cmd!(
1115+
sh,
1116+
"{cfsctl} --no-repo compute-id --no-propagate-usr-to-root {rootfs}"
1117+
)
1118+
.read()?;
1119+
let id_with_repo = cmd!(
1120+
sh,
1121+
"{cfsctl} --insecure --repo {repo} compute-id --no-propagate-usr-to-root {rootfs}"
1122+
)
1123+
.read()?;
1124+
assert_eq!(
1125+
id_no_repo.trim(),
1126+
id_with_repo.trim(),
1127+
"--no-repo and --repo should produce the same digest"
1128+
);
1129+
Ok(())
1130+
}
1131+
integration_test!(test_compute_id_no_repo_matches_repo);
1132+
1133+
fn test_create_dumpfile_no_repo() -> Result<()> {
1134+
let sh = Shell::new()?;
1135+
let cfsctl = cfsctl()?;
1136+
let fixture_dir = tempfile::tempdir()?;
1137+
let rootfs = create_test_rootfs(fixture_dir.path())?;
1138+
1139+
let output = cmd!(
1140+
sh,
1141+
"{cfsctl} --no-repo create-dumpfile --no-propagate-usr-to-root {rootfs}"
1142+
)
1143+
.read()?;
1144+
assert!(
1145+
output.contains("/usr/bin/hello"),
1146+
"expected dumpfile to contain /usr/bin/hello, got: {output}"
1147+
);
1148+
assert!(
1149+
output.contains("/etc/hostname"),
1150+
"expected dumpfile to contain /etc/hostname, got: {output}"
1151+
);
1152+
Ok(())
1153+
}
1154+
integration_test!(test_create_dumpfile_no_repo);
1155+
1156+
fn test_no_repo_rejects_create_image() -> Result<()> {
1157+
let sh = Shell::new()?;
1158+
let cfsctl = cfsctl()?;
1159+
let fixture_dir = tempfile::tempdir()?;
1160+
let rootfs = create_test_rootfs(fixture_dir.path())?;
1161+
1162+
cmd!(
1163+
sh,
1164+
"{cfsctl} --no-repo create-image --no-propagate-usr-to-root {rootfs}"
1165+
)
1166+
.run()
1167+
.expect_err("create-image should fail with --no-repo");
1168+
Ok(())
1169+
}
1170+
integration_test!(test_no_repo_rejects_create_image);
1171+
10821172
fn test_fsck_detects_broken_image_ref() -> Result<()> {
10831173
use cap_std_ext::cap_std;
10841174

0 commit comments

Comments
 (0)