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

On 22.08.2014 14:26, Laszlo Ersek wrote: > 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(a)redhat.com&gt; >> --- >> 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? No, virBufferEscapeString() works slightly different to other virBuffer* APIs. If any of its 3 arguments is NULL is basically NOP. So if loader->templt (sigh, I couldn't use template as it's c++ keyword, but that's different story) is NULL (= it wasn't specified in XML) this line turns into NOP and doesn't output anything.

> >> + if (loader->nvram) >> + virBufferEscapeString(buf, ">%s</nvram>\n", loader->nvram); >> + else >> + virBufferAddLit(buf, "/>\n"); >> + } You were confused by this, weren't you? This is basically to distinguish cases if nvram path was specified or not. If it was, we need to print out <nvram>/some/path</nvram> otherwise the partially written nvram element needs to be enclosed.