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

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* zhanghailiang (zhang.zhanghailiang@huawei.com) 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 <zhang.zhanghailiang@huawei.com>

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

You should split this patch out and submit it by itself; it can go in
without waiting for any other COLO bits.

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 / dgilbert@redhat.com / Manchester, UK