All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/4] Fix VMState design flaws
@ 2009-09-14 20:15 Juan Quintela
  2009-09-14 20:15 ` [Qemu-devel] [PATCH 1/4] vmstate: remove const for put operations Juan Quintela
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Juan Quintela @ 2009-09-14 20:15 UTC (permalink / raw)
  To: qemu-devel

Hi

I know you are not going to believe it, but VMState design have flaws:

(everybody) Ooohhhhhhh!!!!

This patches fixes (some) of them:

  * When we added .pre_save() and .post_save(), we have it basically
    to change values in the variable pointed by the "void *opaque".
    Without this functions, it made sense that this variables were const,
    with them, it means that we have to do
      FooState *s = (void *)opaque
    in every pre/post_save() function.  Just remove the const.
  * Add version_id field to post_load().  Now we can assign default values
    in post_load for old versions of the state (ps2_kbd as example).
    This one is also needed for ioapic in qemu-kvm.
  * Add support for sending partial struct arrays (i.e. only some fields starting
    from the beggining) fdc cleanups for getting pc98 in need it.

That is all for now :)

Later, Juan.

Juan Quintela (4):
  vmstate: remove const for put operations
  vmstate: add version_id argument to post_load
  vmstate: remove ps2_kbd_load_old()
  vmstate: Add support for sending partial arrays

 exec.c             |    2 +-
 hw/acpi.c          |    2 +-
 hw/cirrus_vga.c    |    2 +-
 hw/dma.c           |    2 +-
 hw/fdc.c           |    2 +-
 hw/hpet.c          |    2 +-
 hw/hw.h            |   18 +++++++++++++++---
 hw/pci.c           |    2 +-
 hw/piix_pci.c      |    2 +-
 hw/ps2.c           |   14 +++-----------
 hw/ptimer.c        |    4 ++--
 hw/serial.c        |    2 +-
 hw/slavio_intctl.c |    2 +-
 hw/tcx.c           |    2 +-
 hw/vmmouse.c       |    2 +-
 savevm.c           |   48 ++++++++++++++++++++++++------------------------
 16 files changed, 56 insertions(+), 52 deletions(-)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH 1/4] vmstate: remove const for put operations
  2009-09-14 20:15 [Qemu-devel] [PATCH 0/4] Fix VMState design flaws Juan Quintela
@ 2009-09-14 20:15 ` Juan Quintela
  2009-09-14 20:15 ` [Qemu-devel] [PATCH 2/4] vmstate: add version_id argument to post_load Juan Quintela
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Juan Quintela @ 2009-09-14 20:15 UTC (permalink / raw)
  To: qemu-devel

In a later patch, we introduce pre_save() and post_save() functions.
The whole point of that operation is to change things in the state.
Without this patch, we have to remove the const qualifier in each
use with a cast

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/hw.h     |    4 ++--
 hw/pci.c    |    2 +-
 hw/ptimer.c |    4 ++--
 savevm.c    |   46 +++++++++++++++++++++++-----------------------
 4 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/hw/hw.h b/hw/hw.h
index cf266b3..e407815 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -276,7 +276,7 @@ typedef struct VMStateDescription VMStateDescription;
 struct VMStateInfo {
     const char *name;
     int (*get)(QEMUFile *f, void *pv, size_t size);
-    void (*put)(QEMUFile *f, const void *pv, size_t size);
+    void (*put)(QEMUFile *f, void *pv, size_t size);
 };

 enum VMStateFlags {
@@ -534,7 +534,7 @@ extern const VMStateDescription vmstate_pci_device;
 extern int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
                               void *opaque, int version_id);
 extern void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
-                               const void *opaque);
+                               void *opaque);
 extern int vmstate_register(int instance_id, const VMStateDescription *vmsd,
                             void *base);
 void vmstate_unregister(const VMStateDescription *vmsd, void *opaque);
diff --git a/hw/pci.c b/hw/pci.c
index c12b0be..7d7d619 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -156,7 +156,7 @@ static int get_pci_config_device(QEMUFile *f, void *pv, size_t size)
 }

 /* just put buffer */
-static void put_pci_config_device(QEMUFile *f, const void *pv, size_t size)
+static void put_pci_config_device(QEMUFile *f, void *pv, size_t size)
 {
     const uint8_t *v = pv;
     qemu_put_buffer(f, v, size);
diff --git a/hw/ptimer.c b/hw/ptimer.c
index a4343b6..4ddbc59 100644
--- a/hw/ptimer.c
+++ b/hw/ptimer.c
@@ -220,9 +220,9 @@ static int get_ptimer(QEMUFile *f, void *pv, size_t size)
     return 0;
 }

-static void put_ptimer(QEMUFile *f, const void *pv, size_t size)
+static void put_ptimer(QEMUFile *f, void *pv, size_t size)
 {
-    ptimer_state *v = (void *)pv;
+    ptimer_state *v = pv;

     qemu_put_ptimer(f, v);
 }
diff --git a/savevm.c b/savevm.c
index fd767be..b36c657 100644
--- a/savevm.c
+++ b/savevm.c
@@ -649,9 +649,9 @@ static int get_int8(QEMUFile *f, void *pv, size_t size)
     return 0;
 }

-static void put_int8(QEMUFile *f, const void *pv, size_t size)
+static void put_int8(QEMUFile *f, void *pv, size_t size)
 {
-    const int8_t *v = pv;
+    int8_t *v = pv;
     qemu_put_s8s(f, v);
 }

@@ -670,9 +670,9 @@ static int get_int16(QEMUFile *f, void *pv, size_t size)
     return 0;
 }

-static void put_int16(QEMUFile *f, const void *pv, size_t size)
+static void put_int16(QEMUFile *f, void *pv, size_t size)
 {
-    const int16_t *v = pv;
+    int16_t *v = pv;
     qemu_put_sbe16s(f, v);
 }

@@ -691,9 +691,9 @@ static int get_int32(QEMUFile *f, void *pv, size_t size)
     return 0;
 }

-static void put_int32(QEMUFile *f, const void *pv, size_t size)
+static void put_int32(QEMUFile *f, void *pv, size_t size)
 {
-    const int32_t *v = pv;
+    int32_t *v = pv;
     qemu_put_sbe32s(f, v);
 }

@@ -752,9 +752,9 @@ static int get_int64(QEMUFile *f, void *pv, size_t size)
     return 0;
 }

-static void put_int64(QEMUFile *f, const void *pv, size_t size)
+static void put_int64(QEMUFile *f, void *pv, size_t size)
 {
-    const int64_t *v = pv;
+    int64_t *v = pv;
     qemu_put_sbe64s(f, v);
 }

@@ -773,9 +773,9 @@ static int get_uint8(QEMUFile *f, void *pv, size_t size)
     return 0;
 }

-static void put_uint8(QEMUFile *f, const void *pv, size_t size)
+static void put_uint8(QEMUFile *f, void *pv, size_t size)
 {
-    const uint8_t *v = pv;
+    uint8_t *v = pv;
     qemu_put_8s(f, v);
 }

@@ -794,9 +794,9 @@ static int get_uint16(QEMUFile *f, void *pv, size_t size)
     return 0;
 }

-static void put_uint16(QEMUFile *f, const void *pv, size_t size)
+static void put_uint16(QEMUFile *f, void *pv, size_t size)
 {
-    const uint16_t *v = pv;
+    uint16_t *v = pv;
     qemu_put_be16s(f, v);
 }

@@ -815,9 +815,9 @@ static int get_uint32(QEMUFile *f, void *pv, size_t size)
     return 0;
 }

-static void put_uint32(QEMUFile *f, const void *pv, size_t size)
+static void put_uint32(QEMUFile *f, void *pv, size_t size)
 {
-    const uint32_t *v = pv;
+    uint32_t *v = pv;
     qemu_put_be32s(f, v);
 }

@@ -836,9 +836,9 @@ static int get_uint64(QEMUFile *f, void *pv, size_t size)
     return 0;
 }

-static void put_uint64(QEMUFile *f, const void *pv, size_t size)
+static void put_uint64(QEMUFile *f, void *pv, size_t size)
 {
-    const uint64_t *v = pv;
+    uint64_t *v = pv;
     qemu_put_be64s(f, v);
 }

@@ -877,9 +877,9 @@ static int get_timer(QEMUFile *f, void *pv, size_t size)
     return 0;
 }

-static void put_timer(QEMUFile *f, const void *pv, size_t size)
+static void put_timer(QEMUFile *f, void *pv, size_t size)
 {
-    QEMUTimer *v = (void *)pv;
+    QEMUTimer *v = pv;
     qemu_put_timer(f, v);
 }

@@ -898,9 +898,9 @@ static int get_buffer(QEMUFile *f, void *pv, size_t size)
     return 0;
 }

-static void put_buffer(QEMUFile *f, const void *pv, size_t size)
+static void put_buffer(QEMUFile *f, void *pv, size_t size)
 {
-    uint8_t *v = (void *)pv;
+    uint8_t *v = pv;
     qemu_put_buffer(f, v, size);
 }

@@ -1090,7 +1090,7 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
 }

 void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
-                        const void *opaque)
+                        void *opaque)
 {
     VMStateField *field = vmsd->fields;

@@ -1098,7 +1098,7 @@ void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
         vmsd->pre_save(opaque);
     }
     while(field->name) {
-        const void *base_addr = opaque + field->offset;
+        void *base_addr = opaque + field->offset;
         int i, n_elems = 1;

         if (field->flags & VMS_ARRAY) {
@@ -1110,7 +1110,7 @@ void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
             base_addr = *(void **)base_addr;
         }
         for (i = 0; i < n_elems; i++) {
-            const void *addr = base_addr + field->size * i;
+            void *addr = base_addr + field->size * i;

             if (field->flags & VMS_STRUCT) {
                 vmstate_save_state(f, field->vmsd, addr);
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH 2/4] vmstate: add version_id argument to post_load
  2009-09-14 20:15 [Qemu-devel] [PATCH 0/4] Fix VMState design flaws Juan Quintela
  2009-09-14 20:15 ` [Qemu-devel] [PATCH 1/4] vmstate: remove const for put operations Juan Quintela
@ 2009-09-14 20:15 ` Juan Quintela
  2009-09-14 20:15 ` [Qemu-devel] [PATCH 3/4] vmstate: remove ps2_kbd_load_old() Juan Quintela
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Juan Quintela @ 2009-09-14 20:15 UTC (permalink / raw)
  To: qemu-devel


Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 exec.c             |    2 +-
 hw/acpi.c          |    2 +-
 hw/cirrus_vga.c    |    2 +-
 hw/dma.c           |    2 +-
 hw/fdc.c           |    2 +-
 hw/hpet.c          |    2 +-
 hw/hw.h            |    2 +-
 hw/piix_pci.c      |    2 +-
 hw/serial.c        |    2 +-
 hw/slavio_intctl.c |    2 +-
 hw/tcx.c           |    2 +-
 hw/vmmouse.c       |    2 +-
 savevm.c           |    2 +-
 13 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/exec.c b/exec.c
index c82e767..85076e6 100644
--- a/exec.c
+++ b/exec.c
@@ -528,7 +528,7 @@ static int cpu_common_pre_load(void *opaque)
     return 0;
 }

-static int cpu_common_post_load(void *opaque)
+static int cpu_common_post_load(void *opaque, int version_id)
 {
     CPUState *env = opaque;

diff --git a/hw/acpi.c b/hw/acpi.c
index b14b9f4..e67da6c 100644
--- a/hw/acpi.c
+++ b/hw/acpi.c
@@ -441,7 +441,7 @@ static void pm_write_config(PCIDevice *d,
         pm_io_space_update((PIIX4PMState *)d);
 }

-static int vmstate_acpi_post_load(void *opaque)
+static int vmstate_acpi_post_load(void *opaque, int version_id)
 {
     PIIX4PMState *s = opaque;

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index 7e95f10..004ae7d 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -2955,7 +2955,7 @@ static CPUWriteMemoryFunc * const cirrus_mmio_write[3] = {

 /* load/save state */

-static int cirrus_post_load(void *opaque)
+static int cirrus_post_load(void *opaque, int version_id)
 {
     CirrusVGAState *s = opaque;

diff --git a/hw/dma.c b/hw/dma.c
index f418e42..44c642e 100644
--- a/hw/dma.c
+++ b/hw/dma.c
@@ -517,7 +517,7 @@ static const VMStateDescription vmstate_dma_regs = {
     }
 };

-static int dma_post_load(void *opaque)
+static int dma_post_load(void *opaque, int version_id)
 {
     DMA_run();

diff --git a/hw/fdc.c b/hw/fdc.c
index 389d9e6..c03ab47 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -650,7 +650,7 @@ static void fdc_pre_save(const void *opaque)
     s->dor_vmstate = s->dor | GET_CUR_DRV(s);
 }

-static int fdc_post_load(void *opaque)
+static int fdc_post_load(void *opaque, int version_id)
 {
     fdctrl_t *s = opaque;

diff --git a/hw/hpet.c b/hw/hpet.c
index c1ead34..6535b8e 100644
--- a/hw/hpet.c
+++ b/hw/hpet.c
@@ -164,7 +164,7 @@ static void hpet_pre_save(const void *opaque)
     s->hpet_counter = hpet_get_ticks();
 }

-static int hpet_post_load(void *opaque)
+static int hpet_post_load(void *opaque, int version_id)
 {
     HPETState *s = opaque;

diff --git a/hw/hw.h b/hw/hw.h
index e407815..6f60493 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -307,7 +307,7 @@ struct VMStateDescription {
     int minimum_version_id_old;
     LoadStateHandler *load_state_old;
     int (*pre_load)(void *opaque);
-    int (*post_load)(void *opaque);
+    int (*post_load)(void *opaque, int version_id);
     void (*pre_save)(const void *opaque);
     void (*post_save)(const void *opaque);
     VMStateField *fields;
diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index edd6df0..5c2bb92 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -172,7 +172,7 @@ static int i440fx_load_old(QEMUFile* f, void *opaque, int version_id)
     return 0;
 }

-static int i440fx_post_load(void *opaque)
+static int i440fx_post_load(void *opaque, int version_id)
 {
     PCII440FXState *d = opaque;

diff --git a/hw/serial.c b/hw/serial.c
index 6f7b30e..ae14021 100644
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -650,7 +650,7 @@ static int serial_pre_load(void *opaque)
     return 0;
 }

-static int serial_post_load(void *opaque)
+static int serial_post_load(void *opaque, int version_id)
 {
     SerialState *s = opaque;

diff --git a/hw/slavio_intctl.c b/hw/slavio_intctl.c
index 6a95f9e..ab29ee2 100644
--- a/hw/slavio_intctl.c
+++ b/hw/slavio_intctl.c
@@ -374,7 +374,7 @@ static void slavio_set_irq_all(void *opaque, int irq, int level)
     }
 }

-static int vmstate_intctl_post_load(void *opaque)
+static int vmstate_intctl_post_load(void *opaque, int version_id)
 {
     SLAVIO_INTCTLState *s = opaque;

diff --git a/hw/tcx.c b/hw/tcx.c
index 012d01b..3816c53 100644
--- a/hw/tcx.c
+++ b/hw/tcx.c
@@ -378,7 +378,7 @@ static void tcx24_invalidate_display(void *opaque)
     qemu_console_resize(s->ds, s->width, s->height);
 }

-static int vmstate_tcx_post_load(void *opaque)
+static int vmstate_tcx_post_load(void *opaque, int version_id)
 {
     TCXState *s = opaque;

diff --git a/hw/vmmouse.c b/hw/vmmouse.c
index c207bb2..bb6e605 100644
--- a/hw/vmmouse.c
+++ b/hw/vmmouse.c
@@ -235,7 +235,7 @@ static uint32_t vmmouse_ioport_read(void *opaque, uint32_t addr)
     return data[0];
 }

-static int vmmouse_post_load(void *opaque)
+static int vmmouse_post_load(void *opaque, int version_id)
 {
     VMMouseState *s = opaque;

diff --git a/savevm.c b/savevm.c
index b36c657..fefde7c 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1084,7 +1084,7 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
         field++;
     }
     if (vmsd->post_load) {
-        return vmsd->post_load(opaque);
+        return vmsd->post_load(opaque, version_id);
     }
     return 0;
 }
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH 3/4] vmstate: remove ps2_kbd_load_old()
  2009-09-14 20:15 [Qemu-devel] [PATCH 0/4] Fix VMState design flaws Juan Quintela
  2009-09-14 20:15 ` [Qemu-devel] [PATCH 1/4] vmstate: remove const for put operations Juan Quintela
  2009-09-14 20:15 ` [Qemu-devel] [PATCH 2/4] vmstate: add version_id argument to post_load Juan Quintela
@ 2009-09-14 20:15 ` Juan Quintela
  2009-09-14 20:45   ` [Qemu-devel] " Paolo Bonzini
  2009-09-14 20:15 ` [Qemu-devel] [PATCH 4/4] vmstate: Add support for sending partial arrays Juan Quintela
  2009-09-14 20:48 ` [Qemu-devel] Re: [PATCH 0/4] Fix VMState design flaws Paolo Bonzini
  4 siblings, 1 reply; 10+ messages in thread
From: Juan Quintela @ 2009-09-14 20:15 UTC (permalink / raw)
  To: qemu-devel

Now that we have version_id on post_load() we don't need the old load
function

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/ps2.c |   14 +++-----------
 1 files changed, 3 insertions(+), 11 deletions(-)

diff --git a/hw/ps2.c b/hw/ps2.c
index fff4d13..6fe4b59 100644
--- a/hw/ps2.c
+++ b/hw/ps2.c
@@ -541,19 +541,11 @@ static const VMStateDescription vmstate_ps2_common = {
     }
 };

-static int ps2_kbd_load_old(QEMUFile* f, void* opaque, int version_id)
+static int ps2_kbd_post_load(void* opaque, int version_id)
 {
     PS2KbdState *s = (PS2KbdState*)opaque;

-    if (version_id != 2 && version_id != 3)
-        return -EINVAL;
-
-    vmstate_load_state(f, &vmstate_ps2_common, &s->common, version_id);
-    s->scan_enabled=qemu_get_be32(f);
-    s->translate=qemu_get_be32(f);
-    if (version_id == 3)
-        s->scancode_set=qemu_get_be32(f);
-    else
+    if (version_id == 2)
         s->scancode_set=2;
     return 0;
 }
@@ -563,7 +555,7 @@ static const VMStateDescription vmstate_ps2_keyboard = {
     .version_id = 3,
     .minimum_version_id = 3,
     .minimum_version_id_old = 2,
-    .load_state_old = ps2_kbd_load_old,
+    .post_load = ps2_kbd_post_load,
     .fields      = (VMStateField []) {
         VMSTATE_STRUCT(common, PS2KbdState, 0, vmstate_ps2_common, PS2State),
         VMSTATE_INT32(scan_enabled, PS2KbdState),
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH 4/4] vmstate: Add support for sending partial arrays
  2009-09-14 20:15 [Qemu-devel] [PATCH 0/4] Fix VMState design flaws Juan Quintela
                   ` (2 preceding siblings ...)
  2009-09-14 20:15 ` [Qemu-devel] [PATCH 3/4] vmstate: remove ps2_kbd_load_old() Juan Quintela
@ 2009-09-14 20:15 ` Juan Quintela
  2009-09-14 20:48 ` [Qemu-devel] Re: [PATCH 0/4] Fix VMState design flaws Paolo Bonzini
  4 siblings, 0 replies; 10+ messages in thread
From: Juan Quintela @ 2009-09-14 20:15 UTC (permalink / raw)
  To: qemu-devel

This one is needed for changees happening on fdc.  It allows you to send
arrays of structs whose size we want to send it is another field with type
uint8_t.  (If you have been able to read the whole sentence without
stoping for breathing, you can use it.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/hw.h |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/hw/hw.h b/hw/hw.h
index 6f60493..823c132 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -398,6 +398,18 @@ extern const VMStateInfo vmstate_info_buffer;
         + type_check_array(_type,typeof_field(_state, _field),_num)  \
 }

+#define VMSTATE_STRUCT_ARRAY_SIZE_UINT8(_field, _state, _field__num, _version, _vmsd, _type) { \
+    .name       = (stringify(_field)),                               \
+    .num_offset = offsetof(_state, _field_num)                       \
+        + type_check(uint8_t,typeof_field(_state, _field_num)),       \
+    .version_id = (_version),                                        \
+    .vmsd       = &(_vmsd),                                          \
+    .size       = sizeof(_type),                                     \
+    .flags      = VMS_STRUCT|VMS_ARRAY,                              \
+    .offset     = offsetof(_state, _field)                           \
+        + type_check_array(_type,typeof_field(_state, _field),_num)  \
+}
+
 #define VMSTATE_STATIC_BUFFER(_field, _state, _version) {            \
     .name       = (stringify(_field)),                               \
     .version_id = (_version),                                        \
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] Re: [PATCH 3/4] vmstate: remove ps2_kbd_load_old()
  2009-09-14 20:15 ` [Qemu-devel] [PATCH 3/4] vmstate: remove ps2_kbd_load_old() Juan Quintela
@ 2009-09-14 20:45   ` Paolo Bonzini
  2009-09-14 20:48     ` Juan Quintela
  0 siblings, 1 reply; 10+ messages in thread
From: Paolo Bonzini @ 2009-09-14 20:45 UTC (permalink / raw)
  To: Juan Quintela; +Cc: qemu-devel

On 09/14/2009 10:15 PM, Juan Quintela wrote:
>     .minimum_version_id = 3,

This must become 2.

Paolo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Qemu-devel] Re: [PATCH 3/4] vmstate: remove ps2_kbd_load_old()
  2009-09-14 20:45   ` [Qemu-devel] " Paolo Bonzini
