[libvirt] [PATCH v5 0/2] Allow UEFI paths to be specified at compile time

diff to v4: - added check for list consistence to configure too - slightly reformatted C parsing code 2/2 has been ACKed meanwhile, so sending it just for completeness. Michal Privoznik (2): qemu: Allow UEFI paths to be specified at compile time qemu: Add AAVMF to the list of known UEFIs configure.ac | 17 ++++++ src/qemu/qemu.conf | 12 +++- src/qemu/qemu_conf.c | 67 +++++++++++++++++++--- src/qemu/test_libvirtd_qemu.aug.in | 1 + .../domaincaps-qemu_1.6.50-1.xml | 1 + tests/domaincapstest.c | 16 +++--- 6 files changed, 97 insertions(+), 17 deletions(-) -- 2.0.5

Up until now there are just two ways how to specify UEFI paths to libvirt. The first one is editing qemu.conf, the other is editing qemu_conf.c and recompile which is not that fancy. So, new configure option is introduced: --with-loader-nvram which takes a list of pairs of UEFI firmware and NVRAM store. This way, the compiled in defaults can be passed during compile time without need to change the code itself. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- configure.ac | 17 +++++++++++++++++ src/qemu/qemu_conf.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ tests/domaincapstest.c | 15 ++++++++------- 3 files changed, 74 insertions(+), 7 deletions(-) diff --git a/configure.ac b/configure.ac index f370475..b121777 100644 --- a/configure.ac +++ b/configure.ac @@ -2789,6 +2789,23 @@ AC_ARG_WITH([default-editor], [DEFAULT_EDITOR=vi]) AC_DEFINE_UNQUOTED([DEFAULT_EDITOR], ["$DEFAULT_EDITOR"], [Default editor to use]) +AC_ARG_WITH([loader-nvram], + [AS_HELP_STRING([--with-loader-nvram], + [Pass list of pairs of <loader>:<nvram> paths. Both + pairs and list items are separated by a colon. + @<:default=paths to OVMF and its clones@:>@])], + [if test "$withval" = "no"; then + withval="" + else + l=`echo $withval | tr ':' '\n' | wc -l` + if test "$((l % 2))" -ne 0; then + AC_MSG_ERROR([Malformed --with-loader-nvram argument]) + fi + fi + AC_DEFINE_UNQUOTED([DEFAULT_LOADER_NVRAM], + ["$withval"], + [List of laoder:nvram pairs])]) + # Some GNULIB base64 symbols clash with a kerberos library AC_DEFINE_UNQUOTED([isbase64],[libvirt_gl_isbase64],[Hack to avoid symbol clash]) AC_DEFINE_UNQUOTED([base64_encode],[libvirt_gl_base64_encode],[Hack to avoid symbol clash]) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index a24c5c5..a6183b9 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -107,6 +107,48 @@ void qemuDomainCmdlineDefFree(qemuDomainCmdlineDefPtr def) VIR_FREE(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_LOADER_FILE_PATH "/usr/share/OVMF/OVMF_CODE.fd" #define VIR_QEMU_NVRAM_FILE_PATH "/usr/share/OVMF/OVMF_VARS.fd" @@ -258,6 +300,12 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) cfg->logTimestamp = true; +#ifdef DEFAULT_LOADER_NVRAM + if (virQEMUDriverConfigLoaderNVRAMParse(cfg, DEFAULT_LOADER_NVRAM) < 0) + goto error; + +#else + if (VIR_ALLOC_N(cfg->loader, 1) < 0 || VIR_ALLOC_N(cfg->nvram, 1) < 0) goto error; @@ -266,6 +314,7 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) if (VIR_STRDUP(cfg->loader[0], VIR_QEMU_LOADER_FILE_PATH) < 0 || VIR_STRDUP(cfg->nvram[0], VIR_QEMU_NVRAM_FILE_PATH) < 0) goto error; +#endif return cfg; diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c index 70d2ef3..5ec03a4 100644 --- a/tests/domaincapstest.c +++ b/tests/domaincapstest.c @@ -105,6 +105,7 @@ fillQemuCaps(virDomainCapsPtr domCaps, struct fillQemuCapsData *data = (struct fillQemuCapsData *) opaque; virQEMUCapsPtr qemuCaps = data->qemuCaps; virQEMUDriverConfigPtr cfg = data->cfg; + virDomainCapsLoaderPtr loader = &domCaps->os.loader; if (virQEMUCapsFillDomainCaps(domCaps, qemuCaps, cfg->loader, cfg->nloader) < 0) @@ -122,14 +123,14 @@ fillQemuCaps(virDomainCapsPtr domCaps, /* Moreover, as of f05b6a918e28 we are expecting to see * OVMF_CODE.fd file which may not exists everywhere. */ - if (!domCaps->os.loader.values.nvalues) { - virDomainCapsLoaderPtr loader = &domCaps->os.loader; + while (loader->values.nvalues) + VIR_FREE(loader->values.values[--loader->values.nvalues]); + + if (fillStringValues(&loader->values, + "/usr/share/OVMF/OVMF_CODE.fd", + NULL) < 0) + return -1; - if (fillStringValues(&loader->values, - "/usr/share/OVMF/OVMF_CODE.fd", - NULL) < 0) - return -1; - } return 0; } #endif /* WITH_QEMU */ -- 2.0.5

