Introduced in v8.2.0-rc0~74^2~2, QEMU now allows setting .dynamic-memslots attribute for virtio-mem-pci devices. When turned on, it allows memory exposed to guest to be split into multiple memslots and thus smaller memory footprint (see the original commit for detailed explanation). Therefore, introduce new <target/> attribute which will control that QEMU knob. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- docs/formatdomain.rst | 6 ++++++ src/conf/domain_conf.c | 18 +++++++++++++++++- src/conf/domain_conf.h | 1 + src/conf/schemas/domaincommon.rng | 5 +++++ .../memory-hotplug-virtio-mem.xml | 2 +- 5 files changed, 30 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 96e03a3807..57974de9f4 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -8395,6 +8395,12 @@ Example: usage of the memory devices The ``node`` subelement configures the guest NUMA node to attach the memory to. The element shall be used only if the guest has NUMA nodes configured. + For ``virtio-mem`` optional attribute ``dynamicMemslots`` can be specified + (accepted values "yes"/"no") which allows hypervisor to spread memory into + multiple memory slots (allocate them dynamically based on the amount of + memory exposed to the guest), resulting in smaller memory footprint. + :since:`Since 10.0.0 and QEMU 8.2.0` + The following optional elements may be used: ``label`` diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 39e9879a8a..e677023b17 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13499,6 +13499,10 @@ virDomainMemoryTargetDefParseXML(xmlNodePtr node, &def->target.virtio_mem.requestedsize, false, false) < 0) return -1; + if (virXMLPropTristateBool(node, "dynamicMemslots", VIR_XML_PROP_NONE, + &def->target.virtio_mem.dynamicMemslots) < 0) + return -1; + addrNode = virXPathNode("./address", ctxt); addr = &def->target.virtio_mem.address; break; @@ -21173,6 +21177,12 @@ virDomainMemoryDefCheckABIStability(virDomainMemoryDef *src, src->target.virtio_mem.address); return false; } + + if (src->target.virtio_mem.dynamicMemslots != dst->target.virtio_mem.dynamicMemslots) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Target memory device 'dynamicMemslots' property doesn't match source memory device")); + return false; + } break; case VIR_DOMAIN_MEMORY_MODEL_DIMM: @@ -25374,6 +25384,7 @@ virDomainMemoryTargetDefFormat(virBuffer *buf, unsigned int flags) { g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); + g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; virBufferAsprintf(&childBuf, "<size unit='KiB'>%llu</size>\n", def->size); if (def->targetNode >= 0) @@ -25413,6 +25424,11 @@ virDomainMemoryTargetDefFormat(virBuffer *buf, if (def->target.virtio_mem.address) virBufferAsprintf(&childBuf, "<address base='0x%llx'/>\n", def->target.virtio_mem.address); + + if (def->target.virtio_mem.dynamicMemslots) { + virBufferAsprintf(&attrBuf, " dynamicMemslots='%s'", + virTristateBoolTypeToString(def->target.virtio_mem.dynamicMemslots)); + } break; case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: @@ -25422,7 +25438,7 @@ virDomainMemoryTargetDefFormat(virBuffer *buf, break; } - virXMLFormatElement(buf, "target", NULL, &childBuf); + virXMLFormatElement(buf, "target", &attrBuf, &childBuf); } static int diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 58b2c92f4c..88ff3e1d84 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2674,6 +2674,7 @@ struct _virDomainMemoryDef { unsigned long long currentsize; /* kibibytes, valid for an active domain only and parsed */ unsigned long long address; /* address where memory is mapped */ + virTristateBool dynamicMemslots; } virtio_mem; struct { } sgx_epc; diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index f318c06797..08c408e138 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -7260,6 +7260,11 @@ <define name="memorydev-target"> <element name="target"> + <optional> + <attribute name="dynamicMemslots"> + <ref name="virYesNo"/> + </attribute> + </optional> <interleave> <element name="size"> <ref name="scaledInteger"/> diff --git a/tests/qemuxml2argvdata/memory-hotplug-virtio-mem.xml b/tests/qemuxml2argvdata/memory-hotplug-virtio-mem.xml index c578209d8a..359ece7a77 100644 --- a/tests/qemuxml2argvdata/memory-hotplug-virtio-mem.xml +++ b/tests/qemuxml2argvdata/memory-hotplug-virtio-mem.xml @@ -60,7 +60,7 @@ <nodemask>1-3</nodemask> <pagesize unit='KiB'>2048</pagesize> </source> - <target> + <target dynamicMemslots='yes'> <size unit='KiB'>2097152</size> <node>0</node> <block unit='KiB'>2048</block> -- 2.41.0

On 05.01.24 13:40, Peter Krempa wrote: In combination with some vhost devices (especially vhost-user), starting QEMU might fail until these devices support more memslots. There is ongoing work to improve these vhost-user devices. To make these setups temporarily work, one would have to manually set "dynamic-memslots=off" here, which is not too bad I guess. Besides that, I don't think there is a real drawback -- there are mostly only benefits of having it :) I'm actually hoping that we can default-enable it, at least in RHEL soon. Maybe we can default-enable it right away and document that vhost-user devices will require it to be manually disabled until they support more memslots? -- Cheers, David / dhildenb

On 05.01.24 14:58, Peter Krempa wrote: No, that problem is the combination. Assume you have VM with both virtio-mem to hotplug memory and virtio-user-fs+virtiofsd. The memory that virtio-mem provides has to be usable by virtio-user-fs+virtiofsd. Therefore, we have to map that shared memory (provided by virtio-mem) into the virtiofsd process. However, virtiofsd only supports 32 memslots and virito-mem wants to use up to 256 (like having 256 DIMMs). virtiofsd support is in the works but not upstream yet. So if you start a vm with -device virtio-mem-pci,dynamic-memslots=on,... -device virtio-user-fs-pci, ... QEMU might reject to start if the virtio-user-fs-pci backend has insufficient memory slots. In that case (and only then), we want to disable dynamic memslots. Without the virtio-user complication (that I hate), this wouldn't even be a configurable toggle. Yes. -- Cheers, David / dhildenb

On Fri, Jan 05, 2024 at 13:33:32 +0100, Michal Privoznik wrote: Except for the docs here, which are insufficient as outlined in the other subthread, the code looks good, so for the code: Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt;

On Wed, Jan 10, 2024 at 15:54:18 +0100, Michal Prívozník wrote: I'd add: 'vhost-user devices (such as virtiofsd)'. Optionally you can add more, but virtiofsd is the one users can configure without being aware of it.a also perhaps 'vhost-user device implementations' for the next sentence. IMO the stuff in the parentheses can be dropped. the memory footprint note is redundant from the first sentence. How about: When only modern vhost-user based devices will be used or when no vhost-user devices are expected to be used it's beneficial to enable this feature.

On Fri, Jan 05, 2024 at 13:33:35 +0100, Michal Privoznik wrote: virTristateBool dynamicMemslots; typedef enum { VIR_TRISTATE_BOOL_ABSENT = 0, VIR_TRISTATE_BOOL_YES, VIR_TRISTATE_BOOL_NO, VIR_TRISTATE_BOOL_LAST } virTristateBool; Either VIR_TRISTATE_BOOL_YES or VIR_TRISTATE_BOOL_NO are non-zero, so the typecast to bool will make this set dynamic memslots to true in both cases if the parameter is present ... Since the test case doesn't contain an explicit 'no' ... ... it's not visible here. Use the proper tristate variable and "T:asdf" convertor when formatting the json and add also the explicit 'no' to the test case. With the above Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt;