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

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


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