fix some segmentation faults and migration issues

RFC PATCH Superseded Fei Li 4 participants 13 messages Nov 28, 2018 – Nov 30, 2018

Share & tools

Fei Li

author

2018-11-28 10:40 UTC #0 lore raw

cover fix some segmentation faults and migration issues

These five patches are almost get the Reviewed-by and extracted from previous "[PATCH RFC v7 0/9] qemu_thread_create: propagate errors to callers to check." The mentioned patch series have waited on one multifd issue for a while and still needs a further discussion.

Thus separate(send) these five almost-done patches and hope they can be merged for the next tag. Thanks for the review. :)

Fei Li (5):
Fix segmentation fault when qemu_signal_init fails
qemu_thread_join: fix segmentation fault
migration: fix the multifd code when receiving less channels
migration: remove unused &local_err parameter in multifd_save_cleanup
migration: add more error handling for postcopy_ram_enable_notify

 migration/channel.c      | 11 ++++++-----
 migration/migration.c    | 14 ++++++++------
 migration/migration.h    |  2 +-
 migration/postcopy-ram.c |  1 +
 migration/ram.c          | 18 ++++++++++--------
 migration/ram.h          |  4 ++--
 migration/savevm.c       |  1 +
 util/main-loop.c         |  8 ++++----
 util/qemu-thread-posix.c |  3 +++
 util/qemu-thread-win32.c |  2 +-
 10 files changed, 37 insertions(+), 27 deletions(-)

Signature · 1 line

2.13.7

Fei Li

author

2018-11-28 10:41 UTC #1 lore raw

RFC PATCH 1/5 Fix segmentation fault when qemu_signal_init fails

When qemu_signal_init() fails in qemu_init_main_loop(), we return without setting an error. Its callers crash then when they try to report the error with error_report_err().

To avoid such segmentation fault, add a new Error parameter to make the call trace to propagate the err to the final caller.

Cc Markus Armbruster <[email protected]>

Cc Fam Zheng <[email protected]>

Signed-off-by Fei Li <[email protected]>

Reviewed-by Fam Zheng <[email protected]>

---
 util/main-loop.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

util/main-loop.c +4 -4

index affe0403c5..443cb4cfe8 100644
@@ -71,7 +71,7 @@ static void sigfd_handler(void *opaque)
     }
 }
 
-static int qemu_signal_init(void)
+static int qemu_signal_init(Error **errp)
 {
     int sigfd;
     sigset_t set;

@@ -96,7 +96,7 @@ static int qemu_signal_init(void)
     sigdelset(&set, SIG_IPI);
     sigfd = qemu_signalfd(&set);
     if (sigfd == -1) {
-        fprintf(stderr, "failed to create signalfd\n");
+        error_setg_errno(errp, errno, "failed to create signalfd");
         return -errno;
     }

@@ -109,7 +109,7 @@ static int qemu_signal_init(void)
 
 #else /* _WIN32 */
 
-static int qemu_signal_init(void)
+static int qemu_signal_init(Error **errp)
 {
     return 0;
 }

@@ -148,7 +148,7 @@ int qemu_init_main_loop(Error **errp)
 
     init_clocks(qemu_timer_notify_cb);
 
-    ret = qemu_signal_init();
+    ret = qemu_signal_init(errp);
     if (ret) {
         return ret;
     }

Signature · 1 line

2.13.7

Markus Armbruster

Reviewed-by on v1 1/5

2018-11-28 12:54 UTC #2 lore raw

Re: [Qemu-devel] [PATCH RFC 1/5] Fix segmentation fault when qemu_signal_init fails

Quoted 3 lines · Fei Li

When qemu_signal_init() fails in qemu_init_main_loop(), we return without setting an error. Its callers crash then when they try to report the error with error_report_err().

Yes, that's a bug. Broken in 2f78e491d7b, v2.2.0. Has escaped notice since qemu_signalfd() is quite unlikely to fail. Could go into 3.1 as a bug fix, but I think punting it to the next release is just fine.

Quoted 3 lines

To avoid such segmentation fault, add a new Error parameter to make the call trace to propagate the err to the final caller.

Let's add:

Fixes 2f78e491d7b46542158ce0b8132ee4e05bc0ade4

Quoted 40 lines

Cc Markus Armbruster <[email protected]>

Cc Fam Zheng <[email protected]>

Signed-off-by Fei Li <[email protected]>

Reviewed-by Fam Zheng <[email protected]>

---
 util/main-loop.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

util/main-loop.c +3 -3

index affe0403c5..443cb4cfe8 100644
@@ -71,7 +71,7 @@ static void sigfd_handler(void *opaque)
     }
 }
 
-static int qemu_signal_init(void)
+static int qemu_signal_init(Error **errp)
 {
     int sigfd;
     sigset_t set;

@@ -96,7 +96,7 @@ static int qemu_signal_init(void)
     sigdelset(&set, SIG_IPI);
     sigfd = qemu_signalfd(&set);
     if (sigfd == -1) {
-        fprintf(stderr, "failed to create signalfd\n");
+        error_setg_errno(errp, errno, "failed to create signalfd");
         return -errno;
     }

@@ -109,7 +109,7 @@ static int qemu_signal_init(void)
 
 #else /* _WIN32 */
 
-static int qemu_signal_init(void)
+static int qemu_signal_init(Error **errp)
 {
     return 0;
 }

util/main-loop.c int qemu_init_main_loop(Error **errp) · line 154

Quoted 8 lines

util/main-loop.c +1 -1

@@ -148,7 +148,7 @@ int qemu_init_main_loop(Error **errp)
 
     init_clocks(qemu_timer_notify_cb);
 
-    ret = qemu_signal_init();
+    ret = qemu_signal_init(errp);
     if (ret) {
         return ret;
     }

Reviewed-by Markus Armbruster <[email protected]>

Fei Li

author

2018-11-29 06:17 UTC #3 lore raw

Re: [Qemu-devel] [PATCH RFC 1/5] Fix segmentation fault when qemu_signal_init fails

Quoted 8 lines · Markus Armbruster

Quoted 3 lines · Fei Li
When qemu_signal_init() fails in qemu_init_main_loop(), we return without setting an error. Its callers crash then when they try to report the error with error_report_err().
Yes, that's a bug. Broken in 2f78e491d7b, v2.2.0. Has escaped notice since qemu_signalfd() is quite unlikely to fail. Could go into 3.1 as a bug fix, but I think punting it to the next release is just fine.

Thanks. :) BTW, should I send the next version only includes patch 1/5 and 2/5 separately so that you can merge? (I guess Dave will help to merge the other three migration related patches)

Quoted 7 lines

Quoted 4 lines
To avoid such segmentation fault, add a new Error parameter to make the call trace to propagate the err to the final caller.
Let's add:
   Fixes: 2f78e491d7b46542158ce0b8132ee4e05bc0ade4

ok.

Quoted 42 lines

Cc Markus Armbruster <[email protected]>

Cc Fam Zheng <[email protected]>

Signed-off-by Fei Li <[email protected]>

Reviewed-by Fam Zheng <[email protected]>

---
  util/main-loop.c | 8 ++++----
  1 file changed, 4 insertions(+), 4 deletions(-)

util/main-loop.c +3 -3

index affe0403c5..443cb4cfe8 100644
@@ -71,7 +71,7 @@ static void sigfd_handler(void *opaque)
      }
  }
 
-static int qemu_signal_init(void)
+static int qemu_signal_init(Error **errp)
  {
      int sigfd;
      sigset_t set;

@@ -96,7 +96,7 @@ static int qemu_signal_init(void)
      sigdelset(&set, SIG_IPI);
      sigfd = qemu_signalfd(&set);
      if (sigfd == -1) {
-        fprintf(stderr, "failed to create signalfd\n");
+        error_setg_errno(errp, errno, "failed to create signalfd");
          return -errno;
      }

@@ -109,7 +109,7 @@ static int qemu_signal_init(void)
 
  #else /* _WIN32 */
 
-static int qemu_signal_init(void)
+static int qemu_signal_init(Error **errp)
  {
      return 0;
  }

@@ -148,7 +148,7 @@ int qemu_init_main_loop(Error **errp)

Quoted 8 lines

util/main-loop.c int qemu_init_main_loop(Error **errp) · line 154
Quoted 7 lines
util/main-loop.c +1 -1
      init_clocks(qemu_timer_notify_cb);
 
-    ret = qemu_signal_init();
+    ret = qemu_signal_init(errp);
      if (ret) {
          return ret;
      }
Reviewed-by Markus Armbruster <[email protected]>

Thanks again for the review.

Have a nice day
Fei

Markus Armbruster

2018-11-29 08:35 UTC #4 lore raw

Re: [Qemu-devel] [PATCH RFC 1/5] Fix segmentation fault when qemu_signal_init fails

Quoted 6 lines · Fei Li

Quoted 5 lines · Markus Armbruster
Quoted 3 lines · Fei Li
When qemu_signal_init() fails in qemu_init_main_loop(), we return without setting an error. Its callers crash then when they try to report the error with error_report_err().

Quoted 6 lines

Quoted 3 lines
Yes, that's a bug. Broken in 2f78e491d7b, v2.2.0. Has escaped notice since qemu_signalfd() is quite unlikely to fail. Could go into 3.1 as a bug fix, but I think punting it to the next release is just fine.
Thanks. :) BTW, should I send the next version only includes patch 1/5 and 2/5 separately so that you can merge? (I guess Dave will help to merge the other three migration related patches)

I can pick patches out of a series for merging, and I trust Dave can, too. But keeping unrelated fixes separate is a good idea. I can see three groups: PATCH 1 (main loop), PATCH 2 (thread abstraction), PATCH 3-5 (migration).

[...]

Fei Li

author

2018-11-29 09:15 UTC #5 lore raw

Re: [Qemu-devel] [PATCH RFC 1/5] Fix segmentation fault when qemu_signal_init fails

Quoted 14 lines · Markus Armbruster

Quoted 6 lines · Fei Li
Quoted 5 lines · Markus Armbruster
Quoted 3 lines · Fei Li
When qemu_signal_init() fails in qemu_init_main_loop(), we return without setting an error. Its callers crash then when they try to report the error with error_report_err().
Quoted 6 lines
Quoted 3 lines
Yes, that's a bug. Broken in 2f78e491d7b, v2.2.0. Has escaped notice since qemu_signalfd() is quite unlikely to fail. Could go into 3.1 as a bug fix, but I think punting it to the next release is just fine.
Thanks. :) BTW, should I send the next version only includes patch 1/5 and 2/5 separately so that you can merge? (I guess Dave will help to merge the other three migration related patches)

Quoted 6 lines

I can pick patches out of a series for merging, and I trust Dave can, too. But keeping unrelated fixes separate is a good idea. I can see three groups: PATCH 1 (main loop), PATCH 2 (thread abstraction), PATCH 3-5 (migration).
[...]

Ok, thanks for the detail explanation. I once thought the first two patches are belong to one group, that's why I tried to separate them. But considering too many groups they are belong to and maintainers are willing to pick, I'd like to keep sending them within one patch series in the next version. :)

Have a nice day, thanks a lot
Fei

Fei Li

author

2018-11-28 10:41 UTC #6 lore raw

RFC PATCH 2/5 qemu_thread_join: fix segmentation fault

In reply to Fei Li

To avoid the segmentation fault in qemu_thread_join(), just directly return when the QemuThread *thread failed to be created in either qemu-thread-posix.c or qemu-thread-win32.c.

Signed-off-by Fei Li <[email protected]>

Reviewed-by Fam Zheng <[email protected]>

---
 util/qemu-thread-posix.c | 3 +++
 util/qemu-thread-win32.c | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

util/qemu-thread-posix.c +3 -0

index 865e476df5..b9ab5a4711 100644
@@ -558,6 +558,9 @@ void *qemu_thread_join(QemuThread *thread)
     int err;
     void *ret;
 
