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@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.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 :|