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

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

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

Patch
diff mbox series

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()) {