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

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Markus Armbruster (armbru@redhat.com) wrote:
> "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,

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.

Dave

> 
> 
> [*] Sorry Eric, could not resist poking you again :)
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK