conf: remove duplicated firmware type attribute

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>

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 9392c80113d62314298ae88388cd9807d637d760..741130bf21c9617f917e49cac6974ba2b3138737 100644 (file)
--- 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 1dbfc68f182b45ff258858b89a71a54b3eb1b711..f5ced5b7a2f661929e3034a0412b5f05a25e246b 100644 (file)
--- 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 b0eba9f7bd8f86431bee4423574a74f0eb59300a..d050a519c690edc85c0bbda962a6e8661ef10761 100644 (file)
--- 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 8944ce70bb7d8c381cb44a4c9463e6bdb2af5118..352908f74561ccaaf3d570537e2c743b2421d31c 100644 (file)
--- 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 (file)
index 41360df..0000000
--- 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 12c2b68fd7b9e7b7f29c08cbfe61e2986817be7a..3439f34ef182d57507820476102d804c699a91b5 100644 (file)
--- 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 cb4f3ccfceeff01b3cf9df726b13782129974113..627e285ae1e0128bd326023f07b006c447b6368e 100644 (file)
--- 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 016c5b863f51b8c2d7a91a6dd915a567293e46f9..df6f61421a1625dba7bf6346334b949b596f8d24 100644 (file)
--- 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 fa5eaa3148a8e1ff11c6b7a0bac1bf2c310f2fa8..c383546cc6c361e990efc288ff3664e2755cc93e 100644 (file)
--- 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 382146c23bd18d509f5108278e6fbc41c9eaad04..04d57860e7752d18633c1c0010956ee5a2d7c43c 100644 (file)
--- 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 fa10daf3a69c154ff53d21c101d529b4a385be8d..fee707fe71af6c0c4f5af3076b31e58a55c74aed 100644 (file)
--- 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>