Re: [Qemu-devel] [PATCH v5 1/2] slirp: Add "query-usernet" QMP command

[Markus Armbruster <armbru@redhat.com>]

Eric Blake <eblake@redhat.com> writes:

> On 05/03/2018 04:12 AM, Daniel P. Berrangé wrote:
>> On Thu, May 03, 2018 at 09:25:45AM +0800, Fam Zheng wrote:
>>> HMP "info usernet" has been available but it isn't ideal for programmed
>>> use cases. This closes the gap in QMP by adding a counterpart
>>> "query-usernet" command. It is basically translated from
>>> the HMP slirp_connection_info() loop, which now calls the QMP
>>> implementation and prints the data, just like other HMP info_* commands.
>>>
>>> The TCPS_* macros are now defined as a QAPI enum.
>>>
>>> Signed-off-by: Fam Zheng <famz@redhat.com>
>>
>
>>> +##
>>> +{ 'enum': 'TCPS',
>>
>> I'd suggest avoiding the abbreviation, IOW use  TCPState as
>> the name. Yes that will require changnig the constants in
>> the SLIRP code, but that's worthwhile to get this QAPI
>> naming clearer IMHO.
>>
>> I wonder if we should have a common prefix too eg UsernetTCPState
>> as this is a global QAPI namespace after all .
>
> You could also use
>
> { 'enum': 'UsernetTCPState', 'prefix': 'TCPS', ...
>
> for minimizing code churn (if 'prefix' is present, that overrides what
> the generator would use for enum values); although I'm not sure if
> that is any better (we haven't used 'prefix' much, and Markus wasn't
> the biggest fan of it in the first place, as it is more of a hack).

Please avoid 'prefix' unless you have a really, really good reason to
use it.

    $ git-grep TCPS_ | wc -l
    99

doesn't look like one.