Clean up libvirt XML parsing

With a helper fn.

Signed-off-by: Colin Walters <[email protected]>

Author: cgwalters

crates/kit/src/domain_list.rs

@@ -135,24 +135,8 @@ impl DomainLister {
 
     /// Get domain XML metadata as parsed DOM
     pub fn get_domain_xml(&self, domain_name: &str) -> Result<xml_utils::XmlNode> {
-        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

crates/kit/src/libvirt/base_disks.rs

@@ -461,25 +461,14 @@ pub fn clone_from_base(
         color_eyre::eyre::eyre!("Base disk path has no filename: {:?}", base_disk_path)
     })?;
 
-    let mut dumpxml_cmd = super::run::virsh_command(connect_uri)?;
-    dumpxml_cmd.args(&["vol-dumpxml", "--pool", "default", base_disk_filename]);
-
-    let output = dumpxml_cmd
-        .output()
-        .with_context(|| format!("Failed to run virsh vol-dumpxml for {}", base_disk_filename))?;
-
-    if !output.status.success() {
-        let stderr = String::from_utf8_lossy(&output.stderr);
-        return Err(color_eyre::eyre::eyre!(
-            "Failed to get base disk info: {}",
-            stderr
-        ));
-    }
-
-    // Parse the capacity from vol-dumpxml output (XML with capacity in bytes)
-    let xml = String::from_utf8_lossy(&output.stdout);
-    let dom = crate::xml_utils::parse_xml_dom(&xml)
-        .with_context(|| "Failed to parse vol-dumpxml output")?;
+    let dom = super::run::run_virsh_xml(
+        connect_uri,
+        &["vol-dumpxml", "--pool", "default", base_disk_filename],
+    )
+    .context(format!(
+        "Failed to get base disk info for {}",
+        base_disk_filename
+    ))?;
 
     let capacity_node = dom
         .find("capacity")

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") {

crates/kit/src/libvirt/run.rs

@@ -26,6 +26,36 @@ pub(super) fn virsh_command(connect_uri: Option<&str>) -> Result<std::process::C
     Ok(cmd)
 }
 
+/// 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<xml_utils::XmlNode> {
+    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")]
@@ -343,24 +373,8 @@ pub fn get_libvirt_storage_pool_path(connect_uri: Option<&str>) -> Result<Utf8Pa
     // Ensure pool exists before querying
     ensure_default_pool(connect_uri)?;
 
-    let mut cmd = virsh_command(connect_uri)?;
-    cmd.args(&["pool-dumpxml", "default"]);
-    let output = cmd
-        .output()
-        .with_context(|| "Failed to query libvirt storage pool")?;
-
-    if !output.status.success() {
-        let stderr = String::from_utf8_lossy(&output.stderr);
-        let uri_desc = connect_uri.unwrap_or("default connection");
-        return Err(color_eyre::eyre::eyre!(
-            "Failed to get default storage pool info for {}: {}",
-            uri_desc,
-            stderr
-        ));
-    }
-
-    let xml = String::from_utf8(output.stdout).with_context(|| "Invalid UTF-8 in virsh output")?;
-    let dom = xml_utils::parse_xml_dom(&xml).with_context(|| "Failed to parse storage pool XML")?;
+    let dom = run_virsh_xml(connect_uri, &["pool-dumpxml", "default"])
+        .context("Failed to get default storage pool info")?;
 
     if let Some(path_node) = dom.find("path") {
         let path_str = path_node.text_content().trim();

crates/kit/src/libvirt/ssh.rs

@@ -4,10 +4,12 @@
 //! with SSH key injection, automatically retrieving SSH credentials from domain XML
 //! metadata and establishing connection using embedded private keys.
 
-use crate::xml_utils;
 use base64::Engine;
 use clap::Parser;
-use color_eyre::{eyre::eyre, Result};
+use color_eyre::{
+    eyre::{eyre, Context},
+    Result,
+};
 use std::fs::Permissions;
 use std::io::Write;
 use std::os::unix::fs::PermissionsExt as _;
@@ -85,21 +87,15 @@ impl LibvirtSshOpts {
         &self,
         global_opts: &crate::libvirt::LibvirtOptions,
     ) -> Result<DomainSshConfig> {
-        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
@@ -382,6 +378,7 @@ pub fn run_ssh_impl(
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::xml_utils;
 
     #[test]
     fn test_ssh_metadata_extraction() {