Re: [Qemu-devel] Re: [PATCH 11/11] test-vmstate: add test case to verify we don't change VMState

[Anthony Liguori <anthony@codemonkey.ws>]

On 03/23/2011 10:00 AM, Peter Maydell wrote:
> On 23 March 2011 14:52, Anthony Liguori<anthony@codemonkey.ws>  wrote:
>> I think we ought to merge VMStateDescription into DeviceInfo.  For
>> compatibility, we probably need a vmstate_alias name since the device names
>> don't always map 1-1 with the qdev names.  But this should eliminate the
>> problem of reusing VMStateDescriptions for multiple devices.
> That's a feature, not a bug. Consider eg hw/pl110.c -- there
> are two different DeviceInfo devices but since the underlying
> implementation is the same you definitely don't want to have
> two separate VMStateDescription structures to get out of sync.

No, it's a bug.

Migration uses the VMStateDescription name as a section identifier.  The 
section identifiers MUST be unique for a given device.  Otherwise, if 
both devices are present, migration fails miserably.  It also means that 
if the wrong devices are created on the destination, instead of 
predictable failure, you get unpredictable guest corruption.

The Right Way to support what you're describing above is to have a 
single VMStateField array and two VMStateDescriptions.  IOW:

VMStateField pl110_fields[] = {
         VMSTATE_INT32(versatile, pl110_state),
         VMSTATE_UINT32_ARRAY(timing, pl110_state, 4),
         VMSTATE_UINT32(cr, pl110_state),
         VMSTATE_UINT32(upbase, pl110_state),
         VMSTATE_UINT32(lpbase, pl110_state),
         VMSTATE_UINT32(int_status, pl110_state),
         ....
};

VMStateDescription vmstate_pl110 = {
      .name = "pl110",
      ...
      .fields = pl110_fields,
};

VMStateDescription vmstate_pl110_versatile = {
      .name = "pl110-versatile",
      ...
      .fields = pl110_fields,
};

Merging VMStateDescription with DeviceInfo will force this on people 
making it difficult to make this mistake again.

Regards,

Anthony Liguori



> -- PMM
>