Re: [Qemu-devel] [PATCH for-4.0 v9 05/16] migration: unify error handling for process_incoming_migration_co

[Fei Li]

在 2019/1/3 下午7:25, Dr. David Alan Gilbert 写道:

* Fei Li (f...@suse.com) wrote:

In the current code, if process_incoming_migration_co() fails we do
the same error handing: set the error state, close the source file,
do the cleanup for multifd, and then exit(EXIT_FAILURE). To make the
code clearer, add a "goto fail" to unify the error handling.

Cc: Markus Armbruster <arm...@redhat.com>
Cc: Dr. David Alan Gilbert <dgilb...@redhat.com>
Cc: Peter Xu <pet...@redhat.com>
Signed-off-by: Fei Li <f...@suse.com>
---
  migration/migration.c | 26 +++++++++++++-------------
  1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 5d322eb9d6..ded151b1bf 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -438,15 +438,13 @@ static void process_incoming_migration_co(void *opaque)
          /* Make sure all file formats flush their mutable metadata */
          bdrv_invalidate_cache_all(&local_err);
          if (local_err) {
-            migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
-                    MIGRATION_STATUS_FAILED);
              error_report_err(local_err);
-            exit(EXIT_FAILURE);
+            goto fail;
          }

if (colo_init_ram_cache() < 0) {

              error_report("Init ram cache failed");
-            exit(EXIT_FAILURE);
+            goto fail;
          }

qemu_thread_create(&mis->colo_incoming_thread, "COLO incoming",

@@ -461,20 +459,22 @@ static void process_incoming_migration_co(void *opaque)
      }

if (ret < 0) {

-        Error *local_err = NULL;
-
-        migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
-                          MIGRATION_STATUS_FAILED);
          error_report("load of migration failed: %s", strerror(-ret));
-        qemu_fclose(mis->from_src_file);
-        if (multifd_load_cleanup(&local_err) != 0) {
-            error_report_err(local_err);
-        }
-        exit(EXIT_FAILURE);
+        goto fail;
      }
      mis->bh = qemu_bh_new(process_incoming_migration_bh, mis);
      qemu_bh_schedule(mis->bh);
      mis->migration_incoming_co = NULL;
+    return;
+fail:
+    local_err = NULL;
+    migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
+                      MIGRATION_STATUS_FAILED);
+    qemu_fclose(mis->from_src_file);
+    if (multifd_load_cleanup(&local_err) != 0) {
+        error_report_err(local_err);
+    }
+    exit(EXIT_FAILURE);
  }

static void migration_incoming_setup(QEMUFile *f)

OK, so this is really unifying the normal error case and the two
colo-incoming error cases; so I think that's fine.


Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com>

Thanks for the review :)

Fei

--
2.13.7

--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK