[Qemu-devel] [PATCH] migration: remove subsections in fdc and rtl8139 and bump versions (v2)

[Anthony Liguori]

As Paolo points out, the migration protocol is ambiguous when using subsections
today.  That means that even if we preserve subsections and change the protocol
accordingly, the old protocol w/subsections is still ambiguous.

Remove subsection usage and bump any device using subsections.  This effectively
eliminates the amiguouity and allows for a clean transition to a new protocol
with unambiguous subsections.

Signed-off-by: Anthony Liguori <address@hidden>
--
v1 -> v2
 - Also remove IDE subsections (spotted by Juan Quintela)
---
 hw/fdc.c      |   37 +++-------------
 hw/ide/core.c |  136 +++++++++++++-------------------------------------------
 hw/ide/pci.c  |   62 +++-----------------------
 hw/rtl8139.c  |   34 +-------------
 4 files changed, 47 insertions(+), 222 deletions(-)

diff --git a/hw/fdc.c b/hw/fdc.c
index edf0360..d8d74c9 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -554,45 +554,20 @@ static int fdrive_media_changed_post_load(void *opaque, 
int version_id)
     return 0;
 }
 
-static bool fdrive_media_changed_needed(void *opaque)
-{
-    FDrive *drive = opaque;
-
-    return (drive->bs != NULL && drive->bs->media_changed != 1);
-}
-
-static const VMStateDescription vmstate_fdrive_media_changed = {
-    .name = "fdrive/media_changed",
-    .version_id = 1,
-    .minimum_version_id = 1,
-    .minimum_version_id_old = 1,
-    .pre_save = fdrive_media_changed_pre_save,
-    .post_load = fdrive_media_changed_post_load,
-    .fields      = (VMStateField[]) {
-        VMSTATE_UINT8(media_changed, FDrive),
-        VMSTATE_END_OF_LIST()
-    }
-};
-
 static const VMStateDescription vmstate_fdrive = {
     .name = "fdrive",
-    .version_id = 1,
-    .minimum_version_id = 1,
-    .minimum_version_id_old = 1,
+    .version_id = 2,
+    .minimum_version_id = 2,
+    .minimum_version_id_old = 2,
+    .pre_save = fdrive_media_changed_pre_save,
+    .post_load = fdrive_media_changed_post_load,
     .fields      = (VMStateField[]) {
         VMSTATE_UINT8(head, FDrive),
         VMSTATE_UINT8(track, FDrive),
         VMSTATE_UINT8(sect, FDrive),
+        VMSTATE_UINT8(media_changed, FDrive),
         VMSTATE_END_OF_LIST()
     },
-    .subsections = (VMStateSubsection[]) {
-        {
-            .vmsd = &vmstate_fdrive_media_changed,
-            .needed = &fdrive_media_changed_needed,
-        } , {
-            /* empty */
-        }
-    }
 };
 
 static void fdc_pre_save(void *opaque)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index d145b19..c2137f9 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1916,19 +1916,6 @@ static int transfer_end_table_idx(EndTransferFunc *fn)
     return -1;
 }
 
-static int ide_drive_post_load(void *opaque, int version_id)
-{
-    IDEState *s = opaque;
-
-    if (version_id < 3) {
-        if (s->sense_key == SENSE_UNIT_ATTENTION &&
-            s->asc == ASC_MEDIUM_MAY_HAVE_CHANGED) {
-            s->cdrom_changed = 1;
-        }
-    }
-    return 0;
-}
-
 static int ide_drive_pio_post_load(void *opaque, int version_id)
 {
     IDEState *s = opaque;
@@ -1943,6 +1930,20 @@ static int ide_drive_pio_post_load(void *opaque, int 
version_id)
     return 0;
 }
 
+static int ide_drive_post_load(void *opaque, int version_id)
+{
+    IDEState *s = opaque;
+
+    if (version_id < 3) {
+        if (s->sense_key == SENSE_UNIT_ATTENTION &&
+            s->asc == ASC_MEDIUM_MAY_HAVE_CHANGED) {
+            s->cdrom_changed = 1;
+        }
+    }
+
+    return ide_drive_pio_post_load(s, version_id);
+}
+
 static void ide_drive_pio_pre_save(void *opaque)
 {
     IDEState *s = opaque;
@@ -1961,66 +1962,12 @@ static void ide_drive_pio_pre_save(void *opaque)
     }
 }
 
