Skip to content

Commit b025b25

Browse files
committed
libvirt: Disable firmware debug log (isa-debugcon) by default
Previously I added `isa-debugcon` when debugging a secure boot issue, but it slows down boot significantly if it's found. Make it an opt in thing. Assisted-by: OpenCode (Claude Opus 4) Signed-off-by: Colin Walters <walters@verbum.org>
1 parent 47eb259 commit b025b25

2 files changed

Lines changed: 12 additions & 56 deletions

File tree

crates/kit/src/libvirt/domain.rs

Lines changed: 2 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ pub enum FirmwareLogOutput {
3030
#[allow(dead_code)]
3131
File(String),
3232
/// Make firmware log available via virsh console (pty)
33+
#[allow(dead_code)]
3334
Console,
3435
}
3536

@@ -85,7 +86,7 @@ impl DomainBuilder {
8586
ovmf_code_format: None,
8687
nvram_template: None,
8788
nvram_format: None,
88-
firmware_log: Some(FirmwareLogOutput::Console), // Default to pty for virsh console access
89+
firmware_log: None,
8990
}
9091
}
9192

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

crates/kit/src/libvirt/run.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,10 @@ pub struct LibvirtRunOpts {
285285
#[clap(long)]
286286
pub disable_tpm: bool,
287287

288+
/// Enable firmware debug log (captures OVMF/EDK2 DEBUG output via isa-debugcon)
289+
#[clap(long)]
290+
pub firmware_log: bool,
291+
288292
/// Directory containing secure boot keys (required for uefi-secure)
289293
#[clap(long)]
290294
pub secure_boot_keys: Option<Utf8PathBuf>,
@@ -1091,15 +1095,19 @@ fn create_libvirt_domain_from_disk(
10911095
};
10921096

10931097
// Build domain XML using the existing DomainBuilder with bootc metadata and SSH keys
1094-
let mut domain_builder = DomainBuilder::new()
1098+
let mut builder = DomainBuilder::new()
10951099
.with_name(domain_name)
10961100
.with_memory(memory.into())
10971101
.with_vcpus(cpus)
10981102
.with_disk(disk_path.as_str())
10991103
.with_transient_disk(opts.transient)
11001104
.with_network("none") // Use QEMU args for SSH networking instead
11011105
.with_firmware(opts.firmware)
1102-
.with_tpm(!opts.disable_tpm)
1106+
.with_tpm(!opts.disable_tpm);
1107+
if opts.firmware_log {
1108+
builder = builder.with_firmware_log(crate::libvirt::domain::FirmwareLogOutput::Console);
1109+
}
1110+
let mut domain_builder = builder
11031111
.with_metadata("bootc:source-image", &opts.image)
11041112
.with_metadata("bootc:memory-mb", &memory.to_string())
11051113
.with_metadata("bootc:vcpus", &cpus.to_string())

0 commit comments

Comments
 (0)