On Wed, Jun 12, 2013 at 04:17:53PM -0500, Anthony Liguori wrote: > mdroth <mdr...@linux.vnet.ibm.com> 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 <n...@bytemark.co.uk> > >> Suggested-by: Peter Maydell <peter.mayd...@linaro.org> > >> Cc: qemu-sta...@nongnu.org > >> Signed-off-by: Michael Roth <mdr...@linux.vnet.ibm.com> > > > > 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 > >> >
