Re: [Qemu-devel] [PATCH 2/3] pcdimm: add 'type' field to PCDIMMDeviceInfo

[Markus Armbruster <armbru@redhat.com>]

Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:

> The field is needed to distinguish pc-dimm and nvdimm.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> CC: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> CC: "Michael S. Tsirkin" <mst@redhat.com>
> CC: Igor Mammedov <imammedo@redhat.com>
> CC: Eric Blake <eblake@redhat.com>
> CC: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/mem/pc-dimm.c | 1 +
>  qapi-schema.json | 5 ++++-
>  2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> index 4f30950..7469bd4 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -178,6 +178,7 @@ int qmp_pc_dimm_device_list(Object *obj, void *opaque)
       if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
           DeviceState *dev = DEVICE(obj);

           if (dev->realized) {
[...]
>              di->size = object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP,
>                                                 NULL);
>              di->memdev = object_get_canonical_path(OBJECT(dimm->hostmem));
> +            di->type = g_strdup(object_get_typename(obj));

@type is the type name of a subtype of TYPE_PC_DIMM.

Current subtypes are TYPE_PC_DIMM itself and TYPE_NVDIMM, i.e. "pc-dimm"
and "nvdimm".

>  
>              info->u.dimm = di;
>              elem->value = info;
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 8d04897..3bcc957 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3924,6 +3924,8 @@
>  #
>  # @hotpluggable: true if device if could be added/removed while machine is running
>  #
> +# @type: device type: 'pc-dimm' or 'nvdimm' (since 2.6)
> +#

Not wrong, but brittle: it breaks whenever someone adds another subtype
and doesn't remember to update this spot.  Remembering it seems
unlikely, because it's somewhere else entirely.

Let's document it to be the type name of a subtype of pc-dimm, and that
you can query the possible values with QMP command

    { "execute": "qom-list-types", "arguments": { "implements": "pc-dimm", "abstract": false } }

>  # Since: 2.1
>  ##
>  { 'struct': 'PCDIMMDeviceInfo',
> @@ -3934,7 +3936,8 @@
>              'node': 'int',
>              'memdev': 'str',
>              'hotplugged': 'bool',
> -            'hotpluggable': 'bool'
> +            'hotpluggable': 'bool',
> +            'type': 'str'
>            }
>  }