Re: [Qemu-devel] [RFC 1/5] qmp: add query-memory-devices command

[Benoît Canet]

The Wednesday 28 May 2014 à 09:30:11 (-0600), Eric Blake wrote :
> On 05/28/2014 08:57 AM, Igor Mammedov wrote:
> > ... allowing to get state of present memory devices.
> > Currently implemented only for DimmDevice.
> > 
> > Signed-off-by: Igor Mammedov <address@hidden>
> > ---
> >  hw/mem/dimm.c                |   36 ++++++++++++++++++++++++++++
> >  include/hw/mem/dimm.h        |    2 +
> >  qapi-schema.json             |   53 
> > ++++++++++++++++++++++++++++++++++++++++++
> >  qmp-commands.hx              |   27 +++++++++++++++++++++
> >  qmp.c                        |   11 ++++++++
> >  stubs/Makefile.objs          |    1 +
> >  stubs/qmp_dimm_device_list.c |    7 +++++
> >  7 files changed, 137 insertions(+), 0 deletions(-)
> >  create mode 100644 stubs/qmp_dimm_device_list.c
> 
> Focusing on just the interface for now...
> 
> 
> > +++ b/qapi-schema.json
> > @@ -4722,3 +4722,56 @@
> >                'btn'     : 'InputBtnEvent',
> >                'rel'     : 'InputMoveEvent',
> >                'abs'     : 'InputMoveEvent' } }
> > +
> > +##
> > +# @DIMMDeviceInfo:
> > +#
> > +# DimmDevice state information
> > +#
> > +# @id: the devices's ID
> 
> s/devices's/device's/
> 
> > +#
> > +# @addr: physical address, where DimmDevice is mammped
> 
> s/mammped/mapped/ (or maybe mmapped,  but the fact that you used 'mmap'
> feels like an implementation detail, so the English word is sufficient)
> 
> > +#
> > +# @size: size of memory DimmDevice provides
> > +#
> > +# @slot: slot number at which DimmDevice is plugged in
> > +#
> > +# @node: NUMA node number where DIMM is plugged in
> > +#
> > +# @memdev: memory backend linked with DimmDevice
> > +#
> > +# @hotplugged: true if device was hotplugged
> > +#
> > +# @hoptluggable: true if device if could be added/removed while machine is 
> > running
> 
> s/hoptluggable/hotpluggable/
> 
> > +#
> > +# Since: 2.1
> > +##
> > +{ 'type': 'DIMMDeviceInfo',
> > +  'data': { 'id': 'str',
> > +            'addr': 'int',
> > +            'size': 'int',
> > +            'slot': 'int',
> > +            'node': 'int',
> > +            'memdev': 'str',
> > +            'hotplugged': 'bool',
> > +            'hoptluggable': 'bool'
> 
> s/hoptluggable/hotpluggable/
> 
> > +          }
> > +}
> > +
> > +##
> > +# @MemoryDeviceInfo:
> > +#
> > +# Union containing information about a memory devices
> 
> s/devices/device/
> 
> > +#
> > +# Since: 2.1
> > +##
> > +{ 'union': 'MemoryDeviceInfo', 'data': {'dimm': 'DIMMDeviceInfo'} }
> > +
> > +##
> > +# @query-memory-devices
> > +#
> > +# Lists available memory devices and their state
> > +#
> > +# Since: 2.1
> > +##
> 
> Seems reasonable.
> 
> > +{ 'command': 'query-memory-devices', 'returns': ['MemoryDeviceInfo'] }
> > diff --git a/qmp-commands.hx b/qmp-commands.hx
> > index d8aa4ed..45cb980 100644
> > --- a/qmp-commands.hx
> > +++ b/qmp-commands.hx
> > @@ -3572,3 +3572,30 @@ Example:
> >                     } } ] }
> >  
> >  EQMP
> > +
> > +    {
> > +        .name       = "query-memory-devices",
> > +        .args_type  = "",
> > +        .mhandler.cmd_new = qmp_marshal_input_query_memory_devices,
> > +    },
> > +
> > +SQMP
> > address@hidden
> > +--------------------
> > +
> > +Return a list of memory devices.
> > +
> > +Example:
> > +-> { 'execute': 'query-memory-devices' }
> > +<- { 'return': [ { 'data':
> 
> s/'/"/ throughout your example.  Our schema files allow ' as an
> ease-of-typing shortcut, but JSON itself (and therefore the on-the-wire
> QMP example) requires use of ".
> 
> > +                      { 'addr': 5368709120,
> > +                        'hoptluggable': True,
> > +                        'hotplugged': True,
> 
> s/True/true/ twice (True is not valid JSON)

Well this file is more QSON than JSON ;) Perhaps we should change the extension.

Best regards

Benoît

> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>