Re: [Qemu-devel] [PATCH v18 14/14] amp: add query-memdev

[Paolo Bonzini <pbonzini@redhat.com>]

Il 19/02/2014 08:54, Hu Tao ha scritto:
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  backends/hostmem-ram.c | 71 ++++++++++++++++++++++++++++++++++++++++++++------
>  qapi-schema.json       | 31 ++++++++++++++++++++++
>  qmp-commands.hx        | 30 +++++++++++++++++++++
>  3 files changed, 124 insertions(+), 8 deletions(-)

This is in principle not necessary, because we can query everything via 
qom-get/qom-set; but I can see that it is useful.  If you want this:

(1) please put it in numa.c and code it in a way that does not use 
internal information of HostMemoryBackend; for example, you can walk 
numa_info[...].node_memdev and use object_property_get/set on the object 
to fill in the result.

This will also eliminate all duplicate code between func and the 
property visitors.

(2) please add an HMP variant "info memdev".

Paolo

> diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
> index 2da9341..9f19ab8 100644
> --- a/backends/hostmem-ram.c
> +++ b/backends/hostmem-ram.c
> @@ -15,6 +15,7 @@
>  #include "qapi-visit.h"
>  #include "qemu/config-file.h"
>  #include "qapi/opts-visitor.h"
> +#include "qmp-commands.h"
>
>  #define TYPE_MEMORY_BACKEND_RAM "memory-ram"
>  #define MEMORY_BACKEND_RAM(obj) \
> @@ -37,8 +38,66 @@ struct HostMemoryBackendRam {
>      DECLARE_BITMAP(host_nodes, MAX_NODES);
>      HostMemPolicy policy;
>      bool relative;
> +
> +    QTAILQ_ENTRY(HostMemoryBackendRam) next;
> +};
> +
> +static const char *policies[HOST_MEM_POLICY_MAX + 1] = {
> +    [HOST_MEM_POLICY_DEFAULT] = "default",
> +    [HOST_MEM_POLICY_PREFERRED] = "preferred",
> +    [HOST_MEM_POLICY_MEMBIND] = "membind",
> +    [HOST_MEM_POLICY_INTERLEAVE] = "interleave",
> +    [HOST_MEM_POLICY_MAX] = NULL,
>  };
>
> +static GSList *memdevs;
> +
> +static void func(gpointer data, gpointer user_data)
> +{
> +    HostMemoryBackendRam *backend = data;
> +    MemdevList **list = user_data;
> +    MemdevList *m;
> +    uint16List **node;
> +    unsigned long value;
> +
> +    m = g_malloc0(sizeof(*m));
> +    m->value = g_malloc0(sizeof(*m->value));
> +    m->value->policy = g_strdup(policies[backend->policy]);
> +    m->value->relative = backend->relative;
> +
> +    node = &m->value->host_nodes;
> +
> +    value = find_first_bit(backend->host_nodes, MAX_NODES);
> +    if (value < MAX_NODES) {
> +        *node = g_malloc0(sizeof(**node));
> +        (*node)->value = value;
> +        node = &(*node)->next;
> +
> +        do {
> +            value = find_next_bit(backend->host_nodes, MAX_NODES, value + 1);
> +            if (value == MAX_NODES) {
> +                break;
> +            }
> +
> +            *node = g_malloc0(sizeof(**node));
> +            (*node)->value = value;
> +            node = &(*node)->next;
> +        } while (true);
> +    }
> +
> +    m->next = *list;
> +    *list = m;
> +}
> +
> +MemdevList *qmp_query_memdev(Error **errp)
> +{
> +    MemdevList *list = NULL;
> +
> +    g_slist_foreach(memdevs, func, &list);
> +
> +    return list;
> +}
> +
>  static void
>  get_host_nodes(Object *obj, Visitor *v, void *opaque, const char *name,
>                 Error **errp)
> @@ -86,14 +145,6 @@ set_host_nodes(Object *obj, Visitor *v, void *opaque, const char *name,
>      }
>  }
>
> -static const char *policies[HOST_MEM_POLICY_MAX + 1] = {
> -    [HOST_MEM_POLICY_DEFAULT] = "default",
> -    [HOST_MEM_POLICY_PREFERRED] = "preferred",
> -    [HOST_MEM_POLICY_MEMBIND] = "membind",
> -    [HOST_MEM_POLICY_INTERLEAVE] = "interleave",
> -    [HOST_MEM_POLICY_MAX] = NULL,
> -};
> -
>  static void
>  get_policy(Object *obj, Visitor *v, void *opaque, const char *name,
>             Error **errp)
> @@ -172,6 +223,8 @@ ram_backend_memory_init(HostMemoryBackend *backend, Error **errp)
>  static void
>  ram_backend_initfn(Object *obj)
>  {
> +    HostMemoryBackendRam *ram_backend = MEMORY_BACKEND_RAM(obj);
> +
>      object_property_add(obj, "host-nodes", "int",
>                          get_host_nodes,
>                          set_host_nodes, NULL, NULL, NULL);
> @@ -180,6 +233,8 @@ ram_backend_initfn(Object *obj)
>                          set_policy, NULL, NULL, NULL);
>      object_property_add_bool(obj, "relative",
>                               get_relative, set_relative, NULL);
> +
> +    memdevs = g_slist_append(memdevs, ram_backend);
>  }
>
>  static void
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 9d6370f..7b5027d 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -4474,3 +4474,34 @@
>  ##
>  { 'enum': 'HostMemPolicy',
>    'data': [ 'default', 'preferred', 'membind', 'interleave' ] }
> +
> +##
> +# @Memdev:
> +#
> +# Information of memory device
> +#
> +# @id: memory device id
> +#
> +# @host-nodes: host nodes for its memory policy
> +#
> +# @policy: memory policy of memory device
> +#
> +# Since: 2.0
> +##
> +
> +{ 'type': 'Memdev',
> +  'data': {
> +    'host-nodes': ['uint16'],
> +    'policy': 'str',
> +    'relative': 'bool' }}
> +
> +##
> +# @query-memdev:
> +#
> +# Returns information for all memory devices.
> +#
> +# Returns: a list of @Memdev.
> +#
> +# Since: 2.0
> +##
> +{ 'command': 'query-memdev', 'returns': ['Memdev'] }
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index cce6b81..20368f7 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -3457,3 +3457,33 @@ Example:
>                     } } ] }
>
>  EQMP
> +
> +    {
> +        .name       = "query-memdev",
> +        .args_type  = "",
> +        .mhandler.cmd_new = qmp_marshal_input_query_memdev,
> +    },
> +
> +SQMP
> +query-memdev
> +------------
> +
> +Show memory devices information.
> +
> +
> +Example (1):
> +
> +-> { "execute": "query-memdev" }
> +<- { "return": [
> +       {
> +         "host-nodes": [0, 1],
> +         "policy": "bind"
> +       },
> +       {
> +         "host-nodes": [2, 3],
> +         "policy": "preferred"
> +       }
> +     ]
> +   }
> +
> +EQMP
>