[Qemu-devel] [PATCH 0/9] Optional toplevel sections

[Qemu-devel] [PATCH 0/9] Optional toplevel sections

[Juan Quintela]

Hi

[v2]
- Rebased to current tree
- Added global configuration section, not sent/received for previous
  machine types.

Please, review.

Thanks, Juan.


[v1]
by popular demand, and after too many time, this series.  This is an
RFC to know what people think about how to use them, the interface
proposed, whatever.

* simplify optional subsections moving the "needed" function to
  vmstate description.  I think that this simplification makes sense
  by itself, it is indipendent of the rest of the patches.

* runstate: To make an example of an optional section, I decided to
  use current runstate.  Right now, we have a problem when:
  - we start destination without -S
  - we run migration, and it causes an ioerror on source, but migration finishes
  - we try to run migration on destination anyways, when it is
    possible that we could get disk corruption (the ioerror was there for a reason)
  Luiz: You can see any obvious improvement about how we use runstates?
  Laine: Could you told me if you (libvirt) like this or would want
     something a bit different?

* I sent that option indpendently for new machine types.

* For old machine types I use this as one example of optional section.
  We only sent it when the state is different from "running" or "paused".

  So, the only case where we fail is if we migrate to an old qemu and
  there is one error.

* On the runstate subsection "postload" we can send any event for
  anything that libvirt wants when migration finishes.
  Laine, can you told us what libvirt would preffer for this?

Kevin: You asked for optional sections in the past for the block
   layer, would this proposal be enough for you?

Please review, comment.

Thanks, Juan.

Juan Quintela (9):
  migration: create savevm_state
  migration: Use normal VMStateDescriptions for Subsections
  runstate: Add runstate store
  runstate: create runstate_index function
  runstate: migration allows more transitions now
  migration: create new section to store global state
  global_state: Make section optional
  vmstate: Create optional sections
  migration: Add configuration section

 cpus.c                        |  11 ++--
 docs/migration.txt            |  11 ++--
 exec.c                        |  11 ++--
 hw/acpi/ich9.c                |  10 ++--
 hw/acpi/piix4.c               |  10 ++--
 hw/block/fdc.c                |  37 +++++-------
 hw/char/serial.c              |  41 +++++--------
 hw/display/qxl.c              |  11 ++--
 hw/display/vga.c              |  11 ++--
 hw/i386/pc_piix.c             |   3 +
 hw/i386/pc_q35.c              |   3 +
 hw/ide/core.c                 |  32 ++++------
 hw/ide/pci.c                  |  16 ++---
 hw/input/pckbd.c              |  22 ++++---
 hw/input/ps2.c                |  11 ++--
 hw/intc/apic_common.c         |  10 ++--
 hw/isa/lpc_ich9.c             |  10 ++--
 hw/net/e1000.c                |  11 ++--
 hw/net/rtl8139.c              |  11 ++--
 hw/net/vmxnet3.c              |  12 ++--
 hw/pci-host/piix.c            |  10 ++--
 hw/scsi/scsi-bus.c            |  11 ++--
 hw/timer/hpet.c               |  11 ++--
 hw/timer/mc146818rtc.c        |  23 ++++---
 hw/usb/hcd-ohci.c             |  11 ++--
 hw/usb/redirect.c             |  34 +++++------
 hw/virtio/virtio.c            |  10 ++--
 include/migration/migration.h |   6 ++
 include/migration/vmstate.h   |  10 ++--
 include/sysemu/sysemu.h       |   2 +
 migration/migration.c         | 114 ++++++++++++++++++++++++++++++++++-
 migration/vmstate.c           |  27 ++++++---
 savevm.c                      | 135 ++++++++++++++++++++++++++++++++----------
 target-arm/machine.c          |  26 ++++----
 target-i386/machine.c         |  81 ++++++++++---------------
 target-ppc/machine.c          |  62 ++++++++-----------
 target-s390x/machine.c        |  21 +++----
 vl.c                          |  27 +++++++++
 38 files changed, 508 insertions(+), 407 deletions(-)

-- 
2.4.0

[Qemu-devel] [PATCH 1/9] migration: create savevm_state

[Juan Quintela]

This way, we will put savevm global state here, instead of lots of variables.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 savevm.c | 53 +++++++++++++++++++++++++++++------------------------
 1 file changed, 29 insertions(+), 24 deletions(-)

diff --git a/savevm.c b/savevm.c
index 3b0e222..1014e3e 100644
--- a/savevm.c
+++ b/savevm.c
@@ -235,10 +235,15 @@ typedef struct SaveStateEntry {
     int is_ram;
 } SaveStateEntry;

-
-static QTAILQ_HEAD(savevm_handlers, SaveStateEntry) savevm_handlers =
-    QTAILQ_HEAD_INITIALIZER(savevm_handlers);
-static int global_section_id;
+typedef struct SaveState {
+    QTAILQ_HEAD(, SaveStateEntry) handlers;
+    int global_section_id;
+} SaveState;
+
+static SaveState savevm_state = {
+    .handlers = QTAILQ_HEAD_INITIALIZER(savevm_state.handlers),
+    .global_section_id = 0,
+};

 static void dump_vmstate_vmsd(FILE *out_file,
                               const VMStateDescription *vmsd, int indent,
@@ -383,7 +388,7 @@ static int calculate_new_instance_id(const char *idstr)
     SaveStateEntry *se;
     int instance_id = 0;

-    QTAILQ_FOREACH(se, &savevm_handlers, entry) {
+    QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
         if (strcmp(idstr, se->idstr) == 0
             && instance_id <= se->instance_id) {
             instance_id = se->instance_id + 1;
@@ -397,7 +402,7 @@ static int calculate_compat_instance_id(const char *idstr)
     SaveStateEntry *se;
     int instance_id = 0;

-    QTAILQ_FOREACH(se, &savevm_handlers, entry) {
+    QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
         if (!se->compat) {
             continue;
         }
@@ -425,7 +430,7 @@ int register_savevm_live(DeviceState *dev,

     se = g_malloc0(sizeof(SaveStateEntry));
     se->version_id = version_id;
-    se->section_id = global_section_id++;
+    se->section_id = savevm_state.global_section_id++;
     se->ops = ops;
     se->opaque = opaque;
     se->vmsd = NULL;
@@ -457,7 +462,7 @@ int register_savevm_live(DeviceState *dev,
     }
     assert(!se->compat || se->instance_id == 0);
     /* add at the end of list */
-    QTAILQ_INSERT_TAIL(&savevm_handlers, se, entry);
+    QTAILQ_INSERT_TAIL(&savevm_state.handlers, se, entry);
     return 0;
 }

@@ -491,9 +496,9 @@ void unregister_savevm(DeviceState *dev, const char *idstr, void *opaque)
     }
     pstrcat(id, sizeof(id), idstr);

-    QTAILQ_FOREACH_SAFE(se, &savevm_handlers, entry, new_se) {
+    QTAILQ_FOREACH_SAFE(se, &savevm_state.handlers, entry, new_se) {
         if (strcmp(se->idstr, id) == 0 && se->opaque == opaque) {
-            QTAILQ_REMOVE(&savevm_handlers, se, entry);
+            QTAILQ_REMOVE(&savevm_state.handlers, se, entry);
             if (se->compat) {
                 g_free(se->compat);
             }
@@ -515,7 +520,7 @@ int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,

     se = g_malloc0(sizeof(SaveStateEntry));
     se->version_id = vmsd->version_id;
-    se->section_id = global_section_id++;
+    se->section_id = savevm_state.global_section_id++;
     se->opaque = opaque;
     se->vmsd = vmsd;
     se->alias_id = alias_id;
@@ -543,7 +548,7 @@ int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
     }
     assert(!se->compat || se->instance_id == 0);
     /* add at the end of list */
-    QTAILQ_INSERT_TAIL(&savevm_handlers, se, entry);
+    QTAILQ_INSERT_TAIL(&savevm_state.handlers, se, entry);
     return 0;
 }

@@ -552,9 +557,9 @@ void vmstate_unregister(DeviceState *dev, const VMStateDescription *vmsd,
 {
     SaveStateEntry *se, *new_se;

-    QTAILQ_FOREACH_SAFE(se, &savevm_handlers, entry, new_se) {
+    QTAILQ_FOREACH_SAFE(se, &savevm_state.handlers, entry, new_se) {
         if (se->vmsd == vmsd && se->opaque == opaque) {
-            QTAILQ_REMOVE(&savevm_handlers, se, entry);
+            QTAILQ_REMOVE(&savevm_state.handlers, se, entry);
             if (se->compat) {
                 g_free(se->compat);
             }
@@ -606,7 +611,7 @@ bool qemu_savevm_state_blocked(Error **errp)
 {
     SaveStateEntry *se;

-    QTAILQ_FOREACH(se, &savevm_handlers, entry) {
+    QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
         if (se->vmsd && se->vmsd->unmigratable) {
             error_setg(errp, "State blocked by non-migratable device '%s'",
                        se->idstr);
@@ -623,7 +628,7 @@ void qemu_savevm_state_begin(QEMUFile *f,
     int ret;

     trace_savevm_state_begin();
-    QTAILQ_FOREACH(se, &savevm_handlers, entry) {
+    QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
         if (!se->ops || !se->ops->set_params) {
             continue;
         }
@@ -633,7 +638,7 @@ void qemu_savevm_state_begin(QEMUFile *f,
     qemu_put_be32(f, QEMU_VM_FILE_MAGIC);
     qemu_put_be32(f, QEMU_VM_FILE_VERSION);

-    QTAILQ_FOREACH(se, &savevm_handlers, entry) {
+    QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
         int len;

         if (!se->ops || !se->ops->save_live_setup) {
@@ -676,7 +681,7 @@ int qemu_savevm_state_iterate(QEMUFile *f)
     int ret = 1;

     trace_savevm_state_iterate();
-    QTAILQ_FOREACH(se, &savevm_handlers, entry) {
+    QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
         if (!se->ops || !se->ops->save_live_iterate) {
             continue;
         }
@@ -727,7 +732,7 @@ void qemu_savevm_state_complete(QEMUFile *f)

     cpu_synchronize_all_states();

-    QTAILQ_FOREACH(se, &savevm_handlers, entry) {
+    QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
         if (!se->ops || !se->ops->save_live_complete) {
             continue;
         }
@@ -752,7 +757,7 @@ void qemu_savevm_state_complete(QEMUFile *f)
     vmdesc = qjson_new();
     json_prop_int(vmdesc, "page_size", TARGET_PAGE_SIZE);
     json_start_array(vmdesc, "devices");
-    QTAILQ_FOREACH(se, &savevm_handlers, entry) {
+    QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
         int len;

         if ((!se->ops || !se->ops->save_state) && !se->vmsd) {
@@ -803,7 +808,7 @@ uint64_t qemu_savevm_state_pending(QEMUFile *f, uint64_t max_size)
     SaveStateEntry *se;
     uint64_t ret = 0;

-    QTAILQ_FOREACH(se, &savevm_handlers, entry) {
+    QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
         if (!se->ops || !se->ops->save_live_pending) {
             continue;
         }
@@ -822,7 +827,7 @@ void qemu_savevm_state_cancel(void)
     SaveStateEntry *se;

     trace_savevm_state_cancel();
-    QTAILQ_FOREACH(se, &savevm_handlers, entry) {
+    QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
         if (se->ops && se->ops->cancel) {
             se->ops->cancel(se->opaque);
         }
@@ -872,7 +877,7 @@ static int qemu_save_device_state(QEMUFile *f)

     cpu_synchronize_all_states();

-    QTAILQ_FOREACH(se, &savevm_handlers, entry) {
+    QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
         int len;

         if (se->is_ram) {
@@ -906,7 +911,7 @@ static SaveStateEntry *find_se(const char *idstr, int instance_id)
 {
     SaveStateEntry *se;

-    QTAILQ_FOREACH(se, &savevm_handlers, entry) {
+    QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
         if (!strcmp(se->idstr, idstr) &&
             (instance_id == se->instance_id ||
              instance_id == se->alias_id))
-- 
2.4.0

Re: [Qemu-devel] [PATCH 1/9] migration: create savevm_state

[Dr. David Alan Gilbert]

* Juan Quintela (quintela@redhat.com) wrote:
> This way, we will put savevm global state here, instead of lots of variables.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  savevm.c | 53 +++++++++++++++++++++++++++++------------------------
>  1 file changed, 29 insertions(+), 24 deletions(-)
> 
> diff --git a/savevm.c b/savevm.c
> index 3b0e222..1014e3e 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -235,10 +235,15 @@ typedef struct SaveStateEntry {
>      int is_ram;
>  } SaveStateEntry;
> 
> -
> -static QTAILQ_HEAD(savevm_handlers, SaveStateEntry) savevm_handlers =
> -    QTAILQ_HEAD_INITIALIZER(savevm_handlers);
> -static int global_section_id;
> +typedef struct SaveState {
> +    QTAILQ_HEAD(, SaveStateEntry) handlers;
> +    int global_section_id;
> +} SaveState;
> +
> +static SaveState savevm_state = {
> +    .handlers = QTAILQ_HEAD_INITIALIZER(savevm_state.handlers),
> +    .global_section_id = 0,
> +};
> 
>  static void dump_vmstate_vmsd(FILE *out_file,
>                                const VMStateDescription *vmsd, int indent,
> @@ -383,7 +388,7 @@ static int calculate_new_instance_id(const char *idstr)
>      SaveStateEntry *se;
>      int instance_id = 0;
> 
> -    QTAILQ_FOREACH(se, &savevm_handlers, entry) {
> +    QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>          if (strcmp(idstr, se->idstr) == 0
>              && instance_id <= se->instance_id) {
>              instance_id = se->instance_id + 1;
> @@ -397,7 +402,7 @@ static int calculate_compat_instance_id(const char *idstr)
>      SaveStateEntry *se;
>      int instance_id = 0;
> 
> -    QTAILQ_FOREACH(se, &savevm_handlers, entry) {
> +    QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>          if (!se->compat) {
>              continue;
>          }
> @@ -425,7 +430,7 @@ int register_savevm_live(DeviceState *dev,
> 
>      se = g_malloc0(sizeof(SaveStateEntry));
>      se->version_id = version_id;
> -    se->section_id = global_section_id++;
> +    se->section_id = savevm_state.global_section_id++;
>      se->ops = ops;
>      se->opaque = opaque;
>      se->vmsd = NULL;
> @@ -457,7 +462,7 @@ int register_savevm_live(DeviceState *dev,
>      }
>      assert(!se->compat || se->instance_id == 0);
>      /* add at the end of list */
> -    QTAILQ_INSERT_TAIL(&savevm_handlers, se, entry);
> +    QTAILQ_INSERT_TAIL(&savevm_state.handlers, se, entry);
>      return 0;
>  }
> 
> @@ -491,9 +496,9 @@ void unregister_savevm(DeviceState *dev, const char *idstr, void *opaque)
>      }
>      pstrcat(id, sizeof(id), idstr);
> 
> -    QTAILQ_FOREACH_SAFE(se, &savevm_handlers, entry, new_se) {
> +    QTAILQ_FOREACH_SAFE(se, &savevm_state.handlers, entry, new_se) {
>          if (strcmp(se->idstr, id) == 0 && se->opaque == opaque) {
> -            QTAILQ_REMOVE(&savevm_handlers, se, entry);
> +            QTAILQ_REMOVE(&savevm_state.handlers, se, entry);
>              if (se->compat) {
>                  g_free(se->compat);
>              }
> @@ -515,7 +520,7 @@ int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
> 
>      se = g_malloc0(sizeof(SaveStateEntry));
>      se->version_id = vmsd->version_id;
> -    se->section_id = global_section_id++;
> +    se->section_id = savevm_state.global_section_id++;
>      se->opaque = opaque;
>      se->vmsd = vmsd;
>      se->alias_id = alias_id;
> @@ -543,7 +548,7 @@ int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
>      }
>      assert(!se->compat || se->instance_id == 0);
>      /* add at the end of list */
> -    QTAILQ_INSERT_TAIL(&savevm_handlers, se, entry);
> +    QTAILQ_INSERT_TAIL(&savevm_state.handlers, se, entry);
>      return 0;
>  }
> 
> @@ -552,9 +557,9 @@ void vmstate_unregister(DeviceState *dev, const VMStateDescription *vmsd,
>  {
>      SaveStateEntry *se, *new_se;
> 
> -    QTAILQ_FOREACH_SAFE(se, &savevm_handlers, entry, new_se) {
> +    QTAILQ_FOREACH_SAFE(se, &savevm_state.handlers, entry, new_se) {
>          if (se->vmsd == vmsd && se->opaque == opaque) {
> -            QTAILQ_REMOVE(&savevm_handlers, se, entry);
> +            QTAILQ_REMOVE(&savevm_state.handlers, se, entry);
>              if (se->compat) {
>                  g_free(se->compat);
>              }
> @@ -606,7 +611,7 @@ bool qemu_savevm_state_blocked(Error **errp)
>  {
>      SaveStateEntry *se;
> 
> -    QTAILQ_FOREACH(se, &savevm_handlers, entry) {
> +    QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>          if (se->vmsd && se->vmsd->unmigratable) {
>              error_setg(errp, "State blocked by non-migratable device '%s'",
>                         se->idstr);
> @@ -623,7 +628,7 @@ void qemu_savevm_state_begin(QEMUFile *f,
>      int ret;
> 
>      trace_savevm_state_begin();
> -    QTAILQ_FOREACH(se, &savevm_handlers, entry) {
> +    QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>          if (!se->ops || !se->ops->set_params) {
>              continue;
>          }
> @@ -633,7 +638,7 @@ void qemu_savevm_state_begin(QEMUFile *f,
>      qemu_put_be32(f, QEMU_VM_FILE_MAGIC);
>      qemu_put_be32(f, QEMU_VM_FILE_VERSION);
> 
> -    QTAILQ_FOREACH(se, &savevm_handlers, entry) {
> +    QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>          int len;
> 
>          if (!se->ops || !se->ops->save_live_setup) {
> @@ -676,7 +681,7 @@ int qemu_savevm_state_iterate(QEMUFile *f)
>      int ret = 1;
> 
>      trace_savevm_state_iterate();
> -    QTAILQ_FOREACH(se, &savevm_handlers, entry) {
> +    QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>          if (!se->ops || !se->ops->save_live_iterate) {
>              continue;
>          }
> @@ -727,7 +732,7 @@ void qemu_savevm_state_complete(QEMUFile *f)
> 
>      cpu_synchronize_all_states();
> 
> -    QTAILQ_FOREACH(se, &savevm_handlers, entry) {
> +    QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>          if (!se->ops || !se->ops->save_live_complete) {
>              continue;
>          }
> @@ -752,7 +757,7 @@ void qemu_savevm_state_complete(QEMUFile *f)
>      vmdesc = qjson_new();
>      json_prop_int(vmdesc, "page_size", TARGET_PAGE_SIZE);
>      json_start_array(vmdesc, "devices");
> -    QTAILQ_FOREACH(se, &savevm_handlers, entry) {
> +    QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>          int len;
> 
>          if ((!se->ops || !se->ops->save_state) && !se->vmsd) {
> @@ -803,7 +808,7 @@ uint64_t qemu_savevm_state_pending(QEMUFile *f, uint64_t max_size)
>      SaveStateEntry *se;
>      uint64_t ret = 0;
> 
> -    QTAILQ_FOREACH(se, &savevm_handlers, entry) {
> +    QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>          if (!se->ops || !se->ops->save_live_pending) {
>              continue;
>          }
> @@ -822,7 +827,7 @@ void qemu_savevm_state_cancel(void)
>      SaveStateEntry *se;
> 
>      trace_savevm_state_cancel();
> -    QTAILQ_FOREACH(se, &savevm_handlers, entry) {
> +    QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>          if (se->ops && se->ops->cancel) {
>              se->ops->cancel(se->opaque);
>          }
> @@ -872,7 +877,7 @@ static int qemu_save_device_state(QEMUFile *f)
> 
>      cpu_synchronize_all_states();
> 
> -    QTAILQ_FOREACH(se, &savevm_handlers, entry) {
> +    QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>          int len;
> 
>          if (se->is_ram) {
> @@ -906,7 +911,7 @@ static SaveStateEntry *find_se(const char *idstr, int instance_id)
>  {
>      SaveStateEntry *se;
> 
> -    QTAILQ_FOREACH(se, &savevm_handlers, entry) {
> +    QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>          if (!strcmp(se->idstr, idstr) &&
>              (instance_id == se->instance_id ||
>               instance_id == se->alias_id))
> -- 
> 2.4.0
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

[Qemu-devel] [PATCH 2/9] migration: Use normal VMStateDescriptions for Subsections

[Juan Quintela]

We create optional sections with this patch.  But we already have
optional subsections.  Instead of having two mechanism that do the
same, we can just generalize it.

For subsections we just change:

- Add a needed function to VMStateDescription
- Remove VMStateSubsection (after removal of the needed function
  it is just a VMStateDescription)
- Adjust the whole tree, moving the needed function to the corresponding
  VMStateDescription

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 cpus.c                      | 11 +++---
 docs/migration.txt          | 11 +++---
 exec.c                      | 11 +++---
 hw/acpi/ich9.c              | 10 +++---
 hw/acpi/piix4.c             | 10 +++---
 hw/block/fdc.c              | 37 ++++++++-------------
 hw/char/serial.c            | 41 +++++++++--------------
 hw/display/qxl.c            | 11 +++---
 hw/display/vga.c            | 11 +++---
 hw/ide/core.c               | 32 +++++++-----------
 hw/ide/pci.c                | 16 ++++-----
 hw/input/pckbd.c            | 22 ++++++------
 hw/input/ps2.c              | 11 +++---
 hw/intc/apic_common.c       | 10 +++---
 hw/isa/lpc_ich9.c           | 10 +++---
 hw/net/e1000.c              | 11 +++---
 hw/net/rtl8139.c            | 11 +++---
 hw/net/vmxnet3.c            | 12 +++----
 hw/pci-host/piix.c          | 10 +++---
 hw/scsi/scsi-bus.c          | 11 +++---
 hw/timer/hpet.c             | 11 +++---
 hw/timer/mc146818rtc.c      | 23 ++++++-------
 hw/usb/hcd-ohci.c           | 11 +++---
 hw/usb/redirect.c           | 34 +++++++++----------
 hw/virtio/virtio.c          | 10 +++---
 include/migration/vmstate.h |  8 ++---
 migration/vmstate.c         | 16 ++++-----
 savevm.c                    | 10 +++---
 target-arm/machine.c        | 26 ++++++---------
 target-i386/machine.c       | 81 ++++++++++++++++++---------------------------
 target-ppc/machine.c        | 62 ++++++++++++++--------------------
 target-s390x/machine.c      | 21 +++++-------
 32 files changed, 245 insertions(+), 377 deletions(-)

diff --git a/cpus.c b/cpus.c
index 62d157a..6e9b0a5 100644
--- a/cpus.c
+++ b/cpus.c
@@ -459,6 +459,7 @@ static const VMStateDescription icount_vmstate_timers = {
     .name = "timer/icount",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = icount_state_needed,
     .fields = (VMStateField[]) {
         VMSTATE_INT64(qemu_icount_bias, TimersState),
         VMSTATE_INT64(qemu_icount, TimersState),
@@ -476,13 +477,9 @@ static const VMStateDescription vmstate_timers = {
         VMSTATE_INT64_V(cpu_clock_offset, TimersState, 2),
         VMSTATE_END_OF_LIST()
     },
-    .subsections = (VMStateSubsection[]) {
-        {
-            .vmsd = &icount_vmstate_timers,
-            .needed = icount_state_needed,
-        }, {
-            /* empty */
-        }
+    .subsections = (const VMStateDescription*[]) {
+        &icount_vmstate_timers,
+        NULL
     }
 };

diff --git a/docs/migration.txt b/docs/migration.txt
index 0492a45..f6df4be 100644
--- a/docs/migration.txt
+++ b/docs/migration.txt
@@ -257,6 +257,7 @@ const VMStateDescription vmstate_ide_drive_pio_state = {
     .minimum_version_id = 1,
     .pre_save = ide_drive_pio_pre_save,
     .post_load = ide_drive_pio_post_load,
+    .needed = ide_drive_pio_state_needed,
     .fields = (VMStateField[]) {
         VMSTATE_INT32(req_nb_sectors, IDEState),
         VMSTATE_VARRAY_INT32(io_buffer, IDEState, io_buffer_total_len, 1,
@@ -279,13 +280,9 @@ const VMStateDescription vmstate_ide_drive = {
         .... several fields ....
         VMSTATE_END_OF_LIST()
     },
-    .subsections = (VMStateSubsection []) {
-        {
-            .vmsd = &vmstate_ide_drive_pio_state,
-            .needed = ide_drive_pio_state_needed,
-        }, {
-            /* empty */
-        }
+    .subsections = (const VMStateDescription*[]) {
+        &vmstate_ide_drive_pio_state,
+        NULL
     }
 };

diff --git a/exec.c b/exec.c
index e19ab22..d5c2cfb 100644
--- a/exec.c
+++ b/exec.c
@@ -460,6 +460,7 @@ static const VMStateDescription vmstate_cpu_common_exception_index = {
     .name = "cpu_common/exception_index",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = cpu_common_exception_index_needed,
     .fields = (VMStateField[]) {
         VMSTATE_INT32(exception_index, CPUState),
         VMSTATE_END_OF_LIST()
@@ -477,13 +478,9 @@ const VMStateDescription vmstate_cpu_common = {
         VMSTATE_UINT32(interrupt_request, CPUState),
         VMSTATE_END_OF_LIST()
     },
-    .subsections = (VMStateSubsection[]) {
-        {
-            .vmsd = &vmstate_cpu_common_exception_index,
-            .needed = cpu_common_exception_index_needed,
-        } , {
-            /* empty */
-        }
+    .subsections = (const VMStateDescription*[]) {
+        &vmstate_cpu_common_exception_index,
+        NULL
     }
 };

diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index 84e5bb8..f3e1b6b 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -151,6 +151,7 @@ static const VMStateDescription vmstate_memhp_state = {
     .version_id = 1,
     .minimum_version_id = 1,
     .minimum_version_id_old = 1,
+    .needed = vmstate_test_use_memhp,
     .fields      = (VMStateField[]) {
         VMSTATE_MEMORY_HOTPLUG(acpi_memory_hotplug, ICH9LPCPMRegs),
         VMSTATE_END_OF_LIST()
@@ -174,12 +175,9 @@ const VMStateDescription vmstate_ich9_pm = {
         VMSTATE_UINT32(smi_sts, ICH9LPCPMRegs),
         VMSTATE_END_OF_LIST()
     },
-    .subsections = (VMStateSubsection[]) {
-        {
-            .vmsd = &vmstate_memhp_state,
-            .needed = vmstate_test_use_memhp,
-        },
-        VMSTATE_END_OF_LIST()
+    .subsections = (const VMStateDescription*[]) {
+        &vmstate_memhp_state,
+        NULL
     }
 };

diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 1b28481..e7dff38 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -260,6 +260,7 @@ static const VMStateDescription vmstate_memhp_state = {
     .version_id = 1,
     .minimum_version_id = 1,
     .minimum_version_id_old = 1,
+    .needed = vmstate_test_use_memhp,
     .fields      = (VMStateField[]) {
         VMSTATE_MEMORY_HOTPLUG(acpi_memory_hotplug, PIIX4PMState),
         VMSTATE_END_OF_LIST()
@@ -298,12 +299,9 @@ static const VMStateDescription vmstate_acpi = {
                             vmstate_test_use_acpi_pci_hotplug),
         VMSTATE_END_OF_LIST()
     },
-    .subsections = (VMStateSubsection[]) {
-        {
-            .vmsd = &vmstate_memhp_state,
-            .needed = vmstate_test_use_memhp,
-        },
-        VMSTATE_END_OF_LIST()
+    .subsections = (const VMStateDescription*[]) {
+         &vmstate_memhp_state,
+         NULL
     }
 };

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index d8a8edd..17d427f 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -671,6 +671,7 @@ static const VMStateDescription vmstate_fdrive_media_changed = {
     .name = "fdrive/media_changed",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = fdrive_media_changed_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT8(media_changed, FDrive),
         VMSTATE_END_OF_LIST()
@@ -688,6 +689,7 @@ static const VMStateDescription vmstate_fdrive_media_rate = {
     .name = "fdrive/media_rate",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = fdrive_media_rate_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT8(media_rate, FDrive),
         VMSTATE_END_OF_LIST()
@@ -705,6 +707,7 @@ static const VMStateDescription vmstate_fdrive_perpendicular = {
     .name = "fdrive/perpendicular",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = fdrive_perpendicular_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT8(perpendicular, FDrive),
         VMSTATE_END_OF_LIST()
@@ -728,19 +731,11 @@ static const VMStateDescription vmstate_fdrive = {
         VMSTATE_UINT8(sect, FDrive),
         VMSTATE_END_OF_LIST()
     },
-    .subsections = (VMStateSubsection[]) {
-        {
-            .vmsd = &vmstate_fdrive_media_changed,
-            .needed = &fdrive_media_changed_needed,
-        } , {
-            .vmsd = &vmstate_fdrive_media_rate,
-            .needed = &fdrive_media_rate_needed,
-        } , {
-            .vmsd = &vmstate_fdrive_perpendicular,
-            .needed = &fdrive_perpendicular_needed,
-        } , {
-            /* empty */
-        }
+    .subsections = (const VMStateDescription*[]) {
+        &vmstate_fdrive_media_changed,
+        &vmstate_fdrive_media_rate,
+        &vmstate_fdrive_perpendicular,
+        NULL
     }
 };

@@ -771,6 +766,7 @@ static const VMStateDescription vmstate_fdc_reset_sensei = {
     .name = "fdc/reset_sensei",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = fdc_reset_sensei_needed,
     .fields = (VMStateField[]) {
         VMSTATE_INT32(reset_sensei, FDCtrl),
         VMSTATE_END_OF_LIST()
@@ -788,6 +784,7 @@ static const VMStateDescription vmstate_fdc_result_timer = {
     .name = "fdc/result_timer",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = fdc_result_timer_needed,
     .fields = (VMStateField[]) {
         VMSTATE_TIMER_PTR(result_timer, FDCtrl),
         VMSTATE_END_OF_LIST()
@@ -831,16 +828,10 @@ static const VMStateDescription vmstate_fdc = {
                              vmstate_fdrive, FDrive),
         VMSTATE_END_OF_LIST()
     },
-    .subsections = (VMStateSubsection[]) {
-        {
-            .vmsd = &vmstate_fdc_reset_sensei,
-            .needed = fdc_reset_sensei_needed,
-        } , {
-            .vmsd = &vmstate_fdc_result_timer,
-            .needed = fdc_result_timer_needed,
-        } , {
-            /* empty */
-        }
+    .subsections = (const VMStateDescription*[]) {
+        &vmstate_fdc_reset_sensei,
+        &vmstate_fdc_result_timer,
+        NULL
     }
 };

diff --git a/hw/char/serial.c b/hw/char/serial.c
index 55011cf..513d73c 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -662,6 +662,7 @@ static const VMStateDescription vmstate_serial_thr_ipending = {
     .name = "serial/thr_ipending",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = serial_thr_ipending_needed,
     .fields = (VMStateField[]) {
         VMSTATE_INT32(thr_ipending, SerialState),
         VMSTATE_END_OF_LIST()
@@ -678,6 +679,7 @@ static const VMStateDescription vmstate_serial_tsr = {
     .name = "serial/tsr",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = serial_tsr_needed,
     .fields = (VMStateField[]) {
         VMSTATE_INT32(tsr_retry, SerialState),
         VMSTATE_UINT8(thr, SerialState),
@@ -697,6 +699,7 @@ static const VMStateDescription vmstate_serial_recv_fifo = {
     .name = "serial/recv_fifo",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = serial_recv_fifo_needed,
     .fields = (VMStateField[]) {
         VMSTATE_STRUCT(recv_fifo, SerialState, 1, vmstate_fifo8, Fifo8),
         VMSTATE_END_OF_LIST()
@@ -713,6 +716,7 @@ static const VMStateDescription vmstate_serial_xmit_fifo = {
     .name = "serial/xmit_fifo",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = serial_xmit_fifo_needed,
     .fields = (VMStateField[]) {
         VMSTATE_STRUCT(xmit_fifo, SerialState, 1, vmstate_fifo8, Fifo8),
         VMSTATE_END_OF_LIST()
@@ -729,6 +733,7 @@ static const VMStateDescription vmstate_serial_fifo_timeout_timer = {
     .name = "serial/fifo_timeout_timer",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = serial_fifo_timeout_timer_needed,
     .fields = (VMStateField[]) {
         VMSTATE_TIMER_PTR(fifo_timeout_timer, SerialState),
         VMSTATE_END_OF_LIST()
@@ -745,6 +750,7 @@ static const VMStateDescription vmstate_serial_timeout_ipending = {
     .name = "serial/timeout_ipending",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = serial_timeout_ipending_needed,
     .fields = (VMStateField[]) {
         VMSTATE_INT32(timeout_ipending, SerialState),
         VMSTATE_END_OF_LIST()
@@ -760,6 +766,7 @@ static bool serial_poll_needed(void *opaque)
 static const VMStateDescription vmstate_serial_poll = {
     .name = "serial/poll",
     .version_id = 1,
+    .needed = serial_poll_needed,
     .minimum_version_id = 1,
     .fields = (VMStateField[]) {
         VMSTATE_INT32(poll_msl, SerialState),
@@ -788,31 +795,15 @@ const VMStateDescription vmstate_serial = {
         VMSTATE_UINT8_V(fcr_vmstate, SerialState, 3),
         VMSTATE_END_OF_LIST()
     },
-    .subsections = (VMStateSubsection[]) {
-        {
-            .vmsd = &vmstate_serial_thr_ipending,
-            .needed = &serial_thr_ipending_needed,
-        } , {
-            .vmsd = &vmstate_serial_tsr,
-            .needed = &serial_tsr_needed,
-        } , {
-            .vmsd = &vmstate_serial_recv_fifo,
-            .needed = &serial_recv_fifo_needed,
-        } , {
-            .vmsd = &vmstate_serial_xmit_fifo,
-            .needed = &serial_xmit_fifo_needed,
-        } , {
-            .vmsd = &vmstate_serial_fifo_timeout_timer,
-            .needed = &serial_fifo_timeout_timer_needed,
-        } , {
-            .vmsd = &vmstate_serial_timeout_ipending,
-            .needed = &serial_timeout_ipending_needed,
-        } , {
-            .vmsd = &vmstate_serial_poll,
-            .needed = &serial_poll_needed,
-        } , {
-            /* empty */
-        }
+    .subsections = (const VMStateDescription*[]) {
+        &vmstate_serial_thr_ipending,
+        &vmstate_serial_tsr,
+        &vmstate_serial_recv_fifo,
+        &vmstate_serial_xmit_fifo,
+        &vmstate_serial_fifo_timeout_timer,
+        &vmstate_serial_timeout_ipending,
+        &vmstate_serial_poll,
+        NULL
     }
 };

diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index 0cd314c..9db1e1c 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -2216,6 +2216,7 @@ static VMStateDescription qxl_vmstate_monitors_config = {
     .name               = "qxl/monitors-config",
     .version_id         = 1,
     .minimum_version_id = 1,
+    .needed = qxl_monitors_config_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT64(guest_monitors_config, PCIQXLDevice),
         VMSTATE_END_OF_LIST()
@@ -2249,13 +2250,9 @@ static VMStateDescription qxl_vmstate = {
         VMSTATE_UINT64(guest_cursor, PCIQXLDevice),
         VMSTATE_END_OF_LIST()
     },
-    .subsections = (VMStateSubsection[]) {
-        {
-            .vmsd = &qxl_vmstate_monitors_config,
-            .needed = qxl_monitors_config_needed,
-        }, {
-            /* empty */
-        }
+    .subsections = (const VMStateDescription*[]) {
+        &qxl_vmstate_monitors_config,
+        NULL
     }
 };

diff --git a/hw/display/vga.c b/hw/display/vga.c
index d1d296c..b35d523 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -2035,6 +2035,7 @@ static const VMStateDescription vmstate_vga_endian = {
     .name = "vga.endian",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = vga_endian_state_needed,
     .fields = (VMStateField[]) {
         VMSTATE_BOOL(big_endian_fb, VGACommonState),
         VMSTATE_END_OF_LIST()
@@ -2078,13 +2079,9 @@ const VMStateDescription vmstate_vga_common = {
         VMSTATE_UINT32(vbe_bank_mask, VGACommonState),
         VMSTATE_END_OF_LIST()
     },
-    .subsections = (VMStateSubsection []) {
-        {
-            .vmsd = &vmstate_vga_endian,
-            .needed = vga_endian_state_needed,
-        }, {
-            /* empty */
-        }
+    .subsections = (const VMStateDescription*[]) {
+        &vmstate_vga_endian,
+        NULL
     }
 };

diff --git a/hw/ide/core.c b/hw/ide/core.c
index fcb9080..1efd98a 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2561,6 +2561,7 @@ static const VMStateDescription vmstate_ide_atapi_gesn_state = {
     .name ="ide_drive/atapi/gesn_state",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = ide_atapi_gesn_needed,
     .fields = (VMStateField[]) {
         VMSTATE_BOOL(events.new_media, IDEState),
         VMSTATE_BOOL(events.eject_request, IDEState),
@@ -2572,6 +2573,7 @@ static const VMStateDescription vmstate_ide_tray_state = {
     .name = "ide_drive/tray_state",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = ide_tray_state_needed,
     .fields = (VMStateField[]) {
         VMSTATE_BOOL(tray_open, IDEState),
         VMSTATE_BOOL(tray_locked, IDEState),
@@ -2585,6 +2587,7 @@ static const VMStateDescription vmstate_ide_drive_pio_state = {
     .minimum_version_id = 1,
     .pre_save = ide_drive_pio_pre_save,
     .post_load = ide_drive_pio_post_load,
+    .needed = ide_drive_pio_state_needed,
     .fields = (VMStateField[]) {
         VMSTATE_INT32(req_nb_sectors, IDEState),
         VMSTATE_VARRAY_INT32(io_buffer, IDEState, io_buffer_total_len, 1,
@@ -2626,19 +2629,11 @@ const VMStateDescription vmstate_ide_drive = {
         VMSTATE_UINT8_V(cdrom_changed, IDEState, 3),
         VMSTATE_END_OF_LIST()
     },
-    .subsections = (VMStateSubsection []) {
-        {
-            .vmsd = &vmstate_ide_drive_pio_state,
-            .needed = ide_drive_pio_state_needed,
-        }, {
-            .vmsd = &vmstate_ide_tray_state,
-            .needed = ide_tray_state_needed,
-        }, {
-            .vmsd = &vmstate_ide_atapi_gesn_state,
-            .needed = ide_atapi_gesn_needed,
-        }, {
-            /* empty */
-        }
+    .subsections = (const VMStateDescription*[]) {
+        &vmstate_ide_drive_pio_state,
+        &vmstate_ide_tray_state,
+        &vmstate_ide_atapi_gesn_state,
+        NULL
     }
 };

@@ -2646,6 +2641,7 @@ static const VMStateDescription vmstate_ide_error_status = {
     .name ="ide_bus/error",
     .version_id = 2,
     .minimum_version_id = 1,
+    .needed = ide_error_needed,
     .fields = (VMStateField[]) {
         VMSTATE_INT32(error_status, IDEBus),
         VMSTATE_INT64_V(retry_sector_num, IDEBus, 2),
@@ -2664,13 +2660,9 @@ const VMStateDescription vmstate_ide_bus = {
         VMSTATE_UINT8(unit, IDEBus),
         VMSTATE_END_OF_LIST()
     },
-    .subsections = (VMStateSubsection []) {
-        {
-            .vmsd = &vmstate_ide_error_status,
-            .needed = ide_error_needed,
-        }, {
-            /* empty */
-        }
+    .subsections = (const VMStateDescription*[]) {
+        &vmstate_ide_error_status,
+        NULL
     }
 };

diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 1b3d1c1..1feefef 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -350,6 +350,7 @@ static const VMStateDescription vmstate_bmdma_current = {
     .name = "ide bmdma_current",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = ide_bmdma_current_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT32(cur_addr, BMDMAState),
         VMSTATE_UINT32(cur_prd_last, BMDMAState),
@@ -363,6 +364,7 @@ static const VMStateDescription vmstate_bmdma_status = {
     .name ="ide bmdma/status",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = ide_bmdma_status_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT8(status, BMDMAState),
         VMSTATE_END_OF_LIST()
@@ -383,16 +385,10 @@ static const VMStateDescription vmstate_bmdma = {
         VMSTATE_UINT8(migration_retry_unit, 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 */
-        }
+    .subsections = (const VMStateDescription*[]) {
+        &vmstate_bmdma_current,
+        &vmstate_bmdma_status,
+        NULL
     }
 };

diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
index 9b9a7d7..ddac69d 100644
--- a/hw/input/pckbd.c
+++ b/hw/input/pckbd.c
@@ -391,23 +391,24 @@ static int kbd_outport_post_load(void *opaque, int version_id)
     return 0;
 }

+static bool kbd_outport_needed(void *opaque)
+{
+    KBDState *s = opaque;
+    return s->outport != kbd_outport_default(s);
+}
+
 static const VMStateDescription vmstate_kbd_outport = {
     .name = "pckbd_outport",
     .version_id = 1,
     .minimum_version_id = 1,
     .post_load = kbd_outport_post_load,
+    .needed = kbd_outport_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT8(outport, KBDState),
         VMSTATE_END_OF_LIST()
     }
 };

-static bool kbd_outport_needed(void *opaque)
-{
-    KBDState *s = opaque;
-    return s->outport != kbd_outport_default(s);
-}
-
 static int kbd_post_load(void *opaque, int version_id)
 {
     KBDState *s = opaque;
@@ -430,12 +431,9 @@ static const VMStateDescription vmstate_kbd = {
         VMSTATE_UINT8(pending, KBDState),
         VMSTATE_END_OF_LIST()
     },
-    .subsections = (VMStateSubsection[]) {
-        {
-            .vmsd = &vmstate_kbd_outport,
-            .needed = kbd_outport_needed,
-        },
-        VMSTATE_END_OF_LIST()
+    .subsections = (const VMStateDescription*[]) {
+        &vmstate_kbd_outport,
+        NULL
     }
 };

diff --git a/hw/input/ps2.c b/hw/input/ps2.c
index 4baeea2..fdbe565 100644
--- a/hw/input/ps2.c
+++ b/hw/input/ps2.c
@@ -677,6 +677,7 @@ static const VMStateDescription vmstate_ps2_keyboard_ledstate = {
     .version_id = 3,
     .minimum_version_id = 2,
     .post_load = ps2_kbd_ledstate_post_load,
+    .needed = ps2_keyboard_ledstate_needed,
     .fields = (VMStateField[]) {
         VMSTATE_INT32(ledstate, PS2KbdState),
         VMSTATE_END_OF_LIST()
@@ -717,13 +718,9 @@ static const VMStateDescription vmstate_ps2_keyboard = {
         VMSTATE_INT32_V(scancode_set, PS2KbdState,3),
         VMSTATE_END_OF_LIST()
     },
-    .subsections = (VMStateSubsection []) {
-        {
-            .vmsd = &vmstate_ps2_keyboard_ledstate,
-            .needed = ps2_keyboard_ledstate_needed,
-        }, {
-            /* empty */
-        }
+    .subsections = (const VMStateDescription*[]) {
+        &vmstate_ps2_keyboard_ledstate,
+        NULL
     }
 };

diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index d595d63..0032b97 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -369,6 +369,7 @@ static const VMStateDescription vmstate_apic_common_sipi = {
     .name = "apic_sipi",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = apic_common_sipi_needed,
     .fields = (VMStateField[]) {
         VMSTATE_INT32(sipi_vector, APICCommonState),
         VMSTATE_INT32(wait_for_sipi, APICCommonState),
@@ -408,12 +409,9 @@ static const VMStateDescription vmstate_apic_common = {
                       APICCommonState), /* open-coded timer state */
         VMSTATE_END_OF_LIST()
     },
-    .subsections = (VMStateSubsection[]) {
-        {
-            .vmsd = &vmstate_apic_common_sipi,
-            .needed = apic_common_sipi_needed,
-        },
-        VMSTATE_END_OF_LIST()
+    .subsections = (const VMStateDescription*[]) {
+        &vmstate_apic_common_sipi,
+        NULL
     }
 };

diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index dba7585..f7aff03 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -634,6 +634,7 @@ static const VMStateDescription vmstate_ich9_rst_cnt = {
     .name = "ICH9LPC/rst_cnt",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = ich9_rst_cnt_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT8(rst_cnt, ICH9LPCState),
         VMSTATE_END_OF_LIST()
@@ -653,12 +654,9 @@ static const VMStateDescription vmstate_ich9_lpc = {
         VMSTATE_UINT32(sci_level, ICH9LPCState),
         VMSTATE_END_OF_LIST()
     },
-    .subsections = (VMStateSubsection[]) {
-        {
-            .vmsd = &vmstate_ich9_rst_cnt,
-            .needed = ich9_rst_cnt_needed
-        },
-        { 0 }
+    .subsections = (const VMStateDescription*[]) {
+        &vmstate_ich9_rst_cnt,
+        NULL
     }
 };

diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 091d61a..bab8e2a 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -1370,6 +1370,7 @@ static const VMStateDescription vmstate_e1000_mit_state = {
     .name = "e1000/mit_state",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = e1000_mit_state_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT32(mac_reg[RDTR], E1000State),
         VMSTATE_UINT32(mac_reg[RADV], E1000State),
@@ -1457,13 +1458,9 @@ static const VMStateDescription vmstate_e1000 = {
         VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, VFTA, 128),
         VMSTATE_END_OF_LIST()
     },
-    .subsections = (VMStateSubsection[]) {
-        {
-            .vmsd = &vmstate_e1000_mit_state,
-            .needed = e1000_mit_state_needed,
-        }, {
-            /* empty */
-        }
+    .subsections = (const VMStateDescription*[]) {
+        &vmstate_e1000_mit_state,
+        NULL
     }
 };

diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
index f868108..e0db472 100644
--- a/hw/net/rtl8139.c
+++ b/hw/net/rtl8139.c
@@ -3240,6 +3240,7 @@ static const VMStateDescription vmstate_rtl8139_hotplug_ready ={
     .name = "rtl8139/hotplug_ready",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = rtl8139_hotplug_ready_needed,
     .fields = (VMStateField[]) {
         VMSTATE_END_OF_LIST()
     }
@@ -3335,13 +3336,9 @@ 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 */
-        }
+    .subsections = (const VMStateDescription*[]) {
+        &vmstate_rtl8139_hotplug_ready,
+        NULL
     }
 };

diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index dfb328d..8bcdf3e 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -2226,6 +2226,7 @@ static const VMStateDescription vmxstate_vmxnet3_mcast_list = {
     .version_id = 1,
     .minimum_version_id = 1,
     .pre_load = vmxnet3_mcast_list_pre_load,
+    .needed = vmxnet3_mc_list_needed,
     .fields = (VMStateField[]) {
         VMSTATE_VBUFFER_UINT32(mcast_list, VMXNET3State, 0, NULL, 0,
             mcast_list_buff_size),
@@ -2470,14 +2471,9 @@ static const VMStateDescription vmstate_vmxnet3 = {

             VMSTATE_END_OF_LIST()
     },
-    .subsections = (VMStateSubsection[]) {
-        {
-            .vmsd = &vmxstate_vmxnet3_mcast_list,
-            .needed = vmxnet3_mc_list_needed
-        },
-        {
-            /* empty element. */
-        }
+    .subsections = (const VMStateDescription*[]) {
+        &vmxstate_vmxnet3_mcast_list,
+        NULL
     }
 };

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 723836f..1936fd4 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -578,6 +578,7 @@ static const VMStateDescription vmstate_piix3_rcr = {
     .name = "PIIX3/rcr",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = piix3_rcr_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT8(rcr, PIIX3State),
         VMSTATE_END_OF_LIST()
@@ -596,12 +597,9 @@ static const VMStateDescription vmstate_piix3 = {
                               PIIX_NUM_PIRQS, 3),
         VMSTATE_END_OF_LIST()
     },
-    .subsections = (VMStateSubsection[]) {
-        {
-            .vmsd = &vmstate_piix3_rcr,
-            .needed = piix3_rcr_needed,
-        },
-        { 0 }
+    .subsections = (const VMStateDescription*[]) {
+        &vmstate_piix3_rcr,
+        NULL
     }
 };

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index bd2c0e4..f50b2f0 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -1968,6 +1968,7 @@ static const VMStateDescription vmstate_scsi_sense_state = {
     .name = "SCSIDevice/sense",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = scsi_sense_state_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT8_SUB_ARRAY(sense, SCSIDevice,
                                 SCSI_SENSE_BUF_SIZE_OLD,
@@ -1998,13 +1999,9 @@ const VMStateDescription vmstate_scsi_device = {
         },
         VMSTATE_END_OF_LIST()
     },
-    .subsections = (VMStateSubsection []) {
-        {
-            .vmsd = &vmstate_scsi_sense_state,
-            .needed = scsi_sense_state_needed,
-        }, {
-            /* empty */
-        }
+    .subsections = (const VMStateDescription*[]) {
+        &vmstate_scsi_sense_state,
+        NULL
     }
 };

diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
index b6b8a20..b50071e 100644
--- a/hw/timer/hpet.c
+++ b/hw/timer/hpet.c
@@ -283,6 +283,7 @@ static const VMStateDescription vmstate_hpet_rtc_irq_level = {
     .name = "hpet/rtc_irq_level",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = hpet_rtc_irq_level_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT8(rtc_irq_level, HPETState),
         VMSTATE_END_OF_LIST()
@@ -322,13 +323,9 @@ static const VMStateDescription vmstate_hpet = {
                                     vmstate_hpet_timer, HPETTimer),
         VMSTATE_END_OF_LIST()
     },
-    .subsections = (VMStateSubsection[]) {
-        {
-            .vmsd = &vmstate_hpet_rtc_irq_level,
-            .needed = hpet_rtc_irq_level_needed,
-        }, {
-            /* empty */
-        }
+    .subsections = (const VMStateDescription*[]) {
+        &vmstate_hpet_rtc_irq_level,
+        NULL
     }
 };

diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
index f2b77fa..3204825 100644
--- a/hw/timer/mc146818rtc.c
+++ b/hw/timer/mc146818rtc.c
@@ -733,22 +733,23 @@ static int rtc_post_load(void *opaque, int version_id)
     return 0;
 }

+static bool rtc_irq_reinject_on_ack_count_needed(void *opaque)
+{
+    RTCState *s = (RTCState *)opaque;
+    return s->irq_reinject_on_ack_count != 0;
+}
+
 static const VMStateDescription vmstate_rtc_irq_reinject_on_ack_count = {
     .name = "mc146818rtc/irq_reinject_on_ack_count",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = rtc_irq_reinject_on_ack_count_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT16(irq_reinject_on_ack_count, RTCState),
         VMSTATE_END_OF_LIST()
     }
 };

-static bool rtc_irq_reinject_on_ack_count_needed(void *opaque)
-{
-    RTCState *s = (RTCState *)opaque;
-    return s->irq_reinject_on_ack_count != 0;
-}
-
 static const VMStateDescription vmstate_rtc = {
     .name = "mc146818rtc",
     .version_id = 3,
@@ -770,13 +771,9 @@ static const VMStateDescription vmstate_rtc = {
         VMSTATE_UINT64_V(next_alarm_time, RTCState, 3),
         VMSTATE_END_OF_LIST()
     },
-    .subsections = (VMStateSubsection[]) {
-        {
-            .vmsd = &vmstate_rtc_irq_reinject_on_ack_count,
-            .needed = rtc_irq_reinject_on_ack_count_needed,
-        }, {
-            /* empty */
-        }
+    .subsections = (const VMStateDescription*[]) {
+        &vmstate_rtc_irq_reinject_on_ack_count,
+        NULL
     }
 };

diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index 1a22c9c..7d65818 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -2034,6 +2034,7 @@ static const VMStateDescription vmstate_ohci_eof_timer = {
     .version_id = 1,
     .minimum_version_id = 1,
     .pre_load = ohci_eof_timer_pre_load,
+    .needed = ohci_eof_timer_needed,
     .fields = (VMStateField[]) {
         VMSTATE_TIMER_PTR(eof_timer, OHCIState),
         VMSTATE_END_OF_LIST()
@@ -2081,13 +2082,9 @@ static const VMStateDescription vmstate_ohci_state = {
         VMSTATE_BOOL(async_complete, OHCIState),
         VMSTATE_END_OF_LIST()
     },
-    .subsections = (VMStateSubsection []) {
-        {
-            .vmsd = &vmstate_ohci_eof_timer,
-            .needed = ohci_eof_timer_needed,
-        } , {
-            /* empty */
-        }
+    .subsections = (const VMStateDescription*[]) {
+        &vmstate_ohci_eof_timer,
+        NULL
     }
 };

diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index 242a654..6b4218c 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -2257,40 +2257,42 @@ static const VMStateInfo usbredir_ep_bufpq_vmstate_info = {


 /* For endp_data migration */
+static bool usbredir_bulk_receiving_needed(void *priv)
+{
+    struct endp_data *endp = priv;
+
+    return endp->bulk_receiving_started;
+}
+
 static const VMStateDescription usbredir_bulk_receiving_vmstate = {
     .name = "usb-redir-ep/bulk-receiving",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = usbredir_bulk_receiving_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT8(bulk_receiving_started, struct endp_data),
         VMSTATE_END_OF_LIST()
     }
 };

-static bool usbredir_bulk_receiving_needed(void *priv)
+static bool usbredir_stream_needed(void *priv)
 {
     struct endp_data *endp = priv;

-    return endp->bulk_receiving_started;
+    return endp->max_streams;
 }

 static const VMStateDescription usbredir_stream_vmstate = {
     .name = "usb-redir-ep/stream-state",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = usbredir_stream_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT32(max_streams, struct endp_data),
         VMSTATE_END_OF_LIST()
     }
 };

-static bool usbredir_stream_needed(void *priv)
-{
-    struct endp_data *endp = priv;
-
-    return endp->max_streams;
-}
-
 static const VMStateDescription usbredir_ep_vmstate = {
     .name = "usb-redir-ep",
     .version_id = 1,
@@ -2318,16 +2320,10 @@ static const VMStateDescription usbredir_ep_vmstate = {
         VMSTATE_INT32(bufpq_target_size, struct endp_data),
         VMSTATE_END_OF_LIST()
     },
-    .subsections = (VMStateSubsection[]) {
-        {
-            .vmsd = &usbredir_bulk_receiving_vmstate,
-            .needed = usbredir_bulk_receiving_needed,
-        }, {
-            .vmsd = &usbredir_stream_vmstate,
-            .needed = usbredir_stream_needed,
-        }, {
-            /* empty */
-        }
+    .subsections = (const VMStateDescription*[]) {
+        &usbredir_bulk_receiving_vmstate,
+        &usbredir_stream_vmstate,
+        NULL
     }
 };

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 6985e76..174a1be 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -897,6 +897,7 @@ static const VMStateDescription vmstate_virtio_device_endian = {
     .name = "virtio/device_endian",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = &virtio_device_endian_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT8(device_endian, VirtIODevice),
         VMSTATE_END_OF_LIST()
@@ -911,12 +912,9 @@ static const VMStateDescription vmstate_virtio = {
     .fields = (VMStateField[]) {
         VMSTATE_END_OF_LIST()
     },
-    .subsections = (VMStateSubsection[]) {
-        {
-            .vmsd = &vmstate_virtio_device_endian,
-            .needed = &virtio_device_endian_needed
-        },
-        { 0 }
+    .subsections = (const VMStateDescription*[]) {
+        &vmstate_virtio_device_endian,
+        NULL
     }
 };

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index bc7616a..fc5e643 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -120,11 +120,6 @@ 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 unmigratable;
@@ -135,8 +130,9 @@ struct VMStateDescription {
     int (*pre_load)(void *opaque);
     int (*post_load)(void *opaque, int version_id);
     void (*pre_save)(void *opaque);
+    bool (*needed)(void *opaque);
     VMStateField *fields;
-    const VMStateSubsection *subsections;
+    const VMStateDescription **subsections;
 };

 extern const VMStateDescription vmstate_dummy;
diff --git a/migration/vmstate.c b/migration/vmstate.c
index e5388f0..108995e 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -341,11 +341,11 @@ void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
 }

 static const VMStateDescription *
-    vmstate_get_subsection(const VMStateSubsection *sub, char *idstr)
+vmstate_get_subsection(const VMStateDescription **sub, char *idstr)
 {
-    while (sub && sub->needed) {
-        if (strcmp(idstr, sub->vmsd->name) == 0) {
-            return sub->vmsd;
+    while (sub && *sub && (*sub)->needed) {
+        if (strcmp(idstr, (*sub)->name) == 0) {
+            return *sub;
         }
         sub++;
     }
@@ -405,12 +405,12 @@ static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
 static void vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
                                     void *opaque, QJSON *vmdesc)
 {
-    const VMStateSubsection *sub = vmsd->subsections;
+    const VMStateDescription **sub = vmsd->subsections;
     bool subsection_found = false;

-    while (sub && sub->needed) {
-        if (sub->needed(opaque)) {
-            const VMStateDescription *vmsd = sub->vmsd;
+    while (sub && *sub && (*sub)->needed) {
+        if ((*sub)->needed(opaque)) {
+            const VMStateDescription *vmsd = *sub;
             uint8_t len;

             if (vmdesc) {
diff --git a/savevm.c b/savevm.c
index 1014e3e..01770cd 100644
--- a/savevm.c
+++ b/savevm.c
@@ -268,11 +268,11 @@ static void dump_vmstate_vmsf(FILE *out_file, const VMStateField *field,
 }

 static void dump_vmstate_vmss(FILE *out_file,
-                              const VMStateSubsection *subsection,
+                              const VMStateDescription **subsection,
                               int indent)
 {
-    if (subsection->vmsd != NULL) {
-        dump_vmstate_vmsd(out_file, subsection->vmsd, indent, true);
+    if (*subsection != NULL) {
+        dump_vmstate_vmsd(out_file, *subsection, indent, true);
     }
 }

@@ -313,12 +313,12 @@ static void dump_vmstate_vmsd(FILE *out_file,
         fprintf(out_file, "\n%*s]", indent, "");
     }
     if (vmsd->subsections != NULL) {
-        const VMStateSubsection *subsection = vmsd->subsections;
+        const VMStateDescription **subsection = vmsd->subsections;
         bool first;

         fprintf(out_file, ",\n%*s\"Subsections\": [\n", indent, "");
         first = true;
-        while (subsection->vmsd != NULL) {
+        while (*subsection != NULL) {
             if (!first) {
                 fprintf(out_file, ",\n");
             }
diff --git a/target-arm/machine.c b/target-arm/machine.c
index 9446e5a..36365a5 100644
--- a/target-arm/machine.c
+++ b/target-arm/machine.c
@@ -40,6 +40,7 @@ static const VMStateDescription vmstate_vfp = {
     .name = "cpu/vfp",
     .version_id = 3,
     .minimum_version_id = 3,
+    .needed = vfp_needed,
     .fields = (VMStateField[]) {
         VMSTATE_FLOAT64_ARRAY(env.vfp.regs, ARMCPU, 64),
         /* The xregs array is a little awkward because element 1 (FPSCR)
@@ -72,6 +73,7 @@ static const VMStateDescription vmstate_iwmmxt = {
     .name = "cpu/iwmmxt",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = iwmmxt_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT64_ARRAY(env.iwmmxt.regs, ARMCPU, 16),
         VMSTATE_UINT32_ARRAY(env.iwmmxt.cregs, ARMCPU, 16),
@@ -91,6 +93,7 @@ static const VMStateDescription vmstate_m = {
     .name = "cpu/m",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = m_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT32(env.v7m.other_sp, ARMCPU),
         VMSTATE_UINT32(env.v7m.vecbase, ARMCPU),
@@ -114,6 +117,7 @@ static const VMStateDescription vmstate_thumb2ee = {
     .name = "cpu/thumb2ee",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = thumb2ee_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT32(env.teecr, ARMCPU),
         VMSTATE_UINT32(env.teehbr, ARMCPU),
@@ -282,21 +286,11 @@ const VMStateDescription vmstate_arm_cpu = {
         VMSTATE_BOOL(powered_off, ARMCPU),
         VMSTATE_END_OF_LIST()
     },
-    .subsections = (VMStateSubsection[]) {
-        {
-            .vmsd = &vmstate_vfp,
-            .needed = vfp_needed,
-        } , {
-            .vmsd = &vmstate_iwmmxt,
-            .needed = iwmmxt_needed,
-        } , {
-            .vmsd = &vmstate_m,
-            .needed = m_needed,
-        } , {
-            .vmsd = &vmstate_thumb2ee,
-            .needed = thumb2ee_needed,
-        } , {
-            /* empty */
-        }
+    .subsections = (const VMStateDescription*[]) {
+        &vmstate_vfp,
+        &vmstate_iwmmxt,
+        &vmstate_m,
+        &vmstate_thumb2ee,
+        NULL
     }
 };
diff --git a/target-i386/machine.c b/target-i386/machine.c
index cd1ddd2..a89b0f8 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -400,6 +400,7 @@ static const VMStateDescription vmstate_steal_time_msr = {
     .name = "cpu/steal_time_msr",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = steal_time_msr_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT64(env.steal_time_msr, X86CPU),
         VMSTATE_END_OF_LIST()
@@ -410,6 +411,7 @@ static const VMStateDescription vmstate_async_pf_msr = {
     .name = "cpu/async_pf_msr",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = async_pf_msr_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT64(env.async_pf_en_msr, X86CPU),
         VMSTATE_END_OF_LIST()
@@ -420,6 +422,7 @@ static const VMStateDescription vmstate_pv_eoi_msr = {
     .name = "cpu/async_pv_eoi_msr",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = pv_eoi_msr_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT64(env.pv_eoi_en_msr, X86CPU),
         VMSTATE_END_OF_LIST()
@@ -438,6 +441,7 @@ static const VMStateDescription vmstate_fpop_ip_dp = {
     .name = "cpu/fpop_ip_dp",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = fpop_ip_dp_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT16(env.fpop, X86CPU),
         VMSTATE_UINT64(env.fpip, X86CPU),
@@ -458,6 +462,7 @@ static const VMStateDescription vmstate_msr_tsc_adjust = {
     .name = "cpu/msr_tsc_adjust",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = tsc_adjust_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT64(env.tsc_adjust, X86CPU),
         VMSTATE_END_OF_LIST()
@@ -476,6 +481,7 @@ static const VMStateDescription vmstate_msr_tscdeadline = {
     .name = "cpu/msr_tscdeadline",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = tscdeadline_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT64(env.tsc_deadline, X86CPU),
         VMSTATE_END_OF_LIST()
@@ -502,6 +508,7 @@ static const VMStateDescription vmstate_msr_ia32_misc_enable = {
     .name = "cpu/msr_ia32_misc_enable",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = misc_enable_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT64(env.msr_ia32_misc_enable, X86CPU),
         VMSTATE_END_OF_LIST()
@@ -512,6 +519,7 @@ static const VMStateDescription vmstate_msr_ia32_feature_control = {
     .name = "cpu/msr_ia32_feature_control",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = feature_control_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT64(env.msr_ia32_feature_control, X86CPU),
         VMSTATE_END_OF_LIST()
@@ -546,6 +554,7 @@ static const VMStateDescription vmstate_msr_architectural_pmu = {
     .name = "cpu/msr_architectural_pmu",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = pmu_enable_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT64(env.msr_fixed_ctr_ctrl, X86CPU),
         VMSTATE_UINT64(env.msr_global_ctrl, X86CPU),
@@ -581,6 +590,7 @@ static const VMStateDescription vmstate_mpx = {
     .name = "cpu/mpx",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = mpx_needed,
     .fields = (VMStateField[]) {
         VMSTATE_BND_REGS(env.bnd_regs, X86CPU, 4),
         VMSTATE_UINT64(env.bndcs_regs.cfgu, X86CPU),
@@ -602,6 +612,7 @@ static const VMStateDescription vmstate_msr_hypercall_hypercall = {
     .name = "cpu/msr_hyperv_hypercall",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = hyperv_hypercall_enable_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT64(env.msr_hv_guest_os_id, X86CPU),
         VMSTATE_UINT64(env.msr_hv_hypercall, X86CPU),
@@ -621,6 +632,7 @@ static const VMStateDescription vmstate_msr_hyperv_vapic = {
     .name = "cpu/msr_hyperv_vapic",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = hyperv_vapic_enable_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT64(env.msr_hv_vapic, X86CPU),
         VMSTATE_END_OF_LIST()
@@ -639,6 +651,7 @@ static const VMStateDescription vmstate_msr_hyperv_time = {
     .name = "cpu/msr_hyperv_time",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = hyperv_time_enable_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT64(env.msr_hv_tsc, X86CPU),
         VMSTATE_END_OF_LIST()
@@ -680,6 +693,7 @@ static const VMStateDescription vmstate_avx512 = {
     .name = "cpu/avx512",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = avx512_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT64_ARRAY(env.opmask_regs, X86CPU, NB_OPMASK_REGS),
         VMSTATE_ZMMH_REGS_VARS(env.xmm_regs, X86CPU, 0),
@@ -702,6 +716,7 @@ static const VMStateDescription vmstate_xss = {
     .name = "cpu/xss",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = xss_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT64(env.xss, X86CPU),
         VMSTATE_END_OF_LIST()
@@ -810,54 +825,22 @@ VMStateDescription vmstate_x86_cpu = {
         VMSTATE_END_OF_LIST()
         /* The above list is not sorted /wrt version numbers, watch out! */
     },
-    .subsections = (VMStateSubsection []) {
-        {
-            .vmsd = &vmstate_async_pf_msr,
-            .needed = async_pf_msr_needed,
-        } , {
-            .vmsd = &vmstate_pv_eoi_msr,
-            .needed = pv_eoi_msr_needed,
-        } , {
-            .vmsd = &vmstate_steal_time_msr,
-            .needed = steal_time_msr_needed,
-        } , {
-            .vmsd = &vmstate_fpop_ip_dp,
-            .needed = fpop_ip_dp_needed,
-        }, {
-            .vmsd = &vmstate_msr_tsc_adjust,
-            .needed = tsc_adjust_needed,
-        }, {
-            .vmsd = &vmstate_msr_tscdeadline,
-            .needed = tscdeadline_needed,
-        }, {
-            .vmsd = &vmstate_msr_ia32_misc_enable,
-            .needed = misc_enable_needed,
-        }, {
-            .vmsd = &vmstate_msr_ia32_feature_control,
-            .needed = feature_control_needed,
-        }, {
-            .vmsd = &vmstate_msr_architectural_pmu,
-            .needed = pmu_enable_needed,
-        } , {
-            .vmsd = &vmstate_mpx,
-            .needed = mpx_needed,
-        }, {
-            .vmsd = &vmstate_msr_hypercall_hypercall,
-            .needed = hyperv_hypercall_enable_needed,
-        }, {
-            .vmsd = &vmstate_msr_hyperv_vapic,
-            .needed = hyperv_vapic_enable_needed,
-        }, {
-            .vmsd = &vmstate_msr_hyperv_time,
-            .needed = hyperv_time_enable_needed,
-        }, {
-            .vmsd = &vmstate_avx512,
-            .needed = avx512_needed,
-         }, {
-            .vmsd = &vmstate_xss,
-            .needed = xss_needed,
-        } , {
-            /* empty */
-        }
+    .subsections = (const VMStateDescription*[]) {
+        &vmstate_async_pf_msr,
+        &vmstate_pv_eoi_msr,
+        &vmstate_steal_time_msr,
+        &vmstate_fpop_ip_dp,
+        &vmstate_msr_tsc_adjust,
+        &vmstate_msr_tscdeadline,
+        &vmstate_msr_ia32_misc_enable,
+        &vmstate_msr_ia32_feature_control,
+        &vmstate_msr_architectural_pmu,
+        &vmstate_mpx,
+        &vmstate_msr_hypercall_hypercall,
+        &vmstate_msr_hyperv_vapic,
+        &vmstate_msr_hyperv_time,
+        &vmstate_avx512,
+        &vmstate_xss,
+        NULL
     }
 };
diff --git a/target-ppc/machine.c b/target-ppc/machine.c
index d875211..f4ac761 100644
--- a/target-ppc/machine.c
+++ b/target-ppc/machine.c
@@ -213,6 +213,7 @@ static const VMStateDescription vmstate_fpu = {
     .name = "cpu/fpu",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = fpu_needed,
     .fields = (VMStateField[]) {
         VMSTATE_FLOAT64_ARRAY(env.fpr, PowerPCCPU, 32),
         VMSTATE_UINTTL(env.fpscr, PowerPCCPU),
@@ -231,6 +232,7 @@ static const VMStateDescription vmstate_altivec = {
     .name = "cpu/altivec",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = altivec_needed,
     .fields = (VMStateField[]) {
         VMSTATE_AVR_ARRAY(env.avr, PowerPCCPU, 32),
         VMSTATE_UINT32(env.vscr, PowerPCCPU),
@@ -249,6 +251,7 @@ static const VMStateDescription vmstate_vsx = {
     .name = "cpu/vsx",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = vsx_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT64_ARRAY(env.vsr, PowerPCCPU, 32),
         VMSTATE_END_OF_LIST()
@@ -269,6 +272,7 @@ static const VMStateDescription vmstate_tm = {
     .version_id = 1,
     .minimum_version_id = 1,
     .minimum_version_id_old = 1,
+    .needed = tm_needed,
     .fields      = (VMStateField []) {
         VMSTATE_UINTTL_ARRAY(env.tm_gpr, PowerPCCPU, 32),
         VMSTATE_AVR_ARRAY(env.tm_vsr, PowerPCCPU, 64),
@@ -302,6 +306,7 @@ static const VMStateDescription vmstate_sr = {
     .name = "cpu/sr",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = sr_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINTTL_ARRAY(env.sr, PowerPCCPU, 32),
         VMSTATE_END_OF_LIST()
@@ -351,6 +356,7 @@ static const VMStateDescription vmstate_slb = {
     .name = "cpu/slb",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = slb_needed,
     .fields = (VMStateField[]) {
         VMSTATE_INT32_EQUAL(env.slb_nr, PowerPCCPU),
         VMSTATE_SLB_ARRAY(env.slb, PowerPCCPU, MAX_SLB_ENTRIES),
@@ -383,6 +389,7 @@ static const VMStateDescription vmstate_tlb6xx = {
     .name = "cpu/tlb6xx",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = tlb6xx_needed,
     .fields = (VMStateField[]) {
         VMSTATE_INT32_EQUAL(env.nb_tlb, PowerPCCPU),
         VMSTATE_STRUCT_VARRAY_POINTER_INT32(env.tlb.tlb6, PowerPCCPU,
@@ -429,6 +436,7 @@ static const VMStateDescription vmstate_pbr403 = {
     .name = "cpu/pbr403",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = pbr403_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINTTL_ARRAY(env.pb, PowerPCCPU, 4),
         VMSTATE_END_OF_LIST()
@@ -439,6 +447,7 @@ static const VMStateDescription vmstate_tlbemb = {
     .name = "cpu/tlb6xx",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = tlbemb_needed,
     .fields = (VMStateField[]) {
         VMSTATE_INT32_EQUAL(env.nb_tlb, PowerPCCPU),
         VMSTATE_STRUCT_VARRAY_POINTER_INT32(env.tlb.tlbe, PowerPCCPU,
@@ -448,13 +457,9 @@ static const VMStateDescription vmstate_tlbemb = {
         /* 403 protection registers */
         VMSTATE_END_OF_LIST()
     },
-    .subsections = (VMStateSubsection []) {
-        {
-            .vmsd = &vmstate_pbr403,
-            .needed = pbr403_needed,
-        } , {
-            /* empty */
-        }
+    .subsections = (const VMStateDescription*[]) {
+        &vmstate_pbr403,
+        NULL
     }
 };

@@ -483,6 +488,7 @@ static const VMStateDescription vmstate_tlbmas = {
     .name = "cpu/tlbmas",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = tlbmas_needed,
     .fields = (VMStateField[]) {
         VMSTATE_INT32_EQUAL(env.nb_tlb, PowerPCCPU),
         VMSTATE_STRUCT_VARRAY_POINTER_INT32(env.tlb.tlbm, PowerPCCPU,
@@ -533,38 +539,18 @@ const VMStateDescription vmstate_ppc_cpu = {
         VMSTATE_UINT32_EQUAL(env.nb_BATs, PowerPCCPU),
         VMSTATE_END_OF_LIST()
     },
-    .subsections = (VMStateSubsection []) {
-        {
-            .vmsd = &vmstate_fpu,
-            .needed = fpu_needed,
-        } , {
-            .vmsd = &vmstate_altivec,
-            .needed = altivec_needed,
-        } , {
-            .vmsd = &vmstate_vsx,
-            .needed = vsx_needed,
-        } , {
-            .vmsd = &vmstate_sr,
-            .needed = sr_needed,
-        } , {
+    .subsections = (const VMStateDescription*[]) {
+        &vmstate_fpu,
+        &vmstate_altivec,
+        &vmstate_vsx,
+        &vmstate_sr,
 #ifdef TARGET_PPC64
-            .vmsd = &vmstate_tm,
-            .needed = tm_needed,
-        } , {
-            .vmsd = &vmstate_slb,
-            .needed = slb_needed,
-        } , {
+        &vmstate_tm,
+        &vmstate_slb,
 #endif /* TARGET_PPC64 */
-            .vmsd = &vmstate_tlb6xx,
-            .needed = tlb6xx_needed,
-        } , {
-            .vmsd = &vmstate_tlbemb,
-            .needed = tlbemb_needed,
-        } , {
-            .vmsd = &vmstate_tlbmas,
-            .needed = tlbmas_needed,
-        } , {
-            /* empty */
-        }
+        &vmstate_tlb6xx,
+        &vmstate_tlbemb,
+        &vmstate_tlbmas,
+        NULL
     }
 };
diff --git a/target-s390x/machine.c b/target-s390x/machine.c
index 7853e3c..23d3fa9 100644
--- a/target-s390x/machine.c
+++ b/target-s390x/machine.c
@@ -42,10 +42,16 @@ static void cpu_pre_save(void *opaque)
     }
 }

+static inline bool fpu_needed(void *opaque)
+{
+    return true;
+}
+
 const VMStateDescription vmstate_fpu = {
     .name = "cpu/fpu",
     .version_id = 1,
     .minimum_version_id = 1,
+    .needed = fpu_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT64(env.fregs[0].ll, S390CPU),
         VMSTATE_UINT64(env.fregs[1].ll, S390CPU),
@@ -68,11 +74,6 @@ const VMStateDescription vmstate_fpu = {
     }
 };

-static inline bool fpu_needed(void *opaque)
-{
-    return true;
-}
-
 const VMStateDescription vmstate_s390_cpu = {
     .name = "cpu",
     .post_load = cpu_post_load,
@@ -101,12 +102,8 @@ const VMStateDescription vmstate_s390_cpu = {
                                irqstate_saved_size),
         VMSTATE_END_OF_LIST()
      },
-    .subsections = (VMStateSubsection[]) {
-        {
-            .vmsd = &vmstate_fpu,
-            .needed = fpu_needed,
-        } , {
-            /* empty */
-        }
+    .subsections = (const VMStateDescription*[]) {
+        &vmstate_fpu,
+        NULL
     },
 };
-- 
2.4.0

Re: [Qemu-devel] [PATCH 2/9] migration: Use normal VMStateDescriptions for Subsections

[Dr. David Alan Gilbert]

* Juan Quintela (quintela@redhat.com) wrote:
> We create optional sections with this patch.  But we already have
> optional subsections.  Instead of having two mechanism that do the
> same, we can just generalize it.
> 
> For subsections we just change:
> 
> - Add a needed function to VMStateDescription
> - Remove VMStateSubsection (after removal of the needed function
>   it is just a VMStateDescription)
> - Adjust the whole tree, moving the needed function to the corresponding
>   VMStateDescription
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  cpus.c                      | 11 +++---
>  docs/migration.txt          | 11 +++---
>  exec.c                      | 11 +++---
>  hw/acpi/ich9.c              | 10 +++---
>  hw/acpi/piix4.c             | 10 +++---
>  hw/block/fdc.c              | 37 ++++++++-------------
>  hw/char/serial.c            | 41 +++++++++--------------
>  hw/display/qxl.c            | 11 +++---
>  hw/display/vga.c            | 11 +++---
>  hw/ide/core.c               | 32 +++++++-----------
>  hw/ide/pci.c                | 16 ++++-----
>  hw/input/pckbd.c            | 22 ++++++------
>  hw/input/ps2.c              | 11 +++---
>  hw/intc/apic_common.c       | 10 +++---
>  hw/isa/lpc_ich9.c           | 10 +++---
>  hw/net/e1000.c              | 11 +++---
>  hw/net/rtl8139.c            | 11 +++---
>  hw/net/vmxnet3.c            | 12 +++----
>  hw/pci-host/piix.c          | 10 +++---
>  hw/scsi/scsi-bus.c          | 11 +++---
>  hw/timer/hpet.c             | 11 +++---
>  hw/timer/mc146818rtc.c      | 23 ++++++-------
>  hw/usb/hcd-ohci.c           | 11 +++---
>  hw/usb/redirect.c           | 34 +++++++++----------
>  hw/virtio/virtio.c          | 10 +++---
>  include/migration/vmstate.h |  8 ++---
>  migration/vmstate.c         | 16 ++++-----
>  savevm.c                    | 10 +++---
>  target-arm/machine.c        | 26 ++++++---------
>  target-i386/machine.c       | 81 ++++++++++++++++++---------------------------
>  target-ppc/machine.c        | 62 ++++++++++++++--------------------
>  target-s390x/machine.c      | 21 +++++-------
>  32 files changed, 245 insertions(+), 377 deletions(-)
> 
> diff --git a/cpus.c b/cpus.c
> index 62d157a..6e9b0a5 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -459,6 +459,7 @@ static const VMStateDescription icount_vmstate_timers = {
>      .name = "timer/icount",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = icount_state_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_INT64(qemu_icount_bias, TimersState),
>          VMSTATE_INT64(qemu_icount, TimersState),
> @@ -476,13 +477,9 @@ static const VMStateDescription vmstate_timers = {
>          VMSTATE_INT64_V(cpu_clock_offset, TimersState, 2),
>          VMSTATE_END_OF_LIST()
>      },
> -    .subsections = (VMStateSubsection[]) {
> -        {
> -            .vmsd = &icount_vmstate_timers,
> -            .needed = icount_state_needed,
> -        }, {
> -            /* empty */
> -        }
> +    .subsections = (const VMStateDescription*[]) {
> +        &icount_vmstate_timers,
> +        NULL
>      }
>  };
> 
> diff --git a/docs/migration.txt b/docs/migration.txt
> index 0492a45..f6df4be 100644
> --- a/docs/migration.txt
> +++ b/docs/migration.txt
> @@ -257,6 +257,7 @@ const VMStateDescription vmstate_ide_drive_pio_state = {
>      .minimum_version_id = 1,
>      .pre_save = ide_drive_pio_pre_save,
>      .post_load = ide_drive_pio_post_load,
> +    .needed = ide_drive_pio_state_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_INT32(req_nb_sectors, IDEState),
>          VMSTATE_VARRAY_INT32(io_buffer, IDEState, io_buffer_total_len, 1,
> @@ -279,13 +280,9 @@ const VMStateDescription vmstate_ide_drive = {
>          .... several fields ....
>          VMSTATE_END_OF_LIST()
>      },
> -    .subsections = (VMStateSubsection []) {
> -        {
> -            .vmsd = &vmstate_ide_drive_pio_state,
> -            .needed = ide_drive_pio_state_needed,
> -        }, {
> -            /* empty */
> -        }
> +    .subsections = (const VMStateDescription*[]) {
> +        &vmstate_ide_drive_pio_state,
> +        NULL
>      }
>  };
> 
> diff --git a/exec.c b/exec.c
> index e19ab22..d5c2cfb 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -460,6 +460,7 @@ static const VMStateDescription vmstate_cpu_common_exception_index = {
>      .name = "cpu_common/exception_index",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = cpu_common_exception_index_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_INT32(exception_index, CPUState),
>          VMSTATE_END_OF_LIST()
> @@ -477,13 +478,9 @@ const VMStateDescription vmstate_cpu_common = {
>          VMSTATE_UINT32(interrupt_request, CPUState),
>          VMSTATE_END_OF_LIST()
>      },
> -    .subsections = (VMStateSubsection[]) {
> -        {
> -            .vmsd = &vmstate_cpu_common_exception_index,
> -            .needed = cpu_common_exception_index_needed,
> -        } , {
> -            /* empty */
> -        }
> +    .subsections = (const VMStateDescription*[]) {
> +        &vmstate_cpu_common_exception_index,
> +        NULL
>      }
>  };
> 
> diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> index 84e5bb8..f3e1b6b 100644
> --- a/hw/acpi/ich9.c
> +++ b/hw/acpi/ich9.c
> @@ -151,6 +151,7 @@ static const VMStateDescription vmstate_memhp_state = {
>      .version_id = 1,
>      .minimum_version_id = 1,
>      .minimum_version_id_old = 1,
> +    .needed = vmstate_test_use_memhp,
>      .fields      = (VMStateField[]) {
>          VMSTATE_MEMORY_HOTPLUG(acpi_memory_hotplug, ICH9LPCPMRegs),
>          VMSTATE_END_OF_LIST()
> @@ -174,12 +175,9 @@ const VMStateDescription vmstate_ich9_pm = {
>          VMSTATE_UINT32(smi_sts, ICH9LPCPMRegs),
>          VMSTATE_END_OF_LIST()
>      },
> -    .subsections = (VMStateSubsection[]) {
> -        {
> -            .vmsd = &vmstate_memhp_state,
> -            .needed = vmstate_test_use_memhp,
> -        },
> -        VMSTATE_END_OF_LIST()
> +    .subsections = (const VMStateDescription*[]) {
> +        &vmstate_memhp_state,
> +        NULL
>      }
>  };
> 
> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index 1b28481..e7dff38 100644
> --- a/hw/acpi/piix4.c
> +++ b/hw/acpi/piix4.c
> @@ -260,6 +260,7 @@ static const VMStateDescription vmstate_memhp_state = {
>      .version_id = 1,
>      .minimum_version_id = 1,
>      .minimum_version_id_old = 1,
> +    .needed = vmstate_test_use_memhp,
>      .fields      = (VMStateField[]) {
>          VMSTATE_MEMORY_HOTPLUG(acpi_memory_hotplug, PIIX4PMState),
>          VMSTATE_END_OF_LIST()
> @@ -298,12 +299,9 @@ static const VMStateDescription vmstate_acpi = {
>                              vmstate_test_use_acpi_pci_hotplug),
>          VMSTATE_END_OF_LIST()
>      },
> -    .subsections = (VMStateSubsection[]) {
> -        {
> -            .vmsd = &vmstate_memhp_state,
> -            .needed = vmstate_test_use_memhp,
> -        },
> -        VMSTATE_END_OF_LIST()
> +    .subsections = (const VMStateDescription*[]) {
> +         &vmstate_memhp_state,
> +         NULL
>      }
>  };
> 
> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
> index d8a8edd..17d427f 100644
> --- a/hw/block/fdc.c
> +++ b/hw/block/fdc.c
> @@ -671,6 +671,7 @@ static const VMStateDescription vmstate_fdrive_media_changed = {
>      .name = "fdrive/media_changed",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = fdrive_media_changed_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT8(media_changed, FDrive),
>          VMSTATE_END_OF_LIST()
> @@ -688,6 +689,7 @@ static const VMStateDescription vmstate_fdrive_media_rate = {
>      .name = "fdrive/media_rate",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = fdrive_media_rate_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT8(media_rate, FDrive),
>          VMSTATE_END_OF_LIST()
> @@ -705,6 +707,7 @@ static const VMStateDescription vmstate_fdrive_perpendicular = {
>      .name = "fdrive/perpendicular",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = fdrive_perpendicular_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT8(perpendicular, FDrive),
>          VMSTATE_END_OF_LIST()
> @@ -728,19 +731,11 @@ static const VMStateDescription vmstate_fdrive = {
>          VMSTATE_UINT8(sect, FDrive),
>          VMSTATE_END_OF_LIST()
>      },
> -    .subsections = (VMStateSubsection[]) {
> -        {
> -            .vmsd = &vmstate_fdrive_media_changed,
> -            .needed = &fdrive_media_changed_needed,
> -        } , {
> -            .vmsd = &vmstate_fdrive_media_rate,
> -            .needed = &fdrive_media_rate_needed,
> -        } , {
> -            .vmsd = &vmstate_fdrive_perpendicular,
> -            .needed = &fdrive_perpendicular_needed,
> -        } , {
> -            /* empty */
> -        }
> +    .subsections = (const VMStateDescription*[]) {
> +        &vmstate_fdrive_media_changed,
> +        &vmstate_fdrive_media_rate,
> +        &vmstate_fdrive_perpendicular,
> +        NULL
>      }
>  };
> 
> @@ -771,6 +766,7 @@ static const VMStateDescription vmstate_fdc_reset_sensei = {
>      .name = "fdc/reset_sensei",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = fdc_reset_sensei_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_INT32(reset_sensei, FDCtrl),
>          VMSTATE_END_OF_LIST()
> @@ -788,6 +784,7 @@ static const VMStateDescription vmstate_fdc_result_timer = {
>      .name = "fdc/result_timer",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = fdc_result_timer_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_TIMER_PTR(result_timer, FDCtrl),
>          VMSTATE_END_OF_LIST()
> @@ -831,16 +828,10 @@ static const VMStateDescription vmstate_fdc = {
>                               vmstate_fdrive, FDrive),
>          VMSTATE_END_OF_LIST()
>      },
> -    .subsections = (VMStateSubsection[]) {
> -        {
> -            .vmsd = &vmstate_fdc_reset_sensei,
> -            .needed = fdc_reset_sensei_needed,
> -        } , {
> -            .vmsd = &vmstate_fdc_result_timer,
> -            .needed = fdc_result_timer_needed,
> -        } , {
> -            /* empty */
> -        }
> +    .subsections = (const VMStateDescription*[]) {
> +        &vmstate_fdc_reset_sensei,
> +        &vmstate_fdc_result_timer,
> +        NULL
>      }
>  };
> 
> diff --git a/hw/char/serial.c b/hw/char/serial.c
> index 55011cf..513d73c 100644
> --- a/hw/char/serial.c
> +++ b/hw/char/serial.c
> @@ -662,6 +662,7 @@ static const VMStateDescription vmstate_serial_thr_ipending = {
>      .name = "serial/thr_ipending",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = serial_thr_ipending_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_INT32(thr_ipending, SerialState),
>          VMSTATE_END_OF_LIST()
> @@ -678,6 +679,7 @@ static const VMStateDescription vmstate_serial_tsr = {
>      .name = "serial/tsr",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = serial_tsr_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_INT32(tsr_retry, SerialState),
>          VMSTATE_UINT8(thr, SerialState),
> @@ -697,6 +699,7 @@ static const VMStateDescription vmstate_serial_recv_fifo = {
>      .name = "serial/recv_fifo",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = serial_recv_fifo_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_STRUCT(recv_fifo, SerialState, 1, vmstate_fifo8, Fifo8),
>          VMSTATE_END_OF_LIST()
> @@ -713,6 +716,7 @@ static const VMStateDescription vmstate_serial_xmit_fifo = {
>      .name = "serial/xmit_fifo",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = serial_xmit_fifo_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_STRUCT(xmit_fifo, SerialState, 1, vmstate_fifo8, Fifo8),
>          VMSTATE_END_OF_LIST()
> @@ -729,6 +733,7 @@ static const VMStateDescription vmstate_serial_fifo_timeout_timer = {
>      .name = "serial/fifo_timeout_timer",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = serial_fifo_timeout_timer_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_TIMER_PTR(fifo_timeout_timer, SerialState),
>          VMSTATE_END_OF_LIST()
> @@ -745,6 +750,7 @@ static const VMStateDescription vmstate_serial_timeout_ipending = {
>      .name = "serial/timeout_ipending",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = serial_timeout_ipending_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_INT32(timeout_ipending, SerialState),
>          VMSTATE_END_OF_LIST()
> @@ -760,6 +766,7 @@ static bool serial_poll_needed(void *opaque)
>  static const VMStateDescription vmstate_serial_poll = {
>      .name = "serial/poll",
>      .version_id = 1,
> +    .needed = serial_poll_needed,
>      .minimum_version_id = 1,
>      .fields = (VMStateField[]) {
>          VMSTATE_INT32(poll_msl, SerialState),
> @@ -788,31 +795,15 @@ const VMStateDescription vmstate_serial = {
>          VMSTATE_UINT8_V(fcr_vmstate, SerialState, 3),
>          VMSTATE_END_OF_LIST()
>      },
> -    .subsections = (VMStateSubsection[]) {
> -        {
> -            .vmsd = &vmstate_serial_thr_ipending,
> -            .needed = &serial_thr_ipending_needed,
> -        } , {
> -            .vmsd = &vmstate_serial_tsr,
> -            .needed = &serial_tsr_needed,
> -        } , {
> -            .vmsd = &vmstate_serial_recv_fifo,
> -            .needed = &serial_recv_fifo_needed,
> -        } , {
> -            .vmsd = &vmstate_serial_xmit_fifo,
> -            .needed = &serial_xmit_fifo_needed,
> -        } , {
> -            .vmsd = &vmstate_serial_fifo_timeout_timer,
> -            .needed = &serial_fifo_timeout_timer_needed,
> -        } , {
> -            .vmsd = &vmstate_serial_timeout_ipending,
> -            .needed = &serial_timeout_ipending_needed,
> -        } , {
> -            .vmsd = &vmstate_serial_poll,
> -            .needed = &serial_poll_needed,
> -        } , {
> -            /* empty */
> -        }
> +    .subsections = (const VMStateDescription*[]) {
> +        &vmstate_serial_thr_ipending,
> +        &vmstate_serial_tsr,
> +        &vmstate_serial_recv_fifo,
> +        &vmstate_serial_xmit_fifo,
> +        &vmstate_serial_fifo_timeout_timer,
> +        &vmstate_serial_timeout_ipending,
> +        &vmstate_serial_poll,
> +        NULL
>      }
>  };
> 
> diff --git a/hw/display/qxl.c b/hw/display/qxl.c
> index 0cd314c..9db1e1c 100644
> --- a/hw/display/qxl.c
> +++ b/hw/display/qxl.c
> @@ -2216,6 +2216,7 @@ static VMStateDescription qxl_vmstate_monitors_config = {
>      .name               = "qxl/monitors-config",
>      .version_id         = 1,
>      .minimum_version_id = 1,
> +    .needed = qxl_monitors_config_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT64(guest_monitors_config, PCIQXLDevice),
>          VMSTATE_END_OF_LIST()
> @@ -2249,13 +2250,9 @@ static VMStateDescription qxl_vmstate = {
>          VMSTATE_UINT64(guest_cursor, PCIQXLDevice),
>          VMSTATE_END_OF_LIST()
>      },
> -    .subsections = (VMStateSubsection[]) {
> -        {
> -            .vmsd = &qxl_vmstate_monitors_config,
> -            .needed = qxl_monitors_config_needed,
> -        }, {
> -            /* empty */
> -        }
> +    .subsections = (const VMStateDescription*[]) {
> +        &qxl_vmstate_monitors_config,
> +        NULL
>      }
>  };
> 
> diff --git a/hw/display/vga.c b/hw/display/vga.c
> index d1d296c..b35d523 100644
> --- a/hw/display/vga.c
> +++ b/hw/display/vga.c
> @@ -2035,6 +2035,7 @@ static const VMStateDescription vmstate_vga_endian = {
>      .name = "vga.endian",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = vga_endian_state_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_BOOL(big_endian_fb, VGACommonState),
>          VMSTATE_END_OF_LIST()
> @@ -2078,13 +2079,9 @@ const VMStateDescription vmstate_vga_common = {
>          VMSTATE_UINT32(vbe_bank_mask, VGACommonState),
>          VMSTATE_END_OF_LIST()
>      },
> -    .subsections = (VMStateSubsection []) {
> -        {
> -            .vmsd = &vmstate_vga_endian,
> -            .needed = vga_endian_state_needed,
> -        }, {
> -            /* empty */
> -        }
> +    .subsections = (const VMStateDescription*[]) {
> +        &vmstate_vga_endian,
> +        NULL
>      }
>  };
> 
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index fcb9080..1efd98a 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -2561,6 +2561,7 @@ static const VMStateDescription vmstate_ide_atapi_gesn_state = {
>      .name ="ide_drive/atapi/gesn_state",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = ide_atapi_gesn_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_BOOL(events.new_media, IDEState),
>          VMSTATE_BOOL(events.eject_request, IDEState),
> @@ -2572,6 +2573,7 @@ static const VMStateDescription vmstate_ide_tray_state = {
>      .name = "ide_drive/tray_state",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = ide_tray_state_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_BOOL(tray_open, IDEState),
>          VMSTATE_BOOL(tray_locked, IDEState),
> @@ -2585,6 +2587,7 @@ static const VMStateDescription vmstate_ide_drive_pio_state = {
>      .minimum_version_id = 1,
>      .pre_save = ide_drive_pio_pre_save,
>      .post_load = ide_drive_pio_post_load,
> +    .needed = ide_drive_pio_state_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_INT32(req_nb_sectors, IDEState),
>          VMSTATE_VARRAY_INT32(io_buffer, IDEState, io_buffer_total_len, 1,
> @@ -2626,19 +2629,11 @@ const VMStateDescription vmstate_ide_drive = {
>          VMSTATE_UINT8_V(cdrom_changed, IDEState, 3),
>          VMSTATE_END_OF_LIST()
>      },
> -    .subsections = (VMStateSubsection []) {
> -        {
> -            .vmsd = &vmstate_ide_drive_pio_state,
> -            .needed = ide_drive_pio_state_needed,
> -        }, {
> -            .vmsd = &vmstate_ide_tray_state,
> -            .needed = ide_tray_state_needed,
> -        }, {
> -            .vmsd = &vmstate_ide_atapi_gesn_state,
> -            .needed = ide_atapi_gesn_needed,
> -        }, {
> -            /* empty */
> -        }
> +    .subsections = (const VMStateDescription*[]) {
> +        &vmstate_ide_drive_pio_state,
> +        &vmstate_ide_tray_state,
> +        &vmstate_ide_atapi_gesn_state,
> +        NULL
>      }
>  };
> 
> @@ -2646,6 +2641,7 @@ static const VMStateDescription vmstate_ide_error_status = {
>      .name ="ide_bus/error",
>      .version_id = 2,
>      .minimum_version_id = 1,
> +    .needed = ide_error_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_INT32(error_status, IDEBus),
>          VMSTATE_INT64_V(retry_sector_num, IDEBus, 2),
> @@ -2664,13 +2660,9 @@ const VMStateDescription vmstate_ide_bus = {
>          VMSTATE_UINT8(unit, IDEBus),
>          VMSTATE_END_OF_LIST()
>      },
> -    .subsections = (VMStateSubsection []) {
> -        {
> -            .vmsd = &vmstate_ide_error_status,
> -            .needed = ide_error_needed,
> -        }, {
> -            /* empty */
> -        }
> +    .subsections = (const VMStateDescription*[]) {
> +        &vmstate_ide_error_status,
> +        NULL
>      }
>  };
> 
> diff --git a/hw/ide/pci.c b/hw/ide/pci.c
> index 1b3d1c1..1feefef 100644
> --- a/hw/ide/pci.c
> +++ b/hw/ide/pci.c
> @@ -350,6 +350,7 @@ static const VMStateDescription vmstate_bmdma_current = {
>      .name = "ide bmdma_current",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = ide_bmdma_current_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT32(cur_addr, BMDMAState),
>          VMSTATE_UINT32(cur_prd_last, BMDMAState),
> @@ -363,6 +364,7 @@ static const VMStateDescription vmstate_bmdma_status = {
>      .name ="ide bmdma/status",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = ide_bmdma_status_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT8(status, BMDMAState),
>          VMSTATE_END_OF_LIST()
> @@ -383,16 +385,10 @@ static const VMStateDescription vmstate_bmdma = {
>          VMSTATE_UINT8(migration_retry_unit, 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 */
> -        }
> +    .subsections = (const VMStateDescription*[]) {
> +        &vmstate_bmdma_current,
> +        &vmstate_bmdma_status,
> +        NULL
>      }
>  };
> 
> diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
> index 9b9a7d7..ddac69d 100644
> --- a/hw/input/pckbd.c
> +++ b/hw/input/pckbd.c
> @@ -391,23 +391,24 @@ static int kbd_outport_post_load(void *opaque, int version_id)
>      return 0;
>  }
> 
> +static bool kbd_outport_needed(void *opaque)
> +{
> +    KBDState *s = opaque;
> +    return s->outport != kbd_outport_default(s);
> +}
> +
>  static const VMStateDescription vmstate_kbd_outport = {
>      .name = "pckbd_outport",
>      .version_id = 1,
>      .minimum_version_id = 1,
>      .post_load = kbd_outport_post_load,
> +    .needed = kbd_outport_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT8(outport, KBDState),
>          VMSTATE_END_OF_LIST()
>      }
>  };
> 
> -static bool kbd_outport_needed(void *opaque)
> -{
> -    KBDState *s = opaque;
> -    return s->outport != kbd_outport_default(s);
> -}
> -
>  static int kbd_post_load(void *opaque, int version_id)
>  {
>      KBDState *s = opaque;
> @@ -430,12 +431,9 @@ static const VMStateDescription vmstate_kbd = {
>          VMSTATE_UINT8(pending, KBDState),
>          VMSTATE_END_OF_LIST()
>      },
> -    .subsections = (VMStateSubsection[]) {
> -        {
> -            .vmsd = &vmstate_kbd_outport,
> -            .needed = kbd_outport_needed,
> -        },
> -        VMSTATE_END_OF_LIST()
> +    .subsections = (const VMStateDescription*[]) {
> +        &vmstate_kbd_outport,
> +        NULL
>      }
>  };
> 
> diff --git a/hw/input/ps2.c b/hw/input/ps2.c
> index 4baeea2..fdbe565 100644
> --- a/hw/input/ps2.c
> +++ b/hw/input/ps2.c
> @@ -677,6 +677,7 @@ static const VMStateDescription vmstate_ps2_keyboard_ledstate = {
>      .version_id = 3,
>      .minimum_version_id = 2,
>      .post_load = ps2_kbd_ledstate_post_load,
> +    .needed = ps2_keyboard_ledstate_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_INT32(ledstate, PS2KbdState),
>          VMSTATE_END_OF_LIST()
> @@ -717,13 +718,9 @@ static const VMStateDescription vmstate_ps2_keyboard = {
>          VMSTATE_INT32_V(scancode_set, PS2KbdState,3),
>          VMSTATE_END_OF_LIST()
>      },
> -    .subsections = (VMStateSubsection []) {
> -        {
> -            .vmsd = &vmstate_ps2_keyboard_ledstate,
> -            .needed = ps2_keyboard_ledstate_needed,
> -        }, {
> -            /* empty */
> -        }
> +    .subsections = (const VMStateDescription*[]) {
> +        &vmstate_ps2_keyboard_ledstate,
> +        NULL
>      }
>  };
> 
> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> index d595d63..0032b97 100644
> --- a/hw/intc/apic_common.c
> +++ b/hw/intc/apic_common.c
> @@ -369,6 +369,7 @@ static const VMStateDescription vmstate_apic_common_sipi = {
>      .name = "apic_sipi",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = apic_common_sipi_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_INT32(sipi_vector, APICCommonState),
>          VMSTATE_INT32(wait_for_sipi, APICCommonState),
> @@ -408,12 +409,9 @@ static const VMStateDescription vmstate_apic_common = {
>                        APICCommonState), /* open-coded timer state */
>          VMSTATE_END_OF_LIST()
>      },
> -    .subsections = (VMStateSubsection[]) {
> -        {
> -            .vmsd = &vmstate_apic_common_sipi,
> -            .needed = apic_common_sipi_needed,
> -        },
> -        VMSTATE_END_OF_LIST()
> +    .subsections = (const VMStateDescription*[]) {
> +        &vmstate_apic_common_sipi,
> +        NULL
>      }
>  };
> 
> diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
> index dba7585..f7aff03 100644
> --- a/hw/isa/lpc_ich9.c
> +++ b/hw/isa/lpc_ich9.c
> @@ -634,6 +634,7 @@ static const VMStateDescription vmstate_ich9_rst_cnt = {
>      .name = "ICH9LPC/rst_cnt",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = ich9_rst_cnt_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT8(rst_cnt, ICH9LPCState),
>          VMSTATE_END_OF_LIST()
> @@ -653,12 +654,9 @@ static const VMStateDescription vmstate_ich9_lpc = {
>          VMSTATE_UINT32(sci_level, ICH9LPCState),
>          VMSTATE_END_OF_LIST()
>      },
> -    .subsections = (VMStateSubsection[]) {
> -        {
> -            .vmsd = &vmstate_ich9_rst_cnt,
> -            .needed = ich9_rst_cnt_needed
> -        },
> -        { 0 }
> +    .subsections = (const VMStateDescription*[]) {
> +        &vmstate_ich9_rst_cnt,
> +        NULL
>      }
>  };
> 
> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> index 091d61a..bab8e2a 100644
> --- a/hw/net/e1000.c
> +++ b/hw/net/e1000.c
> @@ -1370,6 +1370,7 @@ static const VMStateDescription vmstate_e1000_mit_state = {
>      .name = "e1000/mit_state",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = e1000_mit_state_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT32(mac_reg[RDTR], E1000State),
>          VMSTATE_UINT32(mac_reg[RADV], E1000State),
> @@ -1457,13 +1458,9 @@ static const VMStateDescription vmstate_e1000 = {
>          VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, VFTA, 128),
>          VMSTATE_END_OF_LIST()
>      },
> -    .subsections = (VMStateSubsection[]) {
> -        {
> -            .vmsd = &vmstate_e1000_mit_state,
> -            .needed = e1000_mit_state_needed,
> -        }, {
> -            /* empty */
> -        }
> +    .subsections = (const VMStateDescription*[]) {
> +        &vmstate_e1000_mit_state,
> +        NULL
>      }
>  };
> 
> diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
> index f868108..e0db472 100644
> --- a/hw/net/rtl8139.c
> +++ b/hw/net/rtl8139.c
> @@ -3240,6 +3240,7 @@ static const VMStateDescription vmstate_rtl8139_hotplug_ready ={
>      .name = "rtl8139/hotplug_ready",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = rtl8139_hotplug_ready_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_END_OF_LIST()
>      }
> @@ -3335,13 +3336,9 @@ 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 */
> -        }
> +    .subsections = (const VMStateDescription*[]) {
> +        &vmstate_rtl8139_hotplug_ready,
> +        NULL
>      }
>  };
> 
> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> index dfb328d..8bcdf3e 100644
> --- a/hw/net/vmxnet3.c
> +++ b/hw/net/vmxnet3.c
> @@ -2226,6 +2226,7 @@ static const VMStateDescription vmxstate_vmxnet3_mcast_list = {
>      .version_id = 1,
>      .minimum_version_id = 1,
>      .pre_load = vmxnet3_mcast_list_pre_load,
> +    .needed = vmxnet3_mc_list_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_VBUFFER_UINT32(mcast_list, VMXNET3State, 0, NULL, 0,
>              mcast_list_buff_size),
> @@ -2470,14 +2471,9 @@ static const VMStateDescription vmstate_vmxnet3 = {
> 
>              VMSTATE_END_OF_LIST()
>      },
> -    .subsections = (VMStateSubsection[]) {
> -        {
> -            .vmsd = &vmxstate_vmxnet3_mcast_list,
> -            .needed = vmxnet3_mc_list_needed
> -        },
> -        {
> -            /* empty element. */
> -        }
> +    .subsections = (const VMStateDescription*[]) {
> +        &vmxstate_vmxnet3_mcast_list,
> +        NULL
>      }
>  };
> 
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index 723836f..1936fd4 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -578,6 +578,7 @@ static const VMStateDescription vmstate_piix3_rcr = {
>      .name = "PIIX3/rcr",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = piix3_rcr_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT8(rcr, PIIX3State),
>          VMSTATE_END_OF_LIST()
> @@ -596,12 +597,9 @@ static const VMStateDescription vmstate_piix3 = {
>                                PIIX_NUM_PIRQS, 3),
>          VMSTATE_END_OF_LIST()
>      },
> -    .subsections = (VMStateSubsection[]) {
> -        {
> -            .vmsd = &vmstate_piix3_rcr,
> -            .needed = piix3_rcr_needed,
> -        },
> -        { 0 }
> +    .subsections = (const VMStateDescription*[]) {
> +        &vmstate_piix3_rcr,
> +        NULL
>      }
>  };
> 
> diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
> index bd2c0e4..f50b2f0 100644
> --- a/hw/scsi/scsi-bus.c
> +++ b/hw/scsi/scsi-bus.c
> @@ -1968,6 +1968,7 @@ static const VMStateDescription vmstate_scsi_sense_state = {
>      .name = "SCSIDevice/sense",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = scsi_sense_state_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT8_SUB_ARRAY(sense, SCSIDevice,
>                                  SCSI_SENSE_BUF_SIZE_OLD,
> @@ -1998,13 +1999,9 @@ const VMStateDescription vmstate_scsi_device = {
>          },
>          VMSTATE_END_OF_LIST()
>      },
> -    .subsections = (VMStateSubsection []) {
> -        {
> -            .vmsd = &vmstate_scsi_sense_state,
> -            .needed = scsi_sense_state_needed,
> -        }, {
> -            /* empty */
> -        }
> +    .subsections = (const VMStateDescription*[]) {
> +        &vmstate_scsi_sense_state,
> +        NULL
>      }
>  };
> 
> diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
> index b6b8a20..b50071e 100644
> --- a/hw/timer/hpet.c
> +++ b/hw/timer/hpet.c
> @@ -283,6 +283,7 @@ static const VMStateDescription vmstate_hpet_rtc_irq_level = {
>      .name = "hpet/rtc_irq_level",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = hpet_rtc_irq_level_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT8(rtc_irq_level, HPETState),
>          VMSTATE_END_OF_LIST()
> @@ -322,13 +323,9 @@ static const VMStateDescription vmstate_hpet = {
>                                      vmstate_hpet_timer, HPETTimer),
>          VMSTATE_END_OF_LIST()
>      },
> -    .subsections = (VMStateSubsection[]) {
> -        {
> -            .vmsd = &vmstate_hpet_rtc_irq_level,
> -            .needed = hpet_rtc_irq_level_needed,
> -        }, {
> -            /* empty */
> -        }
> +    .subsections = (const VMStateDescription*[]) {
> +        &vmstate_hpet_rtc_irq_level,
> +        NULL
>      }
>  };
> 
> diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
> index f2b77fa..3204825 100644
> --- a/hw/timer/mc146818rtc.c
> +++ b/hw/timer/mc146818rtc.c
> @@ -733,22 +733,23 @@ static int rtc_post_load(void *opaque, int version_id)
>      return 0;
>  }
> 
> +static bool rtc_irq_reinject_on_ack_count_needed(void *opaque)
> +{
> +    RTCState *s = (RTCState *)opaque;
> +    return s->irq_reinject_on_ack_count != 0;
> +}
> +
>  static const VMStateDescription vmstate_rtc_irq_reinject_on_ack_count = {
>      .name = "mc146818rtc/irq_reinject_on_ack_count",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = rtc_irq_reinject_on_ack_count_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT16(irq_reinject_on_ack_count, RTCState),
>          VMSTATE_END_OF_LIST()
>      }
>  };
> 
> -static bool rtc_irq_reinject_on_ack_count_needed(void *opaque)
> -{
> -    RTCState *s = (RTCState *)opaque;
> -    return s->irq_reinject_on_ack_count != 0;
> -}
> -
>  static const VMStateDescription vmstate_rtc = {
>      .name = "mc146818rtc",
>      .version_id = 3,
> @@ -770,13 +771,9 @@ static const VMStateDescription vmstate_rtc = {
>          VMSTATE_UINT64_V(next_alarm_time, RTCState, 3),
>          VMSTATE_END_OF_LIST()
>      },
> -    .subsections = (VMStateSubsection[]) {
> -        {
> -            .vmsd = &vmstate_rtc_irq_reinject_on_ack_count,
> -            .needed = rtc_irq_reinject_on_ack_count_needed,
> -        }, {
> -            /* empty */
> -        }
> +    .subsections = (const VMStateDescription*[]) {
> +        &vmstate_rtc_irq_reinject_on_ack_count,
> +        NULL
>      }
>  };
> 
> diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
> index 1a22c9c..7d65818 100644
> --- a/hw/usb/hcd-ohci.c
> +++ b/hw/usb/hcd-ohci.c
> @@ -2034,6 +2034,7 @@ static const VMStateDescription vmstate_ohci_eof_timer = {
>      .version_id = 1,
>      .minimum_version_id = 1,
>      .pre_load = ohci_eof_timer_pre_load,
> +    .needed = ohci_eof_timer_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_TIMER_PTR(eof_timer, OHCIState),
>          VMSTATE_END_OF_LIST()
> @@ -2081,13 +2082,9 @@ static const VMStateDescription vmstate_ohci_state = {
>          VMSTATE_BOOL(async_complete, OHCIState),
>          VMSTATE_END_OF_LIST()
>      },
> -    .subsections = (VMStateSubsection []) {
> -        {
> -            .vmsd = &vmstate_ohci_eof_timer,
> -            .needed = ohci_eof_timer_needed,
> -        } , {
> -            /* empty */
> -        }
> +    .subsections = (const VMStateDescription*[]) {
> +        &vmstate_ohci_eof_timer,
> +        NULL
>      }
>  };
> 
> diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
> index 242a654..6b4218c 100644
> --- a/hw/usb/redirect.c
> +++ b/hw/usb/redirect.c
> @@ -2257,40 +2257,42 @@ static const VMStateInfo usbredir_ep_bufpq_vmstate_info = {
> 
> 
>  /* For endp_data migration */
> +static bool usbredir_bulk_receiving_needed(void *priv)
> +{
> +    struct endp_data *endp = priv;
> +
> +    return endp->bulk_receiving_started;
> +}
> +
>  static const VMStateDescription usbredir_bulk_receiving_vmstate = {
>      .name = "usb-redir-ep/bulk-receiving",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = usbredir_bulk_receiving_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT8(bulk_receiving_started, struct endp_data),
>          VMSTATE_END_OF_LIST()
>      }
>  };
> 
> -static bool usbredir_bulk_receiving_needed(void *priv)
> +static bool usbredir_stream_needed(void *priv)
>  {
>      struct endp_data *endp = priv;
> 
> -    return endp->bulk_receiving_started;
> +    return endp->max_streams;
>  }
> 
>  static const VMStateDescription usbredir_stream_vmstate = {
>      .name = "usb-redir-ep/stream-state",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = usbredir_stream_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT32(max_streams, struct endp_data),
>          VMSTATE_END_OF_LIST()
>      }
>  };
> 
> -static bool usbredir_stream_needed(void *priv)
> -{
> -    struct endp_data *endp = priv;
> -
> -    return endp->max_streams;
> -}
> -
>  static const VMStateDescription usbredir_ep_vmstate = {
>      .name = "usb-redir-ep",
>      .version_id = 1,
> @@ -2318,16 +2320,10 @@ static const VMStateDescription usbredir_ep_vmstate = {
>          VMSTATE_INT32(bufpq_target_size, struct endp_data),
>          VMSTATE_END_OF_LIST()
>      },
> -    .subsections = (VMStateSubsection[]) {
> -        {
> -            .vmsd = &usbredir_bulk_receiving_vmstate,
> -            .needed = usbredir_bulk_receiving_needed,
> -        }, {
> -            .vmsd = &usbredir_stream_vmstate,
> -            .needed = usbredir_stream_needed,
> -        }, {
> -            /* empty */
> -        }
> +    .subsections = (const VMStateDescription*[]) {
> +        &usbredir_bulk_receiving_vmstate,
> +        &usbredir_stream_vmstate,
> +        NULL
>      }
>  };
> 
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 6985e76..174a1be 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -897,6 +897,7 @@ static const VMStateDescription vmstate_virtio_device_endian = {
>      .name = "virtio/device_endian",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = &virtio_device_endian_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT8(device_endian, VirtIODevice),
>          VMSTATE_END_OF_LIST()
> @@ -911,12 +912,9 @@ static const VMStateDescription vmstate_virtio = {
>      .fields = (VMStateField[]) {
>          VMSTATE_END_OF_LIST()
>      },
> -    .subsections = (VMStateSubsection[]) {
> -        {
> -            .vmsd = &vmstate_virtio_device_endian,
> -            .needed = &virtio_device_endian_needed
> -        },
> -        { 0 }
> +    .subsections = (const VMStateDescription*[]) {
> +        &vmstate_virtio_device_endian,
> +        NULL
>      }
>  };
> 
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index bc7616a..fc5e643 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -120,11 +120,6 @@ 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 unmigratable;
> @@ -135,8 +130,9 @@ struct VMStateDescription {
>      int (*pre_load)(void *opaque);
>      int (*post_load)(void *opaque, int version_id);
>      void (*pre_save)(void *opaque);
> +    bool (*needed)(void *opaque);
>      VMStateField *fields;
> -    const VMStateSubsection *subsections;
> +    const VMStateDescription **subsections;
>  };
> 
>  extern const VMStateDescription vmstate_dummy;
> diff --git a/migration/vmstate.c b/migration/vmstate.c
> index e5388f0..108995e 100644
> --- a/migration/vmstate.c
> +++ b/migration/vmstate.c
> @@ -341,11 +341,11 @@ void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
>  }
> 
>  static const VMStateDescription *
> -    vmstate_get_subsection(const VMStateSubsection *sub, char *idstr)
> +vmstate_get_subsection(const VMStateDescription **sub, char *idstr)
>  {
> -    while (sub && sub->needed) {
> -        if (strcmp(idstr, sub->vmsd->name) == 0) {
> -            return sub->vmsd;
> +    while (sub && *sub && (*sub)->needed) {
> +        if (strcmp(idstr, (*sub)->name) == 0) {
> +            return *sub;
>          }
>          sub++;
>      }
> @@ -405,12 +405,12 @@ static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
>  static void vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
>                                      void *opaque, QJSON *vmdesc)
>  {
> -    const VMStateSubsection *sub = vmsd->subsections;
> +    const VMStateDescription **sub = vmsd->subsections;
>      bool subsection_found = false;
> 
> -    while (sub && sub->needed) {
> -        if (sub->needed(opaque)) {
> -            const VMStateDescription *vmsd = sub->vmsd;
> +    while (sub && *sub && (*sub)->needed) {
> +        if ((*sub)->needed(opaque)) {
> +            const VMStateDescription *vmsd = *sub;
>              uint8_t len;
> 
>              if (vmdesc) {
> diff --git a/savevm.c b/savevm.c
> index 1014e3e..01770cd 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -268,11 +268,11 @@ static void dump_vmstate_vmsf(FILE *out_file, const VMStateField *field,
>  }
> 
>  static void dump_vmstate_vmss(FILE *out_file,
> -                              const VMStateSubsection *subsection,
> +                              const VMStateDescription **subsection,
>                                int indent)
>  {
> -    if (subsection->vmsd != NULL) {
> -        dump_vmstate_vmsd(out_file, subsection->vmsd, indent, true);
> +    if (*subsection != NULL) {
> +        dump_vmstate_vmsd(out_file, *subsection, indent, true);
>      }
>  }
> 
> @@ -313,12 +313,12 @@ static void dump_vmstate_vmsd(FILE *out_file,
>          fprintf(out_file, "\n%*s]", indent, "");
>      }
>      if (vmsd->subsections != NULL) {
> -        const VMStateSubsection *subsection = vmsd->subsections;
> +        const VMStateDescription **subsection = vmsd->subsections;
>          bool first;
> 
>          fprintf(out_file, ",\n%*s\"Subsections\": [\n", indent, "");
>          first = true;
> -        while (subsection->vmsd != NULL) {
> +        while (*subsection != NULL) {
>              if (!first) {
>                  fprintf(out_file, ",\n");
>              }
> diff --git a/target-arm/machine.c b/target-arm/machine.c
> index 9446e5a..36365a5 100644
> --- a/target-arm/machine.c
> +++ b/target-arm/machine.c
> @@ -40,6 +40,7 @@ static const VMStateDescription vmstate_vfp = {
>      .name = "cpu/vfp",
>      .version_id = 3,
>      .minimum_version_id = 3,
> +    .needed = vfp_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_FLOAT64_ARRAY(env.vfp.regs, ARMCPU, 64),
>          /* The xregs array is a little awkward because element 1 (FPSCR)
> @@ -72,6 +73,7 @@ static const VMStateDescription vmstate_iwmmxt = {
>      .name = "cpu/iwmmxt",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = iwmmxt_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT64_ARRAY(env.iwmmxt.regs, ARMCPU, 16),
>          VMSTATE_UINT32_ARRAY(env.iwmmxt.cregs, ARMCPU, 16),
> @@ -91,6 +93,7 @@ static const VMStateDescription vmstate_m = {
>      .name = "cpu/m",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = m_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT32(env.v7m.other_sp, ARMCPU),
>          VMSTATE_UINT32(env.v7m.vecbase, ARMCPU),
> @@ -114,6 +117,7 @@ static const VMStateDescription vmstate_thumb2ee = {
>      .name = "cpu/thumb2ee",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = thumb2ee_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT32(env.teecr, ARMCPU),
>          VMSTATE_UINT32(env.teehbr, ARMCPU),
> @@ -282,21 +286,11 @@ const VMStateDescription vmstate_arm_cpu = {
>          VMSTATE_BOOL(powered_off, ARMCPU),
>          VMSTATE_END_OF_LIST()
>      },
> -    .subsections = (VMStateSubsection[]) {
> -        {
> -            .vmsd = &vmstate_vfp,
> -            .needed = vfp_needed,
> -        } , {
> -            .vmsd = &vmstate_iwmmxt,
> -            .needed = iwmmxt_needed,
> -        } , {
> -            .vmsd = &vmstate_m,
> -            .needed = m_needed,
> -        } , {
> -            .vmsd = &vmstate_thumb2ee,
> -            .needed = thumb2ee_needed,
> -        } , {
> -            /* empty */
> -        }
> +    .subsections = (const VMStateDescription*[]) {
> +        &vmstate_vfp,
> +        &vmstate_iwmmxt,
> +        &vmstate_m,
> +        &vmstate_thumb2ee,
> +        NULL
>      }
>  };
> diff --git a/target-i386/machine.c b/target-i386/machine.c
> index cd1ddd2..a89b0f8 100644
> --- a/target-i386/machine.c
> +++ b/target-i386/machine.c
> @@ -400,6 +400,7 @@ static const VMStateDescription vmstate_steal_time_msr = {
>      .name = "cpu/steal_time_msr",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = steal_time_msr_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT64(env.steal_time_msr, X86CPU),
>          VMSTATE_END_OF_LIST()
> @@ -410,6 +411,7 @@ static const VMStateDescription vmstate_async_pf_msr = {
>      .name = "cpu/async_pf_msr",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = async_pf_msr_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT64(env.async_pf_en_msr, X86CPU),
>          VMSTATE_END_OF_LIST()
> @@ -420,6 +422,7 @@ static const VMStateDescription vmstate_pv_eoi_msr = {
>      .name = "cpu/async_pv_eoi_msr",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = pv_eoi_msr_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT64(env.pv_eoi_en_msr, X86CPU),
>          VMSTATE_END_OF_LIST()
> @@ -438,6 +441,7 @@ static const VMStateDescription vmstate_fpop_ip_dp = {
>      .name = "cpu/fpop_ip_dp",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = fpop_ip_dp_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT16(env.fpop, X86CPU),
>          VMSTATE_UINT64(env.fpip, X86CPU),
> @@ -458,6 +462,7 @@ static const VMStateDescription vmstate_msr_tsc_adjust = {
>      .name = "cpu/msr_tsc_adjust",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = tsc_adjust_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT64(env.tsc_adjust, X86CPU),
>          VMSTATE_END_OF_LIST()
> @@ -476,6 +481,7 @@ static const VMStateDescription vmstate_msr_tscdeadline = {
>      .name = "cpu/msr_tscdeadline",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = tscdeadline_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT64(env.tsc_deadline, X86CPU),
>          VMSTATE_END_OF_LIST()
> @@ -502,6 +508,7 @@ static const VMStateDescription vmstate_msr_ia32_misc_enable = {
>      .name = "cpu/msr_ia32_misc_enable",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = misc_enable_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT64(env.msr_ia32_misc_enable, X86CPU),
>          VMSTATE_END_OF_LIST()
> @@ -512,6 +519,7 @@ static const VMStateDescription vmstate_msr_ia32_feature_control = {
>      .name = "cpu/msr_ia32_feature_control",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = feature_control_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT64(env.msr_ia32_feature_control, X86CPU),
>          VMSTATE_END_OF_LIST()
> @@ -546,6 +554,7 @@ static const VMStateDescription vmstate_msr_architectural_pmu = {
>      .name = "cpu/msr_architectural_pmu",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = pmu_enable_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT64(env.msr_fixed_ctr_ctrl, X86CPU),
>          VMSTATE_UINT64(env.msr_global_ctrl, X86CPU),
> @@ -581,6 +590,7 @@ static const VMStateDescription vmstate_mpx = {
>      .name = "cpu/mpx",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = mpx_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_BND_REGS(env.bnd_regs, X86CPU, 4),
>          VMSTATE_UINT64(env.bndcs_regs.cfgu, X86CPU),
> @@ -602,6 +612,7 @@ static const VMStateDescription vmstate_msr_hypercall_hypercall = {
>      .name = "cpu/msr_hyperv_hypercall",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = hyperv_hypercall_enable_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT64(env.msr_hv_guest_os_id, X86CPU),
>          VMSTATE_UINT64(env.msr_hv_hypercall, X86CPU),
> @@ -621,6 +632,7 @@ static const VMStateDescription vmstate_msr_hyperv_vapic = {
>      .name = "cpu/msr_hyperv_vapic",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = hyperv_vapic_enable_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT64(env.msr_hv_vapic, X86CPU),
>          VMSTATE_END_OF_LIST()
> @@ -639,6 +651,7 @@ static const VMStateDescription vmstate_msr_hyperv_time = {
>      .name = "cpu/msr_hyperv_time",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = hyperv_time_enable_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT64(env.msr_hv_tsc, X86CPU),
>          VMSTATE_END_OF_LIST()
> @@ -680,6 +693,7 @@ static const VMStateDescription vmstate_avx512 = {
>      .name = "cpu/avx512",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = avx512_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT64_ARRAY(env.opmask_regs, X86CPU, NB_OPMASK_REGS),
>          VMSTATE_ZMMH_REGS_VARS(env.xmm_regs, X86CPU, 0),
> @@ -702,6 +716,7 @@ static const VMStateDescription vmstate_xss = {
>      .name = "cpu/xss",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = xss_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT64(env.xss, X86CPU),
>          VMSTATE_END_OF_LIST()
> @@ -810,54 +825,22 @@ VMStateDescription vmstate_x86_cpu = {
>          VMSTATE_END_OF_LIST()
>          /* The above list is not sorted /wrt version numbers, watch out! */
>      },
> -    .subsections = (VMStateSubsection []) {
> -        {
> -            .vmsd = &vmstate_async_pf_msr,
> -            .needed = async_pf_msr_needed,
> -        } , {
> -            .vmsd = &vmstate_pv_eoi_msr,
> -            .needed = pv_eoi_msr_needed,
> -        } , {
> -            .vmsd = &vmstate_steal_time_msr,
> -            .needed = steal_time_msr_needed,
> -        } , {
> -            .vmsd = &vmstate_fpop_ip_dp,
> -            .needed = fpop_ip_dp_needed,
> -        }, {
> -            .vmsd = &vmstate_msr_tsc_adjust,
> -            .needed = tsc_adjust_needed,
> -        }, {
> -            .vmsd = &vmstate_msr_tscdeadline,
> -            .needed = tscdeadline_needed,
> -        }, {
> -            .vmsd = &vmstate_msr_ia32_misc_enable,
> -            .needed = misc_enable_needed,
> -        }, {
> -            .vmsd = &vmstate_msr_ia32_feature_control,
> -            .needed = feature_control_needed,
> -        }, {
> -            .vmsd = &vmstate_msr_architectural_pmu,
> -            .needed = pmu_enable_needed,
> -        } , {
> -            .vmsd = &vmstate_mpx,
> -            .needed = mpx_needed,
> -        }, {
> -            .vmsd = &vmstate_msr_hypercall_hypercall,
> -            .needed = hyperv_hypercall_enable_needed,
> -        }, {
> -            .vmsd = &vmstate_msr_hyperv_vapic,
> -            .needed = hyperv_vapic_enable_needed,
> -        }, {
> -            .vmsd = &vmstate_msr_hyperv_time,
> -            .needed = hyperv_time_enable_needed,
> -        }, {
> -            .vmsd = &vmstate_avx512,
> -            .needed = avx512_needed,
> -         }, {
> -            .vmsd = &vmstate_xss,
> -            .needed = xss_needed,
> -        } , {
> -            /* empty */
> -        }
> +    .subsections = (const VMStateDescription*[]) {
> +        &vmstate_async_pf_msr,
> +        &vmstate_pv_eoi_msr,
> +        &vmstate_steal_time_msr,
> +        &vmstate_fpop_ip_dp,
> +        &vmstate_msr_tsc_adjust,
> +        &vmstate_msr_tscdeadline,
> +        &vmstate_msr_ia32_misc_enable,
> +        &vmstate_msr_ia32_feature_control,
> +        &vmstate_msr_architectural_pmu,
> +        &vmstate_mpx,
> +        &vmstate_msr_hypercall_hypercall,
> +        &vmstate_msr_hyperv_vapic,
> +        &vmstate_msr_hyperv_time,
> +        &vmstate_avx512,
> +        &vmstate_xss,
> +        NULL
>      }
>  };
> diff --git a/target-ppc/machine.c b/target-ppc/machine.c
> index d875211..f4ac761 100644
> --- a/target-ppc/machine.c
> +++ b/target-ppc/machine.c
> @@ -213,6 +213,7 @@ static const VMStateDescription vmstate_fpu = {
>      .name = "cpu/fpu",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = fpu_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_FLOAT64_ARRAY(env.fpr, PowerPCCPU, 32),
>          VMSTATE_UINTTL(env.fpscr, PowerPCCPU),
> @@ -231,6 +232,7 @@ static const VMStateDescription vmstate_altivec = {
>      .name = "cpu/altivec",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = altivec_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_AVR_ARRAY(env.avr, PowerPCCPU, 32),
>          VMSTATE_UINT32(env.vscr, PowerPCCPU),
> @@ -249,6 +251,7 @@ static const VMStateDescription vmstate_vsx = {
>      .name = "cpu/vsx",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = vsx_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT64_ARRAY(env.vsr, PowerPCCPU, 32),
>          VMSTATE_END_OF_LIST()
> @@ -269,6 +272,7 @@ static const VMStateDescription vmstate_tm = {
>      .version_id = 1,
>      .minimum_version_id = 1,
>      .minimum_version_id_old = 1,
> +    .needed = tm_needed,
>      .fields      = (VMStateField []) {
>          VMSTATE_UINTTL_ARRAY(env.tm_gpr, PowerPCCPU, 32),
>          VMSTATE_AVR_ARRAY(env.tm_vsr, PowerPCCPU, 64),
> @@ -302,6 +306,7 @@ static const VMStateDescription vmstate_sr = {
>      .name = "cpu/sr",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = sr_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINTTL_ARRAY(env.sr, PowerPCCPU, 32),
>          VMSTATE_END_OF_LIST()
> @@ -351,6 +356,7 @@ static const VMStateDescription vmstate_slb = {
>      .name = "cpu/slb",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = slb_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_INT32_EQUAL(env.slb_nr, PowerPCCPU),
>          VMSTATE_SLB_ARRAY(env.slb, PowerPCCPU, MAX_SLB_ENTRIES),
> @@ -383,6 +389,7 @@ static const VMStateDescription vmstate_tlb6xx = {
>      .name = "cpu/tlb6xx",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = tlb6xx_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_INT32_EQUAL(env.nb_tlb, PowerPCCPU),
>          VMSTATE_STRUCT_VARRAY_POINTER_INT32(env.tlb.tlb6, PowerPCCPU,
> @@ -429,6 +436,7 @@ static const VMStateDescription vmstate_pbr403 = {
>      .name = "cpu/pbr403",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = pbr403_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINTTL_ARRAY(env.pb, PowerPCCPU, 4),
>          VMSTATE_END_OF_LIST()
> @@ -439,6 +447,7 @@ static const VMStateDescription vmstate_tlbemb = {
>      .name = "cpu/tlb6xx",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = tlbemb_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_INT32_EQUAL(env.nb_tlb, PowerPCCPU),
>          VMSTATE_STRUCT_VARRAY_POINTER_INT32(env.tlb.tlbe, PowerPCCPU,
> @@ -448,13 +457,9 @@ static const VMStateDescription vmstate_tlbemb = {
>          /* 403 protection registers */
>          VMSTATE_END_OF_LIST()
>      },
> -    .subsections = (VMStateSubsection []) {
> -        {
> -            .vmsd = &vmstate_pbr403,
> -            .needed = pbr403_needed,
> -        } , {
> -            /* empty */
> -        }
> +    .subsections = (const VMStateDescription*[]) {
> +        &vmstate_pbr403,
> +        NULL
>      }
>  };
> 
> @@ -483,6 +488,7 @@ static const VMStateDescription vmstate_tlbmas = {
>      .name = "cpu/tlbmas",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = tlbmas_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_INT32_EQUAL(env.nb_tlb, PowerPCCPU),
>          VMSTATE_STRUCT_VARRAY_POINTER_INT32(env.tlb.tlbm, PowerPCCPU,
> @@ -533,38 +539,18 @@ const VMStateDescription vmstate_ppc_cpu = {
>          VMSTATE_UINT32_EQUAL(env.nb_BATs, PowerPCCPU),
>          VMSTATE_END_OF_LIST()
>      },
> -    .subsections = (VMStateSubsection []) {
> -        {
> -            .vmsd = &vmstate_fpu,
> -            .needed = fpu_needed,
> -        } , {
> -            .vmsd = &vmstate_altivec,
> -            .needed = altivec_needed,
> -        } , {
> -            .vmsd = &vmstate_vsx,
> -            .needed = vsx_needed,
> -        } , {
> -            .vmsd = &vmstate_sr,
> -            .needed = sr_needed,
> -        } , {
> +    .subsections = (const VMStateDescription*[]) {
> +        &vmstate_fpu,
> +        &vmstate_altivec,
> +        &vmstate_vsx,
> +        &vmstate_sr,
>  #ifdef TARGET_PPC64
> -            .vmsd = &vmstate_tm,
> -            .needed = tm_needed,
> -        } , {
> -            .vmsd = &vmstate_slb,
> -            .needed = slb_needed,
> -        } , {
> +        &vmstate_tm,
> +        &vmstate_slb,
>  #endif /* TARGET_PPC64 */
> -            .vmsd = &vmstate_tlb6xx,
> -            .needed = tlb6xx_needed,
> -        } , {
> -            .vmsd = &vmstate_tlbemb,
> -            .needed = tlbemb_needed,
> -        } , {
> -            .vmsd = &vmstate_tlbmas,
> -            .needed = tlbmas_needed,
> -        } , {
> -            /* empty */
> -        }
> +        &vmstate_tlb6xx,
> +        &vmstate_tlbemb,
> +        &vmstate_tlbmas,
> +        NULL
>      }
>  };
> diff --git a/target-s390x/machine.c b/target-s390x/machine.c
> index 7853e3c..23d3fa9 100644
> --- a/target-s390x/machine.c
> +++ b/target-s390x/machine.c
> @@ -42,10 +42,16 @@ static void cpu_pre_save(void *opaque)
>      }
>  }
> 
> +static inline bool fpu_needed(void *opaque)
> +{
> +    return true;
> +}
> +
>  const VMStateDescription vmstate_fpu = {
>      .name = "cpu/fpu",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .needed = fpu_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT64(env.fregs[0].ll, S390CPU),
>          VMSTATE_UINT64(env.fregs[1].ll, S390CPU),
> @@ -68,11 +74,6 @@ const VMStateDescription vmstate_fpu = {
>      }
>  };
> 
> -static inline bool fpu_needed(void *opaque)
> -{
> -    return true;
> -}
> -
>  const VMStateDescription vmstate_s390_cpu = {
>      .name = "cpu",
>      .post_load = cpu_post_load,
> @@ -101,12 +102,8 @@ const VMStateDescription vmstate_s390_cpu = {
>                                 irqstate_saved_size),
>          VMSTATE_END_OF_LIST()
>       },
> -    .subsections = (VMStateSubsection[]) {
> -        {
> -            .vmsd = &vmstate_fpu,
> -            .needed = fpu_needed,
> -        } , {
> -            /* empty */
> -        }
> +    .subsections = (const VMStateDescription*[]) {
> +        &vmstate_fpu,
> +        NULL
>      },
>  };
> -- 
> 2.4.0
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

[Qemu-devel] [PATCH 3/9] runstate: Add runstate store

[Juan Quintela]

This allows us to store the current state to send it through migration.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 include/sysemu/sysemu.h |  1 +
 vl.c                    | 11 +++++++++++
 2 files changed, 12 insertions(+)

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 8a52934..c1a403e 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -28,6 +28,7 @@ bool runstate_check(RunState state);
 void runstate_set(RunState new_state);
 int runstate_is_running(void);
 bool runstate_needs_reset(void);
+int runstate_store(char *str, int size);
 typedef struct vm_change_state_entry VMChangeStateEntry;
 typedef void VMChangeStateHandler(void *opaque, int running, RunState state);

diff --git a/vl.c b/vl.c
index 15bccc4..7dca13f 100644
--- a/vl.c
+++ b/vl.c
@@ -609,6 +609,17 @@ bool runstate_check(RunState state)
     return current_run_state == state;
 }

+int runstate_store(char *str, int size)
+{
+    const char *state = RunState_lookup[current_run_state];
+
+    if (strlen(state)+1 > size) {
+        return -1;
+    }
+    strncpy(str, state, strlen(state)+1);
+    return 0;
+}
+
 static void runstate_init(void)
 {
     const RunStateTransition *p;
-- 
2.4.0

Re: [Qemu-devel] [PATCH 3/9] runstate: Add runstate store

[Dr. David Alan Gilbert]

* Juan Quintela (quintela@redhat.com) wrote:
> This allows us to store the current state to send it through migration.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  include/sysemu/sysemu.h |  1 +
>  vl.c                    | 11 +++++++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 8a52934..c1a403e 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -28,6 +28,7 @@ bool runstate_check(RunState state);
>  void runstate_set(RunState new_state);
>  int runstate_is_running(void);
>  bool runstate_needs_reset(void);
> +int runstate_store(char *str, int size);
>  typedef struct vm_change_state_entry VMChangeStateEntry;
>  typedef void VMChangeStateHandler(void *opaque, int running, RunState state);
> 
> diff --git a/vl.c b/vl.c
> index 15bccc4..7dca13f 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -609,6 +609,17 @@ bool runstate_check(RunState state)
>      return current_run_state == state;
>  }
> 
> +int runstate_store(char *str, int size)
> +{

size_t size

and perhaps a bool for the return type?

> +    const char *state = RunState_lookup[current_run_state];
> +
> +    if (strlen(state)+1 > size) {
> +        return -1;
> +    }
> +    strncpy(str, state, strlen(state)+1);

Why not a plain strcpy?

(I'd be very tempted by an assert if it's too long; I mean it really
shouldn't happen; although I don't like asserts in the migrate path).

Dave


> +    return 0;
> +}
> +
>  static void runstate_init(void)
>  {
>      const RunStateTransition *p;
> -- 
> 2.4.0
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 3/9] runstate: Add runstate store

[Juan Quintela]

"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
>> This allows us to store the current state to send it through migration.
>> 
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  include/sysemu/sysemu.h |  1 +
>>  vl.c                    | 11 +++++++++++
>>  2 files changed, 12 insertions(+)
>> 
>> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
>> index 8a52934..c1a403e 100644
>> --- a/include/sysemu/sysemu.h
>> +++ b/include/sysemu/sysemu.h
>> @@ -28,6 +28,7 @@ bool runstate_check(RunState state);
>>  void runstate_set(RunState new_state);
>>  int runstate_is_running(void);
>>  bool runstate_needs_reset(void);
>> +int runstate_store(char *str, int size);
>>  typedef struct vm_change_state_entry VMChangeStateEntry;
>>  typedef void VMChangeStateHandler(void *opaque, int running, RunState state);
>> 
>> diff --git a/vl.c b/vl.c
>> index 15bccc4..7dca13f 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -609,6 +609,17 @@ bool runstate_check(RunState state)
>>      return current_run_state == state;
>>  }
>> 
>> +int runstate_store(char *str, int size)
>> +{
>
> size_t size
>
> and perhaps a bool for the return type?

ack

>
>> +    const char *state = RunState_lookup[current_run_state];
>> +
>> +    if (strlen(state)+1 > size) {
>> +        return -1;
>> +    }
>> +    strncpy(str, state, strlen(state)+1);
>
> Why not a plain strcpy?

asked for memcpy, I don't really cared one way or another, it is a
really small string.

Thanks, Juan.

Re: [Qemu-devel] [PATCH 3/9] runstate: Add runstate store

[Denis V. Lunev]

On 14/05/15 19:28, Juan Quintela wrote:
> This allows us to store the current state to send it through migration.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>   include/sysemu/sysemu.h |  1 +
>   vl.c                    | 11 +++++++++++
>   2 files changed, 12 insertions(+)
>
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 8a52934..c1a403e 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -28,6 +28,7 @@ bool runstate_check(RunState state);
>   void runstate_set(RunState new_state);
>   int runstate_is_running(void);
>   bool runstate_needs_reset(void);
> +int runstate_store(char *str, int size);
>   typedef struct vm_change_state_entry VMChangeStateEntry;
>   typedef void VMChangeStateHandler(void *opaque, int running, RunState state);
>
> diff --git a/vl.c b/vl.c
> index 15bccc4..7dca13f 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -609,6 +609,17 @@ bool runstate_check(RunState state)
>       return current_run_state == state;
>   }
>
> +int runstate_store(char *str, int size)
> +{
> +    const char *state = RunState_lookup[current_run_state];
> +
> +    if (strlen(state)+1 > size) {
> +        return -1;
> +    }
> +    strncpy(str, state, strlen(state)+1);
> +    return 0;
> +}
> +
>   static void runstate_init(void)
>   {
>       const RunStateTransition *p;
>

minor. why to call strlen() twice?

if you have the length in hand it would be better to call
memcpy to copy data.

Re: [Qemu-devel] [PATCH 3/9] runstate: Add runstate store

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 672 bytes --]

On 05/18/2015 04:38 AM, Denis V. Lunev wrote:
> On 14/05/15 19:28, Juan Quintela wrote:
>> This allows us to store the current state to send it through migration.
>>
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---

>> +    if (strlen(state)+1 > size) {
>> +        return -1;
>> +    }
>> +    strncpy(str, state, strlen(state)+1);
>> +    return 0;
>> +}
>> +
>>   static void runstate_init(void)
>>   {
>>       const RunStateTransition *p;
>>
> 
> minor. why to call strlen() twice?

Also, why no spaces around binary '+'?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 3/9] runstate: Add runstate store

[Juan Quintela]

"Denis V. Lunev" <den-lists@parallels.com> wrote:
> On 14/05/15 19:28, Juan Quintela wrote:
>> This allows us to store the current state to send it through migration.
>>
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>   include/sysemu/sysemu.h |  1 +
>>   vl.c                    | 11 +++++++++++
>>   2 files changed, 12 insertions(+)
>>
>> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
>> index 8a52934..c1a403e 100644
>> --- a/include/sysemu/sysemu.h
>> +++ b/include/sysemu/sysemu.h
>> @@ -28,6 +28,7 @@ bool runstate_check(RunState state);
>>   void runstate_set(RunState new_state);
>>   int runstate_is_running(void);
>>   bool runstate_needs_reset(void);
>> +int runstate_store(char *str, int size);
>>   typedef struct vm_change_state_entry VMChangeStateEntry;
>>   typedef void VMChangeStateHandler(void *opaque, int running, RunState state);
>>
>> diff --git a/vl.c b/vl.c
>> index 15bccc4..7dca13f 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -609,6 +609,17 @@ bool runstate_check(RunState state)
>>       return current_run_state == state;
>>   }
>>
>> +int runstate_store(char *str, int size)
>> +{
>> +    const char *state = RunState_lookup[current_run_state];
>> +
>> +    if (strlen(state)+1 > size) {
>> +        return -1;
>> +    }
>> +    strncpy(str, state, strlen(state)+1);
>> +    return 0;
>> +}
>> +
>>   static void runstate_init(void)
>>   {
>>       const RunStateTransition *p;
>>
>
> minor. why to call strlen() twice?
>
> if you have the length in hand it would be better to call
> memcpy to copy data.

Done, thanks.

[Qemu-devel] [PATCH 4/9] runstate: create runstate_index function

[Juan Quintela]

Given a string state, we need a way to get the RunState for that string.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 include/sysemu/sysemu.h |  1 +
 vl.c                    | 13 +++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index c1a403e..e5fff07 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -29,6 +29,7 @@ void runstate_set(RunState new_state);
 int runstate_is_running(void);
 bool runstate_needs_reset(void);
 int runstate_store(char *str, int size);
+RunState runstate_index(char *str);
 typedef struct vm_change_state_entry VMChangeStateEntry;
 typedef void VMChangeStateHandler(void *opaque, int running, RunState state);

diff --git a/vl.c b/vl.c
index 7dca13f..ea4319d 100644
--- a/vl.c
+++ b/vl.c
@@ -620,6 +620,19 @@ int runstate_store(char *str, int size)
     return 0;
 }

+RunState runstate_index(char *str)
+{
+    RunState i = 0;
+
+    while (i != RUN_STATE_MAX) {
+        if (strcmp(str, RunState_lookup[i]) == 0) {
+            return i;
+        }
+        i++;
+    }
+    return -1;
+}
+
 static void runstate_init(void)
 {
     const RunStateTransition *p;
-- 
2.4.0

Re: [Qemu-devel] [PATCH 4/9] runstate: create runstate_index function

[Dr. David Alan Gilbert]

* Juan Quintela (quintela@redhat.com) wrote:
> Given a string state, we need a way to get the RunState for that string.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  include/sysemu/sysemu.h |  1 +
>  vl.c                    | 13 +++++++++++++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index c1a403e..e5fff07 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -29,6 +29,7 @@ void runstate_set(RunState new_state);
>  int runstate_is_running(void);
>  bool runstate_needs_reset(void);
>  int runstate_store(char *str, int size);
> +RunState runstate_index(char *str);
>  typedef struct vm_change_state_entry VMChangeStateEntry;
>  typedef void VMChangeStateHandler(void *opaque, int running, RunState state);
> 
> diff --git a/vl.c b/vl.c
> index 7dca13f..ea4319d 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -620,6 +620,19 @@ int runstate_store(char *str, int size)
>      return 0;
>  }
> 
> +RunState runstate_index(char *str)
> +{
> +    RunState i = 0;
> +
> +    while (i != RUN_STATE_MAX) {
> +        if (strcmp(str, RunState_lookup[i]) == 0) {
> +            return i;
> +        }
> +        i++;
> +    }
> +    return -1;

It doesn't seem right to return -1 for the value of an enum
(which is otherwise used as an index into an array of strings).

Make it return a bool and pass the RunState* as a parameter ?

Dave

> +}
> +
>  static void runstate_init(void)
>  {
>      const RunStateTransition *p;
> -- 
> 2.4.0
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 4/9] runstate: create runstate_index function

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1042 bytes --]

On 05/18/2015 03:58 AM, Dr. David Alan Gilbert wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
>> Given a string state, we need a way to get the RunState for that string.
>>
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  include/sysemu/sysemu.h |  1 +
>>  vl.c                    | 13 +++++++++++++
>>  2 files changed, 14 insertions(+)
>>

>>
>> +RunState runstate_index(char *str)
>> +{
>> +    RunState i = 0;
>> +
>> +    while (i != RUN_STATE_MAX) {
>> +        if (strcmp(str, RunState_lookup[i]) == 0) {
>> +            return i;
>> +        }
>> +        i++;
>> +    }
>> +    return -1;
> 
> It doesn't seem right to return -1 for the value of an enum
> (which is otherwise used as an index into an array of strings).
> 
> Make it return a bool and pass the RunState* as a parameter ?

Why open-code this, when we already have qapi_enum_parse() in
qapi/qapi-util.c?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 4/9] runstate: create runstate_index function

[Juan Quintela]

Eric Blake <eblake@redhat.com> wrote:
D> On 05/18/2015 03:58 AM, Dr. David Alan Gilbert wrote:
>> * Juan Quintela (quintela@redhat.com) wrote:
>>> Given a string state, we need a way to get the RunState for that string.
>>>
>>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>>> ---
>>>  include/sysemu/sysemu.h |  1 +
>>>  vl.c                    | 13 +++++++++++++
>>>  2 files changed, 14 insertions(+)
>>>
>
>>>
>>> +RunState runstate_index(char *str)
>>> +{
>>> +    RunState i = 0;
>>> +
>>> +    while (i != RUN_STATE_MAX) {
>>> +        if (strcmp(str, RunState_lookup[i]) == 0) {
>>> +            return i;
>>> +        }
>>> +        i++;
>>> +    }
>>> +    return -1;
>> 
>> It doesn't seem right to return -1 for the value of an enum
>> (which is otherwise used as an index into an array of strings).
>> 
>> Make it return a bool and pass the RunState* as a parameter ?
>
> Why open-code this, when we already have qapi_enum_parse() in
> qapi/qapi-util.c?

Didn't knew about this function.

Thanks, Juan.

Re: [Qemu-devel] [PATCH 4/9] runstate: create runstate_index function

[Juan Quintela]

"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
>> Given a string state, we need a way to get the RunState for that string.
>> 
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  include/sysemu/sysemu.h |  1 +
>>  vl.c                    | 13 +++++++++++++
>>  2 files changed, 14 insertions(+)
>> 
>> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
>> index c1a403e..e5fff07 100644
>> --- a/include/sysemu/sysemu.h
>> +++ b/include/sysemu/sysemu.h
>> @@ -29,6 +29,7 @@ void runstate_set(RunState new_state);
>>  int runstate_is_running(void);
>>  bool runstate_needs_reset(void);
>>  int runstate_store(char *str, int size);
>> +RunState runstate_index(char *str);
>>  typedef struct vm_change_state_entry VMChangeStateEntry;
>>  typedef void VMChangeStateHandler(void *opaque, int running, RunState state);
>> 
>> diff --git a/vl.c b/vl.c
>> index 7dca13f..ea4319d 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -620,6 +620,19 @@ int runstate_store(char *str, int size)
>>      return 0;
>>  }
>> 
>> +RunState runstate_index(char *str)
>> +{
>> +    RunState i = 0;
>> +
>> +    while (i != RUN_STATE_MAX) {
>> +        if (strcmp(str, RunState_lookup[i]) == 0) {
>> +            return i;
>> +        }
>> +        i++;
>> +    }
>> +    return -1;
>
> It doesn't seem right to return -1 for the value of an enum
> (which is otherwise used as an index into an array of strings).
>
> Make it return a bool and pass the RunState* as a parameter ?
>
> Dave
>
>> +}
>> +
>>  static void runstate_init(void)
>>  {
>>      const RunStateTransition *p;
>> -- 
>> 2.4.0
>> 
>> 
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Dropped, see Eric suggestion.

Thanks, Juan.

[Qemu-devel] [PATCH 5/9] runstate: migration allows more transitions now

[Juan Quintela]

Next commit would allow to move from incoming migration to error happening on source.

Should we add more states to this transition?  Luiz?

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 vl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/vl.c b/vl.c
index ea4319d..b8844f6 100644
--- a/vl.c
+++ b/vl.c
@@ -550,6 +550,8 @@ static const RunStateTransition runstate_transitions_def[] = {

     { RUN_STATE_INMIGRATE, RUN_STATE_RUNNING },
     { RUN_STATE_INMIGRATE, RUN_STATE_PAUSED },
+    { RUN_STATE_INMIGRATE, RUN_STATE_INTERNAL_ERROR },
+    { RUN_STATE_INMIGRATE, RUN_STATE_IO_ERROR },

     { RUN_STATE_INTERNAL_ERROR, RUN_STATE_PAUSED },
     { RUN_STATE_INTERNAL_ERROR, RUN_STATE_FINISH_MIGRATE },
-- 
2.4.0

[Qemu-devel] [PATCH 6/9] migration: create new section to store global state

[Juan Quintela]

This includes a new section that for now just stores the current qemu state.

Right now, there are only one way to control what is the state of the
target after migration.

- If you run the target qemu with -S, it would start stopped.
- If you run the target qemu without -S, it would run just after migration finishes.

The problem here is what happens if we start the target without -S and
there happens one error during migration that puts current state as
-EIO.  Migration would ends (notice that the error happend doing block
IO, network IO, i.e. nothing related with migration), and when
migration finish, we would just "continue" running on destination,
probably hanging the guest/corruption data, whatever.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 include/migration/migration.h |  4 ++
 migration/migration.c         | 85 +++++++++++++++++++++++++++++++++++++++++--
 vl.c                          |  1 +
 3 files changed, 87 insertions(+), 3 deletions(-)

diff --git a/include/migration/migration.h b/include/migration/migration.h
index a6e025a..785c2dc 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -180,4 +180,8 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset,
                              ram_addr_t offset, size_t size,
                              uint64_t *bytes_sent);

+void register_global_state(void);
+void global_state_store(void);
+char *global_state_get_runstate(void);
+
 #endif
diff --git a/migration/migration.c b/migration/migration.c
index 732d229..ab69f81 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -133,10 +133,20 @@ static void process_incoming_migration_co(void *opaque)
         exit(EXIT_FAILURE);
     }

-    if (autostart) {
+    /* runstate == "" means that we haven't received it through the
+     * wire, so we obey autostart.  runstate == runing means that we
+     * need to run it, we need to make sure that we do it after
+     * everything else has finished.  Every other state change is done
+     * at the post_load function */
+
+    if (strcmp(global_state_get_runstate(), "running") == 0) {
         vm_start();
-    } else {
-        runstate_set(RUN_STATE_PAUSED);
+    } else if (strcmp(global_state_get_runstate(), "") == 0) {
+        if (autostart) {
+            vm_start();
+        } else {
+            runstate_set(RUN_STATE_PAUSED);
+        }
     }
     migrate_decompress_threads_join();
 }
@@ -760,6 +770,7 @@ static void *migration_thread(void *opaque)
                 qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
                 old_vm_running = runstate_is_running();

+                global_state_store();
                 ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
                 if (ret >= 0) {
                     qemu_file_set_rate_limit(s->file, INT64_MAX);
@@ -855,3 +866,71 @@ void migrate_fd_connect(MigrationState *s)
     qemu_thread_create(&s->thread, "migration", migration_thread, s,
                        QEMU_THREAD_JOINABLE);
 }
+
+typedef struct {
+    int32_t size;
+    uint8_t runstate[100];
+} GlobalState;
+
+static GlobalState global_state;
+
+void global_state_store(void)
+{
+    if (runstate_store((char *)global_state.runstate,
+                       sizeof(global_state.runstate)) == -1) {
+        error_report("Runstate is too big: %s\n", global_state.runstate);
+        exit(-1);
+    }
+}
+
+char *global_state_get_runstate(void)
+{
+    return (char *)global_state.runstate;
+}
+
+static int global_state_post_load(void *opaque, int version_id)
+{
+    GlobalState *s = opaque;
+    int ret = 0;
+    char *runstate = (char *)s->runstate;
+
+    if (strcmp(runstate, "running") != 0) {
+
+        RunState r = runstate_index(runstate);
+
+        if (r == -1) {
+            error_report("Unknown received state %s\n", runstate);
+            return -1;
+        }
+        ret = vm_stop_force_state(r);
+    }
+
+   return ret;
+}
+
+static void global_state_pre_save(void *opaque)
+{
+    GlobalState *s = opaque;
+
+    s->size = strlen((char *)s->runstate) + 1;
+}
+
+static const VMStateDescription vmstate_globalstate = {
+    .name = "globalstate",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .post_load = global_state_post_load,
+    .pre_save = global_state_pre_save,
+    .fields = (VMStateField[]) {
+        VMSTATE_INT32(size, GlobalState),
+        VMSTATE_BUFFER(runstate, GlobalState),
+        VMSTATE_END_OF_LIST()
+    },
+};
+
+void register_global_state(void)
+{
+    /* We would use it independently that we receive it */
+    strcpy((char *)&global_state.runstate, "");
+    vmstate_register(NULL, 0, &vmstate_globalstate, &global_state);
+}
diff --git a/vl.c b/vl.c
index b8844f6..8af6c79 100644
--- a/vl.c
+++ b/vl.c
@@ -4387,6 +4387,7 @@ int main(int argc, char **argv, char **envp)
         return 0;
     }

+    register_global_state();
     if (incoming) {
         Error *local_err = NULL;
         qemu_start_incoming_migration(incoming, &local_err);
-- 
2.4.0

Re: [Qemu-devel] [PATCH 6/9] migration: create new section to store global state

[Dr. David Alan Gilbert]

* Juan Quintela (quintela@redhat.com) wrote:
> This includes a new section that for now just stores the current qemu state.
> 
> Right now, there are only one way to control what is the state of the
> target after migration.
> 
> - If you run the target qemu with -S, it would start stopped.
> - If you run the target qemu without -S, it would run just after migration finishes.
> 
> The problem here is what happens if we start the target without -S and
> there happens one error during migration that puts current state as
> -EIO.  Migration would ends (notice that the error happend doing block
> IO, network IO, i.e. nothing related with migration), and when
> migration finish, we would just "continue" running on destination,
> probably hanging the guest/corruption data, whatever.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  include/migration/migration.h |  4 ++
>  migration/migration.c         | 85 +++++++++++++++++++++++++++++++++++++++++--
>  vl.c                          |  1 +
>  3 files changed, 87 insertions(+), 3 deletions(-)
> 
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index a6e025a..785c2dc 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -180,4 +180,8 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset,
>                               ram_addr_t offset, size_t size,
>                               uint64_t *bytes_sent);
> 
> +void register_global_state(void);
> +void global_state_store(void);
> +char *global_state_get_runstate(void);
> +
>  #endif
> diff --git a/migration/migration.c b/migration/migration.c
> index 732d229..ab69f81 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -133,10 +133,20 @@ static void process_incoming_migration_co(void *opaque)
>          exit(EXIT_FAILURE);
>      }
> 
> -    if (autostart) {
> +    /* runstate == "" means that we haven't received it through the
> +     * wire, so we obey autostart.  runstate == runing means that we
> +     * need to run it, we need to make sure that we do it after
> +     * everything else has finished.  Every other state change is done
> +     * at the post_load function */
> +
> +    if (strcmp(global_state_get_runstate(), "running") == 0) {
>          vm_start();
> -    } else {
> -        runstate_set(RUN_STATE_PAUSED);
> +    } else if (strcmp(global_state_get_runstate(), "") == 0) {
> +        if (autostart) {
> +            vm_start();
> +        } else {
> +            runstate_set(RUN_STATE_PAUSED);
> +        }
>      }
>      migrate_decompress_threads_join();
>  }
> @@ -760,6 +770,7 @@ static void *migration_thread(void *opaque)
>                  qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
>                  old_vm_running = runstate_is_running();
> 
> +                global_state_store();
>                  ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
>                  if (ret >= 0) {
>                      qemu_file_set_rate_limit(s->file, INT64_MAX);
> @@ -855,3 +866,71 @@ void migrate_fd_connect(MigrationState *s)
>      qemu_thread_create(&s->thread, "migration", migration_thread, s,
>                         QEMU_THREAD_JOINABLE);
>  }
> +
> +typedef struct {
> +    int32_t size;
> +    uint8_t runstate[100];
> +} GlobalState;

unsigned for size?  Could use something smaller than a int32_t
I guess for a 100 byte buffer.

> +
> +static GlobalState global_state;
> +
> +void global_state_store(void)
> +{
> +    if (runstate_store((char *)global_state.runstate,
> +                       sizeof(global_state.runstate)) == -1) {
> +        error_report("Runstate is too big: %s\n", global_state.runstate);

error_report's don't need the \n

> +        exit(-1);
> +    }
> +}
> +
> +char *global_state_get_runstate(void)
> +{
> +    return (char *)global_state.runstate;
> +}

Is that needed outside of this file - if not then just make
it static or use the data directly?

> +
> +static int global_state_post_load(void *opaque, int version_id)
> +{
> +    GlobalState *s = opaque;
> +    int ret = 0;
> +    char *runstate = (char *)s->runstate;
> +
> +    if (strcmp(runstate, "running") != 0) {
> +
> +        RunState r = runstate_index(runstate);
> +
> +        if (r == -1) {
> +            error_report("Unknown received state %s\n", runstate);

error_report's don't need the \n

> +            return -1;
> +        }
> +        ret = vm_stop_force_state(r);

Why do this here, rather than in process_incoming_migration_co;
that would seem neater to keep the state change in one place.

Dave

> +    }
> +
> +   return ret;
> +}
> +
> +static void global_state_pre_save(void *opaque)
> +{
> +    GlobalState *s = opaque;
> +
> +    s->size = strlen((char *)s->runstate) + 1;
> +}
> +
> +static const VMStateDescription vmstate_globalstate = {
> +    .name = "globalstate",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .post_load = global_state_post_load,
> +    .pre_save = global_state_pre_save,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_INT32(size, GlobalState),
> +        VMSTATE_BUFFER(runstate, GlobalState),
> +        VMSTATE_END_OF_LIST()
> +    },
> +};
> +
> +void register_global_state(void)
> +{
> +    /* We would use it independently that we receive it */
> +    strcpy((char *)&global_state.runstate, "");
> +    vmstate_register(NULL, 0, &vmstate_globalstate, &global_state);
> +}
> diff --git a/vl.c b/vl.c
> index b8844f6..8af6c79 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4387,6 +4387,7 @@ int main(int argc, char **argv, char **envp)
>          return 0;
>      }
> 
> +    register_global_state();
>      if (incoming) {
>          Error *local_err = NULL;
>          qemu_start_incoming_migration(incoming, &local_err);
> -- 
> 2.4.0
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 6/9] migration: create new section to store global state

[Juan Quintela]

"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
>> This includes a new section that for now just stores the current qemu state.
>> 
>> Right now, there are only one way to control what is the state of the
>> target after migration.
>> 
>> - If you run the target qemu with -S, it would start stopped.
>> - If you run the target qemu without -S, it would run just after migration finishes.
>> 
>> The problem here is what happens if we start the target without -S and
>> there happens one error during migration that puts current state as
>> -EIO.  Migration would ends (notice that the error happend doing block
>> IO, network IO, i.e. nothing related with migration), and when
>> migration finish, we would just "continue" running on destination,
>> probably hanging the guest/corruption data, whatever.
>> 
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> +
>> +typedef struct {
>> +    int32_t size;
>> +    uint8_t runstate[100];
>> +} GlobalState;
>
> unsigned for size?  Could use something smaller than a int32_t
> I guess for a 100 byte buffer.

uint8_t

>
>> +
>> +static GlobalState global_state;
>> +
>> +void global_state_store(void)
>> +{
>> +    if (runstate_store((char *)global_state.runstate,
>> +                       sizeof(global_state.runstate)) == -1) {
>> +        error_report("Runstate is too big: %s\n", global_state.runstate);
>
> error_report's don't need the \n

done.

>
>> +        exit(-1);
>> +    }
>> +}
>> +
>> +char *global_state_get_runstate(void)
>> +{
>> +    return (char *)global_state.runstate;
>> +}
>
> Is that needed outside of this file - if not then just make
> it static or use the data directly?
>

State was born on vl.c, then I noticed that I could put it here, and
forgot to remove globals.  Don.


>> +
>> +static int global_state_post_load(void *opaque, int version_id)
>> +{
>> +    GlobalState *s = opaque;
>> +    int ret = 0;
>> +    char *runstate = (char *)s->runstate;
>> +
>> +    if (strcmp(runstate, "running") != 0) {
>> +
>> +        RunState r = runstate_index(runstate);
>> +
>> +        if (r == -1) {
>> +            error_report("Unknown received state %s\n", runstate);
>
> error_report's don't need the \n
>
>> +            return -1;
>> +        }
>> +        ret = vm_stop_force_state(r);
>
> Why do this here, rather than in process_incoming_migration_co;
> that would seem neater to keep the state change in one place.

If state is unknown for destination here, migration can still be aborted
at this point, later that is not possible.  Not possible in the sense
that source would not noticed that destination has exited because it
didn't understood something.

But we could move that to incoming, just not clear that it is going to
be easier, because then we have to do error control at a point that
don't have it right now.


Later, Juan.

>
> Dave
>
>> +    }
>> +
>> +   return ret;
>> +}
>> +
>> +static void global_state_pre_save(void *opaque)
>> +{
>> +    GlobalState *s = opaque;
>> +
>> +    s->size = strlen((char *)s->runstate) + 1;
>> +}
>> +
>> +static const VMStateDescription vmstate_globalstate = {
>> +    .name = "globalstate",
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .post_load = global_state_post_load,
>> +    .pre_save = global_state_pre_save,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_INT32(size, GlobalState),
>> +        VMSTATE_BUFFER(runstate, GlobalState),
>> +        VMSTATE_END_OF_LIST()
>> +    },
>> +};
>> +
>> +void register_global_state(void)
>> +{
>> +    /* We would use it independently that we receive it */
>> +    strcpy((char *)&global_state.runstate, "");
>> +    vmstate_register(NULL, 0, &vmstate_globalstate, &global_state);
>> +}
>> diff --git a/vl.c b/vl.c
>> index b8844f6..8af6c79 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -4387,6 +4387,7 @@ int main(int argc, char **argv, char **envp)
>>          return 0;
>>      }
>> 
>> +    register_global_state();
>>      if (incoming) {
>>          Error *local_err = NULL;
>>          qemu_start_incoming_migration(incoming, &local_err);
>> -- 
>> 2.4.0
>> 
>> 
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

[Qemu-devel] [PATCH 7/9] global_state: Make section optional

[Juan Quintela]

This section would be sent:

a- for all new machine types
b- for old achine types if section state is different form {running,paused}
   that were the only giving us troubles.

So, in new qemus: it is alwasy there.  In old qemus: they are only
there if it an error has happened, basically stoping on target.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/i386/pc_piix.c             |  2 ++
 hw/i386/pc_q35.c              |  2 ++
 include/migration/migration.h |  2 +-
 migration/migration.c         | 29 +++++++++++++++++++++++++++++
 4 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 212e263..5c04784 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -52,6 +52,7 @@
 #ifdef CONFIG_XEN
 #  include <xen/hvm/hvm_info_table.h>
 #endif
+#include "migration/migration.h"

 #define MAX_IDE_BUS 2

@@ -312,6 +313,7 @@ static void pc_init_pci(MachineState *machine)

 static void pc_compat_2_3(MachineState *machine)
 {
+    global_state_set_optional();
 }

 static void pc_compat_2_2(MachineState *machine)
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index e67f2de..cc5827a 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -45,6 +45,7 @@
 #include "hw/usb.h"
 #include "hw/cpu/icc_bus.h"
 #include "qemu/error-report.h"
+#include "migration/migration.h"

 /* ICH9 AHCI has 6 ports */
 #define MAX_SATA_PORTS     6
@@ -291,6 +292,7 @@ static void pc_q35_init(MachineState *machine)

 static void pc_compat_2_3(MachineState *machine)
 {
+    global_state_set_optional();
 }

 static void pc_compat_2_2(MachineState *machine)
diff --git a/include/migration/migration.h b/include/migration/migration.h
index 785c2dc..f939d88 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -183,5 +183,5 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset,
 void register_global_state(void);
 void global_state_store(void);
 char *global_state_get_runstate(void);
-
+void global_state_set_optional(void);
 #endif
diff --git a/migration/migration.c b/migration/migration.c
index ab69f81..1035689 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -868,6 +868,7 @@ void migrate_fd_connect(MigrationState *s)
 }

 typedef struct {
+    bool optional;
     int32_t size;
     uint8_t runstate[100];
 } GlobalState;
@@ -888,6 +889,33 @@ char *global_state_get_runstate(void)
     return (char *)global_state.runstate;
 }

+void global_state_set_optional(void)
+{
+    global_state.optional = true;
+}
+
+static bool global_state_needed(void *opaque)
+{
+    GlobalState *s = opaque;
+    char *runstate = (char *)s->runstate;
+
+    /* If it is not optional, it is mandatory */
+
+    if (s->optional == false) {
+        return true;
+    }
+
+    /* If state is running or paused, it is not needed */
+
+    if (strcmp(runstate, "running") == 0 ||
+        strcmp(runstate, "paused") == 0) {
+        return false;
+    }
+
+    /* for any other state it is needed */
+    return true;
+}
+
 static int global_state_post_load(void *opaque, int version_id)
 {
     GlobalState *s = opaque;
@@ -921,6 +949,7 @@ static const VMStateDescription vmstate_globalstate = {
     .minimum_version_id = 1,
     .post_load = global_state_post_load,
     .pre_save = global_state_pre_save,
+    .needed = global_state_needed,
     .fields = (VMStateField[]) {
         VMSTATE_INT32(size, GlobalState),
         VMSTATE_BUFFER(runstate, GlobalState),
-- 
2.4.0

Re: [Qemu-devel] [PATCH 7/9] global_state: Make section optional

[Dr. David Alan Gilbert]

* Juan Quintela (quintela@redhat.com) wrote:
> This section would be sent:
> 
> a- for all new machine types
> b- for old achine types if section state is different form {running,paused}
>    that were the only giving us troubles.
> 
> So, in new qemus: it is alwasy there.  In old qemus: they are only
> there if it an error has happened, basically stoping on target.

I worry about whether we should do that for old machine types;
in the current code, if the destination was started with -S  it wouldn't
start in the case you were worried about, couldn't a careful managment
layer spot-the state on the source and ensure it didn't start the destination?

(What happens with guests that have been shutdown during a migrate, or panic
or something - not that I'm sure what happens now).

> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  hw/i386/pc_piix.c             |  2 ++
>  hw/i386/pc_q35.c              |  2 ++
>  include/migration/migration.h |  2 +-
>  migration/migration.c         | 29 +++++++++++++++++++++++++++++
>  4 files changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 212e263..5c04784 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -52,6 +52,7 @@
>  #ifdef CONFIG_XEN
>  #  include <xen/hvm/hvm_info_table.h>
>  #endif
> +#include "migration/migration.h"
> 
>  #define MAX_IDE_BUS 2
> 
> @@ -312,6 +313,7 @@ static void pc_init_pci(MachineState *machine)
> 
>  static void pc_compat_2_3(MachineState *machine)
>  {
> +    global_state_set_optional();
>  }
> 
>  static void pc_compat_2_2(MachineState *machine)
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index e67f2de..cc5827a 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -45,6 +45,7 @@
>  #include "hw/usb.h"
>  #include "hw/cpu/icc_bus.h"
>  #include "qemu/error-report.h"
> +#include "migration/migration.h"
> 
>  /* ICH9 AHCI has 6 ports */
>  #define MAX_SATA_PORTS     6
> @@ -291,6 +292,7 @@ static void pc_q35_init(MachineState *machine)
> 
>  static void pc_compat_2_3(MachineState *machine)
>  {
> +    global_state_set_optional();
>  }
> 
>  static void pc_compat_2_2(MachineState *machine)
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index 785c2dc..f939d88 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -183,5 +183,5 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset,
>  void register_global_state(void);
>  void global_state_store(void);
>  char *global_state_get_runstate(void);
> -
> +void global_state_set_optional(void);
>  #endif
> diff --git a/migration/migration.c b/migration/migration.c
> index ab69f81..1035689 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -868,6 +868,7 @@ void migrate_fd_connect(MigrationState *s)
>  }
> 
>  typedef struct {
> +    bool optional;
>      int32_t size;
>      uint8_t runstate[100];
>  } GlobalState;
> @@ -888,6 +889,33 @@ char *global_state_get_runstate(void)
>      return (char *)global_state.runstate;
>  }
> 
> +void global_state_set_optional(void)
> +{
> +    global_state.optional = true;
> +}


It's a shame that it's not really a device class, because then
you could have used a DEFINE_PROP_BIT.

> +static bool global_state_needed(void *opaque)
> +{
> +    GlobalState *s = opaque;
> +    char *runstate = (char *)s->runstate;
> +
> +    /* If it is not optional, it is mandatory */
> +
> +    if (s->optional == false) {
> +        return true;
> +    }
> +
> +    /* If state is running or paused, it is not needed */
> +
> +    if (strcmp(runstate, "running") == 0 ||
> +        strcmp(runstate, "paused") == 0) {
> +        return false;
> +    }
> +
> +    /* for any other state it is needed */
> +    return true;
> +}
> +
>  static int global_state_post_load(void *opaque, int version_id)
>  {
>      GlobalState *s = opaque;
> @@ -921,6 +949,7 @@ static const VMStateDescription vmstate_globalstate = {
>      .minimum_version_id = 1,
>      .post_load = global_state_post_load,
>      .pre_save = global_state_pre_save,
> +    .needed = global_state_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_INT32(size, GlobalState),
>          VMSTATE_BUFFER(runstate, GlobalState),
> -- 
> 2.4.0
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 7/9] global_state: Make section optional

[Juan Quintela]

"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
>> This section would be sent:
>> 
>> a- for all new machine types
>> b- for old achine types if section state is different form {running,paused}
>>    that were the only giving us troubles.
>> 
>> So, in new qemus: it is alwasy there.  In old qemus: they are only
>> there if it an error has happened, basically stoping on target.
>
> I worry about whether we should do that for old machine types;
> in the current code, if the destination was started with -S  it wouldn't
> start in the case you were worried about, couldn't a careful managment
> layer spot-the state on the source and ensure it didn't start the destination?
>
> (What happens with guests that have been shutdown during a migrate, or panic
> or something - not that I'm sure what happens now).

Old guest:
If we start with -S, destination end in pause.  If there has been an
error during migration, management have to notice it.

Guest hasn't been started with -S, there is one error during migration
on source, but migration happens to end correctly.  Destination will try
to continue and will not notice the error on source.  Crash if we are
lucky, Disk corruption if we aren't.

So, for old guests, if the end state of the source is different of
paused/running, we just break migration, it is the safer thing to do,
antyhing else is just guessing.

>
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  hw/i386/pc_piix.c             |  2 ++
>>  hw/i386/pc_q35.c              |  2 ++
>>  include/migration/migration.h |  2 +-
>>  migration/migration.c         | 29 +++++++++++++++++++++++++++++
>>  4 files changed, 34 insertions(+), 1 deletion(-)
>> 
>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> index 212e263..5c04784 100644
>> --- a/hw/i386/pc_piix.c
>> +++ b/hw/i386/pc_piix.c
>> @@ -52,6 +52,7 @@
>>  #ifdef CONFIG_XEN
>>  #  include <xen/hvm/hvm_info_table.h>
>>  #endif
>> +#include "migration/migration.h"
>> 
>>  #define MAX_IDE_BUS 2
>> 
>> @@ -312,6 +313,7 @@ static void pc_init_pci(MachineState *machine)
>> 
>>  static void pc_compat_2_3(MachineState *machine)
>>  {
>> +    global_state_set_optional();
>>  }
>> 
>>  static void pc_compat_2_2(MachineState *machine)
>> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
>> index e67f2de..cc5827a 100644
>> --- a/hw/i386/pc_q35.c
>> +++ b/hw/i386/pc_q35.c
>> @@ -45,6 +45,7 @@
>>  #include "hw/usb.h"
>>  #include "hw/cpu/icc_bus.h"
>>  #include "qemu/error-report.h"
>> +#include "migration/migration.h"
>> 
>>  /* ICH9 AHCI has 6 ports */
>>  #define MAX_SATA_PORTS     6
>> @@ -291,6 +292,7 @@ static void pc_q35_init(MachineState *machine)
>> 
>>  static void pc_compat_2_3(MachineState *machine)
>>  {
>> +    global_state_set_optional();
>>  }
>> 
>>  static void pc_compat_2_2(MachineState *machine)
>> diff --git a/include/migration/migration.h b/include/migration/migration.h
>> index 785c2dc..f939d88 100644
>> --- a/include/migration/migration.h
>> +++ b/include/migration/migration.h
>> @@ -183,5 +183,5 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset,
>>  void register_global_state(void);
>>  void global_state_store(void);
>>  char *global_state_get_runstate(void);
>> -
>> +void global_state_set_optional(void);
>>  #endif
>> diff --git a/migration/migration.c b/migration/migration.c
>> index ab69f81..1035689 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -868,6 +868,7 @@ void migrate_fd_connect(MigrationState *s)
>>  }
>> 
>>  typedef struct {
>> +    bool optional;
>>      int32_t size;
>>      uint8_t runstate[100];
>>  } GlobalState;
>> @@ -888,6 +889,33 @@ char *global_state_get_runstate(void)
>>      return (char *)global_state.runstate;
>>  }
>> 
>> +void global_state_set_optional(void)
>> +{
>> +    global_state.optional = true;
>> +}
>
>
> It's a shame that it's not really a device class, because then
> you could have used a DEFINE_PROP_BIT.

Yeap.

>
>> +static bool global_state_needed(void *opaque)
>> +{
>> +    GlobalState *s = opaque;
>> +    char *runstate = (char *)s->runstate;
>> +
>> +    /* If it is not optional, it is mandatory */
>> +
>> +    if (s->optional == false) {
>> +        return true;
>> +    }
>> +
>> +    /* If state is running or paused, it is not needed */
>> +
>> +    if (strcmp(runstate, "running") == 0 ||
>> +        strcmp(runstate, "paused") == 0) {
>> +        return false;
>> +    }
>> +
>> +    /* for any other state it is needed */
>> +    return true;
>> +}
>> +
>>  static int global_state_post_load(void *opaque, int version_id)
>>  {
>>      GlobalState *s = opaque;
>> @@ -921,6 +949,7 @@ static const VMStateDescription vmstate_globalstate = {
>>      .minimum_version_id = 1,
>>      .post_load = global_state_post_load,
>>      .pre_save = global_state_pre_save,
>> +    .needed = global_state_needed,
>>      .fields = (VMStateField[]) {
>>          VMSTATE_INT32(size, GlobalState),
>>          VMSTATE_BUFFER(runstate, GlobalState),
>> -- 
>> 2.4.0
>> 
>> 
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

[Qemu-devel] [PATCH 8/9] vmstate: Create optional sections

[Juan Quintela]

To make sections optional, we need to do it at the beggining of the code.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 include/migration/vmstate.h |  2 ++
 migration/vmstate.c         | 11 +++++++++++
 savevm.c                    |  4 ++++
 3 files changed, 17 insertions(+)

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index fc5e643..a8b59eb 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -813,6 +813,8 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
 void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
                         void *opaque, QJSON *vmdesc);

+bool vmstate_save_needed(const VMStateDescription *vmsd, void *opaque);
+
 int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
                                    const VMStateDescription *vmsd,
                                    void *base, int alias_id,
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 108995e..36dab84 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -276,6 +276,17 @@ static void vmsd_desc_field_end(const VMStateDescription *vmsd, QJSON *vmdesc,
     json_end_object(vmdesc);
 }

+
+bool vmstate_save_needed(const VMStateDescription *vmsd, void *opaque)
+{
+    if (vmsd->needed && !vmsd->needed(opaque)) {
+        /* optional section not needed */
+        return false;
+    }
+    return true;
+}
+
+
 void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
                         void *opaque, QJSON *vmdesc)
 {
diff --git a/savevm.c b/savevm.c
index 01770cd..2b4e554 100644
--- a/savevm.c
+++ b/savevm.c
@@ -763,6 +763,10 @@ void qemu_savevm_state_complete(QEMUFile *f)
         if ((!se->ops || !se->ops->save_state) && !se->vmsd) {
             continue;
         }
+        if (se->vmsd && !vmstate_save_needed(se->vmsd, se->opaque)) {
+            continue;
+        }
+
         trace_savevm_section_start(se->idstr, se->section_id);

         json_start_object(vmdesc, NULL);
-- 
2.4.0

Re: [Qemu-devel] [PATCH 8/9] vmstate: Create optional sections

[Dr. David Alan Gilbert]

* Juan Quintela (quintela@redhat.com) wrote:
> To make sections optional, we need to do it at the beggining of the code.

I didn't quite understand your description; but I think the reasoning
here is that to make a top level section optional, you need
to do it in the top level to avoid sending the section header.

> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  include/migration/vmstate.h |  2 ++
>  migration/vmstate.c         | 11 +++++++++++
>  savevm.c                    |  4 ++++
>  3 files changed, 17 insertions(+)
> 
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index fc5e643..a8b59eb 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -813,6 +813,8 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
>  void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
>                          void *opaque, QJSON *vmdesc);
> 
> +bool vmstate_save_needed(const VMStateDescription *vmsd, void *opaque);
> +
>  int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
>                                     const VMStateDescription *vmsd,
>                                     void *base, int alias_id,
> diff --git a/migration/vmstate.c b/migration/vmstate.c
> index 108995e..36dab84 100644
> --- a/migration/vmstate.c
> +++ b/migration/vmstate.c
> @@ -276,6 +276,17 @@ static void vmsd_desc_field_end(const VMStateDescription *vmsd, QJSON *vmdesc,
>      json_end_object(vmdesc);
>  }
> 
> +
> +bool vmstate_save_needed(const VMStateDescription *vmsd, void *opaque)
> +{
> +    if (vmsd->needed && !vmsd->needed(opaque)) {
> +        /* optional section not needed */
> +        return false;
> +    }
> +    return true;
> +}
> +
> +
>  void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
>                          void *opaque, QJSON *vmdesc)
>  {
> diff --git a/savevm.c b/savevm.c
> index 01770cd..2b4e554 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -763,6 +763,10 @@ void qemu_savevm_state_complete(QEMUFile *f)
>          if ((!se->ops || !se->ops->save_state) && !se->vmsd) {
>              continue;
>          }
> +        if (se->vmsd && !vmstate_save_needed(se->vmsd, se->opaque)) {
> +            continue;
> +        }
> +

You might like to add a trace_ for when you skip this, but otherwise fine.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

>          trace_savevm_section_start(se->idstr, se->section_id);
> 
>          json_start_object(vmdesc, NULL);
> -- 
> 2.4.0
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 8/9] vmstate: Create optional sections

[Dr. David Alan Gilbert]

* Juan Quintela (quintela@redhat.com) wrote:
> To make sections optional, we need to do it at the beggining of the code.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>

Do you also need to add this test to qemu_save_device_state ?

Dave

> ---
>  include/migration/vmstate.h |  2 ++
>  migration/vmstate.c         | 11 +++++++++++
>  savevm.c                    |  4 ++++
>  3 files changed, 17 insertions(+)
> 
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index fc5e643..a8b59eb 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -813,6 +813,8 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
>  void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
>                          void *opaque, QJSON *vmdesc);
> 
> +bool vmstate_save_needed(const VMStateDescription *vmsd, void *opaque);
> +
>  int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
>                                     const VMStateDescription *vmsd,
>                                     void *base, int alias_id,
> diff --git a/migration/vmstate.c b/migration/vmstate.c
> index 108995e..36dab84 100644
> --- a/migration/vmstate.c
> +++ b/migration/vmstate.c
> @@ -276,6 +276,17 @@ static void vmsd_desc_field_end(const VMStateDescription *vmsd, QJSON *vmdesc,
>      json_end_object(vmdesc);
>  }
> 
> +
> +bool vmstate_save_needed(const VMStateDescription *vmsd, void *opaque)
> +{
> +    if (vmsd->needed && !vmsd->needed(opaque)) {
> +        /* optional section not needed */
> +        return false;
> +    }
> +    return true;
> +}
> +
> +
>  void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
>                          void *opaque, QJSON *vmdesc)
>  {
> diff --git a/savevm.c b/savevm.c
> index 01770cd..2b4e554 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -763,6 +763,10 @@ void qemu_savevm_state_complete(QEMUFile *f)
>          if ((!se->ops || !se->ops->save_state) && !se->vmsd) {
>              continue;
>          }
> +        if (se->vmsd && !vmstate_save_needed(se->vmsd, se->opaque)) {
> +            continue;
> +        }
> +
>          trace_savevm_section_start(se->idstr, se->section_id);
> 
>          json_start_object(vmdesc, NULL);
> -- 
> 2.4.0
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

[Qemu-devel] [PATCH 9/9] migration: Add configuration section

[Juan Quintela]

It needs to be the first one and it is not optional, that is the reason
why it is opencoded.  For new machine types, it is required than machine
type name is the same in both sides.

It is just done right now for pc's.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/i386/pc_piix.c             |  1 +
 hw/i386/pc_q35.c              |  1 +
 include/migration/migration.h |  2 ++
 savevm.c                      | 72 ++++++++++++++++++++++++++++++++++++++++---
 4 files changed, 71 insertions(+), 5 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 5c04784..95806b3 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -314,6 +314,7 @@ static void pc_init_pci(MachineState *machine)
 static void pc_compat_2_3(MachineState *machine)
 {
     global_state_set_optional();
+    savevm_skip_configuration();
 }

 static void pc_compat_2_2(MachineState *machine)
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index cc5827a..e32c040 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -293,6 +293,7 @@ static void pc_q35_init(MachineState *machine)
 static void pc_compat_2_3(MachineState *machine)
 {
     global_state_set_optional();
+    savevm_skip_configuration();
 }

 static void pc_compat_2_2(MachineState *machine)
diff --git a/include/migration/migration.h b/include/migration/migration.h
index f939d88..da89827 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -34,6 +34,7 @@
 #define QEMU_VM_SECTION_FULL         0x04
 #define QEMU_VM_SUBSECTION           0x05
 #define QEMU_VM_VMDESCRIPTION        0x06
+#define QEMU_VM_CONFIGURATION        0x07

 struct MigrationParams {
     bool blk;
@@ -184,4 +185,5 @@ void register_global_state(void);
 void global_state_store(void);
 char *global_state_get_runstate(void);
 void global_state_set_optional(void);
+void savevm_skip_configuration(void);
 #endif
diff --git a/savevm.c b/savevm.c
index 2b4e554..ea149e7 100644
--- a/savevm.c
+++ b/savevm.c
@@ -238,11 +238,55 @@ typedef struct SaveStateEntry {
 typedef struct SaveState {
     QTAILQ_HEAD(, SaveStateEntry) handlers;
     int global_section_id;
+    bool skip_configuration;
+    uint32_t len;
+    char *name;
 } SaveState;

 static SaveState savevm_state = {
     .handlers = QTAILQ_HEAD_INITIALIZER(savevm_state.handlers),
     .global_section_id = 0,
+    .skip_configuration = false,
+};
+
+void savevm_skip_configuration(void)
+{
+    savevm_state.skip_configuration = true;
+}
+
+
+static void configuration_pre_save(void *opaque)
+{
+    SaveState *state = opaque;
+    const char *current_name = MACHINE_GET_CLASS(current_machine)->name;
+
+    state->len = strlen(current_name);
+    state->name = strdup(current_name);
+}
+
+static int configuration_post_load(void *opaque, int version_id)
+{
+    SaveState *state = opaque;
+    const char *current_name = MACHINE_GET_CLASS(current_machine)->name;
+
+    if (strncmp(state->name, current_name, state->len) != 0) {
+        error_report("Machine type received is '%s' and local is '%s'",
+                     state->name, current_name);
+        return -EINVAL;
+    }
+    return 0;
+}
+
+static const VMStateDescription vmstate_configuration = {
+    .name = "configuartion",
+    .version_id = 1,
+    .post_load = configuration_post_load,
+    .pre_save = configuration_pre_save,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(len, SaveState),
+        VMSTATE_VBUFFER_ALLOC_UINT32(name, SaveState, 0, NULL, 0, len),
+        VMSTATE_END_OF_LIST()
+    },
 };

 static void dump_vmstate_vmsd(FILE *out_file,
@@ -625,7 +669,7 @@ void qemu_savevm_state_begin(QEMUFile *f,
                              const MigrationParams *params)
 {
     SaveStateEntry *se;
-    int ret;
+    int ret, len;

     trace_savevm_state_begin();
     QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
@@ -638,9 +682,13 @@ void qemu_savevm_state_begin(QEMUFile *f,
     qemu_put_be32(f, QEMU_VM_FILE_MAGIC);
     qemu_put_be32(f, QEMU_VM_FILE_VERSION);

-    QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
-        int len;
+    /* We want this to be the 1st section on the migration stream */
+    if (!savevm_state.skip_configuration) {
+        qemu_put_byte(f, QEMU_VM_CONFIGURATION);
+        vmstate_save_state(f, &vmstate_configuration, &savevm_state, 0);
+    }

+    QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
         if (!se->ops || !se->ops->save_live_setup) {
             continue;
         }
@@ -946,7 +994,7 @@ int qemu_loadvm_state(QEMUFile *f)
     Error *local_err = NULL;
     uint8_t section_type;
     unsigned int v;
-    int ret;
+    int ret, len;
     int file_error_after_eof = -1;

     if (qemu_savevm_state_blocked(&local_err)) {
@@ -970,11 +1018,25 @@ int qemu_loadvm_state(QEMUFile *f)
         return -ENOTSUP;
     }

+    /* if it exist, we want the configuration to be the 1st section
+       and we require it to be there.  That is the reason why we don't
+       read it on the while loop. */
+    if (!savevm_state.skip_configuration) {
+        if (qemu_get_byte(f) != QEMU_VM_CONFIGURATION) {
+            error_report("Configuration section missing");
+            return -EINVAL;
+        }
+        ret = vmstate_load_state(f, &vmstate_configuration, &savevm_state, 0);
+
+        if (ret) {
+            return ret;
+        }
+    }
+
     while ((section_type = qemu_get_byte(f)) != QEMU_VM_EOF) {
         uint32_t instance_id, version_id, section_id;
         SaveStateEntry *se;
         char idstr[257];
-        int len;

         trace_qemu_loadvm_state_section(section_type);
         switch (section_type) {
-- 
2.4.0

Re: [Qemu-devel] [PATCH 9/9] migration: Add configuration section

[Dr. David Alan Gilbert]

* Juan Quintela (quintela@redhat.com) wrote:
> It needs to be the first one and it is not optional, that is the reason
> why it is opencoded.  For new machine types, it is required than machine
> type name is the same in both sides.
> 
> It is just done right now for pc's.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  hw/i386/pc_piix.c             |  1 +
>  hw/i386/pc_q35.c              |  1 +
>  include/migration/migration.h |  2 ++
>  savevm.c                      | 72 ++++++++++++++++++++++++++++++++++++++++---
>  4 files changed, 71 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 5c04784..95806b3 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -314,6 +314,7 @@ static void pc_init_pci(MachineState *machine)
>  static void pc_compat_2_3(MachineState *machine)
>  {
>      global_state_set_optional();
> +    savevm_skip_configuration();
>  }
> 
>  static void pc_compat_2_2(MachineState *machine)
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index cc5827a..e32c040 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -293,6 +293,7 @@ static void pc_q35_init(MachineState *machine)
>  static void pc_compat_2_3(MachineState *machine)
>  {
>      global_state_set_optional();
> +    savevm_skip_configuration();

It's a shame that these two functions, that do basically the same thing
(to two different pieces of data) have such different names.

>  }
> 
>  static void pc_compat_2_2(MachineState *machine)
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index f939d88..da89827 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -34,6 +34,7 @@
>  #define QEMU_VM_SECTION_FULL         0x04
>  #define QEMU_VM_SUBSECTION           0x05
>  #define QEMU_VM_VMDESCRIPTION        0x06
> +#define QEMU_VM_CONFIGURATION        0x07
> 
>  struct MigrationParams {
>      bool blk;
> @@ -184,4 +185,5 @@ void register_global_state(void);
>  void global_state_store(void);
>  char *global_state_get_runstate(void);
>  void global_state_set_optional(void);
> +void savevm_skip_configuration(void);
>  #endif
> diff --git a/savevm.c b/savevm.c
> index 2b4e554..ea149e7 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -238,11 +238,55 @@ typedef struct SaveStateEntry {
>  typedef struct SaveState {
>      QTAILQ_HEAD(, SaveStateEntry) handlers;
>      int global_section_id;
> +    bool skip_configuration;
> +    uint32_t len;
> +    char *name;
>  } SaveState;
> 
>  static SaveState savevm_state = {
>      .handlers = QTAILQ_HEAD_INITIALIZER(savevm_state.handlers),
>      .global_section_id = 0,
> +    .skip_configuration = false,
> +};
> +
> +void savevm_skip_configuration(void)
> +{
> +    savevm_state.skip_configuration = true;
> +}
> +
> +
> +static void configuration_pre_save(void *opaque)
> +{
> +    SaveState *state = opaque;
> +    const char *current_name = MACHINE_GET_CLASS(current_machine)->name;
> +
> +    state->len = strlen(current_name);
> +    state->name = strdup(current_name);

Will that ever get freed?  If it never gets freed is it safe to
just make it 

   state->len = current_name;


> +
> +static int configuration_post_load(void *opaque, int version_id)
> +{
> +    SaveState *state = opaque;
> +    const char *current_name = MACHINE_GET_CLASS(current_machine)->name;
> +
> +    if (strncmp(state->name, current_name, state->len) != 0) {
> +        error_report("Machine type received is '%s' and local is '%s'",
> +                     state->name, current_name);
> +        return -EINVAL;
> +    }
> +    return 0;
> +}
> +
> +static const VMStateDescription vmstate_configuration = {
> +    .name = "configuartion",

Typo! configuration

Dave

> +    .version_id = 1,
> +    .post_load = configuration_post_load,
> +    .pre_save = configuration_pre_save,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32(len, SaveState),
> +        VMSTATE_VBUFFER_ALLOC_UINT32(name, SaveState, 0, NULL, 0, len),
> +        VMSTATE_END_OF_LIST()
> +    },
>  };
> 
>  static void dump_vmstate_vmsd(FILE *out_file,
> @@ -625,7 +669,7 @@ void qemu_savevm_state_begin(QEMUFile *f,
>                               const MigrationParams *params)
>  {
>      SaveStateEntry *se;
> -    int ret;
> +    int ret, len;
> 
>      trace_savevm_state_begin();
>      QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> @@ -638,9 +682,13 @@ void qemu_savevm_state_begin(QEMUFile *f,
>      qemu_put_be32(f, QEMU_VM_FILE_MAGIC);
>      qemu_put_be32(f, QEMU_VM_FILE_VERSION);
> 
> -    QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> -        int len;
> +    /* We want this to be the 1st section on the migration stream */
> +    if (!savevm_state.skip_configuration) {
> +        qemu_put_byte(f, QEMU_VM_CONFIGURATION);
> +        vmstate_save_state(f, &vmstate_configuration, &savevm_state, 0);
> +    }
> 
> +    QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>          if (!se->ops || !se->ops->save_live_setup) {
>              continue;
>          }
> @@ -946,7 +994,7 @@ int qemu_loadvm_state(QEMUFile *f)
>      Error *local_err = NULL;
>      uint8_t section_type;
>      unsigned int v;
> -    int ret;
> +    int ret, len;
>      int file_error_after_eof = -1;
> 
>      if (qemu_savevm_state_blocked(&local_err)) {
> @@ -970,11 +1018,25 @@ int qemu_loadvm_state(QEMUFile *f)
>          return -ENOTSUP;
>      }
> 
> +    /* if it exist, we want the configuration to be the 1st section
> +       and we require it to be there.  That is the reason why we don't
> +       read it on the while loop. */
> +    if (!savevm_state.skip_configuration) {
> +        if (qemu_get_byte(f) != QEMU_VM_CONFIGURATION) {
> +            error_report("Configuration section missing");
> +            return -EINVAL;
> +        }
> +        ret = vmstate_load_state(f, &vmstate_configuration, &savevm_state, 0);
> +
> +        if (ret) {
> +            return ret;
> +        }
> +    }
> +
>      while ((section_type = qemu_get_byte(f)) != QEMU_VM_EOF) {
>          uint32_t instance_id, version_id, section_id;
>          SaveStateEntry *se;
>          char idstr[257];
> -        int len;
> 
>          trace_qemu_loadvm_state_section(section_type);
>          switch (section_type) {
> -- 
> 2.4.0
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 9/9] migration: Add configuration section

[Juan Quintela]

"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
>> It needs to be the first one and it is not optional, that is the reason
>> why it is opencoded.  For new machine types, it is required than machine
>> type name is the same in both sides.
>> 
>> It is just done right now for pc's.
>> 
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  hw/i386/pc_piix.c             |  1 +
>>  hw/i386/pc_q35.c              |  1 +
>>  include/migration/migration.h |  2 ++
>>  savevm.c                      | 72 ++++++++++++++++++++++++++++++++++++++++---
>>  4 files changed, 71 insertions(+), 5 deletions(-)
>> 
>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> index 5c04784..95806b3 100644
>> --- a/hw/i386/pc_piix.c
>> +++ b/hw/i386/pc_piix.c
>> @@ -314,6 +314,7 @@ static void pc_init_pci(MachineState *machine)
>>  static void pc_compat_2_3(MachineState *machine)
>>  {
>>      global_state_set_optional();
>> +    savevm_skip_configuration();
>>  }
>> 
>>  static void pc_compat_2_2(MachineState *machine)
>> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
>> index cc5827a..e32c040 100644
>> --- a/hw/i386/pc_q35.c
>> +++ b/hw/i386/pc_q35.c
>> @@ -293,6 +293,7 @@ static void pc_q35_init(MachineState *machine)
>>  static void pc_compat_2_3(MachineState *machine)
>>  {
>>      global_state_set_optional();
>> +    savevm_skip_configuration();
>
> It's a shame that these two functions, that do basically the same thing
> (to two different pieces of data) have such different names.

Code is similar, but meaning is different.

1st one: it is sent only if needed
2nd one: it is completely skiped.

Calling the second: configuration_optional() looks weeird, when the
meaning is NO_configuration.

We could rename the 1st one to global_state_skip_if_not_needed, but not
sure that it would be better :-(

I am open to name changes if you provide better names O:-)

>
>>  }
>> 
>>  static void pc_compat_2_2(MachineState *machine)
>> diff --git a/include/migration/migration.h b/include/migration/migration.h
>> index f939d88..da89827 100644
>> --- a/include/migration/migration.h
>> +++ b/include/migration/migration.h
>> @@ -34,6 +34,7 @@
>>  #define QEMU_VM_SECTION_FULL         0x04
>>  #define QEMU_VM_SUBSECTION           0x05
>>  #define QEMU_VM_VMDESCRIPTION        0x06
>> +#define QEMU_VM_CONFIGURATION        0x07
>> 
>>  struct MigrationParams {
>>      bool blk;
>> @@ -184,4 +185,5 @@ void register_global_state(void);
>>  void global_state_store(void);
>>  char *global_state_get_runstate(void);
>>  void global_state_set_optional(void);
>> +void savevm_skip_configuration(void);
>>  #endif
>> diff --git a/savevm.c b/savevm.c
>> index 2b4e554..ea149e7 100644
>> --- a/savevm.c
>> +++ b/savevm.c
>> @@ -238,11 +238,55 @@ typedef struct SaveStateEntry {
>>  typedef struct SaveState {
>>      QTAILQ_HEAD(, SaveStateEntry) handlers;
>>      int global_section_id;
>> +    bool skip_configuration;
>> +    uint32_t len;
>> +    char *name;
>>  } SaveState;
>> 
>>  static SaveState savevm_state = {
>>      .handlers = QTAILQ_HEAD_INITIALIZER(savevm_state.handlers),
>>      .global_section_id = 0,
>> +    .skip_configuration = false,
>> +};
>> +
>> +void savevm_skip_configuration(void)
>> +{
>> +    savevm_state.skip_configuration = true;
>> +}
>> +
>> +
>> +static void configuration_pre_save(void *opaque)
>> +{
>> +    SaveState *state = opaque;
>> +    const char *current_name = MACHINE_GET_CLASS(current_machine)->name;
>> +
>> +    state->len = strlen(current_name);
>> +    state->name = strdup(current_name);
>
> Will that ever get freed?  If it never gets freed is it safe to
> just make it 
>
>    state->len = current_name;

No, changed.

>
>
>> +
>> +static int configuration_post_load(void *opaque, int version_id)
>> +{
>> +    SaveState *state = opaque;
>> +    const char *current_name = MACHINE_GET_CLASS(current_machine)->name;
>> +
>> +    if (strncmp(state->name, current_name, state->len) != 0) {
>> +        error_report("Machine type received is '%s' and local is '%s'",
>> +                     state->name, current_name);
>> +        return -EINVAL;
>> +    }
>> +    return 0;
>> +}
>> +
>> +static const VMStateDescription vmstate_configuration = {
>> +    .name = "configuartion",
>
> Typo! configuration

Thanks.