Re: [Qemu-devel] [PATCH 07/23] New VMstate save/load infrastructure

[Michael S. Tsirkin]

On Thu, Aug 20, 2009 at 07:42:25PM +0200, Juan Quintela wrote:
> This patch introduces VMState infrastructure, to convert the save/load
> functions of devices to a table approach.  This new approach has the
> following advantages:
> - it is type-safe
> - you can't have load/save functions out of sync
> - will allows us to have new interesting commands, like dump <device>, that
>   shows all its internal state.
> - Just now, the only added type is arrays, but we can add structures.
> - Uses old load_state() function for loading old state.
> 
> Signed-off-by: Juan Quintela <address@hidden>
> ---

...

> +#define VMSTATE_SINGLE(_field, _state, _version, _info, _type) {     \
> +    .name       = (stringify(_field)),                               \
> +    .version_id = (_version),                                        \
> +    .size       = sizeof(_type),                                     \
> +    .info       = &(_info),                                          \
> +    .flags      = VMS_SINGLE,                                        \
> +    .offset     = offsetof(_state, _field)                           \
> +            + type_check(_type,typeof_field(_state, _field))         \
> +}


As I said in a different thread, I think the versioning approach
taken here is too limited. In particular, it does not easily support
optional features such as msix. We can use other mechanisms for this,
such as encoding options in the device name, but I think it's too
limiting.

What I think we should have instead of a version is an identifier for
each field. This way it is easy to add or skip each field separately,
depending on command line options, and we can also verify, at load time,
that all required fields are present. This will also reduce the amount
of duplicated code required each time we extend migration format.

As a bonus, we'll be able to write a utility parsing the state
and checking that it's sane.

-- 
MST