On 05/26/2017 09:08 AM, Eric Blake wrote:
> On 05/24/2017 01:05 PM, Vladislav Yasevich wrote:
>> Add parameters that control RARP/GARP announcement timeouts.
>> The parameters structure is added to the QAPI and a qmp command
>> is added to set/get the parameter data.
>>
>> Based on work by "Dr. David Alan Gilbert" <dgilb...@redhat.com>
>>
>> Signed-off-by: Vladislav Yasevich <vyase...@redhat.com>
>> ---
>
> Just an interface review for now:
>
>> +++ b/qapi-schema.json
>> @@ -569,6 +569,90 @@
>> ##
>> { 'command': 'query-events', 'returns': ['EventInfo'] }
>>
>> +
>> +##
>> +# @AnnounceParameter:
>> +#
>> +# @AnnounceParameter enumeration
>> +#
>> +# @initial: Initial delay (in ms) before sending the first GARP/RARP
>> +# announcement
>> +#
>> +# @max: Maximum delay (in ms) to between GARP/RARP announcemnt packets
>
> s/announcemnt/announcement/
>
>> +#
>> +# @rounds: Number of self-announcement attempts
>> +#
>> +# @step: Delay increate (in ms) after each self-announcment attempt
>
> s/increate/increase/
> s/announcment/announcement/
>
>> +#
>> +# Since: 2.10
>> +##
>> +{ 'enum' : 'AnnounceParameter',
>> + 'data' : [ 'initial', 'max', 'rounds', 'step' ] }
>
> Why are we creating an enum here? Without reading further, it looks
> like you plan on using the enum to delineate members of a union? But
> that feels like it will be overly complicated. A struct should be
> sufficient (each parameter being an optional member of the struct, where
> you can supply as many or as few on input, but all are reported on output).
>
I end up using them for the HMP interface. If it's better, I can move this
definition to the HMP patch.
Thanks
-vlad
