diff mbox series

[RFC,v2,02/20] hw/core/qdev: Add vmstate_qdev_no_state_to_migrate

Message ID 20210117192446.23753-3-f4bug@amsat.org (mailing list archive)
State New, archived
Headers show
Series hw: Mark the device with no migratable fields | expand

Commit Message

Philippe Mathieu-Daudé Jan. 17, 2021, 7:24 p.m. UTC
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>
---
Unresolved issues:

https://www.mail-archive.com/qemu-devel@nongnu.org/msg721695.html
Peter:
> 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").

https://www.mail-archive.com/qemu-devel@nongnu.org/msg727634.html
Dave:
> 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.
---
 include/hw/qdev-core.h      | 2 ++
 include/migration/vmstate.h | 1 +
 hw/core/qdev.c              | 3 +++
 migration/vmstate.c         | 7 +++++++
 stubs/vmstate.c             | 7 +++++++
 5 files changed, 20 insertions(+)

Comments

Dr. David Alan Gilbert Jan. 19, 2021, 9:31 a.m. UTC | #1
* Philippe Mathieu-Daudé (f4bug@amsat.org) wrote:
> 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>
> ---
> Unresolved issues:
> 
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg721695.html
> Peter:
> > 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").
> 
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg727634.html
> Dave:
> > 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.

Yes I still think you need that; if you only use this for base classes
rather than devices themselves you're probably OK; but if you use it on
a device I think you'll end up with an empty-state entry in the
migration stream.

Dave

> ---
>  include/hw/qdev-core.h      | 2 ++
>  include/migration/vmstate.h | 1 +
>  hw/core/qdev.c              | 3 +++
>  migration/vmstate.c         | 7 +++++++
>  stubs/vmstate.c             | 7 +++++++
>  5 files changed, 20 insertions(+)
> 
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index bafc311bfa1..d2c7a46e6a2 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -140,6 +140,8 @@ struct DeviceClass {
>      const char *bus_type;
>  };
>  
> +extern const VMStateDescription *vmstate_qdev_no_state_to_migrate;
> +
>  typedef struct NamedGPIOList NamedGPIOList;
>  
>  struct NamedGPIOList {
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index dda65c9987d..50559598eac 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -197,6 +197,7 @@ struct VMStateDescription {
>  #if defined(CONFIG_USER_ONLY)
>  extern const VMStateDescription vmstate_user_mode_cpu_dummy;
>  #endif
> +extern const VMStateDescription vmstate_no_state_to_migrate;
>  
>  extern const VMStateInfo vmstate_info_bool;
>  
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index cefc5eaa0a9..f0d0afd438d 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -44,6 +44,9 @@
>  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);
> diff --git a/migration/vmstate.c b/migration/vmstate.c
> index 05f87cdddc5..2c373774dfa 100644
> --- a/migration/vmstate.c
> +++ b/migration/vmstate.c
> @@ -20,6 +20,13 @@
>  #include "qemu/error-report.h"
>  #include "trace.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, JSONWriter *vmdesc);
>  static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
> diff --git a/stubs/vmstate.c b/stubs/vmstate.c
> index 8da777a1fb4..f561f9f39bd 100644
> --- a/stubs/vmstate.c
> +++ b/stubs/vmstate.c
> @@ -5,6 +5,13 @@
>  const VMStateDescription vmstate_user_mode_cpu_dummy = {};
>  #endif
>  
> +const VMStateDescription vmstate_no_state_to_migrate = {
> +    .name = "empty-state",
> +    .fields = (VMStateField[]) {
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  int vmstate_register_with_alias_id(VMStateIf *obj,
>                                     uint32_t instance_id,
>                                     const VMStateDescription *vmsd,
> -- 
> 2.26.2
>
Peter Maydell Jan. 19, 2021, 1:31 p.m. UTC | #2
On Sun, 17 Jan 2021 at 19:25, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> 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>
> ---
> Unresolved issues:
>
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg721695.html
> Peter:
> > 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").
>
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg727634.html
> Dave:
> > 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.
> ---

It should be easy to test -- just do a 'savevm' of a running
system with a machine model that uses one of the devices that
has been marked as "no state to migrate", then apply the patchseries,
and see if 'loadvm' works or not.

thanks
-- PMM
diff mbox series

Patch

diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index bafc311bfa1..d2c7a46e6a2 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -140,6 +140,8 @@  struct DeviceClass {
     const char *bus_type;
 };
 
+extern const VMStateDescription *vmstate_qdev_no_state_to_migrate;
+
 typedef struct NamedGPIOList NamedGPIOList;
 
 struct NamedGPIOList {
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index dda65c9987d..50559598eac 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -197,6 +197,7 @@  struct VMStateDescription {
 #if defined(CONFIG_USER_ONLY)
 extern const VMStateDescription vmstate_user_mode_cpu_dummy;
 #endif
+extern const VMStateDescription vmstate_no_state_to_migrate;
 
 extern const VMStateInfo vmstate_info_bool;
 
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index cefc5eaa0a9..f0d0afd438d 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -44,6 +44,9 @@ 
 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);
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 05f87cdddc5..2c373774dfa 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -20,6 +20,13 @@ 
 #include "qemu/error-report.h"
 #include "trace.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, JSONWriter *vmdesc);
 static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
diff --git a/stubs/vmstate.c b/stubs/vmstate.c
index 8da777a1fb4..f561f9f39bd 100644
--- a/stubs/vmstate.c
+++ b/stubs/vmstate.c
@@ -5,6 +5,13 @@ 
 const VMStateDescription vmstate_user_mode_cpu_dummy = {};
 #endif
 
+const VMStateDescription vmstate_no_state_to_migrate = {
+    .name = "empty-state",
+    .fields = (VMStateField[]) {
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 int vmstate_register_with_alias_id(VMStateIf *obj,
                                    uint32_t instance_id,
                                    const VMStateDescription *vmsd,