Re: [RFC 6/6] migration: Deprecated old compression method
Daniel P. Berrangé <berrange(a)redhat.com> wrote:
On Mon, Jun 12, 2023 at 09:33:44PM +0200, Juan Quintela wrote:
> Signed-off-by: Juan Quintela <quintela(a)redhat.com>
> ---
> docs/about/deprecated.rst | 8 ++++
> qapi/migration.json | 92 ++++++++++++++++++++++++---------------
> migration/options.c | 13 ++++++
> 3 files changed, 79 insertions(+), 34 deletions(-)
>
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index 173c5ba5cb..fe7f2bbde8 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -460,3 +460,11 @@ block migration (since 8.1)
> Block migration is too inflexible. It needs to migrate all block
> devices or none. Use driver_mirror+NBD instead.
>
> +old compression method (since 8.1)
>
+''''''''''''''''''''''''''''''''''
> +
> +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 hand randomly. If you need
> +compression, use multifd compression methods.
> +
> diff --git a/qapi/migration.json b/qapi/migration.json
> index a8497de48d..40a8b5d124 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -244,6 +244,7 @@
> #
> # @compression: migration compression statistics, only returned if
> # compression feature is on and status is 'active' or
'completed'
> +# It is obsolete and deprecated. Use multifd compression methods.
> # (Since 3.1)
This doesn't give users an indication /why/ we're saying this. Instead
I'd suggest
This feature is unreliable and not tested. It is recommended to
use multifd migration instead, which offers an alternative reliable
and tested compression implementation.
Much better. Done, thanks.
> # @deprecated: @disk migration is deprecated. Use
driver_mirror+NBD
> -# instead.
> +# instead. @compression is obsolete use multifd compression
> +# methods instead.
For @deprecated, are we supposed to list multiple things at once, or
use a separate @deprecated tag for each one ?
# @unstable: Members @x-colo and @x-ignore-shared are experimental.
This is the only example that I found that is similar.
Only one example. Markus?
Again I'd suggest rewording
@compression is unreliable and untested. It is recommended to
use multifd migration, which offers an alternative compression
implementation that is reliable and tested.
Done.
> @@ -443,6 +443,11 @@ bool migrate_caps_check(bool *old_caps, bool
*new_caps, Error **errp)
> "Use driver_mirror+NBD instead.");
> }
>
> + if (new_caps[MIGRATION_CAPABILITY_BLOCK]) {
Surely MIGRATION_CAPABILITY_COMPRESS not BLOCK ?
Good catch. Copy & paste to its best.
Thanks very much.