On 03/12/2020 13.36, Michal Privoznik wrote: Reviewed-by: Thomas Huth <thuth(a)redhat.com&gt;

The structure has two sets of members: some for the target and some for the source. The latter is model dependant (e.g. path to a nvdimm device applies only to NVDIMM model). Move the members into an union so that it is obvious which members apply to which model. This way it's easier to maintain code cleanliness when introducing a new model. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/conf/domain_conf.c | 57 ++++++++++++++++++------------ src/conf/domain_conf.h | 16 ++++++--- src/qemu/qemu_alias.c | 13 +++++-- src/qemu/qemu_cgroup.c | 10 +++--- src/qemu/qemu_command.c | 60 +++++++++++++++++++++----------- src/qemu/qemu_namespace.c | 2 +- src/qemu/qemu_process.c | 8 ++--- src/security/security_apparmor.c | 6 ++-- src/security/security_dac.c | 4 +-- src/security/security_selinux.c | 4 +-- src/security/virt-aa-helper.c | 2 +- 11 files changed, 113 insertions(+), 69 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7d9e5d14ad..4d462b0627 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3136,8 +3136,18 @@ void virDomainMemoryDefFree(virDomainMemoryDefPtr def) if (!def) return; - VIR_FREE(def->nvdimmPath); - virBitmapFree(def->sourceNodes); + switch (def->model) { + case VIR_DOMAIN_MEMORY_MODEL_DIMM: + virBitmapFree(def->s.dimm.sourceNodes); + break; + case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: + VIR_FREE(def->s.nvdimm.path); + break; + case VIR_DOMAIN_MEMORY_MODEL_NONE: + case VIR_DOMAIN_MEMORY_MODEL_LAST: + break; + } + virDomainDeviceInfoClear(&def->info); VIR_FREE(def); } @@ -6694,7 +6704,7 @@ virDomainMemoryDefValidate(const virDomainMemoryDef *mem, const virDomainDef *def) { if (mem->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) { - if (!mem->nvdimmPath) { + if (!mem->s.nvdimm.path) { virReportError(VIR_ERR_XML_DETAIL, "%s", _("path is required for model 'nvdimm'")); return -1; @@ -16679,15 +16689,15 @@ virDomainMemorySourceDefParseXML(xmlNodePtr node, switch (def->model) { case VIR_DOMAIN_MEMORY_MODEL_DIMM: if (virDomainParseMemory("./pagesize", "./pagesize/@unit", ctxt, - &def->pagesize, false, false) < 0) + &def->s.dimm.pagesize, false, false) < 0) return -1; if ((nodemask = virXPathString("string(./nodemask)", ctxt))) { - if (virBitmapParse(nodemask, &def->sourceNodes, + if (virBitmapParse(nodemask, &def->s.dimm.sourceNodes, VIR_DOMAIN_CPUMASK_LEN) < 0) return -1; - if (virBitmapIsAllClear(def->sourceNodes)) { + if (virBitmapIsAllClear(def->s.dimm.sourceNodes)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Invalid value of 'nodemask': %s"), nodemask); return -1; @@ -16696,14 +16706,14 @@ virDomainMemorySourceDefParseXML(xmlNodePtr node, break; case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: - def->nvdimmPath = virXPathString("string(./path)", ctxt); + def->s.nvdimm.path = virXPathString("string(./path)", ctxt); if (virDomainParseMemory("./alignsize", "./alignsize/@unit", ctxt, - &def->alignsize, false, false) < 0) + &def->s.nvdimm.alignsize, false, false) < 0) return -1; if (virXPathBoolean("boolean(./pmem)", ctxt)) - def->nvdimmPmem = true; + def->s.nvdimm.pmem = true; break; @@ -18583,15 +18593,15 @@ virDomainMemoryFindByDefInternal(virDomainDefPtr def, switch (mem->model) { case VIR_DOMAIN_MEMORY_MODEL_DIMM: /* source stuff -> match with device */ - if (tmp->pagesize != mem->pagesize) + if (tmp->s.dimm.pagesize != mem->s.dimm.pagesize) continue; - if (!virBitmapEqual(tmp->sourceNodes, mem->sourceNodes)) + if (!virBitmapEqual(tmp->s.dimm.sourceNodes, mem->s.dimm.sourceNodes)) continue; break; case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: - if (STRNEQ(tmp->nvdimmPath, mem->nvdimmPath)) + if (STRNEQ(tmp->s.nvdimm.path, mem->s.nvdimm.path)) continue; break; @@ -24186,15 +24196,15 @@ virDomainMemoryDefCheckABIStability(virDomainMemoryDefPtr src, return false; } - if (src->alignsize != dst->alignsize) { + if (src->s.nvdimm.alignsize != dst->s.nvdimm.alignsize) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target NVDIMM alignment '%llu' doesn't match " "source NVDIMM alignment '%llu'"), - src->alignsize, dst->alignsize); + src->s.nvdimm.alignsize, dst->s.nvdimm.alignsize); return false; } - if (src->nvdimmPmem != dst->nvdimmPmem) { + if (src->s.nvdimm.pmem != dst->s.nvdimm.pmem) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Target NVDIMM pmem flag doesn't match " "source NVDIMM pmem flag")); @@ -27844,26 +27854,27 @@ virDomainMemorySourceDefFormat(virBufferPtr buf, switch (def->model) { case VIR_DOMAIN_MEMORY_MODEL_DIMM: - if (def->sourceNodes) { - if (!(bitmap = virBitmapFormat(def->sourceNodes))) + if (def->s.dimm.sourceNodes) { + if (!(bitmap = virBitmapFormat(def->s.dimm.sourceNodes))) return -1; virBufferAsprintf(&childBuf, "<nodemask>%s</nodemask>\n", bitmap); } - if (def->pagesize) + if (def->s.dimm.pagesize) { virBufferAsprintf(&childBuf, "<pagesize unit='KiB'>%llu</pagesize>\n", - def->pagesize); + def->s.dimm.pagesize); + } break; case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: - virBufferEscapeString(&childBuf, "<path>%s</path>\n", def->nvdimmPath); + virBufferEscapeString(&childBuf, "<path>%s</path>\n", def->s.nvdimm.path); - if (def->alignsize) + if (def->s.nvdimm.alignsize) virBufferAsprintf(&childBuf, "<alignsize unit='KiB'>%llu</alignsize>\n", - def->alignsize); + def->s.nvdimm.alignsize); - if (def->nvdimmPmem) + if (def->s.nvdimm.pmem) virBufferAddLit(&childBuf, "<pmem/>\n"); break; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 5853e3b290..e7f8fc156f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2315,11 +2315,17 @@ struct _virDomainMemoryDef { virTristateBool discard; /* source */ - virBitmapPtr sourceNodes; - unsigned long long pagesize; /* kibibytes */ - char *nvdimmPath; - unsigned long long alignsize; /* kibibytes; valid only for NVDIMM */ - bool nvdimmPmem; /* valid only for NVDIMM */ + union { + struct { + virBitmapPtr sourceNodes; + unsigned long long pagesize; /* kibibytes */ + } dimm; /* VIR_DOMAIN_MEMORY_MODEL_DIMM */ + struct { + char *path; + unsigned long long alignsize; /* kibibytes */ + bool pmem; + } nvdimm; /* VIR_DOMAIN_MEMORY_MODEL_NVDIMM */ + } s; /* target */ virDomainMemoryModel model; diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index dcb6c7156d..5ebcd766a9 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -486,15 +486,22 @@ qemuAssignDeviceMemoryAlias(virDomainDefPtr def, size_t i; int maxidx = 0; int idx; - const char *prefix; + const char *prefix = NULL; if (mem->info.alias) return 0; - if (mem->model == VIR_DOMAIN_MEMORY_MODEL_DIMM) + switch (mem->model) { + case VIR_DOMAIN_MEMORY_MODEL_DIMM: prefix = "dimm"; - else + break; + case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: prefix = "nvdimm"; + break; + case VIR_DOMAIN_MEMORY_MODEL_NONE: + case VIR_DOMAIN_MEMORY_MODEL_LAST: + break; + } if (oldAlias) { for (i = 0; i < def->nmems; i++) { diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index f7146a71c9..92caadf840 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -508,11 +508,11 @@ qemuSetupMemoryDevicesCgroup(virDomainObjPtr vm, if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) return 0; - VIR_DEBUG("Setting devices Cgroup for NVDIMM device: %s", mem->nvdimmPath); - rv = virCgroupAllowDevicePath(priv->cgroup, mem->nvdimmPath, + VIR_DEBUG("Setting devices Cgroup for NVDIMM device: %s", mem->s.nvdimm.path); + rv = virCgroupAllowDevicePath(priv->cgroup, mem->s.nvdimm.path, VIR_CGROUP_DEVICE_RW, false); virDomainAuditCgroupPath(vm, priv->cgroup, "allow", - mem->nvdimmPath, "rw", rv); + mem->s.nvdimm.path, "rw", rv); return rv; } @@ -531,10 +531,10 @@ qemuTeardownMemoryDevicesCgroup(virDomainObjPtr vm, if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) return 0; - rv = virCgroupDenyDevicePath(priv->cgroup, mem->nvdimmPath, + rv = virCgroupDenyDevicePath(priv->cgroup, mem->s.nvdimm.path, VIR_CGROUP_DEVICE_RWM, false); virDomainAuditCgroupPath(vm, priv->cgroup, - "deny", mem->nvdimmPath, "rwm", rv); + "deny", mem->s.nvdimm.path, "rwm", rv); return rv; } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index eb64ce84da..4bd45e0638 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2958,10 +2958,13 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps, int rc; g_autoptr(virJSONValue) props = NULL; bool nodeSpecified = virDomainNumatuneNodeSpecified(def->numa, mem->targetNode); - unsigned long long pagesize = mem->pagesize; - bool needHugepage = !!pagesize; - bool useHugepage = !!pagesize; + unsigned long long pagesize = 0; + bool needHugepage = false; + bool useHugepage = false; int discard = mem->discard; + const char *nvdimmPath = NULL; + unsigned long long alignsize = 0; + bool nvdimmPmem = false; /* The difference between @needHugepage and @useHugepage is that the latter * is true whenever huge page is defined for the current memory cell. @@ -2971,6 +2974,23 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps, *backendProps = NULL; + switch (mem->model) { + case VIR_DOMAIN_MEMORY_MODEL_DIMM: + pagesize = mem->s.dimm.pagesize; + needHugepage = !!pagesize; + useHugepage = !!pagesize; + nodemask = mem->s.dimm.sourceNodes; + break; + case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: + nvdimmPath = mem->s.nvdimm.path; + alignsize = mem->s.nvdimm.alignsize; + nvdimmPmem = mem->s.nvdimm.pmem; + break; + case VIR_DOMAIN_MEMORY_MODEL_NONE: + case VIR_DOMAIN_MEMORY_MODEL_LAST: + break; + } + if (mem->targetNode >= 0) { /* memory devices could provide a invalid guest node */ if (mem->targetNode >= virDomainNumaGetNodeCount(def->numa)) { @@ -3076,11 +3096,11 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps, if (qemuBuildMemoryBackendPropsShare(props, memAccess) < 0) return -1; - } else if (useHugepage || mem->nvdimmPath || memAccess || - def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_FILE) { + } else if (useHugepage || nvdimmPath || memAccess || + def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_FILE) { - if (mem->nvdimmPath) { - memPath = g_strdup(mem->nvdimmPath); + if (nvdimmPath) { + memPath = g_strdup(nvdimmPath); prealloc = true; } else if (useHugepage) { if (qemuGetDomainHupageMemPath(priv->driver, def, pagesize, &memPath) < 0) @@ -3098,7 +3118,7 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps, NULL) < 0) return -1; - if (!mem->nvdimmPath && + if (!nvdimmPath && discard == VIR_TRISTATE_BOOL_YES) { if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE_DISCARD)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -3125,18 +3145,18 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps, if (virJSONValueObjectAdd(props, "U:size", mem->size * 1024, NULL) < 0) return -1; - if (mem->alignsize) { + if (alignsize) { if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE_ALIGN)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("nvdimm align property is not available " "with this QEMU binary")); return -1; } - if (virJSONValueObjectAdd(props, "U:align", mem->alignsize * 1024, NULL) < 0) + if (virJSONValueObjectAdd(props, "U:align", alignsize * 1024, NULL) < 0) return -1; } - if (mem->nvdimmPmem) { + if (nvdimmPmem) { if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE_PMEM)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("nvdimm pmem property is not available " @@ -3147,13 +3167,10 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps, return -1; } - if (mem->sourceNodes) { - nodemask = mem->sourceNodes; - } else { - if (virDomainNumatuneMaybeGetNodeset(def->numa, priv->autoNodeset, - &nodemask, mem->targetNode) < 0) - return -1; - } + if (!nodemask && + virDomainNumatuneMaybeGetNodeset(def->numa, priv->autoNodeset, + &nodemask, mem->targetNode) < 0) + return -1; if (nodemask) { if (!virNumaNodesetIsAvailable(nodemask)) @@ -3166,8 +3183,11 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps, } /* If none of the following is requested... */ - if (!needHugepage && !mem->sourceNodes && !nodeSpecified && - !mem->nvdimmPath && + if (!needHugepage && + !(mem->model == VIR_DOMAIN_MEMORY_MODEL_DIMM && + mem->s.dimm.sourceNodes) && + !nodeSpecified && + !nvdimmPath && memAccess == VIR_DOMAIN_MEMORY_ACCESS_DEFAULT && def->mem.source != VIR_DOMAIN_MEMORY_SOURCE_FILE && def->mem.source != VIR_DOMAIN_MEMORY_SOURCE_MEMFD && diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c index 1002455ddf..b8aebd1e61 100644 --- a/src/qemu/qemu_namespace.c +++ b/src/qemu/qemu_namespace.c @@ -354,7 +354,7 @@ qemuDomainSetupMemory(virDomainMemoryDefPtr mem, if (mem->model != VIR_DOMAIN_MEMORY_MODEL_NVDIMM) return 0; - return virStringListAdd(paths, mem->nvdimmPath); + return virStringListAdd(paths, mem->s.nvdimm.path); } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 20e90026e1..8ea7e0df05 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3885,15 +3885,15 @@ qemuProcessNeedHugepagesPath(virDomainDefPtr def, for (i = 0; i < def->nmems; i++) { if (def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_DIMM && - def->mems[i]->pagesize && - def->mems[i]->pagesize != system_pagesize) + def->mems[i]->s.dimm.pagesize && + def->mems[i]->s.dimm.pagesize != system_pagesize) return true; } if (mem && mem->model == VIR_DOMAIN_MEMORY_MODEL_DIMM && - mem->pagesize && - mem->pagesize != system_pagesize) + mem->s.dimm.pagesize && + mem->s.dimm.pagesize != system_pagesize) return true; return false; diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index eed66e460f..78136e751e 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -686,13 +686,13 @@ AppArmorSetMemoryLabel(virSecurityManagerPtr mgr, switch (mem->model) { case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: - if (!virFileExists(mem->nvdimmPath)) { + if (!virFileExists(mem->s.nvdimm.path)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("%s: \'%s\' does not exist"), - __func__, mem->nvdimmPath); + __func__, mem->s.nvdimm.path); return -1; } - return reload_profile(mgr, def, mem->nvdimmPath, true); + return reload_profile(mgr, def, mem->s.nvdimm.path, true); case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_DIMM: case VIR_DOMAIN_MEMORY_MODEL_LAST: diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 4f4a0a069e..44ee42e1bd 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1889,7 +1889,7 @@ virSecurityDACRestoreMemoryLabel(virSecurityManagerPtr mgr, switch (mem->model) { case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: - ret = virSecurityDACRestoreFileLabel(mgr, mem->nvdimmPath); + ret = virSecurityDACRestoreFileLabel(mgr, mem->s.nvdimm.path); break; case VIR_DOMAIN_MEMORY_MODEL_DIMM: @@ -2070,7 +2070,7 @@ virSecurityDACSetMemoryLabel(virSecurityManagerPtr mgr, return -1; ret = virSecurityDACSetOwnership(mgr, NULL, - mem->nvdimmPath, + mem->s.nvdimm.path, user, group, true); break; diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index e9cd95916e..294c9f1db5 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1577,7 +1577,7 @@ virSecuritySELinuxSetMemoryLabel(virSecurityManagerPtr mgr, if (!seclabel || !seclabel->relabel) return 0; - if (virSecuritySELinuxSetFilecon(mgr, mem->nvdimmPath, + if (virSecuritySELinuxSetFilecon(mgr, mem->s.nvdimm.path, seclabel->imagelabel, true) < 0) return -1; break; @@ -1606,7 +1606,7 @@ virSecuritySELinuxRestoreMemoryLabel(virSecurityManagerPtr mgr, if (!seclabel || !seclabel->relabel) return 0; - ret = virSecuritySELinuxRestoreFileLabel(mgr, mem->nvdimmPath, true); + ret = virSecuritySELinuxRestoreFileLabel(mgr, mem->s.nvdimm.path, true); break; case VIR_DOMAIN_MEMORY_MODEL_DIMM: diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 5a6f4a5f7d..a8a05a0a90 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1170,7 +1170,7 @@ get_files(vahControl * ctl) for (i = 0; i < ctl->def->nmems; i++) { if (ctl->def->mems[i] && ctl->def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) { - if (vah_add_file(&buf, ctl->def->mems[i]->nvdimmPath, "rw") != 0) + if (vah_add_file(&buf, ctl->def->mems[i]->s.nvdimm.path, "rw") != 0) goto cleanup; } } -- 2.26.2

The virtio-pmem is a virtio variant of NVDIMM and just like NVDIMM virtio-pmem also allows accessing host pages bypassing guest page cache. The difference is that if a regular file is used to back guest's NVDIMM (model='nvdimm') the persistence of guest writes might not be guaranteed while with virtio-pmem it is. To express this new model at domain XML level, I've chose the following: <memory model='virtio' access='shared'> <source> <path>/tmp/virtio_pmem</path> <pmem/> </source> <target> <size unit='KiB'>524288</size> </target> <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> </memory> The <source/> looks like for model='nvdimm', except <pmem/> is required - this is to allow for future expansion when the model='virtio' without <pmem/> will describe virtio-mem (which is a different device). Another difference between NVDIMM and virtio-pmem is that while the former supports NUMA node locality the latter doesn't. And also, the latter goes onto PCI bus and not into a DIMM module. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- docs/formatdomain.rst | 35 +++++- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 113 ++++++++++++++++-- src/conf/domain_conf.h | 5 + src/qemu/qemu_alias.c | 1 + src/qemu/qemu_command.c | 2 + src/qemu/qemu_domain.c | 26 ++++ src/qemu/qemu_domain_address.c | 88 +++++++++++--- src/qemu/qemu_domain_address.h | 3 +- src/qemu/qemu_hotplug.c | 2 +- src/qemu/qemu_validate.c | 1 + src/security/security_apparmor.c | 1 + src/security/security_dac.c | 2 + src/security/security_selinux.c | 2 + .../memory-hotplug-virtio-pmem.xml | 54 +++++++++ ...mory-hotplug-virtio-pmem.x86_64-latest.xml | 1 + tests/qemuxml2xmltest.c | 2 + 17 files changed, 304 insertions(+), 35 deletions(-) create mode 100644 tests/qemuxml2argvdata/memory-hotplug-virtio-pmem.xml create mode 120000 tests/qemuxml2xmloutdata/memory-hotplug-virtio-pmem.x86_64-latest.xml diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 512939679b..ca6bc0432e 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -7187,20 +7187,31 @@ Example: usage of the memory devices </label> </target> </memory> + <memory model='virtio' access='shared'> + <source> + <path>/tmp/virtio_pmem</path> + <pmem/> + </source> + <target> + <size unit='KiB'>524288</size> + </target> + </memory> </devices> ... ``model`` Provide ``dimm`` to add a virtual DIMM module to the guest. :since:`Since - 1.2.14` Provide ``nvdimm`` model adds a Non-Volatile DIMM module. - :since:`Since 3.2.0` + 1.2.14` Provide ``nvdimm`` model that adds a Non-Volatile DIMM module. + :since:`Since 3.2.0` Provide ``virtio`` model to add a paravirtualized + (persistent) memory device. :since:`Since 7.0.0` ``access`` An optional attribute ``access`` ( :since:`since 3.2.0` ) that provides capability to fine tune mapping of the memory on per module basis. Values are the same as `Memory Backing <#elementsMemoryBacking>`__: ``shared`` and ``private``. For ``nvdimm`` model, if using real NVDIMM DAX device as - backend, ``shared`` is required. + backend, ``shared`` is required. For ``virtio`` model with ``<pmem/>`` + ``shared`` is required. ``discard`` An optional attribute ``discard`` ( :since:`since 4.4.0` ) that provides @@ -7229,9 +7240,9 @@ Example: usage of the memory devices This element can be used to override the default set of NUMA nodes where the memory would be allocated. - For model ``nvdimm`` this element is mandatory. The mandatory child element - ``path`` represents a path in the host that backs the nvdimm module in the - guest. The following optional elements may be used: + For model ``nvdimm`` the ``source`` element is mandatory. The mandatory + child element ``path`` represents a path in the host that backs the nvdimm + module in the guest. The following optional elements may be used: ``alignsize`` The ``alignsize`` element defines the page size alignment used to mmap the @@ -7245,6 +7256,18 @@ Example: usage of the memory devices to guarantee the persistence of writes to the vNVDIMM backend, then use the ``pmem`` element in order to utilize the feature. :since:`Since 5.0.0` + For model ``virtio`` the ``source`` element is mandatory. The following + optional elements may be used: + + ``path`` + Represents a path in the host that backs the virtio memory module in the + guest. It is mandatory for persistent memory (i.e. when ``pmem`` is also + set). + + ``pmem`` + If persistence of data is required then this element must be provided. + + ``target`` The mandatory ``target`` element configures the placement and sizing of the added memory from the perspective of the guest. diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 795b654feb..b385bae84c 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -5973,6 +5973,7 @@ <choice> <value>dimm</value> <value>nvdimm</value> + <value>virtio</value> </choice> </attribute> <optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4d462b0627..935bea1804 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1332,6 +1332,7 @@ VIR_ENUM_IMPL(virDomainMemoryModel, "", "dimm", "nvdimm", + "virtio", ); VIR_ENUM_IMPL(virDomainShmemModel, @@ -3143,6 +3144,9 @@ void virDomainMemoryDefFree(virDomainMemoryDefPtr def) case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: VIR_FREE(def->s.nvdimm.path); break; + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO: + VIR_FREE(def->s.virtio.path); + break; case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_LAST: break; @@ -5373,15 +5377,31 @@ static int virDomainMemoryDefPostParse(virDomainMemoryDefPtr mem, const virDomainDef *def) { - /* Although only the QEMU driver implements PPC64 support, this - * code is related to the platform specification (PAPR), i.e. it - * is hypervisor agnostic, and any future PPC64 hypervisor driver - * will have the same restriction. - */ - if (ARCH_IS_PPC64(def->os.arch) && - mem->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM && - virDomainNVDimmAlignSizePseries(mem) < 0) - return -1; + switch (mem->model) { + case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: + /* Although only the QEMU driver implements PPC64 support, this + * code is related to the platform specification (PAPR), i.e. it + * is hypervisor agnostic, and any future PPC64 hypervisor driver + * will have the same restriction. + */ + if (ARCH_IS_PPC64(def->os.arch) && + virDomainNVDimmAlignSizePseries(mem) < 0) + return -1; + break; + + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO: + /* Virtio-pmem mandates shared access so that guest writes get + * reflected in the underlying file. */ + if (mem->s.virtio.pmem && + mem->access == VIR_DOMAIN_MEMORY_ACCESS_DEFAULT) + mem->access = VIR_DOMAIN_MEMORY_ACCESS_SHARED; + break; + + case VIR_DOMAIN_MEMORY_MODEL_DIMM: + case VIR_DOMAIN_MEMORY_MODEL_NONE: + case VIR_DOMAIN_MEMORY_MODEL_LAST: + break; + } return 0; } @@ -6703,7 +6723,8 @@ static int virDomainMemoryDefValidate(const virDomainMemoryDef *mem, const virDomainDef *def) { - if (mem->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) { + switch (mem->model) { + case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: if (!mem->s.nvdimm.path) { virReportError(VIR_ERR_XML_DETAIL, "%s", _("path is required for model 'nvdimm'")); @@ -6721,6 +6742,39 @@ virDomainMemoryDefValidate(const virDomainMemoryDef *mem, _("label size is required for NVDIMM device")); return -1; } + break; + + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO: + if (mem->s.virtio.pmem) { + if (!mem->s.virtio.path) { + virReportError(VIR_ERR_XML_DETAIL, "%s", + _("path is required for model 'virtio'")); + return -1; + } + + if (mem->access != VIR_DOMAIN_MEMORY_ACCESS_SHARED) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("shared access mode required for virtio-pmem device")); + return -1; + } + + if (mem->targetNode != -1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("virtio-pmem does not support NUMA nodes")); + return -1; + } + } else { + /* TODO: plain virtio-mem behaves differently then virtio-pmem */ + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("virtio-mem is not supported yet. <pmem/> is required")); + return -1; + } + break; + + case VIR_DOMAIN_MEMORY_MODEL_NONE: + case VIR_DOMAIN_MEMORY_MODEL_DIMM: + case VIR_DOMAIN_MEMORY_MODEL_LAST: + break; } return 0; @@ -16717,6 +16771,14 @@ virDomainMemorySourceDefParseXML(xmlNodePtr node, break; + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO: + def->s.virtio.path = virXPathString("string(./path)", ctxt); + + if (virXPathBoolean("boolean(./pmem)", ctxt)) + def->s.virtio.pmem = true; + + break; + case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_LAST: break; @@ -18605,6 +18667,12 @@ virDomainMemoryFindByDefInternal(virDomainDefPtr def, continue; break; + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO: + if (tmp->s.virtio.pmem != mem->s.virtio.pmem || + STRNEQ_NULLABLE(tmp->s.virtio.path, mem->s.virtio.path)) + continue; + break; + case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_LAST: break; @@ -24187,7 +24255,8 @@ virDomainMemoryDefCheckABIStability(virDomainMemoryDefPtr src, return false; } - if (src->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) { + switch (src->model) { + case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: if (src->labelsize != dst->labelsize) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target NVDIMM label size '%llu' doesn't match " @@ -24223,6 +24292,21 @@ virDomainMemoryDefCheckABIStability(virDomainMemoryDefPtr src, _("Target NVDIMM UUID doesn't match source NVDIMM")); return false; } + break; + + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO: + if (src->s.virtio.pmem != dst->s.virtio.pmem) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Target NVDIMM pmem flag doesn't match " + "source NVDIMM pmem flag")); + return false; + } + break; + + case VIR_DOMAIN_MEMORY_MODEL_NONE: + case VIR_DOMAIN_MEMORY_MODEL_DIMM: + case VIR_DOMAIN_MEMORY_MODEL_LAST: + break; } return virDomainDeviceInfoCheckABIStability(&src->info, &dst->info); @@ -27878,6 +27962,13 @@ virDomainMemorySourceDefFormat(virBufferPtr buf, virBufferAddLit(&childBuf, "<pmem/>\n"); break; + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO: + virBufferEscapeString(&childBuf, "<path>%s</path>\n", def->s.virtio.path); + + if (def->s.virtio.pmem) + virBufferAddLit(&childBuf, "<pmem/>\n"); + break; + case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_LAST: break; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index e7f8fc156f..efaa4c5473 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2306,6 +2306,7 @@ typedef enum { VIR_DOMAIN_MEMORY_MODEL_NONE, VIR_DOMAIN_MEMORY_MODEL_DIMM, /* dimm hotpluggable memory device */ VIR_DOMAIN_MEMORY_MODEL_NVDIMM, /* nvdimm memory device */ + VIR_DOMAIN_MEMORY_MODEL_VIRTIO, /* virtio-{p}mem memory device */ VIR_DOMAIN_MEMORY_MODEL_LAST } virDomainMemoryModel; @@ -2325,6 +2326,10 @@ struct _virDomainMemoryDef { unsigned long long alignsize; /* kibibytes */ bool pmem; } nvdimm; /* VIR_DOMAIN_MEMORY_MODEL_NVDIMM */ + struct { + char *path; /* Required for pmem, otherwise optional */ + bool pmem; + } virtio; /* VIR_DOMAIN_MEMORY_MODEL_VIRTIO */ } s; /* target */ diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index 5ebcd766a9..931dbd4e84 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -498,6 +498,7 @@ qemuAssignDeviceMemoryAlias(virDomainDefPtr def, case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: prefix = "nvdimm"; break; + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO: case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_LAST: break; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4bd45e0638..9c50778180 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2986,6 +2986,7 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps, alignsize = mem->s.nvdimm.alignsize; nvdimmPmem = mem->s.nvdimm.pmem; break; + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO: case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_LAST: break; @@ -3339,6 +3340,7 @@ qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem) break; + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO: case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_LAST: break; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index e705e8d8d5..ab7938a355 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8485,6 +8485,8 @@ static int qemuDomainDefValidateMemoryHotplugDevice(const virDomainMemoryDef *mem, const virDomainDef *def) { + bool needsNuma = true; + switch (mem->model) { case VIR_DOMAIN_MEMORY_MODEL_DIMM: case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: @@ -8520,11 +8522,35 @@ qemuDomainDefValidateMemoryHotplugDevice(const virDomainMemoryDef *mem, } break; + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO: + if (mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI && + mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only 'pci' addresses are supported for the " + "virtio-pmem device")); + return -1; + } + + /* virtio-pmem doesn't have .node attribute. */ + if (mem->s.virtio.pmem) + needsNuma = false; + break; + case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_LAST: return -1; } + if (needsNuma && + virDomainNumaGetNodeCount(def->numa) != 0) { + if (mem->targetNode == -1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("target NUMA node needs to be specified for " + "memory device")); + return -1; + } + } + return 0; } diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index d872f75b38..bcf6724da3 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -313,11 +313,11 @@ qemuDomainPrimeVirtioDeviceAddresses(virDomainDefPtr def, virDomainDeviceAddressType type) { /* - declare address-less virtio devices to be of address type 'type' - disks, networks, videos, consoles, controllers, memballoon and rng - in this order - if type is ccw filesystem and vsock devices are declared to be of - address type ccw + Declare address-less virtio devices to be of address type 'type' + disks, networks, videos, consoles, controllers, hostdevs, memballoon, + rngs and memories in this order. + If type is ccw filesystem and vsock devices are declared to be of + address type ccw. */ size_t i; @@ -379,6 +379,12 @@ qemuDomainPrimeVirtioDeviceAddresses(virDomainDefPtr def, def->rngs[i]->info.type = type; } + for (i = 0; i < def->nmems; i++) { + if (def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_VIRTIO && + def->mems[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) + def->mems[i]->info.type = type; + } + if (type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { for (i = 0; i < def->nfss; i++) { if (def->fss[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) @@ -1040,11 +1046,23 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, } break; + case VIR_DOMAIN_DEVICE_MEMORY: + switch (dev->data.memory->model) { + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO: + return virtioFlags; + + case VIR_DOMAIN_MEMORY_MODEL_NONE: + case VIR_DOMAIN_MEMORY_MODEL_DIMM: + case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: + case VIR_DOMAIN_MEMORY_MODEL_LAST: + return 0; + } + break; + /* These devices don't ever connect with PCI */ case VIR_DOMAIN_DEVICE_NVRAM: case VIR_DOMAIN_DEVICE_TPM: case VIR_DOMAIN_DEVICE_PANIC: - case VIR_DOMAIN_DEVICE_MEMORY: case VIR_DOMAIN_DEVICE_HUB: case VIR_DOMAIN_DEVICE_REDIRDEV: case VIR_DOMAIN_DEVICE_SMARTCARD: @@ -2455,6 +2473,17 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, return -1; } + for (i = 0; i < def->nmems; i++) { + virDomainMemoryDefPtr mem = def->mems[i]; + + if (mem->model != VIR_DOMAIN_MEMORY_MODEL_VIRTIO || + !virDeviceInfoPCIAddressIsWanted(&mem->info)) + continue; + + if (qemuDomainPCIAddressReserveNextAddr(addrs, &mem->info) < 0) + return -1; + } + return 0; } @@ -3102,19 +3131,32 @@ qemuAssignMemoryDeviceSlot(virDomainMemoryDefPtr mem, int -qemuDomainAssignMemoryDeviceSlot(virDomainDefPtr def, +qemuDomainAssignMemoryDeviceSlot(virQEMUDriverPtr driver, + virDomainObjPtr vm, virDomainMemoryDefPtr mem) { - virBitmapPtr slotmap = NULL; - int ret; + g_autoptr(virBitmap) slotmap = NULL; + virDomainDeviceDef dev = {.type = VIR_DOMAIN_DEVICE_MEMORY, .data.memory = mem}; - if (!(slotmap = qemuDomainGetMemorySlotMap(def))) - return -1; + switch (mem->model) { + case VIR_DOMAIN_MEMORY_MODEL_DIMM: + case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: + if (!(slotmap = qemuDomainGetMemorySlotMap(vm->def))) + return -1; - ret = qemuAssignMemoryDeviceSlot(mem, slotmap); + return qemuAssignMemoryDeviceSlot(mem, slotmap); + break; - virBitmapFree(slotmap); - return ret; + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO: + return qemuDomainEnsurePCIAddress(vm, &dev, driver); + break; + + case VIR_DOMAIN_MEMORY_MODEL_NONE: + case VIR_DOMAIN_MEMORY_MODEL_LAST: + break; + } + + return 0; } @@ -3132,8 +3174,22 @@ qemuDomainAssignMemorySlots(virDomainDefPtr def) return -1; for (i = 0; i < def->nmems; i++) { - if (qemuAssignMemoryDeviceSlot(def->mems[i], slotmap) < 0) - goto cleanup; + virDomainMemoryDefPtr mem = def->mems[i]; + + switch (mem->model) { + case VIR_DOMAIN_MEMORY_MODEL_DIMM: + case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: + if (qemuAssignMemoryDeviceSlot(def->mems[i], slotmap) < 0) + goto cleanup; + break; + + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO: + /* handled in qemuDomainAssignPCIAddresses() */ + break; + case VIR_DOMAIN_MEMORY_MODEL_NONE: + case VIR_DOMAIN_MEMORY_MODEL_LAST: + break; + } } ret = 0; diff --git a/src/qemu/qemu_domain_address.h b/src/qemu/qemu_domain_address.h index 7ef3308246..20a46160d5 100644 --- a/src/qemu/qemu_domain_address.h +++ b/src/qemu/qemu_domain_address.h @@ -56,7 +56,8 @@ void qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def, void qemuDomainReleaseDeviceAddress(virDomainObjPtr vm, virDomainDeviceInfoPtr info); -int qemuDomainAssignMemoryDeviceSlot(virDomainDefPtr def, +int qemuDomainAssignMemoryDeviceSlot(virQEMUDriverPtr driver, + virDomainObjPtr vm, virDomainMemoryDefPtr mem); int qemuDomainEnsureVirtioAddress(bool *releaseAddr, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 2c12ef60af..1f9df182bf 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2402,7 +2402,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, if (qemuDomainDefValidateMemoryHotplug(vm->def, priv->qemuCaps, mem) < 0) goto cleanup; - if (qemuDomainAssignMemoryDeviceSlot(vm->def, mem) < 0) + if (qemuDomainAssignMemoryDeviceSlot(driver, vm, mem) < 0) goto cleanup; /* in cases where we are using a VM with aliases generated according to the diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 8ceea022d7..fc9bc7a5b4 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -4620,6 +4620,7 @@ qemuValidateDomainDeviceDefMemory(virDomainMemoryDefPtr mem, } break; + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO: case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_LAST: break; diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 78136e751e..b22ee739d8 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -693,6 +693,7 @@ AppArmorSetMemoryLabel(virSecurityManagerPtr mgr, return -1; } return reload_profile(mgr, def, mem->s.nvdimm.path, true); + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO: case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_DIMM: case VIR_DOMAIN_MEMORY_MODEL_LAST: diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 44ee42e1bd..6b681c4021 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1892,6 +1892,7 @@ virSecurityDACRestoreMemoryLabel(virSecurityManagerPtr mgr, ret = virSecurityDACRestoreFileLabel(mgr, mem->s.nvdimm.path); break; + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO: case VIR_DOMAIN_MEMORY_MODEL_DIMM: case VIR_DOMAIN_MEMORY_MODEL_LAST: case VIR_DOMAIN_MEMORY_MODEL_NONE: @@ -2074,6 +2075,7 @@ virSecurityDACSetMemoryLabel(virSecurityManagerPtr mgr, user, group, true); break; + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO: case VIR_DOMAIN_MEMORY_MODEL_DIMM: case VIR_DOMAIN_MEMORY_MODEL_LAST: case VIR_DOMAIN_MEMORY_MODEL_NONE: diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 294c9f1db5..77b69447da 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1582,6 +1582,7 @@ virSecuritySELinuxSetMemoryLabel(virSecurityManagerPtr mgr, return -1; break; + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO: case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_DIMM: case VIR_DOMAIN_MEMORY_MODEL_LAST: @@ -1609,6 +1610,7 @@ virSecuritySELinuxRestoreMemoryLabel(virSecurityManagerPtr mgr, ret = virSecuritySELinuxRestoreFileLabel(mgr, mem->s.nvdimm.path, true); break; + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO: case VIR_DOMAIN_MEMORY_MODEL_DIMM: case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_LAST: diff --git a/tests/qemuxml2argvdata/memory-hotplug-virtio-pmem.xml b/tests/qemuxml2argvdata/memory-hotplug-virtio-pmem.xml new file mode 100644 index 0000000000..3570b7d1b1 --- /dev/null +++ b/tests/qemuxml2argvdata/memory-hotplug-virtio-pmem.xml @@ -0,0 +1,54 @@ +<domain type='kvm'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <maxMemory slots='16' unit='KiB'>1099511627776</maxMemory> + <memory unit='KiB'>2095104</memory> + <currentMemory unit='KiB'>2095104</currentMemory> + <vcpu placement='static' cpuset='0-1'>2</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + <topology sockets='2' dies='1' cores='1' threads='1'/> + <numa> + <cell id='0' cpus='0-1' memory='2095104' unit='KiB'/> + </numa> + </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-i386</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <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'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </memballoon> + <memory model='virtio' access='shared'> + <source> + <path>/tmp/virtio_pmem</path> + <pmem/> + </source> + <target> + <size unit='KiB'>524288</size> + </target> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + </memory> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/memory-hotplug-virtio-pmem.x86_64-latest.xml b/tests/qemuxml2xmloutdata/memory-hotplug-virtio-pmem.x86_64-latest.xml new file mode 120000 index 0000000000..904425abe4 --- /dev/null +++ b/tests/qemuxml2xmloutdata/memory-hotplug-virtio-pmem.x86_64-latest.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/memory-hotplug-virtio-pmem.xml \ No newline at end of file diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index c006719dee..e804db8aee 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1238,6 +1238,8 @@ mymain(void) QEMU_CAPS_DEVICE_NVDIMM_UNARMED); DO_TEST("memory-hotplug-nvdimm-ppc64", QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, QEMU_CAPS_DEVICE_NVDIMM); + DO_TEST_CAPS_LATEST("memory-hotplug-virtio-pmem"); + DO_TEST("net-udp", NONE); DO_TEST("video-virtio-gpu-device", QEMU_CAPS_DEVICE_VIRTIO_GPU); -- 2.26.2

In this commit two new capabilities are introduced that reflect device support by give qemu: QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI, /* -device virtio-mem-pci */ QEMU_CAPS_DEVICE_VIRTIO_PMEM_PCI, /* -device virtio-pmem-pci */ Please note, virtio-pmem device was introduced sooner (QEMU 4.1) than virtio-mem (QEMU 5.1). Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_capabilities.c | 4 ++++ src/qemu/qemu_capabilities.h | 2 ++ tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml | 2 ++ tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml | 2 ++ 7 files changed, 13 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 4d132defbd..14e6892fb2 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -609,6 +609,8 @@ VIR_ENUM_IMPL(virQEMUCaps, "ncr53c90", "dc390", "am53c974", + "virtio-mem-pci", + "virtio-pmem-pci", ); @@ -1325,6 +1327,8 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { */ { "dc390", QEMU_CAPS_SCSI_DC390 }, { "am53c974", QEMU_CAPS_SCSI_AM53C974 }, + { "virtio-mem-pci", QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI }, + { "virtio-pmem-pci", QEMU_CAPS_DEVICE_VIRTIO_PMEM_PCI }, }; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 0f90efa459..b21e56b1db 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -589,6 +589,8 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_SCSI_NCR53C90, /* built-in SCSI */ QEMU_CAPS_SCSI_DC390, /* -device dc-390 */ QEMU_CAPS_SCSI_AM53C974, /* -device am53c974 */ + QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI, /* -device virtio-mem-pci */ + QEMU_CAPS_DEVICE_VIRTIO_PMEM_PCI, /* -device virtio-pmem-pci */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml index 58774fddcc..28a4b0ede0 100644 --- a/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml @@ -231,6 +231,7 @@ <flag name='fsdev.createmode'/> <flag name='dc390'/> <flag name='am53c974'/> + <flag name='virtio-pmem-pci'/> <version>4001000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100241</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml index 1ba8c09374..e150741f11 100644 --- a/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml @@ -240,6 +240,7 @@ <flag name='fsdev.createmode'/> <flag name='dc390'/> <flag name='am53c974'/> + <flag name='virtio-pmem-pci'/> <version>4002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml index 849727eb40..7c56d110f4 100644 --- a/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml @@ -247,6 +247,7 @@ <flag name='fsdev.createmode'/> <flag name='dc390'/> <flag name='am53c974'/> + <flag name='virtio-pmem-pci'/> <version>5000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100241</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml index ff5f42a563..ae8787981a 100644 --- a/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml @@ -249,6 +249,8 @@ <flag name='fsdev.createmode'/> <flag name='dc390'/> <flag name='am53c974'/> + <flag name='virtio-mem-pci'/> + <flag name='virtio-pmem-pci'/> <version>5001000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml index 5e9fa8575a..f0892cdbe5 100644 --- a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml @@ -251,6 +251,8 @@ <flag name='fsdev.createmode'/> <flag name='dc390'/> <flag name='am53c974'/> + <flag name='virtio-mem-pci'/> + <flag name='virtio-pmem-pci'/> <version>5001091</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100243</microcodeVersion> -- 2.26.2

Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_validate.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index fc9bc7a5b4..e4d12db3bd 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -4621,6 +4621,21 @@ qemuValidateDomainDeviceDefMemory(virDomainMemoryDefPtr mem, break; case VIR_DOMAIN_MEMORY_MODEL_VIRTIO: + if (mem->s.virtio.pmem) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_PMEM_PCI)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("virtio-pmem isn't supported by this QEMU binary")); + return -1; + } + } else { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("virtio-mem isn't supported by this QEMU binary")); + return -1; + } + } + break; + case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_LAST: break; -- 2.26.2

On Thu, Dec 03, 2020 at 13:36:18 +0100, Michal Privoznik wrote: Removal of this NULL check is not justified in the commit message and also really seems to be a separate cleanup if it is actually justified. Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt;

On Thu, Dec 03, 2020 at 13:36:21 +0100, Michal Privoznik wrote: This function would really benefit from a refactor to a switch. Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; This cleanup should be applicable even without the virtio-mem patches.

On Thu, Dec 03, 2020 at 13:36:22 +0100, Michal Privoznik wrote: If oldAlias is false this returns 'info.addr.dimm.slot' ... ... and you use it like that here, but virtio-pmem is a PCI device. [...] [...] We can theoretically use a virTristate* and use 'T' type here instead of having two variables. [...] Some of these seem to be specific for some devices (such as unarmed is relevant only for nvdimm)

On Thu, Dec 03, 2020 at 13:36:23 +0100, Michal Privoznik wrote: Docs don't mention interactions of <size> and <requested>. Is size the actual size? In such case you'll need to clear it on startup and populate it with the actual size later. The issue with <pmem> was mentioned earlier. [...] Docs state that blocksize must be also a multiple of page size. Move this to virDomainMemoryDefValidate too. [...] If size is current size it should be optional. If size is the current size and being actively updated, this lookup might not actually work if it's updated between checking and updating. [...] Leftovers?

The majority of the code is prepared since virtio-pmem. The alias for virtio-mem is new ("virtiomem"). And again, prealloc doesn't make much sense - the whole point is to dynamically allocate memory for guest and possibly return it to the host (e.g. if the host is running low on memory) and this wouldn't happen with prealloc. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_alias.c | 5 +- src/qemu/qemu_command.c | 19 +++++-- ...mory-hotplug-virtio-mem.x86_64-latest.args | 53 +++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 4 files changed, 74 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/memory-hotplug-virtio-mem.x86_64-latest.args diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index 68d4a7b480..29c26f0b72 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -521,7 +521,10 @@ qemuAssignDeviceMemoryAlias(virDomainDefPtr def, idx = qemuDeviceMemoryGetAliasID(def, mem, oldAlias, prefix); break; case VIR_DOMAIN_MEMORY_MODEL_VIRTIO: - prefix = "virtiopmem"; + if (mem->s.virtio.pmem) + prefix = "virtiopmem"; + else + prefix = "virtiomem"; idx = qemuDeviceMemoryGetAliasID(def, mem, true, prefix); break; case VIR_DOMAIN_MEMORY_MODEL_NONE: diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 501deff1ee..ccbc55e376 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2992,9 +2992,14 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps, nvdimmPmem = mem->s.nvdimm.pmem; break; case VIR_DOMAIN_MEMORY_MODEL_VIRTIO: + pagesize = mem->s.virtio.pagesize; + needHugepage = !!pagesize; + useHugepage = !!pagesize; + nodemask = mem->s.virtio.sourceNodes; nvdimmPath = mem->s.virtio.path; - /* virtio-pmem doesn't need prealloc, it's very likely exposing a real - * device and thus there's nothing to prealloc */ + /* virtio-pmem doesn't need prealloc. Either it's pmem and thus very + * likely exposing a real device where is nothing to prealloc, OR it's + * virtio-mem where we want to unmap pages on the fly. */ allowPrealloc = false; break; case VIR_DOMAIN_MEMORY_MODEL_NONE: @@ -3322,7 +3327,10 @@ qemuBuildMemoryDeviceStr(const virDomainDef *def, break; case VIR_DOMAIN_MEMORY_MODEL_VIRTIO: - device = "virtio-pmem-pci"; + if (mem->s.virtio.pmem) + device = "virtio-pmem-pci"; + else + device = "virtio-mem-pci"; case VIR_DOMAIN_MEMORY_MODEL_NONE: case VIR_DOMAIN_MEMORY_MODEL_LAST: @@ -3337,6 +3345,11 @@ qemuBuildMemoryDeviceStr(const virDomainDef *def, if (mem->labelsize) virBufferAsprintf(&buf, "label-size=%llu,", mem->labelsize * 1024); + if (mem->blocksize) { + virBufferAsprintf(&buf, "block-size=%llu,", mem->blocksize * 1024); + virBufferAsprintf(&buf, "requested-size=%llu,", mem->requestedsize * 1024); + } + if (virUUIDIsValid(mem->uuid)) { char uuidstr[VIR_UUID_STRING_BUFLEN]; diff --git a/tests/qemuxml2argvdata/memory-hotplug-virtio-mem.x86_64-latest.args b/tests/qemuxml2argvdata/memory-hotplug-virtio-mem.x86_64-latest.args new file mode 100644 index 0000000000..c12248518d --- /dev/null +++ b/tests/qemuxml2argvdata/memory-hotplug-virtio-mem.x86_64-latest.args @@ -0,0 +1,53 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i386 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ +-machine pc,accel=kvm,usb=off,dump-guest-core=off \ +-cpu qemu64 \ +-m size=2095104k,slots=16,maxmem=1099511627776k \ +-overcommit mem-lock=off \ +-smp 2,sockets=2,dies=1,cores=1,threads=1 \ +-object memory-backend-ram,id=ram-node0,size=2145386496 \ +-numa node,nodeid=0,cpus=0-1,memdev=ram-node0 \ +-object memory-backend-ram,id=memvirtiomem0,size=1073741824 \ +-device virtio-mem-pci,node=0,block-size=2097152,requested-size=536870912,\ +memdev=memvirtiomem0,id=virtiomem0,bus=pci.0,addr=0x2 \ +-object memory-backend-file,id=memvirtiomem1,\ +mem-path=/dev/hugepages2M/libvirt/qemu/-1-QEMUGuest1,size=2147483648,\ +host-nodes=1-3,policy=bind \ +-device virtio-mem-pci,node=0,block-size=2097152,requested-size=1073741824,\ +memdev=memvirtiomem1,id=virtiomem1,bus=pci.0,addr=0x4 \ +-object memory-backend-file,id=memvirtiomem2,mem-path=/tmp/virtio_mem,\ +size=4294967296 \ +-device virtio-mem-pci,node=0,block-size=2097152,requested-size=536870912,\ +memdev=memvirtiomem2,id=virtiomem2,bus=pci.0,addr=0x5 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ +-blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1",\ +"node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw",\ +"file":"libvirt-1-storage"}' \ +-device ide-hd,bus=ide.0,unit=0,drive=libvirt-1-format,id=ide0-0-0,bootindex=1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ +resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index c79246e0bb..13678fc891 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -3013,6 +3013,7 @@ mymain(void) DO_TEST_CAPS_LATEST("memory-hotplug-nvdimm-readonly"); DO_TEST_CAPS_ARCH_LATEST("memory-hotplug-nvdimm-ppc64", "ppc64"); DO_TEST_CAPS_LATEST("memory-hotplug-virtio-pmem"); + DO_TEST_CAPS_LATEST("memory-hotplug-virtio-mem"); DO_TEST("machine-aeskeywrap-on-caps", QEMU_CAPS_AES_KEY_WRAP, -- 2.26.2

On Thu, Dec 03, 2020 at 13:36:25 +0100, Michal Privoznik wrote: This doesn't work as the comment expects it to: STRNEQ_NULLABLE(NULL, NULL) == false STRNEQ_NULLABLE("blah", NULL) == true STRNEQ_NULLABLE(NULL, "blah") == true STRNEQ_NULLABLE("blah", "blah") == false Since info->alias is always set, it would not skip the condition if the definition used for lookup didn't specify it. [...]

On Thu, Dec 03, 2020 at 13:36:26 +0100, Michal Privoznik wrote: [...] I don't think we should count the size of virtio-pmem, virtio-mem against the total VM memory size. The mechanism this is provided to the VM is different compared to "traditional" dimms. The OS needs driver to access the memory. The open question is whether we should add a similar event to what we have for memballoon change. For now I'd probably not do it since a guest OS restart (driver unload) can't actually result in more memory consumed than before.

What code tries to achieve is that if no flags were provided to either 'setmem' or 'setmaxmem' commands then the old (no flags) API is called to be able to communicate with older daemons. Well, the code can be simplified a bit. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- tools/virsh-domain.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 51a9fd90d1..eeeeaa8639 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9018,9 +9018,6 @@ cmdSetmem(vshControl *ctl, const vshCmd *cmd) flags |= VIR_DOMAIN_AFFECT_CONFIG; if (live) flags |= VIR_DOMAIN_AFFECT_LIVE; - /* none of the options were specified */ - if (!current && !live && !config) - flags = -1; if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; @@ -9037,7 +9034,7 @@ cmdSetmem(vshControl *ctl, const vshCmd *cmd) } kibibytes = VIR_DIV_UP(bytes, 1024); - if (flags == -1) { + if (flags == 0) { if (virDomainSetMemory(dom, kibibytes) != 0) ret = false; } else { @@ -9090,7 +9087,7 @@ cmdSetmaxmem(vshControl *ctl, const vshCmd *cmd) bool config = vshCommandOptBool(cmd, "config"); bool live = vshCommandOptBool(cmd, "live"); bool current = vshCommandOptBool(cmd, "current"); - unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT | VIR_DOMAIN_MEM_MAXIMUM; + unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; VSH_EXCLUSIVE_OPTIONS_VAR(current, live); VSH_EXCLUSIVE_OPTIONS_VAR(current, config); @@ -9099,9 +9096,6 @@ cmdSetmaxmem(vshControl *ctl, const vshCmd *cmd) flags |= VIR_DOMAIN_AFFECT_CONFIG; if (live) flags |= VIR_DOMAIN_AFFECT_LIVE; - /* none of the options were specified */ - if (!current && !live && !config) - flags = -1; if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; @@ -9118,13 +9112,13 @@ cmdSetmaxmem(vshControl *ctl, const vshCmd *cmd) } kibibytes = VIR_DIV_UP(bytes, 1024); - if (flags == -1) { + if (flags == 0) { if (virDomainSetMaxMemory(dom, kibibytes) != 0) { vshError(ctl, "%s", _("Unable to change MaxMemorySize")); ret = false; } } else { - if (virDomainSetMemoryFlags(dom, kibibytes, flags) < 0) { + if (virDomainSetMemoryFlags(dom, kibibytes, flags | VIR_DOMAIN_MEM_MAXIMUM) < 0) { vshError(ctl, "%s", _("Unable to change MaxMemorySize")); ret = false; } -- 2.26.2

Provide an user friendly way of modifying 'requested-size' of a virtio-mem device. New --node and --alias arguments are introduced but they are needed only if it is not clear which virtio-mem does user mean. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- docs/manpages/virsh.rst | 6 ++ tools/virsh-domain.c | 126 ++++++++++++++++++++++++++++++++++++---- 2 files changed, 122 insertions(+), 10 deletions(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 9ef6b68422..c3c8c27a18 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -4098,6 +4098,7 @@ setmem :: setmem domain size [[--config] [--live] | [--current]] + [--virtio [--node NODE] | [--alias ALIAS]] Change the memory allocation for a guest domain. If *--live* is specified, perform a memory balloon of a running guest. @@ -4107,6 +4108,11 @@ If *--current* is specified, it is equivalent to either *--live* or Both *--live* and *--config* flags may be given, but *--current* is exclusive. If no flag is specified, behavior is different depending on hypervisor. +If *--virtio* is specified then instead of changing the memory allocation for +whole domain the individual memory device with virtio model is changed. If +there is more than one such memory device then *--node* or *--alias* must be +used to specify the device uniquely. *NODE* refers to the guest NUMA node to +which the memory device is attached to and *ALIAS* is the device alias. *size* is a scaled integer (see ``NOTES`` above); it defaults to kibibytes (blocks of 1024 bytes) unless you provide a suffix (and the older option diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index eeeeaa8639..7d8546d806 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -8992,12 +8992,101 @@ static const vshCmdOptDef opts_setmem[] = { .flags = VSH_OFLAG_REQ, .help = N_("new memory size, as scaled integer (default KiB)") }, + {.name = "virtio", + .type = VSH_OT_BOOL, + .help = N_("modify virtio memory instead of top level domain memory") + }, + {.name = "node", + .type = VSH_OT_INT, + .help = N_("guest numa node"), + }, + {.name = "alias", + .type = VSH_OT_STRING, + .help = N_("memory device alias"), + }, VIRSH_COMMON_OPT_DOMAIN_CONFIG, VIRSH_COMMON_OPT_DOMAIN_LIVE, VIRSH_COMMON_OPT_DOMAIN_CURRENT, {.name = NULL} }; + +static int +virshDomainSetmemVirtio(vshControl *ctl, + virDomainPtr dom, + unsigned long kibibytes, + int numaNode, + const char *alias, + unsigned int flags) +{ + g_autoptr(xmlDoc) doc = NULL; + g_autoptr(xmlXPathContext) ctxt = NULL; + g_autofree char *xpath = NULL; + int nmems; + g_autofree xmlNodePtr *mems = NULL; + xmlNodePtr requestedSizeNode = NULL; + g_autofree char *kibibytesStr = NULL; + g_autoptr(xmlBuffer) xmlbuf = NULL; + unsigned int domainXMLFlags = 0; + const char *updatedMemoryXML = NULL; + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) + domainXMLFlags |= VIR_DOMAIN_XML_INACTIVE; + + if (virshDomainGetXMLFromDom(ctl, dom, domainXMLFlags, &doc, &ctxt) < 0) + return -1; + + if (alias) { + xpath = g_strdup_printf("/domain/devices/memory[./alias/@name='%s']", alias); + } else if (numaNode >= 0) { + xpath = g_strdup_printf("/domain/devices/memory[./target/node/text()=%d]", numaNode); + } else { + xpath = g_strdup("/domain/devices/memory[@model='virtio' and not(./source/pmem)]"); + } + + nmems = virXPathNodeSet(xpath, ctxt, &mems); + if (nmems < 0) { + vshSaveLibvirtError(); + return -1; + } else if (nmems == 0) { + vshError(ctl, _("no virtio-mem device found")); + return -1; + } else if (nmems > 1) { + vshError(ctl, _("multiple virtio-mem devices found")); + return -1; + } + + ctxt->node = mems[0]; + + requestedSizeNode = virXPathNode("./target/requested", ctxt); + + if (!requestedSizeNode) { + vshError(ctl, _("virtio-mem device is missing <requested/>")); + return -1; + } + + kibibytesStr = g_strdup_printf("%lu", kibibytes); + xmlNodeSetContent(requestedSizeNode, BAD_CAST kibibytesStr); + + if (!(xmlbuf = xmlBufferCreate())) { + vshError(ctl, _("unable to allocate XML buffer")); + return -1; + } + + if (xmlNodeDump(xmlbuf, doc, mems[0], 0, 1) < 0) { + vshError(ctl, _("unable to format new <memory/> node")); + return -1; + } + + updatedMemoryXML = (const char *)xmlBufferContent(xmlbuf); + + if (virDomainUpdateDeviceFlags(dom, updatedMemoryXML, flags) < 0) + return -1; + + return 0; +} + + static bool cmdSetmem(vshControl *ctl, const vshCmd *cmd) { @@ -9005,7 +9094,7 @@ cmdSetmem(vshControl *ctl, const vshCmd *cmd) unsigned long long bytes = 0; unsigned long long max; unsigned long kibibytes = 0; - bool ret = true; + bool ret = false; bool config = vshCommandOptBool(cmd, "config"); bool live = vshCommandOptBool(cmd, "live"); bool current = vshCommandOptBool(cmd, "current"); @@ -9013,6 +9102,7 @@ cmdSetmem(vshControl *ctl, const vshCmd *cmd) VSH_EXCLUSIVE_OPTIONS_VAR(current, live); VSH_EXCLUSIVE_OPTIONS_VAR(current, config); + VSH_EXCLUSIVE_OPTIONS("node", "alias"); if (config) flags |= VIR_DOMAIN_AFFECT_CONFIG; @@ -9028,20 +9118,36 @@ cmdSetmem(vshControl *ctl, const vshCmd *cmd) max = 1024ull * ULONG_MAX; else max = ULONG_MAX; - if (vshCommandOptScaledInt(ctl, cmd, "size", &bytes, 1024, max) < 0) { - virshDomainFree(dom); - return false; - } + if (vshCommandOptScaledInt(ctl, cmd, "size", &bytes, 1024, max) < 0) + goto cleanup; + kibibytes = VIR_DIV_UP(bytes, 1024); - if (flags == 0) { - if (virDomainSetMemory(dom, kibibytes) != 0) - ret = false; + if (vshCommandOptBool(cmd, "virtio")) { + int numaNode = -1; + const char *alias = NULL; + + if (vshCommandOptInt(ctl, cmd, "node", &numaNode) < 0) + goto cleanup; + + if (vshCommandOptStringReq(ctl, cmd, "alias", &alias) < 0) + goto cleanup; + + if (virshDomainSetmemVirtio(ctl, dom, kibibytes, + numaNode, alias, flags) < 0) + goto cleanup; } else { - if (virDomainSetMemoryFlags(dom, kibibytes, flags) < 0) - ret = false; + if (flags == 0) { + if (virDomainSetMemory(dom, kibibytes) != 0) + goto cleanup; + } else { + if (virDomainSetMemoryFlags(dom, kibibytes, flags) < 0) + goto cleanup; + } } + ret = true; + cleanup: virshDomainFree(dom); return ret; } -- 2.26.2

Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- NEWS.rst | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 135c4e2fe0..adedf15d2b 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -13,6 +13,15 @@ v7.0.0 (unreleased) * **New features** + * qemu: Introduce virtio memory support + + New ``virtio`` model is introduced for ``<memory/>`` device which + translates to ``virtio-pmem`` or ``virtio-mem`` on QEMU command + line. The former servers as a NVDIMM and the latter is a + paravirtualized mechanism of adding/removing memory to/from a VM. + It is exposed via ``virDomainUpdateDeviceFlags()`` API and ``virsh + setmem --virtio`` for user convenience. + * **Improvements** * **Bug fixes** -- 2.26.2