[Qemu-devel] [PATCH 0/6] [PATCH v2 0/6] Migration subsections (and ide as example)

[Qemu-devel] [PATCH 0/6] [PATCH v2 0/6] Migration subsections (and ide as example)

[Juan Quintela]

v3:
- spell checking documentation
- s/[Qq]emu/QEMU/

v2:
- add documentation
- improve commit messages
- remove debug printf's
- rebase latest qemu

v1:

At the end, here is the migration subsections implementation.  As an example I ported the last
two ide changes to migration to work with subsections.  Notes:

- subsections
  I went for qemu_peek_byte() insteadof adding a subsection part in
  qemu_loadvm_state() due to two reasons:
   - it makes mandatory that subsections came after sections (better for error messages)
   - it makes post_load() for the section to be run after subsections are loaded.
     I think that running section post_load() and then subsections can make for some subtle
      errors.
  How does it works?
  We have a new array of subsections at the end of each section (it can be NULL).
  Each subsection is composed of VMStateDescription and a test function.  test function
  checks if subsection is needed or not.  if needed, it is just emmited.
  On load, we peek to see if after a section is loaded, if there is any subsection
  at the end, and if so, we search for it on this section subsections.


- ide: 1st revert is not clear because there has been posterior changes that I honored.
  only change done is that ide_dummy_transfer_stop to transfer_end_table for it to be
  complete.

- testing.  In normal operation this code is not triggered (one of the reason for not wanting to
  sent it in the 1st place).  I used patch attached at the end to trigger it.

Commetnts?

Later, Juan.

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 59341a1..a4e6b82 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -421,6 +421,15 @@ static void ide_sector_read(IDEState *s)
         ide_set_irq(s->bus);
         ide_set_sector(s, sector_num + n);
         s->nsector -= n;
+    if ((s->status & DRQ_STAT)) {
+        static int val = 1;
+        if (((val++) == 1000)) {
+            qemu_aio_flush();
+            vm_stop(0);
+            printf("stopped %d ide_ioport_readaaa\n", val);
+        }
+    }
+
     }
 }

@@ -2744,6 +2753,14 @@ static int ide_drive_pio_post_load(void *opaque, int version_id)
     s->data_ptr = s->io_buffer + s->cur_io_buffer_offset;
     s->data_end = s->data_ptr + s->cur_io_buffer_len;

+    printf("addr %p: status %d\n", s, s->status& DRQ_STAT);
+    printf("\tdata_ptr %p\n", s->data_ptr);
+    printf("\tdata_end %p\n", s->data_end);
+    printf("\tio_buffer %p\n", s->io_buffer);
+    printf("\treq_nb_sectors %d\n", s->req_nb_sectors);
+    printf("\tidx %d\n", transfer_end_table_idx(s->end_transfer_func));
+    printf("\telementary_transfer_size %d\n", s->elementary_transfer_size);
+    printf("\tpacket_transfer_size %d\n", s->packet_transfer_size);
     return 0;
 }

@@ -2763,6 +2780,15 @@ static void ide_drive_pio_pre_save(void *opaque)
     } else {
         s->end_transfer_fn_idx = idx;
     }
+
+    printf("addr %p: status %d\n", s, s->status& DRQ_STAT);
+    printf("\tdata_ptr %p\n", s->data_ptr);
+    printf("\tdata_end %p\n", s->data_end);
+    printf("\tio_buffer %p\n", s->io_buffer);
+    printf("\treq_nb_sectors %d\n", s->req_nb_sectors);
+    printf("\tidx %d\n", transfer_end_table_idx(s->end_transfer_func));
+    printf("\telementary_transfer_size %d\n", s->elementary_transfer_size);
+    printf("\tpacket_transfer_size %d\n", s->packet_transfer_size);
 }

 static bool ide_drive_pio_state_needed(void *opaque)

Juan Quintela (6):
  Revert "ide save/restore pio/atapi cmd transfer fields and io buffer"
  Revert "ide save/restore current transfer fields"
  vmstate: add subsections code
  ide: fix migration in the middle of pio operation
  ide: fix migration in the middle of a bmdma transfer
  Initial documentation for migration

 docs/migration.txt |  303 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/hw.h            |    6 +
 hw/ide/core.c      |   72 +++++++++----
 hw/ide/pci.c       |   38 ++++++-
 savevm.c           |   86 +++++++++++++++-
 5 files changed, 477 insertions(+), 28 deletions(-)
 create mode 100644 docs/migration.txt

[Qemu-devel] [PATCH 1/6] Revert "ide save/restore pio/atapi cmd transfer fields and io buffer"

[Juan Quintela]

This reverts commit ed487bb1d69040b9dac64a4fc076d8dd82b131d6.

The conflicts are due to commit 4fc8d6711aff7a9c11e402c3d77b481609f9f486
that is a fix to the ide_drive_pre_save() function.  It reverts both
(and both are reinstantiated later in the series)

