Re: [Qemu-devel] [PATCH 2/6] migration: move global_state.optional out

[Juan Quintela <quintela@redhat.com>]

Peter Xu <peterx@redhat.com> wrote:
> Put it into MigrationState then we can use the properties to specify
> whether to enable storing global state.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  include/migration/migration.h |  6 ++++++
>  migration/migration.c         | 21 +++++++++++++++++----
>  2 files changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index bd0186c..8aa1ea6 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -162,6 +162,12 @@ struct MigrationState
>      /* Do we have to clean up -b/-i from old migrate parameters */
>      /* This feature is deprecated and will be removed */
>      bool must_remove_block_options;
> +
> +    /*
> +     * Global switch on whether we need to store the global state
> +     * during migration.
> +     */
> +    bool store_global_state;
>  };
>  
>  void migrate_set_state(int *state, int old_state, int new_state);
> diff --git a/migration/migration.c b/migration/migration.c
> index 483b027..0653f49 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -140,13 +140,13 @@ void migration_incoming_state_destroy(void)
>  
>  
>  typedef struct {
> -    bool optional;
>      uint32_t size;
>      uint8_t runstate[100];
>      RunState state;
>      bool received;
>  } GlobalState;
>  
> +/* This is only used if MigrationState.store_global_state is set. */
>  static GlobalState global_state;
>  
>  int global_state_store(void)
> @@ -179,7 +179,7 @@ static RunState global_state_get_runstate(void)
>  
>  void global_state_set_optional(void)
>  {
> -    global_state.optional = true;
> +    migrate_get_current()->store_global_state = false;

Part of the advantage (for me) of using qapi was not to have to export
a function to set this.  I.e.  isn't a way to call

qemu_opt_get_bool(migration_opts, "store_global_state", true)

qapi_<magic>_set_bool(migration_opts, "store_global_state",false);
?

So, I don't have to eport global_state_set_optional()?


My goal would be that when I need to add a new propertly, I add it to
migration_properties and to MigrationState and call it a day?

>  static bool global_state_needed(void *opaque)
> @@ -188,8 +188,7 @@ static bool global_state_needed(void *opaque)
>      char *runstate = (char *)s->runstate;
>  
>      /* If it is not optional, it is mandatory */
> -
> -    if (s->optional == false) {
> +    if (migrate_get_current()->store_global_state) {

Being able to query without having a function would also be nice.

Later, Juan.

>          return true;
>      }
>  
> @@ -2109,6 +2108,19 @@ void migrate_fd_connect(MigrationState *s)
>      s->migration_thread_running = true;
>  }
>  
> +static Property migration_properties[] = {
> +    DEFINE_PROP_BOOL("store-global-state", MigrationState,
> +                     store_global_state, true),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void migration_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->props = migration_properties;
> +}
> +
>  static void migration_instance_init(Object *obj)
>  {
>      MigrationState *ms = MIGRATION_OBJ(obj);
> @@ -2133,6 +2145,7 @@ static void migration_instance_init(Object *obj)
>  static const TypeInfo migration_type = {
>      .name = TYPE_MIGRATION,
>      .parent = TYPE_DEVICE,
> +    .class_init = migration_class_init,
>      .class_size = sizeof(MigrationClass),
>      .instance_size = sizeof(MigrationState),
>      .instance_init = migration_instance_init,