Re: [Qemu-devel] [PATCH COLO-Frame v9 04/32] migration: Add state records for migration incoming

[Dr. David Alan Gilbert]

* zhanghailiang (address@hidden) wrote:
> On 2015/10/10 0:18, Dr. David Alan Gilbert wrote:
> >* zhanghailiang (address@hidden) wrote:
> >>For migration destination, we also need to know its state,
> >>we will use it in COLO.
> >>
> >>Here we add a new member 'state' for MigrationIncomingState,
> >>and also use migrate_set_state() to modify its value.
> >>We fix the first parameter of migrate_set_state(), and make it
> >>public.
> >>
> >>Signed-off-by: zhanghailiang <address@hidden>
> >
> >Reviewed-by: Dr. David Alan Gilbert <address@hidden>
> >
> >You should split this patch out and submit it by itself; it can go in
> >without waiting for any other COLO bits.
> >
> 
> Ok, will do that.
> Besides,  there is still an independent patch in this series.
> "[PATCH COLO-Frame v9 07/32] migration: Rename the'file' member of 
> MigrationState and
> MigrationIncomingState", i know you have done the same thing in your postcopy 
> series.
> (I noticed that you didn't change the 'file' member in MigartionState ...)
> Could you send that modification as an independent series ? So that i can 
> drop that patch in
> next version.
> BTW, is there any plan to merge your postcopy series ?

Hopefully, it's waiting for review, but you know that can take some time.

Dave

> 
> Thanks,
> zhanghailiang
> 
> >Dave
> >
> >>---
> >>  include/migration/migration.h |  3 +++
> >>  migration/migration.c         | 38 +++++++++++++++++++++++---------------
> >>  2 files changed, 26 insertions(+), 15 deletions(-)
> >>
> >>diff --git a/include/migration/migration.h b/include/migration/migration.h
> >>index 05de3a1..a62068f 100644
> >>--- a/include/migration/migration.h
> >>+++ b/include/migration/migration.h
> >>@@ -50,6 +50,7 @@ typedef QLIST_HEAD(, LoadStateEntry) LoadStateEntry_Head;
> >>  struct MigrationIncomingState {
> >>      QEMUFile *file;
> >>
> >>+    int state;
> >>      /* See savevm.c */
> >>      LoadStateEntry_Head loadvm_handlers;
> >>  };
> >>@@ -82,6 +83,8 @@ struct MigrationState
> >>      int64_t dirty_sync_count;
> >>  };
> >>
> >>+void migrate_set_state(int *state, int old_state, int new_state);
> >>+
> >>  void process_incoming_migration(QEMUFile *f);
> >>
> >>  void qemu_start_incoming_migration(const char *uri, Error **errp);
> >>diff --git a/migration/migration.c b/migration/migration.c
> >>index 593cac0..98133f1 100644
> >>--- a/migration/migration.c
> >>+++ b/migration/migration.c
> >>@@ -89,6 +89,7 @@ MigrationIncomingState 
> >>*migration_incoming_state_new(QEMUFile* f)
> >>  {
> >>      mis_current = g_malloc0(sizeof(MigrationIncomingState));
> >>      mis_current->file = f;
> >>+    mis_current->state = MIGRATION_STATUS_NONE;
> >>      QLIST_INIT(&mis_current->loadvm_handlers);
> >>
> >>      return mis_current;
> >>@@ -270,11 +271,13 @@ void qemu_start_incoming_migration(const char *uri, 
> >>Error **errp)
> >>  static void process_incoming_migration_co(void *opaque)
> >>  {
> >>      QEMUFile *f = opaque;
> >>+    MigrationIncomingState *mis;
> >>      Error *local_err = NULL;
> >>      int ret;
> >>
> >>-    migration_incoming_state_new(f);
> >>-    migrate_generate_event(MIGRATION_STATUS_ACTIVE);
> >>+    mis = migration_incoming_state_new(f);
> >>+    migrate_set_state(&mis->state, MIGRATION_STATUS_NONE,
> >>+                      MIGRATION_STATUS_ACTIVE);
> >>      ret = qemu_loadvm_state(f);
> >>
> >>      qemu_fclose(f);
> >>@@ -282,12 +285,14 @@ static void process_incoming_migration_co(void 
> >>*opaque)
> >>      migration_incoming_state_destroy();
> >>
> >>      if (ret < 0) {
> >>-        migrate_generate_event(MIGRATION_STATUS_FAILED);
> >>+        migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
> >>+                          MIGRATION_STATUS_FAILED);
> >>          error_report("load of migration failed: %s", strerror(-ret));
> >>          migrate_decompress_threads_join();
> >>          exit(EXIT_FAILURE);
> >>      }
> >>-    migrate_generate_event(MIGRATION_STATUS_COMPLETED);
> >>+    migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
> >>+                      MIGRATION_STATUS_COMPLETED);
> >>      qemu_announce_self();
> >>
> >>      /* Make sure all file formats flush their mutable metadata */
> >>@@ -543,9 +548,9 @@ void qmp_migrate_set_parameters(bool has_compress_level,
> >>
> >>  /* shared migration helpers */
> >>
> >>-static void migrate_set_state(MigrationState *s, int old_state, int 
> >>new_state)
> >>+void migrate_set_state(int *state, int old_state, int new_state)
> >>  {
> >>-    if (atomic_cmpxchg(&s->state, old_state, new_state) == old_state) {
> >>+    if (atomic_cmpxchg(state, old_state, new_state) == old_state) {
> >>          trace_migrate_set_state(new_state);
> >>          migrate_generate_event(new_state);
> >>      }
> >>@@ -574,7 +579,7 @@ static void migrate_fd_cleanup(void *opaque)
> >>      if (s->state != MIGRATION_STATUS_COMPLETED) {
> >>          qemu_savevm_state_cancel();
> >>          if (s->state == MIGRATION_STATUS_CANCELLING) {
> >>-            migrate_set_state(s, MIGRATION_STATUS_CANCELLING,
> >>+            migrate_set_state(&s->state, MIGRATION_STATUS_CANCELLING,
> >>                                MIGRATION_STATUS_CANCELLED);
> >>          }
> >>      }
> >>@@ -586,7 +591,8 @@ void migrate_fd_error(MigrationState *s)
> >>  {
> >>      trace_migrate_fd_error();
> >>      assert(s->file == NULL);
> >>-    migrate_set_state(s, MIGRATION_STATUS_SETUP, MIGRATION_STATUS_FAILED);
> >>+    migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
> >>+                      MIGRATION_STATUS_FAILED);
> >>      notifier_list_notify(&migration_state_notifiers, s);
> >>  }
> >>
> >>@@ -602,7 +608,7 @@ static void migrate_fd_cancel(MigrationState *s)
> >>              old_state != MIGRATION_STATUS_ACTIVE) {
> >>              break;
> >>          }
> >>-        migrate_set_state(s, old_state, MIGRATION_STATUS_CANCELLING);
> >>+        migrate_set_state(&s->state, old_state, 
> >>MIGRATION_STATUS_CANCELLING);
> >>      } while (s->state != MIGRATION_STATUS_CANCELLING);
> >>
> >>      /*
> >>@@ -670,7 +676,7 @@ static MigrationState *migrate_init(const 
> >>MigrationParams *params)
> >>      s->parameters[MIGRATION_PARAMETER_DECOMPRESS_THREADS] =
> >>                 decompress_thread_count;
> >>      s->bandwidth_limit = bandwidth_limit;
> >>-    migrate_set_state(s, MIGRATION_STATUS_NONE, MIGRATION_STATUS_SETUP);
> >>+    migrate_set_state(&s->state, MIGRATION_STATUS_NONE, 
> >>MIGRATION_STATUS_SETUP);
> >>
> >>      s->total_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
> >>      return s;
> >>@@ -768,7 +774,8 @@ void qmp_migrate(const char *uri, bool has_blk, bool 
> >>blk,
> >>      } else {
> >>          error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "uri",
> >>                     "a valid migration protocol");
> >>-        migrate_set_state(s, MIGRATION_STATUS_SETUP, 
> >>MIGRATION_STATUS_FAILED);
> >>+        migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
> >>+                          MIGRATION_STATUS_FAILED);
> >>          return;
> >>      }
> >>
> >>@@ -948,7 +955,8 @@ static void *migration_thread(void *opaque)
> >>      qemu_savevm_state_begin(s->file, &s->params);
> >>
> >>      s->setup_time = qemu_clock_get_ms(QEMU_CLOCK_HOST) - setup_start;
> >>-    migrate_set_state(s, MIGRATION_STATUS_SETUP, MIGRATION_STATUS_ACTIVE);
> >>+    migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
> >>+                      MIGRATION_STATUS_ACTIVE);
> >>
> >>      while (s->state == MIGRATION_STATUS_ACTIVE) {
> >>          int64_t current_time;
> >>@@ -978,13 +986,13 @@ static void *migration_thread(void *opaque)
> >>                  qemu_mutex_unlock_iothread();
> >>
> >>                  if (ret < 0) {
> >>-                    migrate_set_state(s, MIGRATION_STATUS_ACTIVE,
> >>+                    migrate_set_state(&s->state, MIGRATION_STATUS_ACTIVE,
> >>                                        MIGRATION_STATUS_FAILED);
> >>                      break;
> >>                  }
> >>
> >>                  if (!qemu_file_get_error(s->file)) {
> >>-                    migrate_set_state(s, MIGRATION_STATUS_ACTIVE,
> >>+                    migrate_set_state(&s->state, MIGRATION_STATUS_ACTIVE,
> >>                                        MIGRATION_STATUS_COMPLETED);
> >>                      break;
> >>                  }
> >>@@ -992,7 +1000,7 @@ static void *migration_thread(void *opaque)
> >>          }
> >>
> >>          if (qemu_file_get_error(s->file)) {
> >>-            migrate_set_state(s, MIGRATION_STATUS_ACTIVE,
> >>+            migrate_set_state(&s->state, MIGRATION_STATUS_ACTIVE,
> >>                                MIGRATION_STATUS_FAILED);
> >>              break;
> >>          }
> >>--
> >>1.8.3.1
> >>
> >>
> >--
> >Dr. David Alan Gilbert / address@hidden / Manchester, UK
> >
> >.
> >
> 
> 
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK