qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH for-4.0 v9 13/16] qemu_thread: supplement error


From: Fei Li
Subject: Re: [Qemu-devel] [PATCH for-4.0 v9 13/16] qemu_thread: supplement error handling for migration
Date: Thu, 3 Jan 2019 20:47:07 +0800
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:60.0) Gecko/20100101 Thunderbird/60.4.0


在 2019/1/3 下午8:35, Dr. David Alan Gilbert 写道:
* Fei Li (address@hidden) wrote:
Update qemu_thread_create()'s callers by
- setting an error on qemu_thread_create() failure for callers that
   set an error on failure;
- reporting the error and returning failure for callers that return
   an error code on failure;
- reporting the error and setting some state for callers that just
   report errors and choose not to continue on.

Cc: Markus Armbruster <address@hidden>
Cc: Dr. David Alan Gilbert <address@hidden>
Cc: Peter Xu <address@hidden>
Signed-off-by: Fei Li <address@hidden>
---
  migration/migration.c    | 33 ++++++++++++++++++++++-----------
  migration/postcopy-ram.c | 16 ++++++++++++----
  migration/ram.c          | 44 ++++++++++++++++++++++++++++++--------------
  migration/savevm.c       | 12 ++++++++----
  4 files changed, 72 insertions(+), 33 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index ea5839ff0d..9654bde101 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -447,10 +447,13 @@ static void process_incoming_migration_co(void *opaque)
              goto fail;
          }
- /* TODO: let the further caller handle the error instead of abort() */
-        qemu_thread_create(&mis->colo_incoming_thread, "COLO incoming",
-                           colo_process_incoming_thread, mis,
-                           QEMU_THREAD_JOINABLE, &error_abort);
+        if (!qemu_thread_create(&mis->colo_incoming_thread, "COLO incoming",
+                                colo_process_incoming_thread, mis,
+                                QEMU_THREAD_JOINABLE, &local_err)) {
+            error_reportf_err(local_err, "failed to create "
+                              "colo_process_incoming_thread: ");
+            goto fail;
+        }
          mis->have_colo_incoming_thread = true;
          qemu_coroutine_yield();
OK

@@ -2347,6 +2350,7 @@ out:
  static int open_return_path_on_source(MigrationState *ms,
                                        bool create_thread)
  {
+    Error *local_err = NULL;
ms->rp_state.from_dst_file = qemu_file_get_return_path(ms->to_dst_file);
      if (!ms->rp_state.from_dst_file) {
@@ -2360,10 +2364,13 @@ static int open_return_path_on_source(MigrationState 
*ms,
          return 0;
      }
- /* TODO: let the further caller handle the error instead of abort() here */
-    qemu_thread_create(&ms->rp_state.rp_thread, "return path",
-                       source_return_path_thread, ms,
-                       QEMU_THREAD_JOINABLE, &error_abort);
+    if (!qemu_thread_create(&ms->rp_state.rp_thread, "return path",
+                            source_return_path_thread, ms,
+                            QEMU_THREAD_JOINABLE, &local_err)) {
+        error_reportf_err(local_err,
+                          "failed to create source_return_path_thread: ");
+        return -1;
+     }
I think that has to close the from_dst_file and set the
from_dst_file=NULL.  That file is owned by the thread, and it's normally
the thread that cleans it up.

I think other than that missing close it's fine; and we can do that as a
fix later, so:


Reviewed-by: Dr. David Alan Gilbert <address@hidden>

Ok, I will add the cleanup for the from_dst_file in the next version.

Thanks for the review! Have a nice day :)

Fei


trace_open_return_path_on_source_continue(); @@ -3193,9 +3200,13 @@ void migrate_fd_connect(MigrationState *s, Error *error_in)
          migrate_fd_cleanup(s);
          return;
      }
-    /* TODO: let the further caller handle the error instead of abort() here */
-    qemu_thread_create(&s->thread, "live_migration", migration_thread, s,
-                       QEMU_THREAD_JOINABLE, &error_abort);
+    if (!qemu_thread_create(&s->thread, "live_migration", migration_thread, s,
+                            QEMU_THREAD_JOINABLE, &error_in)) {
+        error_reportf_err(error_in, "failed to create migration_thread: ");
+        migrate_set_state(&s->state, s->state, MIGRATION_STATUS_FAILED);
+        migrate_fd_cleanup(s);
+        return;
+    }
OK

      s->migration_thread_running = true;
  }
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 221ea24919..80bfa9c4a2 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -1083,6 +1083,8 @@ retry:
int postcopy_ram_enable_notify(MigrationIncomingState *mis)
  {
+    Error *local_err = NULL;
+
      /* Open the fd for the kernel to give us userfaults */
      mis->userfault_fd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK);
      if (mis->userfault_fd == -1) {
@@ -1109,10 +1111,16 @@ int postcopy_ram_enable_notify(MigrationIncomingState 
*mis)
      }
qemu_sem_init(&mis->fault_thread_sem, 0);
-    /* TODO: let the further caller handle the error instead of abort() here */
-    qemu_thread_create(&mis->fault_thread, "postcopy/fault",
-                       postcopy_ram_fault_thread, mis,
-                       QEMU_THREAD_JOINABLE, &error_abort);
+    if (!qemu_thread_create(&mis->fault_thread, "postcopy/fault",
+                            postcopy_ram_fault_thread, mis,
+                            QEMU_THREAD_JOINABLE, &local_err)) {
+        error_reportf_err(local_err,
+                          "failed to create postcopy_ram_fault_thread: ");
+        close(mis->userfault_event_fd);
+        close(mis->userfault_fd);
+        qemu_sem_destroy(&mis->fault_thread_sem);
+        return -1;
+    }
      qemu_sem_wait(&mis->fault_thread_sem);
      qemu_sem_destroy(&mis->fault_thread_sem);
      mis->have_fault_thread = true;
