Re: [Qemu-devel] [PATCH v2] wdt_i6300esb: fix vmstate versioning
Applied. Thanks. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH v2] wdt_i6300esb: fix vmstate versioning
mdroth wrote: > 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-sta...@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. Reviewed-by: Juan Quintela I guess that the value was already there, not that the version field hasn't been abused lot of times. I agree that sizeof() makes for an interesting version number, especially if padding enter the equation.
Re: [Qemu-devel] [PATCH v2] wdt_i6300esb: fix vmstate versioning
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-sta...@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 = 1,
>> > +.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
>> >
Re: [Qemu-devel] [PATCH v2] wdt_i6300esb: fix vmstate versioning
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-sta...@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.
>
> > ---
> > 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 = 1,
> > +.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
> >
Re: [Qemu-devel] [PATCH v2] wdt_i6300esb: fix vmstate versioning
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-sta...@nongnu.org
> Signed-off-by: Michael Roth
Ping, looking to pull this in for 1.5.1
> ---
> 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 = 1,
> +.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
>
Re: [Qemu-devel] [PATCH v2] wdt_i6300esb: fix vmstate versioning
On (Wed) 22 May 2013 [11:32:51], 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-sta...@nongnu.org > Signed-off-by: Michael Roth > --- > v2: > * Fixed s/except/accept/ typo (Laszlo) Reviewed-by: Amit Shah Amit
Re: [Qemu-devel] [PATCH v2] wdt_i6300esb: fix vmstate versioning
On 05/22/13 18:32, 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-sta...@nongnu.org > Signed-off-by: Michael Roth > --- > v2: > * Fixed s/except/accept/ typo (Laszlo) > > hw/watchdog/wdt_i6300esb.c | 19 --- > 1 file changed, 16 insertions(+), 3 deletions(-) Always alert to make a difference :), Reviewed-by: Laszlo Ersek
Re: [Qemu-devel] [PATCH v2] wdt_i6300esb: fix vmstate versioning
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-sta...@nongnu.org > Signed-off-by: Michael Roth ACK. I guess no one uses watchdog much, or they don't try to migrate those guests :-( Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
