Re: [libvirt PATCH 8/9] conf: introduce support for firmware auto-selection feature filtering

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux