On 08/22/14 14:43, Michal Privoznik wrote:
> 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@xxxxxxxxxx>
>>> ---
>>> 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.
OK. I thought it'd crash. (And this time I didn't have the cycles to
check the callee myself.)
>
>>
>>> + 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.
Nah, I got that. I was only worried about a NULL deref in the template
formatting.
series
Acked-by: Laszlo Ersek <lersek@xxxxxxxxxx>
Hopefully Dan will be OK with this version.
Cheers,
Laszlo
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list