Am Donnerstag, den 08.03.2018, 14:21 -0600 schrieb Eric Blake:
> On 03/08/2018 02:07 PM, Benjamin Drung wrote:
> > Am Donnerstag, den 08.03.2018, 13:46 -0600 schrieb Eric Blake:
> > > On 03/08/2018 12:57 PM, Benjamin Drung wrote:
> > > > '*dnssearch': ['String'],
> > > > '*domainname': 'str',
> > > > + '*route': ['String'],
> > >
> > > I know we've used ['String'] for previous members, but that's
> > > rather
> > > heavyweight - it transmits over QMP as:
> > >
> > > "dnssearch": [ { "str": "foo" }, { "str": "bar" } ]
> > >
> > > Nicer is ['str'], which transmits as:
> > >
> > > "route": [ "foo", "bar" ]
> > >
> > > so the question boils down to whether cross-member consistency is
> > > more
> > > important than making your additions concise.
> >
> > Agreed that ['str'] is nicer. I will update the patch.
>
> The problem is that ['str'] might not work easily for the command
> line
> glue; I'm more familiar with how QMP exposes things than with the
> command line parsing, and Markus, who is trying to improve command
> line
> parsing to share more common infrastructure with QMP, might have
> better
> comments on the topic, except that he's on leave for a few weeks and
> won't respond until after 2.12 is frozen. Using ['String'] for
> consistency is therefore okay, if you can't get ['str'] working
> quickly.
['str'] works and was easily changeable.
> > > > @@ -1904,7 +1904,7 @@ DEF("netdev", HAS_ARG,
> > > > QEMU_OPTION_netdev,
> > > > " [,ipv6[=on|off]][,ipv6-net=addr[/int]][,ipv6-
> > > > host=addr]\n"
>
> Here's an example where we made the command line smart. ipv6-net
> takes
> TWO pieces of information: addr/int; on the QMP side, we spelled it
> '*ipv6-prefix':'str' + 'ipv6-prefixlen':'int'. So somewhere in the
> command line parsing code for --net (which I'm less familiar with),
> there is some glue code taking the compact representation and
> splitting
> it into the more verbose but more direct QMP representation - well,
> that
> is, if we are converting it into QMP form at all (part of the problem
> is
> that our command line and runtime control don't always share code,
> although we're trying to get better at that).
The split is done net_client_init() before iterating over the options.
This is equivalent to having following command line parameter:
[,ipv6-prefix=addr][,ipv6-prefixlen=int]
instead of
[,ipv6-net=addr[/int]]
> > > > + " [,route=addr/mask[:gateway]][,tftp=dir][,bootfil
> > > > e=f]
> > > > [,hostfwd=rule][,guestfwd=rule]"
> > >
> > > Urgh - your QMP interface HAS to be further parsed to get to the
> > > useful
> > > information. While it's nice to have compact syntax on the
> > > command
> > > line, it is really worth thinking about making information easier
> > > to
> > > consume (that is, NO further parsing required once the
> > > information is
> > > in
> > > JSON format). Would it be any better to send things over the
> > > wire
> > > as:
> > >
> > > "route": [ { "addr": "...", "mask": 24, "gateway": "..." } ]
> >
> > That's looks good.
>
> Okay, doing that would mean using something like:
>
> { 'struct': 'RouteEntry', 'data': { 'addr': 'str', '*mask': 'int',
> '*gateway': 'str' } }
> ...
> 'route': [ 'RouteEntry' ]
>
> (but reuse, rather than inventing a new type, if one of the existing
> QMP
> types already resembles what I proposed for RouteEntry)
>
> The command line can still use route=addr/mask:gateway syntax, parse
> it
> down into components, then compile the QMP array of already-parsed
> structs (rather than making QMP take a direct ['String'] that still
> needs further parsing). It may take more glue code, but the idea is
> that all the glue code should live on the front end, so that the QMP
> backend should be easy to work with.
The problem is that the opts visitor code only supports lists with a
single mandatory scalar member. Bigger changes are needed to support
splitting 'route'. I have looked into the code for several hours, but
found no simple and non-ugly way to add the splitting without major
changes to the code. I am willing to adapt the patch if someone changes
the opts visitor to support lists with multiple members.
--
Benjamin Drung
System Developer
Debian & Ubuntu Developer
ProfitBricks GmbH
Greifswalder Str. 207
D - 10405 Berlin
Email: benjamin.dr...@profitbricks.com
URL: https://www.profitbricks.de
Sitz der Gesellschaft: Berlin
Registergericht: Amtsgericht Charlottenburg, HRB 125506 B
Geschäftsführer: Achim Weiss, Matthias Steinberg
