From 343c98ebdcb3a75cf8a2b8b496da3887ec8f87c3 Mon Sep 17 00:00:00 2001

Message-Id: <343c98ebdcb3a75cf8a2b8b496da3887ec8f87c3@dist-git>

From: Michal Privoznik <mprivozn@redhat.com>

Date: Wed, 10 Sep 2014 10:11:43 +0200

Subject: [PATCH] conf: Extend <loader/> and introduce <nvram/>

https://bugzilla.redhat.com/show_bug.cgi?id=1112257

Up to now, users can configure BIOS via the <loader/> element. With

the upcoming implementation of UEFI this is not enough as BIOS and

UEFI are conceptually different. For instance, while BIOS is ROM, UEFI

is programmable flash (although all writes to code section are

denied). Therefore we need new attribute @type which will

differentiate the two. Then, new attribute @readonly is introduced to

reflect the fact that some images are RO.

Moreover, the OVMF (which is going to be used mostly), works in two

modes:

1) Code and UEFI variable store is mixed in one file.

2) Code and UEFI variable store is separated in two files

The latter has advantage of updating the UEFI code without losing the

configuration. However, in order to represent the latter case we need

yet another XML element: <nvram/>. Currently, it has no additional

attributes, it's just a bare element containing path to the variable

store file.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

Acked-by: Laszlo Ersek <lersek@redhat.com>

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

(cherry picked from commit 68bf13dbef8342eaee0bf57c73cebb60b7de11e8)

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

---

 docs/formatdomain.html.in                          | 19 ++++-

 docs/schemas/domaincommon.rng                      | 21 ++++++

 src/conf/domain_conf.c                             | 87 +++++++++++++++++++++-

 src/conf/domain_conf.h                             | 22 +++++-

 src/libvirt_private.syms                           |  3 +

 src/qemu/qemu_command.c                            |  5 +-

 src/security/virt-aa-helper.c                      |  4 +-

 src/vbox/vbox_common.c                             |  7 +-

 src/xenapi/xenapi_driver.c                         |  3 +-

 src/xenconfig/xen_common.c                         |  7 +-

 src/xenconfig/xen_sxpr.c                           | 16 ++--

 tests/qemuxml2argvdata/qemuxml2argv-bios-nvram.xml | 40 ++++++++++

 .../qemuxml2xmlout-pci-bridge-many-disks.xml       |  2 +-

 tests/qemuxml2xmltest.c                            |  2 +

 tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml      |  2 +-

 tests/sexpr2xmldata/sexpr2xml-fv-empty-kernel.xml  |  2 +-

 tests/sexpr2xmldata/sexpr2xml-fv-force-hpet.xml    |  2 +-

 tests/sexpr2xmldata/sexpr2xml-fv-force-nohpet.xml  |  2 +-

 tests/sexpr2xmldata/sexpr2xml-fv-kernel.xml        |  2 +-

 tests/sexpr2xmldata/sexpr2xml-fv-legacy-vfb.xml    |  2 +-

 tests/sexpr2xmldata/sexpr2xml-fv-localtime.xml     |  2 +-

 tests/sexpr2xmldata/sexpr2xml-fv-net-ioemu.xml     |  2 +-

 tests/sexpr2xmldata/sexpr2xml-fv-net-netfront.xml  |  2 +-

 tests/sexpr2xmldata/sexpr2xml-fv-parallel-tcp.xml  |  2 +-

 .../sexpr2xml-fv-serial-dev-2-ports.xml            |  2 +-

 .../sexpr2xml-fv-serial-dev-2nd-port.xml           |  2 +-

 tests/sexpr2xmldata/sexpr2xml-fv-serial-file.xml   |  2 +-

 tests/sexpr2xmldata/sexpr2xml-fv-serial-null.xml   |  2 +-

 tests/sexpr2xmldata/sexpr2xml-fv-serial-pipe.xml   |  2 +-

 tests/sexpr2xmldata/sexpr2xml-fv-serial-pty.xml    |  2 +-

 tests/sexpr2xmldata/sexpr2xml-fv-serial-stdio.xml  |  2 +-

 .../sexpr2xml-fv-serial-tcp-telnet.xml             |  2 +-

 tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp.xml    |  2 +-

 tests/sexpr2xmldata/sexpr2xml-fv-serial-udp.xml    |  2 +-

 tests/sexpr2xmldata/sexpr2xml-fv-serial-unix.xml   |  2 +-

 tests/sexpr2xmldata/sexpr2xml-fv-sound-all.xml     |  2 +-

 tests/sexpr2xmldata/sexpr2xml-fv-sound.xml         |  2 +-

 tests/sexpr2xmldata/sexpr2xml-fv-usbmouse.xml      |  2 +-

 tests/sexpr2xmldata/sexpr2xml-fv-usbtablet.xml     |  2 +-

 tests/sexpr2xmldata/sexpr2xml-fv-utc.xml           |  2 +-

 tests/sexpr2xmldata/sexpr2xml-fv-v2.xml            |  2 +-

 tests/sexpr2xmldata/sexpr2xml-fv.xml               |  2 +-

 tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml  |  2 +-

 tests/xmconfigdata/test-escape-paths.xml           |  2 +-

 tests/xmconfigdata/test-fullvirt-force-hpet.xml    |  2 +-

 tests/xmconfigdata/test-fullvirt-force-nohpet.xml  |  2 +-

 tests/xmconfigdata/test-fullvirt-localtime.xml     |  2 +-

 tests/xmconfigdata/test-fullvirt-net-ioemu.xml     |  2 +-

 tests/xmconfigdata/test-fullvirt-net-netfront.xml  |  2 +-

 tests/xmconfigdata/test-fullvirt-new-cdrom.xml     |  2 +-

 tests/xmconfigdata/test-fullvirt-old-cdrom.xml     |  2 +-

 tests/xmconfigdata/test-fullvirt-parallel-tcp.xml  |  2 +-

 .../test-fullvirt-serial-dev-2-ports.xml           |  2 +-

 .../test-fullvirt-serial-dev-2nd-port.xml          |  2 +-

 tests/xmconfigdata/test-fullvirt-serial-file.xml   |  2 +-

 tests/xmconfigdata/test-fullvirt-serial-null.xml   |  2 +-

 tests/xmconfigdata/test-fullvirt-serial-pipe.xml   |  2 +-

 tests/xmconfigdata/test-fullvirt-serial-pty.xml    |  2 +-

 tests/xmconfigdata/test-fullvirt-serial-stdio.xml  |  2 +-

 .../test-fullvirt-serial-tcp-telnet.xml            |  2 +-

 tests/xmconfigdata/test-fullvirt-serial-tcp.xml    |  2 +-

 tests/xmconfigdata/test-fullvirt-serial-udp.xml    |  2 +-

 tests/xmconfigdata/test-fullvirt-serial-unix.xml   |  2 +-

 tests/xmconfigdata/test-fullvirt-sound.xml         |  2 +-

 tests/xmconfigdata/test-fullvirt-usbmouse.xml      |  2 +-

 tests/xmconfigdata/test-fullvirt-usbtablet.xml     |  2 +-

 tests/xmconfigdata/test-fullvirt-utc.xml           |  2 +-

 tests/xmconfigdata/test-no-source-cdrom.xml        |  2 +-

 tests/xmconfigdata/test-pci-devs.xml               |  2 +-

 69 files changed, 269 insertions(+), 79 deletions(-)

 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-bios-nvram.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in

index 94236dd..757035a 100644

--- a/docs/formatdomain.html.in

+++ b/docs/formatdomain.html.in

@@ -102,7 +102,8 @@

   ...

   <os>

     <type>hvm</type>

-    <loader>/usr/lib/xen/boot/hvmloader</loader>

+    <loader readonly='on' type='rom'>/usr/lib/xen/boot/hvmloader</loader>

+    <nvram>/var/lib/libvirt/nvram/guest_VARS.fd</nvram>

     <boot dev='hd'/>

     <boot dev='cdrom'/>

     <bootmenu enable='yes' timeout='3000'/>

@@ -129,7 +130,21 @@

         used to assist the domain creation process. It is used by Xen

         fully virtualized domains as well as setting the QEMU BIOS file

         path for QEMU/KVM domains. Xen since 0.1.0,

-        QEMU/KVM since 0.9.12

+        QEMU/KVM since 0.9.12 Then, since

+        1.2.8 it's possible for the element to have two

+        optional attributes: readonly (accepted values are

+        yes and no) to reflect the fact that the

+        image should be writable or read-only. The second attribute

+        type accepts values rom and

+        pflash. It tells the hypervisor where in the guest

+        memory the file should be mapped.  For instance, if the loader

+        path points to an UEFI image, type should be

+        pflash.

+      
nvram

