Skip to content

Commit bc31122

Browse files
committed
Make use of Default for ToDisk structs
This avoids lots of duplication when adding a new field. Signed-off-by: Colin Walters <[email protected]>
1 parent 3cbb4cf commit bc31122

File tree

4 files changed

+80
-85
lines changed

4 files changed

+80
-85
lines changed

crates/kit/src/libvirt/run.rs

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ pub fn run(global_opts: &crate::libvirt::LibvirtOptions, opts: LibvirtRunOpts) -
8181
use crate::cache_metadata;
8282
use crate::images;
8383
use crate::run_ephemeral::CommonVmOpts;
84-
use crate::to_disk::ToDiskOpts;
84+
use crate::to_disk::{ToDiskAdditionalOpts, ToDiskOpts};
8585

8686
let connect_uri = global_opts.connect.as_ref();
8787
let lister = match connect_uri {
@@ -141,22 +141,17 @@ pub fn run(global_opts: &crate::libvirt::LibvirtOptions, opts: LibvirtRunOpts) -
141141
let to_disk_opts = ToDiskOpts {
142142
source_image: opts.image.clone(),
143143
target_disk: disk_path.clone(),
144-
disk_size: Some(opts.disk_size.clone()),
145-
format: crate::to_disk::Format::Raw, // Default to raw format
146144
install: opts.install.clone(),
147-
install_log: None,
148-
common: CommonVmOpts {
149-
memory: opts.memory.clone(),
150-
vcpus: Some(opts.cpus),
151-
kernel_args: vec![],
152-
net: None,
153-
console: false,
154-
debug: false,
155-
virtio_serial_out: vec![],
156-
execute: vec![],
157-
ssh_keygen: true, // Enable SSH key generation
145+
additional: ToDiskAdditionalOpts {
146+
disk_size: Some(opts.disk_size.clone()),
147+
common: CommonVmOpts {
148+
memory: opts.memory.clone(),
149+
vcpus: Some(opts.cpus),
150+
ssh_keygen: true, // Enable SSH key generation
151+
..Default::default()
152+
},
153+
..Default::default()
158154
},
159-
label: vec![],
160155
};
161156

162157
// Run the disk creation

crates/kit/src/libvirt/upload.rs

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
66
use crate::common_opts::MemoryOpts;
77
use crate::install_options::InstallOptions;
8-
use crate::to_disk::{run as to_disk, ToDiskOpts};
8+
use crate::to_disk::{run as to_disk, ToDiskAdditionalOpts, ToDiskOpts};
99
use crate::{images, utils};
1010
use camino::Utf8PathBuf;
1111
use clap::Parser;
@@ -214,20 +214,16 @@ pub fn run(global_opts: &crate::libvirt::LibvirtOptions, opts: LibvirtUploadOpts
214214
source_image: opts.source_image.clone(),
215215
target_disk: temp_disk_path.clone(),
216216
install: opts.install.clone(),
217-
format: crate::to_disk::Format::Raw, // Default to raw format
218-
disk_size: Some(disk_size.to_string()),
219-
label: Default::default(),
220-
install_log: None,
221-
common: crate::run_ephemeral::CommonVmOpts {
222-
memory: opts.memory.clone(),
223-
vcpus: opts.vcpus,
224-
kernel_args: opts.karg.clone(),
225-
net: Some("none".to_string()),
226-
console: false,
227-
debug: false,
228-
virtio_serial_out: vec![],
229-
execute: Default::default(),
230-
ssh_keygen: false,
217+
additional: ToDiskAdditionalOpts {
218+
disk_size: Some(disk_size.to_string()),
219+
common: crate::run_ephemeral::CommonVmOpts {
220+
memory: opts.memory.clone(),
221+
vcpus: opts.vcpus,
222+
kernel_args: opts.karg.clone(),
223+
net: Some("none".to_string()),
224+
..Default::default()
225+
},
226+
..Default::default()
231227
},
232228
};
233229

crates/kit/src/libvirt_upload_disk.rs

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
66
use crate::common_opts::MemoryOpts;
77
use crate::install_options::InstallOptions;
8-
use crate::to_disk::{run as to_disk, ToDiskOpts};
8+
use crate::to_disk::{run as to_disk, ToDiskAdditionalOpts, ToDiskOpts};
99
use crate::xml_utils::{self, XmlWriter};
1010
use crate::{images, utils};
1111
use camino::Utf8Path;
@@ -285,20 +285,16 @@ pub fn run(opts: LibvirtUploadDiskOpts) -> Result<()> {
285285
source_image: opts.source_image.clone(),
286286
target_disk: temp_disk.clone(),
287287
install: opts.install.clone(),
288-
disk_size: Some(disk_size.to_string()),
289-
format: crate::to_disk::Format::Raw, // Default to raw format
290-
label: Default::default(),
291-
install_log: None,
292-
common: crate::run_ephemeral::CommonVmOpts {
293-
memory: opts.memory.clone(),
294-
vcpus: opts.vcpus,
295-
kernel_args: opts.karg.clone(),
296-
net: Some("none".to_string()),
297-
console: false,
298-
debug: false,
299-
virtio_serial_out: vec![],
300-
execute: Default::default(),
301-
ssh_keygen: false,
288+
additional: ToDiskAdditionalOpts {
289+
disk_size: Some(disk_size.to_string()),
290+
common: crate::run_ephemeral::CommonVmOpts {
291+
memory: opts.memory.clone(),
292+
vcpus: opts.vcpus,
293+
kernel_args: opts.karg.clone(),
294+
net: Some("none".to_string()),
295+
..Default::default()
296+
},
297+
..Default::default()
302298
},
303299
};
304300

crates/kit/src/to_disk.rs

Lines changed: 48 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,10 @@ use indoc::indoc;
8888
use tracing::debug;
8989

9090
/// Supported disk image formats
91-
#[derive(Debug, Clone, ValueEnum, PartialEq)]
91+
#[derive(Debug, Clone, ValueEnum, PartialEq, Default)]
9292
pub enum Format {
9393
/// Raw disk image format (default)
94+
#[default]
9495
Raw,
9596
/// QEMU Copy On Write 2 format
9697
Qcow2,
@@ -112,21 +113,9 @@ impl std::fmt::Display for Format {
112113
}
113114
}
114115

115-
/// Configuration options for installing a bootc container image to disk
116-
///
117-
/// See the module-level documentation for details on the installation architecture and workflow.
118-
#[derive(Debug, Parser)]
119-
pub struct ToDiskOpts {
120-
/// Container image to install
121-
pub source_image: String,
122-
123-
/// Target disk/device path
124-
pub target_disk: Utf8PathBuf,
125-
126-
/// Installation options (filesystem, root-size, storage-path)
127-
#[clap(flatten)]
128-
pub install: InstallOptions,
129-
116+
/// Additional configuration options for installing a bootc container image to disk
117+
#[derive(Debug, Parser, Default)]
118+
pub struct ToDiskAdditionalOpts {
130119
/// Disk size to create (e.g. 10G, 5120M, or plain number for bytes)
131120
#[clap(long)]
132121
pub disk_size: Option<String>,
@@ -150,6 +139,26 @@ pub struct ToDiskOpts {
150139
pub label: Vec<String>,
151140
}
152141

142+
/// Configuration options for installing a bootc container image to disk
143+
///
144+
/// See the module-level documentation for details on the installation architecture and workflow.
145+
#[derive(Debug, Parser)]
146+
pub struct ToDiskOpts {
147+
/// Container image to install
148+
pub source_image: String,
149+
150+
/// Target disk/device path
151+
pub target_disk: Utf8PathBuf,
152+
153+
/// Installation options (filesystem, root-size, storage-path)
154+
#[clap(flatten)]
155+
pub install: InstallOptions,
156+
157+
/// Additional installation options
158+
#[clap(flatten)]
159+
pub additional: ToDiskAdditionalOpts,
160+
}
161+
153162
impl ToDiskOpts {
154163
/// Get the container image to use as the installation environment
155164
///
@@ -189,6 +198,7 @@ impl ToDiskOpts {
189198
.to_string();
190199

191200
let install_log = self
201+
.additional
192202
.install_log
193203
.as_deref()
194204
.map(|v| shlex::try_quote(v))
@@ -232,7 +242,7 @@ impl ToDiskOpts {
232242
/// Returns explicit disk_size if provided (parsed from human-readable format),
233243
/// otherwise 2x the image size with a 4GB minimum.
234244
fn calculate_disk_size(&self) -> Result<u64> {
235-
if let Some(ref size_str) = self.disk_size {
245+
if let Some(ref size_str) = self.additional.disk_size {
236246
let parsed = utils::parse_size(size_str)?;
237247
debug!("Using explicit disk size: {} -> {} bytes", size_str, parsed);
238248
return Ok(parsed);
@@ -277,7 +287,7 @@ pub fn run(opts: ToDiskOpts) -> Result<()> {
277287
opts.target_disk.as_std_path(),
278288
&image_digest,
279289
&opts.install,
280-
&opts.common.kernel_args,
290+
&opts.additional.common.kernel_args,
281291
)?;
282292

283293
if matches {
@@ -299,7 +309,7 @@ pub fn run(opts: ToDiskOpts) -> Result<()> {
299309
let storage_path = opts.get_storage_path()?;
300310

301311
// Debug logging for installation configuration
302-
if opts.common.debug {
312+
if opts.additional.common.debug {
303313
debug!("Using container storage: {:?}", storage_path);
304314
debug!("Installing to target disk: {:?}", opts.target_disk);
305315
debug!("Filesystem: {:?}", opts.install.filesystem);
@@ -311,7 +321,7 @@ pub fn run(opts: ToDiskOpts) -> Result<()> {
311321
let disk_size = opts.calculate_disk_size()?;
312322

313323
// Create disk image based on format
314-
match opts.format {
324+
match opts.additional.format {
315325
Format::Raw => {
316326
// Create sparse file - only allocates space as data is written
317327
let file = std::fs::File::create(&opts.target_disk)
@@ -352,7 +362,7 @@ pub fn run(opts: ToDiskOpts) -> Result<()> {
352362
let bootc_install_command = opts.generate_bootc_install_command()?;
353363

354364
// Phase 4: Ephemeral VM configuration
355-
let mut common_opts = opts.common.clone();
365+
let mut common_opts = opts.additional.common.clone();
356366
// Enable SSH key generation for SSH-based installation
357367
common_opts.ssh_keygen = true;
358368

@@ -370,22 +380,22 @@ pub fn run(opts: ToDiskOpts) -> Result<()> {
370380
rm: true, // Clean up container after installation
371381
detach: true, // Run in detached mode for SSH approach
372382
tty,
373-
label: opts.label,
383+
label: opts.additional.label,
374384
..Default::default()
375385
},
376386
// Workaround for https://github.com/containers/container-libs/issues/144#issuecomment-3300424410
377387
// Basically containers-libs allocates a tempfile for a whole serialization of a layer as a tarball
378388
// when fetching, so we need enough memory to do so.
379389
add_swap: Some(format!("{disk_size}")),
380-
bind_mounts: Vec::new(), // No additional bind mounts needed
381-
ro_bind_mounts: Vec::new(), // No additional ro bind mounts needed
382-
systemd_units_dir: None, // No custom systemd units
383-
log_cmdline: opts.common.debug, // Log kernel command line if debug
384-
bind_storage_ro: true, // Mount host container storage read-only
390+
bind_mounts: Vec::new(), // No additional bind mounts needed
391+
ro_bind_mounts: Vec::new(), // No additional ro bind mounts needed
392+
systemd_units_dir: None, // No custom systemd units
393+
log_cmdline: opts.additional.common.debug, // Log kernel command line if debug
394+
bind_storage_ro: true, // Mount host container storage read-only
385395
mount_disk_files: vec![format!(
386396
"{}:output:{}",
387397
opts.target_disk,
388-
opts.format.as_str()
398+
opts.additional.format.as_str()
389399
)], // Attach target disk
390400
};
391401

@@ -441,8 +451,8 @@ pub fn run(opts: ToDiskOpts) -> Result<()> {
441451
&opts.source_image,
442452
&opts.target_disk,
443453
&opts.install,
444-
&opts.common.kernel_args,
445-
&opts.format,
454+
&opts.additional.common.kernel_args,
455+
&opts.additional.format,
446456
);
447457
if let Err(e) = write_result {
448458
debug!("Failed to write metadata to disk image: {}", e);
@@ -505,15 +515,14 @@ mod tests {
505515
let opts = ToDiskOpts {
506516
source_image: "test:latest".to_string(),
507517
target_disk: "/tmp/test.img".into(),
508-
label: Default::default(),
509-
install_log: None,
510518
install: InstallOptions {
511519
filesystem: Some("ext4".to_string()),
512520
..Default::default()
513521
},
514-
disk_size: Some("10G".to_string()),
515-
format: Format::Raw,
516-
common: CommonVmOpts::default(),
522+
additional: ToDiskAdditionalOpts {
523+
disk_size: Some("10G".to_string()),
524+
..Default::default()
525+
},
517526
};
518527

519528
let size = opts.calculate_disk_size()?;
@@ -524,15 +533,14 @@ mod tests {
524533
let opts2 = ToDiskOpts {
525534
source_image: "test:latest".to_string(),
526535
target_disk: "/tmp/test.img".into(),
527-
label: Default::default(),
528-
install_log: None,
529536
install: InstallOptions {
530537
filesystem: Some("ext4".to_string()),
531538
..Default::default()
532539
},
533-
disk_size: Some("5120M".to_string()),
534-
format: Format::Raw,
535-
common: CommonVmOpts::default(),
540+
additional: ToDiskAdditionalOpts {
541+
disk_size: Some("5120M".to_string()),
542+
..Default::default()
543+
},
536544
};
537545

538546
let size2 = opts2.calculate_disk_size()?;

0 commit comments

Comments
 (0)