Re: [libvirt] [PATCH v1 3/3] qemu: Automatically create NVRAM store

When using split UEFI image, it may come handy if libvirt manages per domain _VARS file automatically. While the _CODE file is RO and can be shared among multiple domains, you certainly don't want to do that on the _VARS file. This latter one needs to be per domain. So at the domain startup process, if it's determined that domain needs _VARS file it's copied from this master _VARS file. The location of the master file is configurable in qemu.conf and the default path can be compiled in via --with-qemu-nvram-file configure option.

Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- configure.ac | 27 ++++++++ libvirt.spec.in | 2 + src/Makefile.am | 1 + src/qemu/libvirtd_qemu.aug | 3 + src/qemu/qemu.conf | 9 +++ src/qemu/qemu_conf.c | 8 +++ src/qemu/qemu_conf.h | 3 + src/qemu/qemu_process.c | 125 +++++++++++++++++++++++++++++++++++++ src/qemu/test_libvirtd_qemu.aug.in | 1 + 9 files changed, 179 insertions(+) diff --git a/configure.ac b/configure.ac index 9dd07d2..16ca266 100644 --- a/configure.ac +++ b/configure.ac @@ -569,6 +569,11 @@ AC_ARG_WITH([chrdev-lock-files], [location for UUCP style lock files for character devices (use auto for default paths on some platforms) @<:@default=auto@:>@])]) m4_divert_text([DEFAULTS], [with_chrdev_lock_files=auto]) +AC_ARG_WITH([qemu-nvram-file], + [AS_HELP_STRING([--with-qemu-nvram-file], + [location of OVMF_VARS.fd file + (use auto for default path on some platforms) @<:@default=auto@:>@])]) +m4_divert_text([DEFAULTS], [with_qemu_nvram_file=auto]) AC_ARG_WITH([pm-utils], [AS_HELP_STRING([--with-pm-utils], [use pm-utils for power management @<:@default=yes@:>@])]) @@ -1418,6 +1423,27 @@ platform]) fi AM_CONDITIONAL([VIR_CHRDEV_LOCK_FILE_PATH], [test "$with_chrdev_lock_files" != "no"]) +dnl OVMF_VARS.fd file +if test "$with_qemu_nvram_file" != "no"; then + case $with_qemu_nvram_file in + yes | auto) + dnl Default locations for platforms, or disable if unknown + if test "$with_linux" = "yes"; then + with_qemu_nvram_file=/usr/share/OVMF/OVMF_VARS.fd + elif test "$with_chrdev_lock_files" = "auto"; then + with_qemu_nvram_file=no + fi ;; + esac + if test "$with_qemu_nvram_file" = "yes"; then + AC_MSG_ERROR([You must specify path for the lock files on this +platform]) + fi + if test "$with_qemu_nvram_file" != "no"; then + AC_DEFINE_UNQUOTED([VIR_QEMU_NVRAM_FILE_PATH], "$with_qemu_nvram_file", + [default path to OVMF_VARS.fd file]) + fi +fi +AM_CONDITIONAL([VIR_QEMU_NVRAM_FILE_PATH], [test "$with_qemu_nvram_file" != "no"]) AC_ARG_WITH([secdriver-selinux], [AS_HELP_STRING([--with-secdriver-selinux], @@ -2925,6 +2951,7 @@ AC_MSG_NOTICE([ numad: $with_numad]) AC_MSG_NOTICE([ XML Catalog: $XML_CATALOG_FILE]) AC_MSG_NOTICE([ Init script: $with_init_script]) AC_MSG_NOTICE([Char device locks: $with_chrdev_lock_files]) +AC_MSG_NOTICE([ OVMF_VARS.fd loc: $with_qemu_nvram_file]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([Developer Tools]) AC_MSG_NOTICE([]) diff --git a/libvirt.spec.in b/libvirt.spec.in index 29da071..486fc66 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1947,6 +1947,7 @@ exit 0 %dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/ %dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/channel/ %dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/channel/target/ +%dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/nvram/ %dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/cache/libvirt/qemu/ %{_datadir}/augeas/lenses/libvirtd_qemu.aug %{_datadir}/augeas/lenses/tests/test_libvirtd_qemu.aug @@ -2049,6 +2050,7 @@ exit 0 %dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/ %dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/channel/ %dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/channel/target/ +%dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/nvram/ %dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/cache/libvirt/qemu/ %{_datadir}/augeas/lenses/libvirtd_qemu.aug %{_datadir}/augeas/lenses/tests/test_libvirtd_qemu.aug diff --git a/src/Makefile.am b/src/Makefile.am index 982f63d..9bd38f5 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -2632,6 +2632,7 @@ endif WITH_SANLOCK if WITH_QEMU $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/qemu" $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/qemu/channel/target" + $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/qemu/nvram" $(MKDIR_P) "$(DESTDIR)$(localstatedir)/run/libvirt/qemu" $(MKDIR_P) "$(DESTDIR)$(localstatedir)/cache/libvirt/qemu" $(MKDIR_P) "$(DESTDIR)$(localstatedir)/log/libvirt/qemu" diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index e7db7fe..3d9db99 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -88,6 +88,8 @@ module Libvirtd_qemu = let log_entry = bool_entry "log_timestamp" + let nvram_entry = str_entry "nvram" + (* Each entry in the config is one of the following ... *) let entry = vnc_entry | spice_entry @@ -100,6 +102,7 @@ module Libvirtd_qemu = | rpc_entry | network_entry | log_entry + | nvram_entry let comment = [ label "#comment" . del /#[ \t]*/ "# " . store /([^ \t\n][^\n]*)?/ . del /\n/ "\n" ] let empty = [ label "#empty" . eol ] diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 7bbbe09..a0c9485 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -487,3 +487,12 @@ # Defaults to 1. # #log_timestamp = 0 + + +# Location of master nvram file +# +# When a domain is configured to use UEFI instead of standard +# BIOS it may use a separate storage for UEFI variables. If +# that's the case libvirt creates the variable store per domain +# using this master file as image. +#nvram = "/usr/share/OVMF/OVMF_VARS.fd" diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 6bfa48e..3f6d365 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -255,6 +255,11 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) cfg->logTimestamp = true; +#ifdef VIR_QEMU_NVRAM_FILE_PATH + if (VIR_STRDUP(cfg->nvram, VIR_QEMU_NVRAM_FILE_PATH) < 0) + goto error; +#endif + return cfg; error: @@ -305,6 +310,7 @@ static void virQEMUDriverConfigDispose(void *obj) virStringFreeList(cfg->securityDriverNames); VIR_FREE(cfg->lockManagerName); + VIR_FREE(cfg->nvram); } @@ -654,6 +660,8 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, GET_VALUE_BOOL("log_timestamp", cfg->logTimestamp); + GET_VALUE_STR("nvram", cfg->nvram); + ret = 0; cleanup: diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 90aebef..0bb5333 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -172,6 +172,9 @@ struct _virQEMUDriverConfig { int migrationPortMax; bool logTimestamp; + + char *nvram; /* path to master NVRAM file from which + copies per domain are derived */ }; /* Main driver state */ diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 407da5e..2cb693a 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -66,6 +66,7 @@ #include "virnuma.h" #include "virstring.h" #include "virhostdev.h" +#include "configmake.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -3714,6 +3715,123 @@ qemuProcessVerifyGuestCPU(virQEMUDriverPtr driver, virDomainObjPtr vm) } +static int +qemuPrepareNVRAM(virQEMUDriverConfigPtr cfg, + virDomainDefPtr def, + bool migrated) +{ + int ret = -1; + int srcFD = -1; + int dstFD = -1; + virDomainLoaderDefPtr loader = def->os.loader; + bool generated = false; + + /* Unless domain has RO loader of pflash type, we have + * nothing to do here. If the loader is RW then it's not + * using split code and vars feature, so no nvram file needs + * to be created. */ + if (!loader || loader->type != VIR_DOMAIN_LOADER_TYPE_PFLASH || + loader->readonly != VIR_TRISTATE_SWITCH_ON) + return 0; + + /* If the nvram path is configured already, there's nothing + * we need to do. Unless we are starting the destination side + * of migration in which case nvram is configured in the + * domain XML but the file doesn't exist yet. Moreover, after + * the migration is completed, qemu will invoke a + * synchronization write into the nvram file so we don't have + * to take care about transmitting the real data on the other + * side. */ + if (loader->nvram && !migrated) + return 0; + + /* Autogenerate nvram path if needed.*/ + if (!loader->nvram) { + if (virAsprintf(&loader->nvram, + "%s/lib/libvirt/qemu/nvram/%s_VARS.fd", + LOCALSTATEDIR, def->name) < 0) + goto cleanup; + + generated = true; + } + + if (!virFileExists(loader->nvram)) { + ssize_t r = -1; + + if (!cfg->nvram) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("master NVRAM file is not configured " + "or turned off by administrator")); + goto cleanup; + } + + if ((srcFD = virFileOpenAs(cfg->nvram, O_RDONLY, + 0, -1, -1, 0)) < 0) { + virReportSystemError(-srcFD, + _("Failed to open file '%s'"), + cfg->nvram); + goto cleanup; + } + if ((dstFD = virFileOpenAs(loader->nvram, + O_WRONLY | O_CREAT | O_EXCL, + S_IRUSR | S_IWUSR, + cfg->user, cfg->group, 0)) < 0) { + virReportSystemError(-dstFD, + _("Failed to create file '%s'"), + loader->nvram); + goto cleanup; + } + + while (r) { + char buf[1024]; + ssize_t w = 0; + + if ((r = saferead(srcFD, buf, sizeof(buf))) < 0) { + virReportSystemError(errno, + _("Unable to read from file '%s'"), + cfg->nvram); + goto cleanup; + } + + do { + ssize_t actW; + if ((actW = safewrite(dstFD, buf + w, r - w)) < 0) { + virReportSystemError(errno, + _("Unable to write to file '%s'"), + loader->nvram); + goto cleanup; + } + w += actW; + } while (w != r); + } + + if (VIR_CLOSE(srcFD) < 0) { + virReportSystemError(errno, + _("Unable to close file '%s'"), + cfg->nvram); + goto cleanup; + } + if (VIR_CLOSE(dstFD) < 0) { + virReportSystemError(errno, + _("Unable to close file '%s'"), + loader->nvram); + goto cleanup; + } + } + + ret = 0; + cleanup: + /* We successfully generated the nvram path, but failed to + * copy the file content. Roll back. */ + if (ret < 0 && generated) + VIR_FREE(loader->nvram); + + VIR_FORCE_CLOSE(srcFD); + VIR_FORCE_CLOSE(dstFD); + return ret; +} + + int qemuProcessStart(virConnectPtr conn, virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -3781,6 +3899,13 @@ int qemuProcessStart(virConnectPtr conn, if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; + /* Some things, paths, ... are generated here and we want them to persist. + * Fill them in prior to setting the domain def as transient. */ + VIR_DEBUG("Generating paths"); + + if (qemuPrepareNVRAM(cfg, vm->def, migrateFrom) < 0) + goto cleanup; + /* Do this upfront, so any part of the startup process can add * runtime state to vm->def that won't be persisted. This let's us * report implicit runtime defaults in the XML, like vnc listen/socket diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index 7796acc..1ecb690 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -74,3 +74,4 @@ module Test_libvirtd_qemu = { "migration_port_min" = "49152" } { "migration_port_max" = "49215" } { "log_timestamp" = "0" } +{ "nvram" = "/usr/share/OVMF/OVMF_VARS.fd" }