On 04/03/14 15:53, Eric Blake wrote:
> 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.
Will do. Can you review the actual transport patch please.
>
>
>> 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'?
Yes, but not yet supported in this version. I will cut it out for now.
>
>> +#
>> +# @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',
>>
