FYI, there seems to be a problem withint-mx.corp.redhat.com. It failed to post 0/6 and 3/6 with
550 5.1.1 <libvir-list@redhat.com>... User unknown (in reply to RCPT TO command) I'm rather reluctant to spam the list by resending the series. Regards, Jim On 06/08/2016 05:23 PM, Jim Fehlig wrote: > 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: Jim Fehlig <jfeh...@suse.com> > --- > po/POTFILES.in | 1 + > src/Makefile.am | 1 + > src/libvirt_private.syms | 6 ++ > src/qemu/qemu_capabilities.c | 22 +++---- > src/qemu/qemu_capabilities.h | 5 +- > src/qemu/qemu_conf.c | 127 ++++++--------------------------------- > src/qemu/qemu_conf.h | 7 +-- > src/qemu/qemu_driver.c | 2 +- > src/qemu/qemu_process.c | 6 +- > src/util/virfirmware.c | 137 > +++++++++++++++++++++++++++++++++++++++++++ > src/util/virfirmware.h | 51 ++++++++++++++++ > tests/domaincapstest.c | 3 +- > 12 files changed, 237 insertions(+), 131 deletions(-) > > diff --git a/po/POTFILES.in b/po/POTFILES.in > index 2a6fae4..85adaf3 100644 > --- a/po/POTFILES.in > +++ b/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/virhostdev.c > diff --git a/src/Makefile.am b/src/Makefile.am > index 8c83b0c..9c4eb41 100644 > --- a/src/Makefile.am > +++ b/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 \ > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms > index 27ad7ff..bed4e11 100644 > --- a/src/libvirt_private.syms > +++ b/src/libvirt_private.syms > @@ -1561,6 +1561,12 @@ virFirewallStartRollback; > virFirewallStartTransaction; > > > +# util/virfirmware.h > +virFirmwareFreeList; > +virFirmwareParse; > +virFirmwareParseList; > + > + > # util/virgettext.h > virGettextInitialize; > > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c > index 43ac906..cdb3b6c 100644 > --- a/src/qemu/qemu_capabilities.c > +++ b/src/qemu/qemu_capabilities.c > @@ -4069,18 +4069,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); > @@ -4109,13 +4109,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; > } > @@ -4328,8 +4328,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; > @@ -4340,7 +4340,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 || > diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h > index 77e4b98..39c642c 100644 > --- a/src/qemu/qemu_capabilities.h > +++ b/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 > @@ -490,7 +491,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__*/ > diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c > index e00ddca..030bd5a 100644 > --- a/src/qemu/qemu_conf.c > +++ b/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) > { > @@ -854,14 +772,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) { > @@ -871,16 +782,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; > } > } > diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h > index 1fdef70..469b1dc 100644 > --- a/src/qemu/qemu_conf.h > +++ b/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 > @@ -177,10 +178,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 */ > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index e70d3ce..45f6a82 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -18511,7 +18511,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); > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c > index 397dac7..0b08df5 100644 > --- a/src/qemu/qemu_process.c > +++ b/src/qemu/qemu_process.c > @@ -3727,9 +3727,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; > } > } > diff --git a/src/util/virfirmware.c b/src/util/virfirmware.c > new file mode 100644 > index 0000000..6b20c06 > --- /dev/null > +++ b/src/util/virfirmware.c > @@ -0,0 +1,137 @@ > +/* > + * 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 <jfeh...@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; > +} > diff --git a/src/util/virfirmware.h b/src/util/virfirmware.h > new file mode 100644 > index 0000000..682a865 > --- /dev/null > +++ b/src/util/virfirmware.h > @@ -0,0 +1,51 @@ > +/* > + * 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 <jfeh...@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__ */ > diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c > index e1d0671..83ce0e5 100644 > --- a/tests/domaincapstest.c > +++ b/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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list