Re: [Qemu-devel] The State of the SaveVM format

[Gerd Hoffmann]

  Hi,

> Today, you make no attempt to support older versions even if their
> format is quite sane. Take ps2_kbd as an example.
>
> The new format (v3) is:
>
> VMSTATE_STRUCT(common, PS2KbdState, 0, vmstate_ps2_common, PS2State),
> VMSTATE_INT32(scan_enabled, PS2KbdState),
> VMSTATE_INT32(translate, PS2KbdState),
> VMSTATE_INT32_V(scancode_set, PS2KbdState,3),
>
> This is nice and should support v2 and v3.

It doesn't ...

> if (version_id == 3)
> s->scancode_set=qemu_get_be32(f);
> else
> s->scancode_set=2;

... setting scancode_set when loading v2 is missing.

I think vmstate fields need a default value to handle cases like this 
one without having to keep the old load function.

> Which has to be an error. But this is the real problem with leaving the
> old functions. It encourages sloppiness.

I think we can kill most of the old load functions.  I'd keep the old 
ones only in case emulating the old load function with vmstate would 
make it unreasonable complex.

> static void marshal_pci_irq_levels(void *opaque, const char *name,
> size_t offset, int load, int version)
> {
> if (version == 2) {
> for (i = 0; i < 4; i++)
> d->irq_state->piix3->pci_irq_levels[i] = qemu_get_be32(f);
> }
> }

> VMSTATE_FUNC_V(irq_state->piix3->pci_irq_levels, PCII440FXState,
> marshal_pci_irq_levels, 2)

No.  I don't want any free-form C code in vmstate.  That will kill quite 
a few of the vmstate advantages.  Imagine a tool dumping snapshot data. 
 What this tool should do when it finds such a FUNC field?  It has 
absolutely no idea what is in there ...

cheers,
  Gerd