Skip to content

Commit 144fc5e

Browse files
committed
libvirt: Add QEMU firmware interop JSON descriptor support
Parse the QEMU firmware interop specification JSON descriptors to find firmware instead of doing it manually. This fixes OVMF firmware detection on Debian/Ubuntu derivatives. Add a hidden `bcvk libvirt print-firmware` which is a cheap way to test our parser. Assisted-by: Claude Code (Sonnet 4.5) Signed-off-by: Colin Walters <walters@verbum.org>
1 parent 5ff7bcc commit 144fc5e

File tree

7 files changed

+663
-64
lines changed

7 files changed

+663
-64
lines changed

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

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -959,6 +959,66 @@ fn test_libvirt_run_no_storage_opts_without_bind_storage() -> Result<()> {
959959
}
960960
integration_test!(test_libvirt_run_no_storage_opts_without_bind_storage);
961961

962+
/// Test print-firmware command (hidden debugging command)
963+
fn test_libvirt_print_firmware() -> Result<()> {
964+
let bck = get_bck_command()?;
965+
966+
// Test YAML output (default)
967+
let yaml_output = Command::new(&bck)
968+
.args(["libvirt", "print-firmware"])
969+
.output()
970+
.expect("Failed to run libvirt print-firmware");
971+
972+
let stdout = String::from_utf8_lossy(&yaml_output.stdout);
973+
let stderr = String::from_utf8_lossy(&yaml_output.stderr);
974+
975+
// Should succeed and produce YAML output
976+
assert!(
977+
yaml_output.status.success(),
978+
"libvirt print-firmware should succeed. stderr: {}",
979+
stderr
980+
);
981+
982+
// Verify YAML output contains expected fields
983+
assert!(
984+
stdout.contains("architecture:"),
985+
"YAML output should contain architecture field"
986+
);
987+
988+
println!("libvirt print-firmware YAML output:\n{}", stdout);
989+
990+
// Test JSON output
991+
let json_output = Command::new(&bck)
992+
.args(["libvirt", "print-firmware", "--format", "json"])
993+
.output()
994+
.expect("Failed to run libvirt print-firmware --format json");
995+
996+
let json_stdout = String::from_utf8_lossy(&json_output.stdout);
997+
998+
if json_output.status.success() {
999+
// Verify it's valid JSON
1000+
let json_result: std::result::Result<serde_json::Value, _> =
1001+
serde_json::from_str(&json_stdout);
1002+
assert!(
1003+
json_result.is_ok(),
1004+
"libvirt print-firmware --format json should produce valid JSON: {}",
1005+
json_stdout
1006+
);
1007+
1008+
let json_value = json_result.unwrap();
1009+
assert!(
1010+
json_value.get("architecture").is_some(),
1011+
"JSON output should contain architecture field"
1012+
);
1013+
1014+
println!("libvirt print-firmware JSON output:\n{}", json_stdout);
1015+
}
1016+
1017+
println!("libvirt print-firmware test passed");
1018+
Ok(())
1019+
}
1020+
integration_test!(test_libvirt_print_firmware);
1021+
9621022
/// Test error handling for invalid configurations
9631023
fn test_libvirt_error_handling() -> Result<()> {
9641024
let bck = get_bck_command()?;

crates/kit/src/libvirt/domain.rs

Lines changed: 149 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,16 @@ pub struct VirtiofsFilesystem {
2323
pub readonly: bool,
2424
}
2525

26+
/// Configuration for firmware debug log output
27+
#[derive(Debug, Clone)]
28+
pub enum FirmwareLogOutput {
29+
/// Write firmware log to a file on the host
30+
#[allow(dead_code)]
31+
File(String),
32+
/// Make firmware log available via virsh console (pty)
33+
Console,
34+
}
35+
2636
/// Builder for creating libvirt domain XML configurations
2737
#[derive(Debug)]
2838
pub struct DomainBuilder {
@@ -41,7 +51,10 @@ pub struct DomainBuilder {
4151
firmware: Option<FirmwareType>,
4252
tpm: bool,
4353
ovmf_code_path: Option<String>, // Custom OVMF_CODE path for secure boot
54+
ovmf_code_format: Option<String>, // Format of OVMF_CODE (raw, qcow2)
4455
nvram_template: Option<String>, // Custom NVRAM template with enrolled keys
56+
nvram_format: Option<String>, // Format of NVRAM template (raw, qcow2)
57+
firmware_log: Option<FirmwareLogOutput>, // OVMF debug log output via isa-debugcon
4558
}
4659

4760
impl Default for DomainBuilder {
@@ -69,7 +82,10 @@ impl DomainBuilder {
6982
firmware: None, // Defaults to UEFI
7083
tpm: true, // Default to enabled
7184
ovmf_code_path: None,
85+
ovmf_code_format: None,
7286
nvram_template: None,
87+
nvram_format: None,
88+
firmware_log: Some(FirmwareLogOutput::Console), // Default to pty for virsh console access
7389
}
7490
}
7591

@@ -153,15 +169,38 @@ impl DomainBuilder {
153169
self
154170
}
155171

156-
/// Set custom OVMF_CODE path for secure boot
157-
pub fn with_ovmf_code_path(mut self, path: &str) -> Self {
172+
/// Set custom OVMF_CODE path and format for secure boot
173+
///
174+
/// Format must be specified (either "raw" or "qcow2") and should come from
175+
/// the QEMU firmware interop JSON descriptors.
176+
pub fn with_ovmf_code_path(mut self, path: &str, format: &str) -> Self {
158177
self.ovmf_code_path = Some(path.to_string());
178+
self.ovmf_code_format = Some(format.to_string());
159179
self
160180
}
161181

162-
/// Set custom NVRAM template path with enrolled secure boot keys
163-
pub fn with_nvram_template(mut self, path: &str) -> Self {
182+
/// Set custom NVRAM template path and format with enrolled secure boot keys
183+
///
184+
/// Format must be specified (either "raw" or "qcow2") and should come from
185+
/// the QEMU firmware interop JSON descriptors.
186+
pub fn with_nvram_template(mut self, path: &str, format: &str) -> Self {
164187
self.nvram_template = Some(path.to_string());
188+
self.nvram_format = Some(format.to_string());
189+
self
190+
}
191+
192+
/// Enable firmware debug log output via isa-debugcon (x86_64 only)
193+
///
194+
/// This captures OVMF/EDK2 DEBUG() output which is useful for debugging
195+
/// Secure Boot failures and other firmware issues. The log is available
196+
/// on IO port 0x402.
197+
///
198+
/// Options:
199+
/// - `FirmwareLogOutput::File(path)` - Write to a file on the host
200+
/// - `FirmwareLogOutput::Console` - Access via `virsh console <domain> serial1`
201+
#[allow(dead_code)]
202+
pub fn with_firmware_log(mut self, output: FirmwareLogOutput) -> Self {
203+
self.firmware_log = Some(output);
165204
self
166205
}
167206

@@ -232,22 +271,35 @@ impl DomainBuilder {
232271
if use_uefi {
233272
if let Some(ref ovmf_code) = self.ovmf_code_path {
234273
// Use custom OVMF_CODE path for secure boot
235-
let mut loader_attrs =
236-
vec![("readonly", "yes"), ("type", "pflash"), ("format", "raw")];
274+
// Format is required and comes from QEMU firmware interop JSON descriptors
275+
let code_format = self
276+
.ovmf_code_format
277+
.as_deref()
278+
.expect("ovmf_code_format must be set when ovmf_code_path is set");
279+
let mut loader_attrs = vec![
280+
("readonly", "yes"),
281+
("type", "pflash"),
282+
("format", code_format),
283+
];
237284
if secure_boot {
238285
loader_attrs.push(("secure", "yes"));
239286
}
240287
writer.write_text_element_with_attrs("loader", ovmf_code, &loader_attrs)?;
241288

242289
// Add NVRAM element if template is specified
243290
if let Some(ref nvram_template) = self.nvram_template {
291+
// Format is required and comes from QEMU firmware interop JSON descriptors
292+
let nvram_fmt = self
293+
.nvram_format
294+
.as_deref()
295+
.expect("nvram_format must be set when nvram_template is set");
244296
writer.write_text_element_with_attrs(
245297
"nvram",
246298
"", // Empty content, template attr provides the source
247299
&[
248300
("template", nvram_template),
249-
("templateFormat", "raw"),
250-
("format", "raw"),
301+
("templateFormat", nvram_fmt),
302+
("format", nvram_fmt),
251303
],
252304
)?;
253305
}
@@ -369,6 +421,40 @@ impl DomainBuilder {
369421
writer.write_empty_element("target", &[("type", "virtio")])?;
370422
writer.end_element("console")?;
371423

424+
// Firmware debug log via isa-debugcon (x86_64 only)
425+
// This captures OVMF/EDK2 DEBUG() output on IO port 0x402, useful for
426+
// debugging Secure Boot failures. Access via: virsh console <domain> serial0
427+
// See: https://libvirt.org/formatdomain.html#serial-port (isa-debug target type)
428+
if arch_config.arch == "x86_64" {
429+
if let Some(ref firmware_log) = self.firmware_log {
430+
match firmware_log {
431+
FirmwareLogOutput::Console => {
432+
writer.start_element("serial", &[("type", "pty")])?;
433+
writer.start_element("target", &[("type", "isa-debug"), ("port", "0")])?;
434+
writer.write_empty_element("model", &[("name", "isa-debugcon")])?;
435+
writer.end_element("target")?;
436+
writer.write_empty_element(
437+
"address",
438+
&[("type", "isa"), ("iobase", "0x402")],
439+
)?;
440+
writer.end_element("serial")?;
441+
}
442+
FirmwareLogOutput::File(path) => {
443+
writer.start_element("serial", &[("type", "file")])?;
444+
writer.write_empty_element("source", &[("path", path)])?;
445+
writer.start_element("target", &[("type", "isa-debug"), ("port", "0")])?;
446+
writer.write_empty_element("model", &[("name", "isa-debugcon")])?;
447+
writer.end_element("target")?;
448+
writer.write_empty_element(
449+
"address",
450+
&[("type", "isa"), ("iobase", "0x402")],
451+
)?;
452+
writer.end_element("serial")?;
453+
}
454+
}
455+
}
456+
}
457+
372458
// VNC graphics if enabled
373459
if let Some(vnc_port) = self.vnc_port {
374460
writer.write_empty_element(
@@ -638,8 +724,8 @@ mod tests {
638724
let xml = DomainBuilder::new()
639725
.with_name("test-custom-secboot")
640726
.with_firmware(FirmwareType::UefiSecure)
641-
.with_ovmf_code_path("/usr/share/edk2/ovmf/OVMF_CODE.secboot.fd")
642-
.with_nvram_template("/var/lib/libvirt/qemu/nvram/custom_VARS.fd")
727+
.with_ovmf_code_path("/usr/share/edk2/ovmf/OVMF_CODE.secboot.fd", "raw")
728+
.with_nvram_template("/var/lib/libvirt/qemu/nvram/custom_VARS.fd", "raw")
643729
.build_xml()
644730
.unwrap();
645731

@@ -701,4 +787,57 @@ mod tests {
701787
assert!(xml_ro.contains("source dir=\"/host/storage\""));
702788
assert!(xml_ro.contains("target dir=\"hoststorage\""));
703789
}
790+
791+
#[test]
792+
fn test_firmware_log_default() {
793+
// By default, firmware log should be enabled (pty/console mode)
794+
let xml = DomainBuilder::new()
795+
.with_name("test-firmware-log-default")
796+
.build_xml()
797+
.unwrap();
798+
799+
// On x86_64, should have isa-debugcon serial device
800+
if std::env::consts::ARCH == "x86_64" {
801+
assert!(xml.contains("serial type=\"pty\""));
802+
assert!(xml.contains("target type=\"isa-debug\""));
803+
assert!(xml.contains("model name=\"isa-debugcon\""));
804+
assert!(xml.contains("address type=\"isa\" iobase=\"0x402\""));
805+
}
806+
}
807+
808+
#[test]
809+
fn test_firmware_log_file() {
810+
// Test firmware log to file
811+
let xml = DomainBuilder::new()
812+
.with_name("test-firmware-log-file")
813+
.with_firmware_log(FirmwareLogOutput::File("/tmp/ovmf-debug.log".to_string()))
814+
.build_xml()
815+
.unwrap();
816+
817+
// On x86_64, should have isa-debugcon with file output
818+
if std::env::consts::ARCH == "x86_64" {
819+
assert!(xml.contains("serial type=\"file\""));
820+
assert!(xml.contains("source path=\"/tmp/ovmf-debug.log\""));
821+
assert!(xml.contains("target type=\"isa-debug\""));
822+
assert!(xml.contains("model name=\"isa-debugcon\""));
823+
assert!(xml.contains("address type=\"isa\" iobase=\"0x402\""));
824+
}
825+
}
826+
827+
#[test]
828+
fn test_firmware_log_disabled() {
829+
// Test disabling firmware log by setting firmware_log to None after construction
830+
// Note: There's no public API to disable it once set, but we can test the XML
831+
// generation doesn't include it on non-x86 architectures
832+
let xml = DomainBuilder::new()
833+
.with_name("test-firmware-log")
834+
.build_xml()
835+
.unwrap();
836+
837+
// On non-x86_64, should NOT have isa-debugcon (it's x86-only)
838+
if std::env::consts::ARCH != "x86_64" {
839+
assert!(!xml.contains("isa-debugcon"));
840+
assert!(!xml.contains("isa-debug"));
841+
}
842+
}
704843
}

crates/kit/src/libvirt/mod.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ pub mod domain;
3030
pub mod inspect;
3131
pub mod list;
3232
pub mod list_volumes;
33+
pub mod print_firmware;
3334
pub mod rm;
3435
pub mod rm_all;
3536
pub mod run;
@@ -220,4 +221,8 @@ pub enum LibvirtSubcommands {
220221
/// Manage base disk images used for VM cloning
221222
#[clap(name = "base-disks")]
222223
BaseDisks(base_disks_cli::LibvirtBaseDisksOpts),
224+
225+
/// Print detected firmware paths and configuration
226+
#[clap(name = "print-firmware", hide = true)]
227+
PrintFirmware(print_firmware::LibvirtPrintFirmwareOpts),
223228
}

0 commit comments

Comments
 (0)