Skip to content

Commit b4ac4ce

Browse files
cgwalterscgwalters
committed
Clean up libvirt XML parsing
With a helper fn. Signed-off-by: Colin Walters <[email protected]>
1 parent 6086061 commit b4ac4ce

File tree

5 files changed

+61
-84
lines changed

5 files changed

+61
-84
lines changed

‎crates/kit/src/domain_list.rs‎

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

136136
/// Get domain XML metadata as parsed DOM
137137
pub fn get_domain_xml(&self, domain_name: &str) -> Result<xml_utils::XmlNode> {
138-
let output = self
139-
.virsh_command()
140-
.args(&["dumpxml", domain_name])
141-
.output()
142-
.with_context(|| format!("Failed to dump XML for domain '{}'", domain_name))?;
143-
144-
if !output.status.success() {
145-
let stderr = String::from_utf8_lossy(&output.stderr);
146-
return Err(color_eyre::eyre::eyre!(
147-
"Failed to get XML for domain '{}': {}",
148-
domain_name,
149-
stderr
150-
));
151-
}
152-
153-
let xml = String::from_utf8(output.stdout)?;
154-
xml_utils::parse_xml_dom(&xml)
155-
.with_context(|| format!("Failed to parse XML for domain '{}'", domain_name))
138+
crate::libvirt::run::run_virsh_xml(self.connect_uri.as_deref(), &["dumpxml", domain_name])
139+
.context(format!("Failed to get XML for domain '{}'", domain_name))
156140
}
157141

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

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

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -444,25 +444,14 @@ pub fn clone_from_base(
444444
color_eyre::eyre::eyre!("Base disk path has no filename: {:?}", base_disk_path)
445445
})?;
446446

447-
let mut dumpxml_cmd = super::run::virsh_command(connect_uri)?;
448-
dumpxml_cmd.args(&["vol-dumpxml", "--pool", "default", base_disk_filename]);
449-
450-
let output = dumpxml_cmd
451-
.output()
452-
.with_context(|| format!("Failed to run virsh vol-dumpxml for {}", base_disk_filename))?;
453-
454-
if !output.status.success() {
455-
let stderr = String::from_utf8_lossy(&output.stderr);
456-
return Err(color_eyre::eyre::eyre!(
457-
"Failed to get base disk info: {}",
458-
stderr
459-
));
460-
}
461-
462-
// Parse the capacity from vol-dumpxml output (XML with capacity in bytes)
463-
let xml = String::from_utf8_lossy(&output.stdout);
464-
let dom = crate::xml_utils::parse_xml_dom(&xml)
465-
.with_context(|| "Failed to parse vol-dumpxml output")?;
447+
let dom = super::run::run_virsh_xml(
448+
connect_uri,
449+
&["vol-dumpxml", "--pool", "default", base_disk_filename],
450+
)
451+
.context(format!(
452+
"Failed to get base disk info for {}",
453+
base_disk_filename
454+
))?;
466455

467456
let capacity_node = dom
468457
.find("capacity")

‎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
@@ -26,6 +26,36 @@ pub(super) fn virsh_command(connect_uri: Option<&str>) -> Result<std::process::C
2626
Ok(cmd)
2727
}
2828

29+
/// Run a virsh command that returns XML and parse it directly
30+
///
31+
/// This helper function consolidates the common pattern of:
32+
/// 1. Running a virsh command with arguments
33+
/// 2. Checking if the command succeeded
34+
/// 3. Parsing the XML output from the stdout bytes
35+
///
36+
/// # Arguments
37+
/// * `connect_uri` - Optional libvirt connection URI
38+
/// * `args` - Arguments to pass to virsh
39+
///
40+
/// # Returns
41+
/// The parsed XML as an XmlNode
42+
pub fn run_virsh_xml(connect_uri: Option<&str>, args: &[&str]) -> Result<xml_utils::XmlNode> {
43+
let mut cmd = virsh_command(connect_uri)?;
44+
cmd.args(args);
45+
46+
let output = cmd.output().context("Failed to run virsh")?;
47+
48+
if !output.status.success() {
49+
let stderr = String::from_utf8_lossy(&output.stderr);
50+
return Err(eyre::eyre!("virsh command failed: {}", stderr));
51+
}
52+
53+
// Parse XML directly from bytes
54+
let xml = std::str::from_utf8(&output.stdout).context("Invalid UTF-8 in virsh output")?;
55+
56+
xml_utils::parse_xml_dom(xml).context("Failed to parse XML")
57+
}
58+
2959
/// Firmware type for virtual machines
3060
#[derive(Debug, Clone, Copy, PartialEq, Eq, ValueEnum)]
3161
#[clap(rename_all = "kebab-case")]
@@ -343,24 +373,8 @@ pub fn get_libvirt_storage_pool_path(connect_uri: Option<&str>) -> Result<Utf8Pa
343373
// Ensure pool exists before querying
344374
ensure_default_pool(connect_uri)?;
345375

346-
let mut cmd = virsh_command(connect_uri)?;
347-
cmd.args(&["pool-dumpxml", "default"]);
348-
let output = cmd
349-
.output()
350-
.with_context(|| "Failed to query libvirt storage pool")?;
351-
352-
if !output.status.success() {
353-
let stderr = String::from_utf8_lossy(&output.stderr);
354-
let uri_desc = connect_uri.unwrap_or("default connection");
355-
return Err(color_eyre::eyre::eyre!(
356-
"Failed to get default storage pool info for {}: {}",
357-
uri_desc,
358-
stderr
359-
));
360-
}
361-
362-
let xml = String::from_utf8(output.stdout).with_context(|| "Invalid UTF-8 in virsh output")?;
363-
let dom = xml_utils::parse_xml_dom(&xml).with_context(|| "Failed to parse storage pool XML")?;
376+
let dom = run_virsh_xml(connect_uri, &["pool-dumpxml", "default"])
377+
.context("Failed to get default storage pool info")?;
364378

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

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

Lines changed: 14 additions & 17 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
@@ -382,6 +378,7 @@ pub fn run_ssh_impl(
382378
#[cfg(test)]
383379
mod tests {
384380
use super::*;
381+
use crate::xml_utils;
385382

386383
#[test]
387384
fn test_ssh_metadata_extraction() {

0 commit comments

Comments
 (0)