From c8ea08191b03b713b045417b7af71c6f09e99f4f Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 10 Oct 2025 10:25:09 -0400 Subject: [PATCH] Clean up libvirt XML parsing With a helper fn. Signed-off-by: Colin Walters --- crates/kit/src/domain_list.rs | 20 ++--------- crates/kit/src/libvirt/list_volumes.rs | 17 +++------ crates/kit/src/libvirt/run.rs | 50 ++++++++++++++++---------- crates/kit/src/libvirt/ssh.rs | 32 ++++++++--------- 4 files changed, 53 insertions(+), 66 deletions(-) diff --git a/crates/kit/src/domain_list.rs b/crates/kit/src/domain_list.rs index 691aed69a..cd7cf7ec3 100644 --- a/crates/kit/src/domain_list.rs +++ b/crates/kit/src/domain_list.rs @@ -142,24 +142,8 @@ impl DomainLister { /// Get domain XML metadata as parsed DOM pub fn get_domain_xml(&self, domain_name: &str) -> Result { - let output = self - .virsh_command() - .args(&["dumpxml", domain_name]) - .output() - .with_context(|| format!("Failed to dump XML for domain '{}'", domain_name))?; - - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - return Err(color_eyre::eyre::eyre!( - "Failed to get XML for domain '{}': {}", - domain_name, - stderr - )); - } - - let xml = String::from_utf8(output.stdout)?; - xml_utils::parse_xml_dom(&xml) - .with_context(|| format!("Failed to parse XML for domain '{}'", domain_name)) + crate::libvirt::run::run_virsh_xml(self.connect_uri.as_deref(), &["dumpxml", domain_name]) + .context(format!("Failed to get XML for domain '{}'", domain_name)) } /// Extract podman-bootc metadata from parsed domain XML diff --git a/crates/kit/src/libvirt/list_volumes.rs b/crates/kit/src/libvirt/list_volumes.rs index 3a1c71326..17255414a 100644 --- a/crates/kit/src/libvirt/list_volumes.rs +++ b/crates/kit/src/libvirt/list_volumes.rs @@ -3,7 +3,6 @@ //! This module provides functionality to discover and list bootc volumes //! with their container image metadata and creation information. -use crate::xml_utils; use clap::Parser; use color_eyre::{eyre::eyre, Result}; use comfy_table::{presets::UTF8_FULL, Table}; @@ -177,21 +176,15 @@ impl LibvirtListVolumesOpts { } // Get metadata from volume XML - let xml_output = self - .virsh_command(global_opts) - .args(&["vol-dumpxml", volume_name, "--pool", &self.pool]) - .output()?; - let mut source_image = None; let mut source_digest = None; let mut created = None; - if xml_output.status.success() { - let xml = String::from_utf8(xml_output.stdout)?; - debug!("Volume XML for {}: {}", volume_name, xml); - - // Parse XML once and search for metadata - let dom = xml_utils::parse_xml_dom(&xml)?; + if let Ok(dom) = super::run::run_virsh_xml( + global_opts.connect.as_deref(), + &["vol-dumpxml", volume_name, "--pool", &self.pool], + ) { + debug!("Volume XML retrieved for {}", volume_name); // First try to extract metadata from description field (new format) if let Some(description_node) = dom.find("description") { diff --git a/crates/kit/src/libvirt/run.rs b/crates/kit/src/libvirt/run.rs index 28edb4ffc..3888f8b40 100644 --- a/crates/kit/src/libvirt/run.rs +++ b/crates/kit/src/libvirt/run.rs @@ -41,6 +41,36 @@ pub(crate) fn run_virsh_cmd(connect_uri: Option<&str>, args: &[&str], err_msg: & Ok(()) } +/// Run a virsh command that returns XML and parse it directly +/// +/// This helper function consolidates the common pattern of: +/// 1. Running a virsh command with arguments +/// 2. Checking if the command succeeded +/// 3. Parsing the XML output from the stdout bytes +/// +/// # Arguments +/// * `connect_uri` - Optional libvirt connection URI +/// * `args` - Arguments to pass to virsh +/// +/// # Returns +/// The parsed XML as an XmlNode +pub fn run_virsh_xml(connect_uri: Option<&str>, args: &[&str]) -> Result { + let mut cmd = virsh_command(connect_uri)?; + cmd.args(args); + + let output = cmd.output().context("Failed to run virsh")?; + + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + return Err(eyre::eyre!("virsh command failed: {}", stderr)); + } + + // Parse XML directly from bytes + let xml = std::str::from_utf8(&output.stdout).context("Invalid UTF-8 in virsh output")?; + + xml_utils::parse_xml_dom(xml).context("Failed to parse XML") +} + /// Firmware type for virtual machines #[derive(Debug, Clone, Copy, PartialEq, Eq, ValueEnum)] #[clap(rename_all = "kebab-case")] @@ -525,24 +555,8 @@ pub fn get_libvirt_storage_pool_path(connect_uri: Option<&str>) -> Result Result { - let output = global_opts - .virsh_command() - .args(&["dumpxml", &self.domain_name]) - .output()?; - - if !output.status.success() { - return Err(eyre!("Failed to get domain XML for '{}'", self.domain_name)); - } - - let xml = String::from_utf8(output.stdout)?; - debug!("Domain XML for SSH extraction: {}", xml); - - // Parse XML once for all metadata extraction - let dom = xml_utils::parse_xml_dom(&xml) - .map_err(|e| eyre!("Failed to parse domain XML: {}", e))?; + let dom = super::run::run_virsh_xml( + global_opts.connect.as_deref(), + &["dumpxml", &self.domain_name], + ) + .context(format!( + "Failed to get domain XML for '{}'", + self.domain_name + ))?; + debug!("Domain XML retrieved for SSH extraction"); // Extract SSH metadata from bootc:container section // First try the new base64 encoded format @@ -381,7 +377,7 @@ pub fn run_ssh_impl( #[cfg(test)] mod tests { - use super::*; + use crate::xml_utils; #[test] fn test_ssh_metadata_extraction() {