Re: [Qemu-devel] [RFC qom-cpu v2 6/8] target-alpha: Register VMStateDescription for AlphaCPU

[Juan Quintela]

Andreas Färber <address@hidden> wrote:
> Commit b758aca1f6cdb175634812b79f5560c36c902d00 (target-alpha: Enable
> the alpha-softmmu target.) introduced cpu_{save,load}() functions but
> didn't define CPU_SAVE_VERSION, so they were never registered.
>
> Drop cpu_{save,load}() and register the VMStateDescription via CPUClass.
> This operates on the AlphaCPU object instead of CPUAlphaState.
>
> Signed-off-by: Andreas Färber <address@hidden>


Seeing that we are repeating the code all around.  Could we change this
to something like:

>  
>  #define ENV_GET_CPU(e) CPU(alpha_env_get_cpu(e))
>  
> +#ifdef CONFIG_USER_ONLY
> +#define vmstate_alpha_cpu vmstate_dummy
> +#else
> +extern const struct VMStateDescription vmstate_alpha_cpu;
> +#endif
> +

Change this to:

#ifdef CONFIG_USER_ONLY
#define vmstate_register_cpu(unused, unused)
#else
#define vmstate_register_cpu(env, vmstate_cpu)   (env->vmsd = vmstate_cpu)
#endif

>  #endif
> diff --git a/target-alpha/cpu.c b/target-alpha/cpu.c
> index cec9989..0e739ad 100644
> --- a/target-alpha/cpu.c
> +++ b/target-alpha/cpu.c
> @@ -21,6 +21,7 @@
>  
>  #include "cpu.h"
>  #include "qemu-common.h"
> +#include "migration/vmstate.h"
>  
>  
>  static void alpha_cpu_realizefn(DeviceState *dev, Error **errp)
> @@ -263,6 +264,7 @@ static void alpha_cpu_class_init(ObjectClass *oc, void 
> *data)
>      dc->realize = alpha_cpu_realizefn;
>  
>      cc->class_by_name = alpha_cpu_class_by_name;
> +    cc->vmsd = &vmstate_alpha_cpu;

vmstate_register_cpu(cc, vmstate_alpha_cpu)


And now that I review the 3rd patch that does the same, couldn't be
easier to do just:


static const VMStateDescription vmstate_cpu = {
    .name = "cpu",
    .version_id = 1,
    .minimum_version_id = 1,
    .minimum_version_id_old = 1,
    .fields = vmstate_cpu_fields,
};

to

static const VMStateDescription vmstate_cpu = {
    .name = "cpu",
    .version_id = 1,
    .minimum_version_id = 1,
    .minimum_version_id_old = 1,
    .fields = vmstate_cpu_fields,
};

static const VMStateDescription vmstate_alpha_cpu = {
    .name = "cpu",
    .version_id = 1,
    .minimum_version_id = 1,
    .minimum_version_id_old = 1,
    .fields =  (VMStateField []) {
         VMSTATE_STRUCT(env, AlphaCPU, 1, &vmstate_cpu, CPUAlphaState),
         VMSTATE_END_OF_LIST()
    }
};

????

Only reason that I can think of for all the changes that you have done
is if you are expecting to move fields from CPUAlphaState to AlphaCPU.
Otherwise the method just exposed is much, much easier.