From a9b1375d7d2f7d240dce09c5f8b62e568e386051 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Mon, 29 Mar 2021 19:00:05 +0100 Subject: [PATCH] conf: remove duplicated firmware type attribute MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The repeats the firmware attribute twice. This has no functional benefit, as evidenced by fact that we use a single struct field to store both attributes, while needlessly introducing an error scenario. The XML can just be simplified to: which also means that we don't need to emit the empty element for all existing configs too. Reviewed-by: Pavel Hrdina Signed-off-by: Daniel P. Berrangé --- docs/formatdomain.rst | 8 --- docs/schemas/domaincommon.rng | 10 +--- src/conf/domain_conf.c | 50 ++++++------------- .../os-firmware-efi-no-enrolled-keys.xml | 2 +- .../os-firmware-invalid-type.xml | 28 ----------- tests/qemuxml2argvtest.c | 1 - ...aarch64-os-firmware-efi.aarch64-latest.xml | 1 - .../os-firmware-bios.x86_64-latest.xml | 1 - .../os-firmware-efi-secboot.x86_64-latest.xml | 1 - .../os-firmware-efi.x86_64-latest.xml | 1 - tests/vmx2xmldata/vmx2xml-firmware-efi.xml | 1 - 11 files changed, 19 insertions(+), 85 deletions(-) delete mode 100644 tests/qemuxml2argvdata/os-firmware-invalid-type.xml diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 9392c80113..741130bf21 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -158,14 +158,6 @@ harddisk, cdrom, network) determining where to obtain/find the boot image. ``firmware`` :since:`Since 7.2.0 QEMU/KVM only` - When used together with ``firmware`` attribute of ``os`` element the ``type`` - attribute must have the same value. - - List of mandatory attributes: - - - ``type`` (accepted values are ``bios`` and ``efi``) same as the ``firmware`` - attribute of ``os`` element. - When using firmware auto-selection there are different features enabled in the firmwares. The list of features can be used to limit what firmware should be automatically selected for the VM. The list of features can be specified diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 1dbfc68f18..f5ced5b7a2 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -278,13 +278,7 @@ - - - bios - efi - - - + @@ -296,7 +290,7 @@ - + diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b0eba9f7bd..d050a519c6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -19590,31 +19590,21 @@ virDomainDefParseBootFirmwareOptions(virDomainDefPtr def, xmlXPathContextPtr ctxt) { g_autofree char *firmware = virXPathString("string(./os/@firmware)", ctxt); - g_autofree char *type = virXPathString("string(./os/firmware/@type)", ctxt); g_autofree xmlNodePtr *nodes = NULL; g_autofree int *features = NULL; int fw = 0; int n = 0; size_t i; - if (!firmware && !type) + if (!firmware) return 0; - if (firmware && type && STRNEQ(firmware, type)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("firmware attribute and firmware type has to be the same")); - return -1; - } - - if (!type) - type = g_steal_pointer(&firmware); - - fw = virDomainOsDefFirmwareTypeFromString(type); + fw = virDomainOsDefFirmwareTypeFromString(firmware); if (fw <= 0) { virReportError(VIR_ERR_XML_ERROR, _("unknown firmware value %s"), - type); + firmware); return -1; } @@ -29491,30 +29481,22 @@ virDomainDefFormatInternalSetRootName(virDomainDefPtr def, virBufferAsprintf(buf, ">%s\n", virDomainOSTypeToString(def->os.type)); - if (def->os.firmware) { - virBufferAsprintf(buf, "os.firmware)); + if (def->os.firmwareFeatures) { + virBufferAddLit(buf, "\n"); + virBufferAdjustIndent(buf, 2); - if (def->os.firmwareFeatures) { - virBufferAddLit(buf, ">\n"); + for (i = 0; i < VIR_DOMAIN_OS_DEF_FIRMWARE_FEATURE_LAST; i++) { + if (def->os.firmwareFeatures[i] == VIR_TRISTATE_BOOL_ABSENT) + continue; - virBufferAdjustIndent(buf, 2); - - for (i = 0; i < VIR_DOMAIN_OS_DEF_FIRMWARE_FEATURE_LAST; i++) { - if (def->os.firmwareFeatures[i] == VIR_TRISTATE_BOOL_ABSENT) - continue; - - virBufferAsprintf(buf, "\n", - virTristateBoolTypeToString(def->os.firmwareFeatures[i]), - virDomainOsDefFirmwareFeatureTypeToString(i)); - } - - virBufferAdjustIndent(buf, -2); - - virBufferAddLit(buf, "\n"); - } else { - virBufferAddLit(buf, "/>\n"); + virBufferAsprintf(buf, "\n", + virTristateBoolTypeToString(def->os.firmwareFeatures[i]), + virDomainOsDefFirmwareFeatureTypeToString(i)); } + + virBufferAdjustIndent(buf, -2); + + virBufferAddLit(buf, "\n"); } virBufferEscapeString(buf, "%s\n", diff --git a/tests/qemuxml2argvdata/os-firmware-efi-no-enrolled-keys.xml b/tests/qemuxml2argvdata/os-firmware-efi-no-enrolled-keys.xml index 8944ce70bb..352908f745 100644 --- a/tests/qemuxml2argvdata/os-firmware-efi-no-enrolled-keys.xml +++ b/tests/qemuxml2argvdata/os-firmware-efi-no-enrolled-keys.xml @@ -6,7 +6,7 @@ 1 hvm - + diff --git a/tests/qemuxml2argvdata/os-firmware-invalid-type.xml b/tests/qemuxml2argvdata/os-firmware-invalid-type.xml deleted file mode 100644 index 41360df0f7..0000000000 --- a/tests/qemuxml2argvdata/os-firmware-invalid-type.xml +++ /dev/null @@ -1,28 +0,0 @@ - - fedora - 63840878-0deb-4095-97e6-fc444d9bc9fa - 8192 - 8192 - 1 - - hvm - - - /var/lib/libvirt/qemu/nvram/fedora_VARS.fd - - - - - - - - - - destroy - restart - restart - - /usr/bin/qemu-system-x86_64 - - - diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 12c2b68fd7..3439f34ef1 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -3582,7 +3582,6 @@ mymain(void) DO_TEST_CAPS_LATEST("os-firmware-efi"); DO_TEST_CAPS_LATEST("os-firmware-efi-secboot"); DO_TEST_CAPS_LATEST("os-firmware-efi-no-enrolled-keys"); - DO_TEST_CAPS_LATEST_PARSE_ERROR("os-firmware-invalid-type"); DO_TEST_CAPS_ARCH_LATEST("aarch64-os-firmware-efi", "aarch64"); DO_TEST_CAPS_LATEST("vhost-user-vga"); diff --git a/tests/qemuxml2xmloutdata/aarch64-os-firmware-efi.aarch64-latest.xml b/tests/qemuxml2xmloutdata/aarch64-os-firmware-efi.aarch64-latest.xml index cb4f3ccfce..627e285ae1 100644 --- a/tests/qemuxml2xmloutdata/aarch64-os-firmware-efi.aarch64-latest.xml +++ b/tests/qemuxml2xmloutdata/aarch64-os-firmware-efi.aarch64-latest.xml @@ -6,7 +6,6 @@ 1 hvm - /aarch64.kernel /aarch64.initrd earlyprintk console=ttyAMA0,115200n8 rw root=/dev/vda rootwait diff --git a/tests/qemuxml2xmloutdata/os-firmware-bios.x86_64-latest.xml b/tests/qemuxml2xmloutdata/os-firmware-bios.x86_64-latest.xml index 016c5b863f..df6f61421a 100644 --- a/tests/qemuxml2xmloutdata/os-firmware-bios.x86_64-latest.xml +++ b/tests/qemuxml2xmloutdata/os-firmware-bios.x86_64-latest.xml @@ -6,7 +6,6 @@ 1 hvm - /var/lib/libvirt/qemu/nvram/fedora_VARS.fd diff --git a/tests/qemuxml2xmloutdata/os-firmware-efi-secboot.x86_64-latest.xml b/tests/qemuxml2xmloutdata/os-firmware-efi-secboot.x86_64-latest.xml index fa5eaa3148..c383546cc6 100644 --- a/tests/qemuxml2xmloutdata/os-firmware-efi-secboot.x86_64-latest.xml +++ b/tests/qemuxml2xmloutdata/os-firmware-efi-secboot.x86_64-latest.xml @@ -6,7 +6,6 @@ 1 hvm - /var/lib/libvirt/qemu/nvram/fedora_VARS.fd diff --git a/tests/qemuxml2xmloutdata/os-firmware-efi.x86_64-latest.xml b/tests/qemuxml2xmloutdata/os-firmware-efi.x86_64-latest.xml index 382146c23b..04d57860e7 100644 --- a/tests/qemuxml2xmloutdata/os-firmware-efi.x86_64-latest.xml +++ b/tests/qemuxml2xmloutdata/os-firmware-efi.x86_64-latest.xml @@ -6,7 +6,6 @@ 1 hvm - /var/lib/libvirt/qemu/nvram/fedora_VARS.fd diff --git a/tests/vmx2xmldata/vmx2xml-firmware-efi.xml b/tests/vmx2xmldata/vmx2xml-firmware-efi.xml index fa10daf3a6..fee707fe71 100644 --- a/tests/vmx2xmldata/vmx2xml-firmware-efi.xml +++ b/tests/vmx2xmldata/vmx2xml-firmware-efi.xml @@ -6,7 +6,6 @@ 1 hvm - destroy