On 2/15/22 19:54, Daniel P. Berrangé wrote:
When using <os firmware='...'/> we still parse the
<nvram> path,
but completely ignore it, replacing any user provided content with
a custom generated path. This makes sense since when undefining the
guest, the code to cleanup NVRAM also uses the same generated path.
Instead of silently ignoring user config, we should report an
explicit error message. This shows that some of our tests had the
bogus config scenario present.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/conf/domain_conf.c | 8 +++
tests/qemuxml2argvdata/os-firmware-bios.xml | 1 -
.../os-firmware-efi-bad-nvram-path.err | 1 +
.../os-firmware-efi-bad-nvram-path.xml | 68 +++++++++++++++++++
.../os-firmware-efi-secboot.xml | 1 -
tests/qemuxml2argvdata/os-firmware-efi.xml | 1 -
tests/qemuxml2argvtest.c | 1 +
.../os-firmware-bios.x86_64-latest.xml | 1 -
.../os-firmware-efi-secboot.x86_64-latest.xml | 1 -
.../os-firmware-efi.x86_64-latest.xml | 1 -
10 files changed, 78 insertions(+), 6 deletions(-)
create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-bad-nvram-path.err
create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-bad-nvram-path.xml
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 31b49c4ec9..946a80c239 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4827,6 +4827,14 @@ virDomainDefPostParseOs(virDomainDef *def)
return -1;
}
+ if (def->os.firmware != VIR_DOMAIN_OS_DEF_FIRMWARE_NONE) {
+ if (def->os.loader->nvram) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("NVRAM path is not permitted with firmware
attribute"));
+ return -1;
+ }
+ }
+
return 0;
Again, virDomainDefOSValidate(), ...
}
diff --git a/tests/qemuxml2argvdata/os-firmware-bios.xml
b/tests/qemuxml2argvdata/os-firmware-bios.xml
index 63886666dd..d89fcb6c58 100644
--- a/tests/qemuxml2argvdata/os-firmware-bios.xml
+++ b/tests/qemuxml2argvdata/os-firmware-bios.xml
@@ -7,7 +7,6 @@
<os firmware='bios'>
<type arch='x86_64' machine='pc-q35-4.0'>hvm</type>
<loader secure='no'/>
- <nvram>/var/lib/libvirt/qemu/nvram/fedora_VARS.fd</nvram>
<boot dev='hd'/>
<bootmenu enable='yes'/>
</os>
diff --git a/tests/qemuxml2argvdata/os-firmware-efi-bad-nvram-path.err
b/tests/qemuxml2argvdata/os-firmware-efi-bad-nvram-path.err
new file mode 100644
index 0000000000..2ba8135ad4
--- /dev/null
+++ b/tests/qemuxml2argvdata/os-firmware-efi-bad-nvram-path.err
@@ -0,0 +1 @@
+XML error: NVRAM path is not permitted with firmware attribute
... which will have unfortunate result that this error message will look
different, because one of earlier validators reports an error.
diff --git
a/tests/qemuxml2argvdata/os-firmware-efi-bad-nvram-path.xml
b/tests/qemuxml2argvdata/os-firmware-efi-bad-nvram-path.xml
new file mode 100644
index 0000000000..a4afdb6d0b
--- /dev/null
+++ b/tests/qemuxml2argvdata/os-firmware-efi-bad-nvram-path.xml
@@ -0,0 +1,68 @@
+<domain type='kvm'>
+ <name>fedora</name>
+ <uuid>63840878-0deb-4095-97e6-fc444d9bc9fa</uuid>
+ <memory unit='KiB'>8192</memory>
+ <currentMemory unit='KiB'>8192</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os firmware='efi'>
+ <type arch='x86_64' machine='pc-q35-4.0'>hvm</type>
+ <loader secure='no'/>
+ <nvram>/some/path</nvram>
+ <boot dev='hd'/>
+ <bootmenu enable='yes'/>
+ </os>
+ <features>
+ <acpi/>
+ <apic/>
+ <pae/>
+ </features>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>restart</on_crash>
+ <pm>
+ <suspend-to-mem enabled='yes'/>
+ <suspend-to-disk enabled='no'/>
+ </pm>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ <controller type='usb' index='0' model='ich9-ehci1'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x1d' function='0x7'/>
+ </controller>
+ <controller type='usb' index='0' model='ich9-uhci1'>
+ <master startport='0'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x1d' function='0x0' multifunction='on'/>
+ </controller>
+ <controller type='usb' index='0' model='ich9-uhci2'>
+ <master startport='2'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x1d' function='0x1'/>
+ </controller>
+ <controller type='usb' index='0' model='ich9-uhci3'>
+ <master startport='4'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x1d' function='0x2'/>
+ </controller>
+ <controller type='sata' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x1f' function='0x2'/>
+ </controller>
+ <controller type='pci' index='0' model='pcie-root'/>
+ <controller type='pci' index='1'
model='dmi-to-pci-bridge'>
+ <model name='i82801b11-bridge'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x1e' function='0x0'/>
+ </controller>
+ <controller type='pci' index='2' model='pci-bridge'>
+ <model name='pci-bridge'/>
+ <target chassisNr='2'/>
+ <address type='pci' domain='0x0000' bus='0x01'
slot='0x00' function='0x0'/>
+ </controller>
+ <controller type='pci' index='3'
model='pcie-root-port'>
+ <model name='ioh3420'/>
+ <target chassis='3' port='0x8'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x0'/>
+ </controller>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <memballoon model='virtio'>
+ <address type='pci' domain='0x0000' bus='0x02'
slot='0x01' function='0x0'/>
+ </memballoon>
Nit pick, I've gotten so far that machine type is plain 'q35' and
removed all these devices. They are not needed since this test is bound
to fail anyway.
+ </devices>
+</domain>
Michal