Re: [PATCH v5 05/16] qemu: Build command line for virtio-mem

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(a)redhat.com&gt; > --- > 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