On Thu, Mar 18, 2021 at 01:26:45PM +0100, 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.x86_64-latest.err
> create mode 100644 tests/qemuxml2argvdata/os-firmware-invalid-type.xml
> @@ -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);
> - if (!firmware)
> + if (!firmware && !type)
> return 0;
>
> - fw = virDomainOsDefFirmwareTypeFromString(firmware);
> + if (firmware && type && STRNEQ(firmware, type)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("firmware attribute and firmware type has to be the same"));
> + return -1;
> + }
Why do we need to introduce a second attribute that must be the same as the
attribute we already have ?
This feels like it introduces a error scenario that we don't really need to
have and is redundant data for the user.
> diff --git a/tests/qemuxml2xmloutdata/aarch64-os-firmware-efi.aarch64-latest.xml b/tests/qemuxml2xmloutdata/aarch64-os-firmware-efi.aarch64-latest.xml
> index 627e285ae1..cb4f3ccfce 100644
> --- a/tests/qemuxml2xmloutdata/aarch64-os-firmware-efi.aarch64-latest.xml
> +++ b/tests/qemuxml2xmloutdata/aarch64-os-firmware-efi.aarch64-latest.xml
> @@ -6,6 +6,7 @@
> <vcpu placement='static'>1</vcpu>
> <os firmware='efi'>
> <type arch='aarch64' machine='virt-4.0'>hvm</type>
> + <firmware type='efi'/>
This just looks odd having to set 'efi' twice.
> <kernel>/aarch64.kernel</kernel>
> <initrd>/aarch64.initrd</initrd>
> <cmdline>earlyprintk console=ttyAMA0,115200n8 rw root=/dev/vda rootwait</cmdline>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
