| Message ID | 1471609852-23250-1-git-send-email-marcin.krzeminski@nokia.com |
|---|---|
| State | New |
| Headers | show |
On 19 August 2016 at 13:30, <marcin.krzeminski@nokia.com> wrote: > From: Marcin Krzeminski <marcin.krzeminski@nokia.com> > > Change wrong name of the vmstate structure. Since this breaks > compatibility update version and fields to 0. > > Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com> > --- > > This patch assumes that none migrates m25p80 flash devices. > > hw/block/m25p80.c | 29 ++++++++++++++--------------- > 1 file changed, 14 insertions(+), 15 deletions(-) > > diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c > index 9828ee6..d29ff4c 100644 > --- a/hw/block/m25p80.c > +++ b/hw/block/m25p80.c > @@ -1189,9 +1189,9 @@ static Property m25p80_properties[] = { > }; > > static const VMStateDescription vmstate_m25p80 = { > - .name = "xilinx_spi", > - .version_id = 3, > - .minimum_version_id = 1, > + .name = "m25p80", > + .version_id = 0, > + .minimum_version_id = 0, > .pre_save = m25p80_pre_save, Are you sure that the name is part of the on-the-wire state? I thought it wasn't, in which case this doesn't even need a version bump, much less a version-reset-to-zero. But I could be wrong. David, Juan, Amit? thanks -- PMM
* Peter Maydell (peter.maydell@linaro.org) wrote: > On 19 August 2016 at 13:30, <marcin.krzeminski@nokia.com> wrote: > > From: Marcin Krzeminski <marcin.krzeminski@nokia.com> > > > > Change wrong name of the vmstate structure. Since this breaks > > compatibility update version and fields to 0. > > > > Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com> > > --- > > > > This patch assumes that none migrates m25p80 flash devices. > > > > hw/block/m25p80.c | 29 ++++++++++++++--------------- > > 1 file changed, 14 insertions(+), 15 deletions(-) > > > > diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c > > index 9828ee6..d29ff4c 100644 > > --- a/hw/block/m25p80.c > > +++ b/hw/block/m25p80.c > > @@ -1189,9 +1189,9 @@ static Property m25p80_properties[] = { > > }; > > > > static const VMStateDescription vmstate_m25p80 = { > > - .name = "xilinx_spi", > > - .version_id = 3, > > - .minimum_version_id = 1, > > + .name = "m25p80", > > + .version_id = 0, > > + .minimum_version_id = 0, > > .pre_save = m25p80_pre_save, > > Are you sure that the name is part of the on-the-wire state? > I thought it wasn't, in which case this doesn't even need a > version bump, much less a version-reset-to-zero. But I could > be wrong. David, Juan, Amit? > Yep, it does: 000a9000: 00 1c 04 00 00 00 1d 0a 78 69 6c 69 6e 78 5f 73 ........xilinx_s 000a9010: 70 69 00 00 00 00 00 00 00 03 00 00 00 00 00 00 pi.............. that's from: ./arm-softmmu/qemu-system-arm -M tosa -device at26f004 -S -nographic QEMU 2.6.90 monitor - type 'help' for more information (qemu) migrate "exec:xxd -g 1 > tosa.mig" it's not essential to redo the version_id's, but it's as good a time as any to get rid of the old version code if you've just broken the compatibility anyway. Dave > thanks > -- PMM -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On Fri, Aug 19, 2016 at 7:19 AM, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote: > * Peter Maydell (peter.maydell@linaro.org) wrote: >> On 19 August 2016 at 13:30, <marcin.krzeminski@nokia.com> wrote: >> > From: Marcin Krzeminski <marcin.krzeminski@nokia.com> >> > >> > Change wrong name of the vmstate structure. Since this breaks >> > compatibility update version and fields to 0. s/and/set/g >> > >> > Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com> Looks fine to me. Acked-by: Alistair Francis <alistair.francis@xilinx.com> Thanks, Alistair >> > --- >> > >> > This patch assumes that none migrates m25p80 flash devices. >> > >> > hw/block/m25p80.c | 29 ++++++++++++++--------------- >> > 1 file changed, 14 insertions(+), 15 deletions(-) >> > >> > diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c >> > index 9828ee6..d29ff4c 100644 >> > --- a/hw/block/m25p80.c >> > +++ b/hw/block/m25p80.c >> > @@ -1189,9 +1189,9 @@ static Property m25p80_properties[] = { >> > }; >> > >> > static const VMStateDescription vmstate_m25p80 = { >> > - .name = "xilinx_spi", >> > - .version_id = 3, >> > - .minimum_version_id = 1, >> > + .name = "m25p80", >> > + .version_id = 0, >> > + .minimum_version_id = 0, >> > .pre_save = m25p80_pre_save, >> >> Are you sure that the name is part of the on-the-wire state? >> I thought it wasn't, in which case this doesn't even need a >> version bump, much less a version-reset-to-zero. But I could >> be wrong. David, Juan, Amit? >> > > Yep, it does: > > 000a9000: 00 1c 04 00 00 00 1d 0a 78 69 6c 69 6e 78 5f 73 ........xilinx_s > 000a9010: 70 69 00 00 00 00 00 00 00 03 00 00 00 00 00 00 pi.............. > > that's from: > ./arm-softmmu/qemu-system-arm -M tosa -device at26f004 -S -nographic > QEMU 2.6.90 monitor - type 'help' for more information > (qemu) migrate "exec:xxd -g 1 > tosa.mig" > > it's not essential to redo the version_id's, but it's as good a time as any > to get rid of the old version code if you've just broken the compatibility > anyway. > > Dave > >> thanks >> -- PMM > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK >
On 20/08/2016 00:20, Alistair Francis wrote: >>>> >> > Change wrong name of the vmstate structure. Since this breaks >>>> >> > compatibility update version and fields to 0. > s/and/set/g > Or more likely: "update the VMState version to 0 and make all fields independent of the VMState version". Paolo
> -----Original Message----- > From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo > Bonzini > Sent: Monday, August 22, 2016 8:06 PM > To: Alistair Francis <alistair.francis@xilinx.com>; Dr. David Alan Gilbert > <dgilbert@redhat.com> > Cc: Peter Maydell <peter.maydell@linaro.org>; Andrew Jones > <drjones@redhat.com>; Juan Quintela <quintela@redhat.com>; Krzeminski, > Marcin (Nokia - PL/Wroclaw) <marcin.krzeminski@nokia.com>; QEMU > Developers <qemu-devel@nongnu.org>; qemu-arm <qemu- > arm@nongnu.org>; Amit Shah <amit.shah@redhat.com>; rfsw- > patches@mlist.nokia.com > Subject: Re: [Qemu-arm] [PATCH] block: m25p80c Fix vmstate structure > name > > > > On 20/08/2016 00:20, Alistair Francis wrote: > >>>> >> > Change wrong name of the vmstate structure. Since this breaks > >>>> >> > compatibility update version and fields to 0. > > s/and/set/g > > > > Or more likely: "update the VMState version to 0 and make all fields > independent of the VMState version". Yes, I will rewrite commit ,essage and send V2. Thanks, Marcin > > Paolo
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c index 9828ee6..d29ff4c 100644 --- a/hw/block/m25p80.c +++ b/hw/block/m25p80.c @@ -1189,9 +1189,9 @@ static Property m25p80_properties[] = { }; static const VMStateDescription vmstate_m25p80 = { - .name = "xilinx_spi", - .version_id = 3, - .minimum_version_id = 1, + .name = "m25p80", + .version_id = 0, + .minimum_version_id = 0, .pre_save = m25p80_pre_save, .fields = (VMStateField[]) { VMSTATE_UINT8(state, Flash), @@ -1200,20 +1200,19 @@ static const VMStateDescription vmstate_m25p80 = { VMSTATE_UINT32(pos, Flash), VMSTATE_UINT8(needed_bytes, Flash), VMSTATE_UINT8(cmd_in_progress, Flash), - VMSTATE_UNUSED(4), VMSTATE_UINT32(cur_addr, Flash), VMSTATE_BOOL(write_enable, Flash), - VMSTATE_BOOL_V(reset_enable, Flash, 2), - VMSTATE_UINT8_V(ear, Flash, 2), - VMSTATE_BOOL_V(four_bytes_address_mode, Flash, 2), - VMSTATE_UINT32_V(nonvolatile_cfg, Flash, 2), - VMSTATE_UINT32_V(volatile_cfg, Flash, 2), - VMSTATE_UINT32_V(enh_volatile_cfg, Flash, 2), - VMSTATE_BOOL_V(quad_enable, Flash, 3), - VMSTATE_UINT8_V(spansion_cr1nv, Flash, 3), - VMSTATE_UINT8_V(spansion_cr2nv, Flash, 3), - VMSTATE_UINT8_V(spansion_cr3nv, Flash, 3), - VMSTATE_UINT8_V(spansion_cr4nv, Flash, 3), + VMSTATE_BOOL(reset_enable, Flash), + VMSTATE_UINT8(ear, Flash), + VMSTATE_BOOL(four_bytes_address_mode, Flash), + VMSTATE_UINT32(nonvolatile_cfg, Flash), + VMSTATE_UINT32(volatile_cfg, Flash), + VMSTATE_UINT32(enh_volatile_cfg, Flash), + VMSTATE_BOOL(quad_enable, Flash), + VMSTATE_UINT8(spansion_cr1nv, Flash), + VMSTATE_UINT8(spansion_cr2nv, Flash), + VMSTATE_UINT8(spansion_cr3nv, Flash), + VMSTATE_UINT8(spansion_cr4nv, Flash), VMSTATE_END_OF_LIST() } };