Skip to content

Commit 7860d8c

Browse files
committed
libvirt: Add base disk caching infrastructure
Change how libvirt VMs are created to always start from a "base disk image". We maintain caching which derives from (base image, filesystem, install opts). Add CLI commands to manage these. Asisted-by: Claude Code Signed-off-by: Colin Walters <[email protected]>
1 parent f859990 commit 7860d8c

File tree

16 files changed

+1085
-1092
lines changed

16 files changed

+1085
-1092
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.

Justfile

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
build:
33
make
44

5+
check:
6+
cargo t --workspace --no-run
7+
58
# Run unit tests (excludes integration tests)
69
unit *ARGS:
710
#!/usr/bin/env bash

crates/integration-tests/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ name = "test-cleanup"
1717
path = "src/bin/cleanup.rs"
1818

1919
[dependencies]
20+
bcvk = { path = "../kit" }
2021
color-eyre = { workspace = true }
2122
dirs = "5.0"
2223
tracing = { workspace = true }

crates/integration-tests/src/main.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use xshell::{cmd, Shell};
1010
pub(crate) const INTEGRATION_TEST_LABEL: &str = "bcvk.integration-test=1";
1111

1212
mod tests {
13+
pub mod libvirt_base_disks;
1314
pub mod libvirt_upload_disk;
1415
pub mod libvirt_verb;
1516
pub mod mount_feature;
@@ -212,6 +213,22 @@ fn main() {
212213
tests::libvirt_verb::test_libvirt_bind_storage_ro();
213214
Ok(())
214215
}),
216+
Trial::test("libvirt_base_disk_creation_and_reuse", || {
217+
tests::libvirt_base_disks::test_base_disk_creation_and_reuse();
218+
Ok(())
219+
}),
220+
Trial::test("libvirt_base_disks_list_command", || {
221+
tests::libvirt_base_disks::test_base_disks_list_command();
222+
Ok(())
223+
}),
224+
Trial::test("libvirt_base_disks_prune_dry_run", || {
225+
tests::libvirt_base_disks::test_base_disks_prune_dry_run();
226+
Ok(())
227+
}),
228+
Trial::test("libvirt_vm_disk_references_base", || {
229+
tests::libvirt_base_disks::test_vm_disk_references_base();
230+
Ok(())
231+
}),
215232
];
216233

