On Mon, Jun 12, 2023 at 09:33:44PM +0200, Juan Quintela wrote:
> Signed-off-by: Juan Quintela <quint...@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.
> #
> # @socket-address: Only used for tcp, to know what the real port is
> @@ -261,7 +262,8 @@
> # Features:
> #
> # @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 ?
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.
> diff --git a/migration/options.c b/migration/options.c
> index 374dc0767e..c04e62ba2d 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -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 ?
> + warn_report("Old compression method is deprecated. "
> + "Use multifd compression methods instead.");
> + }
> +
> #ifndef CONFIG_REPLICATION
> if (new_caps[MIGRATION_CAPABILITY_X_COLO]) {
> error_setg(errp, "QEMU compiled without replication module"
> @@ -1196,18 +1201,26 @@ static void migrate_params_apply(MigrateSetParameters
> *params, Error **errp)
> /* TODO use QAPI_CLONE() instead of duplicating it inline */
>
> if (params->has_compress_level) {
> + warn_report("Old compression is deprecated. "
> + "Use multifd compression methods instead.");
> s->parameters.compress_level = params->compress_level;
> }
>
> if (params->has_compress_threads) {
> + warn_report("Old compression is deprecated. "
> + "Use multifd compression methods instead.");
> s->parameters.compress_threads = params->compress_threads;
> }
>
> if (params->has_compress_wait_thread) {
> + warn_report("Old compression is deprecated. "
> + "Use multifd compression methods instead.");
> s->parameters.compress_wait_thread = params->compress_wait_thread;
> }
>
> if (params->has_decompress_threads) {
> + warn_report("Old compression is deprecated. "
> + "Use multifd compression methods instead.");
> s->parameters.decompress_threads = params->decompress_threads;
> }
>
> --
> 2.40.1
>
With 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 :|
