Skip to content

Commit 9063992

Browse files
committed
libvirt: Support generating unique names directly
This is more convenient from the CLI and in theory we can make it race-free. Motivated by supporting tests. Signed-off-by: Colin Walters <[email protected]>
1 parent de51faf commit 9063992

File tree

3 files changed

+263
-115
lines changed

3 files changed

+263
-115
lines changed

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

Lines changed: 56 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -14,29 +14,25 @@ use crate::{get_bck_command, get_test_image, run_bcvk};
1414
pub fn test_base_disk_creation_and_reuse() {
1515
let test_image = get_test_image();
1616

17-
// Generate unique names for test VMs
18-
let timestamp = std::time::SystemTime::now()
19-
.duration_since(std::time::UNIX_EPOCH)
20-
.unwrap()
21-
.as_secs();
22-
let vm1_name = format!("test-base-disk-vm1-{}", timestamp);
23-
let vm2_name = format!("test-base-disk-vm2-{}", timestamp);
17+
// Generate unique names for test VMs using shortuuid pattern
18+
let vm1_name_template = "test-base-disk-vm1-{shortuuid}";
19+
let vm2_name_template = "test-base-disk-vm2-{shortuuid}";
2420

2521
println!("Testing base disk creation and reuse");
26-
println!("VM1: {}", vm1_name);
27-
println!("VM2: {}", vm2_name);
2822

29-
// Cleanup any existing test domains
30-
cleanup_domain(&vm1_name);
31-
cleanup_domain(&vm2_name);
23+
// Create temp files for domain names
24+
let vm1_id_file = tempfile::NamedTempFile::new().expect("Failed to create temp file");
25+
let vm1_id_path = vm1_id_file.path().to_str().expect("Invalid temp file path");
3226

3327
// Create first VM - this should create a new base disk
3428
println!("Creating first VM (should create base disk)...");
3529
let vm1_output = run_bcvk(&[
3630
"libvirt",
3731
"run",
3832
"--name",
39-
&vm1_name,
33+
vm1_name_template,
34+
"--write-id-to",
35+
vm1_id_path,
4036
"--filesystem",
4137
"ext4",
4238
&test_image,
@@ -47,12 +43,19 @@ pub fn test_base_disk_creation_and_reuse() {
4743
println!("VM1 stderr: {}", vm1_output.stderr);
4844

4945
if !vm1_output.success() {
50-
cleanup_domain(&vm1_name);
51-
cleanup_domain(&vm2_name);
52-
46+
// Attempt cleanup before panicking
47+
let _ = std::fs::read_to_string(vm1_id_path).map(|name| cleanup_domain(name.trim()));
5348
panic!("Failed to create first VM: {}", vm1_output.stderr);
5449
}
5550

51+
// Read the domain name from the file
52+
let vm1_name = std::fs::read_to_string(vm1_id_path)
53+
.expect("Failed to read VM1 domain name from file")
54+
.trim()
55+
.to_string();
56+
57+
println!("Created VM1: {}", vm1_name);
58+
5659
// Verify base disk was created
5760
assert!(
5861
vm1_output.stdout.contains("Using base disk") || vm1_output.stdout.contains("base disk"),
@@ -61,11 +64,16 @@ pub fn test_base_disk_creation_and_reuse() {
6164

6265
// Create second VM - this should reuse the base disk
6366
println!("Creating second VM (should reuse base disk)...");
67+
let vm2_id_file = tempfile::NamedTempFile::new().expect("Failed to create temp file");
68+
let vm2_id_path = vm2_id_file.path().to_str().expect("Invalid temp file path");
69+
6470
let vm2_output = run_bcvk(&[
6571
"libvirt",
6672
"run",
6773
"--name",
68-
&vm2_name,
74+
vm2_name_template,
75+
"--write-id-to",
76+
vm2_id_path,
6977
"--filesystem",
7078
"ext4",
7179
&test_image,
@@ -75,14 +83,24 @@ pub fn test_base_disk_creation_and_reuse() {
7583
println!("VM2 stdout: {}", vm2_output.stdout);
7684
println!("VM2 stderr: {}", vm2_output.stderr);
7785

78-
// Cleanup before assertions
79-
cleanup_domain(&vm1_name);
80-
cleanup_domain(&vm2_name);
81-
8286
if !vm2_output.success() {
87+
// Cleanup VM1 before panicking
88+
cleanup_domain(&vm1_name);
8389
panic!("Failed to create second VM: {}", vm2_output.stderr);
8490
}
8591

92+
// Read the domain name from the file
93+
let vm2_name = std::fs::read_to_string(vm2_id_path)
94+
.expect("Failed to read VM2 domain name from file")
95+
.trim()
96+
.to_string();
97+
98+
println!("Created VM2: {}", vm2_name);
99+
100+
// Cleanup before assertions
101+
cleanup_domain(&vm1_name);
102+
cleanup_domain(&vm2_name);
103+
86104
// Verify base disk was reused (should be faster and mention using existing)
87105
assert!(
88106
vm2_output.stdout.contains("Using base disk") || vm2_output.stdout.contains("base disk"),
@@ -170,34 +188,42 @@ pub fn test_base_disks_prune_dry_run() {
170188
pub fn test_vm_disk_references_base() {
171189
let test_image = get_test_image();
172190

173-
let timestamp = std::time::SystemTime::now()
174-
.duration_since(std::time::UNIX_EPOCH)
175-
.unwrap()
176-
.as_secs();
177-
let vm_name = format!("test-disk-ref-{}", timestamp);
191+
let vm_name_template = "test-disk-ref-{shortuuid}";
178192

179193
println!("Testing VM disk references base disk");
180194

181-
cleanup_domain(&vm_name);
195+
// Create temp file for domain name
196+
let id_file = tempfile::NamedTempFile::new().expect("Failed to create temp file");
197+
let id_path = id_file.path().to_str().expect("Invalid temp file path");
182198

183199
// Create VM
184200
let output = run_bcvk(&[
185201
"libvirt",
186202
"run",
187203
"--name",
188-
&vm_name,
204+
vm_name_template,
205+
"--write-id-to",
206+
id_path,
189207
"--filesystem",
190208
"ext4",
191209
&test_image,
192210
])
193211
.expect("Failed to create VM");
194212

195213
if !output.success() {
196-
cleanup_domain(&vm_name);
197-
214+
// Attempt cleanup before panicking
215+
let _ = std::fs::read_to_string(id_path).map(|name| cleanup_domain(name.trim()));
198216
panic!("Failed to create VM: {}", output.stderr);
199217
}
200218

219+
// Read the domain name from the file
220+
let vm_name = std::fs::read_to_string(id_path)
221+
.expect("Failed to read domain name from file")
222+
.trim()
223+
.to_string();
224+
225+
println!("Created VM: {}", vm_name);
226+
201227
// Get VM disk path from domain XML
202228
let dumpxml_output = Command::new("virsh")
203229
.args(&["dumpxml", &vm_name])
@@ -238,7 +264,6 @@ pub fn test_vm_disk_references_base() {
238264
println!("✓ VM disk reference test passed");
239265
}
240266

241-
/// Helper function to cleanup domain and its disk
242267
fn cleanup_domain(domain_name: &str) {
243268
println!("Cleaning up domain: {}", domain_name);
244269

0 commit comments

Comments
 (0)