| Message ID | 1496745042-2379-3-git-send-email-peterx@redhat.com |
|---|---|
| State | New |
| Headers | show |
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,
On Wed, Jun 07, 2017 at 07:42:57PM +0200, Juan Quintela wrote: > 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); > ? 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 <peterx@redhat.com> wrote: > On Wed, Jun 07, 2017 at 07:42:57PM +0200, Juan Quintela wrote: >> > +/* 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?) Yeap. Althought for me would be the same if that is stored anywhare else. I don't really care where it is stored. >> >> 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? Yeap. I *thought* that there was a way to test/set thing programatically also so I didn't have to create/export that functions. My ideal world would be that there were something like that qemu_opt_get_bool(migration_opts, "store_global_state", true); so I only have to export migration_opts (or whatever), and just set/read values from places like xen_init. Im my ideal world, if I have to create a new "property", I don't want to have to export a function to set/read it. For instance, the case of xen_init(). We haven't been able to remove global_state_set_optional() because they don't know about properties. I still love the patches are they are. Boing able to set things from the command line makes things so much better/easier O:-) > I can squash this patch with the next if you like it. That is up to you. Thanks, Juan.
On Thu, Jun 08, 2017 at 01:12:29PM +0200, Juan Quintela wrote: > Peter Xu <peterx@redhat.com> wrote: > > On Wed, Jun 07, 2017 at 07:42:57PM +0200, Juan Quintela wrote: > > >> > +/* 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?) > > Yeap. Althought for me would be the same if that is stored anywhare > else. I don't really care where it is stored. > > >> > >> 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? > > Yeap. I *thought* that there was a way to test/set thing > programatically also so I didn't have to create/export that functions. > My ideal world would be that there were something like that > > qemu_opt_get_bool(migration_opts, "store_global_state", true); > > so I only have to export migration_opts (or whatever), and just set/read > values from places like xen_init. Im my ideal world, if I have to > create a new "property", I don't want to have to export a function to > set/read it. For instance, the case of xen_init(). We haven't been > able to remove global_state_set_optional() because they don't know about > properties. > > I still love the patches are they are. Boing able to set things from > the command line makes things so much better/easier O:-) Oh, looks like what we need to do is just export register_compat_prop(), then xen_init() can use it. I'll try that tomorrow. :)
Peter Xu <peterx@redhat.com> wrote: > On Thu, Jun 08, 2017 at 01:12:29PM +0200, Juan Quintela wrote: >> Peter Xu <peterx@redhat.com> wrote: >> > On Wed, Jun 07, 2017 at 07:42:57PM +0200, Juan Quintela wrote: ... >> >> Yeap. I *thought* that there was a way to test/set thing >> programatically also so I didn't have to create/export that functions. >> My ideal world would be that there were something like that >> >> qemu_opt_get_bool(migration_opts, "store_global_state", true); >> >> so I only have to export migration_opts (or whatever), and just set/read >> values from places like xen_init. Im my ideal world, if I have to >> create a new "property", I don't want to have to export a function to >> set/read it. For instance, the case of xen_init(). We haven't been >> able to remove global_state_set_optional() because they don't know about >> properties. >> >> I still love the patches are they are. Boing able to set things from >> the command line makes things so much better/easier O:-) > > Oh, looks like what we need to do is just export > register_compat_prop(), then xen_init() can use it. > > I'll try that tomorrow. :) Thanks, Juan.
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; } 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) { 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,
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(-)