[Qemu-devel] Re: optional feature

[Juan Quintela <quintela@redhat.com>]

"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Wed, Sep 16, 2009 at 02:13:06PM +0200, Juan Quintela wrote:
>> VMState rules are simple:
>> - Everything is explicit
>
> By the way, pci currently has cmask,
> which performs checking on load, making sure
> that load does not modify a constant field in config space,
> which can't change as a result of guest actions.
> If it does - migration fails.
>
> This is IMO much better and more robust than
> simply hoping that there are no bugs or that
> developers remember to increment a version
> number each time they change some field.

This one is going to be fixed.  Some kind of checksum that assures you
that you haven't added/removed any field of a vmstatedescription.  It is
not difficult to add, but no code/whatever is there.

The only minimal check that it does today is that you put:

VMSTATE_INT32_ARRAY_V(irq_state, PCIDevice, 4, 2),

4 is the length and 2 is the version.

VMstate checks that PCIDevice has an irq_state field of type int32_t
with lenght 4.  I could have calculated the 4, but it is there just in
case someone changes the size of the array in PCIDevice, it gets a
compilation error.  There is not more infrastructure yet to check for
changes on the state.  It should come once devices are ported to
VMState.


> I think it's pretty important to keep this
> feature, and maybe add something similar
> to other devices.
>
> How will VMState support this?

This is the 1st request that I have.  This is what the code does today
(it is the same that was before):


static int get_pci_config_device(QEMUFile *f, void *pv, size_t size)
{
    PCIDevice *s = container_of(pv, PCIDevice, config);
    uint8_t config[size];
    int i;

    qemu_get_buffer(f, config, size);
    for (i = 0; i < size; ++i)
        if ((config[i] ^ s->config[i]) & s->cmask[i] & ~s->wmask[i])
            return -EINVAL;
    memcpy(s->config, config, size);

    pci_update_mappings(s);

    return 0;
}

/* just put buffer */
static void put_pci_config_device(QEMUFile *f, void *pv, size_t size)
{
    const uint8_t *v = pv;
    qemu_put_buffer(f, v, size);
}

i.e. at save time, we save everything that we want to save.
At load time, we only copy some things.  I don't understand what cmask
and wmask means, but I guess you understand this part better than me.

If we need to add more checks on load, we can just hack on that function
whatever you want to check/change/...

VMstate don't really care (as it shouldn't)

Later, Juan.