Re: [PATCH v3 09/14] qemu: Refresh the actual size of virtio-mem on monitor reconnect

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 

Thanks,
Jing Qi

On Fri, Apr 23, 2021 at 9:26 PM Michal Privoznik <mprivozn@xxxxxxxxxx> 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@xxxxxxxxxx>
---
 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 = ""> -    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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux