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@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;
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@xxxxxxxxxx>
