Skip to content

Commit 12342b5

Browse files
committed
Clean up install options + metadata
This makes it easier to handle caching as we add new things to install opts (though that shouldn't happen often). Signed-off-by: Colin Walters <[email protected]>
1 parent 74ed50f commit 12342b5

File tree

3 files changed

+92
-63
lines changed

3 files changed

+92
-63
lines changed

crates/kit/src/cache_metadata.rs

Lines changed: 69 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
//! - A SHA256 hash of all build inputs for cache validation
99
//! - The container image digest for visibility and tracking
1010
11+
use crate::install_options::InstallOptions;
1112
use cap_std_ext::cap_std::{self, fs::Dir};
1213
use cap_std_ext::dirext::CapStdExtDirExt;
1314
use color_eyre::{eyre::Context, Result};
@@ -35,6 +36,9 @@ struct CacheInputs {
3536
/// Root filesystem size if specified
3637
root_size: Option<String>,
3738

39+
/// Whether to use composefs-native storage
40+
composefs_native: bool,
41+
3842
/// Kernel arguments used during installation
3943
kernel_args: Vec<String>,
4044

@@ -54,6 +58,9 @@ pub struct DiskImageMetadata {
5458
/// Root filesystem size if specified
5559
pub root_size: Option<String>,
5660

61+
/// Whether to use composefs-native storage
62+
pub composefs_native: bool,
63+
5764
/// Kernel arguments used during installation
5865
pub kernel_args: Vec<String>,
5966

@@ -62,23 +69,13 @@ pub struct DiskImageMetadata {
6269
}
6370

6471
impl DiskImageMetadata {
65-
/// Create new metadata for a disk image
66-
pub fn new(digest: &str) -> Self {
67-
Self {
68-
version: 1,
69-
digest: digest.to_owned(),
70-
filesystem: None,
71-
root_size: None,
72-
kernel_args: Default::default(),
73-
}
74-
}
75-
7672
/// Generate SHA256 hash of all build inputs
7773
fn compute_cache_hash(&self) -> String {
7874
let inputs = CacheInputs {
7975
image_digest: self.digest.clone(),
8076
filesystem: self.filesystem.clone(),
8177
root_size: self.root_size.clone(),
78+
composefs_native: self.composefs_native,
8279
kernel_args: self.kernel_args.clone(),
8380
version: self.version,
8481
};
@@ -154,12 +151,25 @@ impl DiskImageMetadata {
154151
}
155152
}
156153

154+
impl DiskImageMetadata {
155+
/// Create new metadata from InstallOptions and image digest
156+
pub fn from(options: &InstallOptions, image: &str, kernel_args: &[String]) -> Self {
157+
Self {
158+
version: 1,
159+
digest: image.to_owned(),
160+
filesystem: options.filesystem.clone(),
161+
root_size: options.root_size.clone(),
162+
kernel_args: kernel_args.to_vec(),
163+
composefs_native: options.composefs_native,
164+
}
165+
}
166+
}
167+
157168
/// Check if a cached disk image can be reused by comparing cache hashes
158169
pub fn check_cached_disk(
159170
path: &Path,
160171
image_digest: &str,
161-
filesystem: Option<&str>,
162-
root_size: Option<&str>,
172+
install_options: &InstallOptions,
163173
kernel_args: &[String],
164174
) -> Result<bool> {
165175
if !path.exists() {
@@ -168,10 +178,7 @@ pub fn check_cached_disk(
168178
}
169179

170180
// Create metadata for the current request to compute expected hash
171-
let mut expected_meta = DiskImageMetadata::new(image_digest);
172-
expected_meta.filesystem = filesystem.map(ToOwned::to_owned);
173-
expected_meta.root_size = root_size.map(ToOwned::to_owned);
174-
expected_meta.kernel_args = Vec::from(kernel_args);
181+
let expected_meta = DiskImageMetadata::from(install_options, image_digest, kernel_args);
175182
let expected_hash = expected_meta.compute_cache_hash();
176183

177184
// Read the cache hash from the disk image
@@ -221,15 +228,29 @@ mod tests {
221228

222229
#[test]
223230
fn test_cache_hash_generation() {
224-
let mut metadata1 = DiskImageMetadata::new("sha256:abc123");
225-
metadata1.filesystem = Some("ext4".to_string());
226-
metadata1.root_size = Some("20G".to_string());
227-
metadata1.kernel_args = vec!["console=ttyS0".to_string()];
231+
let install_options1 = InstallOptions {
232+
filesystem: Some("ext4".to_string()),
233+
root_size: Some("20G".to_string()),
234+
storage_path: None,
235+
composefs_native: false,
236+
};
237+
let metadata1 = DiskImageMetadata::from(
238+
&install_options1,
239+
"sha256:abc123",
240+
&["console=ttyS0".to_string()],
241+
);
228242

229-
let mut metadata2 = DiskImageMetadata::new("sha256:abc123");
230-
metadata2.filesystem = Some("ext4".to_string());
231-
metadata2.root_size = Some("20G".to_string());
232-
metadata2.kernel_args = vec!["console=ttyS0".to_string()];
243+
let install_options2 = InstallOptions {
244+
filesystem: Some("ext4".to_string()),
245+
root_size: Some("20G".to_string()),
246+
storage_path: None,
247+
composefs_native: false,
248+
};
249+
let metadata2 = DiskImageMetadata::from(
250+
&install_options2,
251+
"sha256:abc123",
252+
&["console=ttyS0".to_string()],
253+
);
233254

234255
// Same inputs should generate same hash
235256
assert_eq!(
@@ -238,21 +259,35 @@ mod tests {
238259
);
239260

240261
// Different inputs should generate different hashes
241-
let mut metadata3 = DiskImageMetadata::new("sha256:xyz789");
242-
metadata3.filesystem = Some("ext4".to_string());
243-
metadata3.root_size = Some("20G".to_string());
244-
metadata3.kernel_args = vec!["console=ttyS0".to_string()];
262+
let install_options3 = InstallOptions {
263+
filesystem: Some("ext4".to_string()),
264+
root_size: Some("20G".to_string()),
265+
storage_path: None,
266+
composefs_native: false,
267+
};
268+
let metadata3 = DiskImageMetadata::from(
269+
&install_options3,
270+
"sha256:xyz789",
271+
&["console=ttyS0".to_string()],
272+
);
245273

246274
assert_ne!(
247275
metadata1.compute_cache_hash(),
248276
metadata3.compute_cache_hash()
249277
);
250278

251279
// Different filesystem should generate different hash
252-
let mut metadata4 = DiskImageMetadata::new("sha256:abc123");
253-
metadata4.filesystem = Some("xfs".to_string());
254-
metadata4.root_size = Some("20G".to_string());
255-
metadata4.kernel_args = vec!["console=ttyS0".to_string()];
280+
let install_options4 = InstallOptions {
281+
filesystem: Some("xfs".to_string()),
282+
root_size: Some("20G".to_string()),
283+
storage_path: None,
284+
composefs_native: false,
285+
};
286+
let metadata4 = DiskImageMetadata::from(
287+
&install_options4,
288+
"sha256:abc123",
289+
&["console=ttyS0".to_string()],
290+
);
256291

257292
assert_ne!(
258293
metadata1.compute_cache_hash(),
@@ -267,6 +302,7 @@ mod tests {
267302
filesystem: Some("ext4".to_string()),
268303
root_size: Some("20G".to_string()),
269304
kernel_args: vec!["console=ttyS0".to_string()],
305+
composefs_native: false,
270306
version: 1,
271307
};
272308

crates/kit/src/libvirt/run.rs

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ use tracing::{debug, info};
1313

1414
use crate::common_opts::MemoryOpts;
1515
use crate::domain_list::DomainLister;
16+
use crate::install_options::InstallOptions;
1617
use crate::libvirt::domain::VirtiofsFilesystem;
1718
use crate::utils::parse_memory_to_mb;
1819
use crate::xml_utils;
@@ -38,9 +39,9 @@ pub struct LibvirtRunOpts {
3839
#[clap(long, default_value = "20G")]
3940
pub disk_size: String,
4041

41-
/// Root filesystem type for installation
42-
#[clap(long, default_value = "ext4")]
43-
pub filesystem: String,
42+
/// Installation options (filesystem, root-size, etc.)
43+
#[clap(flatten)]
44+
pub install: InstallOptions,
4445

4546
/// Port mapping from host to VM
4647
#[clap(long = "port", short = 'p', action = clap::ArgAction::Append)]
@@ -79,7 +80,6 @@ pub struct LibvirtRunOpts {
7980
pub fn run(global_opts: &crate::libvirt::LibvirtOptions, opts: LibvirtRunOpts) -> Result<()> {
8081
use crate::cache_metadata;
8182
use crate::images;
82-
use crate::install_options::InstallOptions;
8383
use crate::run_ephemeral::CommonVmOpts;
8484
use crate::to_disk::ToDiskOpts;
8585

@@ -118,9 +118,8 @@ pub fn run(global_opts: &crate::libvirt::LibvirtOptions, opts: LibvirtRunOpts) -
118118
&vm_name,
119119
&opts.image,
120120
&image_digest,
121-
&opts.filesystem,
122-
None, // root_size
123-
&[], // kernel_args
121+
&opts.install,
122+
&[], // kernel_args
124123
connect_uri,
125124
)
126125
.with_context(|| "Failed to find or create disk path")?;
@@ -129,9 +128,8 @@ pub fn run(global_opts: &crate::libvirt::LibvirtOptions, opts: LibvirtRunOpts) -
129128
let cached = cache_metadata::check_cached_disk(
130129
disk_path.as_std_path(),
131130
&image_digest,
132-
Some(&opts.filesystem),
133-
None, // root_size
134-
&[], // kernel_args
131+
&opts.install,
132+
&[], // kernel_args
135133
)?;
136134

137135
if cached {
@@ -145,10 +143,7 @@ pub fn run(global_opts: &crate::libvirt::LibvirtOptions, opts: LibvirtRunOpts) -
145143
target_disk: disk_path.clone(),
146144
disk_size: Some(opts.disk_size.clone()),
147145
format: crate::to_disk::Format::Raw, // Default to raw format
148-
install: InstallOptions {
149-
filesystem: Some(opts.filesystem.clone()),
150-
..Default::default()
151-
},
146+
install: opts.install.clone(),
152147
common: CommonVmOpts {
153148
memory: opts.memory.clone(),
154149
vcpus: Some(opts.cpus),
@@ -313,8 +308,7 @@ fn find_or_create_cached_disk(
313308
vm_name: &str,
314309
source_image: &str,
315310
image_digest: &str,
316-
filesystem: &str,
317-
root_size: Option<&str>,
311+
install_options: &InstallOptions,
318312
kernel_args: &[String],
319313
connect_uri: Option<&String>,
320314
) -> Result<Utf8PathBuf> {
@@ -361,8 +355,7 @@ fn find_or_create_cached_disk(
361355
if cache_metadata::check_cached_disk(
362356
path.as_std_path(),
363357
image_digest,
364-
Some(filesystem),
365-
root_size,
358+
install_options,
366359
kernel_args,
367360
)? {
368361
info!("Found matching cached disk image: {:?}", path);
@@ -555,7 +548,13 @@ fn create_libvirt_domain_from_disk(
555548
.with_metadata("bootc:memory-mb", &opts.memory.to_string())
556549
.with_metadata("bootc:vcpus", &opts.cpus.to_string())
557550
.with_metadata("bootc:disk-size-gb", &opts.disk_size.to_string())
558-
.with_metadata("bootc:filesystem", &opts.filesystem)
551+
.with_metadata(
552+
"bootc:filesystem",
553+
opts.install
554+
.filesystem
555+
.as_ref()
556+
.unwrap_or(&"ext4".to_string()),
557+
)
559558
.with_metadata("bootc:network", &opts.network)
560559
.with_metadata("bootc:ssh-generated", "true")
561560
.with_metadata("bootc:ssh-private-key-base64", &private_key_base64)

crates/kit/src/to_disk.rs

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -262,8 +262,7 @@ pub fn run(opts: ToDiskOpts) -> Result<()> {
262262
let matches = crate::cache_metadata::check_cached_disk(
263263
opts.target_disk.as_std_path(),
264264
&image_digest,
265-
opts.install.filesystem.as_deref(),
266-
opts.install.root_size.as_deref(),
265+
&opts.install,
267266
&opts.common.kernel_args,
268267
)?;
269268

@@ -417,8 +416,7 @@ pub fn run(opts: ToDiskOpts) -> Result<()> {
417416
let write_result = write_disk_metadata(
418417
&opts.source_image,
419418
&opts.target_disk,
420-
opts.install.filesystem.as_deref(),
421-
opts.install.root_size.as_deref(),
419+
&opts.install,
422420
&opts.common.kernel_args,
423421
&opts.format,
424422
);
@@ -439,8 +437,7 @@ pub fn run(opts: ToDiskOpts) -> Result<()> {
439437
fn write_disk_metadata(
440438
source_image: &str,
441439
target_disk: &Utf8PathBuf,
442-
filesystem: Option<&str>,
443-
root_size: Option<&str>,
440+
install_options: &InstallOptions,
444441
kernel_args: &[String],
445442
format: &Format,
446443
) -> Result<()> {
@@ -451,11 +448,8 @@ fn write_disk_metadata(
451448
let inspect = images::inspect(source_image)?;
452449
let digest = inspect.digest.to_string();
453450

454-
// Prepare metadata
455-
let mut metadata = DiskImageMetadata::new(&digest);
456-
metadata.filesystem = filesystem.map(|s| s.to_owned());
457-
metadata.root_size = root_size.map(|s| s.to_string());
458-
metadata.kernel_args = kernel_args.to_vec();
451+
// Prepare metadata using the new helper method
452+
let metadata = DiskImageMetadata::from(install_options, &digest, kernel_args);
459453

460454
// Write metadata using rustix fsetxattr
461455
let file = std::fs::OpenOptions::new()

0 commit comments

Comments
 (0)