From: Andrea Bolognani <abologna@redhat.com>
Date: Thu, 27 Nov 2025 18:05:17 +0000 (+0100)
Subject: qemu_firmware: Move copying of nvram.format to loader.format
X-Git-Tag: v12.0.0-rc1~45
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4df091dea4a41767561bab1bcd28c3fd9ac2dcea;p=thirdparty%2Flibvirt.git

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 b85bbbcaf6..9ca5c2450c 100644
--- 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 6c609ece6a..a22853361b 100644
--- 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