Re: [PATCH 5/6] migration: Remove non-multifd compression

2024-04-26 Thread Markus Armbruster
Fabiano Rosas  writes:

> The 'compress' migration capability enables the old compression code
> which has shown issues over the years and is thought to be less stable
> and tested than the more recent multifd-based compression. The old
> compression code has been deprecated in 8.2 and now is time to remove
> it.
>
> Deprecation commit 864128df46 ("migration: Deprecate old compression
> method").
>
> Signed-off-by: Fabiano Rosas 

[...]

> diff --git a/migration/options.c b/migration/options.c
> index 5049bfb78e..5b0658bad1 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -40,13 +40,6 @@
>   * for sending the last part */
>  #define DEFAULT_MIGRATE_SET_DOWNTIME 300
>  
> -/* Default compression thread count */
> -#define DEFAULT_MIGRATE_COMPRESS_THREAD_COUNT 8
> -/* Default decompression thread count, usually decompression is at
> - * least 4 times as fast as compression.*/
> -#define DEFAULT_MIGRATE_DECOMPRESS_THREAD_COUNT 2
> -/*0: means nocompress, 1: best speed, ... 9: best compress ratio */
> -#define DEFAULT_MIGRATE_COMPRESS_LEVEL 1
>  /* Define default autoconverge cpu throttle migration parameters */
>  #define DEFAULT_MIGRATE_THROTTLE_TRIGGER_THRESHOLD 50
>  #define DEFAULT_MIGRATE_CPU_THROTTLE_INITIAL 20
> @@ -92,8 +85,6 @@ Property migration_properties[] = {
>   send_configuration, true),
>  DEFINE_PROP_BOOL("send-section-footer", MigrationState,
>   send_section_footer, true),
> -DEFINE_PROP_BOOL("decompress-error-check", MigrationState,
> -  decompress_error_check, true),
>  DEFINE_PROP_BOOL("multifd-flush-after-each-section", MigrationState,
>multifd_flush_after_each_section, false),
>  DEFINE_PROP_UINT8("x-clear-bitmap-shift", MigrationState,
> @@ -102,17 +93,6 @@ Property migration_properties[] = {
>   preempt_pre_7_2, false),
>  
>  /* Migration parameters */
> -DEFINE_PROP_UINT8("x-compress-level", MigrationState,
> -  parameters.compress_level,
> -  DEFAULT_MIGRATE_COMPRESS_LEVEL),
> -DEFINE_PROP_UINT8("x-compress-threads", MigrationState,
> -  parameters.compress_threads,
> -  DEFAULT_MIGRATE_COMPRESS_THREAD_COUNT),
> -DEFINE_PROP_BOOL("x-compress-wait-thread", MigrationState,
> -  parameters.compress_wait_thread, true),
> -DEFINE_PROP_UINT8("x-decompress-threads", MigrationState,
> -  parameters.decompress_threads,
> -  DEFAULT_MIGRATE_DECOMPRESS_THREAD_COUNT),
>  DEFINE_PROP_UINT8("x-throttle-trigger-threshold", MigrationState,
>parameters.throttle_trigger_threshold,
>DEFAULT_MIGRATE_THROTTLE_TRIGGER_THRESHOLD),
> @@ -188,7 +168,6 @@ Property migration_properties[] = {
>  DEFINE_PROP_MIG_CAP("x-rdma-pin-all", MIGRATION_CAPABILITY_RDMA_PIN_ALL),
>  DEFINE_PROP_MIG_CAP("x-auto-converge", 
> MIGRATION_CAPABILITY_AUTO_CONVERGE),
>  DEFINE_PROP_MIG_CAP("x-zero-blocks", MIGRATION_CAPABILITY_ZERO_BLOCKS),
> -DEFINE_PROP_MIG_CAP("x-compress", MIGRATION_CAPABILITY_COMPRESS),
>  DEFINE_PROP_MIG_CAP("x-events", MIGRATION_CAPABILITY_EVENTS),
>  DEFINE_PROP_MIG_CAP("x-postcopy-ram", MIGRATION_CAPABILITY_POSTCOPY_RAM),
>  DEFINE_PROP_MIG_CAP("x-postcopy-preempt",
> @@ -231,13 +210,6 @@ bool migrate_colo(void)
>  return s->capabilities[MIGRATION_CAPABILITY_X_COLO];
>  }
>  
> -bool migrate_compress(void)
> -{
> -MigrationState *s = migrate_get_current();
> -
> -return s->capabilities[MIGRATION_CAPABILITY_COMPRESS];
> -}
> -

Please also drop the declaration from migration/options.h.

>  bool migrate_dirty_bitmaps(void)
>  {
>  MigrationState *s = migrate_get_current();
> @@ -451,7 +423,6 @@ 
> INITIALIZE_MIGRATE_CAPS_SET(check_caps_background_snapshot,
>  MIGRATION_CAPABILITY_AUTO_CONVERGE,
>  MIGRATION_CAPABILITY_RELEASE_RAM,
>  MIGRATION_CAPABILITY_RDMA_PIN_ALL,
> -MIGRATION_CAPABILITY_COMPRESS,
>  MIGRATION_CAPABILITY_XBZRLE,
>  MIGRATION_CAPABILITY_X_COLO,
>  MIGRATION_CAPABILITY_VALIDATE_UUID,
> @@ -476,11 +447,6 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps, 
> Error **errp)
>  ERRP_GUARD();
>  MigrationIncomingState *mis = migration_incoming_get_current();
>  
> -if (new_caps[MIGRATION_CAPABILITY_COMPRESS]) {
> -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"
> @@ -549,7 +515,6 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps, 
> Error **errp)
>  #ifdef CONFIG_LINUX
>  if (new_caps[MIGRATION_CAPABILITY_ZERO_COPY_SEND] &&
>  (!new_caps[MIGRATION_CAPABILITY_MULTIFD] ||
> - 

[PATCH 5/6] migration: Remove non-multifd compression

2024-04-25 Thread Fabiano Rosas
The 'compress' migration capability enables the old compression code
which has shown issues over the years and is thought to be less stable
and tested than the more recent multifd-based compression. The old
compression code has been deprecated in 8.2 and now is time to remove
it.

Deprecation commit 864128df46 ("migration: Deprecate old compression
method").

Signed-off-by: Fabiano Rosas 
---
 docs/about/deprecated.rst   |  11 -
 docs/about/removed-features.rst |  55 
 hw/core/machine.c   |   1 -
 migration/meson.build   |   1 -
 migration/migration-hmp-cmds.c  |  47 ---
 migration/migration.c   |  14 -
 migration/migration.h   |   7 -
 migration/options.c | 164 --
 migration/ram-compress.c| 564 
 migration/ram.c | 151 +
 qapi/migration.json | 112 ---
 tests/qtest/migration-test.c| 139 
 12 files changed, 63 insertions(+), 1203 deletions(-)
 delete mode 100644 migration/ram-compress.c

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 409c9e0c4d..fadb85f289 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -484,14 +484,3 @@ both, older and future versions of QEMU.
 The ``blacklist`` config file option has been renamed to ``block-rpcs``
 (to be in sync with the renaming of the corresponding command line
 option).
-
-Migration
--
-
-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 because they fail randomly.  If you need
-compression, use multifd compression methods.
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index dd20f37f2c..783f6b479d 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -505,6 +505,11 @@ configurations (e.g. -smp 8,sockets=0) is removed since 
9.0, users have
 to ensure that all the topology members described with -smp are greater
 than zero.
 
+``-global migration.decompress-error-check`` (removed in 9.1)
+'
+
+Removed along with the ``compression`` migration capability.
+
 User-mode emulator command line arguments
 -
 
@@ -641,6 +646,31 @@ Block migration has been removed. For a replacement, see 
"QMP
 invocation for live storage migration with ``blockdev-mirror`` + NBD"
 in docs/interop/live-block-operations.rst.
 
+``migrate-set-parameter`` ``compress-level`` option (removed in 9.1)
+
+
+Use ``multifd-zlib-level`` or ``multifd-zstd-level`` instead.
+
+``migrate-set-parameter`` ``compress-threads`` option (removed in 9.1)
+''
+
+Use ``multifd-channels`` instead.
+
+``migrate-set-parameter`` ``compress-wait-thread`` option (removed in 9.1)
+''
+
+Removed with no replacement.
+
+``migrate-set-parameter`` ``decompress-threads`` option (removed in 9.1)
+
+
+Use ``multifd-channels`` instead.
+
+``migrate-set-capability`` ``compress`` option (removed in 9.1)
+'''
+
+Use ``multifd-compression`` instead.
+
 Human Monitor Protocol (HMP) commands
 -
 
@@ -722,6 +752,31 @@ Block migration has been removed. For a replacement, see 
"QMP
 invocation for live storage migration with ``blockdev-mirror`` + NBD"
 in docs/interop/live-block-operations.rst.
 
+``migrate_set_parameter`` ``compress-level`` option (removed in 9.1)
+
+
+Use ``multifd-zlib-level`` or ``multifd-zstd-level`` instead.
+
+``migrate_set_parameter`` ``compress-threads`` option (removed in 9.1)
+''
+
+Use ``multifd-channels`` instead.
+
+``migrate_set_parameter`` ``compress-wait-thread`` option (removed in 9.1)
+''
+
+Removed with no replacement.
+
+``migrate_set_parameter`` ``decompress-threads`` option (removed in 9.1)
+
+
+Use ``multifd-channels`` instead.
+
+``migrate_set_capability`` ``compress`` option (removed in 9.1)
+'''
+
+Use ``multifd-compression`` instead.
+
 Host Architectures
 --
 
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 582c2df37a..596ff77e3c 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -190,7 +190,6 @@