Re: [Qemu-devel] [PATCH] migrtion: define MigrationState/MigrationIncomingState.state as MigrationStatus

[Dr. David Alan Gilbert]

* Wei Yang (address@hidden) wrote:
> No functional change. Add default case to fix warning.

I think the problem with this is that migrate_set_state uses an
atomic_cmpxchg and so we have to be careful that the type we use
is compatible with that.
MigrationStatus is an enum and I think compilers are allowed to
choose the types of that;  so I'm not sure we're guaranteed
that an enum is always OK for the atomic_cmpxchg, and if it is
then we also might have to make the old_state and new_state
variables match.

Dave

> Signed-off-by: Wei Yang <address@hidden>
> ---
>  migration/migration.c | 8 +++++++-
>  migration/migration.h | 6 +++---
>  2 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 2865ae3fa9..0fd2364961 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -946,6 +946,8 @@ static void fill_source_migration_info(MigrationInfo 
> *info)
>      case MIGRATION_STATUS_CANCELLED:
>          info->has_status = true;
>          break;
> +    default:
> +        return;
>      }
>      info->status = s->state;
>  }
> @@ -1054,6 +1056,8 @@ static void 
> fill_destination_migration_info(MigrationInfo *info)
>          info->has_status = true;
>          fill_destination_postcopy_migration_info(info);
>          break;
> +    default:
> +        return;
>      }
>      info->status = mis->state;
>  }
> @@ -1446,7 +1450,7 @@ void qmp_migrate_start_postcopy(Error **errp)
>  
>  /* shared migration helpers */
>  
> -void migrate_set_state(int *state, int old_state, int new_state)
> +void migrate_set_state(MigrationStatus *state, int old_state, int new_state)
>  {
>      assert(new_state < MIGRATION_STATUS__MAX);
>      if (atomic_cmpxchg(state, old_state, new_state) == old_state) {
> @@ -1683,6 +1687,8 @@ bool migration_is_idle(void)
>          return false;
>      case MIGRATION_STATUS__MAX:
>          g_assert_not_reached();
> +    default:
> +        g_assert_not_reached();
>      }
>  
>      return false;
> diff --git a/migration/migration.h b/migration/migration.h
> index 5e8f09c6db..418ee00053 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -65,7 +65,7 @@ struct MigrationIncomingState {
>  
>      QEMUBH *bh;
>  
> -    int state;
> +    MigrationStatus state;
>  
>      bool have_colo_incoming_thread;
>      QemuThread colo_incoming_thread;
> @@ -151,7 +151,7 @@ struct MigrationState
>      /* params from 'migrate-set-parameters' */
>      MigrationParameters parameters;
>  
> -    int state;
> +    MigrationStatus state;
>  
>      /* State related to return path */
>      struct {
> @@ -234,7 +234,7 @@ struct MigrationState
>      bool decompress_error_check;
>  };
>  
> -void migrate_set_state(int *state, int old_state, int new_state);
> +void migrate_set_state(MigrationStatus *state, int old_state, int new_state);
>  
>  void migration_fd_process_incoming(QEMUFile *f);
>  void migration_ioc_process_incoming(QIOChannel *ioc, Error **errp);
> -- 
> 2.19.1
> 
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK