From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35962) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Umsag-000185-KS for qemu-devel@nongnu.org; Wed, 12 Jun 2013 17:28:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Umsaf-0002wO-MC for qemu-devel@nongnu.org; Wed, 12 Jun 2013 17:28:26 -0400 Sender: fluxion Date: Wed, 12 Jun 2013 16:27:49 -0500 From: mdroth Message-ID: <20130612212749.GA1875@vm> References: <1369175577-18130-1-git-send-email-mdroth@linux.vnet.ibm.com> <20130612201151.GF12585@vm> <87ppvrvxj2.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87ppvrvxj2.fsf@codemonkey.ws> Subject: Re: [Qemu-devel] [PATCH] wdt_i6300esb: fix vmstate versioning List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: qemu-trivial@nongnu.org, peter.maydell@linaro.org, nick@bytemark.co.uk, qemu-devel@nongnu.org, qemu-stable@nongnu.org On Wed, Jun 12, 2013 at 04:17:53PM -0500, Anthony Liguori wrote: > 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. Sorry, pinged the wrong email. v2 is to the latest: http://lists.nongnu.org/archive/html/qemu-devel/2013-05/msg02991.html > > 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 > >> >