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

[Juan Quintela]

Peter Xu <address@hidden> 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.