@ 2009-09-14 20:48     ` Juan Quintela
  0 siblings, 0 replies; 10+ messages in thread
From: Juan Quintela @ 2009-09-14 20:48 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel

Paolo Bonzini <bonzini@gnu.org> wrote:
> On 09/14/2009 10:15 PM, Juan Quintela wrote:
>>     .minimum_version_id = 3,
>
> This must become 2.

you are right.

Later, Juan.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Qemu-devel] Re: [PATCH 0/4] Fix VMState design flaws
  2009-09-14 20:15 [Qemu-devel] [PATCH 0/4] Fix VMState design flaws Juan Quintela
                   ` (3 preceding siblings ...)
  2009-09-14 20:15 ` [Qemu-devel] [PATCH 4/4] vmstate: Add support for sending partial arrays Juan Quintela
@ 2009-09-14 20:48 ` Paolo Bonzini
  2009-09-14 20:50   ` Juan Quintela
  4 siblings, 1 reply; 10+ messages in thread
From: Paolo Bonzini @ 2009-09-14 20:48 UTC (permalink / raw)
  To: Juan Quintela; +Cc: qemu-devel


>    * Add version_id field to post_load().  Now we can assign default values
>      in post_load for old versions of the state (ps2_kbd as example).

While this is a good idea, why don't we first reset the devices upon 
load, which would also set the default values?...

Paolo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Qemu-devel] Re: [PATCH 0/4] Fix VMState design flaws
  2009-09-14 20:48 ` [Qemu-devel] Re: [PATCH 0/4] Fix VMState design flaws Paolo Bonzini
