qemu_firmware: Allow matching stateful ROMs

Stateful ROMs are those that use the uefi-vars QEMU device to
implement access to UEFI variable storage.

Matching works much the same as it does for pflash-based
firmware images. Notably, the <varstore> element is only
allowed for ROM and the <nvram> element is only allowed for
pflash.

The firmware-auto-efi-varstore-q35 and
firmware-auto-efi-varstore-aarch64 fail in a different way
after this change: the input XML is now considered valid, and
the only remaining issue is that the firmware autoselection
process is unable to find a match.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index 1851ed4a808468ff93332485b7b6dd978833cbd4..60635b559ff70921ceb1d888add08a59432fd3a0 100644 (file)
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -893,15 +893,18 @@ qemuFirmwareMatchesMachineArch(const qemuFirmware *fw,
  * qemuFirmwareMatchesPaths:
  * @fw: firmware definition
  * @loader: loader definition
+ * @varstore: varstore definition
  *
  * Checks whether @fw is compatible with the information provided as
  * part of the domain definition.
  *
- * Returns: true if @fw is compatible with @loader, false otherwise
+ * Returns: true if @fw is compatible with @loader and @varstore,
+ *          false otherwise
  */
 static bool
 qemuFirmwareMatchesPaths(const qemuFirmware *fw,
-                         const virDomainLoaderDef *loader)
+                         const virDomainLoaderDef *loader,
+                         const virDomainVarstoreDef *varstore)
 {
     const qemuFirmwareMappingFlash *flash = &fw->mapping.data.flash;
     const qemuFirmwareMappingMemory *memory = &fw->mapping.data.memory;
@@ -922,6 +925,9 @@ qemuFirmwareMatchesPaths(const qemuFirmware *fw,
         if (loader && loader->path &&
             !virFileComparePaths(loader->path, memory->filename))
             return false;
+        if (varstore && varstore->template &&
+            !virFileComparePaths(varstore->template, memory->template))
+            return false;
         break;
     case QEMU_FIRMWARE_DEVICE_NONE:
     case QEMU_FIRMWARE_DEVICE_LAST:
@@ -1112,6 +1118,7 @@ qemuFirmwareMatchDomain(const virDomainDef *def,
                         const char *path)
 {
     const virDomainLoaderDef *loader = def->os.loader;
+    const virDomainVarstoreDef *varstore = def->os.varstore;
     size_t i;
     qemuFirmwareOSInterface want;
     bool wantUEFI = false;
@@ -1166,7 +1173,7 @@ qemuFirmwareMatchDomain(const virDomainDef *def,
         return false;
     }
 
-    if (!qemuFirmwareMatchesPaths(fw, def->os.loader)) {
+    if (!qemuFirmwareMatchesPaths(fw, def->os.loader, def->os.varstore)) {
         VIR_DEBUG("No matching path in '%s'", path);
         return false;
     }
@@ -1279,6 +1286,9 @@ qemuFirmwareMatchDomain(const virDomainDef *def,
     if (fw->mapping.device == QEMU_FIRMWARE_DEVICE_FLASH) {
         const qemuFirmwareMappingFlash *flash = &fw->mapping.data.flash;
 
+        if (varstore)
+            return false;
+
         if (loader && loader->type &&
             loader->type != VIR_DOMAIN_LOADER_TYPE_PFLASH) {
             VIR_DEBUG("Discarding flash loader");
@@ -1377,16 +1387,38 @@ qemuFirmwareMatchDomain(const virDomainDef *def,
             }
         }
     } else if (fw->mapping.device == QEMU_FIRMWARE_DEVICE_MEMORY) {
+        const qemuFirmwareMappingMemory *memory = &fw->mapping.data.memory;
+
+        if (loader && loader->nvram)
+            return false;
+
         if (loader && loader->type &&
             loader->type != VIR_DOMAIN_LOADER_TYPE_ROM) {
             VIR_DEBUG("Discarding rom loader");
             return false;
         }
 
-        if (loader && loader->stateless == VIR_TRISTATE_BOOL_NO) {
+        /* Explicit requests for either a stateless or stateful
+         * firmware should be fulfilled, but if no preference is
+         * provided either one is fine as long as the other match
+         * criteria are satisfied. varstore implies stateful */
+        if (loader &&
+            loader->stateless == VIR_TRISTATE_BOOL_NO &&
+            !memory->template) {
+            VIR_DEBUG("Discarding stateless loader");
+            return false;
+        }
+        if (varstore &&
+            !memory->template) {
             VIR_DEBUG("Discarding stateless loader");
             return false;
         }
+        if (loader &&
+            loader->stateless == VIR_TRISTATE_BOOL_YES &&
+            memory->template) {
+            VIR_DEBUG("Discarding non-stateless loader");
+            return false;
+        }
 
         if (loader && loader->readonly == VIR_TRISTATE_BOOL_NO) {
             VIR_DEBUG("Discarding readonly loader");

diff --git a/tests/qemuxmlconfdata/firmware-auto-efi-varstore-aarch64.aarch64-latest.err b/tests/qemuxmlconfdata/firmware-auto-efi-varstore-aarch64.aarch64-latest.err
index b45d304221cf0a0c3981cc5a94821d16a93360d6..3edb2b34511a589b675c3048df93272640cdb75d 100644 (file)
--- a/tests/qemuxmlconfdata/firmware-auto-efi-varstore-aarch64.aarch64-latest.err
+++ b/tests/qemuxmlconfdata/firmware-auto-efi-varstore-aarch64.aarch64-latest.err
@@ -1 +1 @@
-Only one of NVRAM/varstore can be used
+operation failed: Unable to find 'efi' firmware that is compatible with the current configuration

diff --git a/tests/qemuxmlconfdata/firmware-auto-efi-varstore-aarch64.aarch64-latest.xml b/tests/qemuxmlconfdata/firmware-auto-efi-varstore-aarch64.aarch64-latest.xml
new file mode 100644 (file)
index 0000000..b0fa092
--- /dev/null
+++ b/tests/qemuxmlconfdata/firmware-auto-efi-varstore-aarch64.aarch64-latest.xml
@@ -0,0 +1,28 @@
+<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='aarch64' machine='virt-8.2'>hvm</type>
+    <loader format='raw'/>
+    <varstore/>
+    <boot dev='hd'/>
+  </os>
+  <features>
+    <acpi/>
+    <gic version='2'/>
+  </features>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-aarch64</emulator>
+    <controller type='usb' index='0' model='none'/>
+    <controller type='pci' index='0' model='pcie-root'/>
+    <audio id='1' type='none'/>
+    <memballoon model='none'/>
+  </devices>
+</domain>

diff --git a/tests/qemuxmlconfdata/firmware-auto-efi-varstore-q35.x86_64-latest.err b/tests/qemuxmlconfdata/firmware-auto-efi-varstore-q35.x86_64-latest.err
index b45d304221cf0a0c3981cc5a94821d16a93360d6..3edb2b34511a589b675c3048df93272640cdb75d 100644 (file)
--- a/tests/qemuxmlconfdata/firmware-auto-efi-varstore-q35.x86_64-latest.err
+++ b/tests/qemuxmlconfdata/firmware-auto-efi-varstore-q35.x86_64-latest.err
@@ -1 +1 @@
-Only one of NVRAM/varstore can be used
+operation failed: Unable to find 'efi' firmware that is compatible with the current configuration

diff --git a/tests/qemuxmlconfdata/firmware-auto-efi-varstore-q35.x86_64-latest.xml b/tests/qemuxmlconfdata/firmware-auto-efi-varstore-q35.x86_64-latest.xml
new file mode 100644 (file)
index 0000000..c4d70c9
--- /dev/null
+++ b/tests/qemuxmlconfdata/firmware-auto-efi-varstore-q35.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 firmware='efi'>
+    <type arch='x86_64' machine='pc-q35-8.2'>hvm</type>
+    <loader format='raw'/>
+    <varstore/>
+    <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/qemuxmlconftest.c b/tests/qemuxmlconftest.c
index ca00dc96f0ba18d127937ccb1dee3d6437715c7f..43e994fc937880d8388cab8f262c4bfeca53316c 100644 (file)
--- a/tests/qemuxmlconftest.c
+++ b/tests/qemuxmlconftest.c
@@ -1673,8 +1673,8 @@ mymain(void)
     DO_TEST_CAPS_LATEST("firmware-auto-efi-nvram-file");
     DO_TEST_CAPS_LATEST("firmware-auto-efi-nvram-network-nbd");
     DO_TEST_CAPS_LATEST("firmware-auto-efi-nvram-network-iscsi");
-    DO_TEST_CAPS_LATEST_PARSE_ERROR("firmware-auto-efi-varstore-q35");
-    DO_TEST_CAPS_ARCH_LATEST_PARSE_ERROR("firmware-auto-efi-varstore-aarch64", "aarch64");
+    DO_TEST_CAPS_LATEST_FAILURE("firmware-auto-efi-varstore-q35");
+    DO_TEST_CAPS_ARCH_LATEST_FAILURE("firmware-auto-efi-varstore-aarch64", "aarch64");
 
     DO_TEST_CAPS_LATEST("firmware-auto-efi-format-loader-qcow2");
     DO_TEST_CAPS_LATEST_PARSE_ERROR("firmware-auto-efi-format-loader-qcow2-rom");