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

[Michal Privoznik]

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 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?

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.

>>   }
>
> Looks good otherwise.

Cool!

>
> Thanks
> Laszlo
>

Michal