OK

diff --git a/migration/ram.c b/migration/ram.c
index eed1daf302..1e24a78eaa 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -473,6 +473,7 @@ static void compress_threads_save_cleanup(void)
  static int compress_threads_save_setup(void)
  {
      int i, thread_count;
+    Error *local_err = NULL;
if (!migrate_use_compression()) {
          return 0;
@@ -502,10 +503,12 @@ static int compress_threads_save_setup(void)
          comp_param[i].quit = false;
          qemu_mutex_init(&comp_param[i].mutex);
          qemu_cond_init(&comp_param[i].cond);
-        /* TODO: let the further caller handle the error instead of abort() */
-        qemu_thread_create(compress_threads + i, "compress",
-                           do_data_compress, comp_param + i,
-                           QEMU_THREAD_JOINABLE, &error_abort);
+        if (!qemu_thread_create(compress_threads + i, "compress",
+                                do_data_compress, comp_param + i,
+                                QEMU_THREAD_JOINABLE, &local_err)) {
+            error_reportf_err(local_err, "failed to create do_data_compress: 
");
+            goto exit;
+        }
OK

      }
      return 0;
@@ -1076,9 +1079,14 @@ static void multifd_new_send_channel_async(QIOTask *task, gpointer opaque)
          p->c = QIO_CHANNEL(sioc);
          qio_channel_set_delay(p->c, false);
          p->running = true;
-        /* TODO: let the further caller handle the error instead of abort() */
-        qemu_thread_create(&p->thread, p->name, multifd_send_thread, p,
-                           QEMU_THREAD_JOINABLE, &error_abort);
+        if (!qemu_thread_create(&p->thread, p->name, multifd_send_thread, p,
+                                QEMU_THREAD_JOINABLE, &local_err)) {
+            migrate_set_error(migrate_get_current(), local_err);
+            error_reportf_err(local_err,
+                              "failed to create multifd_send_thread: ");
+            multifd_save_cleanup();
+            return;
+        }
atomic_inc(&multifd_send_state->count);
      }
@@ -1357,9 +1365,13 @@ bool multifd_recv_new_channel(QIOChannel *ioc, Error 
**errp)
      p->num_packets = 1;
p->running = true;
-    /* TODO: let the further caller handle the error instead of abort() here */
-    qemu_thread_create(&p->thread, p->name, multifd_recv_thread, p,
-                       QEMU_THREAD_JOINABLE, &error_abort);
+    if (!qemu_thread_create(&p->thread, p->name, multifd_recv_thread, p,
+                            QEMU_THREAD_JOINABLE, &local_err)) {
+        error_propagate_prepend(errp, local_err,
+                                "failed to create multifd_recv_thread: ");
+        multifd_recv_terminate_threads(local_err);
+        return false;
+    }
      atomic_inc(&multifd_recv_state->count);
      return atomic_read(&multifd_recv_state->count) ==
             migrate_multifd_channels();
@@ -3625,6 +3637,7 @@ static void compress_threads_load_cleanup(void)
  static int compress_threads_load_setup(QEMUFile *f)
  {
      int i, thread_count;
+    Error *local_err = NULL;
if (!migrate_use_compression()) {
          return 0;
@@ -3646,10 +3659,13 @@ static int compress_threads_load_setup(QEMUFile *f)
          qemu_cond_init(&decomp_param[i].cond);
          decomp_param[i].done = true;
          decomp_param[i].quit = false;
-        /* TODO: let the further caller handle the error instead of abort() */
-        qemu_thread_create(decompress_threads + i, "decompress",
-                           do_data_decompress, decomp_param + i,
-                           QEMU_THREAD_JOINABLE, &error_abort);
+        if (!qemu_thread_create(decompress_threads + i, "decompress",
+                                do_data_decompress, decomp_param + i,
+                                QEMU_THREAD_JOINABLE, &local_err)) {
+            error_reportf_err(local_err,
+                              "failed to create do_data_decompress: ");
+            goto exit;
+        }
      }
      return 0;
  exit:
diff --git a/migration/savevm.c b/migration/savevm.c
index 46ce7af239..b8bdcde5d8 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1747,10 +1747,14 @@ static int 
loadvm_postcopy_handle_listen(MigrationIncomingState *mis)
      mis->have_listen_thread = true;
      /* Start up the listening thread and wait for it to signal ready */
      qemu_sem_init(&mis->listen_thread_sem, 0);
-    /* TODO: let the further caller handle the error instead of abort() here */
-    qemu_thread_create(&mis->listen_thread, "postcopy/listen",
-                       postcopy_ram_listen_thread, NULL,
-                       QEMU_THREAD_DETACHED, &error_abort);
+    if (!qemu_thread_create(&mis->listen_thread, "postcopy/listen",
+                            postcopy_ram_listen_thread, NULL,
+                            QEMU_THREAD_DETACHED, &local_err)) {
+        error_reportf_err(local_err,
+                          "failed to create postcopy_ram_listen_thread: ");
+        qemu_sem_destroy(&mis->listen_thread_sem);
+        return -1;
+    }
      qemu_sem_wait(&mis->listen_thread_sem);
      qemu_sem_destroy(&mis->listen_thread_sem);
--
2.13.7

--
Dr. David Alan Gilbert / address@hidden / Manchester, UK




reply via email to

[Prev in Thread] Current Thread [Next in Thread]