"Dr. David Alan Gilbert" <dgilb...@redhat.com> writes:
> * Markus Armbruster (arm...@redhat.com) wrote:
>> "Dr. David Alan Gilbert" <dgilb...@redhat.com> writes:
>>
>> > * Eric Blake (ebl...@redhat.com) wrote:
>> >> On 01/29/2015 09:28 AM, Dr. David Alan Gilbert wrote:
>> >> > * Eric Blake (ebl...@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.
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?
