Tested the patch with libvirt v7.2.0-381-g3c3c55be66 & qemu-kvm-5.2.0-0.7.rc2.fc34.x86_64 S1: Start domain with virtio-mem device # virsh start pc_test # virsh dumpxml pc_test <domain type='kvm' id='11'> <name>pc_test</name> <uuid>927da985-2937-4dfe-ac13-be723293e0d9</uuid> <maxMemory slots='16' unit='KiB'>6291456</maxMemory> <memory unit='KiB'>1179648</memory> <currentMemory unit='KiB'>1179648</currentMemory> ...
<memory model='virtio-mem'> <source> <nodemask>0</nodemask> <pagesize unit='KiB'>2048</pagesize> </source> <target> <size unit='KiB'>131072</size> <node>0</node> <block unit='KiB'>2048</block> <requested unit='KiB'>131072</requested> <actual unit='KiB'>131072</actual> </target> <alias name='virtiomem0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/> </memory> .. 2. Hot plug a virtio-mem device, the values of memory & currentMemory are updated accordingly. # cat mem.xml <memory model='virtio-mem'> <source> <nodemask>0</nodemask> <pagesize unit='KiB'>2048</pagesize> </source> <target> <size unit='KiB'>131072</size> <node>0</node> <block unit='KiB'>2048</block> <requested unit='KiB'>131072</requested> <actual unit='KiB'>131072</actual> </target> </memory> #virsh attach-device pc_test mem.xml Device attached successfully # virsh dumpxml pc_test <memory unit='KiB'>1310720</memory> <currentMemory unit='KiB'>1310720</currentMemory> ... <memory model='virtio-mem'> <source> <nodemask>0</nodemask> <pagesize unit='KiB'>2048</pagesize> </source> <target> <size unit='KiB'>131072</size> <node>0</node> <block unit='KiB'>2048</block> <requested unit='KiB'>131072</requested> <actual unit='KiB'>131072</actual> </target> <alias name='virtiomem0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/> </memory> <memory model='virtio-mem'> <source> <nodemask>0</nodemask> <pagesize unit='KiB'>2048</pagesize> </source> <target> <size unit='KiB'>131072</size> <node>0</node> <block unit='KiB'>2048</block> <requested unit='KiB'>131072</requested> <actual unit='KiB'>131072</actual> </target> <alias name='virtiomem1'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x0b' function='0x0'/> </memory> 3. Updated the requested size, it's changed as expected. # virsh update-memory-device pc_test --alias virtiomem1 --requested-size 100MiB # virsh dumpxml pc_test <memory unit='KiB'>1310720</memory> <currentMemory unit='KiB'>1282048</currentMemory> ... <memory model='virtio-mem'> <source> <nodemask>0</nodemask> <pagesize unit='KiB'>2048</pagesize> </source> <target> <size unit='KiB'>131072</size> <node>0</node> <block unit='KiB'>2048</block> <requested unit='KiB'>102400</requested> <actual unit='KiB'>102400</actual> </target> <alias name='virtiomem1'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x0b' function='0x0'/> </memory> S2: From the docs, "virtio-pmem" works also. # cat pmem.xml <memory model='virtio-pmem' access='shared'> <source> <path>/tmp/nvdimm</path> </source> <target> <size unit='KiB'>131072</size> <label> <size unit='KiB'>128</size> </label> </target> </memory> # virsh attach-device pc_test pmem.xml Device attached successfully #virsh dumpxml pc_test <memory unit='KiB'>1441792</memory> <currentMemory unit='KiB'>1282048</currentMemory> Question: But seems it don't support <node> element , and if add <node> element, below errors are prompted - error: Failed to attach device from pmem error: unsupported configuration: virtio-pmem does not support NUMA nodes Since the <node> element is supported by nvdimm & virtio-mem, can you help to confirm if it's as expected? @Michal Privoznik <mpriv...@redhat.com> Thanks, Jing Qi On Fri, Apr 23, 2021 at 9:26 PM Michal Privoznik <mpriv...@redhat.com> wrote: > If the QEMU driver restarts it loses the track of the actual size > of virtio-mem (because it's runtime type of information and thus > not stored in XML) and therefore, we have to refresh it when > reconnecting to the domain monitor. > > Signed-off-by: Michal Privoznik <mpriv...@redhat.com> > --- > src/qemu/qemu_domain.c | 37 +++++++++++++++++++---- > src/qemu/qemu_monitor.h | 3 ++ > src/qemu/qemu_monitor_json.c | 58 ++++++++++++++++++++++-------------- > src/qemu/qemu_process.c | 3 ++ > 4 files changed, 73 insertions(+), 28 deletions(-) > > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index 226e1d9b79..3c17d8704a 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -8278,9 +8278,21 @@ qemuDomainUpdateMemoryDeviceInfo(virQEMUDriver > *driver, > return -1; > } > > - /* if qemu doesn't support the info request, just carry on */ > - if (rc == -2) > + /* If qemu doesn't support the info request, just carry on, unless we > + * really need it. */ > + if (rc == -2) { > + for (i = 0; i < vm->def->nmems; i++) { > + virDomainMemoryDef *mem = vm->def->mems[i]; > + > + if (mem->model == VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("qemu did not return info on vitio-mem > device")); > + return -1; > + } > + } > + > return 0; > + } > > if (rc < 0) > return -1; > @@ -8295,9 +8307,24 @@ qemuDomainUpdateMemoryDeviceInfo(virQEMUDriver > *driver, > if (!(dimm = virHashLookup(meminfo, mem->info.alias))) > continue; > > - mem->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM; > - mem->info.addr.dimm.slot = dimm->slot; > - mem->info.addr.dimm.base = dimm->address; > + switch (mem->model) { > + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM: > + mem->actualsize = VIR_DIV_UP(dimm->size, 1024); > + break; > + > + case VIR_DOMAIN_MEMORY_MODEL_DIMM: > + case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: > + mem->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM; > + mem->info.addr.dimm.slot = dimm->slot; > + mem->info.addr.dimm.base = dimm->address; > + break; > + > + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: > + case VIR_DOMAIN_MEMORY_MODEL_NONE: > + case VIR_DOMAIN_MEMORY_MODEL_LAST: > + /* nada */ > + break; > + } > } > > virHashFree(meminfo); > diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h > index 078310de61..a76a5c8799 100644 > --- a/src/qemu/qemu_monitor.h > +++ b/src/qemu/qemu_monitor.h > @@ -1358,10 +1358,13 @@ int qemuMonitorSetIOThread(qemuMonitor *mon, > > typedef struct _qemuMonitorMemoryDeviceInfo qemuMonitorMemoryDeviceInfo; > struct _qemuMonitorMemoryDeviceInfo { > + /* For pc-dimm */ > unsigned long long address; > unsigned int slot; > bool hotplugged; > bool hotpluggable; > + /* For virtio-mem */ > + unsigned long long size; /* in bytes */ > }; > > int qemuMonitorGetMemoryDeviceInfo(qemuMonitor *mon, > diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c > index 071c6a19be..b0a65a8617 100644 > --- a/src/qemu/qemu_monitor_json.c > +++ b/src/qemu/qemu_monitor_json.c > @@ -8220,7 +8220,6 @@ qemuMonitorJSONGetMemoryDeviceInfo(qemuMonitor *mon, > virJSONValue *cmd; > virJSONValue *reply = NULL; > virJSONValue *data = NULL; > - qemuMonitorMemoryDeviceInfo *meminfo = NULL; > size_t i; > > if (!(cmd = qemuMonitorJSONMakeCommand("query-memory-devices", NULL))) > @@ -8241,6 +8240,9 @@ qemuMonitorJSONGetMemoryDeviceInfo(qemuMonitor *mon, > > for (i = 0; i < virJSONValueArraySize(data); i++) { > virJSONValue *elem = virJSONValueArrayGet(data, i); > + g_autofree qemuMonitorMemoryDeviceInfo *meminfo = NULL; > + virJSONValue *dimminfo; > + const char *devalias; > const char *type; > > if (!(type = virJSONValueObjectGetString(elem, "type"))) { > @@ -8250,26 +8252,26 @@ qemuMonitorJSONGetMemoryDeviceInfo(qemuMonitor > *mon, > goto cleanup; > } > > + if (!(dimminfo = virJSONValueObjectGetObject(elem, "data"))) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("query-memory-devices reply data doesn't " > + "contain enum data")); > + goto cleanup; > + } > + > + /* While 'id' attribute is marked as optional in QEMU's QAPI > + * specification, Libvirt always sets it. Thus we can fail if not > + * present. */ > + if (!(devalias = virJSONValueObjectGetString(dimminfo, "id"))) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("dimm memory info data is missing 'id'")); > + goto cleanup; > + } > + > + meminfo = g_new0(qemuMonitorMemoryDeviceInfo, 1); > + > /* dimm memory devices */ > if (STREQ(type, "dimm")) { > - virJSONValue *dimminfo; > - const char *devalias; > - > - if (!(dimminfo = virJSONValueObjectGetObject(elem, "data"))) { > - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > - _("query-memory-devices reply data doesn't > " > - "contain enum data")); > - goto cleanup; > - } > - > - if (!(devalias = virJSONValueObjectGetString(dimminfo, > "id"))) { > - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > - _("dimm memory info data is missing > 'id'")); > - goto cleanup; > - } > - > - meminfo = g_new0(qemuMonitorMemoryDeviceInfo, 1); > - > if (virJSONValueObjectGetNumberUlong(dimminfo, "addr", > &meminfo->address) < 0) { > virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > @@ -8300,17 +8302,27 @@ qemuMonitorJSONGetMemoryDeviceInfo(qemuMonitor > *mon, > > } > > - if (virHashAddEntry(info, devalias, meminfo) < 0) > + } else if (STREQ(type, "virtio-mem")) { > + if (virJSONValueObjectGetNumberUlong(dimminfo, "size", > + &meminfo->size) < 0) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("malformed/missing slot in dimm memory > info")); > goto cleanup; > - > - meminfo = NULL; > + } > + } else { > + /* type not handled yet */ > + continue; > } > + > + if (virHashAddEntry(info, devalias, meminfo) < 0) > + goto cleanup; > + > + meminfo = NULL; > } > > ret = 0; > > cleanup: > - VIR_FREE(meminfo); > virJSONValueFree(cmd); > virJSONValueFree(reply); > return ret; > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c > index 47fa4068b0..55f081cc9d 100644 > --- a/src/qemu/qemu_process.c > +++ b/src/qemu/qemu_process.c > @@ -8561,6 +8561,9 @@ qemuProcessReconnect(void *opaque) > > qemuDomainVcpuPersistOrder(obj->def); > > + if (qemuDomainUpdateMemoryDeviceInfo(driver, obj, > QEMU_ASYNC_JOB_NONE) < 0) > + goto error; > + > if (qemuProcessDetectIOThreadPIDs(driver, obj, QEMU_ASYNC_JOB_NONE) < > 0) > goto error; > > -- > 2.26.3 > > -- Thanks & Regards, Jing,Qi