Conflicts:

	hw/ide/core.c

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/ide/core.c     |   62 +---------------------------------------------------
 hw/ide/internal.h |    5 ----
 2 files changed, 2 insertions(+), 65 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index e20f2e7..ccb7d32 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2664,7 +2664,6 @@ static void ide_init1(IDEBus *bus, int unit)
     s->unit = unit;
     s->drive_serial = drive_serial++;
     s->io_buffer = qemu_blockalign(s->bs, IDE_DMA_BUF_SECTORS*512 + 4);
-    s->io_buffer_total_len = IDE_DMA_BUF_SECTORS*512 + 4;
     s->smart_selftest_data = qemu_blockalign(s->bs, 512);
     s->sector_write_timer = qemu_new_timer(vm_clock,
                                            ide_sector_write_timer_cb, s);
@@ -2727,25 +2726,6 @@ static bool is_identify_set(void *opaque, int version_id)
     return s->identify_set != 0;
 }

-static EndTransferFunc* transfer_end_table[] = {
-        ide_sector_read,
-        ide_sector_write,
-        ide_transfer_stop,
-        ide_atapi_cmd_reply_end,
-        ide_atapi_cmd,
-};
-
-static int transfer_end_table_idx(EndTransferFunc *fn)
-{
-    int i;
-
-    for (i = 0; i < ARRAY_SIZE(transfer_end_table); i++)
-        if (transfer_end_table[i] == fn)
-            return i;
-
-    return -1;
-}
-
 static int ide_drive_post_load(void *opaque, int version_id)
 {
     IDEState *s = opaque;
@@ -2756,45 +2736,14 @@ static int ide_drive_post_load(void *opaque, int version_id)
             s->cdrom_changed = 1;
         }
     }
-
-    if (s->cur_io_buffer_len) {
-        s->end_transfer_func = transfer_end_table[s->end_transfer_fn_idx];
-        s->data_ptr = s->io_buffer + s->cur_io_buffer_offset;
-        s->data_end = s->data_ptr + s->cur_io_buffer_len;
-    }
-        
     return 0;
 }

-static void ide_drive_pre_save(void *opaque)
-{
-    IDEState *s = opaque;
-    int idx;
-
-    s->cur_io_buffer_len = 0;
-
-    if (!(s->status & DRQ_STAT))
-        return;
-
-    s->cur_io_buffer_offset = s->data_ptr - s->io_buffer;
-    s->cur_io_buffer_len = s->data_end - s->data_ptr;
-
-    idx = transfer_end_table_idx(s->end_transfer_func);
-    if (idx == -1) {
-        fprintf(stderr, "%s: invalid end_transfer_func for DRQ_STAT\n",
-                        __func__);
-        s->end_transfer_fn_idx = 2;
-    } else {
-        s->end_transfer_fn_idx = idx;
-    }
-}
-
 const VMStateDescription vmstate_ide_drive = {
     .name = "ide_drive",
-    .version_id = 4,
+    .version_id = 3,
     .minimum_version_id = 0,
     .minimum_version_id_old = 0,
-    .pre_save = ide_drive_pre_save,
     .post_load = ide_drive_post_load,
     .fields      = (VMStateField []) {
         VMSTATE_INT32(mult_sectors, IDEState),
@@ -2817,14 +2766,7 @@ const VMStateDescription vmstate_ide_drive = {
         VMSTATE_UINT8(sense_key, IDEState),
         VMSTATE_UINT8(asc, IDEState),
         VMSTATE_UINT8_V(cdrom_changed, IDEState, 3),
-        VMSTATE_INT32_V(req_nb_sectors, IDEState, 4),
-        VMSTATE_VARRAY_INT32(io_buffer, IDEState, io_buffer_total_len, 4,
-			     vmstate_info_uint8, uint8_t),
-        VMSTATE_INT32_V(cur_io_buffer_offset, IDEState, 4),
-        VMSTATE_INT32_V(cur_io_buffer_len, IDEState, 4),
-        VMSTATE_UINT8_V(end_transfer_fn_idx, IDEState, 4),
-        VMSTATE_INT32_V(elementary_transfer_size, IDEState, 4),
-        VMSTATE_INT32_V(packet_transfer_size, IDEState, 4),
+        /* XXX: if a transfer is pending, we do not save it yet */
         VMSTATE_END_OF_LIST()
     }
 };
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 4165543..75745ee 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -419,11 +419,6 @@ struct IDEState {
     uint8_t *data_ptr;
     uint8_t *data_end;
     uint8_t *io_buffer;
-    /* PIO save/restore */
-    int32_t io_buffer_total_len;
-    int cur_io_buffer_offset;
-    int cur_io_buffer_len;
-    uint8_t end_transfer_fn_idx;
     QEMUTimer *sector_write_timer; /* only used for win2k install hack */
     uint32_t irq_count; /* counts IRQs when using win2k install hack */
     /* CF-ATA extended error */
-- 
1.7.1.1

[Qemu-devel] Re: [PATCH 1/6] Revert "ide save/restore pio/atapi cmd transfer fields and io buffer"

[Anthony Liguori]

On 07/26/2010 02:38 PM, Juan Quintela wrote:
> This reverts commit ed487bb1d69040b9dac64a4fc076d8dd82b131d6.
>
> The conflicts are due to commit 4fc8d6711aff7a9c11e402c3d77b481609f9f486
> that is a fix to the ide_drive_pre_save() function.  It reverts both
> (and both are reinstantiated later in the series)
>
> Conflicts:
>
> 	hw/ide/core.c
>
> Signed-off-by: Juan Quintela<quintela@redhat.com>
>    

These don't apply cleanly and fail in a non-obvious way.  Can you rebase?

Regards,

Anthony Liguori

> ---
>   hw/ide/core.c     |   62 +---------------------------------------------------
>   hw/ide/internal.h |    5 ----
>   2 files changed, 2 insertions(+), 65 deletions(-)
>
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index e20f2e7..ccb7d32 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -2664,7 +2664,6 @@ static void ide_init1(IDEBus *bus, int unit)
>       s->unit = unit;
>       s->drive_serial = drive_serial++;
>       s->io_buffer = qemu_blockalign(s->bs, IDE_DMA_BUF_SECTORS*512 + 4);
> -    s->io_buffer_total_len = IDE_DMA_BUF_SECTORS*512 + 4;
>       s->smart_selftest_data = qemu_blockalign(s->bs, 512);
>       s->sector_write_timer = qemu_new_timer(vm_clock,
>                                              ide_sector_write_timer_cb, s);
> @@ -2727,25 +2726,6 @@ static bool is_identify_set(void *opaque, int version_id)
>       return s->identify_set != 0;
>   }
>
> -static EndTransferFunc* transfer_end_table[] = {
> -        ide_sector_read,
> -        ide_sector_write,
> -        ide_transfer_stop,
> -        ide_atapi_cmd_reply_end,
> -        ide_atapi_cmd,
> -};
> -
> -static int transfer_end_table_idx(EndTransferFunc *fn)
> -{
> -    int i;
> -
> -    for (i = 0; i<  ARRAY_SIZE(transfer_end_table); i++)
> -        if (transfer_end_table[i] == fn)
> -            return i;
> -
> -    return -1;
> -}
> -
>   static int ide_drive_post_load(void *opaque, int version_id)
>   {
>       IDEState *s = opaque;
> @@ -2756,45 +2736,14 @@ static int ide_drive_post_load(void *opaque, int version_id)
>               s->cdrom_changed = 1;
>           }
>       }
> -
> -    if (s->cur_io_buffer_len) {
> -        s->end_transfer_func = transfer_end_table[s->end_transfer_fn_idx];
> -        s->data_ptr = s->io_buffer + s->cur_io_buffer_offset;
> -        s->data_end = s->data_ptr + s->cur_io_buffer_len;
> -    }
> -
>       return 0;
>   }
>
> -static void ide_drive_pre_save(void *opaque)
> -{
> -    IDEState *s = opaque;
> -    int idx;
> -
> -    s->cur_io_buffer_len = 0;
> -
> -    if (!(s->status&  DRQ_STAT))
> -        return;
> -
> -    s->cur_io_buffer_offset = s->data_ptr - s->io_buffer;
> -    s->cur_io_buffer_len = s->data_end - s->data_ptr;
> -
> -    idx = transfer_end_table_idx(s->end_transfer_func);
> -    if (idx == -1) {
> -        fprintf(stderr, "%s: invalid end_transfer_func for DRQ_STAT\n",
> -                        __func__);
> -        s->end_transfer_fn_idx = 2;
> -    } else {
> -        s->end_transfer_fn_idx = idx;
> -    }
> -}
> -
>   const VMStateDescription vmstate_ide_drive = {
>       .name = "ide_drive",
> -    .version_id = 4,
> +    .version_id = 3,
>       .minimum_version_id = 0,
>       .minimum_version_id_old = 0,
> -    .pre_save = ide_drive_pre_save,
>       .post_load = ide_drive_post_load,
>       .fields      = (VMStateField []) {
>           VMSTATE_INT32(mult_sectors, IDEState),
> @@ -2817,14 +2766,7 @@ const VMStateDescription vmstate_ide_drive = {
>           VMSTATE_UINT8(sense_key, IDEState),
>           VMSTATE_UINT8(asc, IDEState),
>           VMSTATE_UINT8_V(cdrom_changed, IDEState, 3),
> -        VMSTATE_INT32_V(req_nb_sectors, IDEState, 4),
> -        VMSTATE_VARRAY_INT32(io_buffer, IDEState, io_buffer_total_len, 4,
> -			     vmstate_info_uint8, uint8_t),
> -        VMSTATE_INT32_V(cur_io_buffer_offset, IDEState, 4),
> -        VMSTATE_INT32_V(cur_io_buffer_len, IDEState, 4),
> -        VMSTATE_UINT8_V(end_transfer_fn_idx, IDEState, 4),
> -        VMSTATE_INT32_V(elementary_transfer_size, IDEState, 4),
> -        VMSTATE_INT32_V(packet_transfer_size, IDEState, 4),
> +        /* XXX: if a transfer is pending, we do not save it yet */
>           VMSTATE_END_OF_LIST()
>       }
>   };
> diff --git a/hw/ide/internal.h b/hw/ide/internal.h
> index 4165543..75745ee 100644
> --- a/hw/ide/internal.h
> +++ b/hw/ide/internal.h
> @@ -419,11 +419,6 @@ struct IDEState {
>       uint8_t *data_ptr;
>       uint8_t *data_end;
>       uint8_t *io_buffer;
> -    /* PIO save/restore */
> -    int32_t io_buffer_total_len;
> -    int cur_io_buffer_offset;
> -    int cur_io_buffer_len;
> -    uint8_t end_transfer_fn_idx;
>       QEMUTimer *sector_write_timer; /* only used for win2k install hack */
>       uint32_t irq_count; /* counts IRQs when using win2k install hack */
>       /* CF-ATA extended error */
>    

[Qemu-devel] Re: [PATCH 1/6] Revert "ide save/restore pio/atapi cmd transfer fields and io buffer"

[Juan Quintela]

Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 07/26/2010 02:38 PM, Juan Quintela wrote:
>> This reverts commit ed487bb1d69040b9dac64a4fc076d8dd82b131d6.
>>
>> The conflicts are due to commit 4fc8d6711aff7a9c11e402c3d77b481609f9f486
>> that is a fix to the ide_drive_pre_save() function.  It reverts both
>> (and both are reinstantiated later in the series)
>>
>> Conflicts:
>>
>> 	hw/ide/core.c
>>
>> Signed-off-by: Juan Quintela<quintela@redhat.com>
>>    
>
> These don't apply cleanly and fail in a non-obvious way.  Can you rebase?
>
> Regards,

I guess you mean against stable here (otherwise, they have already been
integrated).

I am on holidays until Monday,  if it is against stable, I will do it there.

Later, Juan.

[Qemu-devel] [PATCH 2/6] Revert "ide save/restore current transfer fields"

[Juan Quintela]

This reverts commit 42ee76fe82093ba914f0dc83d2decbcf68866144.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/ide/pci.c |    8 ++------
 1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 4d95cc5..780fc5f 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -123,7 +123,7 @@ void bmdma_addr_writel(void *opaque, uint32_t addr, uint32_t val)

 static const VMStateDescription vmstate_bmdma = {
     .name = "ide bmdma",
-    .version_id = 4,
+    .version_id = 3,
     .minimum_version_id = 0,
     .minimum_version_id_old = 0,
     .fields      = (VMStateField []) {
@@ -133,10 +133,6 @@ static const VMStateDescription vmstate_bmdma = {
         VMSTATE_INT64(sector_num, BMDMAState),
         VMSTATE_UINT32(nsector, BMDMAState),
         VMSTATE_UINT8(unit, BMDMAState),
-        VMSTATE_UINT32_V(cur_addr, BMDMAState, 4),
-        VMSTATE_UINT32_V(cur_prd_last, BMDMAState, 4),
-        VMSTATE_UINT32_V(cur_prd_addr, BMDMAState, 4),
-        VMSTATE_UINT32_V(cur_prd_len, BMDMAState, 4),
         VMSTATE_END_OF_LIST()
     }
 };
@@ -156,7 +152,7 @@ static int ide_pci_post_load(void *opaque, int version_id)

 const VMStateDescription vmstate_ide_pci = {
     .name = "ide",
-    .version_id = 4,
+    .version_id = 3,
     .minimum_version_id = 0,
     .minimum_version_id_old = 0,
     .post_load = ide_pci_post_load,
-- 
1.7.1.1

[Qemu-devel] [PATCH 3/6] vmstate: add subsections code

[Juan Quintela]

This commit adds subsections for each device section.
Subsections is the way to handle information that don't need to be sent
to de destination of a migration because its values are not needed.  It is
the way to handle optional information.  Notice that only the source can
decide if the information is optional or not.  The destination needs to
understand all subsections that it receives to have a sucessful load.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/hw.h  |    6 ++++
 savevm.c |   86 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 91 insertions(+), 1 deletions(-)

diff --git a/hw/hw.h b/hw/hw.h
index c2de6fe..e3c3db2 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -313,6 +313,11 @@ typedef struct {
     bool (*field_exists)(void *opaque, int version_id);
 } VMStateField;

+typedef struct VMStateSubsection {
+    const VMStateDescription *vmsd;
+    bool (*needed)(void *opaque);
+} VMStateSubsection;
+
 struct VMStateDescription {
     const char *name;
     int version_id;
@@ -323,6 +328,7 @@ struct VMStateDescription {
     int (*post_load)(void *opaque, int version_id);
     void (*pre_save)(void *opaque);
     VMStateField *fields;
+    const VMStateSubsection *subsections;
 };

 extern const VMStateInfo vmstate_info_int8;
diff --git a/savevm.c b/savevm.c
index ee27989..7a1de3c 100644
--- a/savevm.c
+++ b/savevm.c
@@ -551,6 +551,19 @@ int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size1)
     return size1 - size;
 }

+static int qemu_peek_byte(QEMUFile *f)
+{
+    if (f->is_write)
+        abort();
+
+    if (f->buf_index >= f->buf_size) {
+        qemu_fill_buffer(f);
+        if (f->buf_index >= f->buf_size)
+            return 0;
+    }
+    return f->buf[f->buf_index];
+}
+
 int qemu_get_byte(QEMUFile *f)
 {
     if (f->is_write)
@@ -1198,10 +1211,16 @@ void vmstate_unregister(DeviceState *dev, const VMStateDescription *vmsd,
     }
 }

+static void vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
+                                    void *opaque);
+static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
+                                   void *opaque);
+
 int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
                        void *opaque, int version_id)
 {
     VMStateField *field = vmsd->fields;
+    int ret;

     if (version_id > vmsd->version_id) {
         return -EINVAL;
@@ -1223,7 +1242,7 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
             (!field->field_exists &&
              field->version_id <= version_id)) {
             void *base_addr = opaque + field->offset;
-            int ret, i, n_elems = 1;
+            int i, n_elems = 1;
             int size = field->size;

             if (field->flags & VMS_VBUFFER) {
@@ -1261,6 +1280,10 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
         }
         field++;
     }
+    ret = vmstate_subsection_load(f, vmsd, opaque);
+    if (ret != 0) {
+        return ret;
+    }
     if (vmsd->post_load) {
         return vmsd->post_load(opaque, version_id);
     }
@@ -1313,6 +1336,7 @@ void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
         }
         field++;
     }
+    vmstate_subsection_save(f, vmsd, opaque);
 }

 static int vmstate_load(QEMUFile *f, SaveStateEntry *se, int version_id)
@@ -1341,6 +1365,7 @@ static void vmstate_save(QEMUFile *f, SaveStateEntry *se)
 #define QEMU_VM_SECTION_PART         0x02
 #define QEMU_VM_SECTION_END          0x03
 #define QEMU_VM_SECTION_FULL         0x04
+#define QEMU_VM_SUBSECTION           0x05

 int qemu_savevm_state_begin(Monitor *mon, QEMUFile *f, int blk_enable,
                             int shared)
@@ -1529,6 +1554,65 @@ static SaveStateEntry *find_se(const char *idstr, int instance_id)
     return NULL;
 }

+static const VMStateDescription *vmstate_get_subsection(const VMStateSubsection *sub, char *idstr)
+{
+    while(sub && sub->needed) {
+        if (strcmp(idstr, sub->vmsd->name) == 0) {
+            return sub->vmsd;
+        }
+        sub++;
+    }
+    return NULL;
+}
+
+static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
+                                   void *opaque)
+{
+    while (qemu_peek_byte(f) == QEMU_VM_SUBSECTION) {
+        char idstr[256];
+        int ret;
+        uint8_t version_id, subsection, len;
+        const VMStateDescription *sub_vmsd;
+
+        subsection = qemu_get_byte(f);
+        len = qemu_get_byte(f);
+        qemu_get_buffer(f, (uint8_t *)idstr, len);
+        idstr[len] = 0;
+        version_id = qemu_get_be32(f);
+
+        sub_vmsd = vmstate_get_subsection(vmsd->subsections, idstr);
+        if (sub_vmsd == NULL) {
+            return -ENOENT;
+        }
+        ret = vmstate_load_state(f, sub_vmsd, opaque, version_id);
+        if (ret) {
+            return ret;
+        }
+    }
+    return 0;
+}
+
+static void vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
+                                    void *opaque)
+{
+    const VMStateSubsection *sub = vmsd->subsections;
+
+    while (sub && sub->needed) {
+        if (sub->needed(opaque)) {
+            const VMStateDescription *vmsd = sub->vmsd;
+            uint8_t len;
+
+            qemu_put_byte(f, QEMU_VM_SUBSECTION);
+            len = strlen(vmsd->name);
+            qemu_put_byte(f, len);
+            qemu_put_buffer(f, (uint8_t *)vmsd->name, len);
+            qemu_put_be32(f, vmsd->version_id);
+            vmstate_save_state(f, vmsd, opaque);
+        }
+        sub++;
+    }
+}
+
 typedef struct LoadStateEntry {
     QLIST_ENTRY(LoadStateEntry) entry;
     SaveStateEntry *se;
-- 
1.7.1.1

[Qemu-devel] [PATCH 4/6] ide: fix migration in the middle of pio operation

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/ide/core.c     |   90 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 hw/ide/internal.h |    5 +++
 2 files changed, 94 insertions(+), 1 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index ccb7d32..db00083 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2664,6 +2664,7 @@ static void ide_init1(IDEBus *bus, int unit)
     s->unit = unit;
     s->drive_serial = drive_serial++;
     s->io_buffer = qemu_blockalign(s->bs, IDE_DMA_BUF_SECTORS*512 + 4);
+    s->io_buffer_total_len = IDE_DMA_BUF_SECTORS*512 + 4;
     s->smart_selftest_data = qemu_blockalign(s->bs, 512);
     s->sector_write_timer = qemu_new_timer(vm_clock,
                                            ide_sector_write_timer_cb, s);
@@ -2726,6 +2727,26 @@ static bool is_identify_set(void *opaque, int version_id)
     return s->identify_set != 0;
 }

+static EndTransferFunc* transfer_end_table[] = {
+        ide_sector_read,
+        ide_sector_write,
+        ide_transfer_stop,
+        ide_atapi_cmd_reply_end,
+        ide_atapi_cmd,
+        ide_dummy_transfer_stop,
+};
+
+static int transfer_end_table_idx(EndTransferFunc *fn)
+{
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(transfer_end_table); i++)
+        if (transfer_end_table[i] == fn)
+            return i;
+
+    return -1;
+}
+
 static int ide_drive_post_load(void *opaque, int version_id)
 {
     IDEState *s = opaque;
@@ -2739,6 +2760,66 @@ static int ide_drive_post_load(void *opaque, int version_id)
     return 0;
 }

+static int ide_drive_pio_post_load(void *opaque, int version_id)
+{
+    IDEState *s = opaque;
+
+    if (s->end_transfer_fn_idx < 0 ||
+        s->end_transfer_fn_idx > ARRAY_SIZE(transfer_end_table)) {
+        return -EINVAL;
+    }
+    s->end_transfer_func = transfer_end_table[s->end_transfer_fn_idx];
+    s->data_ptr = s->io_buffer + s->cur_io_buffer_offset;
+    s->data_end = s->data_ptr + s->cur_io_buffer_len;
+
+    return 0;
+}
+
+static void ide_drive_pio_pre_save(void *opaque)
+{
+    IDEState *s = opaque;
+    int idx;
+
+    s->cur_io_buffer_offset = s->data_ptr - s->io_buffer;
+    s->cur_io_buffer_len = s->data_end - s->data_ptr;
+
+    idx = transfer_end_table_idx(s->end_transfer_func);
+    if (idx == -1) {
+        fprintf(stderr, "%s: invalid end_transfer_func for DRQ_STAT\n",
+                        __func__);
+        s->end_transfer_fn_idx = 2;
+    } else {
+        s->end_transfer_fn_idx = idx;
+    }
+}
+
+static bool ide_drive_pio_state_needed(void *opaque)
+{
+    IDEState *s = opaque;
+
+    return (s->status & DRQ_STAT) != 0;
+}
+
+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,
@@ -2766,8 +2847,15 @@ const VMStateDescription vmstate_ide_drive = {
         VMSTATE_UINT8(sense_key, IDEState),
         VMSTATE_UINT8(asc, IDEState),
         VMSTATE_UINT8_V(cdrom_changed, IDEState, 3),
-        /* XXX: if a transfer is pending, we do not save it yet */
         VMSTATE_END_OF_LIST()
+    },
+    .subsections = (VMStateSubsection []) {
+        {
+            .vmsd = &vmstate_ide_drive_pio_state,
+            .needed = ide_drive_pio_state_needed,
+        }, {
+            /* empty */
+        }
     }
 };

diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 75745ee..4165543 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -419,6 +419,11 @@ struct IDEState {
     uint8_t *data_ptr;
     uint8_t *data_end;
     uint8_t *io_buffer;
+    /* PIO save/restore */
+    int32_t io_buffer_total_len;
+    int cur_io_buffer_offset;
+    int cur_io_buffer_len;
+    uint8_t end_transfer_fn_idx;
     QEMUTimer *sector_write_timer; /* only used for win2k install hack */
     uint32_t irq_count; /* counts IRQs when using win2k install hack */
     /* CF-ATA extended error */
-- 
1.7.1.1

[Qemu-devel] [PATCH 5/6] ide: fix migration in the middle of a bmdma transfer

[Juan Quintela]

It reintroduces
    Revert "ide save/restore pio/atapi cmd transfer fields and io buffer"

but using subsections.  Added bonus is the addition of ide_dummy_transfer_stop
to transfer_end_table, that was missing.

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

diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 780fc5f..4331d77 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -121,6 +121,28 @@ void bmdma_addr_writel(void *opaque, uint32_t addr, uint32_t val)
     bm->cur_addr = bm->addr;
 }

+static bool ide_bmdma_current_needed(void *opaque)
+{
+    BMDMAState *bm = opaque;
+
+    return (bm->cur_prd_len != 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()
+    }
+};
+
+
 static const VMStateDescription vmstate_bmdma = {
     .name = "ide bmdma",
     .version_id = 3,
@@ -134,6 +156,14 @@ static const VMStateDescription vmstate_bmdma = {
         VMSTATE_UINT32(nsector, BMDMAState),
         VMSTATE_UINT8(unit, BMDMAState),
         VMSTATE_END_OF_LIST()
+    },
+    .subsections = (VMStateSubsection []) {
+        {
+            .vmsd = &vmstate_bmdma_current,
+            .needed = ide_bmdma_current_needed,
+        }, {
+            /* empty */
+        }
     }
 };

-- 
1.7.1.1

[Qemu-devel] [PATCH 6/6] Initial documentation for migration

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 docs/migration.txt |  303 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 303 insertions(+), 0 deletions(-)
 create mode 100644 docs/migration.txt

diff --git a/docs/migration.txt b/docs/migration.txt
new file mode 100644
index 0000000..69d5383
--- /dev/null
+++ b/docs/migration.txt
@@ -0,0 +1,303 @@
+= Migration =
+
+QEMU has code to load/save the state of the guest that it is running.
+This are two complementary operations.  Saving the state just does
+that, saves the state for each device that the guest is running.
+Restoring a guest is just the opposite operation: we need to load the
+state of each device.
+
+For this to work, QEMU has to be launch with the same arguments the
+two times.  I.e. it can only restore the state in one guest that has
+the same devices that the one it was saved (this last requirement can
+be relaxed a bit, but for now we can consider that configuration have
+to be exactly the same).
+
+Once that we are able to save/restore a guest, a new functionality is
+requested: migration.  This means that QEMU is able to start in one
+machine and being "migrated" to other machine.  I.e. being moved to
+other machine.
+
+Next was the "live migration" functionality.  This is important
+because some guests run with a lot of state (specially RAM), and it
+can take a while to move all state from one machine to another.  Live
+migration allows the guest to continue running while the state is
+transferred.  Only while the last part of the state is transferred has
+the guest to be stopped.  Typically the time that the guest is
+unresponsive during live migration is the low hundred of milliseconds
+(notice that this depends on lot of things).
+
+=== Types of migration ===
+
+Now that we have talked about live migration, there are several ways
+to do migration:
+
+- tcp migration: do the migration using tcp sockets
+- unix migration: do the migration using unix sockets
+- exec migration: do the migration using the stdin/stdout through a process.
+- fd migration: do the migration using an file descriptor that is
+  passed to QEMU.  QEMU don't cares how this file descriptor is opened.
+
+All this four migration protocols use the same infrastructure to
+save/restore state devices.  This infrastructure is shared with the
+savevm/loadvm functionality.
+
+=== State Live Migration ==
+
+This is used for RAM and block devices.  It is not yet ported to vmstate.
+<Fill more information here>
+
+=== What is the common infrastructure ===
+
+QEMU uses a QEMUFile abstraction to be able to do migration.  Any type
+of migration that what to use QEMU infrastructure has to create a
+QEMUFile with:
+
+QEMUFile *qemu_fopen_ops(void *opaque,
+	 		 QEMUFilePutBufferFunc *put_buffer,
+                         QEMUFileGetBufferFunc *get_buffer,
+                         QEMUFileCloseFunc *close,
+                         QEMUFileRateLimit *rate_limit,
+                         QEMUFileSetRateLimit *set_rate_limit,
+			 QEMUFileGetRateLimit *get_rate_limit);
+
+The functions have the following functionality:
+
+This function writes a chunk of data to a file at the given position.
+The pos argument can be ignored if the file is only being used for
+streaming.  The handler should try to write all of the data it can.
+
+typedef int (QEMUFilePutBufferFunc)(void *opaque, const uint8_t *buf,
+                                    int64_t pos, int size);
+
+Read a chunk of data from a file at the given position.  The pos argument
+can be ignored if the file is only be used for streaming.  The number of
+bytes actually read should be returned.
+
+typedef int (QEMUFileGetBufferFunc)(void *opaque, uint8_t *buf,
+                                    int64_t pos, int size);
+
+Close a file and return an error code
+
+typedef int (QEMUFileCloseFunc)(void *opaque);
+
+Called to determine if the file has exceeded it's bandwidth allocation.  The
+bandwidth capping is a soft limit, not a hard limit.
+
+typedef int (QEMUFileRateLimit)(void *opaque);
+
+Called to change the current bandwidth allocation. This function must return
+the new actual bandwidth. It should be new_rate if everything goes OK, and
+the old rate otherwise
+
+typedef size_t (QEMUFileSetRateLimit)(void *opaque, size_t new_rate);
+typedef size_t (QEMUFileGetRateLimit)(void *opaque);
+
+You can use any internal state that you need using the opaque void *
+pointer that is passed to all functions.
+
+The rate limiting functions are used to limit the bandwidth used by
+QEMU migration.
+
+The important functions for us are put_buffer()/get_buffer() that
+allow to write/read a buffer into the QEMUFile.
+
+=== How to save the state of one device ==
+
+The state of a device is saved using intermediate buffers.  There are
+some helper functions to assist this saving.
+
+There is a new concept that we have to explain here: device state
+version.  When we migrate a device, we save/load the state as a series
+of fields.  Some times, due to bugs or new functionality, we need to
+change the state to store more/different information.  We use the
+version to identify each time that we do a change.  Each version is
+associated with a series of fields saved.  The save_state always save
+the state as the newer version.  But load_state some times is able to
+load state from an older version.
+
+ === Legacy way ===
+
+This way is going to disappear as soon as all current users are ported to VMSTATE.
+
+Each device has to register two functions, one to save the state and
+another to load the state back.
+
+int register_savevm(DeviceState *dev,
+                    const char *idstr,
+                    int instance_id,
+                    int version_id,
+                    SaveStateHandler *save_state,
+                    LoadStateHandler *load_state,
+                    void *opaque);
+
+typedef void SaveStateHandler(QEMUFile *f, void *opaque);
+typedef int LoadStateHandler(QEMUFile *f, void *opaque, int version_id);
+
+The important functions for the device state format are the save_state
+and load_state.  Notice that load_state receives a version_id
+parameter to know what state format is receiving.  save_state don't
+have a version_id parameter because it uses always the latest version.
+
+=== VMState ===
+
+The legacy way of saving/loading state of the device had the problem
+that we have to maintain in sync two functions.  If we did one change
+in one of them and not on the other, we got a failed migration.
+
+VMState changed the way that state is saved/loaded.  Instead of using
+a function to save the state and another to load it, it was changed to
+a declarative way of what the state consisted of.  Now VMState is able
+to interpret that definition to be able to load/save the state.  As
+the state is declared only once, it can't go out of sync in the
+save/load functions.
+
+An example (from hw/pckbd.c)
+
+static const VMStateDescription vmstate_kbd = {
+    .name = "pckbd",
+    .version_id = 3,
+    .minimum_version_id = 3,
+    .minimum_version_id_old = 3,
+    .fields      = (VMStateField []) {
+        VMSTATE_UINT8(write_cmd, KBDState),
+        VMSTATE_UINT8(status, KBDState),
+        VMSTATE_UINT8(mode, KBDState),
+        VMSTATE_UINT8(pending, KBDState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+We are declaring the state with name "pckbd".
+The version_id is 3, and the fields are 4 uint8_t in a KBDState structure.
+We registered this with:
+
+    vmstate_register(NULL, 0, &vmstate_kbd, s);
+
+Note: talk about how vmstate <-> qdev interact, and what the instance id's mean.
+
+You can search for VMSTATE_* macros for lots of types used in QEMU in
+hw/hw.h.
+
+=== More about versions ==
+
+You can see that there are several version fields:
+
+- version_id: the maximum version_id supported by VMState for that device
+- minimum_version_id: the minimum version_id that VMState is able to understand
+  for that device.
+- minimum_version_id_old: For devices that were not able to port to vmstate, we can
+  assign a function that knows how to read this old state.
+
+So, VMState is able to read versions from minimum_version_id to
+version_id.  And the function load_state_old() is able to load state
+from minimum_version_id_old to minimum_version_id.  This function is
+deprecated and will be removed when no more users are left.
+
+===  Massaging functions ===
+
+Some times, it is not enough to be able to save the state directly
+from one structure, we need to fill the correct values there.  One
+example is when we are using kvm.  Before saving the cpu state, we
+need to ask kvm to copy to QEMU the state that it is using.  And the
+opposite when we are loading the state, we need a way to tell kvm to
+load the state for the cpu that we have just loaded from the QEMUFile.
+
+The functions to do that are inside a vmstate definition, and are called:
+
+- int (*pre_load)(void *opaque);
+
+  This function is called before we load the state of one device.
+
+- int (*post_load)(void *opaque, int version_id);
+
+  This function is called after we load the state of one device.
+
+- void (*pre_save)(void *opaque);
+
+  This function is called before we save the state of one device.
+
+Example: You can look at hpet.c, that uses the three function to
+         massage the state that is transferred.
+
+=== Subsections ===
+
+The use of version_id allows to be able to migrate from older versions
+to newer versions of a device.  But not the other way around.  This
+makes very complicated to fix bugs in stable branches.  If we need to
+add anything to the state to fix a bug, we have to disable migration
+to older versions that don't have that bug-fix (i.e. a new field).
+
+But some time, that bug-fix is only needed sometimes, not always.  For
+instance, if the device is in the middle of a DMA operation, it is
+using a specific functionality, ....
+
+It is impossible to create a way to make migration from any version to
+any other version to work.  But we can do better that only allowing
+migration from older versions no newer ones.  For that fields that are
+only needed sometimes, we add the idea of subsections.  a subsection
+is "like" a device vmstate, but with a particularity, it has a Boolean
+function that tells if that values are needed to be sent or not.  If
+this functions returns false, the subsection is not sent.
+
+On the receiving side, if we found a subsection for a device that we
+don't understand, we just fail the migration.  If we understand all
+the subsections, then we load the state with success.
+
+One important note is that the post_load() function is called "after"
+loading all subsections, because a newer subsection could change same
+value that it uses.
+
+Example:
+
+static bool ide_drive_pio_state_needed(void *opaque)
+{
+    IDEState *s = opaque;
+
+    return (s->status & DRQ_STAT) != 0;
+}
+
+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,
+    .post_load = ide_drive_post_load,
+    .fields      = (VMStateField []) {
+        .... several fields ....
+        VMSTATE_END_OF_LIST()
+    },
+    .subsections = (VMStateSubsection []) {
+        {
+            .vmsd = &vmstate_ide_drive_pio_state,
+            .needed = ide_drive_pio_state_needed,
+        }, {
+            /* empty */
+        }
+    }
+};
+
+Here we have a subsection for the pio state.  We only need to
+save/send this state when we are in the middle of a pio operation
+(that is what ide_drive_pio_state_needed() checks).  If DRQ_STAT is
+not enabled, the values on that fields are garbage and don't need to
+be sent.
-- 
1.7.1.1