From e4b040f7a05e4b160a62cd0ce1bdffed7efe8dfa Mon Sep 17 00:00:00 2001
Message-Id: <e4b040f7a05e4b160a62cd0ce1bdffed7efe8dfa@dist-git>
From: Andrea Bolognani <abologna@redhat.com>
Date: Tue, 11 Apr 2023 17:56:45 +0200
Subject: [PATCH] conf: Fix migration in some firmware autoselection scenarios
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
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>
(cherry picked from commit f9ad3023355bcbfc692bbe4997fdfa774866a980)
Conflicts:
* tests/qemuxml2argvtest.c
* tests/qemuxml2xmltest.c
- missing unrelated changes to surrounding code
* tests/qemuxml2argvdata/firmware-manual-efi-features.x86_64-latest.args
* tests/qemuxml2xmloutdata/firmware-manual-efi-features.x86_64-latest.xml
- had to be regenerated to account for differences in the
input file, as well as the test code and the behavior of
the firmware selection feature. In particular, the
reference to /bad-test-used-env-home/ is caused by the
fact that qemuxml2xmltest, unlike qemuxml2argvtest,
didn't set cfg->nvramDir before e62db9ee5b..e6c1ca3d11
https://bugzilla.redhat.com/show_bug.cgi?id=2186383
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
src/conf/domain_conf.c | 39 ++++++++++++++++++-
...are-manual-efi-features.x86_64-latest.args | 35 +++++++++++++++++
tests/qemuxml2argvtest.c | 6 ++-
...ware-manual-efi-features.x86_64-latest.xml | 32 +++++++++++++++
tests/qemuxml2xmltest.c | 1 +
5 files changed, 110 insertions(+), 3 deletions(-)
create mode 100644 tests/qemuxml2argvdata/firmware-manual-efi-features.x86_64-latest.args
create mode 100644 tests/qemuxml2xmloutdata/firmware-manual-efi-features.x86_64-latest.xml
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 733399e6da..89637bb282 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17021,11 +17021,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;
@@ -17033,6 +17035,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);
@@ -17161,7 +17196,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)
diff --git a/tests/qemuxml2argvdata/firmware-manual-efi-features.x86_64-latest.args b/tests/qemuxml2argvdata/firmware-manual-efi-features.x86_64-latest.args
new file mode 100644
index 0000000000..db6c6d06bc
--- /dev/null
+++ b/tests/qemuxml2argvdata/firmware-manual-efi-features.x86_64-latest.args
@@ -0,0 +1,35 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-test \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-test/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-test/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-test/.config \
+/usr/bin/qemu-system-x86_64 \
+-name guest=test,debug-threads=on \
+-S \
+-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-test/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/test_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,usb=off,dump-guest-core=off,memory-backend=pc.ram,pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-format \
+-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 362d1fc1-df7d-193e-5c18-49a71bd1da66 \
+-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
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 3fb2d5dc74..99392335b6 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1130,7 +1130,11 @@ mymain(void)
QEMU_CAPS_DEVICE_ISA_SERIAL);
DO_TEST_NOCAPS("firmware-manual-efi");
DO_TEST_PARSE_ERROR_NOCAPS("firmware-manual-efi-no-path");
- 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-bios-rw");
DO_TEST_CAPS_LATEST("firmware-manual-bios-rw-implicit");
DO_TEST("firmware-manual-efi-secure",
diff --git a/tests/qemuxml2xmloutdata/firmware-manual-efi-features.x86_64-latest.xml b/tests/qemuxml2xmloutdata/firmware-manual-efi-features.x86_64-latest.xml
new file mode 100644
index 0000000000..d142be9899
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/firmware-manual-efi-features.x86_64-latest.xml
@@ -0,0 +1,32 @@
+<domain type='qemu'>
+ <name>test</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>/bad-test-used-env-home/.config/libvirt/qemu/nvram/test_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>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 72f724bfce..66e038558f 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -936,6 +936,7 @@ mymain(void)
DO_TEST_NOCAPS("firmware-manual-bios");
DO_TEST_NOCAPS("firmware-manual-bios-stateless");
DO_TEST_NOCAPS("firmware-manual-efi");
+ DO_TEST_CAPS_LATEST("firmware-manual-efi-features");
DO_TEST_CAPS_LATEST("firmware-manual-efi-nvram-network-iscsi");
DO_TEST_CAPS_LATEST("firmware-manual-efi-nvram-network-nbd");
DO_TEST_CAPS_LATEST("firmware-manual-efi-nvram-file");
--
2.40.0