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 <mprivozn@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
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 <mprivozn@redhat.com>
Thanks,
Jing Qi
On Fri, Apr 23, 2021 at 9:26 PM Michal Privoznik <mprivozn@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 <mprivozn@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
On 4/26/21 9:52 AM, Jing Qi wrote: > 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 > <mailto:mprivozn@redhat.com> Yes, this is expected. virtio-pmem does not support NUMA node: https://listman.redhat.com/archives/libvir-list/2020-December/msg00211.html Michal
© 2016 - 2026 Red Hat, Inc.