March 2022 - Devel - libvirt List Archives

[PATCH] Introduce network-backed NVRAM
by Rohit Kumar 28 Mar '22

28 Mar '22

Libvirt domain XML allows only local filepaths to specify a NVRAM element. Since VMs can move across different hosts, it should be possibe to allocate NVRAM disks on network storage for uninturrupted access. This Patch extends the NVRAM disk elements to be described as virStorageSource* elements. Sample XML with new annotation: <nvram> <source protocol='iscsi' name='iqn.2013-07.com.example:iscsi-nopool/0'> <host name='example.com' port='6000'/> </source> </nvram> or <nvram> <source file='/var/lib/libvirt/nvram/guest_VARS.fd'/> </nvram> Signed-off-by: Prerna Saxena <prerna.saxena(a)nutanix.com> Signed-off-by: Florian Schmidt <flosch(a)nutanix.com> Signed-off-by: Rohit Kumar <rohit.kumar3(a)nutanix.com> --- docs/schemas/domaincommon.rng | 72 +++++++++------ src/conf/domain_conf.c | 90 +++++++++++++++---- src/conf/domain_conf.h | 2 +- src/qemu/qemu_cgroup.c | 3 +- src/qemu/qemu_command.c | 5 +- src/qemu/qemu_domain.c | 23 +++-- src/qemu/qemu_driver.c | 5 +- src/qemu/qemu_firmware.c | 17 ++-- 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 | 8 +- .../qemuxml2argvdata/bios-nvram-network.args | 35 ++++++++ tests/qemuxml2argvdata/bios-nvram-network.xml | 40 +++++++++ tests/qemuxml2argvtest.c | 1 + 17 files changed, 256 insertions(+), 72 deletions(-) create mode 100644 tests/qemuxml2argvdata/bios-nvram-network.args create mode 100644 tests/qemuxml2argvdata/bios-nvram-network.xml diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 9c1b64a644..a25c84a0b7 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -330,7 +330,17 @@ </attribute> </optional> <optional> - <ref name="absFilePath"/> + <choice> + <group> + <ref name="absFilePath"/> + </group> + <group> + <ref name="diskSourceFileElement"/> + </group> + <group> + <ref name="diskSourceNetworkElement"/> + </group> + </choice> </optional> </element> </optional> @@ -1714,6 +1724,31 @@ </choice> </define> + <define name="diskSourceFileElement"> + <element name="source"> + <interleave> + <optional> + <attribute name="file"> + <choice> + <ref name="absFilePath"/> + <ref name="vmwarePath"/> + </choice> + </attribute> + </optional> + <ref name="diskSourceCommon"/> + <optional> + <ref name="storageStartupPolicy"/> + </optional> + <optional> + <ref name="encryption"/> + </optional> + <zeroOrMore> + <ref name="devSeclabel"/> + </zeroOrMore> + </interleave> + </element> + </define> + <define name="diskSourceFile"> <optional> <attribute name="type"> @@ -1721,28 +1756,7 @@ </attribute> </optional> <optional> - <element name="source"> - <interleave> - <optional> - <attribute name="file"> - <choice> - <ref name="absFilePath"/> - <ref name="vmwarePath"/> - </choice> - </attribute> - </optional> - <ref name="diskSourceCommon"/> - <optional> - <ref name="storageStartupPolicy"/> - </optional> - <optional> - <ref name="encryption"/> - </optional> - <zeroOrMore> - <ref name="devSeclabel"/> - </zeroOrMore> - </interleave> - </element> + <ref name="diskSourceFileElement"/> </optional> </define> @@ -2137,10 +2151,7 @@ </element> </define> - <define name="diskSourceNetwork"> - <attribute name="type"> - <value>network</value> - </attribute> + <define name="diskSourceNetworkElement"> <choice> <ref name="diskSourceNetworkProtocolNBD"/> <ref name="diskSourceNetworkProtocolGluster"/> @@ -2155,6 +2166,13 @@ </choice> </define> + <define name="diskSourceNetwork"> + <attribute name="type"> + <value>network</value> + </attribute> + <ref name="diskSourceNetworkElement"/> + </define> + <define name="diskSourceVolume"> <attribute name="type"> <value>volume</value> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e0dfc9e45f..5fcc09db05 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3535,7 +3535,7 @@ virDomainLoaderDefFree(virDomainLoaderDef *loader) return; g_free(loader->path); - g_free(loader->nvram); + virObjectUnref(loader->nvram); g_free(loader->nvramTemplate); g_free(loader); } @@ -17834,6 +17834,37 @@ virDomainLoaderDefParseXML(xmlNodePtr node, } +static int +virDomainNvramDefParseXML(virStorageSource *nvram, + xmlXPathContextPtr ctxt, + virDomainXMLOption *opt, + unsigned int flags) +{ + g_autofree char *srcTypeFile = NULL; + g_autofree char *srcTypeNetwork = NULL; + xmlNodePtr source; + + srcTypeFile = virXPathString("string(./os/nvram/source/@file)", ctxt); + srcTypeNetwork = virXPathString("string(./os/nvram/source/@protocol)", ctxt); + + if (!srcTypeFile && !srcTypeNetwork) { + nvram->type = VIR_STORAGE_TYPE_FILE; + nvram->path = virXPathString("string(./os/nvram[1])", ctxt); + return 0; + } else { + if (srcTypeFile) { + nvram->type = VIR_STORAGE_TYPE_FILE; + } else { + nvram->type = VIR_STORAGE_TYPE_NETWORK; + } + source = virXPathNode("./os/nvram/source[1]", ctxt); + if (!source) + return -1; + return virDomainStorageSourceParse(source, ctxt, nvram, flags, opt); + } + +} + static int virDomainSchedulerParseCommonAttrs(xmlNodePtr node, virProcessSchedPolicy *policy, @@ -18219,7 +18250,9 @@ 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; @@ -18234,7 +18267,14 @@ virDomainDefParseBootLoaderOptions(virDomainDef *def, fwAutoSelect) < 0) return -1; - def->os.loader->nvram = virXPathString("string(./os/nvram[1])", ctxt); + if (virXPathNode("./os/nvram[1]", ctxt)) { + def->os.loader->nvram = g_new0(virStorageSource, 1); + + if (virDomainNvramDefParseXML(def->os.loader->nvram, + ctxt, xmlopt, flags) < 0) + return -1; + } + if (!fwAutoSelect) def->os.loader->nvramTemplate = virXPathString("string(./os/nvram[1]/@template)", ctxt); @@ -18288,7 +18328,9 @@ virDomainDefParseBootAcpiOptions(virDomainDef *def, static int virDomainDefParseBootOptions(virDomainDef *def, - xmlXPathContextPtr ctxt) + xmlXPathContextPtr ctxt, + virDomainXMLOption *xmlopt, + unsigned int flags) { /* * Booting options for different OS types.... @@ -18306,7 +18348,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) @@ -18322,7 +18364,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; @@ -19606,7 +19648,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 */ @@ -26899,7 +26941,9 @@ virDomainHugepagesFormat(virBuffer *buf, static void virDomainLoaderDefFormat(virBuffer *buf, - virDomainLoaderDef *loader) + virDomainLoaderDef *loader, + virDomainXMLOption *xmlopt, + unsigned int flags) { const char *readonly = virTristateBoolTypeToString(loader->readonly); const char *secure = virTristateBoolTypeToString(loader->secure); @@ -26921,13 +26965,27 @@ virDomainLoaderDefFormat(virBuffer *buf, else virBufferAddLit(buf, "/>\n"); - if (loader->nvram || loader->nvramTemplate) { - virBufferAddLit(buf, "<nvram"); - virBufferEscapeString(buf, " template='%s'", loader->nvramTemplate); - if (loader->nvram) - virBufferEscapeString(buf, ">%s</nvram>\n", loader->nvram); - else - virBufferAddLit(buf, "/>\n"); + if (loader->nvram) { + if (loader->nvram->type == VIR_STORAGE_TYPE_FILE) { + virBufferAddLit(buf, "<nvram"); + virBufferEscapeString(buf, " template='%s'", loader->nvramTemplate); + if (loader->nvram->path) + virBufferEscapeString(buf, ">%s</nvram>\n", loader->nvram->path); + else + virBufferAddLit(buf, "/>\n"); + } else { + virBufferAddLit(buf, "<nvram"); + virBufferEscapeString(buf, " template='%s'", loader->nvramTemplate); + virBufferAdjustIndent(buf, 2); + if (virDomainDiskSourceFormat(buf, loader->nvram, "source", 0, + 0, false, flags, true, xmlopt) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot format NVRAM source")); + return; + } + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</nvram>\n"); + } } } @@ -28122,7 +28180,7 @@ virDomainDefFormatInternalSetRootName(virDomainDef *def, virBufferAsprintf(buf, "<initgroup>%s</initgroup>\n", def->os.initgroup); if (def->os.loader) - virDomainLoaderDefFormat(buf, def->os.loader); + virDomainLoaderDefFormat(buf, def->os.loader, xmlopt, flags); 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 a4de46773c..f65558e82d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2231,7 +2231,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..d46c9ff36a 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) + vm->def->os.loader->nvram->type == VIR_STORAGE_TYPE_FILE && + 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 c836799888..649a6f5b55 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9583,6 +9583,7 @@ qemuBuildDomainLoaderPflashCommandLine(virCommand *cmd, virQEMUCaps *qemuCaps) { g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; + g_autofree char *nvramPath = NULL; int unit = 0; if (loader->secure == VIR_TRISTATE_BOOL_YES) { @@ -9610,8 +9611,10 @@ qemuBuildDomainLoaderPflashCommandLine(virCommand *cmd, virCommandAddArgBuffer(cmd, &buf); if (loader->nvram) { + if (qemuGetDriveSourceString(loader->nvram, NULL, &nvramPath) < 0) + return; virBufferAddLit(&buf, "file="); - virQEMUBuildBufferEscapeComma(&buf, loader->nvram); + virQEMUBuildBufferEscapeComma(&buf, nvramPath); 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 7180ae616b..303d9661a4 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4452,8 +4452,13 @@ 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->path)) { + + if (!def->os.loader->nvram) + def->os.loader->nvram = g_new0(virStorageSource, 1); + def->os.loader->nvram->type = VIR_STORAGE_TYPE_FILE; + qemuDomainNVRAMPathFormat(cfg, def, &def->os.loader->nvram->path); + } if (qemuDomainDefAddDefaultDevices(driver, def, qemuCaps) < 0) return -1; @@ -11133,15 +11138,23 @@ qemuDomainInitializePflashStorageSource(virDomainObj *vm) pflash0->nodeformat = g_strdup("libvirt-pflash0-format"); pflash0->nodestorage = g_strdup("libvirt-pflash0-storage"); - 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); + 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"); + + if (def->os.loader->nvram->type == VIR_STORAGE_TYPE_FILE) { + pflash1->type = VIR_STORAGE_TYPE_FILE; + } else if (def->os.loader->nvram->type == VIR_STORAGE_TYPE_NETWORK) { + pflash1->protocol = def->os.loader->nvram->protocol; + pflash1->hosts = g_new0(virStorageNetHostDef, 1); + pflash1->nhosts = 1; + pflash1->hosts[0].name = def->os.loader->nvram->hosts[0].name; + pflash1->hosts[0].port = def->os.loader->nvram->hosts[0].port; + } } priv->pflash0 = g_steal_pointer(&pflash0); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b7e83c769a..4f4098975c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6605,8 +6605,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 && + vm->def->os.loader->nvram->type == VIR_STORAGE_TYPE_FILE) { + 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..fa99c7d217 100644 --- a/src/qemu/qemu_firmware.c +++ b/src/qemu/qemu_firmware.c @@ -1192,13 +1192,16 @@ 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 = g_new0(virStorageSource, 1); + def->os.loader->nvram->type = VIR_STORAGE_TYPE_FILE; + 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 +1367,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 || + (def->os.loader->nvram && + !reset_nvram && ((def->os.loader->nvram->type == VIR_STORAGE_TYPE_FILE + && virFileExists(def->os.loader->nvram->path)) || + (def->os.loader->nvram->type == VIR_STORAGE_TYPE_NETWORK && + 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..18a24635ad 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 && + loader->nvram->type == VIR_STORAGE_TYPE_FILE) + *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 1ed60917ea..c8a25dbe91 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4475,7 +4475,8 @@ qemuPrepareNVRAM(virQEMUDriver *driver, struct qemuPrepareNVRAMHelperData data; if (!loader || !loader->nvram || - (virFileExists(loader->nvram) && !reset_nvram)) + loader->nvram->type != VIR_STORAGE_TYPE_FILE || + (virFileExists(loader->nvram->path) && !reset_nvram)) return 0; master_nvram_path = loader->nvramTemplate; @@ -4507,7 +4508,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 e9e316551e..66c36c57a3 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1971,7 +1971,8 @@ virSecurityDACRestoreAllLabel(virSecurityManager *mgr, } if (def->os.loader && def->os.loader->nvram && - virSecurityDACRestoreFileLabel(mgr, def->os.loader->nvram) < 0) + def->os.loader->nvram->type == VIR_STORAGE_TYPE_FILE && + virSecurityDACRestoreFileLabel(mgr, def->os.loader->nvram->path) < 0) rc = -1; if (def->os.kernel && @@ -2182,8 +2183,9 @@ virSecurityDACSetAllLabel(virSecurityManager *mgr, } if (def->os.loader && def->os.loader->nvram && + def->os.loader->nvram->type == VIR_STORAGE_TYPE_FILE && 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..1b6b67e8c7 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) + def->os.loader->nvram->type == VIR_STORAGE_TYPE_FILE && + 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 && + def->os.loader->nvram->type == VIR_STORAGE_TYPE_FILE && 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 1f1cce8b3d..5803f9ff8d 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 && + ctl->def->os.loader->nvram->type == VIR_STORAGE_TYPE_FILE) + 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 acd18494d3..d6a482d28b 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -994,10 +994,10 @@ vboxSetBootDeviceOrder(virDomainDef *def, struct _vboxDriver *data, VIR_DEBUG("def->os.cmdline %s", def->os.cmdline); VIR_DEBUG("def->os.root %s", def->os.root); if (def->os.loader) { - 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); + 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->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); diff --git a/tests/qemuxml2argvdata/bios-nvram-network.args b/tests/qemuxml2argvdata/bios-nvram-network.args new file mode 100644 index 0000000000..05075b45de --- /dev/null +++ b/tests/qemuxml2argvdata/bios-nvram-network.args @@ -0,0 +1,35 @@ +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 \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name guest=test-bios,debug-threads=on \ +-S \ +-object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-test-bios/master-key.aes \ +-machine pc,usb=off,dump-guest-core=off \ +-accel tcg \ +-drive file=/usr/share/OVMF/OVMF_CODE.fd,if=pflash,format=raw,unit=0,readonly=on \ +-drive file=iscsi://example.org:6000/iqn.1992-01.com.example,if=pflash,format=raw,unit=1 \ +-m 1024 \ +-realtime mlock=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,path=/tmp/lib/domain--1-test-bios/monitor.sock,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot menu=on,strict=on \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ +-device usb-tablet,id=input0,bus=usb.0,port=1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/bios-nvram-network.xml b/tests/qemuxml2argvdata/bios-nvram-network.xml new file mode 100644 index 0000000000..0e1e23fd31 --- /dev/null +++ b/tests/qemuxml2argvdata/bios-nvram-network.xml @@ -0,0 +1,40 @@ +<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> + <source protocol='iscsi' name='iqn.1992-01.com.example'> + <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> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='tablet' bus='usb'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index e7fecb24d3..7ec45ec74b 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1189,6 +1189,7 @@ mymain(void) QEMU_CAPS_DEVICE_ISA_SERIAL); DO_TEST_NOCAPS("bios-nvram"); DO_TEST_PARSE_ERROR_NOCAPS("bios-nvram-no-path"); + DO_TEST_NOCAPS("bios-nvram-network"); DO_TEST_CAPS_LATEST("bios-nvram-rw"); DO_TEST_CAPS_LATEST("bios-nvram-rw-implicit"); DO_TEST("bios-nvram-secure", -- 2.25.1

2 3

[PATCH] virConnectDomainEventRegisterAny: correct docs
by John Levon 28 Mar '22

28 Mar '22

The callback ID can be zero, not necessarily positive; correct the comment to reflect this. Signed-off-by: John Levon <levon(a)movementarian.org> --- src/libvirt-domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index a197618673..53efb60406 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -9486,7 +9486,7 @@ virDomainMigrateStartPostCopy(virDomainPtr domain, * The reference can be released once the object is no longer required * by calling virDomainFree(). * - * The return value from this method is a positive integer identifier + * The return value from this method is a non-negative integer identifier * for the callback. To unregister a callback, this callback ID should * be passed to the virConnectDomainEventDeregisterAny() method. * -- 2.32.0

3 2

[PATCH] virfile: Report error when changing pipe size fails
by Michal Privoznik 28 Mar '22

28 Mar '22

When changing the size of pipe that virFileWrapperFdNew() creates we start at 1MiB and if that fails because it's above the system wide limit we get EPERM and continue with half of the size. However, we might get another error in which case we should report proper system error and return failure from virFileWrapperFdNew(). Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/util/virfile.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/util/virfile.c b/src/util/virfile.c index 12b359d550..130b0fbace 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -217,7 +217,7 @@ struct _virFileWrapperFd { * * OS note: only for linux, on other OS this is a no-op. */ -static void +static int virFileWrapperSetPipeSize(int fd) { int sz; @@ -230,21 +230,24 @@ virFileWrapperSetPipeSize(int fd) continue; /* retry with half the size */ } if (rv < 0) { - break; + virReportSystemError(errno, "%s", + _("unable to set pipe size")); + return -1; } VIR_DEBUG("fd %d pipe size adjusted to %d", fd, sz); - return; + return 0; } VIR_WARN("unable to set pipe size, data transfer might be slow: %s", g_strerror(errno)); + return 0; } # else /* !__linux__ */ -static void +static int virFileWrapperSetPipeSize(int fd G_GNUC_UNUSED) { - return; + return 0; } # endif /* !__linux__ */ @@ -323,6 +326,9 @@ virFileWrapperFdNew(int *fd, const char *name, unsigned int flags) if (virPipe(pipefd) < 0) goto error; + if (virFileWrapperSetPipeSize(pipefd[output]) < 0) + goto error; + if (!(iohelper_path = virFileFindResource("libvirt_iohelper", abs_top_builddir "/src", LIBEXECDIR))) @@ -330,8 +336,6 @@ virFileWrapperFdNew(int *fd, const char *name, unsigned int flags) ret->cmd = virCommandNewArgList(iohelper_path, name, NULL); - virFileWrapperSetPipeSize(pipefd[output]); - if (output) { virCommandSetInputFD(ret->cmd, pipefd[0]); virCommandSetOutputFD(ret->cmd, fd); -- 2.34.1

3 3

Request to move my GitLab custom executor repo under the libvirt umbrella
by Erik Skultety 28 Mar '22

28 Mar '22

So, we successfully enabled functional testing utilizing GitLab's custom executor feature. The code responsible for creating template images and provisioning and tearing down the VM workers currently lives under my GitLab account [1] and I think it would be better if we hosted it under the libvirt umbrella, especially since the project should serve as a uniform way of setting up custom runners for anyone in the libvirt community. If you think this is not a good idea, then please speak up and raise your concerns. Note there's still quite some work to be done with the project in general, but I'd be happy for any feedback in terms of what needs to be done prior moving the project to the libvirt namespace. Regards, Erik [1] https://gitlab.com/eskultety/libvirt-gitlab-executor

2 3

[libvirt RFCv4] virfile: set pipe size in virFileWrapperFdNew to improve throughput
by Claudio Fontana 28 Mar '22

28 Mar '22

currently the only user of virFileWrapperFdNew is the qemu driver; virsh save is very slow with a default pipe size. This change improves throughput by ~400% on fast nvme or ramdisk. Best value currently measured is 1MB, which happens to be also the kernel default for the pipe-max-size. Signed-off-by: Claudio Fontana <cfontana(a)suse.de> --- src/util/virfile.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) see v2 at https://listman.redhat.com/archives/libvir-list/2022-March/229423.html Changes v3 -> v4: * changed INFO and WARN messages to DEBUG (Daniel) Changes v2 -> v3: * removed reading of max-pipe-size from procfs, instead make multiple attempts on EPERM with smaller sizes. In the regular case, this should succeed on the first try. (Daniel) Changes v1 -> v2: * removed VIR_FILE_WRAPPER_BIG_PIPE, made the new pipe resizing unconditional (Michal) * moved code to separate functions (Michal) * removed ternary op, disliked in libvirt (Michal) * added #ifdef __linux__ (Ani Sinha) * try smallest value between currently best measured value (1MB) and the pipe-max-size setting. If pipe-max-size cannot be read, try kernel default max (1MB). (Daniel) diff --git a/src/util/virfile.c b/src/util/virfile.c index a04f888e06..87539be0b9 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -201,6 +201,50 @@ struct _virFileWrapperFd { }; #ifndef WIN32 + +#ifdef __linux__ + +/** + * virFileWrapperSetPipeSize: + * @fd: the fd of the pipe + * + * Set best pipe size on the passed file descriptor for bulk transfers of data. + * + * default pipe size (usually 64K) is generally not suited for large transfers + * to fast devices. A value of 1MB has been measured to improve virsh save + * by 400% in ideal conditions. We retry multiple times with smaller sizes + * on EPERM to account for possible small values of /proc/sys/fs/pipe-max-size. + * + * OS note: only for linux, on other OS this is a no-op. + */ +static void +virFileWrapperSetPipeSize(int fd) +{ + int sz; + + for (sz = 1024 * 1024; sz >= 64 * 1024; sz /= 2) { + int rv = fcntl(fd, F_SETPIPE_SZ, sz); + if (rv < 0 && errno == EPERM) { + VIR_DEBUG("EPERM trying to set fd %d pipe size to %d", fd, sz); + continue; /* retry with half the size */ + } + if (rv < 0) { + break; + } + VIR_DEBUG("fd %d pipe size adjusted to %d", fd, sz); + return; + } + virReportSystemError(errno, "%s", _("unable to set pipe size, data transfer might be slow")); +} + +#else /* !__linux__ */ +static void virFileWrapperSetPipeSize(int fd) +{ + return; +} +#endif /* !__linux__ */ + + /** * virFileWrapperFdNew: * @fd: pointer to fd to wrap @@ -282,6 +326,8 @@ virFileWrapperFdNew(int *fd, const char *name, unsigned int flags) ret->cmd = virCommandNewArgList(iohelper_path, name, NULL); + virFileWrapperSetPipeSize(pipefd[!output]); + if (output) { virCommandSetInputFD(ret->cmd, pipefd[0]); virCommandSetOutputFD(ret->cmd, fd); -- 2.35.1

3 4

[build-fix PATCH] util: virfile: Fix indentation of preprocessor directives
by Peter Krempa 28 Mar '22

28 Mar '22

stderr: cppi: /home/pipo/libvirt/src/util/virfile.c: line 205: not properly indented cppi: /home/pipo/libvirt/src/util/virfile.c: line 243: not properly indented cppi: /home/pipo/libvirt/src/util/virfile.c: line 249: not properly indented build-aux/syntax-check.mk: incorrect preprocessor indentation make: *** [/home/pipo/libvirt/build-aux/syntax-check.mk:565: sc_preprocessor_indentation] Error 1 Fixes: c61d1e9ba0a0bec18fdb0bdd485060dc27a4e5cc Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- Pushed. src/util/virfile.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/util/virfile.c b/src/util/virfile.c index 6f2af35201..12b359d550 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -202,7 +202,7 @@ struct _virFileWrapperFd { #ifndef WIN32 -#ifdef __linux__ +# ifdef __linux__ /** * virFileWrapperSetPipeSize: @@ -240,13 +240,13 @@ virFileWrapperSetPipeSize(int fd) g_strerror(errno)); } -#else /* !__linux__ */ +# else /* !__linux__ */ static void virFileWrapperSetPipeSize(int fd G_GNUC_UNUSED) { return; } -#endif /* !__linux__ */ +# endif /* !__linux__ */ /** -- 2.35.1

1 0

[PATCH v2 0/6] qemu: Add qemu-namespace XML to override device properties (-set replacement)
by Peter Krempa 28 Mar '22

28 Mar '22

v2: - patch 1/7 from last time was pushed - XML changes: - top level element renamed to "qemu:override" to accomodate possible extension in the future - wrapped 'qemu:props' meant to override the frontend under a 'qemu:frontend' element - code changes: - renamed few bits (dropped 'Device') from code that can theoretically be reused - split out some functions for enhanced extensibility in the future Peter Krempa (6): docs: drvqemu: Document overriding of device properties qemu: domain: Add XML namespace code for overriding device config conf: Introduce VIR_DOMAIN_TAINT_CUSTOM_DEVICE and use it in qemu qemuBuildDeviceCommandlineFromJSON: Pass 'virDomainDef' into the function qemu: command: Override device definition according to the namespace config NEWS: Mention the qemu device property override feature NEWS.rst | 7 + docs/drvqemu.rst | 57 +++++ src/conf/domain_conf.c | 2 + src/conf/domain_conf.h | 1 + src/conf/schemas/domaincommon.rng | 42 ++++ src/qemu/qemu_command.c | 133 +++++++---- src/qemu/qemu_domain.c | 221 ++++++++++++++++++ src/qemu/qemu_domain.h | 33 +++ .../qemu-ns.x86_64-4.0.0.args | 2 +- .../qemu-ns.x86_64-latest.args | 2 +- tests/qemuxml2argvdata/qemu-ns.xml | 14 ++ .../qemu-ns.x86_64-latest.xml | 14 ++ 12 files changed, 478 insertions(+), 50 deletions(-) -- 2.35.1

2 8

[PATCH 0/2] Revert two patches I've pushed mistakenly
by Michal Privoznik 28 Mar '22

28 Mar '22

Firstly, QEMU part is not merged which alone should have prevented me from pushing these patches. Secondly, in the end QEMU is fixing the bug in a different manner that does not require any libvirt work. https://lists.gnu.org/archive/html/qemu-devel/2022-03/msg06077.html https://lists.gnu.org/archive/html/qemu-devel/2022-03/msg06188.html Michal Prívozník (2): Revert "domain: add tsc.on_reboot element" Revert "qemu: add support for tsc.on_reboot element" docs/formatdomain.rst | 4 -- src/conf/domain_conf.c | 17 ------- src/conf/domain_conf.h | 10 ----- src/conf/schemas/domaincommon.rng | 8 ---- src/qemu/qemu_capabilities.c | 2 - src/qemu/qemu_capabilities.h | 1 - src/qemu/qemu_command.c | 11 ----- src/qemu/qemu_validate.c | 45 ++++++++----------- .../caps_7.0.0.x86_64.replies | 4 -- .../caps_7.0.0.x86_64.xml | 1 - .../cpu-tsc-clear-on-reset.args | 32 ------------- .../cpu-tsc-clear-on-reset.x86_64-7.0.0.args | 34 -------------- .../cpu-tsc-clear-on-reset.xml | 35 --------------- tests/qemuxml2argvtest.c | 2 - 14 files changed, 19 insertions(+), 187 deletions(-) delete mode 100644 tests/qemuxml2argvdata/cpu-tsc-clear-on-reset.args delete mode 100644 tests/qemuxml2argvdata/cpu-tsc-clear-on-reset.x86_64-7.0.0.args delete mode 100644 tests/qemuxml2argvdata/cpu-tsc-clear-on-reset.xml -- 2.34.1

2 3

[PATCH v2 0/7] Do async IO when starting swtpm emulator
by Michal Privoznik 28 Mar '22

28 Mar '22

v2 of: https://listman.redhat.com/archives/libvir-list/2022-March/229433.html diff to v1: - More fixes - New test case, yay! Michal Prívozník (7): qemu_tpm: Do async IO when starting swtpm emulator vircommand: Document virCommandSetSendBuffer() behaviour wrt daemonize vircommand: Don't set nonblocking FDs in virCommandDoAsyncIO() commandtest: Use unsigned char in test27() virCommandSetSendBuffer: Take double pointer of @buffer commandtest: Test virCommandSetSendBuffer() with virCommandDoAsyncIO() commandtest: Use virTestCompareToFile() in checkoutput() src/qemu/qemu_tpm.c | 3 +- src/util/vircommand.c | 12 ++-- src/util/vircommand.h | 2 +- tests/commanddata/test29.log | 19 ++++++ tests/commandtest.c | 110 +++++++++++++++++++++++++++++------ 5 files changed, 121 insertions(+), 25 deletions(-) create mode 100644 tests/commanddata/test29.log -- 2.34.1

4 10

[PATCH 0/2] domain, qemu: add support for clearing TSC on reset
by Paolo Bonzini 28 Mar '22

28 Mar '22

Some versions of Windows hang on reboot if their TSC value is greater than 2^54. The workaround is to reset the TSC to a small value, and is implemented by QEMU and ESXi. This series adds a domain attribute for this, and implements it in QEMU. Paolo Paolo Bonzini (2): domain: add tsc.on_reboot element qemu: add support for tsc.on_reboot element docs/formatdomain.rst | 4 +++ src/conf/domain_conf.c | 22 ++++++++++++ src/conf/domain_conf.h | 10 ++++++ src/conf/schemas/domaincommon.rng | 9 +++++ src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 4 +++ src/qemu/qemu_validate.c | 36 +++++++++++++------ .../caps_7.0.0.x86_64.replies | 4 +++ .../caps_7.0.0.x86_64.xml | 1 + ...uency.args => cpu-tsc-clear-on-reset.args} | 2 +- ... cpu-tsc-clear-on-reset.x86_64-7.0.0.args} | 6 ++-- ...equency.xml => cpu-tsc-clear-on-reset.xml} | 2 +- tests/qemuxml2argvtest.c | 2 ++ 14 files changed, 89 insertions(+), 16 deletions(-) copy tests/qemuxml2argvdata/{cpu-tsc-frequency.args => cpu-tsc-clear-on-reset.args} (97%) copy tests/qemuxml2argvdata/{virtio-rng-builtin.x86_64-latest.args => cpu-tsc-clear-on-reset.x86_64-7.0.0.args} (77%) copy tests/qemuxml2argvdata/{cpu-tsc-frequency.xml => cpu-tsc-clear-on-reset.xml} (95%) -- 2.35.1

2 8