conf: Remove some firmware validation checks

libvirt 8.6.0 introduced these checks and very clearly delineated
two possible firmware selection scenarios: manual firmware
selection, where the user is responsible for providing all
information, and firmware autoselection, where a list of desired
features is provided and everything else is handled by libvirt.

In the interest of maintaining the clear separation between these
two scenarios, setting most attributes when firmware autoselection
is active will result in the configuration being rejected.

This works fine, but is unnecessarily restrictive: in most cases,
the additional information that the user has provided matches
the information that libvirt would have discovered on its own by
looking at firmware descriptors, and asking the user to scrub it
from the XML only result in pointless friction.

Remove these checks entirely.

Unsurprisingly, this results in a few test cases that were
rejected until now to suddenly start working and producing
sensible results.

The firmware-auto-efi-loader-path-nonstandard test case is
notable: while we can now enable the xml2xml part of the test,
the xml2argv part is still failing, although in a slightly
different way. This is expected: since the firmware binary is a
non-standard one, libvirt is unable to figure out the missing
information from a firmware descriptor, and the configuration
is still ultimately an invalid one. However, if we were to find
such a configuration on disk at daemon startup, we would not
ignore it completely and instead would offer the user a chance
to fix it.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index 5fb2d4971c07eaaa7edd50ff91ae2b1094adf850..6991cf1dd3d8fc18849885ef33a2364e75157808 100644 (file)
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -1609,44 +1609,6 @@ virDomainDefOSValidate(const virDomainDef *def,
         if (!loader)
             return 0;
 
-        if (loader->readonly) {
-            virReportError(VIR_ERR_XML_DETAIL, "%s",
-                           _("loader attribute 'readonly' cannot be specified "
-                             "when firmware autoselection is enabled"));
-            return -1;
-        }
-        if (loader->type) {
-            virReportError(VIR_ERR_XML_DETAIL, "%s",
-                           _("loader attribute 'type' cannot be specified "
-                             "when firmware autoselection is enabled"));
-            return -1;
-        }
-        if (loader->path) {
-            virReportError(VIR_ERR_XML_DETAIL, "%s",
-                           _("loader path cannot be specified "
-                             "when firmware autoselection is enabled"));
-            return -1;
-        }
-        if (loader->nvramTemplate) {
-            virReportError(VIR_ERR_XML_DETAIL, "%s",
-                           _("nvram attribute 'template' cannot be specified "
-                             "when firmware autoselection is enabled"));
-            return -1;
-        }
-
-        /* We need to accept 'yes' here because the initial implementation
-         * of firmware autoselection used it as a way to request a firmware
-         * with Secure Boot support, so the error message is technically
-         * incorrect; however, we want to discourage people from using this
-         * attribute at all, so it's fine to be a bit more aggressive than
-         * it would be strictly required :) */
-        if (loader->secure == VIR_TRISTATE_BOOL_NO) {
-            virReportError(VIR_ERR_XML_DETAIL, "%s",
-                           _("loader attribute 'secure' cannot be specified "
-                             "when firmware autoselection is enabled"));
-            return -1;
-        }
-
         if (loader->nvram && def->os.firmware != VIR_DOMAIN_OS_DEF_FIRMWARE_EFI) {
             virReportError(VIR_ERR_XML_DETAIL,
                            _("firmware type '%s' does not support nvram"),

diff --git a/tests/qemuxml2argvdata/firmware-auto-efi-loader-insecure.x86_64-latest.args b/tests/qemuxml2argvdata/firmware-auto-efi-loader-insecure.x86_64-latest.args
new file mode 100644 (file)
index 0000000..9326bfe
--- /dev/null
+++ b/tests/qemuxml2argvdata/firmware-auto-efi-loader-insecure.x86_64-latest.args
@@ -0,0 +1,37 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/var/lib/libvirt/qemu/domain--1-guest \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-guest/.local/share \
+XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-guest/.cache \
+XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-guest/.config \
+/usr/bin/qemu-system-x86_64 \
+-name guest=guest,debug-threads=on \
+-S \
+-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-guest/master-key.aes"}' \
+-blockdev '{"driver":"file","filename":"/usr/share/OVMF/OVMF_CODE.fd","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}' \
+-blockdev '{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"raw","file":"libvirt-pflash0-storage"}' \
+-blockdev '{"driver":"file","filename":"/var/lib/libvirt/qemu/nvram/guest_VARS.fd","node-name":"libvirt-pflash1-storage","auto-read-only":true,"discard":"unmap"}' \
+-blockdev '{"node-name":"libvirt-pflash1-format","read-only":false,"driver":"raw","file":"libvirt-pflash1-storage"}' \
+-machine pc-q35-4.0,usb=off,dump-guest-core=off,memory-backend=pc.ram,pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-format,acpi=on \
+-accel kvm \
+-cpu qemu64 \
+-m 1024 \
+-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":1073741824}' \
+-overcommit mem-lock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid 63840878-0deb-4095-97e6-fc444d9bc9fa \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-boot strict=on \
+-audiodev '{"id":"audio1","driver":"none"}' \
+-global ICH9-LPC.noreboot=off \
+-watchdog-action reset \
+-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
+-msg timestamp=on

diff --git a/tests/qemuxml2argvdata/firmware-auto-efi-loader-insecure.x86_64-latest.err b/tests/qemuxml2argvdata/firmware-auto-efi-loader-insecure.x86_64-latest.err
deleted file mode 100644 (file)
index 564f0e6..0000000
--- a/tests/qemuxml2argvdata/firmware-auto-efi-loader-insecure.x86_64-latest.err
+++ /dev/null
@@ -1 +0,0 @@
-loader attribute 'secure' cannot be specified when firmware autoselection is enabled

diff --git a/tests/qemuxml2argvdata/firmware-auto-efi-loader-path-nonstandard.x86_64-latest.err b/tests/qemuxml2argvdata/firmware-auto-efi-loader-path-nonstandard.x86_64-latest.err
index 3f90a88791da755d337cd5f51bc2d6a3aada603b..4cfde1bd2e17035a03c02bf3324f064d5877652c 100644 (file)
--- a/tests/qemuxml2argvdata/firmware-auto-efi-loader-path-nonstandard.x86_64-latest.err
+++ b/tests/qemuxml2argvdata/firmware-auto-efi-loader-path-nonstandard.x86_64-latest.err
@@ -1 +1 @@
-loader attribute 'readonly' cannot be specified when firmware autoselection is enabled
+operation failed: Unable to find any firmware to satisfy 'efi'

diff --git a/tests/qemuxml2argvdata/firmware-auto-efi-loader-path.x86_64-latest.args b/tests/qemuxml2argvdata/firmware-auto-efi-loader-path.x86_64-latest.args
new file mode 100644 (file)
index 0000000..9326bfe
--- /dev/null
+++ b/tests/qemuxml2argvdata/firmware-auto-efi-loader-path.x86_64-latest.args
@@ -0,0 +1,37 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/var/lib/libvirt/qemu/domain--1-guest \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-guest/.local/share \
+XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-guest/.cache \
+XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-guest/.config \
+/usr/bin/qemu-system-x86_64 \
+-name guest=guest,debug-threads=on \
+-S \
+-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-guest/master-key.aes"}' \
+-blockdev '{"driver":"file","filename":"/usr/share/OVMF/OVMF_CODE.fd","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}' \
+-blockdev '{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"raw","file":"libvirt-pflash0-storage"}' \
+-blockdev '{"driver":"file","filename":"/var/lib/libvirt/qemu/nvram/guest_VARS.fd","node-name":"libvirt-pflash1-storage","auto-read-only":true,"discard":"unmap"}' \
+-blockdev '{"node-name":"libvirt-pflash1-format","read-only":false,"driver":"raw","file":"libvirt-pflash1-storage"}' \
+-machine pc-q35-4.0,usb=off,dump-guest-core=off,memory-backend=pc.ram,pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-format,acpi=on \
+-accel kvm \
+-cpu qemu64 \
+-m 1024 \
+-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":1073741824}' \
+-overcommit mem-lock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid 63840878-0deb-4095-97e6-fc444d9bc9fa \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-boot strict=on \
+-audiodev '{"id":"audio1","driver":"none"}' \
+-global ICH9-LPC.noreboot=off \
+-watchdog-action reset \
+-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
+-msg timestamp=on

diff --git a/tests/qemuxml2argvdata/firmware-auto-efi-loader-path.x86_64-latest.err b/tests/qemuxml2argvdata/firmware-auto-efi-loader-path.x86_64-latest.err
deleted file mode 100644 (file)
index 3f90a88..0000000
--- a/tests/qemuxml2argvdata/firmware-auto-efi-loader-path.x86_64-latest.err
+++ /dev/null
@@ -1 +0,0 @@
-loader attribute 'readonly' cannot be specified when firmware autoselection is enabled

diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 23e48b251c036dfe50f0ed312741c1dc05713c09..bd3f46fbe00d53329b96a0d9c75e6ddc24daa117 100644 (file)
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1118,9 +1118,9 @@ mymain(void)
     DO_TEST_CAPS_LATEST("firmware-auto-efi-stateless");
     DO_TEST_CAPS_LATEST("firmware-auto-efi-nvram");
     DO_TEST_CAPS_LATEST("firmware-auto-efi-loader-secure");
-    DO_TEST_CAPS_LATEST_PARSE_ERROR("firmware-auto-efi-loader-insecure");
-    DO_TEST_CAPS_LATEST_PARSE_ERROR("firmware-auto-efi-loader-path");
-    DO_TEST_CAPS_LATEST_PARSE_ERROR("firmware-auto-efi-loader-path-nonstandard");
+    DO_TEST_CAPS_LATEST("firmware-auto-efi-loader-insecure");
+    DO_TEST_CAPS_LATEST("firmware-auto-efi-loader-path");
+    DO_TEST_CAPS_LATEST_FAILURE("firmware-auto-efi-loader-path-nonstandard");
     DO_TEST_CAPS_LATEST("firmware-auto-efi-secboot");
     DO_TEST_CAPS_LATEST("firmware-auto-efi-no-secboot");
     DO_TEST_CAPS_LATEST("firmware-auto-efi-enrolled-keys");

diff --git a/tests/qemuxml2xmloutdata/firmware-auto-efi-loader-insecure.x86_64-latest.xml b/tests/qemuxml2xmloutdata/firmware-auto-efi-loader-insecure.x86_64-latest.xml
new file mode 100644 (file)
index 0000000..a6af551
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/firmware-auto-efi-loader-insecure.x86_64-latest.xml
@@ -0,0 +1,36 @@
+<domain type='kvm'>
+  <name>guest</name>
+  <uuid>63840878-0deb-4095-97e6-fc444d9bc9fa</uuid>
+  <memory unit='KiB'>1048576</memory>
+  <currentMemory unit='KiB'>1048576</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='x86_64' machine='pc-q35-4.0'>hvm</type>
+    <loader readonly='yes' secure='no' type='pflash'>/usr/share/OVMF/OVMF_CODE.fd</loader>
+    <nvram template='/usr/share/OVMF/OVMF_VARS.fd'>/var/lib/libvirt/qemu/nvram/guest_VARS.fd</nvram>
+    <boot dev='hd'/>
+  </os>
+  <features>
+    <acpi/>
+  </features>
+  <cpu mode='custom' match='exact' check='none'>
+    <model fallback='forbid'>qemu64</model>
+  </cpu>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-x86_64</emulator>
+    <controller type='usb' index='0' model='none'/>
+    <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'/>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <audio id='1' type='none'/>
+    <watchdog model='itco' action='reset'/>
+    <memballoon model='none'/>
+  </devices>
+</domain>

diff --git a/tests/qemuxml2xmloutdata/firmware-auto-efi-loader-path-nonstandard.x86_64-latest.xml b/tests/qemuxml2xmloutdata/firmware-auto-efi-loader-path-nonstandard.x86_64-latest.xml
new file mode 100644 (file)
index 0000000..f68cddf
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/firmware-auto-efi-loader-path-nonstandard.x86_64-latest.xml
@@ -0,0 +1,35 @@
+<domain type='kvm'>
+  <name>guest</name>
+  <uuid>63840878-0deb-4095-97e6-fc444d9bc9fa</uuid>
+  <memory unit='KiB'>1048576</memory>
+  <currentMemory unit='KiB'>1048576</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os firmware='efi'>
+    <type arch='x86_64' machine='pc-q35-4.0'>hvm</type>
+    <loader readonly='yes' type='pflash'>/path/to/OVMF_CODE.fd</loader>
+    <boot dev='hd'/>
+  </os>
+  <features>
+    <acpi/>
+  </features>
+  <cpu mode='custom' match='exact' check='none'>
+    <model fallback='forbid'>qemu64</model>
+  </cpu>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-x86_64</emulator>
+    <controller type='usb' index='0' model='none'/>
+    <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'/>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <audio id='1' type='none'/>
+    <watchdog model='itco' action='reset'/>
+    <memballoon model='none'/>
+  </devices>
+</domain>

diff --git a/tests/qemuxml2xmloutdata/firmware-auto-efi-loader-path.x86_64-latest.xml b/tests/qemuxml2xmloutdata/firmware-auto-efi-loader-path.x86_64-latest.xml
new file mode 100644 (file)
index 0000000..8b3853d
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/firmware-auto-efi-loader-path.x86_64-latest.xml
@@ -0,0 +1,36 @@
+<domain type='kvm'>
+  <name>guest</name>
+  <uuid>63840878-0deb-4095-97e6-fc444d9bc9fa</uuid>
+  <memory unit='KiB'>1048576</memory>
+  <currentMemory unit='KiB'>1048576</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='x86_64' machine='pc-q35-4.0'>hvm</type>
+    <loader readonly='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE.fd</loader>
+    <nvram template='/usr/share/OVMF/OVMF_VARS.fd'>/var/lib/libvirt/qemu/nvram/guest_VARS.fd</nvram>
+    <boot dev='hd'/>
+  </os>
+  <features>
+    <acpi/>
+  </features>
+  <cpu mode='custom' match='exact' check='none'>
+    <model fallback='forbid'>qemu64</model>
+  </cpu>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-x86_64</emulator>
+    <controller type='usb' index='0' model='none'/>
+    <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'/>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <audio id='1' type='none'/>
+    <watchdog model='itco' action='reset'/>
+    <memballoon model='none'/>
+  </devices>
+</domain>

diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 7ada880da2126a1d2c583838ee5a3a3d4bfc00d6..6cdead532d7ab75daa584844a0c6083fb7843463 100644 (file)
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -947,6 +947,9 @@ mymain(void)
     DO_TEST_CAPS_LATEST("firmware-auto-efi-stateless");
     DO_TEST_CAPS_LATEST("firmware-auto-efi-nvram");
     DO_TEST_CAPS_LATEST("firmware-auto-efi-loader-secure");
+    DO_TEST_CAPS_LATEST("firmware-auto-efi-loader-insecure");
+    DO_TEST_CAPS_LATEST("firmware-auto-efi-loader-path");
+    DO_TEST_CAPS_LATEST("firmware-auto-efi-loader-path-nonstandard");
     DO_TEST_CAPS_LATEST("firmware-auto-efi-secboot");
     DO_TEST_CAPS_LATEST("firmware-auto-efi-no-secboot");
     DO_TEST_CAPS_LATEST("firmware-auto-efi-enrolled-keys");