Re: [Qemu-devel] [PATCH 2/2 v3] slirp: Add classless static routes support to DHCP server

[Eric Blake]

On 03/14/2018 02:08 PM, Benjamin Drung wrote:
> This patch will allow the user to specify classless static routes for
> the replies from the built-in DHCP server, for example:

This mail was sent as a 2/2, but with no In-Reply-To: header pointing to 
a 0/2 cover letter.  Am I missing something?

>    qemu --net user,route=10.0.2.0/24,route=192.168.0.0/16 [...]
>
> The QMP schema for the "route" option is ['str'], because the opts
> visitor code only supports lists with a single mandatory scalar member.
> The option "ipv6-net" takes two pieces and is split in net_client_init()
> into "ipv6-prefix" and "ipv6-prefixlen" before calling
> visit_type_Netdev()/visit_type_NetLegacy(). But that logic cannot be
> used for the "route" option, because there is no intermediate format to
> store the split parts for further processing without overhauling the
> visitor code. Once the opts visitor supports lists with multiple
> members, please update the QMP schema to:
>
> { 'struct': 'RouteEntry',
>    'data': {
>      'subnet': 'str',
>      'mask_width': 'uint8',
>      '*gateway': 'str' } }
> [...]
> 'route': [ 'RouteEntry' ]

Sadly, once the QMP is released, we don't want to be changing it  (other 
than perhaps by use of an 'alternate' that allows both old and new 
styles in parallel), so that we don't break clients expecting the 
original way to work.  It may be better to bite the bullet and fix the 
command line parser to do what we need (Markus has been doing lots of 
work in this area already, so it may just be a matter of waiting for him 
to return to work from his leave).

I'm also afraid that since this patch wasn't picked up in any of the 
pull requests for soft freeze, but at the same time it feels like a new 
feature, that it's better delaying it to 2.13 anyways.  (But that may be 
a good thing, if it lets us fix the problems with the command line 
parsing limitations)


> +++ b/qapi/net.json
> @@ -163,6 +163,9 @@
>   # @domainname: guest-visible domain name of the virtual nameserver
>   #              (since 2.12)
>   #
> +# @route: guest-visible static classless route of the virtual nameserver
> +#         (since 2.12)

If this still takes a straight string instead of a proper struct, then 
you at least need to document the syntax supported in that string.  And 
again, I'm thinking 2.12 is going to be premature, so your next spin of 
this patch should use 2.13.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org