-static bool ide_drive_pio_state_needed(void *opaque)
-{
-    IDEState *s = opaque;
-
-    return ((s->status & DRQ_STAT) != 0)
-        || (s->bus->error_status & BM_STATUS_PIO_RETRY);
-}
-
-static bool ide_atapi_gesn_needed(void *opaque)
-{
-    IDEState *s = opaque;
-
-    return s->events.new_media || s->events.eject_request;
-}
-
-static bool ide_error_needed(void *opaque)
-{
-    IDEBus *bus = opaque;
-
-    return (bus->error_status != 0);
-}
-
-/* Fields for GET_EVENT_STATUS_NOTIFICATION ATAPI command */
-const VMStateDescription vmstate_ide_atapi_gesn_state = {
-    .name ="ide_drive/atapi/gesn_state",
-    .version_id = 1,
-    .minimum_version_id = 1,
-    .minimum_version_id_old = 1,
-    .fields = (VMStateField []) {
-        VMSTATE_BOOL(events.new_media, IDEState),
-        VMSTATE_BOOL(events.eject_request, IDEState),
-        VMSTATE_END_OF_LIST()
-    }
-};
-
-const VMStateDescription vmstate_ide_drive_pio_state = {
-    .name = "ide_drive/pio_state",
-    .version_id = 1,
-    .minimum_version_id = 1,
-    .minimum_version_id_old = 1,
-    .pre_save = ide_drive_pio_pre_save,
-    .post_load = ide_drive_pio_post_load,
-    .fields      = (VMStateField []) {
-        VMSTATE_INT32(req_nb_sectors, IDEState),
-        VMSTATE_VARRAY_INT32(io_buffer, IDEState, io_buffer_total_len, 1,
-                            vmstate_info_uint8, uint8_t),
-        VMSTATE_INT32(cur_io_buffer_offset, IDEState),
-        VMSTATE_INT32(cur_io_buffer_len, IDEState),
-        VMSTATE_UINT8(end_transfer_fn_idx, IDEState),
-        VMSTATE_INT32(elementary_transfer_size, IDEState),
-        VMSTATE_INT32(packet_transfer_size, IDEState),
-        VMSTATE_END_OF_LIST()
-    }
-};
-
 const VMStateDescription vmstate_ide_drive = {
     .name = "ide_drive",
-    .version_id = 3,
-    .minimum_version_id = 0,
-    .minimum_version_id_old = 0,
+    .version_id = 4,
+    .minimum_version_id = 4,
+    .minimum_version_id_old = 4,
+    .pre_save = ide_drive_pio_pre_save,
     .post_load = ide_drive_post_load,
     .fields      = (VMStateField []) {
         VMSTATE_INT32(mult_sectors, IDEState),
@@ -2043,50 +1990,31 @@ const VMStateDescription vmstate_ide_drive = {
         VMSTATE_UINT8(sense_key, IDEState),
         VMSTATE_UINT8(asc, IDEState),
         VMSTATE_UINT8_V(cdrom_changed, IDEState, 3),
+        VMSTATE_INT32(req_nb_sectors, IDEState),
+        VMSTATE_VARRAY_INT32(io_buffer, IDEState, io_buffer_total_len, 1,
+                            vmstate_info_uint8, uint8_t),
+        VMSTATE_INT32(cur_io_buffer_offset, IDEState),
+        VMSTATE_INT32(cur_io_buffer_len, IDEState),
+        VMSTATE_UINT8(end_transfer_fn_idx, IDEState),
+        VMSTATE_INT32(elementary_transfer_size, IDEState),
+        VMSTATE_INT32(packet_transfer_size, IDEState),
+        VMSTATE_BOOL(events.new_media, IDEState),
+        VMSTATE_BOOL(events.eject_request, IDEState),
         VMSTATE_END_OF_LIST()
     },
-    .subsections = (VMStateSubsection []) {
-        {
-            .vmsd = &vmstate_ide_drive_pio_state,
-            .needed = ide_drive_pio_state_needed,
-        }, {
-            .vmsd = &vmstate_ide_atapi_gesn_state,
-            .needed = ide_atapi_gesn_needed,
-        }, {
-            /* empty */
-        }
-    }
-};
-
-const VMStateDescription vmstate_ide_error_status = {
-    .name ="ide_bus/error",
-    .version_id = 1,
-    .minimum_version_id = 1,
-    .minimum_version_id_old = 1,
-    .fields = (VMStateField []) {
-        VMSTATE_INT32(error_status, IDEBus),
-        VMSTATE_END_OF_LIST()
-    }
 };
 
 const VMStateDescription vmstate_ide_bus = {
     .name = "ide_bus",
-    .version_id = 1,
-    .minimum_version_id = 1,
-    .minimum_version_id_old = 1,
+    .version_id = 2,
+    .minimum_version_id = 2,
+    .minimum_version_id_old = 2,
     .fields      = (VMStateField []) {
         VMSTATE_UINT8(cmd, IDEBus),
         VMSTATE_UINT8(unit, IDEBus),
+        VMSTATE_INT32(error_status, IDEBus),
         VMSTATE_END_OF_LIST()
     },
-    .subsections = (VMStateSubsection []) {
-        {
-            .vmsd = &vmstate_ide_error_status,
-            .needed = ide_error_needed,
-        }, {
-            /* empty */
-        }
-    }
 };
 
 void ide_drive_get(DriveInfo **hd, int max_bus)
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 9f3050a..26201b4 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -359,25 +359,6 @@ const IORangeOps bmdma_addr_ioport_ops = {
     .write = bmdma_addr_write,
 };
 
-static bool ide_bmdma_current_needed(void *opaque)
-{
-    BMDMAState *bm = opaque;
-
-    return (bm->cur_prd_len != 0);
-}
-
-static bool ide_bmdma_status_needed(void *opaque)
-{
-    BMDMAState *bm = opaque;
-
-    /* Older versions abused some bits in the status register for internal
-     * error state. If any of these bits are set, we must add a subsection to
-     * transfer the real status register */
-    uint8_t abused_bits = BM_MIGRATION_COMPAT_STATUS_BITS;
-
-    return ((bm->status & abused_bits) != 0);
-}
-
 static void ide_bmdma_pre_save(void *opaque)
 {
     BMDMAState *bm = opaque;
@@ -403,34 +384,9 @@ static int ide_bmdma_post_load(void *opaque, int 
version_id)
     return 0;
 }
 
-static const VMStateDescription vmstate_bmdma_current = {
-    .name = "ide bmdma_current",
-    .version_id = 1,
-    .minimum_version_id = 1,
-    .minimum_version_id_old = 1,
-    .fields      = (VMStateField []) {
-        VMSTATE_UINT32(cur_addr, BMDMAState),
-        VMSTATE_UINT32(cur_prd_last, BMDMAState),
-        VMSTATE_UINT32(cur_prd_addr, BMDMAState),
-        VMSTATE_UINT32(cur_prd_len, BMDMAState),
-        VMSTATE_END_OF_LIST()
-    }
-};
-
-const VMStateDescription vmstate_bmdma_status = {
-    .name ="ide bmdma/status",
-    .version_id = 1,
-    .minimum_version_id = 1,
-    .minimum_version_id_old = 1,
-    .fields = (VMStateField []) {
-        VMSTATE_UINT8(status, BMDMAState),
-        VMSTATE_END_OF_LIST()
-    }
-};
-
 static const VMStateDescription vmstate_bmdma = {
     .name = "ide bmdma",
-    .version_id = 3,
+    .version_id = 4,
     .minimum_version_id = 0,
     .minimum_version_id_old = 0,
     .pre_save  = ide_bmdma_pre_save,
@@ -441,19 +397,13 @@ static const VMStateDescription vmstate_bmdma = {
         VMSTATE_INT64(sector_num, BMDMAState),
         VMSTATE_UINT32(nsector, BMDMAState),
         VMSTATE_UINT8(unit, BMDMAState),
+        VMSTATE_UINT32(cur_addr, BMDMAState),
+        VMSTATE_UINT32(cur_prd_last, BMDMAState),
+        VMSTATE_UINT32(cur_prd_addr, BMDMAState),
+        VMSTATE_UINT32(cur_prd_len, BMDMAState),
+        VMSTATE_UINT8(status, BMDMAState),
         VMSTATE_END_OF_LIST()
     },
-    .subsections = (VMStateSubsection []) {
-        {
-            .vmsd = &vmstate_bmdma_current,
-            .needed = ide_bmdma_current_needed,
-        }, {
-            .vmsd = &vmstate_bmdma_status,
-            .needed = ide_bmdma_status_needed,
-        }, {
-            /* empty */
-        }
-    }
 };
 
 static int ide_pci_post_load(void *opaque, int version_id)
diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index 5214b8c..11951f2 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -505,9 +505,6 @@ typedef struct RTL8139State {
     /* PCI interrupt timer */
     QEMUTimer *timer;
     int64_t TimerExpire;
-
-    /* Support migration to/from old versions */
-    int rtl8139_mmio_io_addr_dummy;
 } RTL8139State;
 
 static void rtl8139_set_next_tctr_time(RTL8139State *s, int64_t current_time);
@@ -3259,21 +3256,6 @@ static int rtl8139_post_load(void *opaque, int 
version_id)
     return 0;
 }
 
-static bool rtl8139_hotplug_ready_needed(void *opaque)
-{
-    return qdev_machine_modified();
-}
-
-static const VMStateDescription vmstate_rtl8139_hotplug_ready ={
-    .name = "rtl8139/hotplug_ready",
-    .version_id = 1,
-    .minimum_version_id = 1,
-    .minimum_version_id_old = 1,
-    .fields      = (VMStateField []) {
-        VMSTATE_END_OF_LIST()
-    }
-};
-
 static void rtl8139_pre_save(void *opaque)
 {
     RTL8139State* s = opaque;
@@ -3283,14 +3265,13 @@ static void rtl8139_pre_save(void *opaque)
     rtl8139_set_next_tctr_time(s, current_time);
     s->TCTR = muldiv64(current_time - s->TCTR_base, PCI_FREQUENCY,
                        get_ticks_per_sec());
-    s->rtl8139_mmio_io_addr_dummy = s->rtl8139_mmio_io_addr;
 }
 
 static const VMStateDescription vmstate_rtl8139 = {
     .name = "rtl8139",
-    .version_id = 4,
-    .minimum_version_id = 3,
-    .minimum_version_id_old = 3,
+    .version_id = 5,
+    .minimum_version_id = 5,
+    .minimum_version_id_old = 5,
     .post_load = rtl8139_post_load,
     .pre_save  = rtl8139_pre_save,
     .fields      = (VMStateField []) {
@@ -3336,7 +3317,6 @@ static const VMStateDescription vmstate_rtl8139 = {
 
         VMSTATE_UNUSED(4),
         VMSTATE_MACADDR(conf.macaddr, RTL8139State),
-        VMSTATE_INT32(rtl8139_mmio_io_addr_dummy, RTL8139State),
 
         VMSTATE_UINT32(currTxDesc, RTL8139State),
         VMSTATE_UINT32(currCPlusRxDesc, RTL8139State),
@@ -3366,14 +3346,6 @@ static const VMStateDescription vmstate_rtl8139 = {
         VMSTATE_UINT32_V(cplus_enabled, RTL8139State, 4),
         VMSTATE_END_OF_LIST()
     },
-    .subsections = (VMStateSubsection []) {
-        {
-            .vmsd = &vmstate_rtl8139_hotplug_ready,
-            .needed = rtl8139_hotplug_ready_needed,
-        }, {
-            /* empty */
-        }
-    }
 };
 
 /***********************************************************/
-- 
1.7.4.1