+      
Some UEFI firmwares may want to use a non-volatile memory to store

+        some variables. In the host, this is represented as a file and the

+        path to the file is stored in this element. Since

+        1.2.8

       
boot

       
The dev attribute takes one of the values "fd", "hd",

         "cdrom" or "network" and is used to specify the next boot device

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng

index cedceae..5d9c21c 100644

--- a/docs/schemas/domaincommon.rng

+++ b/docs/schemas/domaincommon.rng

@@ -242,6 +242,27 @@

       <interleave>

         <optional>

           <element name="loader">

+            <optional>

+              <attribute name="readonly">

+                <choice>

+                  <value>yes</value>

+                  <value>no</value>

+                </choice>

+              </attribute>

+            </optional>

+            <optional>

+              <attribute name="type">

+                <choice>

+                  <value>rom</value>

+                  <value>pflash</value>

+                </choice>

+              </attribute>

+            </optional>

+            <ref name="absFilePath"/>

+          </element>

+        </optional>

+        <optional>

+          <element name="nvram">

             <ref name="absFilePath"/>

           </element>

         </optional>

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c

index 53ef694..6ee5c17 100644

--- a/src/conf/domain_conf.c

+++ b/src/conf/domain_conf.c

@@ -777,6 +777,11 @@ VIR_ENUM_IMPL(virDomainDiskMirrorState, VIR_DOMAIN_DISK_MIRROR_STATE_LAST,

               "abort",

               "pivot")

+VIR_ENUM_IMPL(virDomainLoader,

+              VIR_DOMAIN_LOADER_TYPE_LAST,

+              "rom",

+              "pflash")

+

 /* Internal mapping: subset of block job types that can be present in

  * <mirror> XML (remaining types are not two-phase). */

 VIR_ENUM_DECL(virDomainBlockJob)

@@ -2010,6 +2015,17 @@ virDomainPanicDefFree(virDomainPanicDefPtr panic)

     VIR_FREE(panic);

 }

+void

+virDomainLoaderDefFree(virDomainLoaderDefPtr loader)

+{

+    if (!loader)

+        return;

+

+    VIR_FREE(loader->path);

+    VIR_FREE(loader->nvram);

+    VIR_FREE(loader);

+}

+

 void virDomainDefFree(virDomainDefPtr def)

 {

     size_t i;

@@ -2115,7 +2131,7 @@ void virDomainDefFree(virDomainDefPtr def)

     VIR_FREE(def->os.cmdline);

     VIR_FREE(def->os.dtb);

     VIR_FREE(def->os.root);

-    VIR_FREE(def->os.loader);

+    virDomainLoaderDefFree(def->os.loader);

     VIR_FREE(def->os.bootloader);

     VIR_FREE(def->os.bootloaderArgs);

@@ -11661,6 +11677,42 @@ virDomainDefMaybeAddHostdevSCSIcontroller(virDomainDefPtr def)

     return 0;

 }

+static int

+virDomainLoaderDefParseXML(xmlNodePtr node,

+                           virDomainLoaderDefPtr loader)

+{

+    int ret = -1;

+    char *readonly_str = NULL;

+    char *type_str = NULL;

+

+    readonly_str = virXMLPropString(node, "readonly");

+    type_str = virXMLPropString(node, "type");

+    loader->path = (char *) xmlNodeGetContent(node);

+

+    if (readonly_str &&

+        (loader->readonly = virTristateBoolTypeFromString(readonly_str)) <= 0) {

+        virReportError(VIR_ERR_XML_DETAIL,

+                       _("unknown readonly value: %s"), readonly_str);

+        goto cleanup;

+    }

+

+    if (type_str) {

+        int type;

+        if ((type = virDomainLoaderTypeFromString(type_str)) < 0) {

+            virReportError(VIR_ERR_XML_DETAIL,

+                           _("unknown type value: %s"), type_str);

+            goto cleanup;

+        }

+        loader->type = type;

+    }

+

+    ret = 0;

+ cleanup:

+    VIR_FREE(readonly_str);

+    VIR_FREE(type_str);

+    return ret;

+}

