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