diff mbox

block: m25p80c Fix vmstate structure name

Message ID 1471609852-23250-1-git-send-email-marcin.krzeminski@nokia.com (mailing list archive)
State New, archived
Headers show

Commit Message

marcin.krzeminski@nokia.com Aug. 19, 2016, 12:30 p.m. UTC
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(-)

Comments

Peter Maydell Aug. 19, 2016, 1:03 p.m. UTC | #1
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
Dr. David Alan Gilbert Aug. 19, 2016, 2:19 p.m. UTC | #2
* 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
Alistair Francis Aug. 19, 2016, 10:20 p.m. UTC | #3
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
>
Paolo Bonzini Aug. 22, 2016, 6:05 p.m. UTC | #4
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
marcin.krzeminski@nokia.com Sept. 6, 2016, 6:42 a.m. UTC | #5
> -----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 mbox

Patch

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()
     }
 };