On Mon, Jan 26, 2015 at 03:38:04PM +0100, Michal Privoznik wrote:
Up until now there are just two ways how to specify UEFI paths to libvirt. The first one is editing qemu.conf, the other is editing qemu_conf.c and recompile which is not that fancy. So, new configure option is introduced: --with-loader-nvram which takes a list of pairs of UEFI firmware and NVRAM store. This way, the compiled in defaults can be passed during compile time without need to change the code itself.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- configure.ac | 17 +++++++++++++++++ src/qemu/qemu_conf.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ tests/domaincapstest.c | 15 ++++++++------- 3 files changed, 74 insertions(+), 7 deletions(-)
diff --git a/configure.ac b/configure.ac index f370475..b121777 100644 --- a/configure.ac +++ b/configure.ac @@ -2789,6 +2789,23 @@ AC_ARG_WITH([default-editor], [DEFAULT_EDITOR=vi]) AC_DEFINE_UNQUOTED([DEFAULT_EDITOR], ["$DEFAULT_EDITOR"], [Default editor to use])
+AC_ARG_WITH([loader-nvram], + [AS_HELP_STRING([--with-loader-nvram], + [Pass list of pairs of <loader>:<nvram> paths. Both + pairs and list items are separated by a colon. + @<:default=paths to OVMF and its clones@:>@])], + [if test "$withval" = "no"; then + withval="" + else + l=`echo $withval | tr ':' '\n' | wc -l` + if test "$((l % 2))" -ne 0; then
Sorry for the confusion I caused to you, but I'd rather you use "`expr $l % 2`". ACK with that changed, my ACK to 2/2 still stands.

Well, even though users can pass the list of UEFI:NVRAM pairs at the configure time, we may maintain the list of widely available UEFI ourselves too. And as arm64 begin to rises, OVMF was ported there too. With a slight name change - it's called AAVMF, with AAVMF_CODE.fd being the UEFI firmware and AAVMF_VARS.fd being the NVRAM store file. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu.conf | 12 +++++++++--- src/qemu/qemu_conf.c | 18 +++++++++++------- src/qemu/test_libvirtd_qemu.aug.in | 1 + .../domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml | 1 + tests/domaincapstest.c | 1 + 5 files changed, 23 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index c6db568..1c589a2 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -506,6 +506,12 @@ # however, have different variables store. Therefore the nvram is # a list of strings when a single item is in form of: # ${PATH_TO_UEFI_FW}:${PATH_TO_UEFI_VARS}. -# Later, when libvirt creates per domain variable store, this -# list is searched for the master image. -#nvram = [ "/usr/share/OVMF/OVMF_CODE.fd:/usr/share/OVMF/OVMF_VARS.fd" ] +# Later, when libvirt creates per domain variable store, this list is +# searched for the master image. The UEFI firmware can be called +# differently for different guest architectures. For instance, it's OVMF +# for x86_64 and i686, but it's AAVMF for aarch64. The libvirt default +# follows this scheme. +#nvram = [ +# "/usr/share/OVMF/OVMF_CODE.fd:/usr/share/OVMF/OVMF_VARS.fd", +# "/usr/share/AAVMF/AAVMF_CODE.fd:/usr/share/AAVMF/AAVMF_VARS.fd" +#] diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index a6183b9..c6b083c 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -149,8 +149,10 @@ virQEMUDriverConfigLoaderNVRAMParse(virQEMUDriverConfigPtr cfg, } -#define VIR_QEMU_LOADER_FILE_PATH "/usr/share/OVMF/OVMF_CODE.fd" -#define VIR_QEMU_NVRAM_FILE_PATH "/usr/share/OVMF/OVMF_VARS.fd" +#define VIR_QEMU_OVMF_LOADER_PATH "/usr/share/AAVMF/AAVMF_CODE.fd" +#define VIR_QEMU_OVMF_NVRAM_PATH "/usr/share/AAVMF/AAVMF_VARS.fd" +#define VIR_QEMU_AAVMF_LOADER_PATH "/usr/share/OVMF/OVMF_CODE.fd" +#define VIR_QEMU_AAVMF_NVRAM_PATH "/usr/share/OVMF/OVMF_VARS.fd" virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) { @@ -306,13 +308,15 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) #else - if (VIR_ALLOC_N(cfg->loader, 1) < 0 || - VIR_ALLOC_N(cfg->nvram, 1) < 0) + if (VIR_ALLOC_N(cfg->loader, 2) < 0 || + VIR_ALLOC_N(cfg->nvram, 2) < 0) goto error; - cfg->nloader = 1; + cfg->nloader = 2; - if (VIR_STRDUP(cfg->loader[0], VIR_QEMU_LOADER_FILE_PATH) < 0 || - VIR_STRDUP(cfg->nvram[0], VIR_QEMU_NVRAM_FILE_PATH) < 0) + if (VIR_STRDUP(cfg->loader[0], VIR_QEMU_OVMF_LOADER_PATH) < 0 || + VIR_STRDUP(cfg->nvram[0], VIR_QEMU_OVMF_NVRAM_PATH) < 0 || + VIR_STRDUP(cfg->loader[1], VIR_QEMU_AAVMF_LOADER_PATH) < 0 || + VIR_STRDUP(cfg->nvram[1], VIR_QEMU_AAVMF_NVRAM_PATH) < 0) goto error; #endif diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index 30fd27e..fc4935b 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -76,4 +76,5 @@ module Test_libvirtd_qemu = { "log_timestamp" = "0" } { "nvram" { "1" = "/usr/share/OVMF/OVMF_CODE.fd:/usr/share/OVMF/OVMF_VARS.fd" } + { "2" = "/usr/share/AAVMF/AAVMF_CODE.fd:/usr/share/AAVMF/AAVMF_VARS.fd" } } diff --git a/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml b/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml index 346ef65..37d2102 100644 --- a/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml +++ b/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml @@ -5,6 +5,7 @@ <arch>x86_64</arch> <os supported='yes'> <loader supported='yes'> + <value>/usr/share/AAVMF/AAVMF_CODE.fd</value> <value>/usr/share/OVMF/OVMF_CODE.fd</value> <enum name='type'> <value>rom</value> diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c index 5ec03a4..de417b9 100644 --- a/tests/domaincapstest.c +++ b/tests/domaincapstest.c @@ -127,6 +127,7 @@ fillQemuCaps(virDomainCapsPtr domCaps, VIR_FREE(loader->values.values[--loader->values.nvalues]); if (fillStringValues(&loader->values, + "/usr/share/AAVMF/AAVMF_CODE.fd", "/usr/share/OVMF/OVMF_CODE.fd", NULL) < 0) return -1; -- 2.0.5
participants (2)
-
Martin Kletzander
-
Michal Privoznik