qemu_firmware: Move copying of nvram.format to loader.format

As explained in the comment that comes along with it, this code
ensures that the user's preference is taken into account when
nvram.format is the only information that's provided. Currently
it lives in the parser, but it makes more sense for it to be
together with the rest of the firmware selection code instead.

Note that this move is not completely seamless: once the code
is moved outside of the parser, it can no longer reliably know
whether the <loader> element actually existed in the domain
XML. The difference is subtle enough that the test suite is
completely unaffected, and we are going to rework the handling
of this scenario in a way that restores the original behavior
later anyway, so it ultimately doesn't matter.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b85bbbcaf621fa4c4f50e30c5d6ab6ffd0feb376..9ca5c2450c502641ef1478ca9b9108bb0be05671 100644 (file)
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17826,24 +17826,8 @@ virDomainLoaderDefParseXMLLoader(virDomainLoaderDef *loader,
 {
     unsigned int format = 0;
 
-    if (!loaderNode) {
-        /* If there is no <loader> element but the <nvram> element
-         * was present, copy the format from the latter to the
-         * former.
-         *
-         * This ensures that a configuration such as
-         *
-         *   <os>
-         *     <nvram format='foo'/>
-         *   </os>
-         *
-         * behaves as expected, that is, results in a firmware build
-         * with format 'foo' being selected */
-        if (loader->nvram)
-            loader->format = loader->nvram->format;
-
+    if (!loaderNode)
         return 0;
-    }
 
     if (virXMLPropTristateBool(loaderNode, "readonly", VIR_XML_PROP_NONE,
                                &loader->readonly) < 0)

diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index 6c609ece6a6e398d93b61f03a633a580fef729e6..a22853361ba12733ae5acf7f6803fce7549ce9dd 100644 (file)
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -1783,6 +1783,21 @@ qemuFirmwareFillDomain(virQEMUDriver *driver,
     bool autoSelection = (def->os.firmware != VIR_DOMAIN_OS_DEF_FIRMWARE_NONE);
     int ret;
 
+    /* If there is no <loader> element but the <nvram> element
+     * was present, copy the format from the latter to the
+     * former.
+     *
+     * This ensures that a configuration such as
+     *
+     *   <os>
+     *     <nvram format='foo'/>
+     *   </os>
+     *
+     * behaves as expected, that is, results in a firmware build
+     * with format 'foo' being selected */
+    if (loader && loader->nvram && !loader->format)
+        loader->format = loader->nvram->format;
+
     /* If we're loading an existing configuration from disk, we
      * should try as hard as possible to preserve historical
      * behavior. In particular, firmware autoselection being enabled