On 11/11/19 9:42 AM, Michal Privoznik wrote: > There are two ways for specifying loader:nvram pairs: > > 1) --with-loader-nvram configure option > 2) nvram variable in qemu.conf > > Since we have FW descriptors, using this old style is > discouraged, but not as strong as one would expect. Produce more > warnings: Oh, I didn't know the old style was discouraged when using FW descriptors. Thanks for mentioning it! > > 1) produce a warning if somebody tries the configure option > 2) produce a warning if somebody sets nvram variable and at > least on FW descriptor was found > > The reason for producing warning in case 1) is that package > maintainers, who set the configure option in the first place > should start moving towards FW descriptors and abandon the > configure option. After all, the warning is printed into config > output only in this case. Should the configure option be removed from the upstream spec file? Regards, Jim > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1763477 > > Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> > --- > m4/virt-loader-nvram.m4 | 10 +++++++++- > src/qemu/qemu.conf | 3 +++ > src/qemu/qemu_conf.c | 19 +++++++++++++++++-- > 3 files changed, 29 insertions(+), 3 deletions(-) > > diff --git a/m4/virt-loader-nvram.m4 b/m4/virt-loader-nvram.m4 > index d7e0c8ca18..ed2ae0cf27 100644 > --- a/m4/virt-loader-nvram.m4 > +++ b/m4/virt-loader-nvram.m4 > @@ -30,6 +30,8 @@ AC_DEFUN([LIBVIRT_CHECK_LOADER_NVRAM], [ > l=$(echo $with_loader_nvram | tr ':' '\n' | wc -l) > if test $(expr $l % 2) -ne 0 ; then > AC_MSG_ERROR([Malformed --with-loader-nvram argument]) > + elif test $l -gt 0 ; then > + AC_MSG_WARN([Note that --with-loader-nvram is obsolete and will be removed soon]) > fi > AC_DEFINE_UNQUOTED([DEFAULT_LOADER_NVRAM], ["$with_loader_nvram"], > [List of loader:nvram pairs]) > @@ -37,5 +39,11 @@ AC_DEFUN([LIBVIRT_CHECK_LOADER_NVRAM], [ > ]) > > AC_DEFUN([LIBVIRT_RESULT_LOADER_NVRAM], [ > - LIBVIRT_RESULT([Loader/NVRAM], [$with_loader_nvram]) > + if test "x$with_loader_nvram" != "xno" && \ > + test "x$with_loader_nvram" != "x" ; then > + LIBVIRT_RESULT([Loader/NVRAM], [$with_loader_nvram], > + [!!! Using this configure option is strongly discouraged !!!]) > + else > + LIBVIRT_RESULT([Loader/NVRAM], [$with_loader_nvram]) > + fi > ]) > diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf > index b3a3428e4c..7a056b037e 100644 > --- a/src/qemu/qemu.conf > +++ b/src/qemu/qemu.conf > @@ -761,6 +761,9 @@ > # source tree. These metadata files are distributed alongside any > # firmware images intended for use with QEMU. > # > +# NOTE: if ANY firmware metadata files are detected, this setting > +# will be COMPLETELY IGNORED. > +# > # ------------------------------------------ > # > # When a domain is configured to use UEFI instead of standard > diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c > index fae697a2ef..293f2635cc 100644 > --- a/src/qemu/qemu_conf.c > +++ b/src/qemu/qemu_conf.c > @@ -32,6 +32,7 @@ > #include "qemu_conf.h" > #include "qemu_capabilities.h" > #include "qemu_domain.h" > +#include "qemu_firmware.h" > #include "qemu_security.h" > #include "viruuid.h" > #include "virbuffer.h" > @@ -799,7 +800,8 @@ virQEMUDriverConfigLoadLogEntry(virQEMUDriverConfigPtr cfg, > > static int > virQEMUDriverConfigLoadNVRAMEntry(virQEMUDriverConfigPtr cfg, > - virConfPtr conf) > + virConfPtr conf, > + bool privileged) > { > VIR_AUTOSTRINGLIST nvram = NULL; > size_t i; > @@ -807,8 +809,21 @@ virQEMUDriverConfigLoadNVRAMEntry(virQEMUDriverConfigPtr cfg, > if (virConfGetValueStringList(conf, "nvram", false, &nvram) < 0) > return -1; > if (nvram) { > + VIR_AUTOSTRINGLIST fwList = NULL; > + > virFirmwareFreeList(cfg->firmwares, cfg->nfirmwares); > > + if (qemuFirmwareFetchConfigs(&fwList, privileged) < 0) > + return -1; > + > + if (fwList) { > + VIR_WARN("Obsolete nvram variable is set while firmware metadata " > + "files found. Note that the nvram config file variable is " > + "going to be ignored."); > + cfg->nfirmwares = 0; > + return 0; > + } > + > cfg->nfirmwares = virStringListLength((const char *const *)nvram); > if (nvram[0] && VIR_ALLOC_N(cfg->firmwares, cfg->nfirmwares) < 0) > return -1; > @@ -1041,7 +1056,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, > if (virQEMUDriverConfigLoadLogEntry(cfg, conf) < 0) > return -1; > > - if (virQEMUDriverConfigLoadNVRAMEntry(cfg, conf) < 0) > + if (virQEMUDriverConfigLoadNVRAMEntry(cfg, conf, privileged) < 0) > return -1; > > if (virQEMUDriverConfigLoadGlusterDebugEntry(cfg, conf) < 0) > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
