On Fri, Nov 04, 2016 at 04:52:39PM +1100, Alexey Kardashevskiy wrote:
> On 30/10/16 22:12, David Gibson wrote:
> > When vmstate for the ppc cpu was introduced in a90db158 "target-ppc:
> > Convert ppc cpu savevm to VMStateDescription", several "sanity check"
> > fields were included, verifying that certain cpu parameters matched between
> > source and destination.
> >
> > This turns out not to have been a good idea. For one thing it's redundant
> > with existing checks for a compatible cpu version at either end. But more
> > importantly the insns_flags and insns_flags2 checks actively break things:
> > they expose what's essentially an internal TCG implementation detail in the
> > migration stream. That means that when new instruction classes are added
> > or rearranged, migration can break.
> >
> > This removes these ill-considered sanity checks.
> >
> > Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
> > ---
> > target-ppc/machine.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/target-ppc/machine.c b/target-ppc/machine.c
> > index 62b9e94..453ef0a 100644
> > --- a/target-ppc/machine.c
> > +++ b/target-ppc/machine.c
> > @@ -602,10 +602,10 @@ const VMStateDescription vmstate_ppc_cpu = {
> > /* FIXME: access_type? */
> >
> > /* Sanity checking */
> > - VMSTATE_UINTTL_EQUAL(env.msr_mask, PowerPCCPU),
> > - VMSTATE_UINT64_EQUAL(env.insns_flags, PowerPCCPU),
> > - VMSTATE_UINT64_EQUAL(env.insns_flags2, PowerPCCPU),
> > - VMSTATE_UINT32_EQUAL(env.nb_BATs, PowerPCCPU),
> > + VMSTATE_UNUSED(sizeof(target_ulong) /* msr_mask */
> > + + sizeof(uint64_t) /* insns_flags */
> > + + sizeof(uint64_t) /* insns_flags2 */
> > + + sizeof(uint32_t)), /* nb_BATs */
>
>
> This breaks migration to older QEMU:
>
> 25055@1478238734.537761:vmstate_load_field_error field "env.msr_mask" load
> failed, ret = -22
Again, I don't think we generally support backwards migration.
That said, it would be nice here to do a "set to this field on
ourgoing migration, but ignore on incoming migration". Do you know a
way to do that?
a
>
>
>
> > VMSTATE_END_OF_LIST()
> > },
> > .subsections = (const VMStateDescription*[]) {
> >
>
>
--
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
