From 4ca3f2f590fb860b01f1eb5fec8929ceba702dc6 Mon Sep 17 00:00:00 2001
Message-Id: <4ca3f2f590fb860b01f1eb5fec8929ceba702dc6@dist-git>
From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= <berrange@redhat.com>
Date: Fri, 21 May 2021 14:16:14 +0200
Subject: [PATCH] conf: remove duplicated firmware type attribute
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The
<os firmware='efi'>
<firmware type='efi'>
<feature enabled='no' name='enrolled-keys'/>
</firmware>
</os>
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:
<os firmware='efi'>
<firmware>
<feature enabled='no' name='enrolled-keys'/>
</firmware>
</os>
which also means that we don't need to emit the empty element
<firmware type='efi'/> for all existing configs too.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
(cherry picked from commit a9b1375d7d2f7d240dce09c5f8b62e568e386051)
Conflicts:
docs/formatdomain.rst
- we still have formatdomain.html.in in downstream
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1929357
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Message-Id: <299fd16fc3ce632bf25ca55cc4bb65a225437d61.1621599207.git.phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
---
docs/formatdomain.html.in | 15 ------
docs/schemas/domaincommon.rng | 10 +---
src/conf/domain_conf.c | 48 ++++++-------------
.../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, 18 insertions(+), 91 deletions(-)
delete mode 100644 tests/qemuxml2argvdata/os-firmware-invalid-type.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 11f31618af..79e2e51c54 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -183,21 +183,6 @@
<dt><a id="elementFirmware"><code>firmware</code></a></dt>
<dd>
<p><span class="since">Since 7.2.0 QEMU/KVM only</span></p>
- <p>
- When used together with <code>firmware</code> attribute of
- <code>os</code> element the <code>type</code> attribute must
- have the same value.
- </p>
- <p>
- List of mandatory attributes:
- <ul>
- <li>
- <code>type</code> (accepted values are <code>bios</code>
- and <code>efi</code>) same as the <code>firmware</code>
- attribute of <code>os</code> element.
- </li>
- </ul>
- </p>
<p>
When using firmware auto-selection there are different features
enabled in the firmwares. The list of features can be used to
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index b7f6a6b494..ec8167e588 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -270,13 +270,7 @@
<ref name="ostypehvm"/>
<optional>
<element name="firmware">
- <attribute name="type">
- <choice>
- <value>bios</value>
- <value>efi</value>
- </choice>
- </attribute>
- <zeroOrMore>
+ <oneOrMore>
<element name="feature">
<attribute name="enabled">
<ref name="virYesNo"/>
@@ -288,7 +282,7 @@
</choice>
</attribute>
</element>
- </zeroOrMore>
+ </oneOrMore>
</element>
</optional>
<optional>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2ffa9c8a2a..6806064016 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -19389,31 +19389,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;
}
@@ -29039,30 +29029,22 @@ virDomainDefFormatInternalSetRootName(virDomainDefPtr def,
virBufferAsprintf(buf, ">%s</type>\n",
virDomainOSTypeToString(def->os.type));
- if (def->os.firmware) {
- virBufferAsprintf(buf, "<firmware type='%s'",
- virDomainOsDefFirmwareTypeToString(def->os.firmware));
-
- if (def->os.firmwareFeatures) {
- virBufferAddLit(buf, ">\n");
-
- virBufferAdjustIndent(buf, 2);
+ if (def->os.firmwareFeatures) {
+ virBufferAddLit(buf, "<firmware>\n");
+ 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;
+ for (i = 0; i < VIR_DOMAIN_OS_DEF_FIRMWARE_FEATURE_LAST; i++) {
+ if (def->os.firmwareFeatures[i] == VIR_TRISTATE_BOOL_ABSENT)
+ continue;
- virBufferAsprintf(buf, "<feature enabled='%s' name='%s'/>\n",
- virTristateBoolTypeToString(def->os.firmwareFeatures[i]),
- virDomainOsDefFirmwareFeatureTypeToString(i));
- }
+ virBufferAsprintf(buf, "<feature enabled='%s' name='%s'/>\n",
+ virTristateBoolTypeToString(def->os.firmwareFeatures[i]),
+ virDomainOsDefFirmwareFeatureTypeToString(i));
+ }
- virBufferAdjustIndent(buf, -2);
+ virBufferAdjustIndent(buf, -2);
- virBufferAddLit(buf, "</firmware>\n");
- } else {
- virBufferAddLit(buf, "/>\n");
- }
+ virBufferAddLit(buf, "</firmware>\n");
}
virBufferEscapeString(buf, "<init>%s</init>\n",
diff --git a/tests/qemuxml2argvdata/os-firmware-efi-no-enrolled-keys.xml b/tests/qemuxml2argvdata/os-firmware-efi-no-enrolled-keys.xml
index 7f8f57a859..4999c4f125 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 @@
<vcpu placement='static'>1</vcpu>
<os firmware='efi'>
<type arch='x86_64' machine='pc-q35-4.0'>hvm</type>
- <firmware type='efi'>
+ <firmware>
<feature enabled='no' name='enrolled-keys'/>
</firmware>
<boot dev='hd'/>
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 @@
-<domain type='kvm'>
- <name>fedora</name>
- <uuid>63840878-0deb-4095-97e6-fc444d9bc9fa</uuid>
- <memory unit='KiB'>8192</memory>
- <currentMemory unit='KiB'>8192</currentMemory>
- <vcpu placement='static'>1</vcpu>
- <os firmware='efi'>
- <type arch='x86_64' machine='pc-q35-4.0'>hvm</type>
- <firmware type='bios'/>
- <loader secure='no'/>
- <nvram>/var/lib/libvirt/qemu/nvram/fedora_VARS.fd</nvram>
- <boot dev='hd'/>
- <bootmenu enable='yes'/>
- </os>
- <features>
- <acpi/>
- <apic/>
- <pae/>
- </features>
- <clock offset='utc'/>
- <on_poweroff>destroy</on_poweroff>
- <on_reboot>restart</on_reboot>
- <on_crash>restart</on_crash>
- <devices>
- <emulator>/usr/bin/qemu-system-x86_64</emulator>
- <memballoon model='none'/>
- </devices>
-</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 5e16d7fd31..be8054fa6a 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -3095,7 +3095,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 3cac8fc5c6..1e51d55305 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 @@
<vcpu placement='static'>1</vcpu>
<os firmware='efi'>
<type arch='aarch64' machine='virt-4.0'>hvm</type>
- <firmware type='efi'/>
<kernel>/aarch64.kernel</kernel>
<initrd>/aarch64.initrd</initrd>
<cmdline>earlyprintk console=ttyAMA0,115200n8 rw root=/dev/vda rootwait</cmdline>
diff --git a/tests/qemuxml2xmloutdata/os-firmware-bios.x86_64-latest.xml b/tests/qemuxml2xmloutdata/os-firmware-bios.x86_64-latest.xml
index ef24f2fece..60d3498765 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 @@
<vcpu placement='static'>1</vcpu>
<os firmware='bios'>
<type arch='x86_64' machine='pc-q35-4.0'>hvm</type>
- <firmware type='bios'/>
<loader secure='no'/>
<nvram>/var/lib/libvirt/qemu/nvram/fedora_VARS.fd</nvram>
<boot dev='hd'/>
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 3757191e8e..938da73711 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 @@
<vcpu placement='static'>1</vcpu>
<os firmware='efi'>
<type arch='x86_64' machine='pc-q35-4.0'>hvm</type>
- <firmware type='efi'/>
<loader secure='yes'/>
<nvram>/var/lib/libvirt/qemu/nvram/fedora_VARS.fd</nvram>
<boot dev='hd'/>
diff --git a/tests/qemuxml2xmloutdata/os-firmware-efi.x86_64-latest.xml b/tests/qemuxml2xmloutdata/os-firmware-efi.x86_64-latest.xml
index f2e6b7f36d..97ce8a75c7 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 @@
<vcpu placement='static'>1</vcpu>
<os firmware='efi'>
<type arch='x86_64' machine='pc-q35-4.0'>hvm</type>
- <firmware type='efi'/>
<loader secure='no'/>
<nvram>/var/lib/libvirt/qemu/nvram/fedora_VARS.fd</nvram>
<boot dev='hd'/>
diff --git a/tests/vmx2xmldata/vmx2xml-firmware-efi.xml b/tests/vmx2xmldata/vmx2xml-firmware-efi.xml
index 375c47d281..e21158cebf 100644
--- a/tests/vmx2xmldata/vmx2xml-firmware-efi.xml
+++ b/tests/vmx2xmldata/vmx2xml-firmware-efi.xml
@@ -5,7 +5,6 @@
<vcpu placement='static'>1</vcpu>
<os firmware='efi'>
<type arch='i686'>hvm</type>
- <firmware type='efi'/>
</os>
<clock offset='utc'/>
<on_poweroff>destroy</on_poweroff>
--
2.31.1