Re: [Qemu-devel] [RFC PATCH v1 1/5] cpu, target-ppc: Move cpu_vmstate_[un]register calls to cpu_common_[un]realize

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

On Wed, Jul 06, 2016 at 04:44:17PM +0200, Igor Mammedov wrote:
> On Wed, 6 Jul 2016 19:46:13 +0530
> Bharata B Rao <bharata@linux.vnet.ibm.com> wrote:
> 
> > On Wed, Jul 06, 2016 at 12:57:49PM +0200, Igor Mammedov wrote:
> > > On Wed,  6 Jul 2016 14:29:17 +0530
> > > Bharata B Rao <bharata@linux.vnet.ibm.com> wrote:
> > > 
> > > > Move vmstate_register() call to cpu_common_realize().
> > > > Introduce cpu_common_unrealize() and move vmstate_unregister() to
> > > > it.
> > > > 
> > > > Change those archs that implement their own CPU unrealize routine
> > > > to mandatorily call CPUClass::unrealize().
> > > > 
> > > > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> > > > ---
> > > >  exec.c                      | 53
> > > > ++++++++++++++++++++++++++++-----------------
> > > > include/qom/cpu.h           |  2 ++ qom/cpu.c
> > > > |  7 ++++++ target-ppc/cpu-qom.h        |  2 ++
> > > >  target-ppc/translate_init.c |  3 +++
> > > >  5 files changed, 47 insertions(+), 20 deletions(-)
> > > > 
> > > > diff --git a/exec.c b/exec.c
> > > > index 0122ef7..fb73910 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);
> > > >  }
> > > > +
> > > > +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);
> > > > +    }
> > > > +}
> > > > +
> > > > +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);
> > > > +    }
> > > > +}
> > > 
> > > Is there any reason to keep these in exec.c,
> > > I'd put them in qom/cpu.c
> > 
> > I started with it, had to move vmstate_cpu_common and its
> > pre/post_load routines and one of them called tlb_flush() whose
> > header qom/cpu.c didn't like. So I saved the movement for later.
> ok,
> 
> alternatively you can check for use_migration_id in exec.c without
> moving vmstate_* around.

Ok, just to be clear, you are suggesting that I move back to my
first implementation which just consolidated vmstate_* calls
and called them from within cpu_exec_init/exit() ?

Regards,
Bharata.