From 911c3cb2f0e7c1cede02c1b8c2c52c7ab5996e9d Mon Sep 17 00:00:00 2001 From: Rohit Kumar Date: Wed, 4 May 2022 09:51:11 -0700 Subject: [PATCH] conf: Convert def->os.loader->nvram a virStorageSource MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently, libvirt allows only local filepaths to specify the location of the 'nvram' image. Changing it to virStorageSource type will allow supporting remote storage for nvram. Signed-off-by: Prerna Saxena Signed-off-by: Florian Schmidt Signed-off-by: Rohit Kumar Signed-off-by: Peter Krempa Reviewed-by: Ján Tomko Tested-by: Rohit Kumar --- 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 761c3f4d87..6f23a2c3b4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3590,7 +3590,7 @@ virDomainLoaderDefFree(virDomainLoaderDef *loader) return; g_free(loader->path); - g_free(loader->nvram); + virObjectUnref(loader->nvram); g_free(loader->nvramTemplate); g_free(loader); } @@ -18402,6 +18402,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; @@ -18413,7 +18414,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); @@ -27179,7 +27186,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 da9e281214..1b0ec83857 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 c4981f4ccf..5e39bca0e3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9690,7 +9690,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 b0d52c315b..e5f819a4b4 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; @@ -11335,7 +11339,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 ded34e97cd..08eb211b8a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6866,8 +6866,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 e53f7c9c79..66c5935d7d 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4609,7 +4609,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; @@ -4641,7 +4642,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 2a033c15ac..ad03ae65e6 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1974,7 +1974,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 && @@ -2185,8 +2186,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 303a2bc505..918012a323 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -2804,7 +2804,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 && @@ -3210,8 +3211,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);