Skip to content

Commit 7625a2b

Browse files
cgwalterscgwalters
authored
Clean up libvirt XML parsing (#104)
With a helper fn. Signed-off-by: Colin Walters <walters@verbum.org>
1 parent 299be54 commit 7625a2b

4 files changed

Lines changed: 53 additions & 66 deletions

File tree

‎crates/kit/src/domain_list.rs‎

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -142,24 +142,8 @@ impl DomainLister {
142142

143143
/// Get domain XML metadata as parsed DOM
144144
pub fn get_domain_xml(&self, domain_name: &str) -> Result<xml_utils::XmlNode> {
145-
let output = self
146-
.virsh_command()
147-
.args(&["dumpxml", domain_name])
148-
.output()
149-
.with_context(|| format!("Failed to dump XML for domain '{}'", domain_name))?;
150-
151-
if !output.status.success() {
152-
let stderr = String::from_utf8_lossy(&output.stderr);
153-
return Err(color_eyre::eyre::eyre!(
154-
"Failed to get XML for domain '{}': {}",
155-
domain_name,
156-
stderr
157-
));
158-
}
159-
160-
let xml = String::from_utf8(output.stdout)?;
161-
xml_utils::parse_xml_dom(&xml)
162-
.with_context(|| format!("Failed to parse XML for domain '{}'", domain_name))
145+
crate::libvirt::run::run_virsh_xml(self.connect_uri.as_deref(), &["dumpxml", domain_name])
146+
.context(format!("Failed to get XML for domain '{}'", domain_name))
163147
}
164148

165149
/// Extract podman-bootc metadata from parsed domain XML

‎crates/kit/src/libvirt/list_volumes.rs‎

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
//! This module provides functionality to discover and list bootc volumes
44
//! with their container image metadata and creation information.
55
6-
use crate::xml_utils;
76
use clap::Parser;
87
use color_eyre::{eyre::eyre, Result};
98
use comfy_table::{presets::UTF8_FULL, Table};
@@ -177,21 +176,15 @@ impl LibvirtListVolumesOpts {
177176
}
178177

179178
// Get metadata from volume XML
180-
let xml_output = self
181-
.virsh_command(global_opts)
182-
.args(&["vol-dumpxml", volume_name, "--pool", &self.pool])
183-
.output()?;
184-
185179
let mut source_image = None;
186180
let mut source_digest = None;
187181
let mut created = None;
188182

189-
if xml_output.status.success() {
190-
let xml = String::from_utf8(xml_output.stdout)?;
191-
debug!("Volume XML for {}: {}", volume_name, xml);
192-
193-
// Parse XML once and search for metadata
194-
let dom = xml_utils::parse_xml_dom(&xml)?;
183+
if let Ok(dom) = super::run::run_virsh_xml(
184+
global_opts.connect.as_deref(),
185+
&["vol-dumpxml", volume_name, "--pool", &self.pool],
186+
) {
187+
debug!("Volume XML retrieved for {}", volume_name);
195188

196189
// First try to extract metadata from description field (new format)
197190
if let Some(description_node) = dom.find("description") {

‎crates/kit/src/libvirt/run.rs‎

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,36 @@ pub(crate) fn run_virsh_cmd(connect_uri: Option<&str>, args: &[&str], err_msg: &
4141
Ok(())
4242
}
4343

44+
/// Run a virsh command that returns XML and parse it directly
45+
///
46+
/// This helper function consolidates the common pattern of:
47+
/// 1. Running a virsh command with arguments
48+
/// 2. Checking if the command succeeded
49+
/// 3. Parsing the XML output from the stdout bytes
50+
///
51+
/// # Arguments
52+
/// * `connect_uri` - Optional libvirt connection URI
53+
/// * `args` - Arguments to pass to virsh
54+
///
55+
/// # Returns
56+
/// The parsed XML as an XmlNode
57+
pub fn run_virsh_xml(connect_uri: Option<&str>, args: &[&str]) -> Result<xml_utils::XmlNode> {
58+
let mut cmd = virsh_command(connect_uri)?;
59+
cmd.args(args);
60+
61+
let output = cmd.output().context("Failed to run virsh")?;
62+
63+
if !output.status.success() {
64+
let stderr = String::from_utf8_lossy(&output.stderr);
65+
return Err(eyre::eyre!("virsh command failed: {}", stderr));
66+
}
67+
68+
// Parse XML directly from bytes
69+
let xml = std::str::from_utf8(&output.stdout).context("Invalid UTF-8 in virsh output")?;
70+
71+
xml_utils::parse_xml_dom(xml).context("Failed to parse XML")
72+
}
73+
4474
/// Firmware type for virtual machines
4575
#[derive(Debug, Clone, Copy, PartialEq, Eq, ValueEnum)]
4676
#[clap(rename_all = "kebab-case")]
@@ -525,24 +555,8 @@ pub fn get_libvirt_storage_pool_path(connect_uri: Option<&str>) -> Result<Utf8Pa
525555
// Ensure pool exists before querying
526556
ensure_default_pool(connect_uri)?;
527557

528-
let mut cmd = virsh_command(connect_uri)?;
529-
cmd.args(&["pool-dumpxml", "default"]);
530-
let output = cmd
531-
.output()
532-
.with_context(|| "Failed to query libvirt storage pool")?;
533-
534-
if !output.status.success() {
535-
let stderr = String::from_utf8_lossy(&output.stderr);
536-
let uri_desc = connect_uri.unwrap_or("default connection");
537-
return Err(color_eyre::eyre::eyre!(
538-
"Failed to get default storage pool info for {}: {}",
539-
uri_desc,
540-
stderr
541-
));
542-
}
543-
544-
let xml = String::from_utf8(output.stdout).with_context(|| "Invalid UTF-8 in virsh output")?;
545-
let dom = xml_utils::parse_xml_dom(&xml).with_context(|| "Failed to parse storage pool XML")?;
558+
let dom = run_virsh_xml(connect_uri, &["pool-dumpxml", "default"])
559+
.context("Failed to get default storage pool info")?;
546560

547561
if let Some(path_node) = dom.find("path") {
548562
let path_str = path_node.text_content().trim();

‎crates/kit/src/libvirt/ssh.rs‎

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,12 @@
44
//! with SSH key injection, automatically retrieving SSH credentials from domain XML
55
//! metadata and establishing connection using embedded private keys.
66
7-
use crate::xml_utils;
87
use base64::Engine;
98
use clap::Parser;
10-
use color_eyre::{eyre::eyre, Result};
9+
use color_eyre::{
10+
eyre::{eyre, Context},
11+
Result,
12+
};
1113
use std::fs::Permissions;
1214
use std::io::Write;
1315
use std::os::unix::fs::PermissionsExt as _;
@@ -85,21 +87,15 @@ impl LibvirtSshOpts {
8587
&self,
8688
global_opts: &crate::libvirt::LibvirtOptions,
8789
) -> Result<DomainSshConfig> {
88-
let output = global_opts
89-
.virsh_command()
90-
.args(&["dumpxml", &self.domain_name])
91-
.output()?;
92-
93-
if !output.status.success() {
94-
return Err(eyre!("Failed to get domain XML for '{}'", self.domain_name));
95-
}
96-
97-
let xml = String::from_utf8(output.stdout)?;
98-
debug!("Domain XML for SSH extraction: {}", xml);
99-
100-
// Parse XML once for all metadata extraction
101-
let dom = xml_utils::parse_xml_dom(&xml)
102-
.map_err(|e| eyre!("Failed to parse domain XML: {}", e))?;
90+
let dom = super::run::run_virsh_xml(
91+
global_opts.connect.as_deref(),
92+
&["dumpxml", &self.domain_name],
93+
)
94+
.context(format!(
95+
"Failed to get domain XML for '{}'",
96+
self.domain_name
97+
))?;
98+
debug!("Domain XML retrieved for SSH extraction");
10399

104100
// Extract SSH metadata from bootc:container section
105101
// First try the new base64 encoded format
@@ -381,7 +377,7 @@ pub fn run_ssh_impl(
381377

382378
#[cfg(test)]
383379
mod tests {
384-
use super::*;
380+
use crate::xml_utils;
385381

386382
#[test]
387383
fn test_ssh_metadata_extraction() {

0 commit comments

Comments
 (0)