[libvirt] [RFC PATCH 06/12] qemu: memdev: Add infrastructure to load memory device information

[John Ferlan]

On 01/30/2015 08:21 AM, Peter Krempa wrote:
> When using 'acpi-dimm' memory devices with qemu, some of the information
> like the slot number and base address need to be reloaded from qemu
> after process start so that it reflects the actual state. The state then
> allows to use memory devices across migrations.
> ---
>  src/qemu/qemu_domain.c       |  43 ++++++++++++++++
>  src/qemu/qemu_domain.h       |   4 ++
>  src/qemu/qemu_monitor.c      |  45 +++++++++++++++++
>  src/qemu/qemu_monitor.h      |  14 ++++++
>  src/qemu/qemu_monitor_json.c | 116 +++++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_monitor_json.h |   5 ++
>  src/qemu/qemu_process.c      |   4 ++
>  7 files changed, 231 insertions(+)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 25540c4..df912a6 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -2773,6 +2773,49 @@ qemuDomainUpdateDeviceList(virQEMUDriverPtr driver,
>      return 0;
>  }
> 
> +
> +int
> +qemuDomainUpdateMemoryDeviceInfo(virQEMUDriverPtr driver,
> +                                 virDomainObjPtr vm,
> +                                 int asyncJob)
> +{
> +    qemuDomainObjPrivatePtr priv = vm->privateData;
> +    virHashTablePtr meminfo = NULL;
> +    size_t i;
> +
> +    if (vm->def->nmems == 0)
> +        return 0;
> +
> +    if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
> +        return -1;
> +    if (qemuMonitorGetMemoryDeviceInfo(priv->mon, &meminfo) < 0) {
> +        ignore_value(qemuDomainObjExitMonitor(driver, vm));
> +        return -1;
> +    }
> +
> +    if (qemuDomainObjExitMonitor(driver, vm) < 0)

This would leak meminfo

> +        return -1;
> +

I know we don't have a "norm" for how this sequence goes, but along with
Jan's point about -2 being checked back in the caller... The "norm"
seems to be to:

   ret = qemuMonitor*();
   if (*ExitMonitor) < 0)
       ret = -1;
       goto cleanup or error;

   if (ret < 0)
       goto cleanup;
...

 Caller would probably want to make sure the VM is still active or not.


> +    for (i = 0; i < vm->def->nmems; i++) {
> +        virDomainMemoryDefPtr mem = vm->def->mems[i];
> +        qemuMonitorMemoryDeviceInfoPtr dimm;
> +
> +        if (!mem->info.alias)
> +            continue;
> +
> +        if (!(dimm = virHashLookup(meminfo, mem->info.alias)))
> +            continue;
> +
> +        mem->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ACPI_DIMM;
> +        mem->info.addr.acpiDimm.slot = dimm->slot;
> +        mem->info.addr.acpiDimm.base = dimm->address;
> +    }
> +

 cleanup:

> +    virHashFree(meminfo);
> +    return ret;
> +}
> +
> +
>  bool
>  qemuDomainDefCheckABIStability(virQEMUDriverPtr driver,
>                                 virDomainDefPtr src,

<...snip...>

> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index b0f7b1c..ba7c8e2 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -4871,6 +4871,10 @@ int qemuProcessStart(virConnectPtr conn,
>      if (qemuDomainUpdateDeviceList(driver, vm, asyncJob) < 0)
>          goto cleanup;
> 
> +    VIR_DEBUG("Updating info of memory devices");
> +    if (qemuDomainUpdateMemoryDeviceInfo(driver, vm, asyncJob) < 0)
> +        goto cleanup;
> +

Haven't looked forward yet, but is this something that needs to be done
at qemuProcessReconnec ?

John

>      /* Technically, qemuProcessStart can be called from inside
>       * QEMU_ASYNC_JOB_MIGRATION_IN, but we are okay treating this like
>       * a sync job since no other job can call into the domain until
>