Re: [Qemu-devel] [PATCH v3 2/5] qmp: distinguish PC-DIMM and NVDIMM in MemoryDeviceInfoList

[Igor Mammedov <imammedo@redhat.com>]

On Mon,  5 Mar 2018 14:57:07 +0800
Haozhong Zhang <haozhong.zhang@intel.com> wrote:

> It may need to treat PC-DIMM and NVDIMM differently, e.g., when
> deciding the necessity of non-volatile flag bit in SRAT memory
> affinity structures.
> 
> NVDIMMDeviceInfo, which inherits from PCDIMMDeviceInfo, is added to
> union type MemoryDeviceInfo to record information of NVDIMM devices.
> The NVDIMM-specific data is currently left empty and will be filled
> when necessary in the future.
Maybe add to commit message that:

It also fixes "info memory-devices"/query-memory-devices
which currently show nvdimm devices as dimm devices since
object_dynamic_cast(obj, TYPE_PC_DIMM) happily cast nvdimm
to TYPE_PC_DIMM which it's been inherited from.

Otherwise patch looks good,
I'll ack it after rebase with is necessary for this patch.

> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> ---
>  hmp.c            | 14 +++++++++++---
>  hw/mem/pc-dimm.c | 20 ++++++++++++++++++--
>  numa.c           | 19 +++++++++++++------
>  qapi-schema.json | 18 +++++++++++++++++-
>  4 files changed, 59 insertions(+), 12 deletions(-)
> 
> diff --git a/hmp.c b/hmp.c
> index ae86bfbade..3f06407c5e 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -2413,7 +2413,18 @@ void hmp_info_memory_devices(Monitor *mon, const QDict *qdict)
>              switch (value->type) {
>              case MEMORY_DEVICE_INFO_KIND_DIMM:
>                  di = value->u.dimm.data;
> +                break;
> +
> +            case MEMORY_DEVICE_INFO_KIND_NVDIMM:
> +                di = qapi_NVDIMMDeviceInfo_base(value->u.nvdimm.data);
> +                break;
> +
> +            default:
> +                di = NULL;
> +                break;
> +            }
>  
> +            if (di) {
>                  monitor_printf(mon, "Memory device [%s]: \"%s\"\n",
>                                 MemoryDeviceInfoKind_str(value->type),
>                                 di->id ? di->id : "");
> @@ -2426,9 +2437,6 @@ void hmp_info_memory_devices(Monitor *mon, const QDict *qdict)
>                                 di->hotplugged ? "true" : "false");
>                  monitor_printf(mon, "  hotpluggable: %s\n",
>                                 di->hotpluggable ? "true" : "false");
> -                break;
> -            default:
> -                break;
>              }
>          }
>      }
> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> index 4d050fe2cd..866ecc699a 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -20,6 +20,7 @@
>  
>  #include "qemu/osdep.h"
>  #include "hw/mem/pc-dimm.h"
> +#include "hw/mem/nvdimm.h"
>  #include "qapi/error.h"
>  #include "qemu/config-file.h"
>  #include "qapi/visitor.h"
> @@ -249,10 +250,19 @@ MemoryDeviceInfoList *qmp_pc_dimm_device_list(void)
>          Object *obj = OBJECT(dimm);
>          MemoryDeviceInfoList *elem = g_new0(MemoryDeviceInfoList, 1);
>          MemoryDeviceInfo *info = g_new0(MemoryDeviceInfo, 1);
> -        PCDIMMDeviceInfo *di = g_new0(PCDIMMDeviceInfo, 1);
> +        PCDIMMDeviceInfo *di;
> +        NVDIMMDeviceInfo *ndi;
> +        bool is_nvdimm = object_dynamic_cast(obj, TYPE_NVDIMM);
>          DeviceClass *dc = DEVICE_GET_CLASS(obj);
>          DeviceState *dev = DEVICE(obj);
>  
> +        if (!is_nvdimm) {
> +            di = g_new0(PCDIMMDeviceInfo, 1);
> +        } else {
> +            ndi = g_new0(NVDIMMDeviceInfo, 1);
> +            di = qapi_NVDIMMDeviceInfo_base(ndi);
> +        }
> +
>          if (dev->id) {
>              di->has_id = true;
>              di->id = g_strdup(dev->id);
> @@ -265,7 +275,13 @@ MemoryDeviceInfoList *qmp_pc_dimm_device_list(void)
>          di->size = object_property_get_uint(obj, PC_DIMM_SIZE_PROP, NULL);
>          di->memdev = object_get_canonical_path(OBJECT(dimm->hostmem));
>  
> -        info->u.dimm.data = di;
> +        if (!is_nvdimm) {
> +            info->u.dimm.data = di;
> +            info->type = MEMORY_DEVICE_INFO_KIND_DIMM;
> +        } else {
> +            info->u.nvdimm.data = ndi;
> +            info->type = MEMORY_DEVICE_INFO_KIND_NVDIMM;
> +        }
>          elem->value = info;
>          elem->next = NULL;
>          if (prev) {
> diff --git a/numa.c b/numa.c
> index c6734ceb8c..23c4371e51 100644
> --- a/numa.c
> +++ b/numa.c
> @@ -529,18 +529,25 @@ static void numa_stat_memory_devices(NumaNodeMem node_mem[])
>  
>          if (value) {
>              switch (value->type) {
> -            case MEMORY_DEVICE_INFO_KIND_DIMM: {
> +            case MEMORY_DEVICE_INFO_KIND_DIMM:
>                  pcdimm_info = value->u.dimm.data;
> +                break;
> +
> +            case MEMORY_DEVICE_INFO_KIND_NVDIMM:
> +                pcdimm_info = qapi_NVDIMMDeviceInfo_base(value->u.nvdimm.data);
> +                break;
> +
> +            default:
> +                pcdimm_info = NULL;
> +                break;
> +            }
> +
> +            if (pcdimm_info) {
>                  node_mem[pcdimm_info->node].node_mem += pcdimm_info->size;
>                  if (pcdimm_info->hotpluggable && pcdimm_info->hotplugged) {
>                      node_mem[pcdimm_info->node].node_plugged_mem +=
>                          pcdimm_info->size;
>                  }
> -                break;
> -            }
> -
> -            default:
> -                break;
>              }
>          }
>      }
> diff --git a/qapi-schema.json b/qapi-schema.json
> index cd98a94388..1c2d281749 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2920,6 +2920,18 @@
>            }
>  }
>  
> +##
> +# @NVDIMMDeviceInfo:
> +#
> +# NVDIMMDevice state information
> +#
> +# Since: 2.12
> +##
> +{ 'struct': 'NVDIMMDeviceInfo',
> +  'base': 'PCDIMMDeviceInfo',
> +  'data': {}
> +}
> +
>  ##
>  # @MemoryDeviceInfo:
>  #
> @@ -2927,7 +2939,11 @@
>  #
>  # Since: 2.1
>  ##
> -{ 'union': 'MemoryDeviceInfo', 'data': {'dimm': 'PCDIMMDeviceInfo'} }
> +{ 'union': 'MemoryDeviceInfo',
> +  'data': { 'dimm': 'PCDIMMDeviceInfo',
> +            'nvdimm': 'NVDIMMDeviceInfo'
> +          }
> +}
>  
>  ##
>  # @query-memory-devices: