Re: [Qemu-devel] [RFC PATCH v0 1/5] cpu: Factor out cpu vmstate_[un]register into separate routines

[Bharata B Rao <bharata@linux.vnet.ibm.com>]

On Tue, Jul 05, 2016 at 02:56:13PM +1000, David Gibson wrote:
> On Tue, Jul 05, 2016 at 10:12:48AM +0530, Bharata B Rao wrote:
> > Consolidates cpu vmstate_[un]register calls into separate routines.
> > No functionality change except that vmstate_unregister calls are
> > now done under !CONFIG_USER_ONLY to match with vmstate_register calls.
> > 
> > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> 
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> 
> > ---
> >  exec.c | 47 ++++++++++++++++++++++++++++-------------------
> >  1 file changed, 28 insertions(+), 19 deletions(-)
> > 
> > diff --git a/exec.c b/exec.c
> > index 0122ef7..8ce8e90 100644
> > --- a/exec.c
> > +++ b/exec.c
> > @@ -594,9 +594,7 @@ AddressSpace *cpu_get_address_space(CPUState *cpu, int asidx)
> >      /* Return the AddressSpace corresponding to the specified index */
> >      return cpu->cpu_ases[asidx].as;
> >  }
> > -#endif
> >  
> > -#ifndef CONFIG_USER_ONLY
> >  static DECLARE_BITMAP(cpu_index_map, MAX_CPUMASK_BITS);
> >  
> >  static int cpu_get_free_index(Error **errp)
> > @@ -617,6 +615,31 @@ static void cpu_release_index(CPUState *cpu)
> >  {
> >      bitmap_clear(cpu_index_map, cpu->cpu_index, 1);
> >  }
> > +
> > +static void cpu_vmstate_register(CPUState *cpu)
> > +{
> > +    CPUClass *cc = CPU_GET_CLASS(cpu);
> > +
> > +    if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
> > +        vmstate_register(NULL, cpu->cpu_index, &vmstate_cpu_common, cpu);
> > +    }
> > +    if (cc->vmsd != NULL) {
> > +        vmstate_register(NULL, cpu->cpu_index, cc->vmsd, cpu);
> > +    }
> > +}
> > +
> > +static void cpu_vmstate_unregister(CPUState *cpu)
> > +{
> > +    CPUClass *cc = CPU_GET_CLASS(cpu);
> > +
> > +    if (cc->vmsd != NULL) {
> > +        vmstate_unregister(NULL, cc->vmsd, cpu);
> > +    }
> > +    if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
> > +        vmstate_unregister(NULL, &vmstate_cpu_common, cpu);
> > +    }
> > +}
> > +
> 
> Given you're factoring this out, would it make sense to defined no-op
> versions for CONFIG_USER_ONLY, to reduce the amount of ifdefs at the
> call site?

I did that in a subsequent patch that moved the calls to these routines
into cpu_common_[un]realize(), but ended up in some unrelated issue and
hence didn't include that patch yet.

But as you note, we could do that with the existing code itself.
Commit 741da0d3 limited the vmstate_register calls to !CONFIG_USER_ONLY.
I am still not sure why cpu vmstate registration isn't needed for
CONFIG_USER_ONLY

Regards,
Bharata.