[libvirt PATCH 05/10] tests: don't permit NVRAM path when using firmware auto-select

[Michal Prívozník]

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