Re: [PATCH V1 1/4] migration: mode parameter

[Juan Quintela]

Steve Sistare <steven.sistare@oracle.com> wrote:
> Create a mode migration parameter that can be used to select alternate
> migration algorithms.  The default mode is normal, representing the
> current migration algorithm, and does not need to be explicitly set.
>
> No functional change until a new mode is added, except that the mode is
> shown by the 'info migrate' command.
>
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>

[... qdev definition ...]

Looks legit, but I am not an expert here.


> @@ -867,6 +870,13 @@ uint64_t migrate_xbzrle_cache_size(void)
>      return s->parameters.xbzrle_cache_size;
>  }
>  
> +MigMode migrate_mode(void)
> +{
> +    MigrationState *s = migrate_get_current();
> +
> +    return s->parameters.mode;
> +}
> +

Inside parameters, I try to get the functions sorted by name.  the same
for options.h

> diff --git a/qapi/migration.json b/qapi/migration.json
> index db3df12..184fb78 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -616,6 +616,15 @@
>              { 'name': 'zstd', 'if': 'CONFIG_ZSTD' } ] }
>  
>  ##
> +# @MigMode:
> +#
> +# @normal: the original form of migration. (since 8.2)
> +#
> +##
> +{ 'enum': 'MigMode',
> +  'data': [ 'normal' ] }
> +
> +##

Here you only have normal, but in qdev you also have exec.


>  # @BitmapMigrationBitmapAliasTransform:
>  #
>  # @persistent: If present, the bitmap will be made persistent or
> @@ -675,6 +684,9 @@
>  #
>  # Migration parameters enumeration
>  #
> +# @mode: Migration mode. See description in @MigMode. Default is 'normal'.
> +#        (Since 8.2)
> +#

You normally put comments and values at the end of the comments and
sections. Your sshould be last.

Feel free to use a single line in the json.  More than one value for
line make it a bit more compress, but makes changes more complicated.

Rest of the patch feels right.

Thanks, Juan.