[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



Il 08/08/2014 12:17, Michal Privoznik ha scritto:
> 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.

The only problem I see with this series is in this patch.

The master _VARS file can be different for different machine types, for
different architectures, and even for different _CODE files.

Perhaps you could introduce a format='...' attribute in <nvram> and use
that to pick the right master file (e.g. a format value could be
uefi128k)?  I don't like it, but I cannot think of anything else.

Or just leave out this patch for now...

Paolo

> Signed-off-by: Michal Privoznik <mprivozn redhat com>
> ---
>  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" }
> 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]