[PATCH v4 00/13] Introduce network backed NVRAM

This version fixes multiple problems which I'd point out when reviewing: - virStorageSource is properly used in all places - logic for initializing NVRAM imgages from template is fixed to avoid touching network backed nvrams - documentation now states the correct version - some cleanups - dropped NEWS entry stashed in a patch with other stuff - fixed/simplified schema Rohit, please give it a try. I didn't yet have time to test this beyond unit tests. This series can also be fetched from my repo: git fetch https://gitlab.com/pipo.sk/libvirt.git network-nvram2 Peter Krempa (9): qemuDomainPrepareStorageSourceBlockdev: Add a variant for custom nodename qemuBuildPflashBlockdevCommandLine: Take virDomainObj instead of private data qemu: Use 'def->os.loader->nvram' directly instead of 'priv->pflash1' qemu: Properly setup the NVRAM virStorageSource qemuProcessReconnect: Don't re-instantiate pflash storage source qemuDomainInitializePflashStorageSource: Properly and fully initialize nvram source qemuFirmwareFillDomain: Don't fill in firmware for network backed nvram conf: Extract formatting of NVRAM out of virDomainLoaderDefFormat virDomainHugepagesFormat: Use virXMLFormatElementEmpty Rohit Kumar (4): conf: Convert def->os.loader->nvram a virStorageSource qemu: validate: Reject virStorageSource features we don't want to support with nvram conf: Add support to parse/format <source> for NVRAM Add unit tests for new specification of nvram. docs/formatdomain.rst | 37 +++++ src/conf/domain_conf.c | 136 ++++++++++++++---- src/conf/domain_conf.h | 3 +- src/conf/schemas/domaincommon.rng | 9 +- src/qemu/qemu_cgroup.c | 2 +- src/qemu/qemu_command.c | 22 +-- src/qemu/qemu_domain.c | 58 +++++--- src/qemu/qemu_domain.h | 9 +- src/qemu/qemu_driver.c | 5 +- src/qemu/qemu_firmware.c | 30 +++- src/qemu/qemu_namespace.c | 6 +- src/qemu/qemu_process.c | 11 +- src/qemu/qemu_validate.c | 115 +++++++++++---- src/security/security_dac.c | 19 +-- src/security/security_selinux.c | 21 +-- src/security/virt-aa-helper.c | 5 +- src/vbox/vbox_common.c | 3 +- .../bios-nvram-file.x86_64-latest.args | 37 +++++ tests/qemuxml2argvdata/bios-nvram-file.xml | 23 +++ .../bios-nvram-network-iscsi.x86_64-4.1.0.err | 1 + ...ios-nvram-network-iscsi.x86_64-latest.args | 38 +++++ .../bios-nvram-network-iscsi.xml | 31 ++++ .../bios-nvram-network-nbd.x86_64-latest.args | 37 +++++ .../bios-nvram-network-nbd.xml | 28 ++++ tests/qemuxml2argvtest.c | 4 + .../bios-nvram-file.x86_64-latest.xml | 39 +++++ ...bios-nvram-network-iscsi.x86_64-latest.xml | 44 ++++++ .../bios-nvram-network-nbd.x86_64-latest.xml | 41 ++++++ tests/qemuxml2xmltest.c | 3 + 29 files changed, 696 insertions(+), 121 deletions(-) create mode 100644 tests/qemuxml2argvdata/bios-nvram-file.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/bios-nvram-file.xml create mode 100644 tests/qemuxml2argvdata/bios-nvram-network-iscsi.x86_64-4.1.0.err create mode 100644 tests/qemuxml2argvdata/bios-nvram-network-iscsi.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/bios-nvram-network-iscsi.xml create mode 100644 tests/qemuxml2argvdata/bios-nvram-network-nbd.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/bios-nvram-network-nbd.xml create mode 100644 tests/qemuxml2xmloutdata/bios-nvram-file.x86_64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/bios-nvram-network-iscsi.x86_64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/bios-nvram-network-nbd.x86_64-latest.xml -- 2.35.3

Extract the internals of qemuDomainPrepareStorageSourceBlockdev into qemuDomainPrepareStorageSourceBlockdevNodename so that we can reuse it when instantiating the virStorageSource for pflash backing. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 31 +++++++++++++++++++++++-------- src/qemu/qemu_domain.h | 5 +++++ 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 916f85e673..86dbf7cb01 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -10855,15 +10855,14 @@ qemuDomainPrepareDiskSourceLegacy(virDomainDiskDef *disk, int -qemuDomainPrepareStorageSourceBlockdev(virDomainDiskDef *disk, - virStorageSource *src, - qemuDomainObjPrivate *priv, - virQEMUDriverConfig *cfg) +qemuDomainPrepareStorageSourceBlockdevNodename(virDomainDiskDef *disk, + virStorageSource *src, + const char *nodenameprefix, + qemuDomainObjPrivate *priv, + virQEMUDriverConfig *cfg) { - src->id = qemuDomainStorageIDNew(priv); - - src->nodestorage = g_strdup_printf("libvirt-%u-storage", src->id); - src->nodeformat = g_strdup_printf("libvirt-%u-format", src->id); + src->nodestorage = g_strdup_printf("%s-storage", nodenameprefix); + src->nodeformat = g_strdup_printf("%s-format", nodenameprefix); if (qemuBlockStorageSourceNeedsStorageSliceLayer(src)) src->sliceStorage->nodename = g_strdup_printf("libvirt-%u-slice-sto", src->id); @@ -10896,6 +10895,22 @@ qemuDomainPrepareStorageSourceBlockdev(virDomainDiskDef *disk, } +int +qemuDomainPrepareStorageSourceBlockdev(virDomainDiskDef *disk, + virStorageSource *src, + qemuDomainObjPrivate *priv, + virQEMUDriverConfig *cfg) +{ + g_autofree char *nodenameprefix = NULL; + + src->id = qemuDomainStorageIDNew(priv); + + nodenameprefix = g_strdup_printf("libvirt-%u", src->id); + + return qemuDomainPrepareStorageSourceBlockdevNodename(disk, src, nodenameprefix, priv, cfg); +} + + static int qemuDomainPrepareDiskSourceBlockdev(virDomainDiskDef *disk, qemuDomainObjPrivate *priv, diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 547d85b5f9..558037204c 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -757,6 +757,11 @@ int qemuDomainStorageSourceAccessAllow(virQEMUDriver *driver, bool newSource, bool chainTop); +int qemuDomainPrepareStorageSourceBlockdevNodename(virDomainDiskDef *disk, + virStorageSource *src, + const char *nodenameprefix, + qemuDomainObjPrivate *priv, + virQEMUDriverConfig *cfg); int qemuDomainPrepareStorageSourceBlockdev(virDomainDiskDef *disk, virStorageSource *src, qemuDomainObjPrivate *priv, -- 2.35.3

On a Friday in 2022, Peter Krempa wrote:
Extract the internals of qemuDomainPrepareStorageSourceBlockdev into qemuDomainPrepareStorageSourceBlockdevNodename so that we can reuse it when instantiating the virStorageSource for pflash backing.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 31 +++++++++++++++++++++++-------- src/qemu/qemu_domain.h | 5 +++++ 2 files changed, 28 insertions(+), 8 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 52e4ef03cd..7dc09fc101 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10122,8 +10122,10 @@ qemuBuildPflashBlockdevOne(virCommand *cmd, static int qemuBuildPflashBlockdevCommandLine(virCommand *cmd, - qemuDomainObjPrivate *priv) + virDomainObj *vm) { + qemuDomainObjPrivate *priv = vm->privateData; + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) return 0; @@ -10504,7 +10506,7 @@ qemuBuildCommandLine(virDomainObj *vm, if (qemuBuildManagedPRCommandLine(cmd, def, priv) < 0) return NULL; - if (qemuBuildPflashBlockdevCommandLine(cmd, priv) < 0) + if (qemuBuildPflashBlockdevCommandLine(cmd, vm) < 0) return NULL; /* QEMU 1.2 and later have a binary flag -enable-fips that must be -- 2.35.3

On a Friday in 2022, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

From: Rohit Kumar <rohit.kumar3@nutanix.com> Currently, libvirt allows only local filepaths to specify the location of the 'nvram' image. Changing it to virStorageSource type will allow to support remote storage for nvram. Signed-off-by: Prerna Saxena <prerna.saxena@nutanix.com> Signed-off-by: Florian Schmidt <flosch@nutanix.com> Signed-off-by: Rohit Kumar <rohit.kumar3@nutanix.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 16 +++++++++++++--- src/conf/domain_conf.h | 2 +- src/qemu/qemu_cgroup.c | 3 ++- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_domain.c | 10 +++++++--- src/qemu/qemu_driver.c | 5 +++-- src/qemu/qemu_firmware.c | 18 +++++++++++++----- src/qemu/qemu_namespace.c | 5 +++-- src/qemu/qemu_process.c | 5 +++-- src/security/security_dac.c | 6 ++++-- src/security/security_selinux.c | 6 ++++-- src/security/virt-aa-helper.c | 5 +++-- src/vbox/vbox_common.c | 3 ++- 13 files changed, 59 insertions(+), 27 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5d0d436a40..252e34dd2a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3576,7 +3576,7 @@ virDomainLoaderDefFree(virDomainLoaderDef *loader) return; g_free(loader->path); - g_free(loader->nvram); + virObjectUnref(loader->nvram); g_free(loader->nvramTemplate); g_free(loader); } @@ -18340,6 +18340,7 @@ virDomainDefParseBootLoaderOptions(virDomainDef *def, { xmlNodePtr loader_node = virXPathNode("./os/loader[1]", ctxt); const bool fwAutoSelect = def->os.firmware != VIR_DOMAIN_OS_DEF_FIRMWARE_NONE; + g_autofree char *nvramPath = NULL; if (!loader_node) return 0; @@ -18351,7 +18352,13 @@ virDomainDefParseBootLoaderOptions(virDomainDef *def, fwAutoSelect) < 0) return -1; - def->os.loader->nvram = virXPathString("string(./os/nvram[1])", ctxt); + if ((nvramPath = virXPathString("string(./os/nvram[1])", ctxt))) { + def->os.loader->nvram = virStorageSourceNew(); + def->os.loader->nvram->path = g_steal_pointer(&nvramPath); + def->os.loader->nvram->type = VIR_STORAGE_TYPE_FILE; + def->os.loader->nvram->format = VIR_STORAGE_FILE_RAW; + } + if (!fwAutoSelect) def->os.loader->nvramTemplate = virXPathString("string(./os/nvram[1]/@template)", ctxt); @@ -27118,7 +27125,10 @@ virDomainLoaderDefFormat(virBuffer *buf, virXMLFormatElementInternal(buf, "loader", &loaderAttrBuf, &loaderChildBuf, false, false); virBufferEscapeString(&nvramAttrBuf, " template='%s'", loader->nvramTemplate); - virBufferEscapeString(&nvramChildBuf, "%s", loader->nvram); + if (loader->nvram) { + if (loader->nvram->type == VIR_STORAGE_TYPE_FILE) + virBufferEscapeString(&nvramChildBuf, "%s", loader->nvram->path); + } virXMLFormatElementInternal(buf, "nvram", &nvramAttrBuf, &nvramChildBuf, false, false); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index e7e0f24443..9ec81067c6 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2253,7 +2253,7 @@ struct _virDomainLoaderDef { virTristateBool readonly; virDomainLoader type; virTristateBool secure; - char *nvram; /* path to non-volatile RAM */ + virStorageSource *nvram; char *nvramTemplate; /* user override of path to master nvram */ }; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index aa0c927578..64baed14e6 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -581,7 +581,8 @@ qemuSetupFirmwareCgroup(virDomainObj *vm) return -1; if (vm->def->os.loader->nvram && - qemuSetupImagePathCgroup(vm, vm->def->os.loader->nvram, false) < 0) + virStorageSourceIsLocalStorage(vm->def->os.loader->nvram) && + qemuSetupImagePathCgroup(vm, vm->def->os.loader->nvram->path, false) < 0) return -1; return 0; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7dc09fc101..952336bafc 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9668,7 +9668,7 @@ qemuBuildDomainLoaderPflashCommandLine(virCommand *cmd, if (loader->nvram) { virBufferAddLit(&buf, "file="); - virQEMUBuildBufferEscapeComma(&buf, loader->nvram); + virQEMUBuildBufferEscapeComma(&buf, loader->nvram->path); virBufferAsprintf(&buf, ",if=pflash,format=raw,unit=%d", unit); virCommandAddArg(cmd, "-drive"); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 86dbf7cb01..1ee3cc3922 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4678,8 +4678,12 @@ qemuDomainDefPostParse(virDomainDef *def, } if (virDomainDefHasOldStyleROUEFI(def) && - !def->os.loader->nvram) - qemuDomainNVRAMPathFormat(cfg, def, &def->os.loader->nvram); + !def->os.loader->nvram) { + def->os.loader->nvram = virStorageSourceNew(); + def->os.loader->nvram->type = VIR_STORAGE_TYPE_FILE; + def->os.loader->nvram->format = VIR_STORAGE_FILE_RAW; + qemuDomainNVRAMPathFormat(cfg, def, &def->os.loader->nvram->path); + } if (qemuDomainDefAddDefaultDevices(driver, def, qemuCaps) < 0) return -1; @@ -11332,7 +11336,7 @@ qemuDomainInitializePflashStorageSource(virDomainObj *vm) pflash1 = virStorageSourceNew(); pflash1->type = VIR_STORAGE_TYPE_FILE; pflash1->format = VIR_STORAGE_FILE_RAW; - pflash1->path = g_strdup(def->os.loader->nvram); + pflash1->path = g_strdup(def->os.loader->nvram->path); pflash1->readonly = false; pflash1->nodeformat = g_strdup("libvirt-pflash1-format"); pflash1->nodestorage = g_strdup("libvirt-pflash1-storage"); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0c6645ed89..c32e3cc8fe 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6731,8 +6731,9 @@ qemuDomainUndefineFlags(virDomainPtr dom, } } - if (vm->def->os.loader && vm->def->os.loader->nvram) { - nvram_path = g_strdup(vm->def->os.loader->nvram); + if (vm->def->os.loader && vm->def->os.loader->nvram && + virStorageSourceIsLocalStorage(vm->def->os.loader->nvram)) { + nvram_path = g_strdup(vm->def->os.loader->nvram->path); } else if (vm->def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_EFI) { qemuDomainNVRAMPathFormat(cfg, vm->def, &nvram_path); } diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c index 51223faadf..dd4273f73a 100644 --- a/src/qemu/qemu_firmware.c +++ b/src/qemu/qemu_firmware.c @@ -1192,13 +1192,17 @@ qemuFirmwareEnableFeatures(virQEMUDriver *driver, VIR_FREE(def->os.loader->nvramTemplate); def->os.loader->nvramTemplate = g_strdup(flash->nvram_template.filename); - if (!def->os.loader->nvram) - qemuDomainNVRAMPathFormat(cfg, def, &def->os.loader->nvram); + if (!def->os.loader->nvram) { + def->os.loader->nvram = virStorageSourceNew(); + def->os.loader->nvram->type = VIR_STORAGE_TYPE_FILE; + def->os.loader->nvram->format = VIR_STORAGE_FILE_RAW; + qemuDomainNVRAMPathFormat(cfg, def, &def->os.loader->nvram->path); + } VIR_DEBUG("decided on firmware '%s' template '%s' NVRAM '%s'", def->os.loader->path, def->os.loader->nvramTemplate, - def->os.loader->nvram); + def->os.loader->nvram->path); break; case QEMU_FIRMWARE_DEVICE_KERNEL: @@ -1364,8 +1368,12 @@ qemuFirmwareFillDomain(virQEMUDriver *driver, * its path in domain XML) but no template for NVRAM was * specified and the varstore doesn't exist ... */ if (!virDomainDefHasOldStyleROUEFI(def) || - def->os.loader->nvramTemplate || - (!reset_nvram && virFileExists(def->os.loader->nvram))) + def->os.loader->nvramTemplate) + return 0; + + if (!reset_nvram && def->os.loader->nvram && + virStorageSourceIsLocalStorage(def->os.loader->nvram) && + virFileExists(def->os.loader->nvram->path)) return 0; /* ... then we want to consult JSON FW descriptors first, diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c index 23681b14a4..9e133587b7 100644 --- a/src/qemu/qemu_namespace.c +++ b/src/qemu/qemu_namespace.c @@ -572,8 +572,9 @@ qemuDomainSetupLoader(virDomainObj *vm, case VIR_DOMAIN_LOADER_TYPE_PFLASH: *paths = g_slist_prepend(*paths, g_strdup(loader->path)); - if (loader->nvram) - *paths = g_slist_prepend(*paths, g_strdup(loader->nvram)); + if (loader->nvram && + virStorageSourceIsLocalStorage(loader->nvram)) + *paths = g_slist_prepend(*paths, g_strdup(loader->nvram->path)); break; case VIR_DOMAIN_LOADER_TYPE_NONE: diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 1593ca7933..dab298085f 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4400,7 +4400,8 @@ qemuPrepareNVRAM(virQEMUDriver *driver, struct qemuPrepareNVRAMHelperData data; if (!loader || !loader->nvram || - (virFileExists(loader->nvram) && !reset_nvram)) + !virStorageSourceIsLocalStorage(loader->nvram) || + (virFileExists(loader->nvram->path) && !reset_nvram)) return 0; master_nvram_path = loader->nvramTemplate; @@ -4432,7 +4433,7 @@ qemuPrepareNVRAM(virQEMUDriver *driver, data.srcFD = srcFD; data.srcPath = master_nvram_path; - if (virFileRewrite(loader->nvram, + if (virFileRewrite(loader->nvram->path, S_IRUSR | S_IWUSR, cfg->user, cfg->group, qemuPrepareNVRAMHelper, diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 69c462de8b..03661efda1 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1975,7 +1975,8 @@ virSecurityDACRestoreAllLabel(virSecurityManager *mgr, } if (def->os.loader && def->os.loader->nvram && - virSecurityDACRestoreFileLabel(mgr, def->os.loader->nvram) < 0) + virStorageSourceIsLocalStorage(def->os.loader->nvram) && + virSecurityDACRestoreFileLabel(mgr, def->os.loader->nvram->path) < 0) rc = -1; if (def->os.kernel && @@ -2186,8 +2187,9 @@ virSecurityDACSetAllLabel(virSecurityManager *mgr, } if (def->os.loader && def->os.loader->nvram && + virStorageSourceIsLocalStorage(def->os.loader->nvram) && virSecurityDACSetOwnership(mgr, NULL, - def->os.loader->nvram, + def->os.loader->nvram->path, user, group, true) < 0) return -1; diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 6f02baf2ce..e026212b13 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -2806,7 +2806,8 @@ virSecuritySELinuxRestoreAllLabel(virSecurityManager *mgr, } if (def->os.loader && def->os.loader->nvram && - virSecuritySELinuxRestoreFileLabel(mgr, def->os.loader->nvram, true) < 0) + virStorageSourceIsLocalStorage(def->os.loader->nvram) && + virSecuritySELinuxRestoreFileLabel(mgr, def->os.loader->nvram->path, true) < 0) rc = -1; if (def->os.kernel && @@ -3212,8 +3213,9 @@ virSecuritySELinuxSetAllLabel(virSecurityManager *mgr, /* This is different than kernel or initrd. The nvram store * is really a disk, qemu can read and write to it. */ if (def->os.loader && def->os.loader->nvram && + virStorageSourceIsLocalStorage(def->os.loader->nvram) && secdef && secdef->imagelabel && - virSecuritySELinuxSetFilecon(mgr, def->os.loader->nvram, + virSecuritySELinuxSetFilecon(mgr, def->os.loader->nvram->path, secdef->imagelabel, true) < 0) return -1; diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 107f217246..2ddf293c2c 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1006,8 +1006,9 @@ get_files(vahControl * ctl) if (vah_add_file(&buf, ctl->def->os.loader->path, "rk") != 0) goto cleanup; - if (ctl->def->os.loader && ctl->def->os.loader->nvram) - if (vah_add_file(&buf, ctl->def->os.loader->nvram, "rwk") != 0) + if (ctl->def->os.loader && ctl->def->os.loader->nvram && + virStorageSourceIsLocalStorage(ctl->def->os.loader->nvram)) + if (vah_add_file(&buf, ctl->def->os.loader->nvram->path, "rwk") != 0) goto cleanup; for (i = 0; i < ctl->def->ngraphics; i++) { diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 34e555644c..e249980195 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -992,7 +992,8 @@ vboxSetBootDeviceOrder(virDomainDef *def, struct _vboxDriver *data, VIR_DEBUG("def->os.loader->path %s", def->os.loader->path); VIR_DEBUG("def->os.loader->readonly %d", def->os.loader->readonly); VIR_DEBUG("def->os.loader->type %d", def->os.loader->type); - VIR_DEBUG("def->os.loader->nvram %s", def->os.loader->nvram); + if (def->os.loader->nvram) + VIR_DEBUG("def->os.loader->nvram->path %s", def->os.loader->nvram->path); } VIR_DEBUG("def->os.bootloader %s", def->os.bootloader); VIR_DEBUG("def->os.bootloaderArgs %s", def->os.bootloaderArgs); -- 2.35.3

On a Friday in 2022, Peter Krempa wrote:
From: Rohit Kumar <rohit.kumar3@nutanix.com>
Currently, libvirt allows only local filepaths to specify the location of the 'nvram' image. Changing it to virStorageSource type will allow to support remote storage for nvram.
will allow supporting or will allow support for
Signed-off-by: Prerna Saxena <prerna.saxena@nutanix.com> Signed-off-by: Florian Schmidt <flosch@nutanix.com> Signed-off-by: Rohit Kumar <rohit.kumar3@nutanix.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 16 +++++++++++++--- src/conf/domain_conf.h | 2 +- src/qemu/qemu_cgroup.c | 3 ++- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_domain.c | 10 +++++++--- src/qemu/qemu_driver.c | 5 +++-- src/qemu/qemu_firmware.c | 18 +++++++++++++----- src/qemu/qemu_namespace.c | 5 +++-- src/qemu/qemu_process.c | 5 +++-- src/security/security_dac.c | 6 ++++-- src/security/security_selinux.c | 6 ++++-- src/security/virt-aa-helper.c | 5 +++-- src/vbox/vbox_common.c | 3 ++- 13 files changed, 59 insertions(+), 27 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Since we now have a full virStorageSource for storing the nvram path we don't need the extra dance of transfering the data into the 'pflash1' variable which was an intermediary solution to use -blockdev. For now we keep it functionally identical to the previous impl. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 14 +++++++++----- src/qemu/qemu_domain.c | 12 ++---------- src/qemu/qemu_domain.h | 1 - 3 files changed, 11 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 952336bafc..c57ff6d281 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7138,11 +7138,12 @@ qemuBuildMachineCommandLine(virCommand *cmd, } } - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV) && + virDomainDefHasOldStyleUEFI(def)) { if (priv->pflash0) virBufferAsprintf(&buf, ",pflash0=%s", priv->pflash0->nodeformat); - if (priv->pflash1) - virBufferAsprintf(&buf, ",pflash1=%s", priv->pflash1->nodeformat); + if (def->os.loader->nvram) + virBufferAsprintf(&buf, ",pflash1=%s", def->os.loader->nvram->nodeformat); } if (virDomainNumaHasHMAT(def->numa)) @@ -10126,6 +10127,9 @@ qemuBuildPflashBlockdevCommandLine(virCommand *cmd, { qemuDomainObjPrivate *priv = vm->privateData; + if (!virDomainDefHasOldStyleUEFI(vm->def)) + return 0; + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) return 0; @@ -10133,8 +10137,8 @@ qemuBuildPflashBlockdevCommandLine(virCommand *cmd, qemuBuildPflashBlockdevOne(cmd, priv->pflash0, priv->qemuCaps) < 0) return -1; - if (priv->pflash1 && - qemuBuildPflashBlockdevOne(cmd, priv->pflash1, priv->qemuCaps) < 0) + if (vm->def->os.loader->nvram && + qemuBuildPflashBlockdevOne(cmd, vm->def->os.loader->nvram, priv->qemuCaps) < 0) return -1; return 0; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 1ee3cc3922..9ed2d1fd86 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1698,7 +1698,6 @@ qemuDomainObjPrivateDataClear(qemuDomainObjPrivate *priv) virHashRemoveAll(priv->blockjobs); g_clear_pointer(&priv->pflash0, virObjectUnref); - g_clear_pointer(&priv->pflash1, virObjectUnref); g_clear_pointer(&priv->backup, virDomainBackupDefFree); /* reset node name allocator */ @@ -11314,7 +11313,6 @@ qemuDomainInitializePflashStorageSource(virDomainObj *vm) qemuDomainObjPrivate *priv = vm->privateData; virDomainDef *def = vm->def; g_autoptr(virStorageSource) pflash0 = NULL; - g_autoptr(virStorageSource) pflash1 = NULL; if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) return 0; @@ -11333,17 +11331,11 @@ qemuDomainInitializePflashStorageSource(virDomainObj *vm) if (def->os.loader->nvram) { - pflash1 = virStorageSourceNew(); - pflash1->type = VIR_STORAGE_TYPE_FILE; - pflash1->format = VIR_STORAGE_FILE_RAW; - pflash1->path = g_strdup(def->os.loader->nvram->path); - pflash1->readonly = false; - pflash1->nodeformat = g_strdup("libvirt-pflash1-format"); - pflash1->nodestorage = g_strdup("libvirt-pflash1-storage"); + def->os.loader->nvram->nodeformat = g_strdup("libvirt-pflash1-format"); + def->os.loader->nvram->nodestorage = g_strdup("libvirt-pflash1-storage"); } priv->pflash0 = g_steal_pointer(&pflash0); - priv->pflash1 = g_steal_pointer(&pflash1); return 0; } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 558037204c..0c6b3eeffa 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -231,7 +231,6 @@ struct _qemuDomainObjPrivate { * pointers hold the temporary virStorageSources for creating the -blockdev * commandline for pflash drives. */ virStorageSource *pflash0; - virStorageSource *pflash1; /* running backup job */ virDomainBackupDef *backup; -- 2.35.3

On a Friday in 2022, Peter Krempa wrote:
Since we now have a full virStorageSource for storing the nvram path we don't need the extra dance of transfering the data into the 'pflash1'
*transferring
variable which was an intermediary solution to use -blockdev.
For now we keep it functionally identical to the previous impl.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 14 +++++++++----- src/qemu/qemu_domain.c | 12 ++---------- src/qemu/qemu_domain.h | 1 - 3 files changed, 11 insertions(+), 16 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Use the designated helpers for virStorageSource instead using the file-based ones with a check. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_cgroup.c | 3 +-- src/qemu/qemu_namespace.c | 5 +++-- src/security/security_dac.c | 21 +++++++++++---------- src/security/security_selinux.c | 23 +++++++++++------------ src/security/virt-aa-helper.c | 6 +++--- 5 files changed, 29 insertions(+), 29 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 64baed14e6..f189ca2bb6 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -581,8 +581,7 @@ qemuSetupFirmwareCgroup(virDomainObj *vm) return -1; if (vm->def->os.loader->nvram && - virStorageSourceIsLocalStorage(vm->def->os.loader->nvram) && - qemuSetupImagePathCgroup(vm, vm->def->os.loader->nvram->path, false) < 0) + qemuSetupImageCgroup(vm, vm->def->os.loader->nvram) < 0) return -1; return 0; diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c index 9e133587b7..59c6dc52ac 100644 --- a/src/qemu/qemu_namespace.c +++ b/src/qemu/qemu_namespace.c @@ -573,8 +573,9 @@ qemuDomainSetupLoader(virDomainObj *vm, *paths = g_slist_prepend(*paths, g_strdup(loader->path)); if (loader->nvram && - virStorageSourceIsLocalStorage(loader->nvram)) - *paths = g_slist_prepend(*paths, g_strdup(loader->nvram->path)); + qemuDomainSetupDisk(loader->nvram, paths) < 0) + return -1; + break; case VIR_DOMAIN_LOADER_TYPE_NONE: diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 03661efda1..bb89e466e1 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1974,10 +1974,11 @@ virSecurityDACRestoreAllLabel(virSecurityManager *mgr, rc = -1; } - if (def->os.loader && def->os.loader->nvram && - virStorageSourceIsLocalStorage(def->os.loader->nvram) && - virSecurityDACRestoreFileLabel(mgr, def->os.loader->nvram->path) < 0) - rc = -1; + if (def->os.loader && def->os.loader->nvram) { + if (virSecurityDACRestoreImageLabelInt(mgr, def, def->os.loader->nvram, + migrated) < 0) + rc = -1; + } if (def->os.kernel && virSecurityDACRestoreFileLabel(mgr, def->os.kernel) < 0) @@ -2186,12 +2187,12 @@ virSecurityDACSetAllLabel(virSecurityManager *mgr, return -1; } - if (def->os.loader && def->os.loader->nvram && - virStorageSourceIsLocalStorage(def->os.loader->nvram) && - virSecurityDACSetOwnership(mgr, NULL, - def->os.loader->nvram->path, - user, group, true) < 0) - return -1; + if (def->os.loader && def->os.loader->nvram) { + if (virSecurityDACSetImageLabel(mgr, def, def->os.loader->nvram, + VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN | + VIR_SECURITY_DOMAIN_IMAGE_PARENT_CHAIN_TOP) < 0) + return -1; + } if (def->os.kernel && virSecurityDACSetOwnership(mgr, NULL, diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index e026212b13..6a9d8e7e59 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -2805,10 +2805,11 @@ virSecuritySELinuxRestoreAllLabel(virSecurityManager *mgr, rc = -1; } - if (def->os.loader && def->os.loader->nvram && - virStorageSourceIsLocalStorage(def->os.loader->nvram) && - virSecuritySELinuxRestoreFileLabel(mgr, def->os.loader->nvram->path, true) < 0) - rc = -1; + if (def->os.loader && def->os.loader->nvram) { + if (virSecuritySELinuxRestoreImageLabelInt(mgr, def, def->os.loader->nvram, + migrated) < 0) + rc = -1; + } if (def->os.kernel && virSecuritySELinuxRestoreFileLabel(mgr, def->os.kernel, true) < 0) @@ -3210,14 +3211,12 @@ virSecuritySELinuxSetAllLabel(virSecurityManager *mgr, return -1; } - /* This is different than kernel or initrd. The nvram store - * is really a disk, qemu can read and write to it. */ - if (def->os.loader && def->os.loader->nvram && - virStorageSourceIsLocalStorage(def->os.loader->nvram) && - secdef && secdef->imagelabel && - virSecuritySELinuxSetFilecon(mgr, def->os.loader->nvram->path, - secdef->imagelabel, true) < 0) - return -1; + if (def->os.loader && def->os.loader->nvram) { + if (virSecuritySELinuxSetImageLabel(mgr, def, def->os.loader->nvram, + VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN | + VIR_SECURITY_DOMAIN_IMAGE_PARENT_CHAIN_TOP) < 0) + return -1; + } if (def->os.kernel && virSecuritySELinuxSetFilecon(mgr, def->os.kernel, diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 2ddf293c2c..d86b0f1cc2 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1006,10 +1006,10 @@ get_files(vahControl * ctl) if (vah_add_file(&buf, ctl->def->os.loader->path, "rk") != 0) goto cleanup; - if (ctl->def->os.loader && ctl->def->os.loader->nvram && - virStorageSourceIsLocalStorage(ctl->def->os.loader->nvram)) - if (vah_add_file(&buf, ctl->def->os.loader->nvram->path, "rwk") != 0) + if (ctl->def->os.loader && ctl->def->os.loader->nvram) { + if (storage_source_add_files(disk->src, &buf, 0) < 0) goto cleanup; + } for (i = 0; i < ctl->def->ngraphics; i++) { virDomainGraphicsDef *graphics = ctl->def->graphics[i]; -- 2.35.3

On a Friday in 2022, Peter Krempa wrote:
Use the designated helpers for virStorageSource instead using the file-based ones with a check.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_cgroup.c | 3 +-- src/qemu/qemu_namespace.c | 5 +++-- src/security/security_dac.c | 21 +++++++++++---------- src/security/security_selinux.c | 23 +++++++++++------------ src/security/virt-aa-helper.c | 6 +++--- 5 files changed, 29 insertions(+), 29 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

We don't really use it besides when starting up the VM so when reconnecting this step is totally pointless. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_process.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index dab298085f..3334c7d536 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8730,10 +8730,6 @@ qemuProcessReconnect(void *opaque) if (qemuDomainPerfRestart(obj) < 0) goto error; - /* recreate the pflash storage sources */ - if (qemuDomainInitializePflashStorageSource(obj) < 0) - goto error; - for (i = 0; i < obj->def->ndisks; i++) { virDomainDiskDef *disk = obj->def->disks[i]; -- 2.35.3

On a Friday in 2022, Peter Krempa wrote:
We don't really use it besides when starting up the VM so when reconnecting this step is totally pointless.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_process.c | 4 ---- 1 file changed, 4 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Setup all fields for use with -blockdev. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 11 ++++++++--- src/qemu/qemu_domain.h | 3 ++- src/qemu/qemu_process.c | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9ed2d1fd86..2f91bdf316 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -11308,7 +11308,8 @@ qemuDomainSupportsCheckpointsBlockjobs(virDomainObj *vm) * 'libvirt-pflash1-format' for pflash1. */ int -qemuDomainInitializePflashStorageSource(virDomainObj *vm) +qemuDomainInitializePflashStorageSource(virDomainObj *vm, + virQEMUDriverConfig *cfg) { qemuDomainObjPrivate *priv = vm->privateData; virDomainDef *def = vm->def; @@ -11331,8 +11332,12 @@ qemuDomainInitializePflashStorageSource(virDomainObj *vm) if (def->os.loader->nvram) { - def->os.loader->nvram->nodeformat = g_strdup("libvirt-pflash1-format"); - def->os.loader->nvram->nodestorage = g_strdup("libvirt-pflash1-storage"); + if (qemuDomainPrepareStorageSourceBlockdevNodename(NULL, + def->os.loader->nvram, + "libvirt-pflash1", + priv, + cfg) < 0) + return -1; } priv->pflash0 = g_steal_pointer(&pflash0); diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 0c6b3eeffa..36325259cc 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -1047,7 +1047,8 @@ int qemuDomainMakeCPUMigratable(virCPUDef *cpu); int -qemuDomainInitializePflashStorageSource(virDomainObj *vm); +qemuDomainInitializePflashStorageSource(virDomainObj *vm, + virQEMUDriverConfig *cfg); bool qemuDomainDiskBlockJobIsSupported(virDomainObj *vm, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 3334c7d536..09409b90b7 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6493,7 +6493,7 @@ qemuProcessPrepareDomain(virQEMUDriver *driver, VIR_DEBUG("Prepare bios/uefi paths"); if (qemuFirmwareFillDomain(driver, vm->def, flags) < 0) return -1; - if (qemuDomainInitializePflashStorageSource(vm) < 0) + if (qemuDomainInitializePflashStorageSource(vm, cfg) < 0) return -1; VIR_DEBUG("Preparing external devices"); -- 2.35.3

On a Friday in 2022, Peter Krempa wrote:
Setup all fields for use with -blockdev.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 11 ++++++++--- src/qemu/qemu_domain.h | 3 ++- src/qemu/qemu_process.c | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

From: Rohit Kumar <rohit.kumar3@nutanix.com> Signed-off-by: Prerna Saxena <prerna.saxena@nutanix.com> Signed-off-by: Florian Schmidt <flosch@nutanix.com> Signed-off-by: Rohit Kumar <rohit.kumar3@nutanix.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_validate.c | 108 ++++++++++++++++++++++++++++++--------- 1 file changed, 84 insertions(+), 24 deletions(-) diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 9b6245e6d7..48b7d08684 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -568,35 +568,95 @@ qemuValidateDomainDefPM(const virDomainDef *def, static int -qemuValidateDomainDefBoot(const virDomainDef *def) +qemuValidateDomainDefNvram(const virDomainDef *def, + virQEMUCaps *qemuCaps) { - if (def->os.loader && - def->os.loader->secure == VIR_TRISTATE_BOOL_YES) { - /* These are the QEMU implementation limitations. But we - * have to live with them for now. */ + virStorageSource *src = def->os.loader->nvram; - if (!qemuDomainIsQ35(def)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Secure boot is supported with q35 machine types only")); - return -1; - } + if (!src) + return 0; - /* Now, technically it is possible to have secure boot on - * 32bits too, but that would require some -cpu xxx magic - * too. Not worth it unless we are explicitly asked. */ - if (def->os.arch != VIR_ARCH_X86_64) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Secure boot is supported for x86_64 architecture only")); - return -1; + switch (src->type) { + case VIR_STORAGE_TYPE_FILE: + case VIR_STORAGE_TYPE_BLOCK: + case VIR_STORAGE_TYPE_NETWORK: + break; + + case VIR_STORAGE_TYPE_DIR: + case VIR_STORAGE_TYPE_VOLUME: + case VIR_STORAGE_TYPE_NVME: + case VIR_STORAGE_TYPE_VHOST_USER: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported nvram disk type '%s'"), + virStorageTypeToString(src->type)); + return -1; + + case VIR_STORAGE_TYPE_NONE: + case VIR_STORAGE_TYPE_LAST: + virReportEnumRangeError(virStorageType, src->type); + return -1; + } + + if (src->sliceStorage) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("slices are not supported with NVRAM")); + return -1; + } + + if (src->pr) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("persistent reservations are not supported with NVRAM")); + return -1; + } + + if (src->backingStore) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("backingStore is not supported with NVRAM")); + return -1; + } + + if (qemuDomainValidateStorageSource(src, qemuCaps, false) < 0) + return -1; + + return 0; +} + + +static int +qemuValidateDomainDefBoot(const virDomainDef *def, + virQEMUCaps *qemuCaps) +{ + if (def->os.loader) { + if (def->os.loader->secure == VIR_TRISTATE_BOOL_YES) { + /* These are the QEMU implementation limitations. But we + * have to live with them for now. */ + + if (!qemuDomainIsQ35(def)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Secure boot is supported with q35 machine types only")); + return -1; + } + + /* Now, technically it is possible to have secure boot on + * 32bits too, but that would require some -cpu xxx magic + * too. Not worth it unless we are explicitly asked. */ + if (def->os.arch != VIR_ARCH_X86_64) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Secure boot is supported for x86_64 architecture only")); + return -1; + } + + /* SMM will be enabled by qemuFirmwareFillDomain() if needed. */ + if (def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_NONE && + def->features[VIR_DOMAIN_FEATURE_SMM] != VIR_TRISTATE_SWITCH_ON) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Secure boot requires SMM feature enabled")); + return -1; + } } - /* SMM will be enabled by qemuFirmwareFillDomain() if needed. */ - if (def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_NONE && - def->features[VIR_DOMAIN_FEATURE_SMM] != VIR_TRISTATE_SWITCH_ON) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Secure boot requires SMM feature enabled")); + if (qemuValidateDomainDefNvram(def, qemuCaps) < 0) return -1; - } } return 0; @@ -1174,7 +1234,7 @@ qemuValidateDomainDef(const virDomainDef *def, if (qemuValidateDomainDefPM(def, qemuCaps) < 0) return -1; - if (qemuValidateDomainDefBoot(def) < 0) + if (qemuValidateDomainDefBoot(def, qemuCaps) < 0) return -1; if (qemuValidateDomainVCpuTopology(def, qemuCaps) < 0) -- 2.35.3

On a Friday in 2022, Peter Krempa wrote:
From: Rohit Kumar <rohit.kumar3@nutanix.com>
Signed-off-by: Prerna Saxena <prerna.saxena@nutanix.com> Signed-off-by: Florian Schmidt <flosch@nutanix.com> Signed-off-by: Rohit Kumar <rohit.kumar3@nutanix.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_validate.c | 108 ++++++++++++++++++++++++++++++--------- 1 file changed, 84 insertions(+), 24 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Prepare for network backed nvram by refusing the reset of nvram on boot and don't check whether it exists. We will not support filling it from a template. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_firmware.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c index dd4273f73a..a219978962 100644 --- a/src/qemu/qemu_firmware.c +++ b/src/qemu/qemu_firmware.c @@ -1371,10 +1371,22 @@ qemuFirmwareFillDomain(virQEMUDriver *driver, def->os.loader->nvramTemplate) return 0; - if (!reset_nvram && def->os.loader->nvram && - virStorageSourceIsLocalStorage(def->os.loader->nvram) && - virFileExists(def->os.loader->nvram->path)) - return 0; + if (def->os.loader->nvram) { + if (!virStorageSourceIsLocalStorage(def->os.loader->nvram)) { + if (reset_nvram) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("resetting of nvram is not supported with network backed nvram")); + return -1; + } + + /* we don't scrutinize whether NVRAM image accessed via network + * is present */ + return 0; + } + + if (!reset_nvram && virFileExists(def->os.loader->nvram->path)) + 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. */ -- 2.35.3

On a Friday in 2022, Peter Krempa wrote:
Prepare for network backed nvram by refusing the reset of nvram on boot and don't check whether it exists. We will not support filling it from a template.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_firmware.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Introduce virDomainLoaderDefFormatNvram and extract the code to it so that it's self-contained in upcoming patches adding more complex logic. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 252e34dd2a..05b2518a71 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -27099,14 +27099,30 @@ virDomainHugepagesFormat(virBuffer *buf, virBufferAddLit(buf, "</hugepages>\n"); } + +static void +virDomainLoaderDefFormatNvram(virBuffer *buf, + virDomainLoaderDef *loader) +{ + g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) childBuf = VIR_BUFFER_INITIALIZER; + + virBufferEscapeString(&attrBuf, " template='%s'", loader->nvramTemplate); + if (loader->nvram) { + if (loader->nvram->type == VIR_STORAGE_TYPE_FILE) + virBufferEscapeString(&childBuf, "%s", loader->nvram->path); + } + + virXMLFormatElementInternal(buf, "nvram", &attrBuf, &childBuf, false, false); +} + + static void virDomainLoaderDefFormat(virBuffer *buf, virDomainLoaderDef *loader) { g_auto(virBuffer) loaderAttrBuf = VIR_BUFFER_INITIALIZER; g_auto(virBuffer) loaderChildBuf = VIR_BUFFER_INITIALIZER; - g_auto(virBuffer) nvramAttrBuf = VIR_BUFFER_INITIALIZER; - g_auto(virBuffer) nvramChildBuf = VIR_BUFFER_INITIALIZER; if (loader->readonly != VIR_TRISTATE_BOOL_ABSENT) virBufferAsprintf(&loaderAttrBuf, " readonly='%s'", @@ -27124,12 +27140,7 @@ virDomainLoaderDefFormat(virBuffer *buf, virXMLFormatElementInternal(buf, "loader", &loaderAttrBuf, &loaderChildBuf, false, false); - virBufferEscapeString(&nvramAttrBuf, " template='%s'", loader->nvramTemplate); - if (loader->nvram) { - if (loader->nvram->type == VIR_STORAGE_TYPE_FILE) - virBufferEscapeString(&nvramChildBuf, "%s", loader->nvram->path); - } - virXMLFormatElementInternal(buf, "nvram", &nvramAttrBuf, &nvramChildBuf, false, false); + virDomainLoaderDefFormatNvram(buf, loader); } static void -- 2.35.3

On a Friday in 2022, Peter Krempa wrote:
Introduce virDomainLoaderDefFormatNvram and extract the code to it so that it's self-contained in upcoming patches adding more complex logic.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

From: Rohit Kumar <rohit.kumar3@nutanix.com> This patch introduces the logic to format and parse remote NVRAM. Update NVRAM element schema, and docs for supporting network backed NVRAM. NVRAM backed over network would give the flexibility to start the VM on any host without having to worry about where to get the latest nvram image. <nvram type='network'> <source protocol='iscsi' name='iqn.2013-07.com.example:iscsi-nopool/0'> <host name='example.com' port='6000'/> </source> </nvram> or <nvram type='file'> <source file='/var/lib/libvirt/nvram/guest_VARS.fd'/> </nvram> In the qemu driver we will support the new definition only with qemu's supporting -blockdev. Signed-off-by: Prerna Saxena <prerna.saxena@nutanix.com> Signed-off-by: Florian Schmidt <flosch@nutanix.com> Signed-off-by: Rohit Kumar <rohit.kumar3@nutanix.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/formatdomain.rst | 37 ++++++++++ src/conf/domain_conf.c | 116 ++++++++++++++++++++++++------ src/conf/domain_conf.h | 1 + src/conf/schemas/domaincommon.rng | 9 ++- src/qemu/qemu_validate.c | 7 ++ 5 files changed, 146 insertions(+), 24 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 312b605a8b..8a9da07612 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -135,6 +135,34 @@ harddisk, cdrom, network) determining where to obtain/find the boot image. </os> ... + <!-- QEMU with UEFI manual firmware, secure boot and with NVRAM type 'file'--> + ... + <os> + <type>hvm</type> + <loader readonly='yes' secure='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE.fd</loader> + <nvram type='file' template='/usr/share/OVMF/OVMF_VARS.fd'> + <source file='/var/lib/libvirt/nvram/guest_VARS.fd'/> + <boot dev='hd'/> + </os> + ... + + <!-- QEMU with UEFI manual firmware, secure boot and with network backed NVRAM'--> + ... + <os> + <type>hvm</type> + <loader readonly='yes' secure='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE.fd</loader> + <nvram type='network'> + <source protocol='iscsi' name='iqn.2013-07.com.example:iscsi-nopool/0'> + <host name='example.com' port='6000'/> + <auth username='myname'> + <secret type='iscsi' usage='mycluster_myname'/> + </auth> + </source> + </nvram> + <boot dev='hd'/> + </os> + ... + <!-- QEMU with automatic UEFI firmware and secure boot --> ... <os firmware='efi'> @@ -224,6 +252,15 @@ harddisk, cdrom, network) determining where to obtain/find the boot image. if the NVRAM file has been created by libvirt it is left behind and it is management application's responsibility to save and remove file (if needed to be persistent). :since:`Since 1.2.8` + + :since:`Since 8.5.0`, it's possible for the element to have ``type`` attribute + (accepts values ``file``, ``block`` and ``network``) in that case the NVRAM + storage is described by a ``<source>`` sub-element with the same syntax as + ``disk``'s source. See `Hard drives, floppy disks, CDROMs`_. + + **Note:** ``network`` backed NVRAM the variables are not instantiated from + the ``template`` and it's user's responsibility to provide a valid NVRAM image. + ``boot`` The ``dev`` attribute takes one of the values "fd", "hd", "cdrom" or "network" and is used to specify the next boot device to consider. The diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 05b2518a71..b34513f943 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17951,6 +17951,51 @@ virDomainLoaderDefParseXML(xmlNodePtr node, } +static int +virDomainNvramDefParseXML(virDomainLoaderDef *loader, + xmlXPathContextPtr ctxt, + virDomainXMLOption *xmlopt, + unsigned int flags) +{ + g_autofree char *nvramType = virXPathString("string(./os/nvram/@type)", ctxt); + g_autoptr(virStorageSource) src = virStorageSourceNew(); + + src->type = VIR_STORAGE_TYPE_FILE; + src->format = VIR_STORAGE_FILE_RAW; + + if (!nvramType) { + char *nvramPath = NULL; + + if (!(nvramPath = virXPathString("string(./os/nvram[1])", ctxt))) + return 0; /* no nvram */ + + src->path = nvramPath; + } else { + xmlNodePtr sourceNode; + + if ((src->type = virStorageTypeFromString(nvramType)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown disk type '%s'"), nvramType); + return -1; + } + + if (!(sourceNode = virXPathNode("./os/nvram/source[1]", ctxt))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Missing source element for nvram")); + return -1; + } + + if (virDomainStorageSourceParse(sourceNode, ctxt, src, flags, xmlopt) < 0) + return -1; + + loader->newStyleNVRAM = true; + } + + loader->nvram = g_steal_pointer(&src); + return 0; +} + + static int virDomainSchedulerParseCommonAttrs(xmlNodePtr node, virProcessSchedPolicy *policy, @@ -18336,11 +18381,12 @@ virDomainDefParseBootFirmwareOptions(virDomainDef *def, static int virDomainDefParseBootLoaderOptions(virDomainDef *def, - xmlXPathContextPtr ctxt) + xmlXPathContextPtr ctxt, + virDomainXMLOption *xmlopt, + unsigned int flags) { xmlNodePtr loader_node = virXPathNode("./os/loader[1]", ctxt); const bool fwAutoSelect = def->os.firmware != VIR_DOMAIN_OS_DEF_FIRMWARE_NONE; - g_autofree char *nvramPath = NULL; if (!loader_node) return 0; @@ -18352,12 +18398,8 @@ virDomainDefParseBootLoaderOptions(virDomainDef *def, fwAutoSelect) < 0) return -1; - if ((nvramPath = virXPathString("string(./os/nvram[1])", ctxt))) { - def->os.loader->nvram = virStorageSourceNew(); - def->os.loader->nvram->path = g_steal_pointer(&nvramPath); - def->os.loader->nvram->type = VIR_STORAGE_TYPE_FILE; - def->os.loader->nvram->format = VIR_STORAGE_FILE_RAW; - } + if (virDomainNvramDefParseXML(def->os.loader, ctxt, xmlopt, flags) < 0) + return -1; if (!fwAutoSelect) def->os.loader->nvramTemplate = virXPathString("string(./os/nvram[1]/@template)", ctxt); @@ -18412,7 +18454,9 @@ virDomainDefParseBootAcpiOptions(virDomainDef *def, static int virDomainDefParseBootOptions(virDomainDef *def, - xmlXPathContextPtr ctxt) + xmlXPathContextPtr ctxt, + virDomainXMLOption *xmlopt, + unsigned int flags) { /* * Booting options for different OS types.... @@ -18430,7 +18474,7 @@ virDomainDefParseBootOptions(virDomainDef *def, if (virDomainDefParseBootFirmwareOptions(def, ctxt) < 0) return -1; - if (virDomainDefParseBootLoaderOptions(def, ctxt) < 0) + if (virDomainDefParseBootLoaderOptions(def, ctxt, xmlopt, flags) < 0) return -1; if (virDomainDefParseBootAcpiOptions(def, ctxt) < 0) @@ -18446,7 +18490,7 @@ virDomainDefParseBootOptions(virDomainDef *def, case VIR_DOMAIN_OSTYPE_UML: virDomainDefParseBootKernelOptions(def, ctxt); - if (virDomainDefParseBootLoaderOptions(def, ctxt) < 0) + if (virDomainDefParseBootLoaderOptions(def, ctxt, xmlopt, flags) < 0) return -1; break; @@ -19746,7 +19790,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, if (virDomainDefClockParse(def, ctxt) < 0) return NULL; - if (virDomainDefParseBootOptions(def, ctxt) < 0) + if (virDomainDefParseBootOptions(def, ctxt, xmlopt, flags) < 0) return NULL; /* analysis of the disk devices */ @@ -27100,26 +27144,48 @@ virDomainHugepagesFormat(virBuffer *buf, } -static void +static int virDomainLoaderDefFormatNvram(virBuffer *buf, - virDomainLoaderDef *loader) + virDomainLoaderDef *loader, + virDomainXMLOption *xmlopt, + unsigned int flags) { g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; - g_auto(virBuffer) childBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) childBufDirect = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) childBufChild = VIR_BUFFER_INIT_CHILD(buf); + virBuffer *childBuf = &childBufDirect; + bool childNewline = false; virBufferEscapeString(&attrBuf, " template='%s'", loader->nvramTemplate); + if (loader->nvram) { - if (loader->nvram->type == VIR_STORAGE_TYPE_FILE) - virBufferEscapeString(&childBuf, "%s", loader->nvram->path); + virStorageSource *src = loader->nvram; + + if (!loader->newStyleNVRAM) { + virBufferEscapeString(&childBufDirect, "%s", src->path); + } else { + childNewline = true; + childBuf = &childBufChild; + + virBufferAsprintf(&attrBuf, " type='%s'", virStorageTypeToString(src->type)); + + if (virDomainDiskSourceFormat(&childBufChild, src, "source", 0, + false, flags, false, false, xmlopt) < 0) + return -1; + } } - virXMLFormatElementInternal(buf, "nvram", &attrBuf, &childBuf, false, false); + virXMLFormatElementInternal(buf, "nvram", &attrBuf, childBuf, false, childNewline); + + return 0; } -static void +static int virDomainLoaderDefFormat(virBuffer *buf, - virDomainLoaderDef *loader) + virDomainLoaderDef *loader, + virDomainXMLOption *xmlopt, + unsigned int flags) { g_auto(virBuffer) loaderAttrBuf = VIR_BUFFER_INITIALIZER; g_auto(virBuffer) loaderChildBuf = VIR_BUFFER_INITIALIZER; @@ -27140,7 +27206,10 @@ virDomainLoaderDefFormat(virBuffer *buf, virXMLFormatElementInternal(buf, "loader", &loaderAttrBuf, &loaderChildBuf, false, false); - virDomainLoaderDefFormatNvram(buf, loader); + if (virDomainLoaderDefFormatNvram(buf, loader, xmlopt, flags) < 0) + return -1; + + return 0; } static void @@ -28341,8 +28410,9 @@ virDomainDefFormatInternalSetRootName(virDomainDef *def, if (def->os.initgroup) virBufferAsprintf(buf, "<initgroup>%s</initgroup>\n", def->os.initgroup); - if (def->os.loader) - virDomainLoaderDefFormat(buf, def->os.loader); + if (def->os.loader && + virDomainLoaderDefFormat(buf, def->os.loader, xmlopt, flags) < 0) + return -1; virBufferEscapeString(buf, "<kernel>%s</kernel>\n", def->os.kernel); virBufferEscapeString(buf, "<initrd>%s</initrd>\n", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 9ec81067c6..1ce6e855b6 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2254,6 +2254,7 @@ struct _virDomainLoaderDef { virDomainLoader type; virTristateBool secure; virStorageSource *nvram; + bool newStyleNVRAM; char *nvramTemplate; /* user override of path to master nvram */ }; diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index cc598212a8..bf829f3a65 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -333,7 +333,14 @@ </attribute> </optional> <optional> - <ref name="absFilePath"/> + <choice> + <group> + <ref name="absFilePath"/> + </group> + <group> + <ref name="diskSource"/> + </group> + </choice> </optional> </element> </optional> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 48b7d08684..2bbe198a11 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -576,6 +576,13 @@ qemuValidateDomainDefNvram(const virDomainDef *def, if (!src) return 0; + if (def->os.loader->newStyleNVRAM && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("modern nvram specification is not supported by this qemu")); + return -1; + } + switch (src->type) { case VIR_STORAGE_TYPE_FILE: case VIR_STORAGE_TYPE_BLOCK: -- 2.35.3

On a Friday in 2022, Peter Krempa wrote:
From: Rohit Kumar <rohit.kumar3@nutanix.com>
This patch introduces the logic to format and parse remote NVRAM.
Update NVRAM element schema, and docs for supporting network backed NVRAM. NVRAM backed over network would give the flexibility to start the VM on any host without having to worry about where to get the latest nvram image.
<nvram type='network'> <source protocol='iscsi' name='iqn.2013-07.com.example:iscsi-nopool/0'> <host name='example.com' port='6000'/> </source> </nvram>
or
<nvram type='file'> <source file='/var/lib/libvirt/nvram/guest_VARS.fd'/> </nvram>
In the qemu driver we will support the new definition only with qemu's supporting -blockdev.
Signed-off-by: Prerna Saxena <prerna.saxena@nutanix.com> Signed-off-by: Florian Schmidt <flosch@nutanix.com> Signed-off-by: Rohit Kumar <rohit.kumar3@nutanix.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/formatdomain.rst | 37 ++++++++++ src/conf/domain_conf.c | 116 ++++++++++++++++++++++++------ src/conf/domain_conf.h | 1 + src/conf/schemas/domaincommon.rng | 9 ++- src/qemu/qemu_validate.c | 7 ++ 5 files changed, 146 insertions(+), 24 deletions(-)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 312b605a8b..8a9da07612 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -135,6 +135,34 @@ harddisk, cdrom, network) determining where to obtain/find the boot image. </os> ...
+ <!-- QEMU with UEFI manual firmware, secure boot and with NVRAM type 'file'--> + ... + <os> + <type>hvm</type> + <loader readonly='yes' secure='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE.fd</loader> + <nvram type='file' template='/usr/share/OVMF/OVMF_VARS.fd'> + <source file='/var/lib/libvirt/nvram/guest_VARS.fd'/>
Unterminated <nvram>
+ <boot dev='hd'/> + </os> + ... + + <!-- QEMU with UEFI manual firmware, secure boot and with network backed NVRAM'--> + ... + <os> + <type>hvm</type> + <loader readonly='yes' secure='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE.fd</loader> + <nvram type='network'> + <source protocol='iscsi' name='iqn.2013-07.com.example:iscsi-nopool/0'> + <host name='example.com' port='6000'/> + <auth username='myname'> + <secret type='iscsi' usage='mycluster_myname'/> + </auth> + </source> + </nvram> + <boot dev='hd'/> + </os> + ... + <!-- QEMU with automatic UEFI firmware and secure boot --> ... <os firmware='efi'>
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

From: Rohit Kumar <rohit.kumar3@nutanix.com> This patch adds unit tests for remote NVRAM. Examples: <nvram type='network'> <source protocol='iscsi' name='iqn.2013-07.com.example:iscsi-nopool/0'> <host name='example.com' port='6000'/> <auth username='myname'> <secret type='iscsi' usage='mycluster_myname'/> </auth> </source> </nvram> and <nvram type='network'> <source protocol='nbd' name='bar'> <host name='example.org' port='6000'/> </source> </nvram> and <nvram type='file'> <source file='/var/lib/libvirt/nvram/guest_VARS.fd'/> </nvram> Signed-off-by: Prerna Saxena <prerna.saxena@nutanix.com> Signed-off-by: Florian Schmidt <flosch@nutanix.com> Signed-off-by: Rohit Kumar <rohit.kumar3@nutanix.com> --- .../bios-nvram-file.x86_64-latest.args | 37 ++++++++++++++++ tests/qemuxml2argvdata/bios-nvram-file.xml | 23 ++++++++++ .../bios-nvram-network-iscsi.x86_64-4.1.0.err | 1 + ...ios-nvram-network-iscsi.x86_64-latest.args | 38 ++++++++++++++++ .../bios-nvram-network-iscsi.xml | 31 +++++++++++++ .../bios-nvram-network-nbd.x86_64-latest.args | 37 ++++++++++++++++ .../bios-nvram-network-nbd.xml | 28 ++++++++++++ tests/qemuxml2argvtest.c | 4 ++ .../bios-nvram-file.x86_64-latest.xml | 39 ++++++++++++++++ ...bios-nvram-network-iscsi.x86_64-latest.xml | 44 +++++++++++++++++++ .../bios-nvram-network-nbd.x86_64-latest.xml | 41 +++++++++++++++++ tests/qemuxml2xmltest.c | 3 ++ 12 files changed, 326 insertions(+) create mode 100644 tests/qemuxml2argvdata/bios-nvram-file.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/bios-nvram-file.xml create mode 100644 tests/qemuxml2argvdata/bios-nvram-network-iscsi.x86_64-4.1.0.err create mode 100644 tests/qemuxml2argvdata/bios-nvram-network-iscsi.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/bios-nvram-network-iscsi.xml create mode 100644 tests/qemuxml2argvdata/bios-nvram-network-nbd.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/bios-nvram-network-nbd.xml create mode 100644 tests/qemuxml2xmloutdata/bios-nvram-file.x86_64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/bios-nvram-network-iscsi.x86_64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/bios-nvram-network-nbd.x86_64-latest.xml diff --git a/tests/qemuxml2argvdata/bios-nvram-file.x86_64-latest.args b/tests/qemuxml2argvdata/bios-nvram-file.x86_64-latest.args new file mode 100644 index 0000000000..4b0aec7539 --- /dev/null +++ b/tests/qemuxml2argvdata/bios-nvram-file.x86_64-latest.args @@ -0,0 +1,37 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-test-bios \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-test-bios/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-test-bios/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-test-bios/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=test-bios,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-test-bios/master-key.aes"}' \ +-blockdev '{"driver":"file","filename":"/usr/share/OVMF/OVMF_CODE.fd","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"raw","file":"libvirt-pflash0-storage"}' \ +-blockdev '{"driver":"file","filename":"/var/lib/libvirt/nvram/guest_VARS.fd","node-name":"libvirt-pflash1-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-pflash1-format","read-only":false,"driver":"raw","file":"libvirt-pflash1-storage"}' \ +-machine pc,usb=off,dump-guest-core=off,pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-format,memory-backend=pc.ram \ +-accel tcg \ +-cpu qemu64 \ +-m 1024 \ +-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":1073741824}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid 362d1fc1-df7d-193e-5c18-49a71bd1da66 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot menu=on,strict=on \ +-device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x2"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/bios-nvram-file.xml b/tests/qemuxml2argvdata/bios-nvram-file.xml new file mode 100644 index 0000000000..8df9412112 --- /dev/null +++ b/tests/qemuxml2argvdata/bios-nvram-file.xml @@ -0,0 +1,23 @@ +<domain type='qemu'> + <name>test-bios</name> + <uuid>362d1fc1-df7d-193e-5c18-49a71bd1da66</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <loader readonly='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE.fd</loader> + <nvram type='file'> + <source file='/var/lib/libvirt/nvram/guest_VARS.fd'/> + </nvram> + <boot dev='hd'/> + <bootmenu enable='yes'/> + </os> + <features> + <acpi/> + </features> + <clock offset='utc'/> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/bios-nvram-network-iscsi.x86_64-4.1.0.err b/tests/qemuxml2argvdata/bios-nvram-network-iscsi.x86_64-4.1.0.err new file mode 100644 index 0000000000..5251bbdbbf --- /dev/null +++ b/tests/qemuxml2argvdata/bios-nvram-network-iscsi.x86_64-4.1.0.err @@ -0,0 +1 @@ +unsupported configuration: modern nvram specification is not supported by this qemu diff --git a/tests/qemuxml2argvdata/bios-nvram-network-iscsi.x86_64-latest.args b/tests/qemuxml2argvdata/bios-nvram-network-iscsi.x86_64-latest.args new file mode 100644 index 0000000000..b8a323358d --- /dev/null +++ b/tests/qemuxml2argvdata/bios-nvram-network-iscsi.x86_64-latest.args @@ -0,0 +1,38 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-test-bios \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-test-bios/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-test-bios/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-test-bios/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=test-bios,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-test-bios/master-key.aes"}' \ +-blockdev '{"driver":"file","filename":"/usr/share/OVMF/OVMF_CODE.fd","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"raw","file":"libvirt-pflash0-storage"}' \ +-object '{"qom-type":"secret","id":"libvirt-pflash1-storage-auth-secret0","data":"9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1","keyid":"masterKey0","iv":"AAECAwQFBgcICQoLDA0ODw==","format":"base64"}' \ +-blockdev '{"driver":"iscsi","portal":"example.com:6000","target":"iqn.2013-07.com.example:iscsi-nopool","lun":0,"transport":"tcp","user":"myname","password-secret":"libvirt-pflash1-storage-auth-secret0","node-name":"libvirt-pflash1-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-pflash1-format","read-only":false,"driver":"raw","file":"libvirt-pflash1-storage"}' \ +-machine pc,usb=off,dump-guest-core=off,pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-format,memory-backend=pc.ram \ +-accel tcg \ +-cpu qemu64 \ +-m 1024 \ +-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":1073741824}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid 362d1fc1-df7d-193e-5c18-49a71bd1da66 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot menu=on,strict=on \ +-device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x2"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/bios-nvram-network-iscsi.xml b/tests/qemuxml2argvdata/bios-nvram-network-iscsi.xml new file mode 100644 index 0000000000..d8a354126d --- /dev/null +++ b/tests/qemuxml2argvdata/bios-nvram-network-iscsi.xml @@ -0,0 +1,31 @@ +<domain type='qemu'> + <name>test-bios</name> + <uuid>362d1fc1-df7d-193e-5c18-49a71bd1da66</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <loader readonly='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE.fd</loader> + <nvram type='network'> + <source protocol='iscsi' name='iqn.2013-07.com.example:iscsi-nopool'> + <host name='example.com' port='6000'/> + <auth username='myname'> + <secret type='iscsi' usage='mycluster_myname'/> + </auth> + </source> + </nvram> + <boot dev='hd'/> + <bootmenu enable='yes'/> + </os> + <features> + <acpi/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/bios-nvram-network-nbd.x86_64-latest.args b/tests/qemuxml2argvdata/bios-nvram-network-nbd.x86_64-latest.args new file mode 100644 index 0000000000..08dbd99335 --- /dev/null +++ b/tests/qemuxml2argvdata/bios-nvram-network-nbd.x86_64-latest.args @@ -0,0 +1,37 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-test-bios \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-test-bios/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-test-bios/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-test-bios/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=test-bios,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-test-bios/master-key.aes"}' \ +-blockdev '{"driver":"file","filename":"/usr/share/OVMF/OVMF_CODE.fd","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"raw","file":"libvirt-pflash0-storage"}' \ +-blockdev '{"driver":"nbd","server":{"type":"inet","host":"example.org","port":"6000"},"export":"bar","node-name":"libvirt-pflash1-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-pflash1-format","read-only":false,"driver":"raw","file":"libvirt-pflash1-storage"}' \ +-machine pc,usb=off,dump-guest-core=off,pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-format,memory-backend=pc.ram \ +-accel tcg \ +-cpu qemu64 \ +-m 1024 \ +-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":1073741824}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid 362d1fc1-df7d-193e-5c18-49a71bd1da66 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot menu=on,strict=on \ +-device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x2"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/bios-nvram-network-nbd.xml b/tests/qemuxml2argvdata/bios-nvram-network-nbd.xml new file mode 100644 index 0000000000..3350914607 --- /dev/null +++ b/tests/qemuxml2argvdata/bios-nvram-network-nbd.xml @@ -0,0 +1,28 @@ +<domain type='qemu'> + <name>test-bios</name> + <uuid>362d1fc1-df7d-193e-5c18-49a71bd1da66</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <loader readonly='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE.fd</loader> + <nvram type='network'> + <source protocol='nbd' name='bar'> + <host name='example.org' port='6000'/> + </source> + </nvram> + <boot dev='hd'/> + <bootmenu enable='yes'/> + </os> + <features> + <acpi/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index bffe7aef8a..fecd863820 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1209,6 +1209,10 @@ mymain(void) QEMU_CAPS_ICH9_AHCI, QEMU_CAPS_VIRTIO_SCSI); DO_TEST_CAPS_LATEST("bios-nvram-template"); + DO_TEST_CAPS_LATEST("bios-nvram-network-iscsi"); + DO_TEST_CAPS_VER_PARSE_ERROR("bios-nvram-network-iscsi", "4.1.0"); + DO_TEST_CAPS_LATEST("bios-nvram-network-nbd"); + DO_TEST_CAPS_LATEST("bios-nvram-file"); /* Make sure all combinations of ACPI and UEFI behave as expected */ DO_TEST_NOCAPS("q35-acpi-uefi"); diff --git a/tests/qemuxml2xmloutdata/bios-nvram-file.x86_64-latest.xml b/tests/qemuxml2xmloutdata/bios-nvram-file.x86_64-latest.xml new file mode 100644 index 0000000000..97e029f70b --- /dev/null +++ b/tests/qemuxml2xmloutdata/bios-nvram-file.x86_64-latest.xml @@ -0,0 +1,39 @@ +<domain type='qemu'> + <name>test-bios</name> + <uuid>362d1fc1-df7d-193e-5c18-49a71bd1da66</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <loader readonly='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE.fd</loader> + <nvram type='file'> + <source file='/var/lib/libvirt/nvram/guest_VARS.fd'/> + </nvram> + <boot dev='hd'/> + <bootmenu enable='yes'/> + </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='piix3-uhci'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/bios-nvram-network-iscsi.x86_64-latest.xml b/tests/qemuxml2xmloutdata/bios-nvram-network-iscsi.x86_64-latest.xml new file mode 100644 index 0000000000..73b7aefe7b --- /dev/null +++ b/tests/qemuxml2xmloutdata/bios-nvram-network-iscsi.x86_64-latest.xml @@ -0,0 +1,44 @@ +<domain type='qemu'> + <name>test-bios</name> + <uuid>362d1fc1-df7d-193e-5c18-49a71bd1da66</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <loader readonly='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE.fd</loader> + <nvram type='network'> + <source protocol='iscsi' name='iqn.2013-07.com.example:iscsi-nopool'> + <host name='example.com' port='6000'/> + <auth username='myname'> + <secret type='iscsi' usage='mycluster_myname'/> + </auth> + </source> + </nvram> + <boot dev='hd'/> + <bootmenu enable='yes'/> + </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>restart</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='usb' index='0' model='piix3-uhci'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/bios-nvram-network-nbd.x86_64-latest.xml b/tests/qemuxml2xmloutdata/bios-nvram-network-nbd.x86_64-latest.xml new file mode 100644 index 0000000000..bc78be11b6 --- /dev/null +++ b/tests/qemuxml2xmloutdata/bios-nvram-network-nbd.x86_64-latest.xml @@ -0,0 +1,41 @@ +<domain type='qemu'> + <name>test-bios</name> + <uuid>362d1fc1-df7d-193e-5c18-49a71bd1da66</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <loader readonly='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE.fd</loader> + <nvram type='network'> + <source protocol='nbd' name='bar'> + <host name='example.org' port='6000'/> + </source> + </nvram> + <boot dev='hd'/> + <bootmenu enable='yes'/> + </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>restart</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='usb' index='0' model='piix3-uhci'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 3bd57306cc..e4b04ae408 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1071,6 +1071,9 @@ mymain(void) DO_TEST_NOCAPS("bios-nvram"); DO_TEST_NOCAPS("bios-nvram-os-interleave"); + DO_TEST_CAPS_LATEST("bios-nvram-network-iscsi"); + DO_TEST_CAPS_LATEST("bios-nvram-network-nbd"); + DO_TEST_CAPS_LATEST("bios-nvram-file"); DO_TEST_NOCAPS("tap-vhost"); DO_TEST_NOCAPS("tap-vhost-incorrect"); -- 2.35.3

On a Friday in 2022, Peter Krempa wrote:
From: Rohit Kumar <rohit.kumar3@nutanix.com>
This patch adds unit tests for remote NVRAM.
Examples:
<nvram type='network'> <source protocol='iscsi' name='iqn.2013-07.com.example:iscsi-nopool/0'> <host name='example.com' port='6000'/> <auth username='myname'> <secret type='iscsi' usage='mycluster_myname'/> </auth> </source> </nvram>
and
<nvram type='network'> <source protocol='nbd' name='bar'> <host name='example.org' port='6000'/> </source> </nvram>
and
<nvram type='file'> <source file='/var/lib/libvirt/nvram/guest_VARS.fd'/> </nvram>
Signed-off-by: Prerna Saxena <prerna.saxena@nutanix.com> Signed-off-by: Florian Schmidt <flosch@nutanix.com> Signed-off-by: Rohit Kumar <rohit.kumar3@nutanix.com> --- .../bios-nvram-file.x86_64-latest.args | 37 ++++++++++++++++ tests/qemuxml2argvdata/bios-nvram-file.xml | 23 ++++++++++ .../bios-nvram-network-iscsi.x86_64-4.1.0.err | 1 + ...ios-nvram-network-iscsi.x86_64-latest.args | 38 ++++++++++++++++ .../bios-nvram-network-iscsi.xml | 31 +++++++++++++ .../bios-nvram-network-nbd.x86_64-latest.args | 37 ++++++++++++++++ .../bios-nvram-network-nbd.xml | 28 ++++++++++++ tests/qemuxml2argvtest.c | 4 ++ .../bios-nvram-file.x86_64-latest.xml | 39 ++++++++++++++++ ...bios-nvram-network-iscsi.x86_64-latest.xml | 44 +++++++++++++++++++ .../bios-nvram-network-nbd.x86_64-latest.xml | 41 +++++++++++++++++ tests/qemuxml2xmltest.c | 3 ++ 12 files changed, 326 insertions(+) create mode 100644 tests/qemuxml2argvdata/bios-nvram-file.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/bios-nvram-file.xml create mode 100644 tests/qemuxml2argvdata/bios-nvram-network-iscsi.x86_64-4.1.0.err create mode 100644 tests/qemuxml2argvdata/bios-nvram-network-iscsi.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/bios-nvram-network-iscsi.xml create mode 100644 tests/qemuxml2argvdata/bios-nvram-network-nbd.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/bios-nvram-network-nbd.xml create mode 100644 tests/qemuxml2xmloutdata/bios-nvram-file.x86_64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/bios-nvram-network-iscsi.x86_64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/bios-nvram-network-nbd.x86_64-latest.xml
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Refactor the function to use modern XML formatting machinery. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b34513f943..4110109bd8 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -27125,22 +27125,15 @@ virDomainHugepagesFormat(virBuffer *buf, virDomainHugePage *hugepages, size_t nhugepages) { + g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); size_t i; - if (nhugepages == 1 && - hugepages[0].size == 0) { - virBufferAddLit(buf, "<hugepages/>\n"); - return; + if (nhugepages != 1 || hugepages[0].size != 0) { + for (i = 0; i < nhugepages; i++) + virDomainHugepagesFormatBuf(&childBuf, &hugepages[i]); } - virBufferAddLit(buf, "<hugepages>\n"); - virBufferAdjustIndent(buf, 2); - - for (i = 0; i < nhugepages; i++) - virDomainHugepagesFormatBuf(buf, &hugepages[i]); - - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</hugepages>\n"); + virXMLFormatElementEmpty(buf, "hugepages", NULL, &childBuf); } -- 2.35.3

On a Friday in 2022, Peter Krempa wrote:
Refactor the function to use modern XML formatting machinery.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

On 03/06/22 5:18 pm, Peter Krempa wrote:
This version fixes multiple problems which I'd point out when reviewing: - virStorageSource is properly used in all places - logic for initializing NVRAM imgages from template is fixed to avoid touching network backed nvrams - documentation now states the correct version - some cleanups - dropped NEWS entry stashed in a patch with other stuff - fixed/simplified schema
Rohit, please give it a try. I didn't yet have time to test this beyond unit tests.
Hi Peter, As we discussed on v3 patchset (https://listman.redhat.com/archives/libvir-list/2022-June/232446.html). Feel free to add "Tested-by:" tag on my behalf. Thanks, Rohit.
This series can also be fetched from my repo:
Peter Krempa (9): qemuDomainPrepareStorageSourceBlockdev: Add a variant for custom nodename qemuBuildPflashBlockdevCommandLine: Take virDomainObj instead of private data qemu: Use 'def->os.loader->nvram' directly instead of 'priv->pflash1' qemu: Properly setup the NVRAM virStorageSource qemuProcessReconnect: Don't re-instantiate pflash storage source qemuDomainInitializePflashStorageSource: Properly and fully initialize nvram source qemuFirmwareFillDomain: Don't fill in firmware for network backed nvram conf: Extract formatting of NVRAM out of virDomainLoaderDefFormat virDomainHugepagesFormat: Use virXMLFormatElementEmpty
Rohit Kumar (4): conf: Convert def->os.loader->nvram a virStorageSource qemu: validate: Reject virStorageSource features we don't want to support with nvram conf: Add support to parse/format <source> for NVRAM Add unit tests for new specification of nvram.
docs/formatdomain.rst | 37 +++++ src/conf/domain_conf.c | 136 ++++++++++++++---- src/conf/domain_conf.h | 3 +- src/conf/schemas/domaincommon.rng | 9 +- src/qemu/qemu_cgroup.c | 2 +- src/qemu/qemu_command.c | 22 +-- src/qemu/qemu_domain.c | 58 +++++--- src/qemu/qemu_domain.h | 9 +- src/qemu/qemu_driver.c | 5 +- src/qemu/qemu_firmware.c | 30 +++- src/qemu/qemu_namespace.c | 6 +- src/qemu/qemu_process.c | 11 +- src/qemu/qemu_validate.c | 115 +++++++++++---- src/security/security_dac.c | 19 +-- src/security/security_selinux.c | 21 +-- src/security/virt-aa-helper.c | 5 +- src/vbox/vbox_common.c | 3 +- .../bios-nvram-file.x86_64-latest.args | 37 +++++ tests/qemuxml2argvdata/bios-nvram-file.xml | 23 +++ .../bios-nvram-network-iscsi.x86_64-4.1.0.err | 1 + ...ios-nvram-network-iscsi.x86_64-latest.args | 38 +++++ .../bios-nvram-network-iscsi.xml | 31 ++++ .../bios-nvram-network-nbd.x86_64-latest.args | 37 +++++ .../bios-nvram-network-nbd.xml | 28 ++++ tests/qemuxml2argvtest.c | 4 + .../bios-nvram-file.x86_64-latest.xml | 39 +++++ ...bios-nvram-network-iscsi.x86_64-latest.xml | 44 ++++++ .../bios-nvram-network-nbd.x86_64-latest.xml | 41 ++++++ tests/qemuxml2xmltest.c | 3 + 29 files changed, 696 insertions(+), 121 deletions(-) create mode 100644 tests/qemuxml2argvdata/bios-nvram-file.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/bios-nvram-file.xml create mode 100644 tests/qemuxml2argvdata/bios-nvram-network-iscsi.x86_64-4.1.0.err create mode 100644 tests/qemuxml2argvdata/bios-nvram-network-iscsi.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/bios-nvram-network-iscsi.xml create mode 100644 tests/qemuxml2argvdata/bios-nvram-network-nbd.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/bios-nvram-network-nbd.xml create mode 100644 tests/qemuxml2xmloutdata/bios-nvram-file.x86_64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/bios-nvram-network-iscsi.x86_64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/bios-nvram-network-nbd.x86_64-latest.xml
participants (3)
-
Ján Tomko
-
Peter Krempa
-
Rohit Kumar