Skip to content

Commit 48f2cf0

Browse files
committed
libvirt: Add --replace to run, make rm -f stop by default
When I say `--force` I mean it; needing to stop the VM manually was just annoying. Add run --replace/-R to forcibly destroy and recreate a VM with the same name, matching podman run --replace behavior. This is useful for quick iteration during development and testing. Assisted-by: Claude Code (Sonnet 4.5) Signed-off-by: Colin Walters <walters@verbum.org>
1 parent cec4402 commit 48f2cf0

File tree

7 files changed

+311
-82
lines changed

7 files changed

+311
-82
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/integration-tests/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ camino = "1.1.12"
3434
regex = "1"
3535
linkme = "0.3.30"
3636
paste = "1.0"
37+
rand = { workspace = true }
3738

3839
[lints]
3940
workspace = true

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

Lines changed: 175 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,16 @@ use crate::{
1717
};
1818
use bcvk::xml_utils::parse_xml_dom;
1919

20+
/// Generate a random alphanumeric suffix for VM names to avoid collisions
21+
fn random_suffix() -> String {
22+
use rand::{distr::Alphanumeric, Rng};
23+
rand::rng()
24+
.sample_iter(&Alphanumeric)
25+
.take(8)
26+
.map(char::from)
27+
.collect()
28+
}
29+
2030
/// Test libvirt list functionality (lists domains)
2131
fn test_libvirt_list_functionality() -> Result<()> {
2232
let bck = get_bck_command()?;
@@ -210,13 +220,7 @@ fn test_libvirt_comprehensive_workflow() -> Result<()> {
210220
let bck = get_bck_command()?;
211221

212222
// Generate unique domain name for this test
213-
let domain_name = format!(
214-
"test-workflow-{}",
215-
std::time::SystemTime::now()
216-
.duration_since(std::time::UNIX_EPOCH)
217-
.unwrap()
218-
.as_secs()
219-
);
223+
let domain_name = format!("test-workflow-{}", random_suffix());
220224

221225
println!(
222226
"Testing comprehensive libvirt workflow for domain: {}",
@@ -383,7 +387,128 @@ fn test_libvirt_comprehensive_workflow() -> Result<()> {
383387
);
384388
println!("✓ VM is running and accessible");
385389

386-
// Cleanup domain
390+
// Test 6: Test `rm -f` stops running VMs by default
391+
println!("Test 6: Testing `rm -f` stops running VMs without --stop flag...");
392+
let rm_test_domain = create_test_vm_and_assert("test-rm", &test_image, &domain_name)?;
393+
394+
// Verify it's running
395+
let rm_dominfo_output = Command::new("virsh")
396+
.args(&["dominfo", &rm_test_domain])
397+
.output()
398+
.expect("Failed to run virsh dominfo for rm test VM");
399+
400+
let rm_info = String::from_utf8_lossy(&rm_dominfo_output.stdout);
401+
assert!(
402+
rm_info.contains("running") || rm_info.contains("idle"),
403+
"Test VM should be running before rm test"
404+
);
405+
println!("✓ Test VM is running: {}", rm_test_domain);
406+
407+
// Test rm -f WITHOUT --stop flag (should succeed and stop the VM)
408+
println!(
409+
"Running `bcvk libvirt rm -f {}` (without --stop)...",
410+
rm_test_domain
411+
);
412+
let rm_output =
413+
run_bcvk(&["libvirt", "rm", "-f", &rm_test_domain]).expect("Failed to run libvirt rm -f");
414+
415+
assert!(
416+
rm_output.success(),
417+
"rm -f should succeed in stopping and removing running VM without --stop flag. stderr: {}",
418+
rm_output.stderr
419+
);
420+
println!("✓ rm -f successfully stopped and removed running VM");
421+
422+
// Verify the VM is actually removed
423+
let list_check = Command::new("virsh")
424+
.args(&["list", "--all", "--name"])
425+
.output()
426+
.expect("Failed to list domains");
427+
428+
let domain_list = String::from_utf8_lossy(&list_check.stdout);
429+
assert!(
430+
!domain_list.contains(&rm_test_domain),
431+
"VM should be removed after rm -f"
432+
);
433+
println!("✓ VM successfully removed from domain list");
434+
435+
// Test 7: Test `run --replace` replaces existing VM
436+
println!("Test 7: Testing `run --replace` replaces existing VM...");
437+
let replace_test_domain = create_test_vm_and_assert("test-replace", &test_image, &domain_name)?;
438+
439+
// Verify initial VM exists
440+
let initial_list = Command::new("virsh")
441+
.args(&["list", "--all", "--name"])
442+
.output()
443+
.expect("Failed to list domains");
444+
445+
let initial_domain_list = String::from_utf8_lossy(&initial_list.stdout);
446+
assert!(
447+
initial_domain_list.contains(&replace_test_domain),
448+
"Initial VM should exist before replace"
449+
);
450+
println!("✓ Initial VM exists: {}", replace_test_domain);
451+
452+
// Run with --replace flag (should replace existing VM)
453+
println!(
454+
"Running `bcvk libvirt run --replace --name {}`...",
455+
replace_test_domain
456+
);
457+
let replace_output = run_bcvk(&[
458+
"libvirt",
459+
"run",
460+
"--replace",
461+
"--name",
462+
&replace_test_domain,
463+
"--label",
464+
LIBVIRT_INTEGRATION_TEST_LABEL,
465+
"--filesystem",
466+
"ext4",
467+
&test_image,
468+
])
469+
.expect("Failed to run libvirt run --replace");
470+
471+
if !replace_output.success() {
472+
cleanup_domain(&replace_test_domain);
473+
cleanup_domain(&domain_name);
474+
panic!(
475+
"Failed to replace VM with --replace flag: {}",
476+
replace_output.stderr
477+
);
478+
}
479+
println!("✓ Successfully replaced VM with --replace flag");
480+
481+
// Verify VM still exists with same name
482+
let replaced_list = Command::new("virsh")
483+
.args(&["list", "--all", "--name"])
484+
.output()
485+
.expect("Failed to list domains");
486+
487+
let replaced_domain_list = String::from_utf8_lossy(&replaced_list.stdout);
488+
assert!(
489+
replaced_domain_list.contains(&replace_test_domain),
490+
"Replaced VM should exist with same name"
491+
);
492+
println!("✓ Replaced VM exists with same name");
493+
494+
// Verify it's a fresh VM (should be running)
495+
let replaced_dominfo = Command::new("virsh")
496+
.args(&["dominfo", &replace_test_domain])
497+
.output()
498+
.expect("Failed to run virsh dominfo for replaced VM");
499+
500+
let replaced_info = String::from_utf8_lossy(&replaced_dominfo.stdout);
501+
assert!(
502+
replaced_info.contains("running") || replaced_info.contains("idle"),
503+
"Replaced VM should be running"
504+
);
505+
println!("✓ Replaced VM is running (fresh instance)");
506+
507+
// Cleanup replace test VM
508+
cleanup_domain(&replace_test_domain);
509+
println!("✓ Cleaned up replace test VM");
510+
511+
// Cleanup original domain
387512
cleanup_domain(&domain_name);
388513

389514
println!("✓ Comprehensive workflow test passed");
@@ -419,6 +544,44 @@ fn cleanup_domain(domain_name: &str) {
419544
}
420545
}
421546

547+
/// Helper function to create a test VM and assert success
548+
///
549+
/// Creates a VM using run_bcvk with the given prefix and test image.
550+
/// Cleans up the original domain on error and returns the created domain name.
551+
fn create_test_vm_and_assert(
552+
domain_prefix: &str,
553+
test_image: &str,
554+
original_domain: &str,
555+
) -> Result<String> {
556+
let domain_name = format!("{}-{}", domain_prefix, random_suffix());
557+
558+
println!("Creating test VM: {}", domain_name);
559+
let create_output = run_bcvk(&[
560+
"libvirt",
561+
"run",
562+
"--name",
563+
&domain_name,
564+
"--label",
565+
LIBVIRT_INTEGRATION_TEST_LABEL,
566+
"--filesystem",
567+
"ext4",
568+
test_image,
569+
])
570+
.expect("Failed to create test VM");
571+
572+
if !create_output.success() {
573+
cleanup_domain(&domain_name);
574+
cleanup_domain(original_domain);
575+
panic!(
576+
"Failed to create test VM '{}': {}",
577+
domain_name, create_output.stderr
578+
);
579+
}
580+
581+
println!("✓ Test VM created: {}", domain_name);
582+
Ok(domain_name)
583+
}
584+
422585
/// Check if libvirt supports readonly virtiofs (requires libvirt 11.0+)
423586
/// Returns true if supported, false if not supported
424587
fn check_libvirt_supports_readonly_virtiofs() -> Result<bool> {
@@ -564,13 +727,7 @@ fn test_libvirt_run_bind_storage_ro() -> Result<()> {
564727
let test_image = get_test_image();
565728

566729
// Generate unique domain name for this test
567-
let domain_name = format!(
568-
"test-bind-storage-{}",
569-
std::time::SystemTime::now()
570-
.duration_since(std::time::UNIX_EPOCH)
571-
.unwrap()
572-
.as_secs()
573-
);
730+
let domain_name = format!("test-bind-storage-{}", random_suffix());
574731

575732
println!("Testing --bind-storage-ro with domain: {}", domain_name);
576733

@@ -709,13 +866,7 @@ fn test_libvirt_run_no_storage_opts_without_bind_storage() -> Result<()> {
709866
let test_image = get_test_image();
710867

711868
// Generate unique domain name for this test
712-
let domain_name = format!(
713-
"test-no-storage-opts-{}",
714-
std::time::SystemTime::now()
715-
.duration_since(std::time::UNIX_EPOCH)
716-
.unwrap()
717-
.as_secs()
718-
);
869+
let domain_name = format!("test-no-storage-opts-{}", random_suffix());
719870

720871
println!(
721872
"Testing that STORAGE_OPTS are not injected without --bind-storage-ro for domain: {}",
@@ -855,13 +1006,7 @@ fn test_libvirt_run_transient_vm() -> Result<()> {
8551006
let test_image = get_test_image();
8561007

8571008
// Generate unique domain name for this test
858-
let domain_name = format!(
859-
"test-transient-{}",
860-
std::time::SystemTime::now()
861-
.duration_since(std::time::UNIX_EPOCH)
862-
.unwrap()
863-
.as_secs()
864-
);
1009+
let domain_name = format!("test-transient-{}", random_suffix());
8651010

8661011
println!("Testing transient VM with domain: {}", domain_name);
8671012

@@ -1021,13 +1166,7 @@ fn test_libvirt_run_bind_mounts() -> Result<()> {
10211166
let test_image = get_test_image();
10221167

10231168
// Generate unique domain name for this test
1024-
let domain_name = format!(
1025-
"test-bind-mounts-{}",
1026-
std::time::SystemTime::now()
1027-
.duration_since(std::time::UNIX_EPOCH)
1028-
.unwrap()
1029-
.as_secs()
1030-
);
1169+
let domain_name = format!("test-bind-mounts-{}", random_suffix());
10311170

10321171
println!("Testing bind mounts and kargs with domain: {}", domain_name);
10331172

0 commit comments

Comments
 (0)