@ 2009-09-14 20:50   ` Juan Quintela
  2009-09-14 21:30     ` Paolo Bonzini
  0 siblings, 1 reply; 10+ messages in thread
From: Juan Quintela @ 2009-09-14 20:50 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel

Paolo Bonzini <bonzini@gnu.org> wrote:
>>    * Add version_id field to post_load().  Now we can assign default values
>>      in post_load for old versions of the state (ps2_kbd as example).
>
> While this is a good idea, why don't we first reset the devices upon
> load, which would also set the default values?...

It is safe to do that unconditionally?
If so, I am all for it.  We can:
- add reset field to vmstate
- add default values to some fields, if it makse sense (haven't yet
  looked at reset code).

Later, Juan.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Qemu-devel] Re: [PATCH 0/4] Fix VMState design flaws
  2009-09-14 20:50   ` Juan Quintela
@ 2009-09-14 21:30     ` Paolo Bonzini
  0 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2009-09-14 21:30 UTC (permalink / raw)
  To: Juan Quintela; +Cc: qemu-devel

On 09/14/2009 10:50 PM, Juan Quintela wrote:
> Paolo Bonzini<bonzini@gnu.org>  wrote:
>>>     * Add version_id field to post_load().  Now we can assign default values
>>>       in post_load for old versions of the state (ps2_kbd as example).
>>
>> While this is a good idea, why don't we first reset the devices upon
>> load, which would also set the default values?...
>
> It is safe to do that unconditionally?
> If so, I am all for it.  We can:
> - add reset field to vmstate
> - add default values to some fields, if it makse sense (haven't yet
>    looked at reset code).

