Skip to contentSkip to conversation

pcspk migration compatibility

PATCH In review Dr. David Alan Gilbert (git) 3 participants 6 messages Nov 28, 2016 – Nov 29, 2016

lore
Share & tools
lore
#0 lore raw

cover pcspk migration compatibility

From: "Dr. David Alan Gilbert" <[email protected]>

Hi,
39c88f56 added VMState for pcspk but turned it on for
all machine types, this breaks backwards compatibility to older machine types.

If this is too late for 2.8 then I suggest we take the 1st
of these two patches, which just makes it a property to flip for those of us who need it. Adding the 2nd patch after 2.8 might cause problems for people using 2.8 with 2.7 machine type.

Dave

Dr. David Alan Gilbert (2):
migration/pcspk: Add a property to state if pcspk is migrated migration/pcspk: Turn migration of pcspk off for 2.7 and older

 hw/audio/pcspk.c     | 10 ++++++++++
 include/hw/i386/pc.h |  5 +++++
 2 files changed, 15 insertions(+)
Signature · 1 line
2.9.3
#1 lore raw

PATCH 1/2 migration/pcspk: Add a property to state if pcspk is migrated

From: "Dr. David Alan Gilbert" <[email protected]>

Allow us to turn migration of pcspk off for compatibility.

---
 hw/audio/pcspk.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
hw/audio/pcspk.c +10 -0
index 984534b..7980022 100644
@@ -54,6 +54,7 @@ typedef struct {
     unsigned int play_pos;
     uint8_t data_on;
     uint8_t dummy_refresh_clock;
+    bool migrate;
 } PCSpkState;
 
 static const char *s_spk = "pcspk";
@@ -187,11 +188,19 @@ static void pcspk_realizefn(DeviceState *dev, Error **errp)
     pcspk_state = s;
 }
 
+static bool migrate_needed(void *opaque)
+{
+    PCSpkState *s = opaque;
+
+    return s->migrate;
+}
+
 static const VMStateDescription vmstate_spk = {
     .name = "pcspk",
     .version_id = 1,
     .minimum_version_id = 1,
     .minimum_version_id_old = 1,
+    .needed = migrate_needed,
     .fields      = (VMStateField[]) {
         VMSTATE_UINT8(data_on, PCSpkState),
         VMSTATE_UINT8(dummy_refresh_clock, PCSpkState),
@@ -201,6 +210,7 @@ static const VMStateDescription vmstate_spk = {
 
 static Property pcspk_properties[] = {
     DEFINE_PROP_UINT32("iobase", PCSpkState, iobase,  -1),
+    DEFINE_PROP_BOOL("migrate", PCSpkState, migrate,  true),
     DEFINE_PROP_END_OF_LIST(),
 };
 
Signature · 1 line
2.9.3
#2 lore raw

PATCH 2/2 migration/pcspk: Turn migration of pcspk off for 2.7 and older

In reply to Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

To keep backwards migration compatibility allow us to turn pcspk migration off.

---
 include/hw/i386/pc.h | 5 +++++
 1 file changed, 5 insertions(+)
include/hw/i386/pc.h +5 -0
index 67a1a9e..4b74130 100644
@@ -395,6 +395,11 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
         .driver   = "Opteron_G3" "-" TYPE_X86_CPU,\
         .property = "stepping",\
         .value    = "1",\
+    },\
+    {\
+        .driver   = "isa-pcspk",\
+        .property = "migrate",\
+        .value    = "off",\
     },
 
 #define PC_COMPAT_2_6 \
Signature · 1 line
2.9.3
#3 lore raw
In reply to Dr. David Alan Gilbert (git)
Quoted 15 lines · Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

Hi,
39c88f56 added VMState for pcspk but turned it on for
all machine types, this breaks backwards compatibility to older machine types.

If this is too late for 2.8 then I suggest we take the 1st
of these two patches, which just makes it a property to flip for those of us who need it. Adding the 2nd patch after 2.8 might cause problems for people using 2.8 with 2.7 machine type.

Dave

Quoted 8 lines

Dr. David Alan Gilbert (2):
migration/pcspk: Add a property to state if pcspk is migrated migration/pcspk: Turn migration of pcspk off for 2.7 and older

 hw/audio/pcspk.c     | 10 ++++++++++
 include/hw/i386/pc.h |  5 +++++
 2 files changed, 15 insertions(+)

Wow, I didn't know optional sections existed. We could have used it for hw/char/parallel.c, but that was added only a couple months before optional sections.

Queued for 2.8.

Paolo

#4 lore raw
Quoted 22 lines · * Paolo Bonzini ([email protected]) wrote:
Quoted 19 lines · Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

Hi,
39c88f56 added VMState for pcspk but turned it on for
all machine types, this breaks backwards compatibility to older machine types.

If this is too late for 2.8 then I suggest we take the 1st
of these two patches, which just makes it a property to flip for those of us who need it. Adding the 2nd patch after 2.8 might cause problems for people using 2.8 with 2.7 machine type.

Dave

Dr. David Alan Gilbert (2):
migration/pcspk: Add a property to state if pcspk is migrated migration/pcspk: Turn migration of pcspk off for 2.7 and older

Quoted 8 lines
Quoted 4 lines
 hw/audio/pcspk.c     | 10 ++++++++++
 include/hw/i386/pc.h |  5 +++++
 2 files changed, 15 insertions(+)

Wow, I didn't know optional sections existed. We could have used it for hw/char/parallel.c, but that was added only a couple months before optional sections.

I'd forgotten that they existed and was about to write them to solve this problem, and then found the code I apparently reviewed a year or so ago :-)

Quoted 1 line

Queued for 2.8.

Thanks.

Dave

Quoted 1 line

Paolo

--
Dr. David Alan Gilbert / [email protected] / Manchester, UK

Reviewed-by on the v1 series
#5 lore raw
In reply to Dr. David Alan Gilbert (git)
Quoted 7 lines · Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

Hi,
39c88f56 added VMState for pcspk but turned it on for
all machine types, this breaks backwards compatibility to older machine types.

Quoted 5 lines

If this is too late for 2.8 then I suggest we take the 1st
of these two patches, which just makes it a property to flip for those of us who need it. Adding the 2nd patch after 2.8 might cause problems for people using 2.8 with 2.7 machine type.

We should take both for 2.8, thanks!

Amit