[PATCH 00/18] hw: Mark the device with no migratable fields

[PATCH 00/18] hw: Mark the device with no migratable fields

[Philippe Mathieu-Daudé]

This is a proof-of-concept after chatting with Peter Maydell
on IRC earlier.

Introduce the vmstate_no_state_to_migrate structure, and
a reference to it: vmstate_qdev_no_state_to_migrate.
Use this reference in devices with no fields to migrate.

This is useful to catch devices missing vmstate, such:
- ads7846
- mcf-uart
- mcf-fec
- versatile_i2c
- ...

I am not sure about:
- gpex-pcihost

Philippe Mathieu-Daudé (18):
  migration/vmstate: Document vmstate_dummy
  migration/vmstate: Introduce vmstate_no_state_to_migrate
  hw/core/qdev: Add vmstate_qdev_no_state_to_migrate
  hw/arm/armv7m: Mark the device with no migratable fields
  hw/arm/aspeed_soc: Mark the device with no migratable fields
  hw/arm/bcm283x: Mark devices with no migratable fields
  hw/arm/msf2-soc: Mark the device with no migratable fields
  hw/core/split-irq: Mark the device with no migratable fields
  hw/cpu/a9mpcore: Mark the device with no migratable fields
  hw/cpu/cluster: Mark the device with no migratable fields
  hw/usb/hcd-ohci: Mark the device with no migratable fields
  hw/intc/arm_gicv2m: Mark the device with no migratable fields
  hw/misc/armsse-cpuid: Mark the device with no migratable fields
  hw/misc/iotkit-sysinfo: Mark the device with no migratable fields
  hw/misc/unimp: Mark the device with no migratable fields
  hw/nubus/mac-nubus-bridge: Mark the device with no migratable fields
  hw/sparc64/sun4u: Mark devices with no migratable fields
  hw/core/qdev: Display warning for devices missing migration state

 configure                    |  9 +++++++++
 hw/usb/hcd-ohci.h            |  2 ++
 include/hw/arm/bcm2836.h     |  7 ++++---
 include/hw/arm/msf2-soc.h    | 11 ++++++-----
 include/hw/cpu/a9mpcore.h    |  3 ++-
 include/hw/qdev-core.h       |  2 ++
 include/migration/vmstate.h  |  3 ++-
 hw/arm/armv7m.c              |  1 +
 hw/arm/aspeed_soc.c          |  1 +
 hw/arm/bcm2835_peripherals.c |  1 +
 hw/arm/bcm2836.c             |  1 +
 hw/arm/msf2-soc.c            |  1 +
 hw/core/qdev.c               |  8 ++++++++
 hw/core/split-irq.c          |  1 +
 hw/cpu/a9mpcore.c            |  1 +
 hw/cpu/cluster.c             |  1 +
 hw/intc/arm_gicv2m.c         |  2 ++
 hw/misc/armsse-cpuid.c       |  1 +
 hw/misc/iotkit-sysinfo.c     |  1 +
 hw/misc/unimp.c              |  1 +
 hw/nubus/mac-nubus-bridge.c  |  1 +
 hw/sparc64/sun4u.c           |  7 ++++++-
 hw/usb/hcd-ohci.c            |  1 +
 migration/vmstate.c          |  7 +++++++
 24 files changed, 63 insertions(+), 11 deletions(-)

-- 
2.21.3

[PATCH 01/18] migration/vmstate: Document vmstate_dummy

[Philippe Mathieu-Daudé]

vmstate_dummy is special and restricted to linux-user. See commit
c71c3e99b8 ("Add a vmstate_dummy struct for CONFIG_USER_ONLY").

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/migration/vmstate.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index f68ed7db13..af7d80cd4e 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -194,7 +194,7 @@ struct VMStateDescription {
     const VMStateDescription **subsections;
 };
 
-extern const VMStateDescription vmstate_dummy;
+extern const VMStateDescription vmstate_dummy; /* Exclusively for linux-user */
 
 extern const VMStateInfo vmstate_info_bool;
 
-- 
2.21.3

[PATCH 02/18] migration/vmstate: Introduce vmstate_no_state_to_migrate

[Philippe Mathieu-Daudé]

Introduce a special state to indicate when an object doesn't
have anything in its state to migrate.

Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/migration/vmstate.h | 1 +
 migration/vmstate.c         | 7 +++++++
 2 files changed, 8 insertions(+)

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index af7d80cd4e..0922bc8efa 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -195,6 +195,7 @@ struct VMStateDescription {
 };
 
 extern const VMStateDescription vmstate_dummy; /* Exclusively for linux-user */
+extern const VMStateDescription vmstate_no_state_to_migrate;
 
 extern const VMStateInfo vmstate_info_bool;
 
diff --git a/migration/vmstate.c b/migration/vmstate.c
index bafa890384..d640cafad3 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -20,6 +20,13 @@
 #include "trace.h"
 #include "qjson.h"
 
+const VMStateDescription vmstate_no_state_to_migrate = {
+    .name = "empty-state",
+    .fields = (VMStateField[]) {
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static int vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
                                    void *opaque, QJSON *vmdesc);
 static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
-- 
2.21.3

[PATCH 03/18] hw/core/qdev: Add vmstate_qdev_no_state_to_migrate

[Philippe Mathieu-Daudé]

Add vmstate_qdev_no_state_to_migrate, which is simply a
pointer to vmstate_no_state_to_migrate. This way all
qdev devices (including "hw/qdev-core.h") don't have to
include "migration/vmstate.h".

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/qdev-core.h | 2 ++
 hw/core/qdev.c         | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index fe78073c70..2e4ddfd75f 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -139,6 +139,8 @@ typedef struct DeviceClass {
     const char *bus_type;
 } DeviceClass;
 
+extern const VMStateDescription *vmstate_qdev_no_state_to_migrate;
+
 typedef struct NamedGPIOList NamedGPIOList;
 
 struct NamedGPIOList {
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 2131c7f951..0d18bc6d93 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -45,6 +45,9 @@ bool qdev_hotplug = false;
 static bool qdev_hot_added = false;
 bool qdev_hot_removed = false;
 
+const VMStateDescription *vmstate_qdev_no_state_to_migrate =
+        &vmstate_no_state_to_migrate;
+
 const VMStateDescription *qdev_get_vmsd(DeviceState *dev)
 {
     DeviceClass *dc = DEVICE_GET_CLASS(dev);
-- 
2.21.3

[PATCH 04/18] hw/arm/armv7m: Mark the device with no migratable fields

[Philippe Mathieu-Daudé]

This device doesn't have fields to migrate. Be explicit
by using vmstate_qdev_no_state_to_migrate.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/arm/armv7m.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index 3308211e9c..3f78fccc12 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -366,6 +366,7 @@ static void bitband_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = bitband_realize;
+    dc->vmsd = vmstate_qdev_no_state_to_migrate;
     device_class_set_props(dc, bitband_properties);
 }
 
-- 
2.21.3

[PATCH 05/18] hw/arm/aspeed_soc: Mark the device with no migratable fields

[Philippe Mathieu-Daudé]

This device doesn't have fields to migrate. Be explicit
by using vmstate_qdev_no_state_to_migrate.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/arm/aspeed_soc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index 311458aa76..b15984e4d3 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -442,6 +442,7 @@ static void aspeed_soc_class_init(ObjectClass *oc, void *data)
     dc->realize = aspeed_soc_realize;
     /* Reason: Uses serial_hds and nd_table in realize() directly */
     dc->user_creatable = false;
+    dc->vmsd = vmstate_qdev_no_state_to_migrate;
     device_class_set_props(dc, aspeed_soc_properties);
 }
 
-- 
2.21.3

[PATCH 06/18] hw/arm/bcm283x: Mark devices with no migratable fields

[Philippe Mathieu-Daudé]

These devices don't have fields to migrate. Be explicit
by using vmstate_qdev_no_state_to_migrate.

Add a more descriptive comment to keep a clear separation
between static property vs runtime changeable.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/arm/bcm2836.h     | 7 ++++---
 hw/arm/bcm2835_peripherals.c | 1 +
 hw/arm/bcm2836.c             | 1 +
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/include/hw/arm/bcm2836.h b/include/hw/arm/bcm2836.h
index 024af8aae4..66e3621bb9 100644
--- a/include/hw/arm/bcm2836.h
+++ b/include/hw/arm/bcm2836.h
@@ -33,14 +33,15 @@ typedef struct BCM283XState {
     DeviceState parent_obj;
     /*< public >*/
 
-    char *cpu_type;
-    uint32_t enabled_cpus;
-
     struct {
         ARMCPU core;
     } cpu[BCM283X_NCPUS];
     BCM2836ControlState control;
     BCM2835PeripheralState peripherals;
+
+    /* Properties */
+    char *cpu_type;
+    uint32_t enabled_cpus;
 } BCM283XState;
 
 typedef struct BCM283XInfo BCM283XInfo;
diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 7ffdf62067..f33500dc27 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -401,6 +401,7 @@ static void bcm2835_peripherals_class_init(ObjectClass *oc, void *data)
     DeviceClass *dc = DEVICE_CLASS(oc);
 
     dc->realize = bcm2835_peripherals_realize;
+    dc->vmsd = vmstate_qdev_no_state_to_migrate;
 }
 
 static const TypeInfo bcm2835_peripherals_type_info = {
diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
index ed1793f7b7..5fc67b6777 100644
--- a/hw/arm/bcm2836.c
+++ b/hw/arm/bcm2836.c
@@ -173,6 +173,7 @@ static void bcm283x_class_init(ObjectClass *oc, void *data)
     device_class_set_props(dc, bcm2836_props);
     /* Reason: Must be wired up in code (see raspi_init() function) */
     dc->user_creatable = false;
+    dc->vmsd = vmstate_qdev_no_state_to_migrate;
 }
 
 static const TypeInfo bcm283x_type_info = {
-- 
2.21.3

[PATCH 07/18] hw/arm/msf2-soc: Mark the device with no migratable fields

[Philippe Mathieu-Daudé]

This device doesn't have fields to migrate. Be explicit
by using vmstate_qdev_no_state_to_migrate.

Add a more descriptive comment to keep a clear separation
between static property vs runtime changeable.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/arm/msf2-soc.h | 11 ++++++-----
 hw/arm/msf2-soc.c         |  1 +
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/include/hw/arm/msf2-soc.h b/include/hw/arm/msf2-soc.h
index c9cb214aa6..8f85ff8295 100644
--- a/include/hw/arm/msf2-soc.h
+++ b/include/hw/arm/msf2-soc.h
@@ -51,6 +51,12 @@ typedef struct MSF2State {
 
     ARMv7MState armv7m;
 
+    MSF2SysregState sysreg;
+    MSSTimerState timer;
+    MSSSpiState spi[MSF2_NUM_SPIS];
+    MSF2EmacState emac;
+
+    /* Properties */
     char *cpu_type;
     char *part_name;
     uint64_t envm_size;
@@ -59,11 +65,6 @@ typedef struct MSF2State {
     uint32_t m3clk;
     uint8_t apb0div;
     uint8_t apb1div;
-
-    MSF2SysregState sysreg;
-    MSSTimerState timer;
-    MSSSpiState spi[MSF2_NUM_SPIS];
-    MSF2EmacState emac;
 } MSF2State;
 
 #endif
diff --git a/hw/arm/msf2-soc.c b/hw/arm/msf2-soc.c
index 3235c76194..bcec389a01 100644
--- a/hw/arm/msf2-soc.c
+++ b/hw/arm/msf2-soc.c
@@ -245,6 +245,7 @@ static void m2sxxx_soc_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = m2sxxx_soc_realize;
+    dc->vmsd = vmstate_qdev_no_state_to_migrate;
     device_class_set_props(dc, m2sxxx_soc_properties);
 }
 
-- 
2.21.3

[PATCH 08/18] hw/core/split-irq: Mark the device with no migratable fields

[Philippe Mathieu-Daudé]

This device doesn't have fields to migrate. Be explicit
by using vmstate_qdev_no_state_to_migrate.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/core/split-irq.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/core/split-irq.c b/hw/core/split-irq.c
index 3b90af2e8f..a7072f922c 100644
--- a/hw/core/split-irq.c
+++ b/hw/core/split-irq.c
@@ -71,6 +71,7 @@ static void split_irq_class_init(ObjectClass *klass, void *data)
     /* No state to reset or migrate */
     device_class_set_props(dc, split_irq_properties);
     dc->realize = split_irq_realize;
+    dc->vmsd = vmstate_qdev_no_state_to_migrate;
 
     /* Reason: Needs to be wired up to work */
     dc->user_creatable = false;
-- 
2.21.3

[PATCH 09/18] hw/cpu/a9mpcore: Mark the device with no migratable fields

[Philippe Mathieu-Daudé]

This device doesn't have fields to migrate. Be explicit
by using vmstate_qdev_no_state_to_migrate.

Add a more descriptive comment to keep a clear separation
between static property vs runtime changeable.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/cpu/a9mpcore.h | 3 ++-
 hw/cpu/a9mpcore.c         | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/hw/cpu/a9mpcore.h b/include/hw/cpu/a9mpcore.h
index 5d67ca22c4..212f5db391 100644
--- a/include/hw/cpu/a9mpcore.h
+++ b/include/hw/cpu/a9mpcore.h
@@ -25,10 +25,11 @@ typedef struct A9MPPrivState {
     SysBusDevice parent_obj;
     /*< public >*/
 
+    /* Properties */
     uint32_t num_cpu;
-    MemoryRegion container;
     uint32_t num_irq;
 
+    MemoryRegion container;
     A9SCUState scu;
     GICState gic;
     A9GTimerState gtimer;
diff --git a/hw/cpu/a9mpcore.c b/hw/cpu/a9mpcore.c
index 642363d2f4..5012c7a81a 100644
--- a/hw/cpu/a9mpcore.c
+++ b/hw/cpu/a9mpcore.c
@@ -176,6 +176,7 @@ static void a9mp_priv_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = a9mp_priv_realize;
+    dc->vmsd = vmstate_qdev_no_state_to_migrate;
     device_class_set_props(dc, a9mp_priv_properties);
 }
 
-- 
2.21.3

[PATCH 10/18] hw/cpu/cluster: Mark the device with no migratable fields

[Philippe Mathieu-Daudé]

This device doesn't have fields to migrate. Be explicit
by using vmstate_qdev_no_state_to_migrate.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/cpu/cluster.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/cpu/cluster.c b/hw/cpu/cluster.c
index e444b7c29d..95653a643a 100644
--- a/hw/cpu/cluster.c
+++ b/hw/cpu/cluster.c
@@ -80,6 +80,7 @@ static void cpu_cluster_class_init(ObjectClass *klass, void *data)
 
     device_class_set_props(dc, cpu_cluster_properties);
     dc->realize = cpu_cluster_realize;
+    dc->vmsd = vmstate_qdev_no_state_to_migrate;
 
     /* This is not directly for users, CPU children must be attached by code */
     dc->user_creatable = false;
-- 
2.21.3

[PATCH 11/18] hw/usb/hcd-ohci: Mark the device with no migratable fields

[Philippe Mathieu-Daudé]

This device doesn't have fields to migrate. Be explicit
by using vmstate_qdev_no_state_to_migrate.

Add a more descriptive comment to keep a clear separation
between static property vs runtime changeable.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/usb/hcd-ohci.h | 2 ++
 hw/usb/hcd-ohci.c | 1 +
 2 files changed, 3 insertions(+)

diff --git a/hw/usb/hcd-ohci.h b/hw/usb/hcd-ohci.h
index 5c8819aedf..3bcbad1269 100644
--- a/hw/usb/hcd-ohci.h
+++ b/hw/usb/hcd-ohci.h
@@ -100,6 +100,8 @@ typedef struct {
     /*< public >*/
 
     OHCIState ohci;
+
+    /* Properties */
     char *masterbus;
     uint32_t num_ports;
     uint32_t firstport;
diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index 1e6e85e86a..6c76f65734 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -1983,6 +1983,7 @@ static void ohci_sysbus_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = ohci_realize_pxa;
+    dc->vmsd = vmstate_qdev_no_state_to_migrate;
     set_bit(DEVICE_CATEGORY_USB, dc->categories);
     dc->desc = "OHCI USB Controller";
     device_class_set_props(dc, ohci_sysbus_properties);
-- 
2.21.3

[PATCH 12/18] hw/intc/arm_gicv2m: Mark the device with no migratable fields

[Philippe Mathieu-Daudé]

This device doesn't have fields to migrate. Be explicit
by using vmstate_qdev_no_state_to_migrate.

Add a more descriptive comment to keep a clear separation
between static property vs runtime changeable.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/intc/arm_gicv2m.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/intc/arm_gicv2m.c b/hw/intc/arm_gicv2m.c
index 0b7e2b4f84..c8b066066b 100644
--- a/hw/intc/arm_gicv2m.c
+++ b/hw/intc/arm_gicv2m.c
@@ -54,6 +54,7 @@ typedef struct ARMGICv2mState {
     MemoryRegion iomem;
     qemu_irq spi[GICV2M_NUM_SPI_MAX];
 
+    /* Properties */
     uint32_t base_spi;
     uint32_t num_spi;
 } ARMGICv2mState;
@@ -181,6 +182,7 @@ static void gicv2m_class_init(ObjectClass *klass, void *data)
 
     device_class_set_props(dc, gicv2m_properties);
     dc->realize = gicv2m_realize;
+    dc->vmsd = vmstate_qdev_no_state_to_migrate;
 }
 
 static const TypeInfo gicv2m_info = {
-- 
2.21.3

[PATCH 13/18] hw/misc/armsse-cpuid: Mark the device with no migratable fields

[Philippe Mathieu-Daudé]

This device doesn't have fields to migrate. Be explicit
by using vmstate_qdev_no_state_to_migrate.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/misc/armsse-cpuid.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/misc/armsse-cpuid.c b/hw/misc/armsse-cpuid.c
index d58138dc28..61251d538b 100644
--- a/hw/misc/armsse-cpuid.c
+++ b/hw/misc/armsse-cpuid.c
@@ -115,6 +115,7 @@ static void armsse_cpuid_class_init(ObjectClass *klass, void *data)
      * This device has no guest-modifiable state and so it
      * does not need a reset function or VMState.
      */
+    dc->vmsd = vmstate_qdev_no_state_to_migrate;
 
     device_class_set_props(dc, armsse_cpuid_props);
 }
-- 
2.21.3

[PATCH 14/18] hw/misc/iotkit-sysinfo: Mark the device with no migratable fields

[Philippe Mathieu-Daudé]

This device doesn't have fields to migrate. Be explicit
by using vmstate_qdev_no_state_to_migrate.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/misc/iotkit-sysinfo.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/misc/iotkit-sysinfo.c b/hw/misc/iotkit-sysinfo.c
index b2dcfc4376..8bb9a2ef8b 100644
--- a/hw/misc/iotkit-sysinfo.c
+++ b/hw/misc/iotkit-sysinfo.c
@@ -120,6 +120,7 @@ static void iotkit_sysinfo_class_init(ObjectClass *klass, void *data)
      * This device has no guest-modifiable state and so it
      * does not need a reset function or VMState.
      */
+    dc->vmsd = vmstate_qdev_no_state_to_migrate;
 
     device_class_set_props(dc, iotkit_sysinfo_props);
 }
