On Wed, Jan 21, 2015 at 08:21:28PM +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@xxxxxxxxxx>
---
configure.ac | 12 ++++++++++++
src/qemu/qemu_conf.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
tests/domaincapstest.c | 15 ++++++++-------
3 files changed, 70 insertions(+), 7 deletions(-)
diff --git a/configure.ac b/configure.ac
index f370475..7153fa5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2789,6 +2789,18 @@ 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=""
+ 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..3e51b49 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -107,6 +107,49 @@ 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;
+
+ if (!(token = virStringSplit(list, ":", 0)))
+ goto cleanup;
+
+ for (i = 0; token[i]; i += 2) {
+ if (!token[i] || !token[i + 1] ||
Double check for "token[i]"; it'd be easier to do:
for (i = 0; token[i] && token[i + 1]; i += 2) {
...
}
+ STREQ(token[i], "") || STREQ(token[i + 1], "")) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Invalid --with-loader-nvram list: %s"),
+ list);
+ goto cleanup;
But erroring out here is maybe even more ugly than that bash parsing in configure.ac :( There really is no other option to check this built-time? Can we at least do something like: l="$(echo "$with_loader_nvram" | tr ':' '\n' | wc -l)" test $((l % 2)) -eq 0 || echo ERROR this only checks that there's even number of parameters when split by ':', but it works in dash (and csh should be fine too, but I haven't tested that). And the only thing you were checking was the number of parameters, so this has the same meaning and it's done compile-time.
+ }
+ }
+
+ if (i) {
+ if (VIR_ALLOC_N(cfg->loader, (i + 1) / 2) < 0 ||
+ VIR_ALLOC_N(cfg->nvram, (i + 1) / 2) < 0)
+ goto cleanup;
+ cfg->nloader = (i + 1) / 2;
+
+ while (i) {
+ if (VIR_STRDUP(cfg->loader[(i - 1) / 2 ], token[i - 2]) < 0 ||
+ VIR_STRDUP(cfg->nvram[(i - 1) / 2], token[i - 1]) < 0)
You are not checking whether token[i] is '\0', but I don't know whether that breaks later in the code or not. Anyway, if something like this happens, I think working around that and just printing a warning would be better. You can behave like if the file does not exist -- we don't quit the daemon/driver in that case, do we?
Attachment:
pgpooBnFxe7lf.pgp
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list