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>

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

docs/schemas/domaincommon.rng

@@ -278,13 +278,7 @@
         <ref name="ostypehvm"/>
         <optional>
           <element name="firmware">
-            <attribute name="type">
+            <oneOrMore>
-              <choice>
-                <value>bios</value>
-                <value>efi</value>
-              </choice>
-            </attribute>
-            <zeroOrMore>
               <element name="feature">
                 <attribute name="enabled">
                   <ref name="virYesNo"/>
@@ -296,7 +290,7 @@
                   </choice>
                 </attribute>
               </element>
-            </zeroOrMore>
+            </oneOrMore>
           </element>
         </optional>
         <optional>

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)) {
+    fw = virDomainOsDefFirmwareTypeFromString(firmware);
-        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);
 
     if (fw <= 0) {
         virReportError(VIR_ERR_XML_ERROR,
                        _("unknown firmware value %s"),
-                       type);
+                       firmware);
         return -1;
     }
 
@@ -29491,13 +29481,8 @@ 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");
+        virBufferAddLit(buf, "<firmware>\n");
-
         virBufferAdjustIndent(buf, 2);
 
         for (i = 0; i < VIR_DOMAIN_OS_DEF_FIRMWARE_FEATURE_LAST; i++) {
@@ -29512,9 +29497,6 @@ virDomainDefFormatInternalSetRootName(virDomainDefPtr def,
         virBufferAdjustIndent(buf, -2);
 
         virBufferAddLit(buf, "</firmware>\n");
-        } else {
-            virBufferAddLit(buf, "/>\n");
-        }
     }
 
     virBufferEscapeString(buf, "<init>%s</init>\n",

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'/>

tests/qemuxml2argvdata/os-firmware-invalid-type.xml

@@ -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>

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");

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>

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'/>

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'/>

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'/>

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>