Stateful ROMs are those that use the uefi-vars QEMU device to implement access to UEFI variable storage. Matching works much the same as it does for pflash-based firmware images. Notably, the <varstore> element is only allowed for ROM and the <nvram> element is only allowed for pflash. The firmware-auto-efi-varstore-q35 and firmware-auto-efi-varstore-aarch64 fail in a different way after this change: the input XML is now considered valid, and the only remaining issue is that the firmware autoselection process is unable to find a match. Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Acked-by: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> --- src/qemu/qemu_firmware.c | 40 +++++++++++++++++-- ...to-efi-varstore-aarch64.aarch64-latest.err | 2 +- ...to-efi-varstore-aarch64.aarch64-latest.xml | 28 +++++++++++++ ...re-auto-efi-varstore-q35.x86_64-latest.err | 2 +- ...re-auto-efi-varstore-q35.x86_64-latest.xml | 36 +++++++++++++++++ tests/qemuxmlconftest.c | 4 +- 6 files changed, 104 insertions(+), 8 deletions(-) create mode 100644 tests/qemuxmlconfdata/firmware-auto-efi-varstore-aarch64.aarch64-latest.xml create mode 100644 tests/qemuxmlconfdata/firmware-auto-efi-varstore-q35.x86_64-latest.xml diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c index 1851ed4a80..60635b559f 100644 --- a/src/qemu/qemu_firmware.c +++ b/src/qemu/qemu_firmware.c @@ -893,15 +893,18 @@ qemuFirmwareMatchesMachineArch(const qemuFirmware *fw, * qemuFirmwareMatchesPaths: * @fw: firmware definition * @loader: loader definition + * @varstore: varstore definition * * Checks whether @fw is compatible with the information provided as * part of the domain definition. * - * Returns: true if @fw is compatible with @loader, false otherwise + * Returns: true if @fw is compatible with @loader and @varstore, + * false otherwise */ static bool qemuFirmwareMatchesPaths(const qemuFirmware *fw, - const virDomainLoaderDef *loader) + const virDomainLoaderDef *loader, + const virDomainVarstoreDef *varstore) { const qemuFirmwareMappingFlash *flash = &fw->mapping.data.flash; const qemuFirmwareMappingMemory *memory = &fw->mapping.data.memory; @@ -922,6 +925,9 @@ qemuFirmwareMatchesPaths(const qemuFirmware *fw, if (loader && loader->path && !virFileComparePaths(loader->path, memory->filename)) return false; + if (varstore && varstore->template && + !virFileComparePaths(varstore->template, memory->template)) + return false; break; case QEMU_FIRMWARE_DEVICE_NONE: case QEMU_FIRMWARE_DEVICE_LAST: @@ -1112,6 +1118,7 @@ qemuFirmwareMatchDomain(const virDomainDef *def, const char *path) { const virDomainLoaderDef *loader = def->os.loader; + const virDomainVarstoreDef *varstore = def->os.varstore; size_t i; qemuFirmwareOSInterface want; bool wantUEFI = false; @@ -1166,7 +1173,7 @@ qemuFirmwareMatchDomain(const virDomainDef *def, return false; } - if (!qemuFirmwareMatchesPaths(fw, def->os.loader)) { + if (!qemuFirmwareMatchesPaths(fw, def->os.loader, def->os.varstore)) { VIR_DEBUG("No matching path in '%s'", path); return false; } @@ -1279,6 +1286,9 @@ qemuFirmwareMatchDomain(const virDomainDef *def, if (fw->mapping.device == QEMU_FIRMWARE_DEVICE_FLASH) { const qemuFirmwareMappingFlash *flash = &fw->mapping.data.flash; + if (varstore) + return false; + if (loader && loader->type && loader->type != VIR_DOMAIN_LOADER_TYPE_PFLASH) { VIR_DEBUG("Discarding flash loader"); @@ -1377,16 +1387,38 @@ qemuFirmwareMatchDomain(const virDomainDef *def, } } } else if (fw->mapping.device == QEMU_FIRMWARE_DEVICE_MEMORY) { + const qemuFirmwareMappingMemory *memory = &fw->mapping.data.memory; + + if (loader && loader->nvram) + return false; + if (loader && loader->type && loader->type != VIR_DOMAIN_LOADER_TYPE_ROM) { VIR_DEBUG("Discarding rom loader"); return false; } - if (loader && loader->stateless == VIR_TRISTATE_BOOL_NO) { + /* Explicit requests for either a stateless or stateful + * firmware should be fulfilled, but if no preference is + * provided either one is fine as long as the other match + * criteria are satisfied. varstore implies stateful */ + if (loader && + loader->stateless == VIR_TRISTATE_BOOL_NO && + !memory->template) { + VIR_DEBUG("Discarding stateless loader"); + return false; + } + if (varstore && + !memory->template) { VIR_DEBUG("Discarding stateless loader"); return false; } + if (loader && + loader->stateless == VIR_TRISTATE_BOOL_YES && + memory->template) { + VIR_DEBUG("Discarding non-stateless loader"); + return false; + } if (loader && loader->readonly == VIR_TRISTATE_BOOL_NO) { VIR_DEBUG("Discarding readonly loader"); diff --git a/tests/qemuxmlconfdata/firmware-auto-efi-varstore-aarch64.aarch64-latest.err b/tests/qemuxmlconfdata/firmware-auto-efi-varstore-aarch64.aarch64-latest.err index b45d304221..3edb2b3451 100644 --- a/tests/qemuxmlconfdata/firmware-auto-efi-varstore-aarch64.aarch64-latest.err +++ b/tests/qemuxmlconfdata/firmware-auto-efi-varstore-aarch64.aarch64-latest.err @@ -1 +1 @@ -Only one of NVRAM/varstore can be used +operation failed: Unable to find 'efi' firmware that is compatible with the current configuration diff --git a/tests/qemuxmlconfdata/firmware-auto-efi-varstore-aarch64.aarch64-latest.xml b/tests/qemuxmlconfdata/firmware-auto-efi-varstore-aarch64.aarch64-latest.xml new file mode 100644 index 0000000000..b0fa092509 --- /dev/null +++ b/tests/qemuxmlconfdata/firmware-auto-efi-varstore-aarch64.aarch64-latest.xml @@ -0,0 +1,28 @@ +<domain type='kvm'> + <name>guest</name> + <uuid>63840878-0deb-4095-97e6-fc444d9bc9fa</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os firmware='efi'> + <type arch='aarch64' machine='virt-8.2'>hvm</type> + <loader format='raw'/> + <varstore/> + <boot dev='hd'/> + </os> + <features> + <acpi/> + <gic version='2'/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <controller type='usb' index='0' model='none'/> + <controller type='pci' index='0' model='pcie-root'/> + <audio id='1' type='none'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxmlconfdata/firmware-auto-efi-varstore-q35.x86_64-latest.err b/tests/qemuxmlconfdata/firmware-auto-efi-varstore-q35.x86_64-latest.err index b45d304221..3edb2b3451 100644 --- a/tests/qemuxmlconfdata/firmware-auto-efi-varstore-q35.x86_64-latest.err +++ b/tests/qemuxmlconfdata/firmware-auto-efi-varstore-q35.x86_64-latest.err @@ -1 +1 @@ -Only one of NVRAM/varstore can be used +operation failed: Unable to find 'efi' firmware that is compatible with the current configuration diff --git a/tests/qemuxmlconfdata/firmware-auto-efi-varstore-q35.x86_64-latest.xml b/tests/qemuxmlconfdata/firmware-auto-efi-varstore-q35.x86_64-latest.xml new file mode 100644 index 0000000000..c4d70c9fc5 --- /dev/null +++ b/tests/qemuxmlconfdata/firmware-auto-efi-varstore-q35.x86_64-latest.xml @@ -0,0 +1,36 @@ +<domain type='kvm'> + <name>guest</name> + <uuid>63840878-0deb-4095-97e6-fc444d9bc9fa</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os firmware='efi'> + <type arch='x86_64' machine='pc-q35-8.2'>hvm</type> + <loader format='raw'/> + <varstore/> + <boot dev='hd'/> + </os> + <features> + <acpi/> + </features> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='usb' index='0' model='none'/> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pcie-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <watchdog model='itco' action='reset'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c index ca00dc96f0..43e994fc93 100644 --- a/tests/qemuxmlconftest.c +++ b/tests/qemuxmlconftest.c @@ -1673,8 +1673,8 @@ mymain(void) DO_TEST_CAPS_LATEST("firmware-auto-efi-nvram-file"); DO_TEST_CAPS_LATEST("firmware-auto-efi-nvram-network-nbd"); DO_TEST_CAPS_LATEST("firmware-auto-efi-nvram-network-iscsi"); - DO_TEST_CAPS_LATEST_PARSE_ERROR("firmware-auto-efi-varstore-q35"); - DO_TEST_CAPS_ARCH_LATEST_PARSE_ERROR("firmware-auto-efi-varstore-aarch64", "aarch64"); + DO_TEST_CAPS_LATEST_FAILURE("firmware-auto-efi-varstore-q35"); + DO_TEST_CAPS_ARCH_LATEST_FAILURE("firmware-auto-efi-varstore-aarch64", "aarch64"); DO_TEST_CAPS_LATEST("firmware-auto-efi-format-loader-qcow2"); DO_TEST_CAPS_LATEST_PARSE_ERROR("firmware-auto-efi-format-loader-qcow2-rom"); -- 2.53.0