From 796515f4f719f11089ef0205e0036aebee670ff0 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 20 Oct 2025 13:51:48 -0400 Subject: [PATCH] libvirt: Fix memory display unit conversion I noticed the output was wonky. Assisted-by: Claude Code Signed-off-by: Colin Walters --- crates/kit/src/domain_list.rs | 5 +-- crates/kit/src/lib.rs | 1 + crates/kit/src/utils.rs | 57 +++++++++++++++++++++++++++++++++++ crates/kit/src/xml_utils.rs | 36 ++++++++++++++++++++++ 4 files changed, 95 insertions(+), 4 deletions(-) diff --git a/crates/kit/src/domain_list.rs b/crates/kit/src/domain_list.rs index 68b562fb9..657849ae0 100644 --- a/crates/kit/src/domain_list.rs +++ b/crates/kit/src/domain_list.rs @@ -192,10 +192,7 @@ impl DomainLister { .unwrap_or_default(); // Extract memory and vcpu from domain XML - let memory_mb = dom.find("memory").and_then(|node| { - // Memory might have unit attribute, but we'll try to parse the value - node.text_content().parse::().ok() - }); + let memory_mb = dom.find("memory").and_then(|node| node.parse_memory_mb()); let vcpus = dom .find("vcpu") diff --git a/crates/kit/src/lib.rs b/crates/kit/src/lib.rs index 310efb3c4..ed4a54ae0 100644 --- a/crates/kit/src/lib.rs +++ b/crates/kit/src/lib.rs @@ -1,4 +1,5 @@ //! bcvk library - exposes internal modules for testing pub mod qemu_img; +pub mod utils; pub mod xml_utils; diff --git a/crates/kit/src/utils.rs b/crates/kit/src/utils.rs index 5f5ec9404..9ed1c2716 100644 --- a/crates/kit/src/utils.rs +++ b/crates/kit/src/utils.rs @@ -173,3 +173,60 @@ pub(crate) fn parse_memory_to_mb(memory_str: &str) -> Result { Err(eyre!("Memory specification cannot be empty - please provide a value like '2G', '1024M', or '512'")) } } + +/// Convert memory value with unit to megabytes (MiB) +/// Handles libvirt-style units distinguishing between decimal (KB, MB, GB - powers of 1000) +/// and binary (KiB, MiB, GiB - powers of 1024) units per libvirt specification +pub(crate) fn convert_memory_to_mb(value: u32, unit: &str) -> u32 { + // Use u128 for calculations to prevent overflow with large units like TB + let value_u128 = value as u128; + let mib_u128 = 1024 * 1024; + + let mb = match unit { + // Binary prefixes (powers of 1024), converting to MiB + "k" | "K" | "KiB" => value_u128 / 1024, + "M" | "MiB" => value_u128, + "G" | "GiB" => value_u128 * 1024, + "T" | "TiB" => value_u128 * 1024 * 1024, + + // Decimal prefixes (powers of 1000), converting to MiB + "B" | "bytes" => value_u128 / mib_u128, + "KB" => (value_u128 * 1_000) / mib_u128, + "MB" => (value_u128 * 1_000_000) / mib_u128, + "GB" => (value_u128 * 1_000_000_000) / mib_u128, + "TB" => (value_u128 * 1_000_000_000_000) / mib_u128, + + // Libvirt default is KiB for memory + _ => value_u128 / 1024, + }; + mb as u32 +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_convert_memory_to_mb() { + // Test binary units (powers of 1024) + assert_eq!(convert_memory_to_mb(4194304, "KiB"), 4096); + assert_eq!(convert_memory_to_mb(2097152, "KiB"), 2048); + assert_eq!(convert_memory_to_mb(2048, "MiB"), 2048); + assert_eq!(convert_memory_to_mb(4096, "MiB"), 4096); + assert_eq!(convert_memory_to_mb(4, "GiB"), 4096); + assert_eq!(convert_memory_to_mb(2, "GiB"), 2048); + + // Test short forms (binary) + assert_eq!(convert_memory_to_mb(4, "G"), 4096); + assert_eq!(convert_memory_to_mb(2048, "M"), 2048); + assert_eq!(convert_memory_to_mb(2097152, "K"), 2048); + + // Test decimal units (powers of 1000) + assert_eq!(convert_memory_to_mb(1048576, "KB"), 1000); + assert_eq!(convert_memory_to_mb(1024, "MB"), 976); + assert_eq!(convert_memory_to_mb(4, "GB"), 3814); + + // Test default/unknown unit (defaults to KiB) + assert_eq!(convert_memory_to_mb(4194304, "unknown"), 4096); + } +} diff --git a/crates/kit/src/xml_utils.rs b/crates/kit/src/xml_utils.rs index c78d4738f..7ef137200 100644 --- a/crates/kit/src/xml_utils.rs +++ b/crates/kit/src/xml_utils.rs @@ -140,6 +140,19 @@ impl XmlNode { pub fn text_content(&self) -> &str { &self.text } + + /// Parse memory value from an XML node with unit attribute + /// Returns the value in megabytes (MB) + pub fn parse_memory_mb(&self) -> Option { + let value = self.text_content().parse::().ok()?; + // Convert to MB based on unit attribute (default is KiB per libvirt spec) + let unit = self + .attributes + .get("unit") + .map(|s| s.as_str()) + .unwrap_or("KiB"); + Some(crate::utils::convert_memory_to_mb(value, unit)) + } } /// Parse XML string into a simple DOM structure @@ -370,4 +383,27 @@ mod tests { assert!(xml.contains("raw content")); assert!(xml.contains("")); } + + #[test] + fn test_parse_memory_mb() { + // Test KiB (default unit) + let xml = r#"4194304"#; + let dom = parse_xml_dom(xml).unwrap(); + assert_eq!(dom.parse_memory_mb(), Some(4096)); + + // Test MiB + let xml = r#"2048"#; + let dom = parse_xml_dom(xml).unwrap(); + assert_eq!(dom.parse_memory_mb(), Some(2048)); + + // Test GiB + let xml = r#"4"#; + let dom = parse_xml_dom(xml).unwrap(); + assert_eq!(dom.parse_memory_mb(), Some(4096)); + + // Test KB (decimal unit: 1000-based) + let xml = r#"1048576"#; + let dom = parse_xml_dom(xml).unwrap(); + assert_eq!(dom.parse_memory_mb(), Some(1000)); + } }