[libvirt PATCH] conf: remove duplicated firmware type attribute
Daniel P. Berrangé
berrange at redhat.com
Mon Mar 29 18:03:52 UTC 2021
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.
Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
---
docs/formatdomain.rst | 8 ----
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(+), 84 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 @@
<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"/>
@@ -296,7 +290,7 @@
</choice>
</attribute>
</element>
- </zeroOrMore>
+ </oneOrMore>
</element>
</optional>
<optional>
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</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 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 @@
<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 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 @@
<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 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 @@
<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 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 @@
<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 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 @@
<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 fa10daf3a6..fee707fe71 100644
--- a/tests/vmx2xmldata/vmx2xml-firmware-efi.xml
+++ b/tests/vmx2xmldata/vmx2xml-firmware-efi.xml
@@ -6,7 +6,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.30.2
More information about the libvir-list
mailing list