On Wed, Jun 07, 2017 at 07:42:57PM +0200, Juan Quintela wrote:
> Peter Xu <pet...@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 <pet...@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);
> ?
I didn't catch the comment here... Do you mean e.g.
qemu_opt_set_bool()? Here can we use it in some way?
(I thought we were using the "-global migration.store_global_state"
parameter, then it'll setup MigrationState.store_global_state, isn't
that the trick?)
>
> So, I don't have to eport global_state_set_optional()?
>
As mentioned in latter patch, xen_init() still uses it, so looks like
we still need it?
I can squash this patch with the next if you like it.
Thanks,
>
> 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,
--
Peter Xu
