Re: [Qemu-devel] [RFC 1/1] Execute arbitrary QMP commands from command line

[Markus Armbruster <armbru@redhat.com>]

"Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:

> * Eric Blake (eblake@redhat.com) wrote:
>> On 01/29/2015 09:28 AM, Dr. David Alan Gilbert wrote:
>> > * Eric Blake (eblake@redhat.com) wrote:
>> > > On 01/29/2015 08:54 AM, Dr. David Alan Gilbert wrote:
>> > > >> The idea of a QMP command to trigger incoming migration looks
>> > > >> reasonable.  We can probably use a qapi union for a nicer syntax,
>> > > >> something like:
>> > > >>
>> > > >> {"execute": "migrate-incoming", "arguments": {
>> > > >>   "type": "tcp", "port": 44 } }
>> > > >> vs.
>> > > >> {"execute": "migrate-incoming", "arguments": {
>> > > >>   "type": "fd", "fd": 0 } }
>> > > >> vs.
>> > > >> {"execute": "migrate-incoming", "arguments": {
>> > > >>   "type": "exec", "command": [ "cat", "/path/to/file" ] } }
>> > > >>
>> > > >> and so forth.
>> > > >
>> > > > Compared to just taking a URI argument that Dan suggested, that's quite a
>> > > > bit of rework to do the reworking of each transport which is pretty
>> > > > trivial.
>> > >
>> > > Yes, but getting the interface right means that adding future extensions
>> > > will be easier, with less string parsing hacks.

We have a general rule for QMP: no syntax embedded in string arguments,
use JSON.

>> > I guess so, but I still have to maintain the -incoming string interface
>> > and an HMP equivalent of whatever we come up with here.

The HMP equivalent may or may not be needed.  If we decide we want it,
reusing the command line's parser there probably makes more sense than
inventing yet another syntax.

>> > So what would the .args_type look like in qmp-commands.hx;
>> > something like this?
>> >
>> >   .args-type = "type:s,port:-i,host:-s,command:-s"
>>
>> No, it would be more like the blockdev-add interface, where one command
>> accepts a dictionary object containing a union of valid values, where
>> the set of valid values is determined by the discriminator field.
>> .args_type = "options:q".

Note that blockdev-add has wraps its arguments rather inelegantly: it
takes a single argument 'options' of union type 'BlockdevOptions'.
Because of that, you have to write

    "arguments": { "options" : { ... } }

instead of just

    "arguments": { ... }

I'd love to get that cleaned up, but Kevin is already worrying about
backwards compatibility.  He has a point in theory, because we neglected
to mark blockdev-add as unstable.  I have a point in practice, because
blockdev-add hasn't been usable for real work (as some of our poor users
discovered the hard way) due to numerous restrictions we're still busy
lifting.  Anyway, I digressed, back to the topic at hand.

> What causes the parser to generate a 'BlockdevOptions' as opposed to any
> standard options type for the parameter of qmp_blockdev_add?

qmp-commands.hx is a relict.  It's still around because we still haven't
completed the conversion of QMP to QAPI.  We suck :)

The QAPI schema defines QMP commands.  The marshalling / unmarshalling
code mapping between QMP the text protocol and actual QMP command
handlers written in C gets generated from the schema.

docs/qapi-code-gen.txt explains this.  A much improved version is
sitting in Eric's queue[*].  Perhaps Eric can provide a pointer to his
current version.

qmp-commands.hx duplicates some of the schema information, partly in
dumbed down form, and adds a bit more.


[*] Sorry Eric, could not resist poking you again :)