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

[Peter Xu]

On Thu, Jun 08, 2017 at 01:12:29PM +0200, Juan Quintela wrote:
> 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:-)

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