-- 
2.21.3

[PATCH 15/18] hw/misc/unimp: Mark the device with no migratable fields

[Philippe Mathieu-Daudé]

This device doesn't have fields to migrate. Be explicit
by using vmstate_qdev_no_state_to_migrate.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/misc/unimp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/misc/unimp.c b/hw/misc/unimp.c
index bc4084d344..8a0db25a77 100644
--- a/hw/misc/unimp.c
+++ b/hw/misc/unimp.c
@@ -79,6 +79,7 @@ static void unimp_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = unimp_realize;
+    dc->vmsd = vmstate_qdev_no_state_to_migrate;
     device_class_set_props(dc, unimp_properties);
 }
 
-- 
2.21.3

[PATCH 16/18] hw/nubus/mac-nubus-bridge: Mark the device with no migratable fields

[Philippe Mathieu-Daudé]

This device doesn't have fields to migrate. Be explicit
by using vmstate_qdev_no_state_to_migrate.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/nubus/mac-nubus-bridge.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/nubus/mac-nubus-bridge.c b/hw/nubus/mac-nubus-bridge.c
index 7c329300b8..ede36ccc5d 100644
--- a/hw/nubus/mac-nubus-bridge.c
+++ b/hw/nubus/mac-nubus-bridge.c
@@ -27,6 +27,7 @@ static void mac_nubus_bridge_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->desc = "Nubus bridge";
+    dc->vmsd = vmstate_qdev_no_state_to_migrate;
 }
 
 static const TypeInfo mac_nubus_bridge_info = {
-- 
2.21.3

[PATCH 17/18] hw/sparc64/sun4u: Mark devices with no migratable fields

[Philippe Mathieu-Daudé]

These devices don't have fields to migrate. Be explicit
by using vmstate_qdev_no_state_to_migrate.

Add a more descriptive comment to keep a clear separation
between static property vs runtime changeable.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sparc64/sun4u.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 9c8655cffc..2653893625 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -82,12 +82,15 @@ struct hwdef {
 typedef struct EbusState {
     /*< private >*/
     PCIDevice parent_obj;
+    /*< public >*/
 
     ISABus *isa_bus;
     qemu_irq isa_bus_irqs[ISA_NUM_IRQS];
-    uint64_t console_serial_base;
     MemoryRegion bar0;
     MemoryRegion bar1;
+
+    /* Properties */
+    uint64_t console_serial_base;
 } EbusState;
 
 #define TYPE_EBUS "ebus"
@@ -384,6 +387,7 @@ static void ebus_class_init(ObjectClass *klass, void *data)
     k->device_id = PCI_DEVICE_ID_SUN_EBUS;
     k->revision = 0x01;
     k->class_id = PCI_CLASS_BRIDGE_OTHER;
+    dc->vmsd = vmstate_qdev_no_state_to_migrate;
     device_class_set_props(dc, ebus_properties);
 }
 
@@ -476,6 +480,7 @@ static void prom_class_init(ObjectClass *klass, void *data)
 
     device_class_set_props(dc, prom_properties);
     dc->realize = prom_realize;
+    dc->vmsd = vmstate_qdev_no_state_to_migrate;
 }
 
 static const TypeInfo prom_info = {
-- 
2.21.3

[RFC PATCH 18/18] hw/core/qdev: Display warning for devices missing migration state

[Philippe Mathieu-Daudé]

When built with --enable-qdev-debug, QEMU displays warnings
listing devices missing migration state:

  $ qemu-system-arm -S -M spitz
  qemu-system-arm: warning: missing migration state for type: 'pxa270-c0-arm-cpu'
  qemu-system-arm: warning: missing migration state for type: 'serial'
  qemu-system-arm: warning: missing migration state for type: 'pxa2xx-pcmcia'
  qemu-system-arm: warning: missing migration state for type: 'pxa2xx-pcmcia'
  qemu-system-arm: warning: missing migration state for type: 'pxa2xx-i2c-slave'
  qemu-system-arm: warning: missing migration state for type: 'pxa2xx-i2c-slave'
  qemu-system-arm: warning: missing migration state for type: 'ads7846'
  qemu-system-arm: warning: missing migration state for type: 'max1111'

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
RFC because there might be something simpler than --enable-qdev-debug.

 configure      | 9 +++++++++
 hw/core/qdev.c | 5 +++++
 2 files changed, 14 insertions(+)

diff --git a/configure b/configure
index 8a65240d4a..d38efd5605 100755
--- a/configure
+++ b/configure
@@ -441,6 +441,7 @@ edk2_blobs="no"
 pkgversion=""
 pie=""
 qom_cast_debug="yes"
+qdev_debug="no"
 trace_backends="log"
 trace_file="trace"
 spice=""
@@ -1124,6 +1125,10 @@ for opt do
   ;;
   --enable-qom-cast-debug) qom_cast_debug="yes"
   ;;
