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

[Markus Armbruster]

"Dr. David Alan Gilbert" <address@hidden> writes:

> * Markus Armbruster (address@hidden) wrote:
>> "Dr. David Alan Gilbert" <address@hidden> writes:
>> 
>> > * Eric Blake (address@hidden) wrote:
>> >> On 01/29/2015 09:28 AM, Dr. David Alan Gilbert wrote:
>> >> > * Eric Blake (address@hidden) 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,
>
> I treat HMP as important as QMP, I don't break it or lose functionality on 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.
>
> OK, to summarise how I'm hearing things so far:
>   a) We want this as a QMP command
>   b) With nice structured json arguments
>   c) But the QMP parser is broken and the example that Eric wants me
>      to follow isn't pretty.
>
> Am I missing something from that? Because at the moment I seem to
> be walking into a minefield of QMP parsers to add a simple bit
> of migration functionality.

There is just one QMP parser.  I wouldn't call it "broken".  It parses
fine.  It's use by blockdev-add is inelegant.

Would you like me to prototype a "migrate-incoming" command for you that
does nothing?