From f9ad3023355bcbfc692bbe4997fdfa774866a980 Mon Sep 17 00:00:00 2001 From: Andrea Bolognani 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 Reviewed-by: Ján Tomko --- 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 | 36 +++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 5 files changed, 114 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 8df751cc46..4d446d7ec1 100644 --- a/src/conf/domain_conf.c +++ b/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 + * + * + * + * + * + * Notice how there's no firmware='foo' attribute for the + * element, meaning that firmware autoselection is disabled, and + * yet some 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 + * 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 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) 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..57c71542cd --- /dev/null +++ b/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 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index e055d372fa..1808d9fc02 100644 --- a/tests/qemuxml2argvtest.c +++ b/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"); 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..dc4b8bb97f --- /dev/null +++ b/tests/qemuxml2xmloutdata/firmware-manual-efi-features.x86_64-latest.xml @@ -0,0 +1,36 @@ + + guest + 63840878-0deb-4095-97e6-fc444d9bc9fa + 1048576 + 1048576 + 1 + + hvm + + + + + /usr/share/OVMF/OVMF_CODE.fd + /var/lib/libvirt/qemu/nvram/guest_VARS.fd + + + + + + + qemu64 + + + destroy + restart + destroy + + /usr/bin/qemu-system-x86_64 + + + + + + diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index e19aba0878..b1bc6e9216 100644 --- a/tests/qemuxml2xmltest.c +++ b/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");