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

[Laszlo Ersek]

one question

On 08/22/14 14:08, Michal Privoznik wrote:
> 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.
> 
> Temporary, on per domain basis the location of master NVRAM file can
> be overridden by this @template attribute I'm inventing to the
> <nvram/> element. All it does is holding path to the master NVRAM file
> from which local copy is created. If that's the case, the map in
> qemu.conf is not consulted.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  docs/formatdomain.html.in                          |  11 +-
>  docs/schemas/domaincommon.rng                      |   9 +-
>  libvirt.spec.in                                    |   2 +
>  src/Makefile.am                                    |   1 +
>  src/conf/domain_conf.c                             |  11 +-
>  src/conf/domain_conf.h                             |   1 +
>  src/qemu/libvirtd_qemu.aug                         |   3 +
>  src/qemu/qemu.conf                                 |  14 +++
>  src/qemu/qemu_conf.c                               |  94 ++++++++++++++
>  src/qemu/qemu_conf.h                               |   5 +
>  src/qemu/qemu_process.c                            | 137 +++++++++++++++++++++
>  src/qemu/test_libvirtd_qemu.aug.in                 |   3 +
>  tests/domainschemadata/domain-bios-nvram-empty.xml |  40 ++++++
>  13 files changed, 325 insertions(+), 6 deletions(-)
>  create mode 100644 tests/domainschemadata/domain-bios-nvram-empty.xml


> @@ -17682,7 +17684,14 @@ virDomainLoaderDefFormat(virBufferPtr buf,
>      virBufferAsprintf(buf, " type='%s'>", type);
>  
>      virBufferEscapeString(buf, "%s</loader>\n", loader->path);
> -    virBufferEscapeString(buf, "<nvram>%s</nvram>\n", loader->nvram);
> +    if (loader->nvram || loader->templt) {
> +        virBufferAddLit(buf, "<nvram");
> +        virBufferEscapeString(buf, " template='%s'", loader->templt);

Shouldn't you protect this virBufferEscapeString() call with an "if" too?

> +        if (loader->nvram)
> +            virBufferEscapeString(buf, ">%s</nvram>\n", loader->nvram);
> +        else
> +            virBufferAddLit(buf, "/>\n");
> +    }
>  }

Looks good otherwise.

Thanks
Laszlo