+  --disable-qdev-debug) qdev_debug="no"
+  ;;
+  --enable-qdev-debug) qdev_debug="yes"
+  ;;
   --disable-virtfs) virtfs="no"
   ;;
   --enable-virtfs) virtfs="yes"
@@ -6966,6 +6971,7 @@ echo "gcov enabled      $gcov"
 echo "TPM support       $tpm"
 echo "libssh support    $libssh"
 echo "QOM debugging     $qom_cast_debug"
+echo "QDEV debugging    $qdev_debug"
 echo "Live block migration $live_block_migration"
 echo "lzo support       $lzo"
 echo "snappy support    $snappy"
@@ -7594,6 +7600,9 @@ fi
 if test "$qom_cast_debug" = "yes" ; then
   echo "CONFIG_QOM_CAST_DEBUG=y" >> $config_host_mak
 fi
+if test "$qdev_debug" = "yes" ; then
+  echo "CONFIG_QDEV_DEBUG=y" >> $config_host_mak
+fi
 if test "$rbd" = "yes" ; then
   echo "CONFIG_RBD=m" >> $config_host_mak
   echo "RBD_CFLAGS=$rbd_cflags" >> $config_host_mak
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 0d18bc6d93..c2da1e0509 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -932,6 +932,11 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
                                                &local_err) < 0) {
                 goto post_realize_fail;
             }
+        } else {
+#ifdef CONFIG_QDEV_DEBUG
+            warn_report("missing migration state for type: '%s'",
+                        object_get_typename(OBJECT(dev)));
+#endif
         }
 
         /*
-- 
2.21.3

Re: [PATCH 00/18] hw: Mark the device with no migratable fields

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200703201911.26573-1-f4bug@amsat.org/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  LINK    tests/test-io-task
  LINK    tests/test-io-channel-socket
hw/core/qdev.o:(.data.rel+0x0): undefined reference to `vmstate_no_state_to_migrate'
collect2: error: ld returned 1 exit status
make: *** [tests/test-qdev-global-props] Error 1
make: *** Waiting for unfinished jobs....
  TEST    iotest-qcow2: 002
  TEST    iotest-qcow2: 003
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=ba983f0b8bc94d39a0a649a449a91f08', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-mg_r93vd/src/docker-src.2020-07-03-16.46.29.22759:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=ba983f0b8bc94d39a0a649a449a91f08
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-mg_r93vd/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    16m8.994s
user    0m8.902s


The full log is available at
http://patchew.org/logs/20200703201911.26573-1-f4bug@amsat.org/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH 02/18] migration/vmstate: Introduce vmstate_no_state_to_migrate

[Peter Maydell]

On Fri, 3 Jul 2020 at 21:19, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Introduce a special state to indicate when an object doesn't
> have anything in its state to migrate.
>
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  include/migration/vmstate.h | 1 +
>  migration/vmstate.c         | 7 +++++++
>  2 files changed, 8 insertions(+)
>
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index af7d80cd4e..0922bc8efa 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -195,6 +195,7 @@ struct VMStateDescription {
>  };
>
>  extern const VMStateDescription vmstate_dummy; /* Exclusively for linux-user */
> +extern const VMStateDescription vmstate_no_state_to_migrate;
>
>  extern const VMStateInfo vmstate_info_bool;
>
> diff --git a/migration/vmstate.c b/migration/vmstate.c
> index bafa890384..d640cafad3 100644
> --- a/migration/vmstate.c
> +++ b/migration/vmstate.c
> @@ -20,6 +20,13 @@
>  #include "trace.h"
>  #include "qjson.h"
>
> +const VMStateDescription vmstate_no_state_to_migrate = {
> +    .name = "empty-state",
> +    .fields = (VMStateField[]) {
> +        VMSTATE_END_OF_LIST()
> +    }
> +};

Does this definitely not put any data into the migration stream?
We don't want to change what's on the wire for machines that
use devices that start using this. (If it does by default, it
would be easy to make the migration code special case the
magic symbol to act like "no vmsd specified").

thanks
-- PMM

Re: [PATCH 04/18] hw/arm/armv7m: Mark the device with no migratable fields

[Peter Maydell]

On Fri, 3 Jul 2020 at 21:19, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> This device doesn't have fields to migrate. Be explicit
> by using vmstate_qdev_no_state_to_migrate.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/arm/armv7m.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
> index 3308211e9c..3f78fccc12 100644
> --- a/hw/arm/armv7m.c
> +++ b/hw/arm/armv7m.c
> @@ -366,6 +366,7 @@ static void bitband_class_init(ObjectClass *klass, void *data)
>      DeviceClass *dc = DEVICE_CLASS(klass);
>
>      dc->realize = bitband_realize;
> +    dc->vmsd = vmstate_qdev_no_state_to_migrate;
>      device_class_set_props(dc, bitband_properties);
>  }

The commit message implies that this is changing the
TYPE_ARMV7M device, but it's actually affecting TYPE_BITBAND.
Code change looks good.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH 08/18] hw/core/split-irq: Mark the device with no migratable fields

[Peter Maydell]

On Fri, 3 Jul 2020 at 21:19, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> This device doesn't have fields to migrate. Be explicit
> by using vmstate_qdev_no_state_to_migrate.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/core/split-irq.c | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [RFC PATCH 18/18] hw/core/qdev: Display warning for devices missing migration state

[Peter Maydell]

On Fri, 3 Jul 2020 at 21:19, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> When built with --enable-qdev-debug, QEMU displays warnings
> listing devices missing migration state:
>
>   $ qemu-system-arm -S -M spitz
>   qemu-system-arm: warning: missing migration state for type: 'pxa270-c0-arm-cpu'
>   qemu-system-arm: warning: missing migration state for type: 'serial'
>   qemu-system-arm: warning: missing migration state for type: 'pxa2xx-pcmcia'
>   qemu-system-arm: warning: missing migration state for type: 'pxa2xx-pcmcia'
>   qemu-system-arm: warning: missing migration state for type: 'pxa2xx-i2c-slave'
>   qemu-system-arm: warning: missing migration state for type: 'pxa2xx-i2c-slave'
>   qemu-system-arm: warning: missing migration state for type: 'ads7846'
>   qemu-system-arm: warning: missing migration state for type: 'max1111'
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> RFC because there might be something simpler than --enable-qdev-debug.

I think where we'd like to get to is installing a migration
blocker if the machine has any devices which don't have a vmsd.
But for that we'd need to be pretty sure we'd got all the devices
on machines where we care about migration, and we're clearly a
fair way from that (eg we need to do something about the
devices like the CPU which don't have a vmsd but handle their
migration some other way so they don't trigger the condition
for warning/migration-blocker).

I don't have a strong objection to this --enable-qdev-debug, I guess.
Another option halfway between this and a full migration-blocker
would be do a warn_report() for the relevant devices when savevm
tries to migrate them.

thanks
-- PMM

Re: [PATCH 00/18] hw: Mark the device with no migratable fields

[Peter Maydell]

On Fri, 3 Jul 2020 at 21:19, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> This is a proof-of-concept after chatting with Peter Maydell
> on IRC earlier.
>
> Introduce the vmstate_no_state_to_migrate structure, and
> a reference to it: vmstate_qdev_no_state_to_migrate.
> Use this reference in devices with no fields to migrate.
>
> This is useful to catch devices missing vmstate, such:
> - ads7846
> - mcf-uart
> - mcf-fec
> - versatile_i2c
> - ...
>
> I am not sure about:
> - gpex-pcihost

I think it's correct that this has no internal state:
the only interesting state is in the GPEXRootState, which
is a TYPE_GPEX_ROOT_DEVICE which migrates itself.

I made some comments on the "meaty" bits of the patchset,
and reviewed one or two of the "mark this device as
having no migration state" patches, but it doesn't seem
worth reviewing all of them until the migration submaintainers
have a chance to weigh in on whether they like the concept
(I expect they're busy right now with freeze-related stuff :-))

thanks
-- PMM

Re: [PATCH 01/18] migration/vmstate: Document vmstate_dummy

[Dr. David Alan Gilbert]

* Philippe Mathieu-Daudé (f4bug@amsat.org) wrote:
> vmstate_dummy is special and restricted to linux-user. See commit
> c71c3e99b8 ("Add a vmstate_dummy struct for CONFIG_USER_ONLY").
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  include/migration/vmstate.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index f68ed7db13..af7d80cd4e 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -194,7 +194,7 @@ struct VMStateDescription {
>      const VMStateDescription **subsections;
>  };
>  
> -extern const VMStateDescription vmstate_dummy;
> +extern const VMStateDescription vmstate_dummy; /* Exclusively for linux-user */

Which does mean it's a unfortunate name I guess for something so
specific.
(It's also very weird having a vmsd defined with no VMSTATE_END_OF_LIST
marker).

Dave

>  extern const VMStateInfo vmstate_info_bool;
>  
> -- 
> 2.21.3
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH 01/18] migration/vmstate: Document vmstate_dummy

[Daniel P. Berrangé]

On Fri, Jul 03, 2020 at 10:18:54PM +0200, Philippe Mathieu-Daudé wrote:
> vmstate_dummy is special and restricted to linux-user. See commit
> c71c3e99b8 ("Add a vmstate_dummy struct for CONFIG_USER_ONLY").
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  include/migration/vmstate.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index f68ed7db13..af7d80cd4e 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -194,7 +194,7 @@ struct VMStateDescription {
>      const VMStateDescription **subsections;
>  };
>  
> -extern const VMStateDescription vmstate_dummy;
> +extern const VMStateDescription vmstate_dummy; /* Exclusively for linux-user */

Originally in the commit mentioned above, this was enforced at build time:

  +#ifdef CONFIG_USER_ONLY
  +extern const VMStateDescription vmstate_dummy;
  +#endif


but this was removed in

  commit 6afc14e92ac81b29c25f097468f7751d5df1b5bc
  Author: Stefan Weil <sw@weilnetz.de>
  Date:   Fri Feb 6 22:43:10 2015 +0100

    migration: Fix warning caused by missing declaration of vmstate_dummy
    
    Warning from the Sparse static analysis tool:
    
    stubs/vmstate.c:4:26: warning:
     symbol 'vmstate_dummy' was not declared. Should it be static?


So if this is really intended to only be used by linux-user, then I
suggest we put CONFIG_USER_ONLY back, and figure out a different
way to address the undeclared symbol problem. I guess the problem
was that stub code is shared both both user/softmmu builds. So
perhaps we need to stub this in linux-user only code, instead of
having it in the main stub library that is common.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 01/18] migration/vmstate: Document vmstate_dummy

[Philippe Mathieu-Daudé]

On 7/16/20 11:52 AM, Daniel P. Berrangé wrote:
> On Fri, Jul 03, 2020 at 10:18:54PM +0200, Philippe Mathieu-Daudé wrote:
>> vmstate_dummy is special and restricted to linux-user. See commit
>> c71c3e99b8 ("Add a vmstate_dummy struct for CONFIG_USER_ONLY").
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  include/migration/vmstate.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
>> index f68ed7db13..af7d80cd4e 100644
>> --- a/include/migration/vmstate.h
>> +++ b/include/migration/vmstate.h
>> @@ -194,7 +194,7 @@ struct VMStateDescription {
>>      const VMStateDescription **subsections;
>>  };
>>  
>> -extern const VMStateDescription vmstate_dummy;
>> +extern const VMStateDescription vmstate_dummy; /* Exclusively for linux-user */
> 
> Originally in the commit mentioned above, this was enforced at build time:
> 
>   +#ifdef CONFIG_USER_ONLY
>   +extern const VMStateDescription vmstate_dummy;
>   +#endif
> 
> 
> but this was removed in
> 
>   commit 6afc14e92ac81b29c25f097468f7751d5df1b5bc
>   Author: Stefan Weil <sw@weilnetz.de>
>   Date:   Fri Feb 6 22:43:10 2015 +0100
> 
>     migration: Fix warning caused by missing declaration of vmstate_dummy
>     
>     Warning from the Sparse static analysis tool:
>     
>     stubs/vmstate.c:4:26: warning:
>      symbol 'vmstate_dummy' was not declared. Should it be static?
> 
> 
> So if this is really intended to only be used by linux-user, then I
> suggest we put CONFIG_USER_ONLY back, and figure out a different
> way to address the undeclared symbol problem. I guess the problem
> was that stub code is shared both both user/softmmu builds. So
> perhaps we need to stub this in linux-user only code, instead of
> having it in the main stub library that is common.

Good idea, I'll look at it.

Thanks,

Phil.

Re: [PATCH 02/18] migration/vmstate: Introduce vmstate_no_state_to_migrate

[Dr. David Alan Gilbert]

* Peter Maydell (peter.maydell@linaro.org) wrote:
> On Fri, 3 Jul 2020 at 21:19, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> >
> > Introduce a special state to indicate when an object doesn't
> > have anything in its state to migrate.
> >
> > Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > ---
> >  include/migration/vmstate.h | 1 +
> >  migration/vmstate.c         | 7 +++++++
> >  2 files changed, 8 insertions(+)
> >
> > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> > index af7d80cd4e..0922bc8efa 100644
> > --- a/include/migration/vmstate.h
> > +++ b/include/migration/vmstate.h
> > @@ -195,6 +195,7 @@ struct VMStateDescription {
> >  };
> >
> >  extern const VMStateDescription vmstate_dummy; /* Exclusively for linux-user */
> > +extern const VMStateDescription vmstate_no_state_to_migrate;
> >
> >  extern const VMStateInfo vmstate_info_bool;
> >
> > diff --git a/migration/vmstate.c b/migration/vmstate.c
> > index bafa890384..d640cafad3 100644
> > --- a/migration/vmstate.c
> > +++ b/migration/vmstate.c
> > @@ -20,6 +20,13 @@
> >  #include "trace.h"
> >  #include "qjson.h"
> >
> > +const VMStateDescription vmstate_no_state_to_migrate = {
> > +    .name = "empty-state",
> > +    .fields = (VMStateField[]) {
> > +        VMSTATE_END_OF_LIST()
> > +    }
> > +};
> 
> Does this definitely not put any data into the migration stream?
> We don't want to change what's on the wire for machines that
> use devices that start using this. (If it does by default, it
> would be easy to make the migration code special case the
> magic symbol to act like "no vmsd specified").

I'd need to test it to be sure, but I think if we added a .needed
to vmstate_no_state_to_migrate with a function that always returned
false, then I think the stream would stay unchanged.

Dave

> thanks
> -- PMM
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH 00/18] hw: Mark the device with no migratable fields

[Philippe Mathieu-Daudé]

On 7/9/20 9:19 PM, Peter Maydell wrote:
> On Fri, 3 Jul 2020 at 21:19, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>
>> This is a proof-of-concept after chatting with Peter Maydell
>> on IRC earlier.
>>
>> Introduce the vmstate_no_state_to_migrate structure, and
>> a reference to it: vmstate_qdev_no_state_to_migrate.
>> Use this reference in devices with no fields to migrate.
>>
>> This is useful to catch devices missing vmstate, such:
>> - ads7846
>> - mcf-uart
>> - mcf-fec
>> - versatile_i2c
>> - ...
>>
>> I am not sure about:
>> - gpex-pcihost
> 
> I think it's correct that this has no internal state:
> the only interesting state is in the GPEXRootState, which
> is a TYPE_GPEX_ROOT_DEVICE which migrates itself.
> 
> I made some comments on the "meaty" bits of the patchset,
> and reviewed one or two of the "mark this device as
> having no migration state" patches, but it doesn't seem
> worth reviewing all of them until the migration submaintainers
> have a chance to weigh in on whether they like the concept
> (I expect they're busy right now with freeze-related stuff :-))

Now that we are far from freeze-date is a good time to ping
again on this concept :)

Most of the devices are ARM except:
- cpu-cluster (Eduardo/Marcel)
- hcd-ohci (Gerd)
- mac-nubus-bridge (Laurent)
- generic QOM (Daniel, Paolo)

Is someone against this proposal?

Regards,

Phil.

Re: [RFC PATCH 18/18] hw/core/qdev: Display warning for devices missing migration state

[Philippe Mathieu-Daudé]

