Re: [Qemu-devel] [PATCH v2 2/9] migration: Add announce parameters

[Daniel P . Berrangé]

On Wed, Jan 30, 2019 at 10:32:29AM +0000, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <address@hidden>
> 
> Add migration parameters that control RARP/GARP announcement timeouts.
> 
> Based on earlier patches by myself and
>   Vladislav Yasevich <address@hidden>
> 
> Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> ---
>  hmp.c                    |  28 +++++++++++
>  include/migration/misc.h |   2 +
>  include/qemu/typedefs.h  |   1 +
>  migration/migration.c    | 100 +++++++++++++++++++++++++++++++++++++++
>  qapi/migration.json      |  54 +++++++++++++++++++--
>  5 files changed, 182 insertions(+), 3 deletions(-)
> 
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 7a795ecc16..113bb5d925 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -6,6 +6,7 @@
>  ##
>  
>  { 'include': 'common.json' }
> +{ 'include': 'net.json' }


> @@ -572,6 +588,18 @@
>  ##
>  # @MigrateSetParameters:
>  #
> +# @announce-initial: Initial delay (in ms) before sending the first announce
> +#          (Since 4.0)
> +#
> +# @announce-max: Maximum delay (in ms) between packets in the announcment
> +#          (Since 4.0)
> +#
> +# @announce-rounds: Number of self-announce packets sent after migration
> +#          (Since 4.0)
> +#
> +# @announce-step: Increase in delay (in ms) between subsequent packets in
> +#          the announcement (Since 4.0)
> +#
>  # @compress-level: compression level
>  #
>  # @compress-threads: compression thread count
> @@ -653,7 +681,11 @@
>  # TODO either fuse back into MigrationParameters, or make
>  # MigrationParameters members mandatory
>  { 'struct': 'MigrateSetParameters',
> -  'data': { '*compress-level': 'int',
> +  'data': { '*announce-initial': 'size',
> +            '*announce-max': 'size',
> +            '*announce-rounds': 'size',
> +            '*announce-step': 'size',
> +            '*compress-level': 'int',
>              '*compress-threads': 'int',
>              '*compress-wait-thread': 'bool',
>              '*decompress-threads': 'int',

Historically we've just had a flat list of migration parameters, but
QAPI doesn't require this. So I wonder about just referencing the
type you defined in the previous patch:

   '*announce': 'AnnounceParameters

from a QMP pov this is trivial & feels more natural. The only downside
I see is that for HMP it would need to be flattened to what it is here.
We generally tend to prefer QMP's natural style, even if it doesn't
match what's nicest for HMP.

> @@ -692,6 +724,18 @@
>  #
>  # The optional members aren't actually optional.
>  #
> +# @announce-initial: Initial delay (in ms) before sending the first announce
> +#          (Since 4.0)
> +#
> +# @announce-max: Maximum delay (in ms) between packets in the announcment
> +#          (Since 4.0)
> +#
> +# @announce-rounds: Number of self-announce packets sent after migration
> +#          (Since 4.0)
> +#
> +# @announce-step: Increase in delay (in ms) between subsequent packets in
> +#          the announcement (Since 4.0)
> +#
>  # @compress-level: compression level
>  #
>  # @compress-threads: compression thread count
> @@ -769,7 +813,11 @@
>  # Since: 2.4
>  ##
>  { 'struct': 'MigrationParameters',
> -  'data': { '*compress-level': 'uint8',
> +  'data': { '*announce-initial': 'size',
> +            '*announce-max': 'size',
> +            '*announce-rounds': 'size',
> +            '*announce-step': 'size',

Same here about just referencing the AnnounceParameters type.

> +            '*compress-level': 'uint8',
>              '*compress-threads': 'uint8',
>              '*compress-wait-thread': 'bool',
>              '*decompress-threads': 'uint8',

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|