From eb1696222ceeac6f7ea739a03cc62b93490f5db7 Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Tue, 17 Dec 2019 17:45:50 +0100 Subject: [PATCH 3/3] qemu_firmware: Try to autofill for old style UEFI specification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While we discourage people to use the old style of specifying UEFI for their domains (the old style is putting path to the FW image under /domain/os/loader/ whilst the new one is using /domain/os/@firmware), some applications might have not adapted yet. They still rely on libvirt autofilling NVRAM path and figuring out NVRAM template when using the old way (notably virt-install does this). We must preserve backcompat for this previously supported config approach. However, since we really want distro maintainers to leave --with-loader-nvram configure option and rely on JSON descriptors, we need to implement autofilling of NVRAM template for the old way too. Fedora: https://bugzilla.redhat.com/show_bug.cgi?id=1782778 RHEL: https://bugzilla.redhat.com/show_bug.cgi?id=1776949 Signed-off-by: Michal Privoznik Reviewed-by: Daniel P. Berrangé --- src/qemu/qemu_firmware.c | 47 +++++++++++++++++++++++++++++++++++----- 1 file changed, 42 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c index c52e2f6592..4fa29ca447 100644 --- a/src/qemu/qemu_firmware.c +++ b/src/qemu/qemu_firmware.c @@ -961,6 +961,21 @@ qemuFirmwareMatchDomain(const virDomainDef *def, want = qemuFirmwareOSInterfaceTypeFromOsDefFirmware(def->os.firmware); + if (want == QEMU_FIRMWARE_OS_INTERFACE_NONE && + def->os.loader) { + want = qemuFirmwareOSInterfaceTypeFromOsDefFirmware(def->os.loader->type); + + if (fw->mapping.device != QEMU_FIRMWARE_DEVICE_FLASH || + STRNEQ(def->os.loader->path, fw->mapping.data.flash.executable.filename)) { + VIR_DEBUG("Not matching FW interface %s or loader " + "path '%s' for user provided path '%s'", + qemuFirmwareDeviceTypeToString(fw->mapping.device), + fw->mapping.data.flash.executable.filename, + def->os.loader->path); + return false; + } + } + for (i = 0; i < fw->ninterfaces; i++) { if (fw->interfaces[i] == want) break; @@ -1228,14 +1243,29 @@ qemuFirmwareFillDomain(virQEMUDriverPtr driver, qemuFirmwarePtr *firmwares = NULL; ssize_t nfirmwares = 0; const qemuFirmware *theone = NULL; + bool needResult = true; size_t i; int ret = -1; if (!(flags & VIR_QEMU_PROCESS_START_NEW)) return 0; - if (vm->def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_NONE) - return 0; + /* Fill in FW paths if either os.firmware is enabled, or + * loader path was provided with no nvram varstore. */ + if (vm->def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_NONE) { + /* This is horrific check, but loosely said, if UEFI + * image was provided by the old method (by specifying + * its path in domain XML) but no template for NVRAM was + * specified and the varstore doesn't exist ... */ + if (!virDomainDefHasOldStyleROUEFI(vm->def) || + vm->def->os.loader->templt || + virFileExists(vm->def->os.loader->nvram)) + return 0; + + /* ... then we want to consult JSON FW descriptors first, + * but we don't want to fail if we haven't found a match. */ + needResult = false; + } if ((nfirmwares = qemuFirmwareFetchParsedConfigs(driver->privileged, &firmwares, &paths)) < 0) @@ -1251,9 +1281,16 @@ qemuFirmwareFillDomain(virQEMUDriverPtr driver, } if (!theone) { - virReportError(VIR_ERR_OPERATION_FAILED, - _("Unable to find any firmware to satisfy '%s'"), - virDomainOsDefFirmwareTypeToString(vm->def->os.firmware)); + if (needResult) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("Unable to find any firmware to satisfy '%s'"), + virDomainOsDefFirmwareTypeToString(vm->def->os.firmware)); + } else { + VIR_DEBUG("Unable to find NVRAM template for '%s', " + "falling back to old style", + NULLSTR(vm->def->os.loader ? vm->def->os.loader->path : NULL)); + ret = 0; + } goto cleanup; } -- 2.24.1