[Qemu-devel] Re: [PATCH v2 1/2] hw/arm_sysctl.c: Add the Versatile Express system registers

[Juan Quintela]

Peter Maydell <address@hidden> wrote:
> On 5 March 2011 14:59, Paolo Bonzini <address@hidden> wrote:
>> On 03/05/2011 01:34 PM, Peter Maydell wrote:

sorry, I miss this email before.

>
> Ah. I was just following Juan's suggestion:
>> - if you don't care about backward compatibility, just add +1 to all the
>> version fields and you are done.
>
> but I guess if it's that trivial we might as well support it.
> (My guess is that basically nobody is doing any kind of
> migration of ARM TCG VMs, so I think it's all a bit moot.)
>
> What do the new fields get set to when you do a load from
> a v1 snapshot?
>
> Other random snapshot/vmstate questions while I'm here:
>
> (1) Is there supposed to be any kind of guard on trying to
> do a vmsave on a system with devices that don't implement
> save/load? IME it just produces a snapshot which doesn't
> work when you reload it...

only real way to do it is to set a device that is not migratable.  Only
having devices that don't have a savevm section would not work, because
for instance usb devices don't need a savevm section.

> (2) How do you track incompatible changes at the machine
> level? For instance, suppose we accidentally forgot to
> model a NOT gate in an IRQ line, so we add a qemu_irq_invert()
> to the machine init function. None of the devices have
> changed, but you can't load the state of an old version
> of the machine into a new version, because then the
> devices on either end of the inverter would be inconsistent
> about the state of the line. But there's no version number
> for a machine as far as I can see.

Dunno.  I have only worked to x86*, no clue about integrated boards that
have "interesting" conections.  At some point we would have to "define"
the machine board better, just now it is not there.

> I've appended a draft of a suggested extra section for
> docs/migration.txt so you can tell me if I've misunderstood
> it all :-)
>
> ---begin---
> === Adding state fields to a device ===
>
> If you make a bugfix or enhancement to a device which requires the
> addition of extra state, you need to add these new state fields
> to the VMStateDescription so that:
>  (1) they are saved and loaded correctly
>  (2) migration between the new and old versions either works
>      or fails cleanly.
>
> If the change is such that migration between versions would not
> work anyway, you can just add the new fields using the usual
> VMSTATE_* macros, increment the version_id and set the
> minimum_version_id to be the same as the version_id.
>
> Migration from the old version to the new version can be supported
> if it is OK for the new fields to remain in their default state
> [XXX is this right? are they zeroed, or do they get the value
> the device's reset function sets them to, or something else?]

You can initialize in your init function at the value that you want, or
use foo_post_load() function (that receives the version as a parameter)
to assign to any correct values that you need.

> when the state of an old-version snapshot is loaded. To implement
> this you need to use the VMSTATE_*_V macros which let you specify
> the version in which a field was introduced, for instance:
>
>  VMSTATE_UINT32_V(sys_cfgdata, arm_sysctl_state, 2)
>
> for a field introduced in version 2. You should also increment
> the version_id, but leave the minimum_version_id unchanged.
> Newly added VMSTATE_*_V fields should go at the end of the
> VMState description.

Just to make things more complicated, this has been "deprecated" O:-)

Ok, I am going to try to explain it myself.

We have one vmstate section.

const VMStateDescription vmstate_foo = {
    .name = "foo",
    .version_id = 0,
    .minimum_version_id = 0,
    .minimum_version_id_old = 0,
    .fields      = (VMStateField []) {
        VMSTATE_INT32(bar, FOOState),
        VMSTATE_END_OF_LIST()
    }
};

And we then notice that we need another int (bar2) on the state.
Posibilities:

- We know that old device was wrong, and that there is no way we can
  load (reliabely) from version 0.  Then we just increase the version:

const VMStateDescription vmstate_foo = {
    .name = "foo",
    .version_id = 1,
    .minimum_version_id = 1,
    .minimum_version_id_old = 1,
    .fields      = (VMStateField []) {
        VMSTATE_INT32(bar, FOOState),
        VMSTATE_INT32(bar2, FOOState),
        VMSTATE_END_OF_LIST()
    }
};

  And we are done.

- We know that we can load from v1.  But that we want to always sent
  bar2 for migration, then we just increase versions to:


const VMStateDescription vmstate_foo = {
    .name = "foo",
    .version_id = 2,
    .minimum_version_id = 1,
    .minimum_version_id_old = 1,
    .fields      = (VMStateField []) {
        VMSTATE_INT32(bar, FOOState),
        VMSTATE_INT32_V(bar2, FOOState, 1),
        VMSTATE_END_OF_LIST()
    }
};

And we are done.  We are able to receive state 0 and 1, and we would
always sent version 1.

- We know that bar2 is only need sometimes, and we know what sometimes
  mean.  Then we use a new subsection.

/* this function returns true if bar2 is needed */
static bool foo_bar2_needed(void *opaque)
{
    FOOState *s = opaque;

    return ......;
}

const VMStateDescription vmstate_bar2 = {
    .name = "foo/bar2",
    .version_id = 0,
    .minimum_version_id = 0,
    .minimum_version_id_old = 0,
    .fields      = (VMStateField []) {
        VMSTATE_INT32(bar2, FOOState),
        VMSTATE_END_OF_LIST()
    }
};

const VMStateDescription vmstate_foo = {
    .name = "foo",
    .version_id = 2,
    .minimum_version_id = 1,
    .minimum_version_id_old = 1,
    .fields      = (VMStateField []) {
        VMSTATE_INT32(bar, FOOState),
        VMSTATE_INT32_V(bar2, FOOState, 1),
        VMSTATE_END_OF_LIST()
    }
,
    .subsections = (VMStateSubsection []) {
        {
            .vmsd = &vmstate_bar2,
            .needed = foo_bar2_needed,
        }, {
            /* empty */
        }
    }
};

Why do we want to go to all this trouble? To be able to do a migration
from a new version to one old version.  If bar2 is not needed, we know
that we can _not_ send bar2 subsection. If bar2 is needed, we just sent
it and we fail the migration to the old version.

Notice that this only makes sense if foo_bar2_needed() normally returns
false, if it returns always true, it is just as good to just increase
the version.   An example on when this is useful is with ide.  When we
defined vmstate_ide_drive, we forgot the pio state.  So, it always
worked well execpt if we were in the middle of a pio operation.  PIO
operations are only used (normally) during boot.   Adding a new
subsection means that we normally don't sent the pio state, except if it
is really needed.

Have I manage to explain myself a little bit?

Later, Juan.