Currently we're doing a weird dance to avoid overwriting the user-provided path to the NVRAM template, which might potentially be one we actually know about but just so happens not to be listed first. Explaining why we're doing things this way requires a fairly long comment. We can make things simpler: if the NVRAM template path is present in the domain XML, include it into the matching criteria. This is consistent with how we match firmware descriptors. Handling of format, both for the firmware executable and the NVRAM template, is improved too. Legacy paths were used before non-raw firmware builds existed, so we can set the format to raw unconditionally. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_firmware.c | 65 +++++++++++++++++++--------------------- 1 file changed, 30 insertions(+), 35 deletions(-) diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c index 2b16d66818..a7bb8f7e45 100644 --- a/src/qemu/qemu_firmware.c +++ b/src/qemu/qemu_firmware.c @@ -1653,6 +1653,7 @@ qemuFirmwareFillDomainLegacy(virQEMUDriver *driver, { g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); virDomainLoaderDef *loader = def->os.loader; + virFirmware *theone = NULL; size_t i; if (!loader) @@ -1681,6 +1682,13 @@ qemuFirmwareFillDomainLegacy(virQEMUDriver *driver, return 1; } + if (loader->nvramTemplateFormat && + loader->nvramTemplateFormat != VIR_STORAGE_FILE_RAW) { + VIR_DEBUG("Ignoring legacy entries for loader with nvram template format '%s'", + virStorageFileFormatTypeToString(loader->nvramTemplateFormat)); + return 1; + } + for (i = 0; i < cfg->nfirmwares; i++) { virFirmware *fw = cfg->firmwares[i]; @@ -1690,47 +1698,34 @@ qemuFirmwareFillDomainLegacy(virQEMUDriver *driver, continue; } - loader->type = VIR_DOMAIN_LOADER_TYPE_PFLASH; - loader->readonly = VIR_TRISTATE_BOOL_YES; - loader->format = VIR_STORAGE_FILE_RAW; - - /* Only use the default template path if one hasn't been - * provided by the user. Assume that the template is in 'raw' format. - * - * In addition to fully-custom templates, which are a valid - * use case, we could simply be in a situation where - * qemu.conf contains - * - * nvram = [ - * "/path/to/OVMF_CODE.secboot.fd:/path/to/OVMF_VARS.fd", - * "/path/to/OVMF_CODE.secboot.fd:/path/to/OVMF_VARS.secboot.fd" - * ] - * - * and the domain has been configured as - * - * <os> - * <loader readonly='yes' type='pflash'>/path/to/OVMF_CODE.secboot.fd</loader> - * <nvram template='/path/to/OVMF/OVMF_VARS.secboot.fd'> - * </os> - * - * In this case, the global default is to have Secure Boot - * disabled, but the domain configuration explicitly enables - * it, and we shouldn't overrule this choice */ - if (!loader->nvramTemplate) { - loader->nvramTemplate = g_strdup(cfg->firmwares[i]->nvram); - loader->nvramTemplateFormat = VIR_STORAGE_FILE_RAW; + if (loader->nvramTemplate && + !virFileComparePaths(fw->nvram, loader->nvramTemplate)) { + VIR_DEBUG("Not matching nvram template path '%s' for user provided path '%s'", + fw->nvram, loader->nvramTemplate); + continue; } - if (loader->nvramTemplateFormat == VIR_STORAGE_FILE_NONE) - loader->nvramTemplateFormat = VIR_STORAGE_FILE_RAW; + theone = fw; + break; + } - VIR_DEBUG("decided on firmware '%s' template '%s'", - loader->path, NULLSTR(loader->nvramTemplate)); + if (!theone) + return 1; - return 0; + loader->type = VIR_DOMAIN_LOADER_TYPE_PFLASH; + loader->readonly = VIR_TRISTATE_BOOL_YES; + + loader->format = VIR_STORAGE_FILE_RAW; + loader->nvramTemplateFormat = VIR_STORAGE_FILE_RAW; + + if (!loader->nvramTemplate) { + loader->nvramTemplate = g_strdup(theone->nvram); } - return 1; + VIR_DEBUG("decided on firmware '%s' template '%s'", + loader->path, loader->nvramTemplate); + + return 0; } -- 2.52.0