qemu_firmware: Remove NVRAM to loader format copy hack

Now that the hack is gone, a few test cases that were failing
before start succeeding instead.

The firmware-auto-efi-format-nvramtemplate-qcow2 test case
originally passed but produced wrong results, then started
failing once we began taking templateFormat into account, and now
passes once again, finally producing the correct results.

The firmware-auto-efi-format-nvram-raw-loader-path and
firmware-auto-efi-format-nvram-raw-nvramtemplate-path test cases,
on the other hand, never passed before now, because the hack
resulted in information contradicting those provided by the user
being injected into the configuration, which in turn made it
impossible to successfully pick a firmware build. With the hack
gone they can finally succeed, as they should have in the first
place.

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

diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index 8714538ba3aad70f728dbf88325669b5734eddfe..70ac88c37333bc5b264587a20aa4abf3f41c12d6 100644 (file)
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -1859,21 +1859,6 @@ qemuFirmwareFillDomain(virQEMUDriver *driver,
     bool autoSelection = (def->os.firmware != VIR_DOMAIN_OS_DEF_FIRMWARE_NONE);
     int ret;
 
-    /* If there is no <loader> element but the <nvram> element
-     * was present, copy the format from the latter to the
-     * former.
-     *
-     * This ensures that a configuration such as
-     *
-     *   <os>
-     *     <nvram format='foo'/>
-     *   </os>
-     *
-     * behaves as expected, that is, results in a firmware build
-     * with format 'foo' being selected */
-    if (loader && loader->nvram && !loader->format)
-        loader->format = loader->nvram->format;
-
     /* If we're loading an existing configuration from disk, we
      * should try as hard as possible to preserve historical
      * behavior. In particular, firmware autoselection being enabled
@@ -1889,7 +1874,9 @@ qemuFirmwareFillDomain(virQEMUDriver *driver,
             def->os.loader = virDomainLoaderDefNew();
             loader = def->os.loader;
         }
-        if (!loader->format) {
+        if (!loader->format &&
+            !loader->nvramTemplateFormat &&
+            (!loader->nvram || !loader->nvram->format)) {
             loader->format = VIR_STORAGE_FILE_RAW;
         }
     }

diff --git a/tests/qemuxmlconfdata/firmware-auto-efi-format-nvram-raw-loader-path.x86_64-latest.args b/tests/qemuxmlconfdata/firmware-auto-efi-format-nvram-raw-loader-path.x86_64-latest.args
new file mode 100644 (file)
index 0000000..14027c2
--- /dev/null
+++ b/tests/qemuxmlconfdata/firmware-auto-efi-format-nvram-raw-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/edk2/ovmf/OVMF_CODE_4M.secboot.qcow2","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}' \
+-blockdev '{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"qcow2","file":"libvirt-pflash0-storage","backing":null}' \
+-blockdev '{"driver":"file","filename":"/var/lib/libvirt/qemu/nvram/guest_VARS.fd","node-name":"libvirt-pflash1-storage","read-only":false}' \
+-machine pc-q35-10.0,usb=off,smm=on,dump-guest-core=off,memory-backend=pc.ram,pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-storage,acpi=on \
+-accel kvm \
+-cpu qemu64 \
+-global driver=cfi.pflash01,property=secure,value=on \
+-m size=1048576k \
+-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/qemuxmlconfdata/firmware-auto-efi-format-nvram-raw-loader-path.x86_64-latest.err b/tests/qemuxmlconfdata/firmware-auto-efi-format-nvram-raw-loader-path.x86_64-latest.err
deleted file mode 100644 (file)
index 3edb2b3..0000000
--- a/tests/qemuxmlconfdata/firmware-auto-efi-format-nvram-raw-loader-path.x86_64-latest.err
+++ /dev/null
@@ -1 +0,0 @@
-operation failed: Unable to find 'efi' firmware that is compatible with the current configuration

diff --git a/tests/qemuxmlconfdata/firmware-auto-efi-format-nvram-raw-loader-path.x86_64-latest.xml b/tests/qemuxmlconfdata/firmware-auto-efi-format-nvram-raw-loader-path.x86_64-latest.xml
index 6bb1ad1507260865fa474f4043d4311f4ffb1def..a02714d7b9d2b5f782237646ff3b9684c093a809 100644 (file)
--- a/tests/qemuxmlconfdata/firmware-auto-efi-format-nvram-raw-loader-path.x86_64-latest.xml
+++ b/tests/qemuxmlconfdata/firmware-auto-efi-format-nvram-raw-loader-path.x86_64-latest.xml
@@ -6,12 +6,17 @@
   <vcpu placement='static'>1</vcpu>
   <os firmware='efi'>
     <type arch='x86_64' machine='pc-q35-10.0'>hvm</type>
-    <loader type='pflash' format='raw'>/usr/share/edk2/ovmf/OVMF_CODE_4M.secboot.qcow2</loader>
-    <nvram format='raw'/>
+    <firmware>
+      <feature enabled='yes' name='enrolled-keys'/>
+      <feature enabled='yes' name='secure-boot'/>
+    </firmware>
+    <loader readonly='yes' secure='yes' type='pflash' format='qcow2'>/usr/share/edk2/ovmf/OVMF_CODE_4M.secboot.qcow2</loader>
+    <nvram template='/usr/share/edk2/ovmf/OVMF_VARS_4M.secboot.qcow2' templateFormat='qcow2' format='raw'>/var/lib/libvirt/qemu/nvram/guest_VARS.fd</nvram>
     <boot dev='hd'/>
   </os>
   <features>
     <acpi/>
+    <smm state='on'/>
   </features>
   <cpu mode='custom' match='exact' check='none'>
     <model fallback='forbid'>qemu64</model>

diff --git a/tests/qemuxmlconfdata/firmware-auto-efi-format-nvram-raw-nvramtemplate-path.x86_64-latest.args b/tests/qemuxmlconfdata/firmware-auto-efi-format-nvram-raw-nvramtemplate-path.x86_64-latest.args
new file mode 100644 (file)
index 0000000..14027c2
--- /dev/null
+++ b/tests/qemuxmlconfdata/firmware-auto-efi-format-nvram-raw-nvramtemplate-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/edk2/ovmf/OVMF_CODE_4M.secboot.qcow2","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}' \
+-blockdev '{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"qcow2","file":"libvirt-pflash0-storage","backing":null}' \
+-blockdev '{"driver":"file","filename":"/var/lib/libvirt/qemu/nvram/guest_VARS.fd","node-name":"libvirt-pflash1-storage","read-only":false}' \
+-machine pc-q35-10.0,usb=off,smm=on,dump-guest-core=off,memory-backend=pc.ram,pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-storage,acpi=on \
+-accel kvm \
+-cpu qemu64 \
+-global driver=cfi.pflash01,property=secure,value=on \
+-m size=1048576k \
+-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/qemuxmlconfdata/firmware-auto-efi-format-nvram-raw-nvramtemplate-path.x86_64-latest.err b/tests/qemuxmlconfdata/firmware-auto-efi-format-nvram-raw-nvramtemplate-path.x86_64-latest.err
deleted file mode 100644 (file)
index 3edb2b3..0000000
--- a/tests/qemuxmlconfdata/firmware-auto-efi-format-nvram-raw-nvramtemplate-path.x86_64-latest.err
+++ /dev/null
@@ -1 +0,0 @@
-operation failed: Unable to find 'efi' firmware that is compatible with the current configuration

diff --git a/tests/qemuxmlconfdata/firmware-auto-efi-format-nvram-raw-nvramtemplate-path.x86_64-latest.xml b/tests/qemuxmlconfdata/firmware-auto-efi-format-nvram-raw-nvramtemplate-path.x86_64-latest.xml
index 8bb8f1b26ccb29496f16580452b5b6dbc98c6dce..a02714d7b9d2b5f782237646ff3b9684c093a809 100644 (file)
--- a/tests/qemuxmlconfdata/firmware-auto-efi-format-nvram-raw-nvramtemplate-path.x86_64-latest.xml
+++ b/tests/qemuxmlconfdata/firmware-auto-efi-format-nvram-raw-nvramtemplate-path.x86_64-latest.xml
@@ -6,12 +6,17 @@
   <vcpu placement='static'>1</vcpu>
   <os firmware='efi'>
     <type arch='x86_64' machine='pc-q35-10.0'>hvm</type>
-    <loader format='raw'/>
-    <nvram template='/usr/share/edk2/ovmf/OVMF_VARS_4M.secboot.qcow2' format='raw'/>
+    <firmware>
+      <feature enabled='yes' name='enrolled-keys'/>
+      <feature enabled='yes' name='secure-boot'/>
+    </firmware>
+    <loader readonly='yes' secure='yes' type='pflash' format='qcow2'>/usr/share/edk2/ovmf/OVMF_CODE_4M.secboot.qcow2</loader>
+    <nvram template='/usr/share/edk2/ovmf/OVMF_VARS_4M.secboot.qcow2' templateFormat='qcow2' format='raw'>/var/lib/libvirt/qemu/nvram/guest_VARS.fd</nvram>
     <boot dev='hd'/>
   </os>
   <features>
     <acpi/>
+    <smm state='on'/>
   </features>
   <cpu mode='custom' match='exact' check='none'>
     <model fallback='forbid'>qemu64</model>

diff --git a/tests/qemuxmlconfdata/firmware-auto-efi-format-nvramtemplate-qcow2.x86_64-latest.args b/tests/qemuxmlconfdata/firmware-auto-efi-format-nvramtemplate-qcow2.x86_64-latest.args
new file mode 100644 (file)
index 0000000..468ad43
--- /dev/null
+++ b/tests/qemuxmlconfdata/firmware-auto-efi-format-nvramtemplate-qcow2.x86_64-latest.args
@@ -0,0 +1,38 @@
+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/edk2/ovmf/OVMF_CODE_4M.secboot.qcow2","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}' \
+-blockdev '{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"qcow2","file":"libvirt-pflash0-storage","backing":null}' \
+-blockdev '{"driver":"file","filename":"/var/lib/libvirt/qemu/nvram/guest_VARS.qcow2","node-name":"libvirt-pflash1-storage","auto-read-only":true,"discard":"unmap"}' \
+-blockdev '{"node-name":"libvirt-pflash1-format","read-only":false,"driver":"qcow2","file":"libvirt-pflash1-storage","backing":null}' \
+-machine pc-q35-10.0,usb=off,smm=on,dump-guest-core=off,memory-backend=pc.ram,pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-format,acpi=on \
+-accel kvm \
+-cpu qemu64 \
+-global driver=cfi.pflash01,property=secure,value=on \
+-m size=1048576k \
+-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/qemuxmlconfdata/firmware-auto-efi-format-nvramtemplate-qcow2.x86_64-latest.err b/tests/qemuxmlconfdata/firmware-auto-efi-format-nvramtemplate-qcow2.x86_64-latest.err
deleted file mode 100644 (file)
index 3edb2b3..0000000
--- a/tests/qemuxmlconfdata/firmware-auto-efi-format-nvramtemplate-qcow2.x86_64-latest.err
+++ /dev/null
@@ -1 +0,0 @@
-operation failed: Unable to find 'efi' firmware that is compatible with the current configuration

diff --git a/tests/qemuxmlconfdata/firmware-auto-efi-format-nvramtemplate-qcow2.x86_64-latest.xml b/tests/qemuxmlconfdata/firmware-auto-efi-format-nvramtemplate-qcow2.x86_64-latest.xml
index 1f039061ba9cc14eef31bc6025dfecfd11f18ae5..4061a0ae35b3da786c8a21b7bc8c94b9d019d2e0 100644 (file)
--- a/tests/qemuxmlconfdata/firmware-auto-efi-format-nvramtemplate-qcow2.x86_64-latest.xml
+++ b/tests/qemuxmlconfdata/firmware-auto-efi-format-nvramtemplate-qcow2.x86_64-latest.xml
@@ -6,12 +6,17 @@
   <vcpu placement='static'>1</vcpu>
   <os firmware='efi'>
     <type arch='x86_64' machine='pc-q35-10.0'>hvm</type>
-    <loader format='raw'/>
-    <nvram templateFormat='qcow2'/>
+    <firmware>
+      <feature enabled='yes' name='enrolled-keys'/>
+      <feature enabled='yes' name='secure-boot'/>
+    </firmware>
+    <loader readonly='yes' secure='yes' type='pflash' format='qcow2'>/usr/share/edk2/ovmf/OVMF_CODE_4M.secboot.qcow2</loader>
+    <nvram template='/usr/share/edk2/ovmf/OVMF_VARS_4M.secboot.qcow2' templateFormat='qcow2' format='qcow2'>/var/lib/libvirt/qemu/nvram/guest_VARS.qcow2</nvram>
     <boot dev='hd'/>
   </os>
   <features>
     <acpi/>
+    <smm state='on'/>
   </features>
   <cpu mode='custom' match='exact' check='none'>
     <model fallback='forbid'>qemu64</model>

diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c
index 75db051e3244826915ed1def664123b4cf18de8c..a87863410f057431beccc2820f1ce94ef6218b57 100644 (file)
--- a/tests/qemuxmlconftest.c
+++ b/tests/qemuxmlconftest.c
@@ -1610,11 +1610,11 @@ mymain(void)
     DO_TEST_CAPS_LATEST("firmware-auto-efi-format-nvram-qcow2-network-nbd");
     DO_TEST_CAPS_LATEST("firmware-auto-efi-format-nvram-raw");
     DO_TEST_CAPS_LATEST_ABI_UPDATE("firmware-auto-efi-format-nvram-raw");
-    DO_TEST_CAPS_LATEST_FAILURE("firmware-auto-efi-format-nvram-raw-loader-path");
-    DO_TEST_CAPS_LATEST_FAILURE("firmware-auto-efi-format-nvram-raw-nvramtemplate-path");
+    DO_TEST_CAPS_LATEST("firmware-auto-efi-format-nvram-raw-loader-path");
+    DO_TEST_CAPS_LATEST("firmware-auto-efi-format-nvram-raw-nvramtemplate-path");
     DO_TEST_CAPS_ARCH_LATEST("firmware-auto-efi-format-loader-raw", "aarch64");
     DO_TEST_CAPS_ARCH_LATEST_ABI_UPDATE("firmware-auto-efi-format-loader-raw", "aarch64");
-    DO_TEST_CAPS_LATEST_FAILURE("firmware-auto-efi-format-nvramtemplate-qcow2");
+    DO_TEST_CAPS_LATEST("firmware-auto-efi-format-nvramtemplate-qcow2");
     DO_TEST_CAPS_LATEST("firmware-auto-efi-format-mismatch");
     DO_TEST_CAPS_LATEST_FAILURE("firmware-auto-efi-format-mismatch-nvramtemplate");