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

[David Gibson]

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 <address@hidden>

Reviewed-by: David Gibson <address@hidden>

> ---
>  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?

>  #else
>  
>  static int cpu_get_free_index(Error **errp)
> @@ -638,8 +661,6 @@ static void cpu_release_index(CPUState *cpu)
>  
>  void cpu_exec_exit(CPUState *cpu)
>  {
> -    CPUClass *cc = CPU_GET_CLASS(cpu);
> -
>  #if defined(CONFIG_USER_ONLY)
>      cpu_list_lock();
>  #endif
> @@ -656,19 +677,13 @@ void cpu_exec_exit(CPUState *cpu)
>      cpu->cpu_index = -1;
>  #if defined(CONFIG_USER_ONLY)
>      cpu_list_unlock();
> +#else
> +    cpu_vmstate_unregister(cpu);
>  #endif
> -
> -    if (cc->vmsd != NULL) {
> -        vmstate_unregister(NULL, cc->vmsd, cpu);
> -    }
> -    if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
> -        vmstate_unregister(NULL, &vmstate_cpu_common, cpu);
> -    }
>  }
>  
>  void cpu_exec_init(CPUState *cpu, Error **errp)
>  {
> -    CPUClass *cc = CPU_GET_CLASS(cpu);
>      Error *local_err = NULL;
>  
>      cpu->as = NULL;
> @@ -705,15 +720,9 @@ void cpu_exec_init(CPUState *cpu, Error **errp)
>      }
>      QTAILQ_INSERT_TAIL(&cpus, cpu, node);
>  #if defined(CONFIG_USER_ONLY)
> -    (void) cc;
>      cpu_list_unlock();
>  #else
> -    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);
> -    }
> +    cpu_vmstate_register(cpu);
>  #endif
>  }
>  

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature