driver config: Introduce virFirmware object

The virQEMUDriverConfig object contains lists of
loader:nvram pairs to advertise firmwares supported by
by the driver, and qemu_conf.c contains code to populate
the lists, all of which is useful for other drivers too.

To avoid code duplication, introduce a virFirmware object
to encapsulate firmware details and switch the qemu driver
to use it.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>

po/POTFILES.in

@@ -194,6 +194,7 @@ src/util/virerror.h
 src/util/vireventpoll.c
 src/util/virfile.c
 src/util/virfirewall.c
+src/util/virfirmware.c
 src/util/virhash.c
 src/util/virhook.c
 src/util/virhostcpu.c

src/Makefile.am

@@ -114,6 +114,7 @@ UTIL_SOURCES =							\
 		util/virfile.c util/virfile.h			\
 		util/virfirewall.c util/virfirewall.h		\
 		util/virfirewallpriv.h				\
+		util/virfirmware.c util/virfirmware.h		\
 		util/virgettext.c util/virgettext.h		\
 		util/virgic.c util/virgic.h			\
 		util/virhash.c util/virhash.h			\

src/libvirt_private.syms

@@ -1564,6 +1564,12 @@ virFirewallStartRollback;
 virFirewallStartTransaction;
 
 
+# util/virfirmware.h
+virFirmwareFreeList;
+virFirmwareParse;
+virFirmwareParseList;
+
+
 # util/virgettext.h
 virGettextInitialize;
 

src/qemu/qemu_capabilities.c

@@ -4071,18 +4071,18 @@ virQEMUCapsGetDefaultMachine(virQEMUCapsPtr qemuCaps)
 
 static int
 virQEMUCapsFillDomainLoaderCaps(virDomainCapsLoaderPtr capsLoader,
-                                char **loader,
-                                size_t nloader)
+                                virFirmwarePtr *firmwares,
+                                size_t nfirmwares)
 {
     size_t i;
 
     capsLoader->supported = true;
 
-    if (VIR_ALLOC_N(capsLoader->values.values, nloader) < 0)
+    if (VIR_ALLOC_N(capsLoader->values.values, nfirmwares) < 0)
         return -1;
 
-    for (i = 0; i < nloader; i++) {
-        const char *filename = loader[i];
+    for (i = 0; i < nfirmwares; i++) {
+        const char *filename = firmwares[i]->name;
 
         if (!virFileExists(filename)) {
             VIR_DEBUG("loader filename=%s does not exist", filename);
@@ -4111,13 +4111,13 @@ virQEMUCapsFillDomainLoaderCaps(virDomainCapsLoaderPtr capsLoader,
 
 static int
 virQEMUCapsFillDomainOSCaps(virDomainCapsOSPtr os,
-                            char **loader,
-                            size_t nloader)
+                            virFirmwarePtr *firmwares,
+                            size_t nfirmwares)
 {
     virDomainCapsLoaderPtr capsLoader = &os->loader;
 
     os->supported = true;
-    if (virQEMUCapsFillDomainLoaderCaps(capsLoader, loader, nloader) < 0)
+    if (virQEMUCapsFillDomainLoaderCaps(capsLoader, firmwares, nfirmwares) < 0)
         return -1;
     return 0;
 }
@@ -4330,8 +4330,8 @@ virQEMUCapsFillDomainFeatureGICCaps(virQEMUCapsPtr qemuCaps,
 int
 virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps,
                           virQEMUCapsPtr qemuCaps,
-                          char **loader,
-                          size_t nloader)
+                          virFirmwarePtr *firmwares,
+                          size_t nfirmwares)
 {
     virDomainCapsOSPtr os = &domCaps->os;
     virDomainCapsDeviceDiskPtr disk = &domCaps->disk;
@@ -4342,7 +4342,7 @@ virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps,
 
     domCaps->maxvcpus = maxvcpus;
 
-    if (virQEMUCapsFillDomainOSCaps(os, loader, nloader) < 0 ||
+    if (virQEMUCapsFillDomainOSCaps(os, firmwares, nfirmwares) < 0 ||
         virQEMUCapsFillDomainDeviceDiskCaps(qemuCaps,
                                             domCaps->machine, disk) < 0 ||
         virQEMUCapsFillDomainDeviceGraphicsCaps(qemuCaps, graphics) < 0 ||

src/qemu/qemu_capabilities.h

@@ -29,6 +29,7 @@
 # include "vircommand.h"
 # include "qemu_monitor.h"
 # include "domain_capabilities.h"
+# include "virfirmware.h"
 
 /*
  * Internal flags to keep track of qemu command line capabilities
@@ -491,7 +492,7 @@ int virQEMUCapsInitGuestFromBinary(virCapsPtr caps,
 
 int virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps,
                               virQEMUCapsPtr qemuCaps,
-                              char **loader,
-                              size_t nloader);
+                              virFirmwarePtr *firmwares,
+                              size_t nfirmwares);
 
 #endif /* __QEMU_CAPABILITIES_H__*/

src/qemu/qemu_conf.c

@@ -124,47 +124,6 @@ void qemuDomainCmdlineDefFree(qemuDomainCmdlineDefPtr def)
 }
 
 
-static int ATTRIBUTE_UNUSED
-virQEMUDriverConfigLoaderNVRAMParse(virQEMUDriverConfigPtr cfg,
-                                    const char *list)
-{
-    int ret = -1;
-    char **token;
-    size_t i, j;
-
-    if (!(token = virStringSplit(list, ":", 0)))
-        goto cleanup;
-
-    for (i = 0; token[i]; i += 2) {
-        if (!token[i] || !token[i + 1] ||
-            STREQ(token[i], "") || STREQ(token[i + 1], "")) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("Invalid --with-loader-nvram list: %s"),
-                           list);
-            goto cleanup;
-        }
-    }
-
-    if (i) {
-        if (VIR_ALLOC_N(cfg->loader, i / 2) < 0 ||
-            VIR_ALLOC_N(cfg->nvram, i / 2) < 0)
-            goto cleanup;
-        cfg->nloader = i / 2;
-
-        for (j = 0; j < i / 2; j++) {
-            if (VIR_STRDUP(cfg->loader[j], token[2 * j]) < 0 ||
-                VIR_STRDUP(cfg->nvram[j], token[2 * j + 1]) < 0)
-                goto cleanup;
-        }
-    }
-
-    ret = 0;
- cleanup:
-    virStringFreeList(token);
-    return ret;
-}
-
-
 #define VIR_QEMU_OVMF_LOADER_PATH "/usr/share/OVMF/OVMF_CODE.fd"
 #define VIR_QEMU_OVMF_NVRAM_PATH "/usr/share/OVMF/OVMF_VARS.fd"
 #define VIR_QEMU_AAVMF_LOADER_PATH "/usr/share/AAVMF/AAVMF_CODE.fd"
@@ -327,20 +286,22 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
     cfg->stdioLogD = true;
 
 #ifdef DEFAULT_LOADER_NVRAM
-    if (virQEMUDriverConfigLoaderNVRAMParse(cfg, DEFAULT_LOADER_NVRAM) < 0)
+    if (virFirmwareParseList(DEFAULT_LOADER_NVRAM,
+                             &cfg->firmwares,
+                             &cfg->nfirmwares) < 0)
         goto error;
 
 #else
-
-    if (VIR_ALLOC_N(cfg->loader, 2) < 0 ||
-        VIR_ALLOC_N(cfg->nvram, 2) < 0)
+    if (VIR_ALLOC_N(cfg->firmwares, 2) < 0)
+        goto error;
+    cfg->nfirmwares = 2;
+    if (VIR_ALLOC(cfg->firmwares[0]) < 0 || VIR_ALLOC(cfg->firmwares[1]) < 0)
         goto error;
-    cfg->nloader = 2;
 
-    if (VIR_STRDUP(cfg->loader[0], VIR_QEMU_AAVMF_LOADER_PATH) < 0 ||
-        VIR_STRDUP(cfg->nvram[0], VIR_QEMU_AAVMF_NVRAM_PATH) < 0  ||
-        VIR_STRDUP(cfg->loader[1], VIR_QEMU_OVMF_LOADER_PATH) < 0 ||
-        VIR_STRDUP(cfg->nvram[1], VIR_QEMU_OVMF_NVRAM_PATH) < 0)
+    if (VIR_STRDUP(cfg->firmwares[0]->name, VIR_QEMU_AAVMF_LOADER_PATH) < 0 ||
+        VIR_STRDUP(cfg->firmwares[0]->nvram, VIR_QEMU_AAVMF_NVRAM_PATH) < 0  ||
+        VIR_STRDUP(cfg->firmwares[1]->name, VIR_QEMU_OVMF_LOADER_PATH) < 0 ||
+        VIR_STRDUP(cfg->firmwares[1]->nvram, VIR_QEMU_OVMF_NVRAM_PATH) < 0)
         goto error;
 #endif
 
@@ -397,13 +358,7 @@ static void virQEMUDriverConfigDispose(void *obj)
 
     VIR_FREE(cfg->lockManagerName);
 
-    while (cfg->nloader) {
-        VIR_FREE(cfg->loader[cfg->nloader - 1]);
-        VIR_FREE(cfg->nvram[cfg->nloader - 1]);
-        cfg->nloader--;
-    }
-    VIR_FREE(cfg->loader);
-    VIR_FREE(cfg->nvram);
+    virFirmwareFreeList(cfg->firmwares, cfg->nfirmwares);
 }
 
 
@@ -427,43 +382,6 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
 }
 
 
-static int
-virQEMUDriverConfigNVRAMParse(const char *str,
-                              char **loader,
-                              char **nvram)
-{
-    int ret = -1;
-    char **token;
-
-    if (!(token = virStringSplit(str, ":", 0)))
-        goto cleanup;
-
-    if (token[0]) {
-        virSkipSpaces((const char **) &token[0]);
-        if (token[1])
-            virSkipSpaces((const char **) &token[1]);
-    }
-
-    /* Exactly two tokens are expected */
-    if (!token[0] || !token[1] || token[2] ||
-        STREQ(token[0], "") || STREQ(token[1], "")) {
-        virReportError(VIR_ERR_CONF_SYNTAX,
-                       _("Invalid nvram format: '%s'"),
-                       str);
-        goto cleanup;
-    }
-
-    if (VIR_STRDUP(*loader, token[0]) < 0 ||
-        VIR_STRDUP(*nvram, token[1]) < 0)
-        goto cleanup;
-
-    ret = 0;
- cleanup:
-    virStringFreeList(token);
-    return ret;
-}
-
-
 int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
                                 const char *filename)
 {
@@ -855,14 +773,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
 
         CHECK_TYPE("nvram", VIR_CONF_LIST);
 
-        while (cfg->nloader) {
-            VIR_FREE(cfg->loader[cfg->nloader - 1]);
-            VIR_FREE(cfg->nvram[cfg->nloader - 1]);
-            cfg->nloader--;
-        }
-        VIR_FREE(cfg->loader);
-        VIR_FREE(cfg->nvram);
-
+        virFirmwareFreeList(cfg->firmwares, cfg->nfirmwares);
         /* Calc length and check items */
         for (len = 0, pp = p->list; pp; len++, pp = pp->next) {
             if (pp->type != VIR_CONF_STRING) {
@@ -872,16 +783,14 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
             }
         }
 
-        if (len &&
-            (VIR_ALLOC_N(cfg->loader, len) < 0 ||
-             VIR_ALLOC_N(cfg->nvram, len) < 0))
+        if (len && VIR_ALLOC_N(cfg->firmwares, len) < 0)
             goto cleanup;
-        cfg->nloader = len;
+        cfg->nfirmwares = len;
 
         for (i = 0, pp = p->list; pp; i++, pp = pp->next) {
-            if (virQEMUDriverConfigNVRAMParse(pp->str,
-                                              &cfg->loader[i],
-                                              &cfg->nvram[i]) < 0)
+            if (VIR_ALLOC(cfg->firmwares[i]) < 0)
+                goto cleanup;
+            if (virFirmwareParse(pp->str, cfg->firmwares[i]) < 0)
                 goto cleanup;
         }
     }

