Re: [Qemu-devel] Contribution - L2TPv3 transport

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3678 bytes --]

On 03/04/2014 08:22 AM, Anton Ivanov (antivano) wrote:
> Apologies, missed to diff the json definitions.
> 
> Attached.
> 

Missing a commit message and a Signed-off-by line, so it can't be
applied as-is.  Also, we prefer inline patches (as sent by 'git
send-email') over attached patches; I suggest using 'git send-email' to
first send the patches to yourself to make sure your settings are correct.


> diff --git a/qapi-schema.json b/qapi-schema.json
> index 83fa485..56eac6d 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2940,6 +2940,62 @@
>      '*localaddr': 'str',
>      '*udp':       'str' } }
>  
> +# @NetdevL2TPv3Options
> +#
> +# Connect the VLAN to Ethernet over L2TPv3 Static tunnel
> +#
> +# @ipv6: #bool, use ipv6 
> +#

This should be:

# @ipv6: #optional true to use ipv6, default false

That is, we have a marking '#optional', but do not have a marking
'#bool' (you can read the actual definition below to learn that 'ipv6'
is a bool type).

Also, it makes it easier if you document options in the same order as
they appear in the struct below (there, you have 'fd' first).

> +# @udp: #bool use the udp version of the L2TPv3 encapsulation

Again, #optional, not #bool, and mention the default value

> +#
> +# @cookie64 : #use 64 bit cookies

#optional

> +#
> +# @offset : #extra offset

#optional

> +#
> +# @counter : #have sequence counter

#optional

> +#
> +# @fd: #optional file descriptor of an already opened socket

This doesn't seem to take into account my earlier comments - is the goal
to allow opening both files from the file system and the magic string
'/dev/fdset/...' supported by use of 'add-fd'?

> +#
> +# @src: #source address

Should be:

# @src: source address

The only use of # inside the docs has been for the tag '#optional'

> +#
> +# @srcport: #source port - mandatory for udp, optional for ip
> +#
> +# @dst: #destination address
> +#
> +# @dstport: #destination port - mandatory for udp, optional for ip
> +#
> +# @txcookie: #optional 32 or 64 bit tx cookie for the tunnel 
> +#
> +# @rxcookie: #optional 32 or 64 bit rx cookie for the tunnel
> +#
> +# @txsession: #tx 32 bit session
> +#
> +# @rxsession: #rx 32 bit session - if unset value for txsession is used

Should be:

# @rxsession: #optional rx 32 bit session, defaults to @txsession

> +#
> +#
> +# Since 1.2

Should be:

Since 2.0

> +##
> +##
> +{ 'type': 'NetdevL2TPv3Options',
> +  'data': {
> +    '*fd':        'str',
> +    'src':	  'str', 
> +    'dst':	  'str', 
> +    '*srcport':	  'str', 
> +    '*dstport':	  'str', 
> +    '*ipv6':	  'bool', 
> +    '*udp':	  'bool', 
> +    '*cookie64':  'bool', 
> +    '*counter':   'bool', 
> +    '*txcookie':  'uint64',
> +    '*rxcookie':  'uint64',
> +    'txsession': 'uint32',
> +    '*rxsession': 'uint32',
> +    '*offset':  'uint32' 
> +

Why the blank line?
> +} }
> +
> +##
>  ##
>  # @NetdevVdeOptions
>  #
> @@ -3014,13 +3070,16 @@
>  # A discriminated record of network device traits.
>  #
>  # Since 1.2
> -##
> +#
> +# Added in 2.0 - l2tpv3 
> +#
>  { 'union': 'NetClientOptions',
>    'data': {
>      'none':     'NetdevNoneOptions',
>      'nic':      'NetLegacyNicOptions',
>      'user':     'NetdevUserOptions',
>      'tap':      'NetdevTapOptions',
> +    'l2tpv3':   'NetdevL2TPv3Options',
>      'socket':   'NetdevSocketOptions',
>      'vde':      'NetdevVdeOptions',
>      'dump':     'NetdevDumpOptions',
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]