At the moment, any kind of issue being detected in any of the
firmware descriptor files will result in the entire process
being aborted.

In particular, installing a build of edk2 for an architecture
that libvirt doesn't yet know about, for example loongarch64,
will break most firmware-related functionality: it will no
longer be possible to define new EFI VMs, start existing ones,
or even just obtain the domcapabilities for any architecture.

This is obviously unnecessarily harsh. Adopt a more relaxed
approach and simply ignore the firmware descriptors that we
are unable to parse correctly.

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

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 src/qemu/qemu_firmware.c | 86 ++++++++++------------------------------
 1 file changed, 20 insertions(+), 66 deletions(-)

diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index 4fc7dd3b71..78844e3066 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -307,9 +307,7 @@ qemuFirmwareInterfaceParse(const char *path,
     size_t i;
 
     if (!(interfacesJSON = virJSONValueObjectGetArray(doc, "interface-types"))) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("failed to get interface-types from '%1$s'"),
-                       path);
+        VIR_DEBUG("failed to get interface-types from '%s'", path);
         return -1;
     }
 
@@ -323,9 +321,7 @@ qemuFirmwareInterfaceParse(const char *path,
         int tmp;
 
         if ((tmp = qemuFirmwareOSInterfaceTypeFromString(tmpStr)) <= 0) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("unknown interface type: '%1$s'"),
-                           tmpStr);
+            VIR_DEBUG("unknown interface type: '%s'", tmpStr);
             return -1;
         }
 
@@ -351,18 +347,14 @@ qemuFirmwareFlashFileParse(const char *path,
     const char *format;
 
     if (!(filename = virJSONValueObjectGetString(doc, "filename"))) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("missing 'filename' in '%1$s'"),
-                       path);
+        VIR_DEBUG("missing 'filename' in '%s'", path);
         return -1;
     }
 
     flash->filename = g_strdup(filename);
 
     if (!(format = virJSONValueObjectGetString(doc, "format"))) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("missing 'format' in '%1$s'"),
-                       path);
+        VIR_DEBUG("missing 'format' in '%s'", path);
         return -1;
     }
 
@@ -388,24 +380,19 @@ qemuFirmwareMappingFlashParse(const char *path,
         const char *modestr = virJSONValueGetString(mode);
         int modeval;
         if (!modestr) {
-            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                           _("Firmware flash mode value was malformed"));
+            VIR_DEBUG("Firmware flash mode value was malformed");
             return -1;
         }
         modeval = qemuFirmwareFlashModeTypeFromString(modestr);
         if (modeval < 0) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("Firmware flash mode value '%1$s' unexpected"),
-                           modestr);
+            VIR_DEBUG("Firmware flash mode value '%s' unexpected", modestr);
             return -1;
         }
         flash->mode = modeval;
     }
 
     if (!(executable = virJSONValueObjectGet(doc, "executable"))) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("missing 'executable' in '%1$s'"),
-                       path);
+        VIR_DEBUG("missing 'executable' in '%s'", path);
         return -1;
     }
 
@@ -414,9 +401,7 @@ qemuFirmwareMappingFlashParse(const char *path,
 
     if (flash->mode == QEMU_FIRMWARE_FLASH_MODE_SPLIT) {
         if (!(nvram_template = virJSONValueObjectGet(doc, "nvram-template"))) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("missing 'nvram-template' in '%1$s'"),
-                           path);
+            VIR_DEBUG("missing 'nvram-template' in '%s'", path);
             return -1;
         }
 
@@ -436,9 +421,7 @@ qemuFirmwareMappingKernelParse(const char *path,
     const char *filename;
 
     if (!(filename = virJSONValueObjectGetString(doc, "filename"))) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("missing 'filename' in '%1$s'"),
-                       path);
+        VIR_DEBUG("missing 'filename' in '%s'", path);
         return -1;
     }
 
@@ -456,9 +439,7 @@ qemuFirmwareMappingMemoryParse(const char *path,
     const char *filename;
 
     if (!(filename = virJSONValueObjectGetString(doc, "filename"))) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("missing 'filename' in '%1$s'"),
-                       path);
+        VIR_DEBUG("missing 'filename' in '%s'", path);
         return -1;
     }
 
@@ -478,23 +459,17 @@ qemuFirmwareMappingParse(const char *path,
     int tmp;
 
     if (!(mapping = virJSONValueObjectGet(doc, "mapping"))) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("missing mapping in '%1$s'"),
-                       path);
+        VIR_DEBUG("missing mapping in '%s'", path);
         return -1;
     }
 
     if (!(deviceStr = virJSONValueObjectGetString(mapping, "device"))) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("missing device type in '%1$s'"),
-                       path);
+        VIR_DEBUG("missing device type in '%s'", path);
         return -1;
     }
 
     if ((tmp = qemuFirmwareDeviceTypeFromString(deviceStr)) <= 0) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("unknown device type in '%1$s'"),
-                       path);
+        VIR_DEBUG("unknown device type in '%s'", path);
         return -1;
     }
 
@@ -535,9 +510,7 @@ qemuFirmwareTargetParse(const char *path,
     int ret = -1;
 
     if (!(targetsJSON = virJSONValueObjectGetArray(doc, "targets"))) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("failed to get targets from '%1$s'"),
-                       path);
+        VIR_DEBUG("failed to get targets from '%s'", path);
         return -1;
     }
 
@@ -556,23 +529,17 @@ qemuFirmwareTargetParse(const char *path,
         t = g_new0(qemuFirmwareTarget, 1);
 
         if (!(architectureStr = virJSONValueObjectGetString(item, "architecture"))) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("missing 'architecture' in '%1$s'"),
-                           path);
+            VIR_DEBUG("missing 'architecture' in '%s'", path);
             goto cleanup;
         }
 
         if ((t->architecture = virQEMUCapsArchFromString(architectureStr)) == VIR_ARCH_NONE) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("unknown architecture '%1$s'"),
-                           architectureStr);
+            VIR_DEBUG("unknown architecture '%s'", architectureStr);
             goto cleanup;
         }
 
         if (!(machines = virJSONValueObjectGetArray(item, "machines"))) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("missing 'machines' in '%1$s'"),
-                           path);
+            VIR_DEBUG("missing 'machines' in '%s'", path);
             goto cleanup;
         }
 
@@ -617,9 +584,7 @@ qemuFirmwareFeatureParse(const char *path,
     size_t i;
 
     if (!(featuresJSON = virJSONValueObjectGetArray(doc, "features"))) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("failed to get features from '%1$s'"),
-                       path);
+        VIR_DEBUG("failed to get features from '%s'", path);
         return -1;
     }
 
@@ -661,9 +626,7 @@ qemuFirmwareParse(const char *path)
         return NULL;
 
     if (!(doc = virJSONValueFromString(cont))) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("unable to parse json file '%1$s'"),
-                       path);
+        VIR_DEBUG("unable to parse json file '%s'", path);
         return NULL;
     }
 
@@ -1579,7 +1542,7 @@ qemuFirmwareFetchParsedConfigs(bool privileged,
         qemuFirmware *firmware = qemuFirmwareParse(*currentPath);
 
         if (!firmware)
-            goto error;
+            continue;
 
         VIR_APPEND_ELEMENT(firmwares, nfirmwares, firmware);
 
@@ -1597,15 +1560,6 @@ qemuFirmwareFetchParsedConfigs(bool privileged,
     }
 
     return nfirmwares;
-
- error:
-    while (nfirmwares > 0)
-        qemuFirmwareFree(firmwares[--nfirmwares]);
-    VIR_FREE(firmwares);
-    while (npaths > 0)
-        VIR_FREE(paths[--npaths]);
-    VIR_FREE(paths);
-    return -1;
 }
 
 
-- 
2.43.2
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org