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@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