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

[Andreas Färber]

Am 22.02.2013 14:22, schrieb Juan Quintela:
> 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.

Thanks for your review of this series!

Did you actually read the cover letter though? :)

Short-term I do need to access the halted field outside of CPUX86State
in the referenced pending series, mid-term I would like to be able to
move more non-TCG fields into FooCPU and not be limited by VMState only
being able to access CPUArchState fields.

Question: Is the VMSTATE_STRUCT() usage above version-compatible for
previously-migratable targets such as lm32? We could leave x86 as is
(modulo the vmsd registration discussion) and do the struct indirection
for lm32 for now to keep changes minimal as long as non-CPULM32State
fields are not yet needed.

For alpha and openrisc RFCs my big question was rather whether we want
to use some VMSTATE_ macro to embed "cpu_common" as first field of "cpu"
and assign it to DeviceClass::vmsd? Not sure which of the macros would
allow us to register a VMStateDescription standalone for the "legacy"
targets and as part of another VMStateDescription without sub-field such
as 'env' above or whether we would need to duplicate the fields into a
subsection definition then?

More generally it would be a some-targets-do-it-differently-than-others
kind of issue. But we already have some targets using VMState and others
not, so you could say we have the issue today already. ;)

In short: My pressing need is a version-compatible solution for x86
registering VMState for X86CPU (CPUState) rather than CPUX86State,
anything else could be postponed pending further discussions.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg