[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. >