diff mbox series

[RFC,v2,01/20] migration/vmstate: Restrict vmstate_dummy to user-mode

Message ID 20210117192446.23753-2-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
'vmstate_dummy' is special and only used for user-mode. Rename
it to something more specific.
It was introduced restricted to user-mode in commit c71c3e99b8
("Add a vmstate_dummy struct for CONFIG_USER_ONLY") but this
restriction was later removed in commit 6afc14e92ac ("migration:
Fix warning caused by missing declaration of vmstate_dummy").
Avoid the missing declaration warning by adding a stub for the
symbol, and restore the #ifdef'ry.

Suggested-by: Daniel Berrangé <berrange@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/core/cpu.h       | 2 +-
 include/migration/vmstate.h | 4 +++-
 stubs/vmstate.c             | 4 +++-
 3 files changed, 7 insertions(+), 3 deletions(-)

Comments

Dr. David Alan Gilbert Jan. 18, 2021, 11:48 a.m. UTC | #1
* Philippe Mathieu-Daudé (f4bug@amsat.org) wrote:
> 'vmstate_dummy' is special and only used for user-mode. Rename
> it to something more specific.
> It was introduced restricted to user-mode in commit c71c3e99b8
> ("Add a vmstate_dummy struct for CONFIG_USER_ONLY") but this
> restriction was later removed in commit 6afc14e92ac ("migration:
> Fix warning caused by missing declaration of vmstate_dummy").
> Avoid the missing declaration warning by adding a stub for the
> symbol, and restore the #ifdef'ry.
> 
> Suggested-by: Daniel Berrangé <berrange@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

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

> ---
>  include/hw/core/cpu.h       | 2 +-
>  include/migration/vmstate.h | 4 +++-
>  stubs/vmstate.c             | 4 +++-
>  3 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 140fa32a5e3..c79a58db9b9 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -1132,7 +1132,7 @@ bool target_words_bigendian(void);
>  #ifdef CONFIG_SOFTMMU
>  extern const VMStateDescription vmstate_cpu_common;
>  #else
> -#define vmstate_cpu_common vmstate_dummy
> +#define vmstate_cpu_common vmstate_user_mode_cpu_dummy
>  #endif
>  
>  #define VMSTATE_CPU() {                                                     \
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index 075ee800960..dda65c9987d 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -194,7 +194,9 @@ struct VMStateDescription {
>      const VMStateDescription **subsections;
>  };
>  
> -extern const VMStateDescription vmstate_dummy;
> +#if defined(CONFIG_USER_ONLY)
> +extern const VMStateDescription vmstate_user_mode_cpu_dummy;
> +#endif
>  
>  extern const VMStateInfo vmstate_info_bool;
>  
> diff --git a/stubs/vmstate.c b/stubs/vmstate.c
> index cc4fe41dfc2..8da777a1fb4 100644
> --- a/stubs/vmstate.c
> +++ b/stubs/vmstate.c
> @@ -1,7 +1,9 @@
>  #include "qemu/osdep.h"
>  #include "migration/vmstate.h"
>  
> -const VMStateDescription vmstate_dummy = {};
> +#if defined(CONFIG_USER_ONLY)
> +const VMStateDescription vmstate_user_mode_cpu_dummy = {};
> +#endif
>  
>  int vmstate_register_with_alias_id(VMStateIf *obj,
>                                     uint32_t instance_id,
> -- 
> 2.26.2
>
Peter Maydell Jan. 19, 2021, 1:50 p.m. UTC | #2
On Sun, 17 Jan 2021 at 19:24, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> 'vmstate_dummy' is special and only used for user-mode. Rename
> it to something more specific.
> It was introduced restricted to user-mode in commit c71c3e99b8
> ("Add a vmstate_dummy struct for CONFIG_USER_ONLY") but this
> restriction was later removed in commit 6afc14e92ac ("migration:
> Fix warning caused by missing declaration of vmstate_dummy").
> Avoid the missing declaration warning by adding a stub for the
> symbol, and restore the #ifdef'ry.

So what is the actual use of vmstate_dummy ? I had a grep
through and as far as I can see the points where vmstate_cpu_common
is used are all in softmmu-only code. I tried this patch
and QEMU seems to compile OK:

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 140fa32a5e3..a827417a4d8 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -1131,8 +1131,6 @@ bool target_words_bigendian(void);

 #ifdef CONFIG_SOFTMMU
 extern const VMStateDescription vmstate_cpu_common;
-#else
-#define vmstate_cpu_common vmstate_dummy
 #endif

 #define VMSTATE_CPU() {                                                     \
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 075ee800960..8df7b69f389 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -194,8 +194,6 @@ struct VMStateDescription {
     const VMStateDescription **subsections;
 };

-extern const VMStateDescription vmstate_dummy;
-
 extern const VMStateInfo vmstate_info_bool;

 extern const VMStateInfo vmstate_info_int8;
diff --git a/stubs/vmstate.c b/stubs/vmstate.c
index cc4fe41dfc2..8513d9204e4 100644
--- a/stubs/vmstate.c
+++ b/stubs/vmstate.c
@@ -1,8 +1,6 @@
 #include "qemu/osdep.h"
 #include "migration/vmstate.h"

-const VMStateDescription vmstate_dummy = {};
-
 int vmstate_register_with_alias_id(VMStateIf *obj,
                                    uint32_t instance_id,
                                    const VMStateDescription *vmsd,

thanks
-- PMM
Philippe Mathieu-Daudé Jan. 19, 2021, 4:37 p.m. UTC | #3
On 1/19/21 2:50 PM, Peter Maydell wrote:
> On Sun, 17 Jan 2021 at 19:24, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>
>> 'vmstate_dummy' is special and only used for user-mode. Rename
>> it to something more specific.
>> It was introduced restricted to user-mode in commit c71c3e99b8
>> ("Add a vmstate_dummy struct for CONFIG_USER_ONLY") but this
>> restriction was later removed in commit 6afc14e92ac ("migration:
>> Fix warning caused by missing declaration of vmstate_dummy").
>> Avoid the missing declaration warning by adding a stub for the
>> symbol, and restore the #ifdef'ry.
> 
> So what is the actual use of vmstate_dummy ? I had a grep
> through and as far as I can see the points where vmstate_cpu_common
> is used are all in softmmu-only code.

No clue, maybe simply remnant from unfinished work?

> I tried this patch
> and QEMU seems to compile OK:
> 
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 140fa32a5e3..a827417a4d8 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -1131,8 +1131,6 @@ bool target_words_bigendian(void);
> 
>  #ifdef CONFIG_SOFTMMU
>  extern const VMStateDescription vmstate_cpu_common;
> -#else
> -#define vmstate_cpu_common vmstate_dummy
>  #endif
> 
>  #define VMSTATE_CPU() {                                                     \

Great! Maybe even restricting VMSTATE_CPU() to softmmu-only:

-- >8 --
@@ -1131,9 +1131,6 @@ bool target_words_bigendian(void);

 #ifdef CONFIG_SOFTMMU
 extern const VMStateDescription vmstate_cpu_common;
-#else
-#define vmstate_cpu_common vmstate_dummy
-#endif

 #define VMSTATE_CPU() {
     \
     .name = "parent_obj",
     \
@@ -1142,6 +1139,7 @@ extern const VMStateDescription vmstate_cpu_common;
     .flags = VMS_STRUCT,
     \
     .offset = 0,
     \
 }
+#endif

 #endif /* NEED_CPU_H */
---

I'll wait if David/Juan have any comment, else respin based
on your patch.

Thanks,

Phil.
Dr. David Alan Gilbert Jan. 20, 2021, 11:03 a.m. UTC | #4
* Philippe Mathieu-Daudé (f4bug@amsat.org) wrote:
> On 1/19/21 2:50 PM, Peter Maydell wrote:
> > On Sun, 17 Jan 2021 at 19:24, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> >>
> >> 'vmstate_dummy' is special and only used for user-mode. Rename
> >> it to something more specific.
> >> It was introduced restricted to user-mode in commit c71c3e99b8
> >> ("Add a vmstate_dummy struct for CONFIG_USER_ONLY") but this
> >> restriction was later removed in commit 6afc14e92ac ("migration:
> >> Fix warning caused by missing declaration of vmstate_dummy").
> >> Avoid the missing declaration warning by adding a stub for the
> >> symbol, and restore the #ifdef'ry.
> > 
> > So what is the actual use of vmstate_dummy ? I had a grep
> > through and as far as I can see the points where vmstate_cpu_common
> > is used are all in softmmu-only code.
> 
> No clue, maybe simply remnant from unfinished work?

Not sure either; but it looks like Paolo fixed some of it up in d9f24bf5724
a few months ago; prior to that cpu_exec_unrealizefn used it even on a
USER_ONLY build.

Dave

> > I tried this patch
> > and QEMU seems to compile OK:
> > 
> > diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> > index 140fa32a5e3..a827417a4d8 100644
> > --- a/include/hw/core/cpu.h
> > +++ b/include/hw/core/cpu.h
> > @@ -1131,8 +1131,6 @@ bool target_words_bigendian(void);
> > 
> >  #ifdef CONFIG_SOFTMMU
> >  extern const VMStateDescription vmstate_cpu_common;
> > -#else
> > -#define vmstate_cpu_common vmstate_dummy
> >  #endif
> > 
> >  #define VMSTATE_CPU() {                                                     \
> 
> Great! Maybe even restricting VMSTATE_CPU() to softmmu-only:
> 
> -- >8 --
> @@ -1131,9 +1131,6 @@ bool target_words_bigendian(void);
> 
>  #ifdef CONFIG_SOFTMMU
>  extern const VMStateDescription vmstate_cpu_common;
> -#else
> -#define vmstate_cpu_common vmstate_dummy
> -#endif
> 
>  #define VMSTATE_CPU() {
>      \
>      .name = "parent_obj",
>      \
> @@ -1142,6 +1139,7 @@ extern const VMStateDescription vmstate_cpu_common;
>      .flags = VMS_STRUCT,
>      \
>      .offset = 0,
>      \
>  }
> +#endif
> 
>  #endif /* NEED_CPU_H */
> ---
> 
> I'll wait if David/Juan have any comment, else respin based
> on your patch.
> 
> Thanks,
> 
> Phil.
>
diff mbox series

Patch

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 140fa32a5e3..c79a58db9b9 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -1132,7 +1132,7 @@  bool target_words_bigendian(void);
 #ifdef CONFIG_SOFTMMU
 extern const VMStateDescription vmstate_cpu_common;
 #else
-#define vmstate_cpu_common vmstate_dummy
+#define vmstate_cpu_common vmstate_user_mode_cpu_dummy
 #endif
 
 #define VMSTATE_CPU() {                                                     \
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 075ee800960..dda65c9987d 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -194,7 +194,9 @@  struct VMStateDescription {
     const VMStateDescription **subsections;
 };
 
-extern const VMStateDescription vmstate_dummy;
+#if defined(CONFIG_USER_ONLY)
+extern const VMStateDescription vmstate_user_mode_cpu_dummy;
+#endif
 
 extern const VMStateInfo vmstate_info_bool;
 
diff --git a/stubs/vmstate.c b/stubs/vmstate.c
index cc4fe41dfc2..8da777a1fb4 100644
--- a/stubs/vmstate.c
+++ b/stubs/vmstate.c
@@ -1,7 +1,9 @@ 
 #include "qemu/osdep.h"
 #include "migration/vmstate.h"
 
-const VMStateDescription vmstate_dummy = {};
+#if defined(CONFIG_USER_ONLY)
+const VMStateDescription vmstate_user_mode_cpu_dummy = {};
+#endif
 
 int vmstate_register_with_alias_id(VMStateIf *obj,
                                    uint32_t instance_id,