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

[Peter Krempa]

On Fri, Apr 23, 2021 at 15:24:31 +0200, 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 at 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;

IMO this error doesn't justify losing a VM on reconnect.


> +            }
> +        }
> +
>          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 */

Remove the pointless coment.

> +            break;
> +        }
>      }
>  
>      virHashFree(meminfo);


[...]

> 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

[...]

> @@ -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"));

Misleading error message.


>                  goto cleanup;
> -
> -            meminfo = NULL;
> +            }
> +        } else {
> +            /* type not handled yet */
> +            continue;
>          }
> +

With the above issues resolved:

Reviewed-by: Peter Krempa <pkrempa at redhat.com>