[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 01/12] migration: Introduce announce parameters
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 01/12] migration: Introduce announce parameters |
Date: |
Fri, 26 May 2017 08:08:59 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 |
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" <address@hidden>
>
> Signed-off-by: Vladislav Yasevich <address@hidden>
> ---
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).
> +
> +##
> +# @AnnounceParameters:
> +#
> +# Optional members may be omited on input, but all values will be present
s/omited/omitted/
> +# on output.
> +#
> +# @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
> +##
> +
> +{ 'struct': 'AnnounceParameters',
> + 'data': { '*initial': 'int',
> + '*max': 'int',
> + '*rounds': 'int',
> + '*step': 'int' } }
> +
> +##
> +# @announce-set-parameters:
> +#
> +# Set qemu announce parameters.
> +#
> +# Since: 2.10
> +#
> +# Example:
> +#
> +# -> { "execute": "announce-set-parameters",
> +# "arguments": { "announce-rounds": 10 } }
> +#
> +##
> +{ 'command': 'announce-set-parameters', 'boxed': true,
> + 'data': 'AnnounceParameters' }
> +
> +##
> +# @query-announce-parameters:
> +#
> +# Returns information about the current announce parameters
> +#
> +# Returns: @AnnounceParameters
> +#
> +# Since: 2.10
> +#
> +# Example:
> +#
> +# -> { "execute": "query-announce-parameters" }
> +# <- { "return": {
> +# "initial": 50,
> +# "max": 500,
> +# "rounds": 5,
> +# "step": 100
> +# }
> +# }
> +#
> +##
> +{ 'command': 'query-announce-parameters',
> + 'returns': 'AnnounceParameters' }
Yep, I'm right. The enum is bogus. The struct is sufficient, so you
don't need the enum.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature