From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=58174 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1Q2QuJ-00017J-Rh
	for qemu-devel@nongnu.org; Wed, 23 Mar 2011 12:27:40 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <anthony@codemonkey.ws>) id 1Q2QuI-0002K3-JP
	for qemu-devel@nongnu.org; Wed, 23 Mar 2011 12:27:39 -0400
Received: from mail-gy0-f173.google.com ([209.85.160.173]:38510)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <anthony@codemonkey.ws>) id 1Q2QuI-0002Jo-Gj
	for qemu-devel@nongnu.org; Wed, 23 Mar 2011 12:27:38 -0400
Received: by gyf1 with SMTP id 1so3443674gyf.4
	for <qemu-devel@nongnu.org>; Wed, 23 Mar 2011 09:27:37 -0700 (PDT)
Message-ID: <4D8A1F6F.8060003@codemonkey.ws>
Date: Wed, 23 Mar 2011 11:27:27 -0500
From: Anthony Liguori <anthony@codemonkey.ws>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] Re: [PATCH 11/11] test-vmstate: add test case to
	verify we don't change VMState
References: <1300839376-22520-1-git-send-email-aliguori@us.ibm.com>	<1300839376-22520-12-git-send-email-aliguori@us.ibm.com>	<AANLkTi=RXFu-_YrxAs7NzNEG=CtqFNs_kLKLWfoXnD9o@mail.gmail.com>	<4D89EABE.50204@codemonkey.ws>
	<m3tyetrkf7.fsf@neno.mitica>	<4D8A0947.5080809@codemonkey.ws>
	<AANLkTimCR0Ck67pXiNTFw1CVS08hy7Z_FbjDp48qKSif@mail.gmail.com>
In-Reply-To: <AANLkTimCR0Ck67pXiNTFw1CVS08hy7Z_FbjDp48qKSif@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Jan Kiszka <jan.kiszka@siemens.com>, qemu-devel@nongnu.org, quintela@redhat.com

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
>