On Tue, Sep 14, 2021 at 9:07 PM David Hildenbrand <david@redhat.com> wrote:

On 13.09.21 16:52, Michal Privoznik wrote:
> Nothing special is happening here. All important changes were
> done when for 'virtio-pmem' (adjusting the code to put virtio
> memory on PCI bus, generating alias using
> qemuDomainDeviceAliasIndex(). The only bit that might look
> suspicious is no prealloc for virtio-mem. But if you think about
> it, the whole purpose of this device is to change amount of
> memory exposed to guest on the fly. There is no point in locking
> the whole backend in memory.

Do I understand correctly that we'll always try setting "reserve=off",
and "prealloc=off"? That would be awesome :)

I do wonder if we want to warn or bail out if "priv->memPrealloc" is set
but we are temporarily not able to support it -- as discussed, because
virtio-mem in QEMU yet has to be taught about it.

FYI: QEMU will bail out if "reserve=off,prealloc=on" is set in combination.

>
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
> src/qemu/qemu_alias.c | 9 +++-
> src/qemu/qemu_command.c | 24 +++++++++--
> ...mory-hotplug-virtio-mem.x86_64-latest.args | 41 +++++++++++++++++++
> tests/qemuxml2argvtest.c | 1 +
> 4 files changed, 70 insertions(+), 5 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/memory-hotplug-virtio-mem.x86_64-latest.args
>
> diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
> index 79e8953b2f..81a1e7eeed 100644
> --- a/src/qemu/qemu_alias.c
> +++ b/src/qemu/qemu_alias.c
> @@ -475,8 +475,11 @@ qemuDeviceMemoryGetAliasID(virDomainDef *def,
> size_t i;
> int maxidx = 0;
>
> - /* virtio-pmem goes onto PCI bus and thus DIMM address is not valid */
> - if (!oldAlias && mem->model != VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM)
> + /* virtio-pmem and virtio-mem go onto PCI bus and thus DIMM address is not
> + * valid */
> + if (!oldAlias &&
> + mem->model != VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM &&
> + mem->model != VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM)
> return mem->info.addr.dimm.slot;
>
> for (i = 0; i < def->nmems; i++) {
> @@ -523,6 +526,8 @@ qemuAssignDeviceMemoryAlias(virDomainDef *def,
> prefix = "virtiopmem";
> break;
> case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM:
> + prefix = "virtiomem";
> + break;
> case VIR_DOMAIN_MEMORY_MODEL_NONE:
> case VIR_DOMAIN_MEMORY_MODEL_LAST:
> default:
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 594e5643b1..91f3094ac8 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -3136,9 +3136,19 @@ qemuBuildMemoryBackendProps(virJSONValue **backendProps,
> virJSONValueObjectAdd(props, "b:x-use-canonical-path-for-ramblock-id", false, NULL) < 0)
> return -1;
>
> - if (!priv->memPrealloc &&
> - virJSONValueObjectAdd(props, "B:prealloc", prealloc, NULL) < 0)
> - return -1;
> + if (mem->model == VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM) {
> + /* Explicitly disable prealloc for virtio-mem */
> + if (priv->memPrealloc &&
> + virJSONValueObjectAppendBoolean(props, "prealloc", 0) < 0)
> + return -1;
> + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MEMORY_BACKEND_RESERVE) &&
> + virJSONValueObjectAppendBoolean(props, "reserve", 0) < 0)
> + return -1;
> + } else {
> + if (!priv->memPrealloc &&
> + virJSONValueObjectAdd(props, "B:prealloc", prealloc, NULL) < 0)
> + return -1;
> + }
>
> if (virJSONValueObjectAdd(props, "U:size", mem->size * 1024, NULL) < 0)
> return -1;
> @@ -3318,6 +3328,9 @@ qemuBuildMemoryDeviceStr(const virDomainDef *def,
> break;
>
> case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM:
> + device = "virtio-mem-pci";
> + break;
> +
> case VIR_DOMAIN_MEMORY_MODEL_NONE:
> case VIR_DOMAIN_MEMORY_MODEL_LAST:
> default:
> @@ -3334,6 +3347,11 @@ qemuBuildMemoryDeviceStr(const virDomainDef *def,
> if (mem->labelsize)
> virBufferAsprintf(&buf, "label-size=%llu,", mem->labelsize * 1024);
>
> + if (mem->blocksize) {
> + virBufferAsprintf(&buf, "block-size=%llu,", mem->blocksize * 1024);
> + virBufferAsprintf(&buf, "requested-size=%llu,", mem->requestedsize * 1024);
> + }
> +
> if (mem->uuid) {
> char uuidstr[VIR_UUID_STRING_BUFLEN];
>
> diff --git a/tests/qemuxml2argvdata/memory-hotplug-virtio-mem.x86_64-latest.args b/tests/qemuxml2argvdata/memory-hotplug-virtio-mem.x86_64-latest.args
> new file mode 100644
> index 0000000000..22ee1bc459
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/memory-hotplug-virtio-mem.x86_64-latest.args
> @@ -0,0 +1,41 @@
> +LC_ALL=C \
> +PATH=/bin \
> +HOME=/tmp/lib/domain--1-QEMUGuest1 \
> +USER=test \
> +LOGNAME=test \
> +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \
> +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \
> +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
> +/usr/bin/qemu-system-i386 \
> +-name guest=QEMUGuest1,debug-threads=on \
> +-S \
> +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \
> +-machine pc,accel=kvm,usb=off,dump-guest-core=off \
> +-cpu qemu64 \
> +-m size=2095104k,slots=16,maxmem=1099511627776k \
> +-overcommit mem-lock=off \
> +-smp 2,sockets=2,dies=1,cores=1,threads=1 \
> +-object '{"qom-type":"memory-backend-ram","id":"ram-node0","size":2145386496}' \
> +-numa node,nodeid=0,cpus=0-1,memdev=ram-node0 \
> +-object '{"qom-type":"memory-backend-ram","id":"memvirtiomem0","reserve":false,"size":1073741824}' \
> +-device virtio-mem-pci,node=0,block-size=2097152,requested-size=536870912,memdev=memvirtiomem0,id=virtiomem0,bus=pci.0,addr=0x2 \
> +-object '{"qom-type":"memory-backend-file","id":"memvirtiomem1","mem-path":"/dev/hugepages2M/libvirt/qemu/-1-QEMUGuest1","reserve":false,"size":2147483648,"host-nodes":[1,2,3],"policy":"bind"}' \
> +-device virtio-mem-pci,node=0,block-size=2097152,requested-size=1073741824,memdev=memvirtiomem1,id=virtiomem1,bus=pci.0,addr=0x3 \
> +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
> +-display none \
> +-no-user-config \
> +-nodefaults \
> +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \
> +-mon chardev=charmonitor,id=monitor,mode=control \
> +-rtc base=utc \
> +-no-shutdown \
> +-no-acpi \
> +-boot strict=on \
> +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \
> +-blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \
> +-blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw","file":"libvirt-1-storage"}' \
> +-device ide-hd,bus=ide.0,unit=0,drive=libvirt-1-format,id=ide0-0-0,bootindex=1 \
> +-audiodev id=audio1,driver=none \
> +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 \
> +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
> +-msg timestamp=on
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index 74af93b08f..23f4a8471f 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -3143,6 +3143,7 @@ mymain(void)
> ARG_END);
> DO_TEST_CAPS_VER("memory-hotplug-virtio-pmem", "5.2.0");
> DO_TEST_CAPS_LATEST("memory-hotplug-virtio-pmem");
> + DO_TEST_CAPS_LATEST("memory-hotplug-virtio-mem");
>
> DO_TEST("machine-aeskeywrap-on-caps",
> QEMU_CAPS_AES_KEY_WRAP,
>

--
Thanks,

David / dhildenb