diff mbox series

[2/2] migration/compress: disable compress if failed to setup

Message ID 20191012023932.1863-3-richardw.yang@linux.intel.com
State New, archived
Headers show
Series migration/compress: refine the compress case | expand

Commit Message

Wei Yang Oct. 12, 2019, 2:39 a.m. UTC
In current logic, if compress_threads_save_setup() returns -1 the whole
migration would fail, while we could handle it gracefully by disable
compress.

Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
---
 migration/migration.c |  9 +++++++++
 migration/migration.h |  1 +
 migration/ram.c       | 15 ++++++++-------
 3 files changed, 18 insertions(+), 7 deletions(-)

Comments

Dr. David Alan Gilbert Nov. 7, 2019, 12:11 p.m. UTC | #1
* Wei Yang (richardw.yang@linux.intel.com) wrote:
> In current logic, if compress_threads_save_setup() returns -1 the whole
> migration would fail, while we could handle it gracefully by disable
> compress.

I think it's fine for migration to fail here; the user askd for
compression - if it doesn't work then it's OK to fail and they can
switch it off; since it fails right at the start there's nothing lost.

Dave

> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
> ---
>  migration/migration.c |  9 +++++++++
>  migration/migration.h |  1 +
>  migration/ram.c       | 15 ++++++++-------
>  3 files changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 5f7e4d15e9..02b95f4223 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -2093,6 +2093,15 @@ bool migrate_use_compression(void)
>      return s->enabled_capabilities[MIGRATION_CAPABILITY_COMPRESS];
>  }
>  
> +void migrate_disable_compression(void)
> +{
> +    MigrationState *s;
> +
> +    s = migrate_get_current();
> +
> +    s->enabled_capabilities[MIGRATION_CAPABILITY_COMPRESS] = false;
> +}
> +
>  int migrate_compress_level(void)
>  {
>      MigrationState *s;
> diff --git a/migration/migration.h b/migration/migration.h
> index 4f2fe193dc..51368d3a6e 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -309,6 +309,7 @@ bool migrate_use_return_path(void);
>  uint64_t ram_get_total_transferred_pages(void);
>  
>  bool migrate_use_compression(void);
> +void migrate_disable_compression(void);
>  int migrate_compress_level(void);
>  int migrate_compress_threads(void);
>  int migrate_compress_wait_thread(void);
> diff --git a/migration/ram.c b/migration/ram.c
> index 96c9b16402..39279161a8 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -533,12 +533,12 @@ static void compress_threads_save_cleanup(void)
>      comp_param = NULL;
>  }
>  
> -static int compress_threads_save_setup(void)
> +static void compress_threads_save_setup(void)
>  {
>      int i, thread_count;
>  
>      if (!migrate_use_compression()) {
> -        return 0;
> +        return;
>      }
>      thread_count = migrate_compress_threads();
>      compress_threads = g_new0(QemuThread, thread_count);
> @@ -569,11 +569,14 @@ static int compress_threads_save_setup(void)
>                             do_data_compress, comp_param + i,
>                             QEMU_THREAD_JOINABLE);
>      }
> -    return 0;
> +    return;
>  
>  exit:
>      compress_threads_save_cleanup();
> -    return -1;
> +    migrate_disable_compression();
> +    error_report("%s: failed to setup compress threads, compress disabled",
> +                 __func__);
> +    return;
>  }
>  
>  /* Multiple fd's */
> @@ -3338,9 +3341,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
>      RAMState **rsp = opaque;
>      RAMBlock *block;
>  
> -    if (compress_threads_save_setup()) {
> -        return -1;
> -    }
> +    compress_threads_save_setup();
>  
>      /* migration has already setup the bitmap, reuse it. */
>      if (!migration_in_colo_state()) {
> -- 
> 2.17.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Juan Quintela Jan. 27, 2020, 3:13 p.m. UTC | #2
Wei Yang <richardw.yang@linux.intel.com> wrote:
> In current logic, if compress_threads_save_setup() returns -1 the whole
> migration would fail, while we could handle it gracefully by disable
> compress.
>
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>

Fully agree with Dave here.

If user asks for compression, and compression fails, we fail migration.
If user wants to continue without compression, it just have to disable
compression.

Thanks, Juan.
diff mbox series

Patch

diff --git a/migration/migration.c b/migration/migration.c
index 5f7e4d15e9..02b95f4223 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2093,6 +2093,15 @@  bool migrate_use_compression(void)
     return s->enabled_capabilities[MIGRATION_CAPABILITY_COMPRESS];
 }
 
+void migrate_disable_compression(void)
+{
+    MigrationState *s;
+
+    s = migrate_get_current();
+
+    s->enabled_capabilities[MIGRATION_CAPABILITY_COMPRESS] = false;
+}
+
 int migrate_compress_level(void)
 {
     MigrationState *s;
diff --git a/migration/migration.h b/migration/migration.h
index 4f2fe193dc..51368d3a6e 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -309,6 +309,7 @@  bool migrate_use_return_path(void);
 uint64_t ram_get_total_transferred_pages(void);
 
 bool migrate_use_compression(void);
+void migrate_disable_compression(void);
 int migrate_compress_level(void);
 int migrate_compress_threads(void);
 int migrate_compress_wait_thread(void);
diff --git a/migration/ram.c b/migration/ram.c
index 96c9b16402..39279161a8 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -533,12 +533,12 @@  static void compress_threads_save_cleanup(void)
     comp_param = NULL;
 }
 
-static int compress_threads_save_setup(void)
+static void compress_threads_save_setup(void)
 {
     int i, thread_count;
 
     if (!migrate_use_compression()) {
-        return 0;
+        return;
     }
     thread_count = migrate_compress_threads();
     compress_threads = g_new0(QemuThread, thread_count);
@@ -569,11 +569,14 @@  static int compress_threads_save_setup(void)
                            do_data_compress, comp_param + i,
                            QEMU_THREAD_JOINABLE);
     }
-    return 0;
+    return;
 
 exit:
     compress_threads_save_cleanup();
-    return -1;
+    migrate_disable_compression();
+    error_report("%s: failed to setup compress threads, compress disabled",
+                 __func__);
+    return;
 }
 
 /* Multiple fd's */
@@ -3338,9 +3341,7 @@  static int ram_save_setup(QEMUFile *f, void *opaque)
     RAMState **rsp = opaque;
     RAMBlock *block;
 
-    if (compress_threads_save_setup()) {
-        return -1;
-    }
+    compress_threads_save_setup();
 
     /* migration has already setup the bitmap, reuse it. */
     if (!migration_in_colo_state()) {