12:10 p.m.
On Thu, Mar 18, 2021 at 05:18:36PM +0100, Michal Privoznik wrote:
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(a)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?
Nice catch, the first one is leftover after rename, I'll drop it.
> 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()
Right, I'll change it before pushing.
Thanks,
Pavel