The PS2 reset is this:

static void ps2_common_reset(PS2State *s)
{
     PS2Queue *q;
     s->write_cmd = -1;
     q = &s->queue;
     q->rptr = 0;
     q->wptr = 0;
     q->count = 0;
     s->update_irq(s->update_arg, 0); // calls kbd_update_kbd_irq
}

static void ps2_kbd_reset(void *opaque)
{
     PS2KbdState *s = (PS2KbdState *) opaque;

     ps2_common_reset(&s->common);
     s->scan_enabled = 0;
     s->translate = 0;
     s->scancode_set = 0;
}

All of these are later reset by load.  The update_irq may seem unsafe, 
but maybe deasserting an IRQ is better than leaving it in whatever state 
it was left by the previous VM...

Paolo

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2009-09-14 21:30 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-09-14 20:15 [Qemu-devel] [PATCH 0/4] Fix VMState design flaws Juan Quintela
2009-09-14 20:15 ` [Qemu-devel] [PATCH 1/4] vmstate: remove const for put operations Juan Quintela
2009-09-14 20:15 ` [Qemu-devel] [PATCH 2/4] vmstate: add version_id argument to post_load Juan Quintela
2009-09-14 20:15 ` [Qemu-devel] [PATCH 3/4] vmstate: remove ps2_kbd_load_old() Juan Quintela
2009-09-14 20:45   ` [Qemu-devel] " Paolo Bonzini
2009-09-14 20:48     ` Juan Quintela
2009-09-14 20:15 ` [Qemu-devel] [PATCH 4/4] vmstate: Add support for sending partial arrays Juan Quintela
2009-09-14 20:48 ` [Qemu-devel] Re: [PATCH 0/4] Fix VMState design flaws Paolo Bonzini
2009-09-14 20:50   ` Juan Quintela
2009-09-14 21:30     ` Paolo Bonzini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.