conf: Fix migration in some firmware autoselection scenarios

Introduce a small kludge in the parser to avoid unnecessarily
blocking incoming migration from a range of recent libvirt
releases.

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

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

src/conf/domain_conf.c

@@ -17182,11 +17182,13 @@ virDomainDefParseBootKernelOptions(virDomainDef *def,
 
 static int
 virDomainDefParseBootFirmwareOptions(virDomainDef *def,
-                                     xmlXPathContextPtr ctxt)
+                                     xmlXPathContextPtr ctxt,
+                                     unsigned int flags)
 {
     g_autofree char *firmware = virXPathString("string(./os/@firmware)", ctxt);
     g_autofree xmlNodePtr *nodes = NULL;
     g_autofree int *features = NULL;
+    bool abiUpdate = !!(flags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE);
     int fw = 0;
     int n = 0;
     size_t i;
@@ -17194,6 +17196,39 @@ virDomainDefParseBootFirmwareOptions(virDomainDef *def,
     if ((n = virXPathNodeSet("./os/firmware/feature", ctxt, &nodes)) < 0)
         return -1;
 
+    /* Migration compatibility kludge.
+     *
+     * Between 8.6.0 and 9.1.0 (extremes included), the migratable
+     * XML produced when feature-based firmware autoselection was
+     * enabled looked like
+     *
+     *   <os>
+     *     <firmware>
+     *       <feature name='foo' enabled='yes'/>
+     *
+     * Notice how there's no firmware='foo' attribute for the <os>
+     * element, meaning that firmware autoselection is disabled, and
+     * yet some <feature> elements, which are used to control the
+     * firmware autoselection process, are present. We don't consider
+     * this to be a valid combination, and want such a configuration
+     * to get rejected when submitted by users.
+     *
+     * In order to achieve that, while at the same time keeping
+     * migration coming from the libvirt versions listed above
+     * working, we can simply stop parsing early and ignore the
+     * <feature> tags when firmware autoselection is not enabled,
+     * *except* if we're defining a new domain.
+     *
+     * This is safe to do because the configuration will either come
+     * from another libvirt instance, in which case it will have a
+     * properly filled in <loader> element that contains enough
+     * information to successfully define and start the domain, or it
+     * will be a random configuration that lacks such information, in
+     * which case a different failure will be reported anyway.
+     */
+    if (n > 0 && !firmware && !abiUpdate)
+        return 0;
+
     if (n > 0)
         features = g_new0(int, VIR_DOMAIN_OS_DEF_FIRMWARE_FEATURE_LAST);
 
@@ -17322,7 +17357,7 @@ virDomainDefParseBootOptions(virDomainDef *def,
     case VIR_DOMAIN_OSTYPE_HVM:
         virDomainDefParseBootKernelOptions(def, ctxt);
 
-        if (virDomainDefParseBootFirmwareOptions(def, ctxt) < 0)
+        if (virDomainDefParseBootFirmwareOptions(def, ctxt, flags) < 0)
             return -1;
 
         if (virDomainDefParseBootLoaderOptions(def, ctxt, xmlopt, flags) < 0)

tests/qemuxml2argvdata/firmware-manual-efi-features.x86_64-latest.args

@@ -0,0 +1,35 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/var/lib/libvirt/qemu/domain--1-guest \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-guest/.local/share \
+XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-guest/.cache \
+XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-guest/.config \
+/usr/bin/qemu-system-x86_64 \
+-name guest=guest,debug-threads=on \
+-S \
+-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-guest/master-key.aes"}' \
+-blockdev '{"driver":"file","filename":"/usr/share/OVMF/OVMF_CODE.fd","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}' \
+-blockdev '{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"raw","file":"libvirt-pflash0-storage"}' \
+-blockdev '{"driver":"file","filename":"/var/lib/libvirt/qemu/nvram/guest_VARS.fd","node-name":"libvirt-pflash1-storage","auto-read-only":true,"discard":"unmap"}' \
+-blockdev '{"node-name":"libvirt-pflash1-format","read-only":false,"driver":"raw","file":"libvirt-pflash1-storage"}' \
+-machine pc-i440fx-4.0,usb=off,dump-guest-core=off,memory-backend=pc.ram,pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-format,acpi=on \
+-accel tcg \
+-cpu qemu64 \
+-m 1024 \
+-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":1073741824}' \
+-overcommit mem-lock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid 63840878-0deb-4095-97e6-fc444d9bc9fa \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-boot strict=on \
+-audiodev '{"id":"audio1","driver":"none"}' \
+-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
+-msg timestamp=on

tests/qemuxml2argvtest.c

@@ -1051,7 +1051,11 @@ mymain(void)
     DO_TEST_CAPS_LATEST("firmware-manual-bios-stateless");
     DO_TEST_CAPS_LATEST_PARSE_ERROR("firmware-manual-bios-not-stateless");
     DO_TEST_CAPS_LATEST("firmware-manual-efi");
-    DO_TEST_CAPS_LATEST_PARSE_ERROR("firmware-manual-efi-features");
+    DO_TEST_CAPS_LATEST("firmware-manual-efi-features");
+    DO_TEST_CAPS_ARCH_LATEST_FULL("firmware-manual-efi-features", "x86_64",
+                                  ARG_FLAGS, FLAG_EXPECT_PARSE_ERROR,
+                                  ARG_PARSEFLAGS, VIR_DOMAIN_DEF_PARSE_ABI_UPDATE,
+                                  ARG_END);
     DO_TEST_CAPS_LATEST("firmware-manual-efi-rw");
     DO_TEST_CAPS_LATEST("firmware-manual-efi-rw-implicit");
     DO_TEST_CAPS_LATEST("firmware-manual-efi-loader-secure");

tests/qemuxml2xmloutdata/firmware-manual-efi-features.x86_64-latest.xml

@@ -0,0 +1,36 @@
+<domain type='qemu'>
+  <name>guest</name>
+  <uuid>63840878-0deb-4095-97e6-fc444d9bc9fa</uuid>
+  <memory unit='KiB'>1048576</memory>
+  <currentMemory unit='KiB'>1048576</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os firmware='efi'>
+    <type arch='x86_64' machine='pc-i440fx-4.0'>hvm</type>
+    <firmware>
+      <feature enabled='no' name='enrolled-keys'/>
+      <feature enabled='no' name='secure-boot'/>
+    </firmware>
+    <loader readonly='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE.fd</loader>
+    <nvram template='/usr/share/OVMF/OVMF_VARS.fd'>/var/lib/libvirt/qemu/nvram/guest_VARS.fd</nvram>
+    <boot dev='hd'/>
+  </os>
+  <features>
+    <acpi/>
+  </features>
+  <cpu mode='custom' match='exact' check='none'>
+    <model fallback='forbid'>qemu64</model>
+  </cpu>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-x86_64</emulator>
+    <controller type='usb' index='0' model='none'/>
+    <controller type='pci' index='0' model='pci-root'/>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <audio id='1' type='none'/>
+    <memballoon model='none'/>
+  </devices>
+</domain>

tests/qemuxml2xmltest.c

@@ -866,6 +866,7 @@ mymain(void)
     DO_TEST_CAPS_LATEST("firmware-manual-bios");
     DO_TEST_CAPS_LATEST("firmware-manual-bios-stateless");
     DO_TEST_CAPS_LATEST("firmware-manual-efi");
+    DO_TEST_CAPS_LATEST("firmware-manual-efi-features");
     DO_TEST_CAPS_LATEST("firmware-manual-efi-rw");
     DO_TEST_CAPS_LATEST("firmware-manual-efi-rw-implicit");
     DO_TEST_CAPS_LATEST("firmware-manual-efi-loader-secure");