From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55001) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Up0sk-0002ZE-Cy for qemu-devel@nongnu.org; Tue, 18 Jun 2013 14:43:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Up0sh-0001y6-79 for qemu-devel@nongnu.org; Tue, 18 Jun 2013 14:43:54 -0400 Received: from e7.ny.us.ibm.com ([32.97.182.137]:53733) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Up0sh-0001xt-3T for qemu-devel@nongnu.org; Tue, 18 Jun 2013 14:43:51 -0400 Received: from /spool/local by e7.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 18 Jun 2013 14:43:50 -0400 From: Anthony Liguori In-Reply-To: <20130618180727.GC12685@vm> References: <1369240371-21253-1-git-send-email-mdroth@linux.vnet.ibm.com> <20130611215351.GA12585@vm> <20130618180727.GC12685@vm> Date: Tue, 18 Jun 2013 13:43:40 -0500 Message-ID: <87a9mnw97n.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: Re: [Qemu-devel] [PATCH v2] wdt_i6300esb: fix vmstate versioning List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: mdroth , qemu-devel@nongnu.org Cc: peter.maydell@linaro.org, nick@bytemark.co.uk, lersek@redhat.com, qemu-stable@nongnu.org, quintela@redhat.com mdroth writes: > On Tue, Jun 11, 2013 at 04:53:51PM -0500, mdroth wrote: >> On Wed, May 22, 2013 at 11:32:51AM -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 >> >> Ping, looking to pull this in for 1.5.1 > > Anthony, Juan? Not sure if this is on your radar. Looking to get it > applied prior to stable freeze tomorrow. I'll pick it up. Regards, Anthony Liguori > >> >> > --- >> > v2: >> > * Fixed s/except/accept/ typo (Laszlo) >> > >> > 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..05af0b1 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 accept 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 >> >