+    if (!thread->thread) {
+        return NULL;
+    }
     err = pthread_join(thread->thread, &ret);
     if (err) {
         error_exit(err, __func__);

util/qemu-thread-win32.c +1 -1

index 4a363ca675..1a27e1cf6f 100644
@@ -366,7 +366,7 @@ void *qemu_thread_join(QemuThread *thread)
     HANDLE handle;
 
     data = thread->data;
-    if (data->mode == QEMU_THREAD_DETACHED) {
+    if (data == NULL || data->mode == QEMU_THREAD_DETACHED) {
         return NULL;
     }

Signature · 1 line

2.13.7

Philippe Mathieu-Daudé

2018-11-29 14:32 UTC #7 lore raw

Re: [Qemu-devel] [PATCH RFC 2/5] qemu_thread_join: fix segmentation fault

Hi Fei,

Quoted 30 lines · Fei Li

To avoid the segmentation fault in qemu_thread_join(), just directly return when the QemuThread *thread failed to be created in either qemu-thread-posix.c or qemu-thread-win32.c.
Signed-off-by Fei Li <[email protected]>
Reviewed-by Fam Zheng <[email protected]>
---
 util/qemu-thread-posix.c | 3 +++
 util/qemu-thread-win32.c | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)
util/qemu-thread-posix.c +3 -0
index 865e476df5..b9ab5a4711 100644
@@ -558,6 +558,9 @@ void *qemu_thread_join(QemuThread *thread)
     int err;
     void *ret;
 
+    if (!thread->thread) {
+        return NULL;
+    }
     err = pthread_join(thread->thread, &ret);
     if (err) {
         error_exit(err, __func__);
util/qemu-thread-win32.c +0 -0
index 4a363ca675..1a27e1cf6f 100644

util/qemu-thread-win32.c void *qemu_thread_join(QemuThread *thread) · line 369

Quoted 5 lines

util/qemu-thread-win32.c +1 -1

@@ -366,7 +366,7 @@ void *qemu_thread_join(QemuThread *thread)
     HANDLE handle;
 
     data = thread->data;
-    if (data->mode == QEMU_THREAD_DETACHED) {
+    if (data == NULL || data->mode == QEMU_THREAD_DETACHED) {

Isn't it a way to hide a problem from the caller?

qemu_thread_create() always initializes thread->data.

If the thread doesn't exist you shouldn't try to join() it.

If you try to fix a bug, I'd use here:

       assert(thread->data);

Regards,

Phil.

Quoted 4 lines

util/qemu-thread-win32.c +0 -0

         return NULL;
     }

Fei Li

author

2018-11-30 11:38 UTC #8 lore raw

Re: [Qemu-devel] [PATCH RFC 2/5] qemu_thread_join: fix segmentation fault

Quoted 44 lines · Philippe Mathieu-Daudé

Hi Fei,
Quoted 30 lines · Fei Li
To avoid the segmentation fault in qemu_thread_join(), just directly return when the QemuThread *thread failed to be created in either qemu-thread-posix.c or qemu-thread-win32.c.
Signed-off-by Fei Li <[email protected]>
Reviewed-by Fam Zheng <[email protected]>
---
  util/qemu-thread-posix.c | 3 +++
  util/qemu-thread-win32.c | 2 +-
  2 files changed, 4 insertions(+), 1 deletion(-)
util/qemu-thread-posix.c +3 -0
index 865e476df5..b9ab5a4711 100644
@@ -558,6 +558,9 @@ void *qemu_thread_join(QemuThread *thread)
      int err;
      void *ret;
 
+    if (!thread->thread) {
+        return NULL;
+    }
      err = pthread_join(thread->thread, &ret);
      if (err) {
          error_exit(err, __func__);
util/qemu-thread-win32.c +0 -0
index 4a363ca675..1a27e1cf6f 100644
util/qemu-thread-win32.c void *qemu_thread_join(QemuThread *thread) · line 369
Quoted 5 lines
util/qemu-thread-win32.c +1 -1
@@ -366,7 +366,7 @@ void *qemu_thread_join(QemuThread *thread)
      HANDLE handle;
 
      data = thread->data;
-    if (data->mode == QEMU_THREAD_DETACHED) {
+    if (data == NULL || data->mode == QEMU_THREAD_DETACHED) {
Isn't it a way to hide a problem from the caller?
qemu_thread_create() always initializes thread->data.
If the thread doesn't exist you shouldn't try to join() it.

Quoted 7 lines

If you try to fix a bug, I'd use here:
        assert(thread->data);
Regards,
Phil.

Emm, this patch is actually should not be abstracted separately... It should be along with the "[PATCH RFC v7 9/9] qemu_thread_create:
propagate errors to callers to check."

For the 9/9 patch, when qemu_thread_create() fails, we would like the callers to handle the error instead of directly abort in order to robust our qemu code (e.g. do not let qemu crash when fails to hot-plug a cpu). And only within that patch, we need to check whether the thread has been
created: if not just return NULL, which means no handling for that thread. This is because for many cleanups, we just use a loop to join() all the thread_count. e.g. when compress_threads_save_setup() fails while trying to create the second/third do_data_compress thread. (Supposing there are more than three compress threads).

I will remove this patch in next version. Thanks for pointing this out. :)

Have a nice day
Fei

Quoted 6 lines

Quoted 5 lines
util/qemu-thread-win32.c +0 -0
          return NULL;
      }
 

Fei Li

author

2018-11-28 10:41 UTC #9 lore raw

RFC PATCH 3/5 migration: fix the multifd code when receiving less channels

In reply to Fei Li

In our current code, when multifd is used during migration, if there is an error before the destination receives all new channels, the source keeps running, however the destination does not exit but keeps waiting until the source is killed deliberately.

Fix this by dumping the specific error and let users decide whether to quit from the destination side when failing to receive packet via some channel.

Cc Dr. David Alan Gilbert <[email protected]>

Cc Peter Xu <[email protected]>

Signed-off-by Fei Li <[email protected]>

---
 migration/channel.c   | 11 ++++++-----
 migration/migration.c |  9 +++++++--
 migration/migration.h |  2 +-
 migration/ram.c       |  7 ++++++-
 migration/ram.h       |  2 +-
 5 files changed, 21 insertions(+), 10 deletions(-)

migration/channel.c +6 -5

index 33e0e9b82f..20e4c8e2dc 100644
@@ -30,6 +30,7 @@
 void migration_channel_process_incoming(QIOChannel *ioc)
 {
     MigrationState *s = migrate_get_current();
+    Error *local_err = NULL;
 
     trace_migration_set_incoming_channel(
         ioc, object_get_typename(OBJECT(ioc)));

@@ -38,13 +39,13 @@ void migration_channel_process_incoming(QIOChannel *ioc)
         *s->parameters.tls_creds &&
         !object_dynamic_cast(OBJECT(ioc),
                              TYPE_QIO_CHANNEL_TLS)) {
-        Error *local_err = NULL;
         migration_tls_channel_process_incoming(s, ioc, &local_err);
-        if (local_err) {
-            error_report_err(local_err);
-        }
     } else {
-        migration_ioc_process_incoming(ioc);
+        migration_ioc_process_incoming(ioc, &local_err);
+    }
+
+    if (local_err) {
+        error_report_err(local_err);
     }
 }

migration/migration.c +7 -2

index 49ffb9997a..72106bddf0 100644
@@ -541,7 +541,7 @@ void migration_fd_process_incoming(QEMUFile *f)
     migration_incoming_process();
 }
 
-void migration_ioc_process_incoming(QIOChannel *ioc)
+void migration_ioc_process_incoming(QIOChannel *ioc, Error **errp)
 {
     MigrationIncomingState *mis = migration_incoming_get_current();
     bool start_migration;

@@ -563,9 +563,14 @@ void migration_ioc_process_incoming(QIOChannel *ioc)
          */
         start_migration = !migrate_use_multifd();
     } else {
+        Error *local_err = NULL;
         /* Multiple connections */
         assert(migrate_use_multifd());
-        start_migration = multifd_recv_new_channel(ioc);
+        start_migration = multifd_recv_new_channel(ioc, &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            return;
+        }
     }
 
     if (start_migration) {

migration/migration.h +1 -1

index e413d4d8b6..02b7304610 100644
@@ -229,7 +229,7 @@ struct MigrationState
 void migrate_set_state(int *state, int old_state, int new_state);
 
 void migration_fd_process_incoming(QEMUFile *f);
-void migration_ioc_process_incoming(QIOChannel *ioc);
+void migration_ioc_process_incoming(QIOChannel *ioc, Error **errp);
 void migration_incoming_process(void);
 
 bool  migration_has_all_channels(void);

migration/ram.c +6 -1

index 7e7deec4d8..e13b9349d0 100644
@@ -1323,7 +1323,7 @@ bool multifd_recv_all_channels_created(void)
 }
 
 /* Return true if multifd is ready for the migration, otherwise false */
-bool multifd_recv_new_channel(QIOChannel *ioc)
+bool multifd_recv_new_channel(QIOChannel *ioc, Error **errp)
 {
     MultiFDRecvParams *p;
     Error *local_err = NULL;

@@ -1331,6 +1331,10 @@ bool multifd_recv_new_channel(QIOChannel *ioc)
 
     id = multifd_recv_initial_packet(ioc, &local_err);
     if (id < 0) {
+        error_propagate_prepend(errp, local_err,
+                                "failed to receive packet"
+                                " via multifd channel %d: ",
+                                multifd_recv_state->count);
         multifd_recv_terminate_threads(local_err);
         return false;
     }

@@ -1340,6 +1344,7 @@ bool multifd_recv_new_channel(QIOChannel *ioc)
         error_setg(&local_err, "multifd: received id '%d' already setup'",
                    id);
         multifd_recv_terminate_threads(local_err);
+        error_propagate(errp, local_err);
         return false;
     }
     p->c = ioc;

migration/ram.h +1 -1

index 83ff1bc11a..046d3074be 100644
@@ -47,7 +47,7 @@ int multifd_save_cleanup(Error **errp);
 int multifd_load_setup(void);
 int multifd_load_cleanup(Error **errp);
 bool multifd_recv_all_channels_created(void);
-bool multifd_recv_new_channel(QIOChannel *ioc);
+bool multifd_recv_new_channel(QIOChannel *ioc, Error **errp);
 
 uint64_t ram_pagesize_summary(void);
 int ram_save_queue_pages(const char *rbname, ram_addr_t start, ram_addr_t len);

Signature · 1 line

2.13.7

Peter Xu

Reviewed-by on v1 3/5

2018-11-28 12:20 UTC #10 lore raw

Re: [Qemu-devel] [PATCH RFC 3/5] migration: fix the multifd code when receiving less channels

Quoted 5 lines · Fei Li

In our current code, when multifd is used during migration, if there is an error before the destination receives all new channels, the source keeps running, however the destination does not exit but keeps waiting until the source is killed deliberately.

Quoted 7 lines

Fix this by dumping the specific error and let users decide whether to quit from the destination side when failing to receive packet via some channel.
Cc Dr. David Alan Gilbert <[email protected]>
Cc Peter Xu <[email protected]>
Signed-off-by Fei Li <[email protected]>

Reviewed-by Peter Xu <[email protected]>

Signature · 1 line

Peter Xu

Fei Li

author

2018-11-28 10:41 UTC #11 lore raw

RFC PATCH 4/5 migration: remove unused &local_err parameter in multifd_save_cleanup

In reply to Fei Li

Always call migrate_set_error() to set the error state without relying on whether multifd_save_cleanup() succeeds. As the passed &local_err is never used in multifd_save_cleanup(), remove it. And make the function be: void multifd_save_cleanup(void).

Signed-off-by Fei Li <[email protected]>

Reviewed-by Juan Quintela <[email protected]>

---
 migration/migration.c |  5 +----
 migration/ram.c       | 11 ++++-------
 migration/ram.h       |  2 +-
 3 files changed, 6 insertions(+), 12 deletions(-)

migration/migration.c +1 -4

index 72106bddf0..0537fc0c26 100644
@@ -1386,7 +1386,6 @@ static void migrate_fd_cleanup(void *opaque)
     qemu_savevm_state_cleanup();
 
     if (s->to_dst_file) {
-        Error *local_err = NULL;
         QEMUFile *tmp;
 
         trace_migrate_fd_cleanup();

@@ -1397,9 +1396,7 @@ static void migrate_fd_cleanup(void *opaque)
         }
         qemu_mutex_lock_iothread();
 
-        if (multifd_save_cleanup(&local_err) != 0) {
-            error_report_err(local_err);
-        }
+        multifd_save_cleanup();
         qemu_mutex_lock(&s->qemu_file_lock);
         tmp = s->to_dst_file;
         s->to_dst_file = NULL;

migration/ram.c +4 -7

index e13b9349d0..c44cb6f56d 100644
@@ -917,13 +917,12 @@ static void multifd_send_terminate_threads(Error *err)
     }
 }
 
