[PATCH 07/10] qemu: Refresh the actual size of virtio-mem on monitor reconnect

Michal Privoznik posted 10 patches 5 years ago
There is a newer version of this series
[PATCH 07/10] qemu: Refresh the actual size of virtio-mem on monitor reconnect
Posted by Michal Privoznik 5 years ago
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 | 55 +++++++++++++++++++++---------------
 src/qemu/qemu_process.c      |  3 ++
 4 files changed, 70 insertions(+), 28 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 783756b191..5c40c02180 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -7992,9 +7992,21 @@ qemuDomainUpdateMemoryDeviceInfo(virQEMUDriverPtr 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++) {
+            virDomainMemoryDefPtr 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;
@@ -8009,9 +8021,24 @@ qemuDomainUpdateMemoryDeviceInfo(virQEMUDriverPtr 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 00428c14d2..9668e287a1 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1391,10 +1391,13 @@ typedef struct _qemuMonitorMemoryDeviceInfo qemuMonitorMemoryDeviceInfo;
 typedef qemuMonitorMemoryDeviceInfo *qemuMonitorMemoryDeviceInfoPtr;
 
 struct _qemuMonitorMemoryDeviceInfo {
+    /* For pc-dimm */
     unsigned long long address;
     unsigned int slot;
     bool hotplugged;
     bool hotpluggable;
+    /* For virtio-mem */
+    unsigned long long size;
 };
 
 int qemuMonitorGetMemoryDeviceInfo(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index b1dc527e8b..1922f84a5e 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -8228,7 +8228,6 @@ qemuMonitorJSONGetMemoryDeviceInfo(qemuMonitorPtr mon,
     virJSONValuePtr cmd;
     virJSONValuePtr reply = NULL;
     virJSONValuePtr data = NULL;
-    qemuMonitorMemoryDeviceInfoPtr meminfo = NULL;
     size_t i;
 
     if (!(cmd = qemuMonitorJSONMakeCommand("query-memory-devices", NULL)))
@@ -8249,6 +8248,9 @@ qemuMonitorJSONGetMemoryDeviceInfo(qemuMonitorPtr mon,
 
     for (i = 0; i < virJSONValueArraySize(data); i++) {
         virJSONValuePtr elem = virJSONValueArrayGet(data, i);
+        g_autofree qemuMonitorMemoryDeviceInfoPtr meminfo = NULL;
+        virJSONValuePtr dimminfo;
+        const char *devalias;
         const char *type;
 
         if (!(type = virJSONValueObjectGetString(elem, "type"))) {
@@ -8258,26 +8260,23 @@ qemuMonitorJSONGetMemoryDeviceInfo(qemuMonitorPtr 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;
+        }
+
+        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")) {
-            virJSONValuePtr 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",
@@ -8308,17 +8307,27 @@ qemuMonitorJSONGetMemoryDeviceInfo(qemuMonitorPtr 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 37cc6a28e5..8d41f947af 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8494,6 +8494,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.2

Re: [PATCH 07/10] qemu: Refresh the actual size of virtio-mem on monitor reconnect
Posted by Peter Krempa 5 years ago
On Fri, Jan 22, 2021 at 13:50:29 +0100, Michal Privoznik 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 | 55 +++++++++++++++++++++---------------
>  src/qemu/qemu_process.c      |  3 ++
>  4 files changed, 70 insertions(+), 28 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 783756b191..5c40c02180 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c

[...]

> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index 00428c14d2..9668e287a1 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -1391,10 +1391,13 @@ typedef struct _qemuMonitorMemoryDeviceInfo qemuMonitorMemoryDeviceInfo;
>  typedef qemuMonitorMemoryDeviceInfo *qemuMonitorMemoryDeviceInfoPtr;
>  
>  struct _qemuMonitorMemoryDeviceInfo {
> +    /* For pc-dimm */
>      unsigned long long address;
>      unsigned int slot;
>      bool hotplugged;
>      bool hotpluggable;
> +    /* For virtio-mem */
> +    unsigned long long size;

Menion the unit here.

[...]

> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index b1dc527e8b..1922f84a5e 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -8228,7 +8228,6 @@ qemuMonitorJSONGetMemoryDeviceInfo(qemuMonitorPtr mon,
>      virJSONValuePtr cmd;
>      virJSONValuePtr reply = NULL;
>      virJSONValuePtr data = NULL;
> -    qemuMonitorMemoryDeviceInfoPtr meminfo = NULL;
>      size_t i;
>  
>      if (!(cmd = qemuMonitorJSONMakeCommand("query-memory-devices", NULL)))
> @@ -8249,6 +8248,9 @@ qemuMonitorJSONGetMemoryDeviceInfo(qemuMonitorPtr mon,
>  
>      for (i = 0; i < virJSONValueArraySize(data); i++) {
>          virJSONValuePtr elem = virJSONValueArrayGet(data, i);
> +        g_autofree qemuMonitorMemoryDeviceInfoPtr meminfo = NULL;
> +        virJSONValuePtr dimminfo;
> +        const char *devalias;
>          const char *type;
>  
>          if (!(type = virJSONValueObjectGetString(elem, "type"))) {
> @@ -8258,26 +8260,23 @@ qemuMonitorJSONGetMemoryDeviceInfo(qemuMonitorPtr 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;
> +        }
> +
> +        if (!(devalias = virJSONValueObjectGetString(dimminfo, "id"))) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("dimm memory info data is missing 'id'"));
> +            goto cleanup;
> +        }

This isn't future proof. The 'MemoryDeviceInfo' QAPI type which is the
return value of 'query-memory-devices' doesn't have any permanent
members, it's just an union of distinct sub types.

The fields you are querying are re-defined for every type, but are not
guaranteed to be present for any new type.

Thus our code must not require them to be present, only when we are
certain that we've got the correct discriminator.

> +
> +        meminfo = g_new0(qemuMonitorMemoryDeviceInfo, 1);
> +
>          /* dimm memory devices */
>          if (STREQ(type, "dimm")) {

[...]