The only test we do when checking for UUID validity is that whether all bytes are the same (invalid UUID) or not (valid UUID). The algorithm we use is needlessly complicated. Also, the checked UUID is not modified and hence the argument can be of 'const' type. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/util/viruuid.c | 17 +++++++---------- src/util/viruuid.h | 2 +- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/util/viruuid.c b/src/util/viruuid.c index 908b09945d..558fbb9c0d 100644 --- a/src/util/viruuid.c +++ b/src/util/viruuid.c @@ -170,25 +170,22 @@ virUUIDFormat(const unsigned char *uuid, char *uuidstr) * Basic tests: * - Not all of the digits may be equal */ -int -virUUIDIsValid(unsigned char *uuid) +bool +virUUIDIsValid(const unsigned char *uuid) { size_t i; - unsigned int ctr = 1; - unsigned char c; if (!uuid) - return 0; - - c = uuid[0]; + return false; for (i = 1; i < VIR_UUID_BUFLEN; i++) - if (uuid[i] == c) - ctr++; + if (uuid[i] != uuid[0]) + return true; - return ctr != VIR_UUID_BUFLEN; + return false; } + static int getDMISystemUUID(char *uuid, int len) { diff --git a/src/util/viruuid.h b/src/util/viruuid.h index 5d64e58405..b403b1906a 100644 --- a/src/util/viruuid.h +++ b/src/util/viruuid.h @@ -43,7 +43,7 @@ int virSetHostUUIDStr(const char *host_uuid); int virGetHostUUID(unsigned char *host_uuid) ATTRIBUTE_NONNULL(1) G_GNUC_NO_INLINE; -int virUUIDIsValid(unsigned char *uuid); +bool virUUIDIsValid(const unsigned char *uuid); int virUUIDGenerate(unsigned char *uuid) G_GNUC_NO_INLINE; -- 2.26.2

On PPC platform it is required that a NVDIMM has an UUID. If none is provided then libvirt generates one during parsing (see v6.2.0-rc1~96 and friends). However, the example provided in our documentation is not valid XML. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- docs/formatdomain.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index ff64996af2..512939679b 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -7159,7 +7159,7 @@ Example: usage of the memory devices </target> </memory> <memory model='nvdimm'> - <uuid> + <uuid>9066901e-c90a-46ad-8b55-c18868cf92ae</uuid> <source> <path>/tmp/nvdimm</path> </source> @@ -7173,7 +7173,7 @@ Example: usage of the memory devices </target> </memory> <memory model='nvdimm' access='shared'> - <uuid> + <uuid>e39080c8-7f99-4b12-9c43-d80014e977b8</uuid> <source> <path>/dev/dax0.0</path> <alignsize unit='KiB'>2048</alignsize> @@ -7211,7 +7211,7 @@ Example: usage of the memory devices ``uuid`` For pSeries guests, an uuid can be set to identify the nvdimm module. If - absent, libvirt will generate an uuid. automatically. This attribute is + absent, libvirt will generate an uuid automatically. This attribute is allowed only for ``model='nvdimm'`` for pSeries guests. :since:`Since 6.2.0` ``source`` -- 2.26.2

On 11/27/20 12:02 PM, Michal Privoznik wrote: Thanks for fixing this up. Reviewed-by: Daniel Henrique Barboza <danielhb413(a)gmail.com&gt;

On 27/11/2020 16.02, Michal Privoznik wrote: Looks fine to me, but docs/coding-style.rst still suggest to format code with "indent -l75", so is this really the right thing to do here? Not related to your patch, but an idea for a future clean-up: That QEMU_CAPS_VIRTIO_S390 seems to belong to the ancient "s390-virtio" (without ccw) machine that has been removed in QEMU v2.6 already: https://git.qemu.org/?p=qemu.git;a=commitdiff;h=7b3fdbd9a82 https://git.qemu.org/?p=qemu.git;a=commitdiff;h=3538fb6f89d IIRC, that machine was already considered as deprecated since a couple of earlier QEMU releases, so I really doubt that anybody is still using that in production today. Thus I think that all code related to QEMU_CAPS_VIRTIO_S390 could likely be removed from libvirt nowadays. Thomas

On Mon, Nov 30, 2020 at 11:18:20AM +0100, Michal Privoznik wrote: It was set due to the base RHEL-7 QEMU version. RHEL-7 will fall out of our support matrix at start of May 2021, so in a few months time we'll have a massive QEMU version bump we can do, along with a more general cleanup of RHEL-7 vintage cruft. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On 30/11/2020 11.18, Michal Privoznik wrote: Fair point, but then this should IMHO be reflected in the coding-style doc first. Otherwise the next contributor to this file might simply undo your change to fit everything again into the 80 (or even 75) columns limit...? Thomas

Our code expects that a nvdimm has a path defined always. And the parser does check for that. Well, not fully - only when parsing <source/> (which is an optional element). So if the element is not in the XML then the check is not performed and the assumption is broken. Verify in the memory def validator that a path was set. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/conf/domain_conf.c | 12 +++++++----- src/security/security_apparmor.c | 6 ------ 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 631165c3fa..6c1fa93b0a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6699,6 +6699,12 @@ virDomainMemoryDefValidate(const virDomainMemoryDef *mem, const virDomainDef *def) { if (mem->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) { + if (!mem->nvdimmPath) { + virReportError(VIR_ERR_XML_DETAIL, "%s", + _("path is required for model 'nvdimm'")); + return -1; + } + if (mem->discard == VIR_TRISTATE_BOOL_YES) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("discard is not supported for nvdimms")); @@ -16713,11 +16719,7 @@ virDomainMemorySourceDefParseXML(xmlNodePtr node, break; case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: - if (!(def->nvdimmPath = virXPathString("string(./path)", ctxt))) { - virReportError(VIR_ERR_XML_DETAIL, "%s", - _("path is required for model 'nvdimm'")); - return -1; - } + def->nvdimmPath = virXPathString("string(./path)", ctxt); if (virDomainParseMemory("./alignsize", "./alignsize/@unit", ctxt, &def->alignsize, false, false) < 0) diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index c2d86c6940..f306af8dd3 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -686,12 +686,6 @@ AppArmorSetMemoryLabel(virSecurityManagerPtr mgr, switch ((virDomainMemoryModel) mem->model) { case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: - if (mem->nvdimmPath == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("%s: nvdimm without a path"), - __func__); - return -1; - } if (!virFileExists(mem->nvdimmPath)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("%s: \'%s\' does not exist"), -- 2.26.2

The virDomainMemorySourceDefFormat() uses good old style of formatting child buffer (virBufferAdjustIndent()). When switched to virXMLFormatElement() we can save a couple of lines. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/conf/domain_conf.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 38ddbacd66..8f10fe14d3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -27877,37 +27877,32 @@ static int virDomainMemorySourceDefFormat(virBufferPtr buf, virDomainMemoryDefPtr def) { + g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); g_autofree char *bitmap = NULL; - if (!def->pagesize && !def->sourceNodes && !def->nvdimmPath) - return 0; - - virBufferAddLit(buf, "<source>\n"); - virBufferAdjustIndent(buf, 2); - switch (def->model) { case VIR_DOMAIN_MEMORY_MODEL_DIMM: if (def->sourceNodes) { if (!(bitmap = virBitmapFormat(def->sourceNodes))) return -1; - virBufferAsprintf(buf, "<nodemask>%s</nodemask>\n", bitmap); + virBufferAsprintf(&childBuf, "<nodemask>%s</nodemask>\n", bitmap); } if (def->pagesize) - virBufferAsprintf(buf, "<pagesize unit='KiB'>%llu</pagesize>\n", + virBufferAsprintf(&childBuf, "<pagesize unit='KiB'>%llu</pagesize>\n", def->pagesize); break; case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: - virBufferEscapeString(buf, "<path>%s</path>\n", def->nvdimmPath); + virBufferEscapeString(&childBuf, "<path>%s</path>\n", def->nvdimmPath); if (def->alignsize) - virBufferAsprintf(buf, "<alignsize unit='KiB'>%llu</alignsize>\n", + virBufferAsprintf(&childBuf, "<alignsize unit='KiB'>%llu</alignsize>\n", def->alignsize); if (def->nvdimmPmem) - virBufferAddLit(buf, "<pmem/>\n"); + virBufferAddLit(&childBuf, "<pmem/>\n"); break; case VIR_DOMAIN_MEMORY_MODEL_NONE: @@ -27915,8 +27910,7 @@ virDomainMemorySourceDefFormat(virBufferPtr buf, break; } - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</source>\n"); + virXMLFormatElement(buf, "source", NULL, &childBuf); return 0; } -- 2.26.2

There is this function qemuDomainDefValidateMemoryHotplug() which is called explicitly from hotplug path and the qemu's domain def validator. This is not really necessary because we can move the part that validates feature against qemuCaps into device validator which is called implicitly (from qemu driver's POV). Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 32 +------------------------------- src/qemu/qemu_validate.c | 16 +++++++++++++++- 2 files changed, 16 insertions(+), 32 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 19a699540f..e705e8d8d5 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8544,14 +8544,12 @@ qemuDomainDefValidateMemoryHotplugDevice(const virDomainMemoryDef *mem, */ int qemuDomainDefValidateMemoryHotplug(const virDomainDef *def, - virQEMUCapsPtr qemuCaps, + virQEMUCapsPtr qemuCaps G_GNUC_UNUSED, const virDomainMemoryDef *mem) { unsigned int nmems = def->nmems; unsigned long long hotplugSpace; unsigned long long hotplugMemory = 0; - bool needPCDimmCap = false; - bool needNvdimmCap = false; size_t i; hotplugSpace = def->mem.max_memory - virDomainDefGetMemoryInitial(def); @@ -8598,40 +8596,12 @@ qemuDomainDefValidateMemoryHotplug(const virDomainDef *def, for (i = 0; i < def->nmems; i++) { hotplugMemory += def->mems[i]->size; - switch (def->mems[i]->model) { - case VIR_DOMAIN_MEMORY_MODEL_DIMM: - needPCDimmCap = true; - break; - - case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: - needNvdimmCap = true; - break; - - case VIR_DOMAIN_MEMORY_MODEL_NONE: - case VIR_DOMAIN_MEMORY_MODEL_LAST: - break; - } - /* already existing devices don't need to be checked on hotplug */ if (!mem && qemuDomainDefValidateMemoryHotplugDevice(def->mems[i], def) < 0) return -1; } - if (needPCDimmCap && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PC_DIMM)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("memory hotplug isn't supported by this QEMU binary")); - return -1; - } - - if (needNvdimmCap && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("nvdimm isn't supported by this QEMU binary")); - return -1; - } - if (hotplugMemory > hotplugSpace) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("memory device total size exceeds hotplug space")); diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 6f4662b25a..8ceea022d7 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -4595,7 +4595,16 @@ static int qemuValidateDomainDeviceDefMemory(virDomainMemoryDefPtr mem, virQEMUCapsPtr qemuCaps) { - if (mem->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) { + switch (mem->model) { + case VIR_DOMAIN_MEMORY_MODEL_DIMM: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PC_DIMM)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("memory hotplug isn't supported by this QEMU binary")); + return -1; + } + break; + + case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("nvdimm isn't supported by this QEMU binary")); @@ -4609,6 +4618,11 @@ qemuValidateDomainDeviceDefMemory(virDomainMemoryDefPtr mem, "with this QEMU binary")); return -1; } + break; + + case VIR_DOMAIN_MEMORY_MODEL_NONE: + case VIR_DOMAIN_MEMORY_MODEL_LAST: + break; } return 0; -- 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 60a0fda27e..d80ab014bb 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; } @@ -6708,7 +6728,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'")); @@ -6726,6 +6747,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; @@ -16740,6 +16794,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; @@ -18624,6 +18686,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; @@ -24221,7 +24289,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 " @@ -24257,6 +24326,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); @@ -27916,6 +28000,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 611dac8bc4..5c37ba6513 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 d3321b30e9..649403d638 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2969,6 +2969,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; @@ -3322,6 +3323,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

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

Some users might wanto have virtio-pmem backed by a block device in which case we have to allow the device in CGroups. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_cgroup.c | 43 ++++++++++++++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 519158eccd..b746963b0b 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -500,19 +500,32 @@ qemuSetupMemoryDevicesCgroup(virDomainObjPtr vm, virDomainMemoryDefPtr mem) { qemuDomainObjPrivatePtr priv = vm->privateData; + const char *path = NULL; int rv; - if (mem->model != VIR_DOMAIN_MEMORY_MODEL_NVDIMM) + switch (mem->model) { + case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: + path = mem->s.nvdimm.path; + break; + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO: + path = mem->s.virtio.path; + break; + case VIR_DOMAIN_MEMORY_MODEL_DIMM: + case VIR_DOMAIN_MEMORY_MODEL_LAST: + case VIR_DOMAIN_MEMORY_MODEL_NONE: + break; + } + + if (!path) return 0; if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) return 0; - VIR_DEBUG("Setting devices Cgroup for NVDIMM device: %s", mem->s.nvdimm.path); - rv = virCgroupAllowDevicePath(priv->cgroup, mem->s.nvdimm.path, + VIR_DEBUG("Setting devices Cgroup for memory device: %s", path); + rv = virCgroupAllowDevicePath(priv->cgroup, path, VIR_CGROUP_DEVICE_RW, false); - virDomainAuditCgroupPath(vm, priv->cgroup, "allow", - mem->s.nvdimm.path, "rw", rv); + virDomainAuditCgroupPath(vm, priv->cgroup, "allow", path, "rw", rv); return rv; } @@ -523,18 +536,28 @@ qemuTeardownMemoryDevicesCgroup(virDomainObjPtr vm, virDomainMemoryDefPtr mem) { qemuDomainObjPrivatePtr priv = vm->privateData; + const char *path = NULL; int rv; - if (mem->model != VIR_DOMAIN_MEMORY_MODEL_NVDIMM) - return 0; + switch (mem->model) { + case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: + path = mem->s.nvdimm.path; + break; + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO: + path = mem->s.virtio.path; + break; + case VIR_DOMAIN_MEMORY_MODEL_DIMM: + case VIR_DOMAIN_MEMORY_MODEL_LAST: + case VIR_DOMAIN_MEMORY_MODEL_NONE: + break; + } if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) return 0; - rv = virCgroupDenyDevicePath(priv->cgroup, mem->s.nvdimm.path, + rv = virCgroupDenyDevicePath(priv->cgroup, path, VIR_CGROUP_DEVICE_RWM, false); - virDomainAuditCgroupPath(vm, priv->cgroup, - "deny", mem->s.nvdimm.path, "rwm", rv); + virDomainAuditCgroupPath(vm, priv->cgroup, "deny", path, "rwm", rv); return rv; } -- 2.26.2

So far our memory modules could go only into DIMM slots. But with virtio model this assumption is no longer true - virtio-pmem goes onto PCI bus. But for formatting PCI address onto command line we already have a function - qemuBuildDeviceAddressStr(). Therefore, mode DIMM address generation into it so that we don't have to special case address building later on. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_command.c | 17 +++++++++-------- src/qemu/qemu_command.h | 5 ++++- src/qemu/qemu_hotplug.c | 2 +- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 649403d638..ae1b7bc81b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -376,6 +376,10 @@ qemuBuildDeviceAddressStr(virBufferPtr buf, virBufferAsprintf(buf, ",iobase=0x%x,irq=0x%x", info->addr.isa.iobase, info->addr.isa.irq); + } else if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM) { + virBufferAsprintf(buf, ",slot=%d", info->addr.dimm.slot); + if (info->addr.dimm.base) + virBufferAsprintf(buf, ",addr=%llu", info->addr.dimm.base); } return 0; @@ -3273,7 +3277,9 @@ qemuBuildMemoryDimmBackendStr(virBufferPtr buf, char * -qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem) +qemuBuildMemoryDeviceStr(const virDomainDef *def, + virDomainMemoryDefPtr mem, + virQEMUCapsPtr qemuCaps) { g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; const char *device; @@ -3315,12 +3321,7 @@ qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem) virBufferAsprintf(&buf, "memdev=mem%s,id=%s", mem->info.alias, mem->info.alias); - if (mem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM) { - virBufferAsprintf(&buf, ",slot=%d", mem->info.addr.dimm.slot); - if (mem->info.addr.dimm.base) - virBufferAsprintf(&buf, ",addr=%llu", mem->info.addr.dimm.base); - } - + qemuBuildDeviceAddressStr(&buf, def, &mem->info, qemuCaps); break; case VIR_DOMAIN_MEMORY_MODEL_VIRTIO: @@ -7465,7 +7466,7 @@ qemuBuildMemoryDeviceCommandLine(virCommandPtr cmd, virCommandAddArg(cmd, "-object"); virCommandAddArgBuffer(cmd, &buf); - if (!(dimmStr = qemuBuildMemoryDeviceStr(def->mems[i]))) + if (!(dimmStr = qemuBuildMemoryDeviceStr(def, def->mems[i], priv->qemuCaps))) return -1; virCommandAddArgList(cmd, "-device", dimmStr, NULL); diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 3da07e25a1..3cfe6ff3e9 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -155,7 +155,10 @@ int qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps, const virDomainMemoryDef *mem, bool force); -char *qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem); +char * +qemuBuildMemoryDeviceStr(const virDomainDef *def, + virDomainMemoryDefPtr mem, + virQEMUCapsPtr qemuCaps); /* Current, best practice */ char *qemuBuildPCIHostdevDevStr(const virDomainDef *def, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 1f9df182bf..bc5dc02a2f 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2412,7 +2412,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, objalias = g_strdup_printf("mem%s", mem->info.alias); - if (!(devstr = qemuBuildMemoryDeviceStr(mem))) + if (!(devstr = qemuBuildMemoryDeviceStr(vm->def, mem, priv->qemuCaps))) goto cleanup; if (qemuBuildMemoryBackendProps(&props, objalias, cfg, -- 2.26.2

QEMU gained this new virtio-mem model. It's similar to pc-dimm in a sense that guest uses it as memory, but in a sense very different from it as it can dynamically change allocation, without need for hotplug. More specifically, the device has two attributes more (it has more of course, but these two are important here): 1) block-size - the granularity of the device. You can imagine the device being divided into blocks, each 'block-size' long. 2) requested-size - the portion of the device that is in use by the guest. And it all works like this: at guest startup/hotplug both block-size and requested-size are specified. When sysadmin wants to give some more memory to the guest, or take some back, they change the 'requested-size' attribute which is propagated to the guest where virtio-mem module takes corresponding action. This means, that 'requested-size' must be a whole number product of 'block-size' and of course has to be in rage [0, max-size] (including). The 'max-size' is yet another attribute but if not set it's "inherited" from corresponding memory-backend-* object. Therefore, two new elements are introduced under <target/>, to reflect these attributes: <memory model='virtio'> <target> <size unit='KiB'>4194304</size> <node>0</node> <block unit='KiB'>2048</block> <requested unit='KiB'>524288</requested> </target> <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> </memory> The intent here is that <requested/> will be allowed to change via virDomainUpdateDeviceFlags() API. Note, QEMU does inform us about success of allocation via an event - this is covered in next patches. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- docs/formatdomain.rst | 22 ++++ docs/schemas/domaincommon.rng | 10 ++ src/conf/domain_conf.c | 103 ++++++++++++++++-- src/conf/domain_conf.h | 5 + .../memory-hotplug-virtio-mem.xml | 78 +++++++++++++ ...emory-hotplug-virtio-mem.x86_64-latest.xml | 1 + tests/qemuxml2xmltest.c | 1 + 7 files changed, 213 insertions(+), 7 deletions(-) create mode 100644 tests/qemuxml2argvdata/memory-hotplug-virtio-mem.xml create mode 120000 tests/qemuxml2xmloutdata/memory-hotplug-virtio-mem.x86_64-latest.xml diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index ca6bc0432e..3990728939 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -7187,6 +7187,17 @@ Example: usage of the memory devices </label> </target> </memory> + <memory model='virtio'> + <source> + <path>/tmp/virtio_mem</path> + </source> + <target> + <size unit='KiB'>1048576</size> + <node>0</node> + <block unit='KiB'>2048</block> + <requested unit='KiB'>524288</requested> + </target> + </memory> <memory model='virtio' access='shared'> <source> <path>/tmp/virtio_pmem</path> @@ -7300,6 +7311,17 @@ Example: usage of the memory devices so other backend types should use the ``readonly`` element. :since:`Since 5.0.0` + ``block`` + The size of an individual block, granularity of division of memory module. + Must be power of two and at least equal to size of a transparent hugepage + (2MiB on x84_64). The default is hypervisor dependant. This is valid for + ``virtio`` model only and mutually exclusive with ``pmem``. + + ``requested`` + The total size of blocks exposed to the guest. Must respect ``block`` + granularity. This is valid for ``virtio`` model only and mutually + exclusive with ``pmem``. + :anchor:`<a id="elementsIommu"/>` IOMMU devices diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index b385bae84c..d478b639fa 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -6053,6 +6053,16 @@ <element name="size"> <ref name="scaledInteger"/> </element> + <optional> + <element name="block"> + <ref name="scaledInteger"/> + </element> + </optional> + <optional> + <element name="requested"> + <ref name="scaledInteger"/> + </element> + </optional> <optional> <element name="node"> <ref name="unsignedInt"/> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d80ab014bb..0a402f1a51 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6769,10 +6769,23 @@ virDomainMemoryDefValidate(const virDomainMemoryDef *mem, 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; + if (mem->requestedsize > mem->size) { + virReportError(VIR_ERR_XML_DETAIL, "%s", + _("requested size must be smaller than @size")); + return -1; + } + + if (!VIR_IS_POW2(mem->blocksize)) { + virReportError(VIR_ERR_XML_DETAIL, "%s", + _("block size must be a power of two")); + return -1; + } + + if (mem->requestedsize % mem->blocksize != 0) { + virReportError(VIR_ERR_XML_DETAIL, "%s", + _("requested size must be an integer multiple of block size")); + return -1; + } } break; @@ -16797,9 +16810,25 @@ virDomainMemorySourceDefParseXML(xmlNodePtr node, case VIR_DOMAIN_MEMORY_MODEL_VIRTIO: def->s.virtio.path = virXPathString("string(./path)", ctxt); - if (virXPathBoolean("boolean(./pmem)", ctxt)) + if (virXPathBoolean("boolean(./pmem)", ctxt)) { def->s.virtio.pmem = true; + } else { + if (virDomainParseMemory("./pagesize", "./pagesize/@unit", ctxt, + &def->s.virtio.pagesize, false, false) < 0) + return -1; + if ((nodemask = virXPathString("string(./nodemask)", ctxt))) { + if (virBitmapParse(nodemask, &def->s.virtio.sourceNodes, + VIR_DOMAIN_CPUMASK_LEN) < 0) + return -1; + + if (virBitmapIsAllClear(def->s.virtio.sourceNodes)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Invalid value of 'nodemask': %s"), nodemask); + return -1; + } + } + } break; case VIR_DOMAIN_MEMORY_MODEL_NONE: @@ -16835,7 +16864,8 @@ virDomainMemoryTargetDefParseXML(xmlNodePtr node, &def->size, true, false) < 0) return -1; - if (def->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) { + switch (def->model) { + case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: if (virDomainParseMemory("./label/size", "./label/size/@unit", ctxt, &def->labelsize, false, false) < 0) return -1; @@ -16854,6 +16884,27 @@ virDomainMemoryTargetDefParseXML(xmlNodePtr node, if (virXPathBoolean("boolean(./readonly)", ctxt)) def->readonly = true; + + break; + + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO: + if (!def->s.virtio.pmem) { + if (virDomainParseMemory("./block", "./block/@unit", ctxt, + &def->blocksize, false, false) < 0) + return -1; + + if (virDomainParseMemory("./requested", "./requested/@unit", ctxt, + &def->requestedsize, false, false) < 0) + return -1; + } + + break; + + case VIR_DOMAIN_MEMORY_MODEL_NONE: + case VIR_DOMAIN_MEMORY_MODEL_DIMM: + case VIR_DOMAIN_MEMORY_MODEL_LAST: + /* nada */ + break; } return 0; @@ -18668,7 +18719,9 @@ virDomainMemoryFindByDefInternal(virDomainDefPtr def, /* target info -> always present */ if (tmp->model != mem->model || tmp->targetNode != mem->targetNode || - tmp->size != mem->size) + tmp->size != mem->size || + tmp->blocksize != mem->blocksize || + tmp->requestedsize != mem->requestedsize) continue; switch (mem->model) { @@ -24289,6 +24342,22 @@ virDomainMemoryDefCheckABIStability(virDomainMemoryDefPtr src, return false; } + if (src->blocksize != dst->blocksize) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target memory device block size '%llu' doesn't match " + "source memory device block size '%llu'"), + dst->blocksize, src->blocksize); + return false; + } + + if (src->requestedsize != dst->requestedsize) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target memory device requested size '%llu' doesn't match " + "source memory device requested size '%llu'"), + dst->requestedsize, src->requestedsize); + return false; + } + switch (src->model) { case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: if (src->labelsize != dst->labelsize) { @@ -28005,6 +28074,18 @@ virDomainMemorySourceDefFormat(virBufferPtr buf, if (def->s.virtio.pmem) virBufferAddLit(&childBuf, "<pmem/>\n"); + + if (def->s.virtio.sourceNodes) { + if (!(bitmap = virBitmapFormat(def->s.virtio.sourceNodes))) + return -1; + + virBufferAsprintf(&childBuf, "<nodemask>%s</nodemask>\n", bitmap); + } + + if (def->s.virtio.pagesize) { + virBufferAsprintf(&childBuf, "<pagesize unit='KiB'>%llu</pagesize>\n", + def->s.virtio.pagesize); + } break; case VIR_DOMAIN_MEMORY_MODEL_NONE: @@ -28036,6 +28117,14 @@ virDomainMemoryTargetDefFormat(virBufferPtr buf, if (def->readonly) virBufferAddLit(&childBuf, "<readonly/>\n"); + if (def->blocksize) { + virBufferAsprintf(&childBuf, "<block unit='KiB'>%llu</block>\n", + def->blocksize); + + virBufferAsprintf(&childBuf, "<requested unit='KiB'>%llu</requested>\n", + def->requestedsize); + } + virXMLFormatElement(buf, "target", NULL, &childBuf); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 5c37ba6513..3f111e994b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2327,8 +2327,11 @@ struct _virDomainMemoryDef { bool pmem; } nvdimm; /* VIR_DOMAIN_MEMORY_MODEL_NVDIMM */ struct { + // nodemask + hugepages + no prealloc char *path; /* Required for pmem, otherwise optional */ bool pmem; + virBitmapPtr sourceNodes; + unsigned long long pagesize; /* kibibytes */ } virtio; /* VIR_DOMAIN_MEMORY_MODEL_VIRTIO */ } s; @@ -2337,6 +2340,8 @@ struct _virDomainMemoryDef { int targetNode; unsigned long long size; /* kibibytes */ unsigned long long labelsize; /* kibibytes; valid only for NVDIMM */ + unsigned long long blocksize; /* kibibytes, valid for virtio-mem only */ + unsigned long long requestedsize; /* kibibytes, valid for virtio-mem only */ bool readonly; /* valid only for NVDIMM */ /* required for QEMU NVDIMM ppc64 support */ diff --git a/tests/qemuxml2argvdata/memory-hotplug-virtio-mem.xml b/tests/qemuxml2argvdata/memory-hotplug-virtio-mem.xml new file mode 100644 index 0000000000..0c8d1d970e --- /dev/null +++ b/tests/qemuxml2argvdata/memory-hotplug-virtio-mem.xml @@ -0,0 +1,78 @@ +<domain type='kvm'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <maxMemory slots='16' unit='KiB'>1099511627776</maxMemory> + <memory unit='KiB'>8388608</memory> + <currentMemory unit='KiB'>8388608</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'> + <target> + <size unit='KiB'>1048576</size> + <node>0</node> + <block unit='KiB'>2048</block> + <requested unit='KiB'>524288</requested> + </target> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </memory> + <memory model='virtio'> + <source> + <nodemask>1-3</nodemask> + <pagesize unit='KiB'>2048</pagesize> + </source> + <target> + <size unit='KiB'>2097152</size> + <node>0</node> + <block unit='KiB'>2048</block> + <requested unit='KiB'>1048576</requested> + </target> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </memory> + <memory model='virtio'> + <source> + <path>/tmp/virtio_mem</path> + </source> + <target> + <size unit='KiB'>4194304</size> + <node>0</node> + <block unit='KiB'>2048</block> + <requested unit='KiB'>524288</requested> + </target> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + </memory> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/memory-hotplug-virtio-mem.x86_64-latest.xml b/tests/qemuxml2xmloutdata/memory-hotplug-virtio-mem.x86_64-latest.xml new file mode 120000 index 0000000000..a9d298129c --- /dev/null +++ b/tests/qemuxml2xmloutdata/memory-hotplug-virtio-mem.x86_64-latest.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/memory-hotplug-virtio-mem.xml \ No newline at end of file diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index e804db8aee..00d8f171bc 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1239,6 +1239,7 @@ mymain(void) 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_CAPS_LATEST("memory-hotplug-virtio-mem"); DO_TEST("net-udp", NONE); -- 2.26.2

As advertised in the previous commit, we want' to be able to change 'requested-size' attribute of virtio-mem on the fly. This commit does exactly that. Changing anything else is checked for and forbidden. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/conf/domain_conf.c | 23 +++++ src/conf/domain_conf.h | 3 + src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 165 ++++++++++++++++++++++++++++++++++- src/qemu/qemu_hotplug.c | 18 ++++ src/qemu/qemu_hotplug.h | 5 ++ src/qemu/qemu_monitor.c | 13 +++ src/qemu/qemu_monitor.h | 4 + src/qemu/qemu_monitor_json.c | 15 ++++ src/qemu/qemu_monitor_json.h | 5 ++ 10 files changed, 251 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0a402f1a51..5db1fee16b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18781,6 +18781,29 @@ virDomainMemoryFindInactiveByDef(virDomainDefPtr def, } +ssize_t +virDomainMemoryFindByDeviceInfo(virDomainDefPtr def, + virDomainDeviceInfoPtr info) +{ + size_t i; + + for (i = 0; i < def->nmems; i++) { + virDomainMemoryDefPtr tmp = def->mems[i]; + + if (!virDomainDeviceInfoAddressIsEqual(&tmp->info, info)) + continue; + + /* alias, if present */ + if (STRNEQ_NULLABLE(tmp->info.alias, info->alias)) + continue; + + return i; + } + + return -1; +} + + /** * virDomainMemoryInsert: * diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3f111e994b..31892c4941 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3580,6 +3580,9 @@ int virDomainMemoryFindByDef(virDomainDefPtr def, virDomainMemoryDefPtr mem) int virDomainMemoryFindInactiveByDef(virDomainDefPtr def, virDomainMemoryDefPtr mem) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT; +ssize_t virDomainMemoryFindByDeviceInfo(virDomainDefPtr dev, + virDomainDeviceInfoPtr info) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT; int virDomainShmemDefInsert(virDomainDefPtr def, virDomainShmemDefPtr shmem) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f581676227..1bed019aac 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -497,6 +497,7 @@ virDomainMemballoonModelTypeFromString; virDomainMemballoonModelTypeToString; virDomainMemoryDefFree; virDomainMemoryFindByDef; +virDomainMemoryFindByDeviceInfo; virDomainMemoryFindInactiveByDef; virDomainMemoryInsert; virDomainMemoryModelTypeToString; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 391596ba11..677f921920 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7109,6 +7109,158 @@ qemuDomainChangeDiskLive(virDomainObjPtr vm, return 0; } + +static bool +qemuDomainChangeMemoryLiveValidateChange(const virDomainMemoryDef *oldDef, + const virDomainMemoryDef *newDef) +{ + /* The only thing that is allowed to change is 'requestedsize' for virtio + * model. */ + if (oldDef->model != newDef->model) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot modify memory model from '%s' to '%s'"), + virDomainMemoryModelTypeToString(oldDef->model), + virDomainMemoryModelTypeToString(newDef->model)); + return false; + } + + if (oldDef->access != newDef->access) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot modify memory access from '%s' to '%s'"), + virDomainMemoryAccessTypeToString(oldDef->access), + virDomainMemoryAccessTypeToString(newDef->access)); + return false; + } + + if (oldDef->discard != newDef->discard) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot modify memory discard from '%s' to '%s'"), + virTristateBoolTypeToString(oldDef->discard), + virTristateBoolTypeToString(newDef->discard)); + return false; + } + + if (oldDef->targetNode != newDef->targetNode) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot modify memory targetNode from '%d' to '%d'"), + oldDef->targetNode, newDef->targetNode); + return false; + } + + if (oldDef->size != newDef->size) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot modify memory size from '%llu' to '%llu'"), + oldDef->size, newDef->size); + return false; + } + + if (oldDef->blocksize != newDef->blocksize) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot modify memory block size from '%llu' to '%llu'"), + oldDef->blocksize, newDef->blocksize); + return false; + } + + /* requestedsize can change */ + + if (oldDef->readonly != newDef->readonly) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("cannot modify memory pmem flag")); + return false; + } + + if (virUUIDIsValid(oldDef->uuid) && + memcmp(oldDef->uuid, newDef, VIR_UUID_BUFLEN) != 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("cannot modify memory UUID")); + return false; + } + + switch (oldDef->model) { + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO: + if (STRNEQ_NULLABLE(oldDef->s.virtio.path, newDef->s.virtio.path)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot modify memory path from '%s' to '%s'"), + NULLSTR(oldDef->s.virtio.path), + NULLSTR(newDef->s.virtio.path)); + return false; + } + + if (oldDef->s.virtio.pmem != newDef->s.virtio.pmem) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("cannot modify memory pmem flag")); + return false; + } + + if (!virBitmapEqual(oldDef->s.virtio.sourceNodes, + newDef->s.virtio.sourceNodes)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("cannot modify memory source nodes")); + return false; + } + + if (oldDef->s.virtio.pagesize != newDef->s.virtio.pagesize) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot modify memory pagesize from '%llu' to '%llu'"), + oldDef->s.virtio.pagesize, + newDef->s.virtio.pagesize); + return false; + } + break; + + case VIR_DOMAIN_MEMORY_MODEL_NONE: + case VIR_DOMAIN_MEMORY_MODEL_DIMM: + case VIR_DOMAIN_MEMORY_MODEL_LAST: + case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot modify memory of model '%s'"), + virDomainMemoryModelTypeToString(oldDef->model)); + return false; + break; + } + + return true; +} + + +static int +qemuDomainChangeMemoryLive(virQEMUDriverPtr driver G_GNUC_UNUSED, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev) +{ + virDomainMemoryDefPtr newDef = dev->data.memory; + virDomainMemoryDefPtr oldDef = NULL; + ssize_t idx; + + idx = virDomainMemoryFindByDeviceInfo(vm->def, &dev->data.memory->info); + if (idx < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("memory '%s' not found"), dev->data.memory->info.alias); + return -1; + } + + oldDef = vm->def->mems[idx]; + + if (newDef->model != VIR_DOMAIN_MEMORY_MODEL_VIRTIO || + newDef->s.virtio.pmem) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("changing anything but <requested/> size for virtio-mem is not implemented yet")); + return -1; + } + + if (!qemuDomainChangeMemoryLiveValidateChange(oldDef, newDef)) + return -1; + + if (qemuDomainChangeMemoryRequestedSize(driver, vm, + newDef->info.alias, + newDef->requestedsize) < 0) + return -1; + + oldDef->requestedsize = newDef->requestedsize; + return 0; +} + + static int qemuDomainUpdateDeviceLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev, @@ -7150,6 +7302,18 @@ qemuDomainUpdateDeviceLive(virDomainObjPtr vm, ret = qemuDomainChangeNet(driver, vm, dev); break; + case VIR_DOMAIN_DEVICE_MEMORY: + if ((idx = virDomainMemoryFindByDeviceInfo(vm->def, &dev->data.memory->info)) >= 0) { + oldDev.data.memory = vm->def->mems[idx]; + if (virDomainDefCompatibleDevice(vm->def, dev, &oldDev, + VIR_DOMAIN_DEVICE_ACTION_UPDATE, + true) < 0) + return -1; + } + + ret = qemuDomainChangeMemoryLive(driver, vm, dev); + break; + case VIR_DOMAIN_DEVICE_FS: case VIR_DOMAIN_DEVICE_INPUT: case VIR_DOMAIN_DEVICE_SOUND: @@ -7165,7 +7329,6 @@ qemuDomainUpdateDeviceLive(virDomainObjPtr vm, case VIR_DOMAIN_DEVICE_HOSTDEV: case VIR_DOMAIN_DEVICE_CONTROLLER: case VIR_DOMAIN_DEVICE_REDIRDEV: - case VIR_DOMAIN_DEVICE_MEMORY: case VIR_DOMAIN_DEVICE_CHR: case VIR_DOMAIN_DEVICE_NONE: case VIR_DOMAIN_DEVICE_TPM: diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index bc5dc02a2f..a18fa6d362 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -6672,3 +6672,21 @@ qemuDomainSetVcpuInternal(virQEMUDriverPtr driver, virBitmapFree(livevcpus); return ret; } + + +int +qemuDomainChangeMemoryRequestedSize(virQEMUDriverPtr driver, + virDomainObjPtr vm, + const char *alias, + unsigned long long requestedsize) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + int rc; + + qemuDomainObjEnterMonitor(driver, vm); + rc = qemuMonitorChangeMemoryRequestedSize(priv->mon, alias, requestedsize); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + return -1; + + return rc; +} diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index 6287c5b5e8..9e551a1f82 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -160,3 +160,8 @@ int qemuHotplugAttachDBusVMState(virQEMUDriverPtr driver, int qemuHotplugRemoveDBusVMState(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuDomainAsyncJob asyncJob); + +int qemuDomainChangeMemoryRequestedSize(virQEMUDriverPtr driver, + virDomainObjPtr vm, + const char *alias, + unsigned long long requestedsize); diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index f2ed165b22..ace7c889d4 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4769,3 +4769,16 @@ qemuMonitorTransactionBackup(virJSONValuePtr actions, return qemuMonitorJSONTransactionBackup(actions, device, jobname, target, bitmap, syncmode); } + + +int +qemuMonitorChangeMemoryRequestedSize(qemuMonitorPtr mon, + const char *alias, + unsigned long long requestedsize) +{ + VIR_DEBUG("alias=%s requestedsize=%llu", alias, requestedsize); + + QEMU_CHECK_MONITOR(mon); + + return qemuMonitorJSONChangeMemoryRequestedSize(mon, alias, requestedsize); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index d301568e40..c792c95c46 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1527,3 +1527,7 @@ qemuMonitorTransactionBackup(virJSONValuePtr actions, const char *target, const char *bitmap, qemuMonitorTransactionBackupSyncMode syncmode); + +int qemuMonitorChangeMemoryRequestedSize(qemuMonitorPtr mon, + const char *alias, + unsigned long long requestedsize); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 723bdb4426..3d94181afb 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -9635,3 +9635,18 @@ qemuMonitorJSONGetCPUMigratable(qemuMonitorPtr mon, return virJSONValueGetBoolean(virJSONValueObjectGet(reply, "return"), migratable); } + + +int +qemuMonitorJSONChangeMemoryRequestedSize(qemuMonitorPtr mon, + const char *alias, + unsigned long long requestedsize) +{ + g_autofree char *path = g_strdup_printf("/machine/peripheral/%s", alias); + qemuMonitorJSONObjectProperty prop = { + .type = QEMU_MONITOR_OBJECT_PROPERTY_ULONG, + .val.ul = requestedsize * 1024, /* monitor needs bytes */ + }; + + return qemuMonitorJSONSetObjectProperty(mon, path, "requested-size", &prop); +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index b588722d90..dcf101a165 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -710,3 +710,8 @@ int qemuMonitorJSONSetDBusVMStateIdList(qemuMonitorPtr mon, int qemuMonitorJSONGetCPUMigratable(qemuMonitorPtr mon, bool *migratable); + +int +qemuMonitorJSONChangeMemoryRequestedSize(qemuMonitorPtr mon, + const char *alias, + unsigned long long requestedsize); -- 2.26.2

As advertised in previous commit, this event is delivered to us when virtio-mem module changes the allocation inside the guest. It comes with one attribute - size - which holds the new size of the virtio-mem (well, allocated size), in bytes. Mind you, this is not necessarily the same number as 'requested size'. It almost certainly will be when sizing the memory up, but it might not be when sizing the memory down - the guest kernel might be unable to free some blocks. This actual size is reported in the domain XML as an output element only. TODO: Fix up total domain memory calculation. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- docs/formatdomain.rst | 7 ++++++ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 24 ++++++++++++++++++-- src/conf/domain_conf.h | 7 ++++++ src/libvirt_private.syms | 1 + src/qemu/qemu_domain.c | 3 +++ src/qemu/qemu_domain.h | 1 + src/qemu/qemu_driver.c | 33 ++++++++++++++++++++++++++++ src/qemu/qemu_monitor.c | 24 ++++++++++++++++++++ src/qemu/qemu_monitor.h | 20 +++++++++++++++++ src/qemu/qemu_monitor_json.c | 24 ++++++++++++++++++++ src/qemu/qemu_process.c | 41 +++++++++++++++++++++++++++++++++++ 12 files changed, 188 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 3990728939..ac87d03b33 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -7196,6 +7196,7 @@ Example: usage of the memory devices <node>0</node> <block unit='KiB'>2048</block> <requested unit='KiB'>524288</requested> + <actual unit='KiB'>524288</requested> </target> </memory> <memory model='virtio' access='shared'> @@ -7322,6 +7323,12 @@ Example: usage of the memory devices granularity. This is valid for ``virtio`` model only and mutually exclusive with ``pmem``. + ``actual`` + The active XML for a ``virtio`` model may contain ``actual`` element that + reflects the actual size of the corresponding virtio memory device. The + element is formatted into live XML and never parsed, i.e. it is + output-only element. + :anchor:`<a id="elementsIommu"/>` IOMMU devices diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index d478b639fa..3b12902e04 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -6063,6 +6063,11 @@ <ref name="scaledInteger"/> </element> </optional> + <optional> + <element name="actual"> + <ref name="scaledInteger"/> + </element> + </optional> <optional> <element name="node"> <ref name="unsignedInt"/> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5db1fee16b..05f5d70cee 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18804,6 +18804,21 @@ virDomainMemoryFindByDeviceInfo(virDomainDefPtr def, } +ssize_t +virDomainMemoryFindByDeviceAlias(virDomainDefPtr def, + const char *alias) +{ + size_t i; + + for (i = 0; i < def->nmems; i++) { + if (STREQ_NULLABLE(def->mems[i]->info.alias, alias)) + return i; + } + + return -1; +} + + /** * virDomainMemoryInsert: * @@ -28124,7 +28139,8 @@ virDomainMemorySourceDefFormat(virBufferPtr buf, static void virDomainMemoryTargetDefFormat(virBufferPtr buf, - virDomainMemoryDefPtr def) + virDomainMemoryDefPtr def, + unsigned int flags) { g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); @@ -28146,6 +28162,10 @@ virDomainMemoryTargetDefFormat(virBufferPtr buf, virBufferAsprintf(&childBuf, "<requested unit='KiB'>%llu</requested>\n", def->requestedsize); + if (!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE)) { + virBufferAsprintf(&childBuf, "<actual unit='KiB'>%llu</actual>\n", + def->actualsize); + } } virXMLFormatElement(buf, "target", NULL, &childBuf); @@ -28180,7 +28200,7 @@ virDomainMemoryDefFormat(virBufferPtr buf, if (virDomainMemorySourceDefFormat(buf, def) < 0) return -1; - virDomainMemoryTargetDefFormat(buf, def); + virDomainMemoryTargetDefFormat(buf, def, flags); virDomainDeviceInfoFormat(buf, &def->info, flags); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 31892c4941..633c07b59c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2342,6 +2342,9 @@ struct _virDomainMemoryDef { unsigned long long labelsize; /* kibibytes; valid only for NVDIMM */ unsigned long long blocksize; /* kibibytes, valid for virtio-mem only */ unsigned long long requestedsize; /* kibibytes, valid for virtio-mem only */ + unsigned long long actualsize; /* kibibytes, valid for virtio-mem and + active domain only, only to report never + parse */ bool readonly; /* valid only for NVDIMM */ /* required for QEMU NVDIMM ppc64 support */ @@ -3584,6 +3587,10 @@ ssize_t virDomainMemoryFindByDeviceInfo(virDomainDefPtr dev, virDomainDeviceInfoPtr info) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT; +ssize_t virDomainMemoryFindByDeviceAlias(virDomainDefPtr def, + const char *alias) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT; + int virDomainShmemDefInsert(virDomainDefPtr def, virDomainShmemDefPtr shmem) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT; bool virDomainShmemDefEquals(virDomainShmemDefPtr src, virDomainShmemDefPtr dst) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1bed019aac..0fdec594ba 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -497,6 +497,7 @@ virDomainMemballoonModelTypeFromString; virDomainMemballoonModelTypeToString; virDomainMemoryDefFree; virDomainMemoryFindByDef; +virDomainMemoryFindByDeviceAlias; virDomainMemoryFindByDeviceInfo; virDomainMemoryFindInactiveByDef; virDomainMemoryInsert; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ab7938a355..fc994ec282 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -10554,6 +10554,9 @@ qemuProcessEventFree(struct qemuProcessEvent *event) case QEMU_PROCESS_EVENT_JOB_STATUS_CHANGE: virObjectUnref(event->data); break; + case QEMU_PROCESS_EVENT_MEMORY_DEVICE_SIZE_CHANGE: + qemuMonitorMemoryDeviceSizeChangeFree(event->data); + break; case QEMU_PROCESS_EVENT_PR_DISCONNECT: case QEMU_PROCESS_EVENT_LAST: break; diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 010bae285d..376679da77 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -441,6 +441,7 @@ typedef enum { QEMU_PROCESS_EVENT_PR_DISCONNECT, QEMU_PROCESS_EVENT_RDMA_GID_STATUS_CHANGED, QEMU_PROCESS_EVENT_GUEST_CRASHLOADED, + QEMU_PROCESS_EVENT_MEMORY_DEVICE_SIZE_CHANGE, QEMU_PROCESS_EVENT_LAST } qemuProcessEventType; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 677f921920..0c5db18dff 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4289,6 +4289,36 @@ processGuestCrashloadedEvent(virQEMUDriverPtr driver, } +static void +processMemoryDeviceSizeChange(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuMonitorMemoryDeviceSizeChangePtr info) +{ + virDomainMemoryDefPtr mem = NULL; + ssize_t idx; + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + return; + + if (!virDomainObjIsActive(vm)) { + VIR_DEBUG("Domain is not running"); + goto endjob; + } + + idx = virDomainMemoryFindByDeviceAlias(vm->def, info->devAlias); + if (idx < 0) { + VIR_DEBUG("Memory device '%s' not found", info->devAlias); + goto endjob; + } + + mem = vm->def->mems[idx]; + mem->actualsize = info->size / 1024; + + endjob: + qemuDomainObjEndJob(driver, vm); +} + + static void qemuProcessEventHandler(void *data, void *opaque) { struct qemuProcessEvent *processEvent = data; @@ -4338,6 +4368,9 @@ static void qemuProcessEventHandler(void *data, void *opaque) case QEMU_PROCESS_EVENT_GUEST_CRASHLOADED: processGuestCrashloadedEvent(driver, vm); break; + case QEMU_PROCESS_EVENT_MEMORY_DEVICE_SIZE_CHANGE: + processMemoryDeviceSizeChange(driver, vm, processEvent->data); + break; case QEMU_PROCESS_EVENT_LAST: break; } diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index ace7c889d4..5f1cae9f48 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1435,6 +1435,20 @@ qemuMonitorEmitSpiceMigrated(qemuMonitorPtr mon) } +int +qemuMonitorEmitMemoryDeviceSizeChange(qemuMonitorPtr mon, + const char *devAlias, + unsigned long long size) +{ + int ret = -1; + VIR_DEBUG("mon=%p, devAlias='%s', size=%llu", mon, devAlias, size); + + QEMU_MONITOR_CALLBACK(mon, ret, domainMemoryDeviceSizeChange, mon->vm, devAlias, size); + + return ret; +} + + int qemuMonitorEmitMemoryFailure(qemuMonitorPtr mon, qemuMonitorEventMemoryFailurePtr mfp) @@ -4456,6 +4470,16 @@ qemuMonitorEventRdmaGidStatusFree(qemuMonitorRdmaGidStatusPtr info) } +void +qemuMonitorMemoryDeviceSizeChangeFree(qemuMonitorMemoryDeviceSizeChangePtr info) +{ + if (!info) + return; + + VIR_FREE(info->devAlias); +} + + int qemuMonitorSetWatchdogAction(qemuMonitorPtr mon, const char *action) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index c792c95c46..63c52ce6e8 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -108,6 +108,14 @@ struct _qemuMonitorRdmaGidStatus { }; +typedef struct _qemuMonitorMemoryDeviceSizeChange qemuMonitorMemoryDeviceSizeChange; +typedef qemuMonitorMemoryDeviceSizeChange *qemuMonitorMemoryDeviceSizeChangePtr; +struct _qemuMonitorMemoryDeviceSizeChange { + char *devAlias; + unsigned long long size; +}; + + typedef enum { QEMU_MONITOR_JOB_TYPE_UNKNOWN, /* internal value, not exposed by qemu */ QEMU_MONITOR_JOB_TYPE_COMMIT, @@ -153,6 +161,7 @@ struct _qemuMonitorJobInfo { char *qemuMonitorGuestPanicEventInfoFormatMsg(qemuMonitorEventPanicInfoPtr info); void qemuMonitorEventPanicInfoFree(qemuMonitorEventPanicInfoPtr info); void qemuMonitorEventRdmaGidStatusFree(qemuMonitorRdmaGidStatusPtr info); +void qemuMonitorMemoryDeviceSizeChangeFree(qemuMonitorMemoryDeviceSizeChangePtr info); typedef void (*qemuMonitorDestroyCallback)(qemuMonitorPtr mon, virDomainObjPtr vm, @@ -374,6 +383,12 @@ typedef int (*qemuMonitorDomainMemoryFailureCallback)(qemuMonitorPtr mon, qemuMonitorEventMemoryFailurePtr mfp, void *opaque); +typedef int (*qemuMonitorDomainMemoryDeviceSizeChange)(qemuMonitorPtr mon, + virDomainObjPtr vm, + const char *alias, + unsigned long long size, + void *opaque); + typedef struct _qemuMonitorCallbacks qemuMonitorCallbacks; typedef qemuMonitorCallbacks *qemuMonitorCallbacksPtr; struct _qemuMonitorCallbacks { @@ -411,6 +426,7 @@ struct _qemuMonitorCallbacks { qemuMonitorDomainRdmaGidStatusChangedCallback domainRdmaGidStatusChanged; qemuMonitorDomainGuestCrashloadedCallback domainGuestCrashloaded; qemuMonitorDomainMemoryFailureCallback domainMemoryFailure; + qemuMonitorDomainMemoryDeviceSizeChange domainMemoryDeviceSizeChange; }; qemuMonitorPtr qemuMonitorOpen(virDomainObjPtr vm, @@ -511,6 +527,10 @@ int qemuMonitorEmitSerialChange(qemuMonitorPtr mon, bool connected); int qemuMonitorEmitSpiceMigrated(qemuMonitorPtr mon); +int qemuMonitorEmitMemoryDeviceSizeChange(qemuMonitorPtr mon, + const char *devAlias, + unsigned long long size); + int qemuMonitorEmitMemoryFailure(qemuMonitorPtr mon, qemuMonitorEventMemoryFailurePtr mfp); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 3d94181afb..0c050b27b7 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -113,6 +113,7 @@ static void qemuMonitorJSONHandleDumpCompleted(qemuMonitorPtr mon, virJSONValueP static void qemuMonitorJSONHandlePRManagerStatusChanged(qemuMonitorPtr mon, virJSONValuePtr data); static void qemuMonitorJSONHandleRdmaGidStatusChanged(qemuMonitorPtr mon, virJSONValuePtr data); static void qemuMonitorJSONHandleMemoryFailure(qemuMonitorPtr mon, virJSONValuePtr data); +static void qemuMonitorJSONHandleMemoryDeviceSizeChange(qemuMonitorPtr mon, virJSONValuePtr data); typedef struct { const char *type; @@ -133,6 +134,7 @@ static qemuEventHandler eventHandlers[] = { { "GUEST_CRASHLOADED", qemuMonitorJSONHandleGuestCrashloaded, }, { "GUEST_PANICKED", qemuMonitorJSONHandleGuestPanic, }, { "JOB_STATUS_CHANGE", qemuMonitorJSONHandleJobStatusChange, }, + { "MEMORY_DEVICE_SIZE_CHANGE", qemuMonitorJSONHandleMemoryDeviceSizeChange, }, { "MEMORY_FAILURE", qemuMonitorJSONHandleMemoryFailure, }, { "MIGRATION", qemuMonitorJSONHandleMigrationStatus, }, { "MIGRATION_PASS", qemuMonitorJSONHandleMigrationPass, }, @@ -1335,6 +1337,28 @@ qemuMonitorJSONHandleSpiceMigrated(qemuMonitorPtr mon, } +static void +qemuMonitorJSONHandleMemoryDeviceSizeChange(qemuMonitorPtr mon, + virJSONValuePtr data) +{ + const char *name; + unsigned long long size; + + if (!(name = virJSONValueObjectGetString(data, "id"))) { + VIR_WARN("missing device alias in MEMORY_DEVICE_SIZE_CHANGE event"); + return; + } + + if (virJSONValueObjectGetNumberUlong(data, "size", &size) < 0) { + VIR_WARN("missing new size for '%s' in MEMORY_DEVICE_SIZE_CHANGE event", name); + return; + } + + + qemuMonitorEmitMemoryDeviceSizeChange(mon, name, size); +} + + static void qemuMonitorJSONHandleMemoryFailure(qemuMonitorPtr mon, virJSONValuePtr data) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e68df5abe7..64576377b8 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1932,6 +1932,46 @@ qemuProcessHandleMemoryFailure(qemuMonitorPtr mon G_GNUC_UNUSED, } +static int +qemuProcessHandleMemoryDeviceSizeChange(qemuMonitorPtr mon G_GNUC_UNUSED, + virDomainObjPtr vm, + const char *devAlias, + unsigned long long size, + void *opaque) +{ + virQEMUDriverPtr driver = opaque; + struct qemuProcessEvent *processEvent = NULL; + qemuMonitorMemoryDeviceSizeChangePtr info = NULL; + int ret = -1; + + virObjectLock(vm); + + VIR_DEBUG("Memory device '%s' changed size to '%llu' in domain '%s'", + devAlias, size, vm->def->name); + + info = g_new0(qemuMonitorMemoryDeviceSizeChange, 1); + info->devAlias = g_strdup(devAlias); + info->size = size; + + processEvent = g_new0(struct qemuProcessEvent, 1); + processEvent->eventType = QEMU_PROCESS_EVENT_MEMORY_DEVICE_SIZE_CHANGE; + processEvent->vm = virObjectRef(vm); + processEvent->data = g_steal_pointer(&info); + + if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) { + qemuProcessEventFree(processEvent); + virObjectUnref(vm); + goto cleanup; + } + + ret = 0; + cleanup: + qemuMonitorMemoryDeviceSizeChangeFree(info); + virObjectUnlock(vm); + return ret; +} + + static qemuMonitorCallbacks monitorCallbacks = { .eofNotify = qemuProcessHandleMonitorEOF, .errorNotify = qemuProcessHandleMonitorError, @@ -1965,6 +2005,7 @@ static qemuMonitorCallbacks monitorCallbacks = { .domainRdmaGidStatusChanged = qemuProcessHandleRdmaGidStatusChanged, .domainGuestCrashloaded = qemuProcessHandleGuestCrashloaded, .domainMemoryFailure = qemuProcessHandleMemoryFailure, + .domainMemoryDeviceSizeChange = qemuProcessHandleMemoryDeviceSizeChange, }; static void -- 2.26.2

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 7203403b31..2784e278b0 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