On 7/9/20 9:14 PM, Peter Maydell wrote:
> On Fri, 3 Jul 2020 at 21:19, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>
>> When built with --enable-qdev-debug, QEMU displays warnings
>> listing devices missing migration state:
>>
>>   $ qemu-system-arm -S -M spitz
>>   qemu-system-arm: warning: missing migration state for type: 'pxa270-c0-arm-cpu'
>>   qemu-system-arm: warning: missing migration state for type: 'serial'
>>   qemu-system-arm: warning: missing migration state for type: 'pxa2xx-pcmcia'
>>   qemu-system-arm: warning: missing migration state for type: 'pxa2xx-pcmcia'
>>   qemu-system-arm: warning: missing migration state for type: 'pxa2xx-i2c-slave'
>>   qemu-system-arm: warning: missing migration state for type: 'pxa2xx-i2c-slave'
>>   qemu-system-arm: warning: missing migration state for type: 'ads7846'
>>   qemu-system-arm: warning: missing migration state for type: 'max1111'
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> RFC because there might be something simpler than --enable-qdev-debug.
> 
> I think where we'd like to get to is installing a migration
> blocker if the machine has any devices which don't have a vmsd.
> But for that we'd need to be pretty sure we'd got all the devices
> on machines where we care about migration, and we're clearly a
> fair way from that (eg we need to do something about the
> devices like the CPU which don't have a vmsd but handle their
> migration some other way so they don't trigger the condition
> for warning/migration-blocker).

Dave made a comment about it, I'd rather let him have a look.

> I don't have a strong objection to this --enable-qdev-debug, I guess.
> Another option halfway between this and a full migration-blocker
> would be do a warn_report() for the relevant devices when savevm
> tries to migrate them.

OK. The problem is vmstate_save_state() is not qdev specific, it
migrates a blob, which we can not report much about. I'll repost
using 2 warnings.

Thanks for your review,

Phil.

Re: [PATCH 00/18] hw: Mark the device with no migratable fields

[Laurent Vivier]

Le 14/01/2021 à 16:49, Philippe Mathieu-Daudé a écrit :
> On 7/9/20 9:19 PM, Peter Maydell wrote:
>> On Fri, 3 Jul 2020 at 21:19, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>>
>>> This is a proof-of-concept after chatting with Peter Maydell
>>> on IRC earlier.
>>>
>>> Introduce the vmstate_no_state_to_migrate structure, and
>>> a reference to it: vmstate_qdev_no_state_to_migrate.
>>> Use this reference in devices with no fields to migrate.
>>>
>>> This is useful to catch devices missing vmstate, such:
>>> - ads7846
>>> - mcf-uart
>>> - mcf-fec
>>> - versatile_i2c
>>> - ...
>>>
>>> I am not sure about:
>>> - gpex-pcihost
>>
>> I think it's correct that this has no internal state:
>> the only interesting state is in the GPEXRootState, which
>> is a TYPE_GPEX_ROOT_DEVICE which migrates itself.
>>
>> I made some comments on the "meaty" bits of the patchset,
>> and reviewed one or two of the "mark this device as
>> having no migration state" patches, but it doesn't seem
>> worth reviewing all of them until the migration submaintainers
>> have a chance to weigh in on whether they like the concept
>> (I expect they're busy right now with freeze-related stuff :-))
> 
> Now that we are far from freeze-date is a good time to ping
> again on this concept :)
> 
> Most of the devices are ARM except:
> - cpu-cluster (Eduardo/Marcel)
> - hcd-ohci (Gerd)
> - mac-nubus-bridge (Laurent)
> - generic QOM (Daniel, Paolo)
> 
> Is someone against this proposal?

I'm not against the proposal, but I don't understand why we need this.

Thanks,
Laurent

Re: [PATCH 00/18] hw: Mark the device with no migratable fields

[Philippe Mathieu-Daudé]

Hi Laurent,

On 1/18/21 8:33 AM, Laurent Vivier wrote:
> Le 14/01/2021 à 16:49, Philippe Mathieu-Daudé a écrit :
>> On 7/9/20 9:19 PM, Peter Maydell wrote:
>>> On Fri, 3 Jul 2020 at 21:19, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>>>
>>>> This is a proof-of-concept after chatting with Peter Maydell
>>>> on IRC earlier.
>>>>
>>>> Introduce the vmstate_no_state_to_migrate structure, and
>>>> a reference to it: vmstate_qdev_no_state_to_migrate.
>>>> Use this reference in devices with no fields to migrate.
>>>>
>>>> This is useful to catch devices missing vmstate, such:
>>>> - ads7846
>>>> - mcf-uart
>>>> - mcf-fec
>>>> - versatile_i2c
>>>> - ...
>>>>
>>>> I am not sure about:
>>>> - gpex-pcihost
>>>
>>> I think it's correct that this has no internal state:
>>> the only interesting state is in the GPEXRootState, which
>>> is a TYPE_GPEX_ROOT_DEVICE which migrates itself.
>>>
>>> I made some comments on the "meaty" bits of the patchset,
>>> and reviewed one or two of the "mark this device as
>>> having no migration state" patches, but it doesn't seem
>>> worth reviewing all of them until the migration submaintainers
>>> have a chance to weigh in on whether they like the concept
>>> (I expect they're busy right now with freeze-related stuff :-))
>>
>> Now that we are far from freeze-date is a good time to ping
>> again on this concept :)
>>
>> Most of the devices are ARM except:
>> - cpu-cluster (Eduardo/Marcel)
>> - hcd-ohci (Gerd)
>> - mac-nubus-bridge (Laurent)
>> - generic QOM (Daniel, Paolo)
>>
>> Is someone against this proposal?
> 
> I'm not against the proposal, but I don't understand why we need this.

IIRC the IRC discussion followed this thread:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg554453.html

Quoting Peter:

> I think we should care about migration on all architectures
> and devices, in the sense that we want savevm/loadvm to work.
> This is a really useful debugging and user tool, and when
> I'm reviewing devices it's the minimum bar I think new
> devices should clear. You then get migration "for free" but
> I don't particularly expect it to be used compared to
> snapshot save/restore. (Of course some of our existing code
> doesn't support this, and we don't have a good way of testing
> so bugs creep in easily, but as a principle I think it's
> good.)

Currently there is no automatic way to catch missing vmstate,
we rely on code review (mostly from Peter...).

To be able to add a code check to catch the future device added,
we need to first clean the (old) devices missing VMState, justifying
why each doesn't have any field to migrate.

Also IMO it is simpler to have an unified API, rather than explaining
each experienced and new contributor why "old style qdev" are allowed
to do things than "new introduced qdev" can't do anymore.

Regards,

Phil.