The canonical order for <os> child elements is <firmware> then <loader>. Signed-off-by: Andrea Bolognani <abologna(a)redhat.com&gt; --- tests/qemuxml2argvdata/firmware-manual-efi-features.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemuxml2argvdata/firmware-manual-efi-features.xml b/tests/qemuxml2argvdata/firmware-manual-efi-features.xml index 8e5aff7559..092739af56 100644 --- a/tests/qemuxml2argvdata/firmware-manual-efi-features.xml +++ b/tests/qemuxml2argvdata/firmware-manual-efi-features.xml @@ -5,10 +5,10 @@ <vcpu placement='static'>1</vcpu> <os> <type arch='x86_64' machine='pc-i440fx-4.0'>hvm</type> - <loader readonly='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE.fd</loader> <firmware> <feature enabled='no' name='enrolled-keys'/> </firmware> + <loader readonly='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE.fd</loader> </os> <features> <acpi/> -- 2.39.2

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(a)redhat.com&gt; --- 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 70e4d52ee6..bc20acf103 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17210,11 +17210,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; @@ -17222,6 +17224,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); @@ -17350,7 +17385,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 @@ +<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> 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"); -- 2.39.2