src/qemu/qemu_conf.h

@@ -48,6 +48,7 @@
 # include "virclosecallbacks.h"
 # include "virhostdev.h"
 # include "virfile.h"
+# include "virfirmware.h"
 
 # ifdef CPU_SETSIZE /* Linux */
 #  define QEMUD_CPUMASK_LEN CPU_SETSIZE
@@ -178,10 +179,8 @@ struct _virQEMUDriverConfig {
     bool logTimestamp;
     bool stdioLogD;
 
-    /* Pairs of loader:nvram paths. The list is @nloader items long */
-    char **loader;
-    char **nvram;
-    size_t nloader;
+    virFirmwarePtr *firmwares;
+    size_t nfirmwares;
 };
 
 /* Main driver state */

src/qemu/qemu_driver.c

@@ -18534,7 +18534,7 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn,
         goto cleanup;
 
     if (virQEMUCapsFillDomainCaps(domCaps, qemuCaps,
-                                  cfg->loader, cfg->nloader) < 0)
+                                  cfg->firmwares, cfg->nfirmwares) < 0)
         goto cleanup;
 
     ret = virDomainCapsFormat(domCaps);

src/qemu/qemu_process.c

@@ -3724,9 +3724,9 @@ qemuPrepareNVRAM(virQEMUDriverConfigPtr cfg,
     master_nvram_path = loader->templt;
     if (!loader->templt) {
         size_t i;
-        for (i = 0; i < cfg->nloader; i++) {
-            if (STREQ(cfg->loader[i], loader->path)) {
-                master_nvram_path = cfg->nvram[i];
+        for (i = 0; i < cfg->nfirmwares; i++) {
+            if (STREQ(cfg->firmwares[i]->name, loader->path)) {
+                master_nvram_path = cfg->firmwares[i]->nvram;
                 break;
             }
         }

src/util/virfirmware.c

+/*
+ * virfirmware.c: Definition of firmware object and supporting functions
+ *
+ * Copyright (C) 2016 SUSE LINUX Products GmbH, Nuernberg, Germany.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * <http://www.gnu.org/licenses/>.
+ *
+ * Author: Jim Fehlig <jfehlig@suse.com>
+ */
+
+#include <config.h>
+
+#include "viralloc.h"
+#include "virerror.h"
+#include "virfirmware.h"
+#include "virlog.h"
+#include "virstring.h"
+
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+VIR_LOG_INIT("util.firmware");
+
+
+static void
+virFirmwareFree(virFirmwarePtr firmware)
+{
+    if (!firmware)
+        return;
+
+    VIR_FREE(firmware->name);
+    VIR_FREE(firmware->nvram);
+    VIR_FREE(firmware);
+}
+
+
+void
+virFirmwareFreeList(virFirmwarePtr *firmwares, size_t nfirmwares)
+{
+    size_t i;
+
+    for (i = 0; i < nfirmwares; i++)
+        virFirmwareFree(firmwares[i]);
+
+    VIR_FREE(firmwares);
+}
+
+
+int
+virFirmwareParse(const char *str, virFirmwarePtr firmware)
+{
+    int ret = -1;
+    char **token;
+
+    if (!(token = virStringSplit(str, ":", 0)))
+        goto cleanup;
+
+    if (token[0]) {
+        virSkipSpaces((const char **) &token[0]);
+        if (token[1])
+            virSkipSpaces((const char **) &token[1]);
+    }
+
+    /* Exactly two tokens are expected */
+    if (!token[0] || !token[1] || token[2] ||
+        STREQ(token[0], "") || STREQ(token[1], "")) {
+        virReportError(VIR_ERR_CONF_SYNTAX,
+                       _("Invalid nvram format: '%s'"),
+                       str);
+        goto cleanup;
+    }
+
+    if (VIR_STRDUP(firmware->name, token[0]) < 0 ||
+        VIR_STRDUP(firmware->nvram, token[1]) < 0)
+        goto cleanup;
+
+    ret = 0;
+ cleanup:
+    virStringFreeList(token);
+    return ret;
+}
+
+
+int
+virFirmwareParseList(const char *list,
+                     virFirmwarePtr **firmwares,
+                     size_t *nfirmwares)
+{
+    int ret = -1;
+    char **token;
+    size_t i, j;
+
+    if (!(token = virStringSplit(list, ":", 0)))
+        goto cleanup;
+
+    for (i = 0; token[i]; i += 2) {
+        if (!token[i] || !token[i + 1] ||
+            STREQ(token[i], "") || STREQ(token[i + 1], "")) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("Invalid --with-loader-nvram list: %s"),
+                           list);
+            goto cleanup;
+        }
+    }
+
+    if (i) {
+        if (VIR_ALLOC_N(*firmwares, i / 2) < 0)
+            goto cleanup;
+        *nfirmwares = i / 2;
+
+        for (j = 0; j < i / 2; j++) {
+            virFirmwarePtr *fws = *firmwares;
+
+            if (VIR_ALLOC(fws[j]) < 0)
+                goto cleanup;
+            if (VIR_STRDUP(fws[j]->name, token[2 * j]) < 0 ||
+                VIR_STRDUP(fws[j]->nvram, token[2 * j + 1]) < 0)
+                goto cleanup;
+        }
+    }
+
+    ret = 0;
+ cleanup:
+    virStringFreeList(token);
+    return ret;
+}

src/util/virfirmware.h

+/*
+ * virfirmware.h: Declaration of firmware object and supporting functions
+ *
+ * Copyright (C) 2016 SUSE LINUX Products GmbH, Nuernberg, Germany.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * <http://www.gnu.org/licenses/>.
+ *
+ * Author: Jim Fehlig <jfehlig@suse.com>
+ */
+
+#ifndef __VIR_FIRMWARE_H__
+# define __VIR_FIRMWARE_H__
+
+# include "internal.h"
+
+typedef struct _virFirmware virFirmware;
+typedef virFirmware *virFirmwarePtr;
+
+struct _virFirmware {
+    char *name;
+    char *nvram;
+};
+
+
+void
+virFirmwareFreeList(virFirmwarePtr *firmwares, size_t nfirmwares);
+
+int
+virFirmwareParse(const char *str, virFirmwarePtr firmware)
+    ATTRIBUTE_NONNULL(2);
+
+int
+virFirmwareParseList(const char *list,
+                     virFirmwarePtr **firmwares,
+                     size_t *nfirmwares)
+    ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+
+
+#endif /* __VIR_FIRMWARE_H__ */

tests/domaincapstest.c

@@ -128,7 +128,8 @@ fillQemuCaps(virDomainCapsPtr domCaps,
         goto cleanup;
 
     if (virQEMUCapsFillDomainCaps(domCaps, qemuCaps,
-                                  cfg->loader, cfg->nloader) < 0)
+                                  cfg->firmwares,
+                                  cfg->nfirmwares) < 0)
         goto cleanup;
 
     /* The function above tries to query host's KVM & VFIO capabilities by