On 3/18/21 1:26 PM, Pavel Hrdina wrote:
When the firmware auto-selection was introduced it always picked first
usable firmware based on the JSON descriptions on the host. It is
possible to add/remove/change the JSON files but it will always be for
the whole host.

This patch introduces support for configuring the auto-selection per VM
by adding users an option to limit what features they would like to have
available in the firmware.

Signed-off-by: Pavel Hrdina <phrd...@redhat.com>
---
  docs/formatdomain.rst                         | 31 +++++++
  docs/schemas/domaincommon.rng                 | 23 +++++
  src/conf/domain_conf.c                        | 83 ++++++++++++++++++-
  src/conf/domain_conf.h                        | 10 +++
  .../os-firmware-efi-invalid-type.xml          | 28 +++++++
  ...os-firmware-invalid-type.x86_64-latest.err |  1 +
  .../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 +
  13 files changed, 207 insertions(+), 3 deletions(-)

  create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-invalid-type.xml
>   create mode 100644 tests/qemuxml2argvdata/os-firmware-invalid-type.xml

These two are identical. Have you intended them to be different?

  create mode 100644 
tests/qemuxml2argvdata/os-firmware-invalid-type.x86_64-latest.err


diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index c101d5a1f1..dd063b0794 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -155,6 +155,37 @@ harddisk, cdrom, network) determining where to obtain/find 
the boot image.
     the host native arch will be chosen. For the ``test``, ``ESX`` and 
``VMWare``
     hypervisor drivers, however, the ``i686`` arch will always be chosen even 
on
     an ``x86_64`` host. :since:`Since 0.0.1`
+``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
+   using zero or more ``feature`` elements. Libvirt will take into 
consideration
+   only the listed features and ignore the rest when selecting the firmware.
+
+   ``feature``
+      The list of mandatory attributes:
+
+      - ``enabled`` (accepted values are ``yes`` and ``no``) is used to tell 
libvirt
+        if the feature must be enabled or not in the automatically selected 
firmware
+
+      - ``name`` the name of the feature, the list of the features:
+
+        - ``enrolled-keys`` whether the selected nvram template has default
+          certificate enrolled. Firmware with Secure Boot feature but without
+          enrolled keys will successfully boot non-signed binaries as well.
+          Valid only for firmwares with Secure Boot feature.
+
+        - ``secure-boot`` whether the firmware implements UEFI Secure boot 
feature.
  ``loader``
     The optional ``loader`` tag refers to a firmware blob, which is specified 
by
     absolute path, used to assist the domain creation process. It is used by 
Xen
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index e6db2f5b74..1dbfc68f18 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -276,6 +276,29 @@
            </attribute>
          </optional>
          <ref name="ostypehvm"/>
+        <optional>
+          <element name="firmware">
+            <attribute name="type">
+              <choice>
+                <value>bios</value>
+                <value>efi</value>
+              </choice>
+            </attribute>
+            <zeroOrMore>
+              <element name="feature">
+                <attribute name="enabled">
+                  <ref name="virYesNo"/>
+                </attribute>
+                <attribute name="name">
+                  <choice>
+                    <value>enrolled-keys</value>
+                    <value>secure-boot</value>
+                  </choice>
+                </attribute>
+              </element>
+            </zeroOrMore>
+          </element>
+        </optional>
          <optional>
            <element name="loader">
              <optional>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7729333897..dcfe5c0d03 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1318,6 +1318,12 @@ VIR_ENUM_IMPL(virDomainOsDefFirmware,
                "efi",
  );
+VIR_ENUM_IMPL(virDomainOsDefFirmwareFeature,
+              VIR_DOMAIN_OS_DEF_FIRMWARE_FEATURE_LAST,
+              "enrolled-keys",
+              "secure-boot",
+);
+
  VIR_ENUM_IMPL(virDomainCFPC,
                VIR_DOMAIN_CFPC_LAST,
                "none",
@@ -19600,22 +19606,67 @@ 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)
+    if (!firmware && !type)
          return 0;
- fw = virDomainOsDefFirmwareTypeFromString(firmware);
+    if (firmware && type && STRNEQ(firmware, type)) {

Or STRNEQ_NULLABLE()

+        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"),
-                       firmware);
+                       type);
          return -1;
      }
def->os.firmware = fw; + if ((n = virXPathNodeSet("./os/firmware/feature", ctxt, &nodes)) < 0)
+        return -1;
+
+    if (n > 0)
+        features = g_new0(int, VIR_DOMAIN_OS_DEF_FIRMWARE_FEATURE_LAST);
+
+    for (i = 0; i < n; i++) {
+        g_autofree char *name = virXMLPropString(nodes[i], "name");
+        g_autofree char *enabled = virXMLPropString(nodes[i], "enabled");
+        int feature = virDomainOsDefFirmwareFeatureTypeFromString(name);
+        int val = virTristateBoolTypeFromString(enabled);
+
+        if (feature < 0) {
+            virReportError(VIR_ERR_XML_ERROR,
+                           _("invalid firmware feature name '%s'"),
+                           name);
+            return -1;
+        }
+
+        if (val < 0) {
+            virReportError(VIR_ERR_XML_ERROR,
+                           _("invalid firmware feature enabled value '%s'"),
+                           enabled);
+            return -1;
+        }
+
+        features[feature] = val;
+    }
+
+    def->os.firmwareFeatures = g_steal_pointer(&features);
+
      return 0;
  }

Michal

Reply via email to