217234
// Run the tests and exit with the result
Lines changed: 285 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,285 @@
1+
//! Integration tests for libvirt base disk functionality
2+
//!
3+
//! Tests the base disk caching and CoW cloning system:
4+
//! - Base disk creation and reuse
5+
//! - Multiple VMs sharing the same base disk
6+
//! - base-disks list command
7+
//! - base-disks prune command
8+
9+
use std::process::Command;
10+
11+
use crate::{get_bck_command, get_test_image};
12+
13+
/// Test that base disk is created and reused for multiple VMs
14+
pub fn test_base_disk_creation_and_reuse() {
15+
let bck = get_bck_command().unwrap();
16+
let test_image = get_test_image();
17+
18+
// Generate unique names for test VMs
19+
let timestamp = std::time::SystemTime::now()
20+
.duration_since(std::time::UNIX_EPOCH)
21+
.unwrap()
22+
.as_secs();
23+
let vm1_name = format!("test-base-disk-vm1-{}", timestamp);
24+
let vm2_name = format!("test-base-disk-vm2-{}", timestamp);
25+
26+
println!("Testing base disk creation and reuse");
27+
println!("VM1: {}", vm1_name);
28+
println!("VM2: {}", vm2_name);
29+
30+
// Cleanup any existing test domains
31+
cleanup_domain(&vm1_name);
32+
cleanup_domain(&vm2_name);
33+
34+
// Create first VM - this should create a new base disk
35+
println!("Creating first VM (should create base disk)...");
36+
let vm1_output = Command::new("timeout")
37+
.args([
38+
"300s",
39+
&bck,
40+
"libvirt",
41+
"run",
42+
"--name",
43+
&vm1_name,
44+
"--filesystem",
45+
"ext4",
46+
&test_image,
47+
])
48+
.output()
49+
.expect("Failed to create first VM");
50+
51+
let vm1_stdout = String::from_utf8_lossy(&vm1_output.stdout);
52+
let vm1_stderr = String::from_utf8_lossy(&vm1_output.stderr);
53+
54+
println!("VM1 stdout: {}", vm1_stdout);
55+
println!("VM1 stderr: {}", vm1_stderr);
56+
57+
if !vm1_output.status.success() {
58+
cleanup_domain(&vm1_name);
59+
cleanup_domain(&vm2_name);
60+
61+
panic!("Failed to create first VM: {}", vm1_stderr);
62+
}
63+
64+
// Verify base disk was created
65+
assert!(
66+
vm1_stdout.contains("Using base disk") || vm1_stdout.contains("base disk"),
67+
"Should mention base disk creation"
68+
);
69+
70+
// Create second VM - this should reuse the base disk
71+
println!("Creating second VM (should reuse base disk)...");
72+
let vm2_output = Command::new("timeout")
73+
.args([
74+
"300s",
75+
&bck,
76+
"libvirt",
77+
"run",
78+
"--name",
79+
&vm2_name,
80+
"--filesystem",
81+
"ext4",
82+
&test_image,
83+
])
84+
.output()
85+
.expect("Failed to create second VM");
86+
87+
let vm2_stdout = String::from_utf8_lossy(&vm2_output.stdout);
88+
let vm2_stderr = String::from_utf8_lossy(&vm2_output.stderr);
89+
90+
println!("VM2 stdout: {}", vm2_stdout);
91+
println!("VM2 stderr: {}", vm2_stderr);
92+
93+
// Cleanup before assertions
94+
cleanup_domain(&vm1_name);
95+
cleanup_domain(&vm2_name);
96+
97+
if !vm2_output.status.success() {
98+
panic!("Failed to create second VM: {}", vm2_stderr);
99+
}
100+
101+
// Verify base disk was reused (should be faster and mention using existing)
102+
assert!(
103+
vm2_stdout.contains("Using base disk") || vm2_stdout.contains("base disk"),
104+
"Should mention using base disk"
105+
);
106+
107+
println!("✓ Base disk creation and reuse test passed");
108+
}
109+
110+
/// Test base-disks list command
111+
pub fn test_base_disks_list_command() {
112+
let bck = get_bck_command().unwrap();
113+
114+
println!("Testing base-disks list command");
115+
116+
let output = Command::new(&bck)
117+
.args(["libvirt", "base-disks", "list"])
118+
.output()
119+
.expect("Failed to run base-disks list");
120+
121+
let stdout = String::from_utf8_lossy(&output.stdout);
122+
let stderr = String::from_utf8_lossy(&output.stderr);
123+
124+
if output.status.success() {
125+
println!("base-disks list output: {}", stdout);
126+
127+
// Should show table header or empty message
128+
assert!(
129+
stdout.contains("NAME")
130+
|| stdout.contains("No base disk")
131+
|| stdout.contains("no base disk")
132+
|| stdout.is_empty(),
133+
"Should show table format or empty message, got: {}",
134+
stdout
135+
);
136+
137+
println!("✓ base-disks list command works");
138+
} else {
139+
println!("base-disks list failed (may be expected): {}", stderr);
140+
141+
// Should fail gracefully
142+
assert!(
143+
stderr.contains("pool") || stderr.contains("libvirt") || stderr.contains("connect"),
144+
"Should have meaningful error about libvirt connectivity"
145+
);
146+
}
147+
}
148+
149+
/// Test base-disks prune command with dry-run
150+
pub fn test_base_disks_prune_dry_run() {
151+
let bck = get_bck_command().unwrap();
152+
153+
println!("Testing base-disks prune --dry-run command");
154+
155+
let output = Command::new(&bck)
156+
.args(["libvirt", "base-disks", "prune", "--dry-run"])
157+
.output()
158+
.expect("Failed to run base-disks prune --dry-run");
159+
160+
let stdout = String::from_utf8_lossy(&output.stdout);
161+
let stderr = String::from_utf8_lossy(&output.stderr);
162+
163+
if output.status.success() {
164+
println!("base-disks prune --dry-run output: {}", stdout);
165+
166+
// Should show what would be removed or indicate nothing to prune
167+
assert!(
168+
stdout.contains("Would remove") || stdout.contains("No") || stdout.is_empty(),
169+
"Should show dry-run output"
170+
);
171+
172+
println!("✓ base-disks prune --dry-run command works");
173+
} else {
174+
println!("base-disks prune failed (may be expected): {}", stderr);
175+
176+
// Should fail gracefully
177+
assert!(
178+
stderr.contains("pool") || stderr.contains("libvirt") || stderr.contains("connect"),
179+
"Should have meaningful error about libvirt connectivity"
180+
);
181+
}
182+
}
183+
184+
/// Test that VM disks reference base disks correctly
185+
pub fn test_vm_disk_references_base() {
186+
let bck = get_bck_command().unwrap();
187+
let test_image = get_test_image();
188+
189+
let timestamp = std::time::SystemTime::now()
190+
.duration_since(std::time::UNIX_EPOCH)
191+
.unwrap()
192+
.as_secs();
193+
let vm_name = format!("test-disk-ref-{}", timestamp);
194+
195+
println!("Testing VM disk references base disk");
196+
197+
cleanup_domain(&vm_name);
198+
199+
// Create VM
200+
let output = Command::new("timeout")
201+
.args([
202+
"300s",
203+
&bck,
204+
"libvirt",
205+
"run",
206+
"--name",
207+
&vm_name,
208+
"--filesystem",
209+
"ext4",
210+
&test_image,
211+
])
212+
.output()
213+
.expect("Failed to create VM");
214+
215+
if !output.status.success() {
216+
let stderr = String::from_utf8_lossy(&output.stderr);
217+
cleanup_domain(&vm_name);
218+
219+
panic!("Failed to create VM: {}", stderr);
220+
}
221+
222+
// Get VM disk path from domain XML
223+
let dumpxml_output = Command::new("virsh")
224+
.args(&["dumpxml", &vm_name])
225+
.output()
226+
.expect("Failed to dump domain XML");
227+
228+
if !dumpxml_output.status.success() {
229+
cleanup_domain(&vm_name);
230+
panic!("Failed to get domain XML");
231+
}
232+
233+
let domain_xml = String::from_utf8_lossy(&dumpxml_output.stdout);
234+
235+
// Parse XML using bcvk's xml_utils to extract disk path
236+
let dom = bcvk::xml_utils::parse_xml_dom(&domain_xml).expect("Failed to parse domain XML");
237+
238+
let disk_path = dom
239+
.find("disk")
240+
.expect("No disk element found in domain XML")
241+
.children
242+
.iter()
243+
.find(|child| child.name == "source")
244+
.expect("No source element found in disk")
245+
.attributes
246+
.get("file")
247+
.expect("No file attribute found in source element");
248+
249+
cleanup_domain(&vm_name);
250+
251+
println!("VM disk path: {}", disk_path);
252+
253+
// Disk should be named after the VM, not a base disk
254+
assert!(
255+
disk_path.contains(&vm_name) && !disk_path.contains("bootc-base-"),
256+
"VM should use its own disk, not directly use base disk"
257+
);
258+
259+
println!("✓ VM disk reference test passed");
260+
}
261+
262+
/// Helper function to cleanup domain and its disk
263+
fn cleanup_domain(domain_name: &str) {
264+
println!("Cleaning up domain: {}", domain_name);
265+
266+
// Stop domain if running
267+
let _ = Command::new("virsh")
268+
.args(&["destroy", domain_name])
269+
.output();
270+
271+
// Use bcvk libvirt rm for proper cleanup
272+
let bck = get_bck_command().unwrap();
273+
let cleanup_output = Command::new(&bck)
274+
.args(&["libvirt", "rm", domain_name, "--force", "--stop"])
275+
.output();
276+
277+
if let Ok(output) = cleanup_output {
278+
if output.status.success() {
279+
println!("Successfully cleaned up domain: {}", domain_name);
280+
} else {
281+
let stderr = String::from_utf8_lossy(&output.stderr);
282+
println!("Cleanup warning (may be expected): {}", stderr);
283+
}
284+
}
285+
}

0 commit comments

Comments
 (0)