qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] Contribution - L2TPv3 transport


From: Anton Ivanov (antivano)
Subject: Re: [Qemu-devel] Contribution - L2TPv3 transport
Date: Fri, 28 Feb 2014 14:03:03 +0000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130922 Icedove/17.0.9

[snip]

>>> s/1.0/2.0/
>> OK - just to clarify, which version is this referring to - qemu, api, etc?
> This field is the version of qemu that first contains the release.  If
> your patch is on time to make the qemu 2.0 release, then 2.0 is
> appropriate; but we're close to feature freeze so it may end up being 2.1.

Understood - we have used an earlier version as an internal testing tool
since 1.0, that is why it was marked 1.0 in the patchset.

I will do my best to turn around on the comments as quick as I can so it
can be considered for 2.0 or 2.1

>
>>>> +##
>>>> +##
>>>> +{ 'type': 'NetdevL2TPv3Options',
>>>> +  'data': {
>>>> +    '*fd':        'str',
>>>> +    '*src':         'str', 
>>> You didn't list 'src' as optional above.
>> That is intended. It is not optional for the ip command in Linux either.
>> For L2TPv3 tunnels you have to specify the source address.
> But listing it as '*src' means it is optional.  If it is mandatory, list
> it as 'src'.

Understood.

>
>>> Trailing whitespace - run your submission through checkpatch.pl.
>>>
>>>> +    '*dst':         'str', 
>>>> +    '*mode':        'str', 
>>> As mentioned above, 'mode' should not be a string.
>>>
>>>> +    '*txcookie':  'str',
>>>> +    '*rxcookie':  'str',
>>>> +    '*txsession': 'str',
>>>> +    '*rxsession': 'str'
>>> These should probably be 'int', not 'str'.  If you have to hand-parse a
>>> string into a numeric value, you encoded the QMP wrong.
>> These by spec are either 32 bit or 64 bit integers. I will fix them
>> accordingly.
> The qapi type 'int' is 64-bit.  You can further range check the values
> in your C code and fail the command if the user passed values that don't
> fit in 32 bits when they requested a 32-bit mode.

OK, thanks.

A.

>

reply via email to

[Prev in Thread] Current Thread [Next in Thread]