Re: [PATCH v3 4/4] migration: Deprecate old compression method

[Markus Armbruster <armbru@redhat.com>]

Juan Quintela <quintela@redhat.com> writes:

> Signed-off-by: Juan Quintela <quintela@redhat.com>
> Acked-by: Peter Xu <peterx@redhat.com>
> ---
>  docs/about/deprecated.rst |   8 +++
>  qapi/migration.json       | 102 ++++++++++++++++++++++++--------------
>  migration/options.c       |  13 +++++
>  3 files changed, 86 insertions(+), 37 deletions(-)
>
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index e46f3df376..b92621996f 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -441,3 +441,11 @@ Please see "QMP invocation for live storage migration with
>  ``driver-mirror`` + NBD" in docs/interop/live-block-operations.rst for
>  a detailed explanation.
>  
> +old compression method (since 8.2)
> +''''''''''''''''''''''''''''''''''
> +
> +Compression method fails too much.  Too many races.  We are going to
> +remove it if nobody fixes it.  For starters, migration-test
> +compression tests are disabled becase they fail randomly.  If you need
> +compression, use multifd compression methods.
> +
> diff --git a/qapi/migration.json b/qapi/migration.json
> index bf6e1ac5b5..549306fa76 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -243,7 +243,9 @@
>  #
>  # @compression: migration compression statistics, only returned if
>  #     compression feature is on and status is 'active' or 'completed'
> -#     (Since 3.1)
> +#     This feature is unreliable and not tested. It is recommended to
> +#     use multifd migration instead, which offers an alternative
> +#     reliable and tested compression implementation.  (Since 3.1)
>  #
>  # @socket-address: Only used for tcp, to know what the real port is
>  #     (Since 4.0)
> @@ -271,8 +273,11 @@
>  #
>  # Features:
>  #
> -# @deprecated: @disk migration is deprecated.  Use driver-mirror
> -#     with NBD instead.
> +# @deprecated: @disk migration is deprecated.  Use driver-mirror with
> +#     NBD instead.  @compression is unreliable and untested. It is
> +#     recommended to use multifd migration, which offers an
> +#     alternative compression implementation that is reliable and
> +#     tested.

Again, single deprecation note under "Features:", please.

>  #
>  # Since: 0.14
>  ##
> @@ -290,7 +295,7 @@
>             '*blocked-reasons': ['str'],
>             '*postcopy-blocktime': 'uint32',
>             '*postcopy-vcpu-blocktime': ['uint32'],
> -           '*compression': 'CompressionStats',
> +           '*compression': { 'type': 'CompressionStats', 'features': ['deprecated'] },
>             '*socket-address': ['SocketAddress'],
>             '*dirty-limit-throttle-time-per-round': 'uint64',
>             '*dirty-limit-ring-full-time': 'uint64'} }
> @@ -445,7 +450,8 @@
>  #     compress and xbzrle are both on, compress only takes effect in
>  #     the ram bulk stage, after that, it will be disabled and only
>  #     xbzrle takes effect, this can help to minimize migration
> -#     traffic.  The feature is disabled by default.  (since 2.4 )
> +#     traffic.  The feature is disabled by default.  Obsolete.  Use

Some places call it "deprecated", others "obsolete".  Best to always use
"deprecated".

More of the same below.

> +#     multifd compression methods if needed. (since 2.4 )
>  #
>  # @events: generate events for each migration state change (since 2.4
>  #     )

[...]

Migration has grown way too many options.  Thanks for pruning them back
some.