Fixing the transient definition bug in a0d2ec63b99c4e0f500d6b61a208be48b3e3b396 exposed another issue: the startup filled firmware data is only available in the live definition. Thus, after shutdown, there is no information about the VARS file, so `virsh undefine --nvram` cannot remove it.
Fix that by filling firmware data in the domain XML post-parse stage. Signed-off-by: Roman Bogorodskiy <[email protected]> --- src/bhyve/bhyve_domain.c | 11 ++++++++++- src/bhyve/bhyve_driver.c | 10 +++++++--- src/bhyve/bhyve_firmware.c | 12 ++++++------ src/bhyve/bhyve_firmware.h | 2 +- src/bhyve/bhyve_process.c | 4 +++- .../three_firmwares/BHYVE_UEFI_VARS.fd | 0 .../x86_64/bhyvexml2xmlout-numa.xml | 2 ++ tests/bhyvexml2xmltest.c | 10 +++++++++- 8 files changed, 38 insertions(+), 13 deletions(-) create mode 100644 tests/bhyvefirmwaredata/three_firmwares/BHYVE_UEFI_VARS.fd diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c index 3b5a9b47a3..08c4156f2b 100644 --- a/src/bhyve/bhyve_domain.c +++ b/src/bhyve/bhyve_domain.c @@ -21,6 +21,7 @@ #include <config.h> +#include "bhyve_firmware.h" #include "bhyve_driver.h" #include "bhyve_conf.h" #include "bhyve_device.h" @@ -119,7 +120,7 @@ bhyveDomainDefNeedsISAController(virDomainDef *def) static int bhyveDomainDefPostParse(virDomainDef *def, - unsigned int parseFlags G_GNUC_UNUSED, + unsigned int parseFlags, void *opaque, void *parseOpaque G_GNUC_UNUSED) { @@ -130,6 +131,8 @@ bhyveDomainDefPostParse(virDomainDef *def, size_t virtio_serial_controllers = 0; size_t virtio_serial_existing_controllers = 0; size_t virtio_serial_controllers_to_create = 0; + bool abiUpdate = !!(parseFlags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE); + if (!caps) return -1; @@ -203,6 +206,12 @@ bhyveDomainDefPostParse(virDomainDef *def, } } + if (bhyveFirmwareFillDomain(driver, def, abiUpdate) < 0) { + if (abiUpdate) + return -1; + virResetLastError(); + } + return 0; } diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 2c4ec911a7..41bcf9b559 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -541,7 +541,8 @@ bhyveDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flag virDomainObj *vm = NULL; virObjectEvent *event = NULL; g_autoptr(virCaps) caps = NULL; - unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE | + VIR_DOMAIN_DEF_PARSE_ABI_UPDATE; virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); @@ -760,7 +761,9 @@ bhyveConnectDomainXMLToNative(virConnectPtr conn, } if (!(def = virDomainDefParseString(xmlData, privconn->xmlopt, - NULL, VIR_DOMAIN_DEF_PARSE_INACTIVE))) + NULL, + VIR_DOMAIN_DEF_PARSE_INACTIVE | + VIR_DOMAIN_DEF_PARSE_ABI_UPDATE))) return NULL; if (bhyveDomainAssignAddresses(def, NULL) < 0) @@ -951,7 +954,8 @@ bhyveDomainCreateXML(virConnectPtr conn, virDomainObj *vm = NULL; virObjectEvent *event = NULL; unsigned int start_flags = 0; - unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE | + VIR_DOMAIN_DEF_PARSE_ABI_UPDATE; virCheckFlags(VIR_DOMAIN_START_AUTODESTROY | VIR_DOMAIN_START_VALIDATE, NULL); diff --git a/src/bhyve/bhyve_firmware.c b/src/bhyve/bhyve_firmware.c index 54e3ce296a..9b8eb98ca6 100644 --- a/src/bhyve/bhyve_firmware.c +++ b/src/bhyve/bhyve_firmware.c @@ -41,7 +41,8 @@ VIR_LOG_INIT("bhyve.bhyve_firmware"); static void bhyveFirmwareEnsureNVRAM(virDomainDef *def, - bhyveConn *driver) + bhyveConn *driver, + bool abiUpdate) { g_autoptr(virBhyveDriverConfig) cfg = virBhyveDriverGetConfig(driver); virDomainLoaderDef *loader = def->os.loader; @@ -98,7 +99,8 @@ bhyveFirmwareEnsureNVRAM(virDomainDef *def, * * If we're loading an existing domain, however, we need to * stick with the .fd extension to ensure compatibility */ - if (loader->nvramTemplate && + if (abiUpdate && + loader->nvramTemplate && virStringHasSuffix(loader->nvramTemplate, ".raw")) ext = ".raw"; else @@ -112,7 +114,7 @@ bhyveFirmwareEnsureNVRAM(virDomainDef *def, int bhyveFirmwareFillDomain(bhyveConn *driver, virDomainDef *def, - unsigned int flags) + bool abiUpdate) { g_autoptr(DIR) dir = NULL; g_autoptr(virBhyveDriverConfig) cfg = virBhyveDriverGetConfig(driver); @@ -123,8 +125,6 @@ bhyveFirmwareFillDomain(bhyveConn *driver, g_autofree char *matching_nvram_template = NULL; g_autofree char *first_found = NULL; - virCheckFlags(0, -1); - if (!ARCH_IS_X86(def->os.arch)) return 0; @@ -190,7 +190,7 @@ bhyveFirmwareFillDomain(bhyveConn *driver, loader->path = g_build_filename(firmware_dir, matching_firmware, NULL); out: - bhyveFirmwareEnsureNVRAM(def, driver); + bhyveFirmwareEnsureNVRAM(def, driver, abiUpdate); return 0; } diff --git a/src/bhyve/bhyve_firmware.h b/src/bhyve/bhyve_firmware.h index ae7bd8a2b6..1ff7853909 100644 --- a/src/bhyve/bhyve_firmware.h +++ b/src/bhyve/bhyve_firmware.h @@ -27,4 +27,4 @@ int bhyveFirmwareFillDomain(bhyveConn *driver, virDomainDef *def, - unsigned int flags); + bool abiUpdate); diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c index dee3368cfc..65cf61c578 100644 --- a/src/bhyve/bhyve_process.c +++ b/src/bhyve/bhyve_process.c @@ -449,7 +449,9 @@ bhyveProcessPrepareDomain(bhyveConn *driver, virDomainObj *vm, unsigned int flags) { - if (bhyveFirmwareFillDomain(driver, vm->def, flags) < 0) + virCheckFlags(0, -1); + + if (bhyveFirmwareFillDomain(driver, vm->def, false) < 0) return -1; return 0; diff --git a/tests/bhyvefirmwaredata/three_firmwares/BHYVE_UEFI_VARS.fd b/tests/bhyvefirmwaredata/three_firmwares/BHYVE_UEFI_VARS.fd new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/bhyvexml2xmloutdata/x86_64/bhyvexml2xmlout-numa.xml b/tests/bhyvexml2xmloutdata/x86_64/bhyvexml2xmlout-numa.xml index ecc147db78..0066de9d6e 100644 --- a/tests/bhyvexml2xmloutdata/x86_64/bhyvexml2xmlout-numa.xml +++ b/tests/bhyvexml2xmloutdata/x86_64/bhyvexml2xmlout-numa.xml @@ -6,6 +6,8 @@ <vcpu placement='static'>8</vcpu> <os firmware='efi'> <type arch='x86_64'>hvm</type> + <loader readonly='yes' type='pflash' format='raw'>fakefirmwaredir/BHYVE_UEFI.fd</loader> + <nvram template='fakefirmwaredir/BHYVE_UEFI_VARS.fd' format='raw'>fakenvramdir/bhyve_VARS.fd</nvram> <boot dev='hd'/> </os> <cpu> diff --git a/tests/bhyvexml2xmltest.c b/tests/bhyvexml2xmltest.c index 120bdd42e5..9071f5e7f0 100644 --- a/tests/bhyvexml2xmltest.c +++ b/tests/bhyvexml2xmltest.c @@ -56,6 +56,8 @@ testCompareXMLToXMLHelper(const void *data) static int mymain(void) { + g_autofree char *fakefirmwaredir = g_strdup("fakefirmwaredir"); + g_autofree char *fakenvramdir = g_strdup("fakenvramdir"); g_autofree char *fakeubootpath = g_strdup("fakeubootpath/u-boot.bin"); int ret = 0; @@ -68,7 +70,12 @@ mymain(void) if (!(driver.config = virBhyveDriverConfigNew())) return EXIT_FAILURE; - driver.config->ubootPath = fakeubootpath; + VIR_FREE(driver.config->firmwareDir); + VIR_FREE(driver.config->nvramDir); + VIR_FREE(driver.config->ubootPath); + driver.config->firmwareDir = g_steal_pointer(&fakefirmwaredir); + driver.config->nvramDir = g_steal_pointer(&fakenvramdir); + driver.config->ubootPath = g_steal_pointer(&fakeubootpath); # define DO_TEST_FULL(name, flags) \ do { \ @@ -176,6 +183,7 @@ mymain(void) virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt); + virObjectUnref(driver.config); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.52.0
