| Message ID | 66e17e32ccb10ca0ae262103fcf170b84511c3f8.1402299637.git.hutao@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
On Mon, 9 Jun 2014 18:25:33 +0800 Hu Tao <hutao@cn.fujitsu.com> wrote: > Add qmp command query-memdev to query for information > of memory devices > > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> > --- > numa.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > qapi-schema.json | 34 ++++++++++++++++++++++++++ > qmp-commands.hx | 32 +++++++++++++++++++++++++ > 3 files changed, 138 insertions(+) > > diff --git a/numa.c b/numa.c > index 1a83733..4e2fdc4 100644 > --- a/numa.c > +++ b/numa.c > @@ -31,9 +31,14 @@ > #include "qapi-visit.h" > #include "qapi/opts-visitor.h" > #include "qapi/dealloc-visitor.h" > +#include "qapi/qmp-output-visitor.h" > +#include "qapi/qmp-input-visitor.h" > +#include "qapi/string-output-visitor.h" > +#include "qapi/string-input-visitor.h" > #include "qapi/qmp/qerror.h" > #include "hw/boards.h" > #include "sysemu/hostmem.h" > +#include "qmp-commands.h" > > QemuOptsList qemu_numa_opts = { > .name = "numa", > @@ -280,3 +285,70 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner, > addr += size; > } > } > + > +MemdevList *qmp_query_memdev(Error **errp) > +{ > + MemdevList *list = NULL, *m; > + HostMemoryBackend *backend; > + Error *err = NULL; > + int i; > + > + for (i = 0; i < nb_numa_nodes; i++) { > + backend = numa_info[i].node_memdev; > + > + m = g_malloc0(sizeof(*m)); > + m->value = g_malloc0(sizeof(*m->value)); > + m->value->size = object_property_get_int(OBJECT(backend), "size", > + &err); > + if (err) { > + goto error; > + } > + > + m->value->merge = object_property_get_bool(OBJECT(backend), "merge", > + &err); > + if (err) { > + goto error; > + } > + > + m->value->dump = object_property_get_bool(OBJECT(backend), "dump", > + &err); > + if (err) { > + goto error; > + } > + > + m->value->prealloc = object_property_get_bool(OBJECT(backend), > + "prealloc", &err); > + if (err) { > + goto error; > + } > + > + m->value->policy = object_property_get_enum(OBJECT(backend), > + "policy", > + HostMemPolicy_lookup, > + &err); > + if (err) { > + goto error; > + } > + > + object_property_get_uint16List(OBJECT(backend), "host-nodes", > + &m->value->host_nodes, &err); > + if (err) { > + goto error; > + } > + > + m->next = list; > + list = m; > + } > + > + return list; > + > +error: > + while (list) { > + m = list; > + list = list->next; > + g_free(m->value); > + g_free(m); > + } > + qerror_report_err(err); > + return NULL; > +} > diff --git a/qapi-schema.json b/qapi-schema.json > index 0898c00..f23c3f1 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -4779,3 +4779,37 @@ > ## > { 'enum': 'HostMemPolicy', > 'data': [ 'default', 'preferred', 'bind', 'interleave' ] } > + > +## > +# @Memdev: > +# > +# Information of memory device > +# > +# @size: memory device size > +# > +# @host-nodes: host nodes for its memory policy > +# > +# @policy: memory policy of memory device > +# > +# Since: 2.1 > +## > + > +{ 'type': 'Memdev', > + 'data': { > + 'size': 'size', > + 'merge': 'bool', > + 'dump': 'bool', > + 'prealloc': 'bool', > + 'host-nodes': ['uint16'], > + 'policy': 'HostMemPolicy' }} > + > +## > +# @query-memdev: > +# > +# Returns information for all memory devices. > +# > +# Returns: a list of @Memdev. > +# > +# Since: 2.1 > +## > +{ 'command': 'query-memdev', 'returns': ['Memdev'] } Could we make it union, that returns MemdevRam + MemdevFile MemdevFile will have additional file-only specific properties. > diff --git a/qmp-commands.hx b/qmp-commands.hx > index d8aa4ed..ea8958f 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -3572,3 +3572,35 @@ 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": [ > + { > + "size": 536870912, > + "host-nodes": [0, 1], > + "policy": "bind" > + }, > + { > + "size": 536870912, > + "host-nodes": [2, 3], > + "policy": "preferred" > + } > + ] > + } > + > +EQMP > -- > 1.9.3 >
Il 09/06/2014 14:36, Igor Mammedov ha scritto: >> > +{ 'type': 'Memdev', >> > + 'data': { >> > + 'size': 'size', >> > + 'merge': 'bool', >> > + 'dump': 'bool', >> > + 'prealloc': 'bool', >> > + 'host-nodes': ['uint16'], >> > + 'policy': 'HostMemPolicy' }} >> > + >> > +## >> > +# @query-memdev: >> > +# >> > +# Returns information for all memory devices. >> > +# >> > +# Returns: a list of @Memdev. >> > +# >> > +# Since: 2.1 >> > +## >> > +{ 'command': 'query-memdev', 'returns': ['Memdev'] } > Could we make it union, that returns MemdevRam + MemdevFile > > MemdevFile will have additional file-only specific properties. > Which are the file-only properties (in the current definition of Memdev)? Paolo
On Mon, 09 Jun 2014 14:58:39 +0200 Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 09/06/2014 14:36, Igor Mammedov ha scritto: > >> > +{ 'type': 'Memdev', > >> > + 'data': { > >> > + 'size': 'size', > >> > + 'merge': 'bool', > >> > + 'dump': 'bool', > >> > + 'prealloc': 'bool', > >> > + 'host-nodes': ['uint16'], > >> > + 'policy': 'HostMemPolicy' }} > >> > + > >> > +## > >> > +# @query-memdev: > >> > +# > >> > +# Returns information for all memory devices. > >> > +# > >> > +# Returns: a list of @Memdev. > >> > +# > >> > +# Since: 2.1 > >> > +## > >> > +{ 'command': 'query-memdev', 'returns': ['Memdev'] } > > Could we make it union, that returns MemdevRam + MemdevFile > > > > MemdevFile will have additional file-only specific properties. > > > > Which are the file-only properties (in the current definition of Memdev)? in current none, but for file backend exposing 'path' property might be useful alternatively instead of union we could add 'type' and optional 'path' fields to Memdev > > Paolo >
Il 09/06/2014 15:32, Igor Mammedov ha scritto: >>>>> > >> > +{ 'command': 'query-memdev', 'returns': ['Memdev'] } >>> > > Could we make it union, that returns MemdevRam + MemdevFile >>> > > >>> > > MemdevFile will have additional file-only specific properties. >>> > > >> > >> > Which are the file-only properties (in the current definition of Memdev)? > in current none, but for file backend exposing 'path' property might be useful > alternatively instead of union we could add 'type' and optional 'path' fields > to Memdev > Yes, I agree. I think the latest additions to QAPI actually let you do that with a QAPI union while keeping backwards-compatible output for other fields. Ok to do this later? It should be acceptable for soft freeze. Paolo
On Mon, 09 Jun 2014 15:40:56 +0200 Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 09/06/2014 15:32, Igor Mammedov ha scritto: > >>>>> > >> > +{ 'command': 'query-memdev', 'returns': ['Memdev'] } > >>> > > Could we make it union, that returns MemdevRam + MemdevFile > >>> > > > >>> > > MemdevFile will have additional file-only specific properties. > >>> > > > >> > > >> > Which are the file-only properties (in the current definition of Memdev)? > > in current none, but for file backend exposing 'path' property might be useful > > alternatively instead of union we could add 'type' and optional 'path' fields > > to Memdev > > > > Yes, I agree. I think the latest additions to QAPI actually let you do > that with a QAPI union while keeping backwards-compatible output for > other fields. Ok to do this later? It should be acceptable for soft > freeze. sure. Actually, all my comments could be addressed as follow up patches before freeze, there is no point in respining huge series for more or less cosmetic changes. > > Paolo
On 06/09/2014 07:40 AM, Paolo Bonzini wrote: > Il 09/06/2014 15:32, Igor Mammedov ha scritto: >>>>>> > >> > +{ 'command': 'query-memdev', 'returns': ['Memdev'] } >>>> > > Could we make it union, that returns MemdevRam + MemdevFile >>>> > > >>>> > > MemdevFile will have additional file-only specific properties. >>>> > > >>> > >>> > Which are the file-only properties (in the current definition of >>> Memdev)? >> in current none, but for file backend exposing 'path' property might >> be useful >> alternatively instead of union we could add 'type' and optional 'path' >> fields >> to Memdev >> > > Yes, I agree. I think the latest additions to QAPI actually let you do > that with a QAPI union while keeping backwards-compatible output for > other fields. Ok to do this later? It should be acceptable for soft > freeze. Correct, use of a discriminated union can add a new 'type' parameter, which in turn controls what other parameters are also present as a group, all within the same dictionary passed over the wire, so it is a back-compat friendly change to convert from a single struct to a QAPI union, and can be deferred to the point where you need such a change.
On 06/09/2014 04:25 AM, Hu Tao wrote: > Add qmp command query-memdev to query for information > of memory devices > > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> > --- > numa.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > qapi-schema.json | 34 ++++++++++++++++++++++++++ > qmp-commands.hx | 32 +++++++++++++++++++++++++ > 3 files changed, 138 insertions(+) > > + > +## > +# @Memdev: > +# > +# Information of memory device > +# > +# @size: memory device size > +# > +# @host-nodes: host nodes for its memory policy > +# > +# @policy: memory policy of memory device > +# You documented three parameters... > +# Since: 2.1 > +## > + > +{ 'type': 'Memdev', > + 'data': { > + 'size': 'size', > + 'merge': 'bool', > + 'dump': 'bool', > + 'prealloc': 'bool', > + 'host-nodes': ['uint16'], > + 'policy': 'HostMemPolicy' }} ...but implemented six, all listed as mandatory,... > +Show memory devices information. > + > + > +Example (1): > + > +-> { "execute": "query-memdev" } > +<- { "return": [ > + { > + "size": 536870912, > + "host-nodes": [0, 1], > + "policy": "bind" > + }, ...and then only demonstrate 3 in the example. Something's not quite right.
On Mon, Jun 09, 2014 at 11:24:51AM -0600, Eric Blake wrote: > On 06/09/2014 04:25 AM, Hu Tao wrote: > > Add qmp command query-memdev to query for information > > of memory devices > > > > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> > > --- > > numa.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > qapi-schema.json | 34 ++++++++++++++++++++++++++ > > qmp-commands.hx | 32 +++++++++++++++++++++++++ > > 3 files changed, 138 insertions(+) > > > > > + > > +## > > +# @Memdev: > > +# > > +# Information of memory device > > +# > > +# @size: memory device size > > +# > > +# @host-nodes: host nodes for its memory policy > > +# > > +# @policy: memory policy of memory device > > +# > > You documented three parameters... > > > +# Since: 2.1 > > +## > > + > > +{ 'type': 'Memdev', > > + 'data': { > > + 'size': 'size', > > + 'merge': 'bool', > > + 'dump': 'bool', > > + 'prealloc': 'bool', > > + 'host-nodes': ['uint16'], > > + 'policy': 'HostMemPolicy' }} > > ...but implemented six, all listed as mandatory,... > > > +Show memory devices information. > > + > > + > > +Example (1): > > + > > +-> { "execute": "query-memdev" } > > +<- { "return": [ > > + { > > + "size": 536870912, > > + "host-nodes": [0, 1], > > + "policy": "bind" > > + }, > > ...and then only demonstrate 3 in the example. Something's not quite right. Thanks for catching this! Hu
diff --git a/numa.c b/numa.c index 1a83733..4e2fdc4 100644 --- a/numa.c +++ b/numa.c @@ -31,9 +31,14 @@ #include "qapi-visit.h" #include "qapi/opts-visitor.h" #include "qapi/dealloc-visitor.h" +#include "qapi/qmp-output-visitor.h" +#include "qapi/qmp-input-visitor.h" +#include "qapi/string-output-visitor.h" +#include "qapi/string-input-visitor.h" #include "qapi/qmp/qerror.h" #include "hw/boards.h" #include "sysemu/hostmem.h" +#include "qmp-commands.h" QemuOptsList qemu_numa_opts = { .name = "numa", @@ -280,3 +285,70 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner, addr += size; } } + +MemdevList *qmp_query_memdev(Error **errp) +{ + MemdevList *list = NULL, *m; + HostMemoryBackend *backend; + Error *err = NULL; + int i; + + for (i = 0; i < nb_numa_nodes; i++) { + backend = numa_info[i].node_memdev; + + m = g_malloc0(sizeof(*m)); + m->value = g_malloc0(sizeof(*m->value)); + m->value->size = object_property_get_int(OBJECT(backend), "size", + &err); + if (err) { + goto error; + } + + m->value->merge = object_property_get_bool(OBJECT(backend), "merge", + &err); + if (err) { + goto error; + } + + m->value->dump = object_property_get_bool(OBJECT(backend), "dump", + &err); + if (err) { + goto error; + } + + m->value->prealloc = object_property_get_bool(OBJECT(backend), + "prealloc", &err); + if (err) { + goto error; + } + + m->value->policy = object_property_get_enum(OBJECT(backend), + "policy", + HostMemPolicy_lookup, + &err); + if (err) { + goto error; + } + + object_property_get_uint16List(OBJECT(backend), "host-nodes", + &m->value->host_nodes, &err); + if (err) { + goto error; + } + + m->next = list; + list = m; + } + + return list; + +error: + while (list) { + m = list; + list = list->next; + g_free(m->value); + g_free(m); + } + qerror_report_err(err); + return NULL; +} diff --git a/qapi-schema.json b/qapi-schema.json index 0898c00..f23c3f1 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -4779,3 +4779,37 @@ ## { 'enum': 'HostMemPolicy', 'data': [ 'default', 'preferred', 'bind', 'interleave' ] } + +## +# @Memdev: +# +# Information of memory device +# +# @size: memory device size +# +# @host-nodes: host nodes for its memory policy +# +# @policy: memory policy of memory device +# +# Since: 2.1 +## + +{ 'type': 'Memdev', + 'data': { + 'size': 'size', + 'merge': 'bool', + 'dump': 'bool', + 'prealloc': 'bool', + 'host-nodes': ['uint16'], + 'policy': 'HostMemPolicy' }} + +## +# @query-memdev: +# +# Returns information for all memory devices. +# +# Returns: a list of @Memdev. +# +# Since: 2.1 +## +{ 'command': 'query-memdev', 'returns': ['Memdev'] } diff --git a/qmp-commands.hx b/qmp-commands.hx index d8aa4ed..ea8958f 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -3572,3 +3572,35 @@ 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": [ + { + "size": 536870912, + "host-nodes": [0, 1], + "policy": "bind" + }, + { + "size": 536870912, + "host-nodes": [2, 3], + "policy": "preferred" + } + ] + } + +EQMP
Add qmp command query-memdev to query for information of memory devices Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- numa.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ qapi-schema.json | 34 ++++++++++++++++++++++++++ qmp-commands.hx | 32 +++++++++++++++++++++++++ 3 files changed, 138 insertions(+)