Skip to content

Commit 4e4c573

Browse files
committed
utils: Add DiskSize type for parsing human-readable sizes
Add a DiskSize newtype that wraps u64 bytes and implements FromStr, allowing clap to parse disk sizes directly from command line arguments. This eliminates the need for manual parsing at each call site. The type supports human-readable formats like '10G', '5120M', '1T' and provides both from_bytes() constructor and as_bytes() accessor. Update all disk_size Option<String> fields to use Option<DiskSize>: - to_disk::ToDiskAdditionalOpts - libvirt/upload::LibvirtUploadOpts - libvirt_upload_disk::LibvirtUploadDiskOpts - libvirt/base_disks (internal usage) Assisted-by: OpenCode (Claude sonnet-4-20250514) Signed-off-by: Colin Walters <walters@verbum.org>
1 parent be88173 commit 4e4c573

5 files changed

Lines changed: 79 additions & 23 deletions

File tree

crates/kit/src/libvirt/base_disks.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
77
use crate::cache_metadata::DiskImageMetadata;
88
use crate::install_options::InstallOptions;
9+
use crate::utils::DiskSize;
910
use camino::{Utf8Path, Utf8PathBuf};
1011
use color_eyre::eyre::{eyre, Context};
1112
use color_eyre::Result;
@@ -109,8 +110,9 @@ fn create_base_disk(
109110
additional: ToDiskAdditionalOpts {
110111
disk_size: install_options
111112
.root_size
112-
.clone()
113-
.or(Some(super::LIBVIRT_DEFAULT_DISK_SIZE.to_string())),
113+
.as_ref()
114+
.and_then(|s| s.parse::<DiskSize>().ok())
115+
.or_else(|| super::LIBVIRT_DEFAULT_DISK_SIZE.parse::<DiskSize>().ok()),
114116
format: Format::Qcow2, // Use qcow2 for CoW cloning
115117
common: CommonVmOpts {
116118
memory: crate::common_opts::MemoryOpts {

crates/kit/src/libvirt/upload.rs

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,10 @@
44
//! to libvirt storage pools, maintaining container image metadata as libvirt annotations.
55
66
use crate::common_opts::MemoryOpts;
7+
use crate::images;
78
use crate::install_options::InstallOptions;
89
use crate::to_disk::{run as to_disk, ToDiskAdditionalOpts, ToDiskOpts};
9-
use crate::{images, utils};
10+
use crate::utils::DiskSize;
1011
use camino::Utf8PathBuf;
1112
use clap::Parser;
1213
use color_eyre::{eyre::eyre, Result};
@@ -30,7 +31,7 @@ pub struct LibvirtUploadOpts {
3031

3132
/// Size of the disk image (e.g., '20G', '10240M'). If not specified, uses the actual size of the created disk.
3233
#[clap(long)]
33-
pub disk_size: Option<String>,
34+
pub disk_size: Option<DiskSize>,
3435

3536
/// Installation options (filesystem, root-size, storage-path)
3637
#[clap(flatten)]
@@ -189,14 +190,14 @@ pub fn run(global_opts: &crate::libvirt::LibvirtOptions, opts: LibvirtUploadOpts
189190
debug!("Container image digest: {}", image_digest);
190191

191192
// Phase 2: Calculate disk size to use
192-
let disk_size = if let Some(ref size_str) = opts.disk_size {
193+
let disk_size = if let Some(size) = opts.disk_size {
193194
// Use explicit size if provided
194-
utils::parse_size(size_str)?
195+
size
195196
} else {
196197
// Use same logic as to_disk: 2x source image size with 4GB minimum
197198
let image_size = images::get_image_size(&opts.source_image)?;
198199

199-
std::cmp::max(image_size * 2, 4u64 * 1024 * 1024 * 1024)
200+
DiskSize::from_bytes(std::cmp::max(image_size * 2, 4u64 * 1024 * 1024 * 1024))
200201
};
201202

202203
// Phase 2: Create temporary disk path
@@ -211,7 +212,7 @@ pub fn run(global_opts: &crate::libvirt::LibvirtOptions, opts: LibvirtUploadOpts
211212
target_disk: temp_disk_path.clone(),
212213
install: opts.install.clone(),
213214
additional: ToDiskAdditionalOpts {
214-
disk_size: Some(disk_size.to_string()),
215+
disk_size: Some(disk_size),
215216
common: crate::run_ephemeral::CommonVmOpts {
216217
memory: opts.memory.clone(),
217218
vcpus: opts.vcpus,
@@ -226,7 +227,7 @@ pub fn run(global_opts: &crate::libvirt::LibvirtOptions, opts: LibvirtUploadOpts
226227
opts.upload_to_libvirt(
227228
global_opts,
228229
temp_disk_path.as_std_path(),
229-
disk_size,
230+
disk_size.as_bytes(),
230231
&image_digest,
231232
)?;
232233

crates/kit/src/libvirt_upload_disk.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,11 @@
44
//! to libvirt storage pools, maintaining container image metadata as libvirt annotations.
55
66
use crate::common_opts::MemoryOpts;
7+
use crate::images;
78
use crate::install_options::InstallOptions;
89
use crate::to_disk::{run as to_disk, ToDiskAdditionalOpts, ToDiskOpts};
10+
use crate::utils::DiskSize;
911
use crate::xml_utils::{self, XmlWriter};
10-
use crate::{images, utils};
1112
use camino::Utf8Path;
1213
use clap::Parser;
1314
use color_eyre::{eyre::eyre, Result};
@@ -31,7 +32,7 @@ pub struct LibvirtUploadDiskOpts {
3132

3233
/// Size of the disk image (e.g., '20G', '10240M'). If not specified, uses the actual size of the created disk.
3334
#[clap(long)]
34-
pub disk_size: Option<String>,
35+
pub disk_size: Option<DiskSize>,
3536

3637
/// Installation options (filesystem, root-size, storage-path)
3738
#[clap(flatten)]
@@ -258,9 +259,9 @@ pub fn run(opts: LibvirtUploadDiskOpts) -> Result<()> {
258259
);
259260

260261
// Phase 1: Calculate disk size to use
261-
let disk_size = if let Some(ref size_str) = opts.disk_size {
262+
let disk_size = if let Some(size) = opts.disk_size {
262263
// Use explicit size if provided
263-
utils::parse_size(size_str)?
264+
size.as_bytes()
264265
} else {
265266
// Use same logic as to_disk: 2x source image size with 4GB minimum
266267
let image_size = images::get_image_size(&opts.source_image)?;
@@ -282,7 +283,7 @@ pub fn run(opts: LibvirtUploadDiskOpts) -> Result<()> {
282283
target_disk: temp_disk.clone(),
283284
install: opts.install.clone(),
284285
additional: ToDiskAdditionalOpts {
285-
disk_size: Some(disk_size.to_string()),
286+
disk_size: Some(DiskSize::from_bytes(disk_size)),
286287
common: crate::run_ephemeral::CommonVmOpts {
287288
memory: opts.memory.clone(),
288289
vcpus: opts.vcpus,

crates/kit/src/to_disk.rs

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ use crate::cache_metadata::DiskImageMetadata;
7979
use crate::install_options::InstallOptions;
8080
use crate::run_ephemeral::{run_detached, CommonVmOpts, RunEphemeralOpts};
8181
use crate::run_ephemeral_ssh::wait_for_ssh_ready;
82+
use crate::utils::DiskSize;
8283
use crate::{images, ssh, utils};
8384
use camino::Utf8PathBuf;
8485
use clap::{Parser, ValueEnum};
@@ -119,7 +120,7 @@ impl std::fmt::Display for Format {
119120
pub struct ToDiskAdditionalOpts {
120121
/// Disk size to create (e.g. 10G, 5120M, or plain number for bytes)
121122
#[clap(long)]
122-
pub disk_size: Option<String>,
123+
pub disk_size: Option<DiskSize>,
123124

124125
/// Output disk image format
125126
#[clap(long, default_value_t = Format::Raw)]
@@ -337,13 +338,11 @@ EOF
337338

338339
/// Calculate the optimal target disk size based on the source image or explicit size
339340
///
340-
/// Returns explicit disk_size if provided (parsed from human-readable format),
341-
/// otherwise 2x the image size with a 4GB minimum.
341+
/// Returns explicit disk_size if provided, otherwise 2x the image size with a 4GB minimum.
342342
fn calculate_disk_size(&self) -> Result<u64> {
343-
if let Some(ref size_str) = self.additional.disk_size {
344-
let parsed = utils::parse_size(size_str)?;
345-
debug!("Using explicit disk size: {} -> {} bytes", size_str, parsed);
346-
return Ok(parsed);
343+
if let Some(size) = self.additional.disk_size {
344+
debug!("Using explicit disk size: {} bytes", size.as_bytes());
345+
return Ok(size.as_bytes());
347346
}
348347

349348
// Get the image size and multiply by 2 for installation space
@@ -639,7 +638,7 @@ mod tests {
639638
..Default::default()
640639
},
641640
additional: ToDiskAdditionalOpts {
642-
disk_size: Some("10G".to_string()),
641+
disk_size: Some("10G".parse().unwrap()),
643642
..Default::default()
644643
},
645644
};
@@ -657,7 +656,7 @@ mod tests {
657656
..Default::default()
658657
},
659658
additional: ToDiskAdditionalOpts {
660-
disk_size: Some("5120M".to_string()),
659+
disk_size: Some("5120M".parse().unwrap()),
661660
..Default::default()
662661
},
663662
};

crates/kit/src/utils.rs

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,59 @@ pub(crate) fn validate_container_storage_path(path: &Utf8Path) -> Result<()> {
167167
Ok(())
168168
}
169169

170+
/// A disk/file size in bytes, parsed from human-readable strings like "10G", "5120M", "1T"
171+
///
172+
/// Implements `FromStr` for direct use with clap's `value_parser`.
173+
///
174+
/// # Examples
175+
///
176+
/// ```ignore
177+
/// use bcvk::utils::DiskSize;
178+
///
179+
/// let size: DiskSize = "10G".parse().unwrap();
180+
/// assert_eq!(size.as_bytes(), 10 * 1024 * 1024 * 1024);
181+
/// ```
182+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
183+
pub struct DiskSize(u64);
184+
185+
impl DiskSize {
186+
/// Create a new DiskSize from bytes
187+
pub fn from_bytes(bytes: u64) -> Self {
188+
Self(bytes)
189+
}
190+
191+
/// Get the size in bytes
192+
pub fn as_bytes(&self) -> u64 {
193+
self.0
194+
}
195+
}
196+
197+
impl std::str::FromStr for DiskSize {
198+
type Err = color_eyre::eyre::Error;
199+
200+
fn from_str(s: &str) -> Result<Self> {
201+
parse_size(s).map(DiskSize)
202+
}
203+
}
204+
205+
impl std::fmt::Display for DiskSize {
206+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
207+
// Display in human-readable form
208+
let bytes = self.0;
209+
if bytes >= 1024 * 1024 * 1024 * 1024 && bytes % (1024 * 1024 * 1024 * 1024) == 0 {
210+
write!(f, "{}T", bytes / (1024 * 1024 * 1024 * 1024))
211+
} else if bytes >= 1024 * 1024 * 1024 && bytes % (1024 * 1024 * 1024) == 0 {
212+
write!(f, "{}G", bytes / (1024 * 1024 * 1024))
213+
} else if bytes >= 1024 * 1024 && bytes % (1024 * 1024) == 0 {
214+
write!(f, "{}M", bytes / (1024 * 1024))
215+
} else if bytes >= 1024 && bytes % 1024 == 0 {
216+
write!(f, "{}K", bytes / 1024)
217+
} else {
218+
write!(f, "{}", bytes)
219+
}
220+
}
221+
}
222+
170223
/// Parse size string (e.g., "10G", "5120M", "1T") to bytes
171224
pub(crate) fn parse_size(size_str: &str) -> Result<u64> {
172225
let size_str = size_str.trim().to_uppercase();

0 commit comments

Comments
 (0)