Re: [Qemu-devel] [PATCH 1/3] qapi: Allow QMP/QAPI commands to have array inputs

[Luiz Capitulino]

On Wed, 22 Feb 2012 14:31:17 -0600
Anthony Liguori <address@hidden> wrote:

> On 02/22/2012 02:25 PM, Luiz Capitulino wrote:
> > On Wed, 22 Feb 2012 11:35:26 -0600
> > Anthony Liguori<address@hidden>  wrote:
> >
> >> On 02/22/2012 10:12 AM, Jeff Cody wrote:
> >>> On 02/22/2012 09:53 AM, Anthony Liguori wrote:
> >>>> On 02/20/2012 11:31 AM, Jeff Cody wrote:
> >>>>> The QAPI scripts allow for generating commands that receive parameter
> >>>>> input consisting of a list of custom structs, but the QMP input paramter
> >>>>> checking did not support receiving a qlist as opposed to a qdict for
> >>>>> input.
> >>>>
> >>>> What are you trying to send on the wire? Something like
> >>>>
> >>>> {"execute": "foo", "arguments": [ "a", "b", "c" ]}
> >>>>
> >>>> ?
> >>>>
> >>>> That's not valid per the QMP protocol. "arguments" must always be a QMP
> >>>> dictionary.
> >>>>
> >>>
> >>> Not a bare list like that (perhaps my wording was poor), but rather an 
> >>> array of
> >>> custom structs, that contain QMP dicts. In this particular case:
> >>>
> >>> {"execute": "blockdev-group-snapshot-sync", "arguments": [{ "device":
> >>> "ide-hd0","snapshot-file":"/some/place/my-image","format": "qcow2" }]}
> >>>
> >>> When specifying this in the schema JSON file as shown below[1], the QAPI 
> >>> scripts
> >>> generate the marshaller&  visitor functions that handle everything 
> >>> correctly.
> >>> The monitor code, however, could not deal with the QList container for 
> >>> the input
> >>> parameters, prior to passing the data to the generated functions.
> >>>
> >>>
> >>> [1] JSON schema definition:
> >>>
> >>> { 'type': 'SnapshotDev',
> >>> 'data': {'device': 'str', 'snapshot-file': 'str', '*format': 'str' } }
> >>>
> >>> { 'command': 'blockdev-group-snapshot-sync',
> >>> 'data': { 'dev': [ 'SnapshotDev' ] } }
> >>
> >>
> >> This will end up looking like:
> >>
> >> { "execute": "blockdev-group-snapshot-sync",
> >>     "arguments": { "dev": [{"device": "ide-hd0"....}] } }
> >>
> >> Which should work as expected in QMP.  HMP argument validation doesn't 
> >> handle
> >> arguments of lists but IMHO, it's time to kill that code.
> >>
> >> Luiz, what do you think?
> >
> > I don't think the code can just be dropped (if that's what you meant), as 
> > there
> > are some features on top of it, like auto-completion, suffix support (G, M, 
> > k, etc),
> > but it certainly can be improved.
> >
> > Also note that QMP does argument validation too, which is unfortunate 
> > enough to
> > be based on HMP's,
> 
> Once everything is converted to middle mode, this validation can be dropped 
> for 
> QMP.  The generated code should do thorough checking of arguments and can 
> handle 
> complex types.

That's cool, although we'll be switching to the new server too (hence the whole
current server will be dropped).