From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33836) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UmsQd-0000Qo-Kx for qemu-devel@nongnu.org; Wed, 12 Jun 2013 17:18:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UmsQa-0007kU-0J for qemu-devel@nongnu.org; Wed, 12 Jun 2013 17:18:03 -0400 Received: from mail-ob0-x22b.google.com ([2607:f8b0:4003:c01::22b]:37298) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UmsQZ-0007kH-RR for qemu-devel@nongnu.org; Wed, 12 Jun 2013 17:17:59 -0400 Received: by mail-ob0-f171.google.com with SMTP id dn14so13933435obc.16 for ; Wed, 12 Jun 2013 14:17:59 -0700 (PDT) From: Anthony Liguori In-Reply-To: <20130612201151.GF12585@vm> References: <1369175577-18130-1-git-send-email-mdroth@linux.vnet.ibm.com> <20130612201151.GF12585@vm> Date: Wed, 12 Jun 2013 16:17:53 -0500 Message-ID: <87ppvrvxj2.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: Re: [Qemu-devel] [PATCH] wdt_i6300esb: fix vmstate versioning List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: mdroth , qemu-trivial@nongnu.org Cc: peter.maydell@linaro.org, nick@bytemark.co.uk, qemu-devel@nongnu.org, qemu-stable@nongnu.org mdroth writes: > On Tue, May 21, 2013 at 05:32:57PM -0500, Michael Roth wrote: >> When this VMSD was introduced it's version fields were set to >> sizeof(I6300State), making them essentially random from build to build, >> version to version. >> >> To fix this, we lock in a high version id and low minimum version id to >> support old->new migration from all prior versions of this device's >> state. This should work since the device state has not changed since >> its introduction. >> >> The potentially breaks migration from 1.5+ to 1.5, but since the >> versioning was essentially random prior to this patch, new->old >> migration was not consistently functional to begin with. >> >> Reported-by: Nicholas Thomas >> Suggested-by: Peter Maydell >> Cc: qemu-stable@nongnu.org >> Signed-off-by: Michael Roth > > CC'ing qemu-trivial. Looking to get this in for 1.5.1 v2? There were some review comments that haven't been addressed. Regards, Anthony Liguori > >> --- >> hw/watchdog/wdt_i6300esb.c | 19 ++++++++++++++++--- >> 1 file changed, 16 insertions(+), 3 deletions(-) >> >> diff --git a/hw/watchdog/wdt_i6300esb.c b/hw/watchdog/wdt_i6300esb.c >> index 1407fba..851b664 100644 >> --- a/hw/watchdog/wdt_i6300esb.c >> +++ b/hw/watchdog/wdt_i6300esb.c >> @@ -374,9 +374,22 @@ static const MemoryRegionOps i6300esb_ops = { >> >> static const VMStateDescription vmstate_i6300esb = { >> .name = "i6300esb_wdt", >> - .version_id = sizeof(I6300State), >> - .minimum_version_id = sizeof(I6300State), >> - .minimum_version_id_old = sizeof(I6300State), >> + /* With this VMSD's introduction, version_id/minimum_version_id were >> + * erroneously set to sizeof(I6300State), causing a somewhat random >> + * version_id to be set for every build. This eventually broke >> + * migration. >> + * >> + * To correct this without breaking old->new migration for older versions >> + * of QEMU, we've set version_id to a value high enough to exceed all past >> + * values of sizeof(I6300State) across various build environments, and have >> + * reset minimum_version_id_old/minimum_version_id to 1, since this VMSD >> + * has never changed and thus can except all past versions. >> + * >> + * For future changes we can treat these values as we normally would. >> + */ >> + .version_id = 10000, >> + .minimum_version_id = 1, >> + .minimum_version_id_old = 1, >> .fields = (VMStateField []) { >> VMSTATE_PCI_DEVICE(dev, I6300State), >> VMSTATE_INT32(reboot_enabled, I6300State), >> -- >> 1.7.9.5 >>