qemu_firmware: Extend qemuFirmwareGetSupported to return FW paths

The qemuFirmwareGetSupported() function is called from qemu
driver to generate domain capabilities XML based on FW descriptor
files. However, the function currently reports only some features
from domcapabilities XML and not actual FW image paths. The paths
reported in the domcapabilities XML are still from pre-FW
descriptor era and therefore the XML might be a bit confusing.
For instance, it may say that secure boot is supported but
secboot enabled FW is not in the listed FW image paths.

To resolve this problem, change qemuFirmwareGetSupported() so
that it also returns a list of FW images (we have the list
anyway). Luckily, we already have a structure to represent a FW
image - virFirmware.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1733940

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 136999ad0dbc530f6c56e448bb6b7f014d78a4fb..9ff432f8c42d6f55502c6ec4483a2996e6b53135 100644 (file)
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -5185,7 +5185,8 @@ virQEMUCapsFillDomainOSCaps(virDomainCapsOSPtr os,
     os->supported = VIR_TRISTATE_BOOL_YES;
     os->firmware.report = true;
 
-    if (qemuFirmwareGetSupported(machine, arch, privileged, &autoFirmwares, &secure) < 0)
+    if (qemuFirmwareGetSupported(machine, arch, privileged,
+                                 &autoFirmwares, &secure, NULL, NULL) < 0)
         return -1;
 
     if (autoFirmwares & (1ULL << VIR_DOMAIN_OS_DEF_FIRMWARE_BIOS))

diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index 1676a9c3d03ca2f616c9c1f99622edd181db5c50..f316a26a5ba5c14d993fa8b96df32eba3c1f0997 100644 (file)
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -1420,6 +1420,8 @@ qemuFirmwareFillDomain(virQEMUDriverPtr driver,
  * @privileged: whether running as privileged user
  * @supported: returned bitmap of supported interfaces
  * @secure: true if at least one secure boot enabled FW was found
+ * @fws: (optional) list of found firmwares
+ * @nfws: (optional) number of members in @fws
  *
  * Parse all FW descriptors (depending whether running as @privileged this may
  * or may not include user's $HOME) and for given combination of @machine and
@@ -1429,6 +1431,15 @@ qemuFirmwareFillDomain(virQEMUDriverPtr driver,
  * FW descriptor signalizes secure boot (although, this is checked against SMM
  * rather than SECURE_BOOT because reasons).
  *
+ * If @fws and @nfws are not NULL, then @fws is allocated (must be freed by
+ * caller when no longer needed) and contains list of firmwares found in form
+ * of virFirmware. This can be useful if caller wants to know the paths to
+ * firmware images (e.g. to present them in domain capabilities XML).
+ * Moreover, to allow the caller distinguish between no FW descriptors found
+ * and no matching FW descriptors found (nfws == 0 in both cases), the @fws is
+ * going to be allocated in case of the latter anyway (with no real content
+ * though).
+ *
  * Returns: 0 on success,
  *         -1 otherwise.
  */
@@ -1437,7 +1448,9 @@ qemuFirmwareGetSupported(const char *machine,
                          virArch arch,
                          bool privileged,
                          uint64_t *supported,
-                         bool *secure)
+                         bool *secure,
+                         virFirmwarePtr **fws,
+                         size_t *nfws)
 {
     qemuFirmwarePtr *firmwares = NULL;
     ssize_t nfirmwares = 0;
@@ -1446,12 +1459,21 @@ qemuFirmwareGetSupported(const char *machine,
     *supported = VIR_DOMAIN_OS_DEF_FIRMWARE_NONE;
     *secure = false;
 
+    if (fws) {
+        *fws = NULL;
+        *nfws = 0;
+    }
+
     if ((nfirmwares = qemuFirmwareFetchParsedConfigs(privileged,
                                                      &firmwares, NULL)) < 0)
         return -1;
 
     for (i = 0; i < nfirmwares; i++) {
         qemuFirmwarePtr fw = firmwares[i];
+        const qemuFirmwareMappingFlash *flash = &fw->mapping.data.flash;
+        const qemuFirmwareMappingMemory *memory = &fw->mapping.data.memory;
+        const char *fwpath = NULL;
+        const char *nvrampath = NULL;
         size_t j;
 
         if (!qemuFirmwareMatchesMachineArch(fw, machine, arch))
@@ -1491,8 +1513,46 @@ qemuFirmwareGetSupported(const char *machine,
                 break;
             }
         }
+
+        switch (fw->mapping.device) {
+        case QEMU_FIRMWARE_DEVICE_FLASH:
+            fwpath = flash->executable.filename;
+            nvrampath = flash->nvram_template.filename;
+            break;
+
+        case QEMU_FIRMWARE_DEVICE_MEMORY:
+            fwpath = memory->filename;
+            break;
+
+        case QEMU_FIRMWARE_DEVICE_KERNEL:
+        case QEMU_FIRMWARE_DEVICE_NONE:
+        case QEMU_FIRMWARE_DEVICE_LAST:
+            break;
+        }
+
+        if (fws && fwpath) {
+            VIR_AUTOPTR(virFirmware) tmp = NULL;
+
+            /* Append only unique pairs. */
+            for (j = 0; j < *nfws; j++) {
+                if (STREQ((*fws)[j]->name, fwpath) &&
+                    STREQ_NULLABLE((*fws)[j]->nvram, nvrampath))
+                    break;
+            }
+
+            if (j == *nfws &&
+                (VIR_ALLOC(tmp) < 0 ||
+                 VIR_STRDUP(tmp->name, fwpath) < 0 ||
+                 VIR_STRDUP(tmp->nvram, nvrampath) < 0 ||
+                 VIR_APPEND_ELEMENT(*fws, *nfws, tmp) < 0))
+                return -1;
+        }
     }
 
+    if (fws && !*fws && nfirmwares &&
+        VIR_REALLOC_N(*fws, 0) < 0)
+        return -1;
+
     for (i = 0; i < nfirmwares; i++)
         qemuFirmwareFree(firmwares[i]);
     VIR_FREE(firmwares);

diff --git a/src/qemu/qemu_firmware.h b/src/qemu/qemu_firmware.h
index 6a3b6061f495b795f8415763c0a786e8239fe60b..28e8322ec9223ac9702ef2a8f9da203265b3932f 100644 (file)
--- a/src/qemu/qemu_firmware.h
+++ b/src/qemu/qemu_firmware.h
@@ -24,6 +24,7 @@
 #include "qemu_conf.h"
 #include "virautoclean.h"
 #include "virarch.h"
+#include "virfirmware.h"
 
 typedef struct _qemuFirmware qemuFirmware;
 typedef qemuFirmware *qemuFirmwarePtr;
@@ -53,6 +54,8 @@ qemuFirmwareGetSupported(const char *machine,
                          virArch arch,
                          bool privileged,
                          uint64_t *supported,
-                         bool *secure);
+                         bool *secure,
+                         virFirmwarePtr **fws,
+                         size_t *nfws);
 
 verify(VIR_DOMAIN_OS_DEF_FIRMWARE_LAST <= 64);

diff --git a/tests/qemufirmwaretest.c b/tests/qemufirmwaretest.c
index 2228359a7bea3d62bed66b2e01861ea1ff6c451c..bab23f696e3529df0c2852d3fd2c0a00ff7d35cb 100644 (file)
--- a/tests/qemufirmwaretest.c
+++ b/tests/qemufirmwaretest.c
@@ -123,7 +123,7 @@ testSupportedFW(const void *opaque)
         expectedInterfaces |= 1ULL << data->interfaces[i];
 
     if (qemuFirmwareGetSupported(data->machine, data->arch, false,
-                                 &actualInterfaces, &actualSecure) < 0) {
+                                 &actualInterfaces, &actualSecure, NULL, NULL) < 0) {
         fprintf(stderr, "Unable to get list of supported interfaces\n");
         return -1;
     }