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;
}
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,
--
2.7.4
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
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
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.
© 2016 - 2025 Red Hat, Inc.