-int multifd_save_cleanup(Error **errp)
+void multifd_save_cleanup(void)
 {
     int i;
-    int ret = 0;
 
     if (!migrate_use_multifd()) {
-        return 0;
+        return;
     }
     multifd_send_terminate_threads(NULL);
     for (i = 0; i < migrate_multifd_channels(); i++) {

@@ -953,7 +952,6 @@ int multifd_save_cleanup(Error **errp)
     multifd_send_state->pages = NULL;
     g_free(multifd_send_state);
     multifd_send_state = NULL;
-    return ret;
 }
 
 static void multifd_send_sync_main(void)

@@ -1071,9 +1069,8 @@ static void multifd_new_send_channel_async(QIOTask *task, gpointer opaque)
     Error *local_err = NULL;
 
     if (qio_task_propagate_error(task, &local_err)) {
-        if (multifd_save_cleanup(&local_err) != 0) {
-            migrate_set_error(migrate_get_current(), local_err);
-        }
+        migrate_set_error(migrate_get_current(), local_err);
+        multifd_save_cleanup();
     } else {
         p->c = QIO_CHANNEL(sioc);
         qio_channel_set_delay(p->c, false);

migration/ram.h +1 -1

index 046d3074be..936177b3e9 100644
@@ -43,7 +43,7 @@ uint64_t ram_bytes_remaining(void);
 uint64_t ram_bytes_total(void);
 
 int multifd_save_setup(void);
-int multifd_save_cleanup(Error **errp);
+void multifd_save_cleanup(void);
 int multifd_load_setup(void);
 int multifd_load_cleanup(Error **errp);
 bool multifd_recv_all_channels_created(void);

Signature · 1 line

2.13.7

Fei Li

author

2018-11-28 10:41 UTC #12 lore raw

RFC PATCH 5/5 migration: add more error handling for postcopy_ram_enable_notify

In reply to Fei Li

Call postcopy_ram_incoming_cleanup() to do the cleanup when postcopy_ram_enable_notify fails. Besides, report the error message when qemu_ram_foreach_migratable_block() fails.

Cc Dr. David Alan Gilbert <[email protected]>

Signed-off-by Fei Li <[email protected]>

---
 migration/postcopy-ram.c | 1 +
 migration/savevm.c       | 1 +
 2 files changed, 2 insertions(+)

migration/postcopy-ram.c +1 -0

index e5c02a32c5..fa09dba534 100644
@@ -1117,6 +1117,7 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis)
 
     /* Mark so that we get notified of accesses to unwritten areas */
     if (qemu_ram_foreach_migratable_block(ram_block_enable_notify, mis)) {
+        error_report("ram_block_enable_notify failed");
         return -1;
     }

migration/savevm.c +1 -0

index ef707b8c43..0d76232931 100644
@@ -1728,6 +1728,7 @@ static int loadvm_postcopy_handle_listen(MigrationIncomingState *mis)
      */
     if (migrate_postcopy_ram()) {
         if (postcopy_ram_enable_notify(mis)) {
+            postcopy_ram_incoming_cleanup(mis);
             return -1;
         }
     }

Signature · 1 line

2.13.7