+

 static virDomainDefPtr

 virDomainDefParseXML(xmlDocPtr xml,

                      xmlNodePtr root,

@@ -12701,12 +12753,22 @@ virDomainDefParseXML(xmlDocPtr xml,

     if (STREQ(def->os.type, "xen") ||

         STREQ(def->os.type, "hvm") ||

         STREQ(def->os.type, "uml")) {

+        xmlNodePtr loader_node;

+

         def->os.kernel = virXPathString("string(./os/kernel[1])", ctxt);

         def->os.initrd = virXPathString("string(./os/initrd[1])", ctxt);

         def->os.cmdline = virXPathString("string(./os/cmdline[1])", ctxt);

         def->os.dtb = virXPathString("string(./os/dtb[1])", ctxt);

         def->os.root = virXPathString("string(./os/root[1])", ctxt);

-        def->os.loader = virXPathString("string(./os/loader[1])", ctxt);

+        if ((loader_node = virXPathNode("./os/loader[1]", ctxt))) {

+            if (VIR_ALLOC(def->os.loader) < 0)

+                goto error;

+

+            if (virDomainLoaderDefParseXML(loader_node, def->os.loader) < 0)

+                goto error;

+

+            def->os.loader->nvram = virXPathString("string(./os/nvram[1])", ctxt);

+        }

     }

     if (STREQ(def->os.type, "hvm")) {

@@ -17789,6 +17851,23 @@ virDomainHugepagesFormat(virBufferPtr buf,

     virBufferAddLit(buf, "</hugepages>\n");

 }

+static void

+virDomainLoaderDefFormat(virBufferPtr buf,

+                         virDomainLoaderDefPtr loader)

+{

+    const char *readonly = virTristateBoolTypeToString(loader->readonly);

+    const char *type = virDomainLoaderTypeToString(loader->type);

+

+    virBufferAddLit(buf, "

+

+    if (loader->readonly)

+        virBufferAsprintf(buf, " readonly='%s'", readonly);

+

+    virBufferAsprintf(buf, " type='%s'>", type);

+

+    virBufferEscapeString(buf, "%s</loader>\n", loader->path);

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

+}

 static bool

 virDomainDefHasCapabilitiesFeatures(virDomainDefPtr def)

@@ -18109,8 +18188,8 @@ virDomainDefFormatInternal(virDomainDefPtr def,

     for (i = 0; def->os.initargv && def->os.initargv[i]; i++)

         virBufferEscapeString(buf, "<initarg>%s</initarg>\n",

                               def->os.initargv[i]);

-    virBufferEscapeString(buf, "<loader>%s</loader>\n",

-                          def->os.loader);

+    if (def->os.loader)

+        virDomainLoaderDefFormat(buf, def->os.loader);

     virBufferEscapeString(buf, "<kernel>%s</kernel>\n",

                           def->os.kernel);

     virBufferEscapeString(buf, "<initrd>%s</initrd>\n",

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h

index 9586c3b..c97a10c 100644

--- a/src/conf/domain_conf.h

+++ b/src/conf/domain_conf.h

@@ -1628,6 +1628,26 @@ struct _virDomainBIOSDef {

     int rt_delay;

 };

+typedef enum {

+    VIR_DOMAIN_LOADER_TYPE_ROM = 0,

+    VIR_DOMAIN_LOADER_TYPE_PFLASH,

+

+    VIR_DOMAIN_LOADER_TYPE_LAST

+} virDomainLoader;

+

+VIR_ENUM_DECL(virDomainLoader)

+

+typedef struct _virDomainLoaderDef virDomainLoaderDef;

+typedef virDomainLoaderDef *virDomainLoaderDefPtr;

+struct _virDomainLoaderDef {

+    char *path;

+    int readonly;   /* enum virTristateBool */

+    virDomainLoader type;

+    char *nvram;    /* path to non-volatile RAM */

+};

+

+void virDomainLoaderDefFree(virDomainLoaderDefPtr loader);

+

 /* Operating system configuration data & machine / arch */

 typedef struct _virDomainOSDef virDomainOSDef;

 typedef virDomainOSDef *virDomainOSDefPtr;

@@ -1647,7 +1667,7 @@ struct _virDomainOSDef {

     char *cmdline;

     char *dtb;

     char *root;

-    char *loader;

+    virDomainLoaderDefPtr loader;

     char *bootloader;

     char *bootloaderArgs;

     int smbios_mode;

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms

index 71fc063..b4b13f9 100644

--- a/src/libvirt_private.syms

+++ b/src/libvirt_private.syms

@@ -313,6 +313,9 @@ virDomainLifecycleTypeFromString;

 virDomainLifecycleTypeToString;

 virDomainListFree;

 virDomainLiveConfigHelperMethod;

+virDomainLoaderDefFree;

+virDomainLoaderTypeFromString;

+virDomainLoaderTypeToString;

 virDomainLockFailureTypeFromString;

 virDomainLockFailureTypeToString;

 virDomainMemballoonModelTypeFromString;

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c

index 2184caa..3cb2e0b 100644

--- a/src/qemu/qemu_command.c

+++ b/src/qemu/qemu_command.c

@@ -7527,7 +7527,7 @@ qemuBuildCommandLine(virConnectPtr conn,

     if (def->os.loader) {

         virCommandAddArg(cmd, "-bios");

-        virCommandAddArg(cmd, def->os.loader);

+        virCommandAddArg(cmd, def->os.loader->path);

     }

     /* Set '-m MB' based on maxmem, because the lower 'memory' limit

@@ -11347,7 +11347,8 @@ qemuParseCommandLine(virCapsPtr qemuCaps,

                 goto error;

         } else if (STREQ(arg, "-bios")) {

             WANT_VALUE();

-            if (VIR_STRDUP(def->os.loader, val) < 0)

+            if (VIR_ALLOC(def->os.loader) < 0 ||

+                VIR_STRDUP(def->os.loader->path, val) < 0)

                 goto error;

         } else if (STREQ(arg, "-initrd")) {

             WANT_VALUE();

diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c

index a0b104c..311ce3b 100644

--- a/src/security/virt-aa-helper.c

+++ b/src/security/virt-aa-helper.c

@@ -1006,8 +1006,8 @@ get_files(vahControl * ctl)

         if (vah_add_file(&buf, ctl->def->os.dtb, "r") != 0)

             goto cleanup;

-    if (ctl->def->os.loader && ctl->def->os.loader)

-        if (vah_add_file(&buf, ctl->def->os.loader, "r") != 0)

+    if (ctl->def->os.loader && ctl->def->os.loader->path)

+        if (vah_add_file(&buf, ctl->def->os.loader->path, "r") != 0)

             goto cleanup;

     for (i = 0; i < ctl->def->ngraphics; i++) {

diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c

index b186ea8..6f56c59 100644

--- a/src/vbox/vbox_common.c

+++ b/src/vbox/vbox_common.c

@@ -988,7 +988,12 @@ vboxSetBootDeviceOrder(virDomainDefPtr def, vboxGlobalData *data,

     VIR_DEBUG("def->os.initrd           %s", def->os.initrd);

     VIR_DEBUG("def->os.cmdline          %s", def->os.cmdline);

     VIR_DEBUG("def->os.root             %s", def->os.root);

-    VIR_DEBUG("def->os.loader           %s", def->os.loader);

+    if (def->os.loader) {

+        VIR_DEBUG("def->os.loader->path     %s", def->os.loader->path);

+        VIR_DEBUG("def->os.loader->readonly %d", def->os.loader->readonly);

+        VIR_DEBUG("def->os.loader->type     %d", def->os.loader->type);

+        VIR_DEBUG("def->os.loader->nvram    %s", def->os.loader->nvram);

+    }

     VIR_DEBUG("def->os.bootloader       %s", def->os.bootloader);

     VIR_DEBUG("def->os.bootloaderArgs   %s", def->os.bootloaderArgs);

diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c

index 50331c9..a6eaccc 100644

--- a/src/xenapi/xenapi_driver.c

+++ b/src/xenapi/xenapi_driver.c

@@ -1427,7 +1427,8 @@ xenapiDomainGetXMLDesc(virDomainPtr dom, unsigned int flags)

             VIR_FREE(boot_policy);

             goto error;

         }

-        if (VIR_STRDUP(defPtr->os.loader, "pygrub") < 0) {

+        if (VIR_ALLOC(defPtr->os.loader) < 0 ||

+            VIR_STRDUP(defPtr->os.loader->path, "pygrub") < 0) {

             VIR_FREE(boot_policy);

             goto error;

         }

diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c

index 9beaf6c..abd77b5 100644

--- a/src/xenconfig/xen_common.c

+++ b/src/xenconfig/xen_common.c

@@ -1065,7 +1065,8 @@ xenParseOS(virConfPtr conf, virDomainDefPtr def)

     if (STREQ(def->os.type, "hvm")) {

         const char *boot;

-       if (xenConfigCopyString(conf, "kernel", &def->os.loader) < 0)

+        if (VIR_ALLOC(def->os.loader) < 0 ||

+            xenConfigCopyString(conf, "kernel", &def->os.loader->path) < 0)

             return -1;

         if (xenConfigGetString(conf, "boot", &boot, "c") < 0)

@@ -1740,8 +1741,8 @@ xenFormatOS(virConfPtr conf, virDomainDefPtr def)

         if (xenXMConfigSetString(conf, "builder", "hvm") < 0)

             return -1;

-        if (def->os.loader &&

-            xenXMConfigSetString(conf, "kernel", def->os.loader) < 0)

+        if (def->os.loader && def->os.loader->path &&

+            xenXMConfigSetString(conf, "kernel", def->os.loader->path) < 0)

             return -1;

         for (i = 0; i < def->os.nBootDevs; i++) {

diff --git a/src/xenconfig/xen_sxpr.c b/src/xenconfig/xen_sxpr.c

index ff81c36..e8b9f59 100644

--- a/src/xenconfig/xen_sxpr.c

+++ b/src/xenconfig/xen_sxpr.c

@@ -93,13 +93,15 @@ xenParseSxprOS(const struct sexpr *node,

                int hvm)

 {

     if (hvm) {

-        if (sexpr_node_copy(node, "domain/image/hvm/loader", &def->os.loader) < 0)

+        if (VIR_ALLOC(def->os.loader) < 0)

             goto error;

-        if (def->os.loader == NULL) {

-            if (sexpr_node_copy(node, "domain/image/hvm/kernel", &def->os.loader) < 0)

+        if (sexpr_node_copy(node, "domain/image/hvm/loader", &def->os.loader->path) < 0)

+            goto error;

+        if (def->os.loader->path == NULL) {

+            if (sexpr_node_copy(node, "domain/image/hvm/kernel", &def->os.loader->path) < 0)

                 goto error;

-            if (def->os.loader == NULL) {

+            if (def->os.loader->path == NULL) {

                 virReportError(VIR_ERR_INTERNAL_ERROR,

                                "%s", _("domain information incomplete, missing HVM loader"));

                 return -1;

@@ -128,7 +130,7 @@ xenParseSxprOS(const struct sexpr *node,

     /* If HVM kenrel == loader, then old xend, so kill off kernel */

     if (hvm &&

         def->os.kernel &&

-        STREQ(def->os.kernel, def->os.loader)) {

+        STREQ(def->os.kernel, def->os.loader->path)) {

         VIR_FREE(def->os.kernel);

     }

     /* Drop kernel argument that has no value */

@@ -2279,9 +2281,9 @@ xenFormatSxpr(virConnectPtr conn,

         if (hvm) {

             char bootorder[VIR_DOMAIN_BOOT_LAST+1];

             if (def->os.kernel)

-                virBufferEscapeSexpr(&buf, "(loader '%s')", def->os.loader);

+                virBufferEscapeSexpr(&buf, "(loader '%s')", def->os.loader->path);

             else

-                virBufferEscapeSexpr(&buf, "(kernel '%s')", def->os.loader);

+                virBufferEscapeSexpr(&buf, "(kernel '%s')", def->os.loader->path);

             virBufferAsprintf(&buf, "(vcpus %u)", def->maxvcpus);

             if (def->vcpus < def->maxvcpus)

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-bios-nvram.xml b/tests/qemuxml2argvdata/qemuxml2argv-bios-nvram.xml

new file mode 100644

index 0000000..d8270b1

--- /dev/null

+++ b/tests/qemuxml2argvdata/qemuxml2argv-bios-nvram.xml

@@ -0,0 +1,40 @@

+<domain type='qemu'>

+  <name>test-bios</name>

+  <uuid>362d1fc1-df7d-193e-5c18-49a71bd1da66</uuid>

+  <memory unit='KiB'>1048576</memory>

+  <currentMemory unit='KiB'>1048576</currentMemory>

+  <vcpu placement='static'>1</vcpu>

+  <os>

+    <type arch='x86_64' machine='pc'>hvm</type>

+    <loader readonly='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE.fd</loader>

+    <nvram>/usr/share/OVMF/OVMF_VARS.fd</nvram>

+    <boot dev='hd'/>

+    <bootmenu enable='yes'/>

+  </os>

+  <features>

+    <acpi/>

+  </features>

+  <clock offset='utc'/>

+  <on_poweroff>destroy</on_poweroff>

+  <on_reboot>restart</on_reboot>

+  <on_crash>restart</on_crash>

+  <devices>

+    <emulator>/usr/bin/qemu</emulator>

+    <disk type='block' device='disk'>

+      <source dev='/dev/HostVG/QEMUGuest1'/>

+      <target dev='hda' bus='ide'/>

+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>

+    </disk>

+    <controller type='usb' index='0'/>

+    <controller type='ide' index='0'/>

+    <controller type='pci' index='0' model='pci-root'/>

+    <serial type='pty'>

+      <target port='0'/>

+    </serial>

+    <console type='pty'>

+      <target type='serial' port='0'/>

+    </console>

+    <input type='tablet' bus='usb'/>

+    <memballoon model='virtio'/>

+  </devices>

+</domain>

diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pci-bridge-many-disks.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pci-bridge-many-disks.xml

index d469b8b..d49f5f4 100644

--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pci-bridge-many-disks.xml

+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pci-bridge-many-disks.xml

@@ -6,7 +6,7 @@

   <vcpu placement='static'>1</vcpu>

   <os>

     <type arch='x86_64' machine='pc-i440fx-1.4'>hvm</type>

-    <loader>/usr/share/seabios/bios.bin</loader>

+    <loader type='rom'>/usr/share/seabios/bios.bin</loader>

     <boot dev='hd'/>

   </os>

   <features>

diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c

index b4ab671..1835fe6 100644

--- a/tests/qemuxml2xmltest.c

+++ b/tests/qemuxml2xmltest.c

@@ -395,6 +395,8 @@ mymain(void)

     DO_TEST_DIFFERENT("numatune-memnode");

     DO_TEST("numatune-memnode-no-memory");

+    DO_TEST("bios-nvram");

+

     virObjectUnref(driver.caps);

     virObjectUnref(driver.xmlopt);

diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml b/tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml

index 69fe9ef..761952c 100644

--- a/tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml

+++ b/tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml

@@ -6,7 +6,7 @@

   <vcpu placement='static'>1</vcpu>

   <os>

     <type>hvm</type>

-    <loader>/usr/lib/xen/boot/hvmloader</loader>

+    <loader type='rom'>/usr/lib/xen/boot/hvmloader</loader>

     <boot dev='cdrom'/>

   </os>

   <features>

diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-empty-kernel.xml b/tests/sexpr2xmldata/sexpr2xml-fv-empty-kernel.xml

index 3c3147d..2898098 100644

--- a/tests/sexpr2xmldata/sexpr2xml-fv-empty-kernel.xml

+++ b/tests/sexpr2xmldata/sexpr2xml-fv-empty-kernel.xml

@@ -6,7 +6,7 @@

   <vcpu placement='static'>1</vcpu>

   <os>

     <type>hvm</type>

-    <loader>/usr/lib/xen/boot/hvmloader</loader>

+    <loader type='rom'>/usr/lib/xen/boot/hvmloader</loader>

     <boot dev='cdrom'/>

   </os>

   <features>

diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-force-hpet.xml b/tests/sexpr2xmldata/sexpr2xml-fv-force-hpet.xml

index 716f16b..a0fe30d 100644

--- a/tests/sexpr2xmldata/sexpr2xml-fv-force-hpet.xml

+++ b/tests/sexpr2xmldata/sexpr2xml-fv-force-hpet.xml

@@ -6,7 +6,7 @@

   <vcpu placement='static'>1</vcpu>

   <os>

     <type>hvm</type>

-    <loader>/usr/lib/xen/boot/hvmloader</loader>

+    <loader type='rom'>/usr/lib/xen/boot/hvmloader</loader>

     <boot dev='hd'/>

   </os>

   <features>

diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-force-nohpet.xml b/tests/sexpr2xmldata/sexpr2xml-fv-force-nohpet.xml

index 3dd648b..851797d 100644

--- a/tests/sexpr2xmldata/sexpr2xml-fv-force-nohpet.xml

+++ b/tests/sexpr2xmldata/sexpr2xml-fv-force-nohpet.xml

@@ -6,7 +6,7 @@

   <vcpu placement='static'>1</vcpu>

   <os>

     <type>hvm</type>

-    <loader>/usr/lib/xen/boot/hvmloader</loader>

+    <loader type='rom'>/usr/lib/xen/boot/hvmloader</loader>

     <boot dev='hd'/>

   </os>

   <features>

diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-kernel.xml b/tests/sexpr2xmldata/sexpr2xml-fv-kernel.xml

index 29c1335..09cfe19 100644

--- a/tests/sexpr2xmldata/sexpr2xml-fv-kernel.xml

+++ b/tests/sexpr2xmldata/sexpr2xml-fv-kernel.xml

@@ -6,7 +6,7 @@

   <vcpu placement='static'>2</vcpu>

   <os>

     <type>hvm</type>

-    <loader>/usr/lib/xen/boot/hvmloader</loader>

+    <loader type='rom'>/usr/lib/xen/boot/hvmloader</loader>

     <kernel>/var/lib/xen/vmlinuz.2Dn2YT</kernel>

     <initrd>/var/lib/xen/initrd.img.0u-Vhq</initrd>

     <cmdline> method=http://download.fedora.devel.redhat.com/pub/fedora/linux/core/test/5.91/x86_64/os  </cmdline>

diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-legacy-vfb.xml b/tests/sexpr2xmldata/sexpr2xml-fv-legacy-vfb.xml

index 9c59644..44c0f61 100644

--- a/tests/sexpr2xmldata/sexpr2xml-fv-legacy-vfb.xml

+++ b/tests/sexpr2xmldata/sexpr2xml-fv-legacy-vfb.xml

@@ -6,7 +6,7 @@

   <vcpu placement='static'>1</vcpu>

   <os>

     <type>hvm</type>

-    <loader>/usr/lib/xen/boot/hvmloader</loader>

+    <loader type='rom'>/usr/lib/xen/boot/hvmloader</loader>

     <boot dev='hd'/>

   </os>

   <features>

diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-localtime.xml b/tests/sexpr2xmldata/sexpr2xml-fv-localtime.xml

index 67b0b95..29007f0 100644

--- a/tests/sexpr2xmldata/sexpr2xml-fv-localtime.xml

+++ b/tests/sexpr2xmldata/sexpr2xml-fv-localtime.xml

@@ -6,7 +6,7 @@

   <vcpu placement='static'>1</vcpu>

   <os>

     <type>hvm</type>

-    <loader>/usr/lib/xen/boot/hvmloader</loader>

+    <loader type='rom'>/usr/lib/xen/boot/hvmloader</loader>

     <boot dev='hd'/>

   </os>

   <features>

diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-net-ioemu.xml b/tests/sexpr2xmldata/sexpr2xml-fv-net-ioemu.xml

index 86b32e9..3dbc999 100644

--- a/tests/sexpr2xmldata/sexpr2xml-fv-net-ioemu.xml

+++ b/tests/sexpr2xmldata/sexpr2xml-fv-net-ioemu.xml

@@ -6,7 +6,7 @@

   <vcpu placement='static'>1</vcpu>

   <os>

     <type>hvm</type>

-    <loader>/usr/lib/xen/boot/hvmloader</loader>

+    <loader type='rom'>/usr/lib/xen/boot/hvmloader</loader>

     <boot dev='hd'/>

   </os>

   <features>

diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-net-netfront.xml b/tests/sexpr2xmldata/sexpr2xml-fv-net-netfront.xml

index ed7da80..d96350e 100644

--- a/tests/sexpr2xmldata/sexpr2xml-fv-net-netfront.xml

+++ b/tests/sexpr2xmldata/sexpr2xml-fv-net-netfront.xml

@@ -6,7 +6,7 @@

   <vcpu placement='static'>1</vcpu>

   <os>

     <type>hvm</type>

-    <loader>/usr/lib/xen/boot/hvmloader</loader>

+    <loader type='rom'>/usr/lib/xen/boot/hvmloader</loader>

     <boot dev='hd'/>

   </os>

   <features>

diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-parallel-tcp.xml b/tests/sexpr2xmldata/sexpr2xml-fv-parallel-tcp.xml

index ed3fde6..7ad377c 100644

--- a/tests/sexpr2xmldata/sexpr2xml-fv-parallel-tcp.xml

+++ b/tests/sexpr2xmldata/sexpr2xml-fv-parallel-tcp.xml

@@ -6,7 +6,7 @@

   <vcpu placement='static'>1</vcpu>

   <os>

     <type>hvm</type>

-    <loader>/usr/lib/xen/boot/hvmloader</loader>

+    <loader type='rom'>/usr/lib/xen/boot/hvmloader</loader>

     <boot dev='hd'/>

   </os>

   <features>

diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-serial-dev-2-ports.xml b/tests/sexpr2xmldata/sexpr2xml-fv-serial-dev-2-ports.xml

index 7f5a729..adba6cb 100644