Skip to content

Commit 087e8d5

Browse files
committed
libvirt: Fix --transient --replace removing non-existent domains
When replacing a transient VM, the old VM disappears after `virsh destroy` since transient domains are not persistent. Previously, the remove logic would then fail trying to `virsh undefine` a domain that no longer exists. Detect transient VMs explicitly to handle their removal. Assisted-by: OpenCode (claude-sonnet-4-20250514) Signed-off-by: Colin Walters <[email protected]>
1 parent d5cee3e commit 087e8d5

File tree

2 files changed

+186
-4
lines changed

2 files changed

+186
-4
lines changed

crates/integration-tests/src/tests/libvirt_verb.rs

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1211,6 +1211,128 @@ fn test_libvirt_run_transient_vm() -> Result<()> {
12111211
}
12121212
integration_test!(test_libvirt_run_transient_vm);
12131213

1214+
/// Test transient VM with --replace functionality
1215+
///
1216+
/// This tests that `bcvk libvirt run --transient --replace` works correctly:
1217+
/// 1. Create a transient VM
1218+
/// 2. Replace it with another transient VM using --replace
1219+
/// 3. Verify the replacement works (no errors about undefine on transient domains)
1220+
fn test_libvirt_run_transient_replace() -> Result<()> {
1221+
let test_image = get_test_image();
1222+
1223+
// Generate unique domain name for this test
1224+
let domain_name = format!("test-transient-replace-{}", random_suffix());
1225+
1226+
println!(
1227+
"Testing transient VM with --replace, domain: {}",
1228+
domain_name
1229+
);
1230+
1231+
// Cleanup any existing domain with this name
1232+
cleanup_domain(&domain_name);
1233+
1234+
// Create initial transient domain
1235+
println!("Creating initial transient domain...");
1236+
let create_output = run_bcvk(&[
1237+
"libvirt",
1238+
"run",
1239+
"--name",
1240+
&domain_name,
1241+
"--label",
1242+
LIBVIRT_INTEGRATION_TEST_LABEL,
1243+
"--transient",
1244+
"--filesystem",
1245+
"ext4",
1246+
&test_image,
1247+
])
1248+
.expect("Failed to run libvirt run with --transient");
1249+
1250+
if !create_output.success() {
1251+
cleanup_domain(&domain_name);
1252+
panic!(
1253+
"Failed to create initial transient domain: {}",
1254+
create_output.stderr
1255+
);
1256+
}
1257+
println!("✓ Initial transient domain created: {}", domain_name);
1258+
1259+
// Verify domain is transient
1260+
let dominfo_output = Command::new("virsh")
1261+
.args(&["dominfo", &domain_name])
1262+
.output()
1263+
.expect("Failed to run virsh dominfo");
1264+
1265+
let dominfo = String::from_utf8_lossy(&dominfo_output.stdout);
1266+
assert!(
1267+
dominfo.contains("Persistent:") && dominfo.contains("no"),
1268+
"Domain should be transient. dominfo: {}",
1269+
dominfo
1270+
);
1271+
println!("✓ Initial domain is transient (Persistent: no)");
1272+
1273+
// Now replace the transient domain with another transient domain
1274+
println!("Replacing transient domain with --transient --replace...");
1275+
let replace_output = run_bcvk(&[
1276+
"libvirt",
1277+
"run",
1278+
"--name",
1279+
&domain_name,
1280+
"--label",
1281+
LIBVIRT_INTEGRATION_TEST_LABEL,
1282+
"--transient",
1283+
"--replace",
1284+
"--filesystem",
1285+
"ext4",
1286+
&test_image,
1287+
])
1288+
.expect("Failed to run libvirt run with --transient --replace");
1289+
1290+
println!("Replace stdout: {}", replace_output.stdout);
1291+
println!("Replace stderr: {}", replace_output.stderr);
1292+
1293+
if !replace_output.success() {
1294+
cleanup_domain(&domain_name);
1295+
panic!(
1296+
"Failed to replace transient domain: {}",
1297+
replace_output.stderr
1298+
);
1299+
}
1300+
println!("✓ Successfully replaced transient domain");
1301+
1302+
// Verify the new domain exists and is transient
1303+
let dominfo_output = Command::new("virsh")
1304+
.args(&["dominfo", &domain_name])
1305+
.output()
1306+
.expect("Failed to run virsh dominfo after replace");
1307+
1308+
if !dominfo_output.status.success() {
1309+
cleanup_domain(&domain_name);
1310+
panic!("Domain should exist after --transient --replace");
1311+
}
1312+
1313+
let dominfo = String::from_utf8_lossy(&dominfo_output.stdout);
1314+
assert!(
1315+
dominfo.contains("Persistent:") && dominfo.contains("no"),
1316+
"Replaced domain should still be transient. dominfo: {}",
1317+
dominfo
1318+
);
1319+
println!("✓ Replaced domain is transient (Persistent: no)");
1320+
1321+
// Verify it's running
1322+
assert!(
1323+
dominfo.contains("running") || dominfo.contains("idle"),
1324+
"Replaced transient domain should be running. dominfo: {}",
1325+
dominfo
1326+
);
1327+
println!("✓ Replaced transient domain is running");
1328+
1329+
// Cleanup
1330+
cleanup_domain(&domain_name);
1331+
println!("✓ Transient --replace test passed");
1332+
Ok(())
1333+
}
1334+
integration_test!(test_libvirt_run_transient_replace);
1335+
12141336
/// Test automatic bind mount functionality with systemd mount units
12151337
/// Also validates kernel argument (--karg) functionality
12161338
fn test_libvirt_run_bind_mounts() -> Result<()> {

crates/kit/src/libvirt/rm.rs

Lines changed: 64 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,41 @@
66
use clap::Parser;
77
use color_eyre::Result;
88

9+
/// Check if a domain is persistent (vs transient)
10+
///
11+
/// Returns true if the domain is persistent, false if transient.
12+
/// Transient domains disappear when destroyed, so they don't need undefine.
13+
fn is_domain_persistent(
14+
global_opts: &crate::libvirt::LibvirtOptions,
15+
vm_name: &str,
16+
) -> Result<bool> {
17+
let output = global_opts
18+
.virsh_command()
19+
.args(&["dominfo", vm_name])
20+
.output()
21+
.map_err(|e| color_eyre::eyre::eyre!("Failed to get domain info: {}", e))?;
22+
23+
if !output.status.success() {
24+
let stderr = String::from_utf8_lossy(&output.stderr);
25+
return Err(color_eyre::eyre::eyre!(
26+
"Failed to get domain info for '{}': {}",
27+
vm_name,
28+
stderr
29+
));
30+
}
31+
32+
let stdout = String::from_utf8_lossy(&output.stdout);
33+
// Look for "Persistent: yes" or "Persistent: no"
34+
for line in stdout.lines() {
35+
if let Some(value) = line.strip_prefix("Persistent:") {
36+
return Ok(value.trim() == "yes");
37+
}
38+
}
39+
40+
// Default to persistent if we can't determine
41+
Ok(true)
42+
}
43+
944
/// Options for removing a libvirt domain
1045
#[derive(Debug, Parser)]
1146
pub struct LibvirtRmOpts {
@@ -29,6 +64,7 @@ fn remove_vm_impl(
2964
global_opts: &crate::libvirt::LibvirtOptions,
3065
vm_name: &str,
3166
state: &str,
67+
is_persistent: bool,
3268
domain_info: &crate::domain_list::PodmanBootcDomain,
3369
stop_if_running: bool,
3470
) -> Result<()> {
@@ -51,6 +87,11 @@ fn remove_vm_impl(
5187
stderr
5288
));
5389
}
90+
91+
// Transient VMs disappear after destroy, so we're done
92+
if !is_persistent {
93+
return Ok(());
94+
}
5495
} else {
5596
return Err(color_eyre::eyre::eyre!(
5697
"VM '{}' is running. Cannot remove without stopping.",
@@ -104,16 +145,31 @@ pub fn remove_vm_forced(
104145
};
105146

106147
// Check if domain exists and get its state
107-
let state = lister
108-
.get_domain_state(vm_name)
109-
.map_err(|_| color_eyre::eyre::eyre!("VM '{}' not found", vm_name))?;
148+
let state = match lister.get_domain_state(vm_name) {
149+
Ok(s) => s,
150+
Err(_) => {
151+
// Domain doesn't exist - this is OK for replace scenarios
152+
// where a transient VM was already destroyed
153+
return Ok(());
154+
}
155+
};
156+
157+
// Check if domain is persistent (transient VMs disappear after destroy)
158+
let is_persistent = is_domain_persistent(global_opts, vm_name)?;
110159

111160
// Get domain info for disk cleanup
112161
let domain_info = lister
113162
.get_domain_info(vm_name)
114163
.with_context(|| format!("Failed to get info for VM '{}'", vm_name))?;
115164

116-
remove_vm_impl(global_opts, vm_name, &state, &domain_info, stop_if_running)
165+
remove_vm_impl(
166+
global_opts,
167+
vm_name,
168+
&state,
169+
is_persistent,
170+
&domain_info,
171+
stop_if_running,
172+
)
117173
}
118174

119175
/// Execute the libvirt rm command
@@ -132,6 +188,9 @@ pub fn run(global_opts: &crate::libvirt::LibvirtOptions, opts: LibvirtRmOpts) ->
132188
.get_domain_state(&opts.name)
133189
.map_err(|_| color_eyre::eyre::eyre!("VM '{}' not found", opts.name))?;
134190

191+
// Check if domain is persistent (transient VMs disappear after destroy)
192+
let is_persistent = is_domain_persistent(global_opts, &opts.name)?;
193+
135194
// Get domain info for display
136195
let domain_info = lister
137196
.get_domain_info(&opts.name)
@@ -175,6 +234,7 @@ pub fn run(global_opts: &crate::libvirt::LibvirtOptions, opts: LibvirtRmOpts) ->
175234
global_opts,
176235
&opts.name,
177236
&state,
237+
is_persistent,
178238
&domain_info,
179239
opts.stop || opts.force,
180240
)?;

0 commit comments

Comments
 (0)