[Qemu-devel] [PATCH] s390/stattrib: Make SaveVMHandlers data static

Dr. David Alan Gilbert (git) posted 1 patch 6 years, 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180212154903.8859-1-dgilbert@redhat.com
Test checkpatch passed
Test docker-build@min-glib passed
Test docker-mingw@fedora passed
Test docker-quick@centos6 passed
Test ppcbe passed
Test ppcle passed
Test s390x passed
hw/s390x/s390-stattrib.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
[Qemu-devel] [PATCH] s390/stattrib: Make SaveVMHandlers data static
Posted by Dr. David Alan Gilbert (git) 6 years, 2 months ago
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

There's no need for this to be dynamic, make it static.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 hw/s390x/s390-stattrib.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/hw/s390x/s390-stattrib.c b/hw/s390x/s390-stattrib.c
index a1d2135a60..adf07ef312 100644
--- a/hw/s390x/s390-stattrib.c
+++ b/hw/s390x/s390-stattrib.c
@@ -365,22 +365,22 @@ static inline void s390_stattrib_set_migration_enabled(Object *obj, bool value,
     s->migration_enabled = value;
 }
 
+static SaveVMHandlers savevm_s390_stattrib_handlers = {
+    .save_setup = cmma_save_setup,
+    .save_live_iterate = cmma_save_iterate,
+    .save_live_complete_precopy = cmma_save_complete,
+    .save_live_pending = cmma_save_pending,
+    .save_cleanup = cmma_save_cleanup,
+    .load_state = cmma_load,
+    .is_active = cmma_active,
+};
+
 static void s390_stattrib_instance_init(Object *obj)
 {
     S390StAttribState *sas = S390_STATTRIB(obj);
-    SaveVMHandlers *ops;
-
-    /* ops will always be freed by qemu when unregistering */
-    ops = g_new0(SaveVMHandlers, 1);
-
-    ops->save_setup = cmma_save_setup;
-    ops->save_live_iterate = cmma_save_iterate;
-    ops->save_live_complete_precopy = cmma_save_complete;
-    ops->save_live_pending = cmma_save_pending;
-    ops->save_cleanup = cmma_save_cleanup;
-    ops->load_state = cmma_load;
-    ops->is_active = cmma_active;
-    register_savevm_live(NULL, TYPE_S390_STATTRIB, 0, 0, ops, sas);
+
+    register_savevm_live(NULL, TYPE_S390_STATTRIB, 0, 0,
+                         &savevm_s390_stattrib_handlers, sas);
 
     object_property_add_bool(obj, "migration-enabled",
                              s390_stattrib_get_migration_enabled,
-- 
2.14.3


Re: [Qemu-devel] [PATCH] s390/stattrib: Make SaveVMHandlers data static
Posted by David Hildenbrand 6 years, 2 months ago
On 12.02.2018 16:49, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> There's no need for this to be dynamic, make it static.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  hw/s390x/s390-stattrib.c | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/s390x/s390-stattrib.c b/hw/s390x/s390-stattrib.c
> index a1d2135a60..adf07ef312 100644
> --- a/hw/s390x/s390-stattrib.c
> +++ b/hw/s390x/s390-stattrib.c
> @@ -365,22 +365,22 @@ static inline void s390_stattrib_set_migration_enabled(Object *obj, bool value,
>      s->migration_enabled = value;
>  }
>  
> +static SaveVMHandlers savevm_s390_stattrib_handlers = {
> +    .save_setup = cmma_save_setup,
> +    .save_live_iterate = cmma_save_iterate,
> +    .save_live_complete_precopy = cmma_save_complete,
> +    .save_live_pending = cmma_save_pending,
> +    .save_cleanup = cmma_save_cleanup,
> +    .load_state = cmma_load,
> +    .is_active = cmma_active,
> +};
> +
>  static void s390_stattrib_instance_init(Object *obj)
>  {
>      S390StAttribState *sas = S390_STATTRIB(obj);
> -    SaveVMHandlers *ops;
> -
> -    /* ops will always be freed by qemu when unregistering */
> -    ops = g_new0(SaveVMHandlers, 1);
> -
> -    ops->save_setup = cmma_save_setup;
> -    ops->save_live_iterate = cmma_save_iterate;
> -    ops->save_live_complete_precopy = cmma_save_complete;
> -    ops->save_live_pending = cmma_save_pending;
> -    ops->save_cleanup = cmma_save_cleanup;
> -    ops->load_state = cmma_load;
> -    ops->is_active = cmma_active;
> -    register_savevm_live(NULL, TYPE_S390_STATTRIB, 0, 0, ops, sas);
> +
> +    register_savevm_live(NULL, TYPE_S390_STATTRIB, 0, 0,
> +                         &savevm_s390_stattrib_handlers, sas);
>  
>      object_property_add_bool(obj, "migration-enabled",
>                               s390_stattrib_get_migration_enabled,
> 

Looks sane to me.

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH] s390/stattrib: Make SaveVMHandlers data static
Posted by Claudio Imbrenda 6 years, 2 months ago
On Mon, 12 Feb 2018 16:52:08 +0100
David Hildenbrand <david@redhat.com> wrote:

> On 12.02.2018 16:49, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > There's no need for this to be dynamic, make it static.
> > 
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> >  hw/s390x/s390-stattrib.c | 26 +++++++++++++-------------
> >  1 file changed, 13 insertions(+), 13 deletions(-)
> > 
> > diff --git a/hw/s390x/s390-stattrib.c b/hw/s390x/s390-stattrib.c
> > index a1d2135a60..adf07ef312 100644
> > --- a/hw/s390x/s390-stattrib.c
> > +++ b/hw/s390x/s390-stattrib.c
> > @@ -365,22 +365,22 @@ static inline void
> > s390_stattrib_set_migration_enabled(Object *obj, bool value,
> > s->migration_enabled = value; }
> >  
> > +static SaveVMHandlers savevm_s390_stattrib_handlers = {
> > +    .save_setup = cmma_save_setup,
> > +    .save_live_iterate = cmma_save_iterate,
> > +    .save_live_complete_precopy = cmma_save_complete,
> > +    .save_live_pending = cmma_save_pending,
> > +    .save_cleanup = cmma_save_cleanup,
> > +    .load_state = cmma_load,
> > +    .is_active = cmma_active,
> > +};
> > +
> >  static void s390_stattrib_instance_init(Object *obj)
> >  {
> >      S390StAttribState *sas = S390_STATTRIB(obj);
> > -    SaveVMHandlers *ops;
> > -
> > -    /* ops will always be freed by qemu when unregistering */
> > -    ops = g_new0(SaveVMHandlers, 1);
> > -
> > -    ops->save_setup = cmma_save_setup;
> > -    ops->save_live_iterate = cmma_save_iterate;
> > -    ops->save_live_complete_precopy = cmma_save_complete;
> > -    ops->save_live_pending = cmma_save_pending;
> > -    ops->save_cleanup = cmma_save_cleanup;
> > -    ops->load_state = cmma_load;
> > -    ops->is_active = cmma_active;
> > -    register_savevm_live(NULL, TYPE_S390_STATTRIB, 0, 0, ops, sas);
> > +
> > +    register_savevm_live(NULL, TYPE_S390_STATTRIB, 0, 0,
> > +                         &savevm_s390_stattrib_handlers, sas);
> >  
> >      object_property_add_bool(obj, "migration-enabled",
> >                               s390_stattrib_get_migration_enabled,
> >   
> 
> Looks sane to me.
> 
> Reviewed-by: David Hildenbrand <david@redhat.com>


Reviewed-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>


Re: [Qemu-devel] [PATCH] s390/stattrib: Make SaveVMHandlers data static
Posted by Thomas Huth 6 years, 2 months ago
On 12.02.2018 16:49, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> There's no need for this to be dynamic, make it static.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  hw/s390x/s390-stattrib.c | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/s390x/s390-stattrib.c b/hw/s390x/s390-stattrib.c
> index a1d2135a60..adf07ef312 100644
> --- a/hw/s390x/s390-stattrib.c
> +++ b/hw/s390x/s390-stattrib.c
> @@ -365,22 +365,22 @@ static inline void s390_stattrib_set_migration_enabled(Object *obj, bool value,
>      s->migration_enabled = value;
>  }
>  
> +static SaveVMHandlers savevm_s390_stattrib_handlers = {
> +    .save_setup = cmma_save_setup,
> +    .save_live_iterate = cmma_save_iterate,
> +    .save_live_complete_precopy = cmma_save_complete,
> +    .save_live_pending = cmma_save_pending,
> +    .save_cleanup = cmma_save_cleanup,
> +    .load_state = cmma_load,
> +    .is_active = cmma_active,
> +};
> +
>  static void s390_stattrib_instance_init(Object *obj)
>  {
>      S390StAttribState *sas = S390_STATTRIB(obj);
> -    SaveVMHandlers *ops;
> -
> -    /* ops will always be freed by qemu when unregistering */
> -    ops = g_new0(SaveVMHandlers, 1);
> -
> -    ops->save_setup = cmma_save_setup;
> -    ops->save_live_iterate = cmma_save_iterate;
> -    ops->save_live_complete_precopy = cmma_save_complete;
> -    ops->save_live_pending = cmma_save_pending;
> -    ops->save_cleanup = cmma_save_cleanup;
> -    ops->load_state = cmma_load;
> -    ops->is_active = cmma_active;
> -    register_savevm_live(NULL, TYPE_S390_STATTRIB, 0, 0, ops, sas);
> +
> +    register_savevm_live(NULL, TYPE_S390_STATTRIB, 0, 0,
> +                         &savevm_s390_stattrib_handlers, sas);
>  
>      object_property_add_bool(obj, "migration-enabled",
>                               s390_stattrib_get_migration_enabled,
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [PATCH] s390/stattrib: Make SaveVMHandlers data static
Posted by Richard Henderson 6 years, 2 months ago
On 02/12/2018 07:49 AM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> There's no need for this to be dynamic, make it static.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  hw/s390x/s390-stattrib.c | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/s390x/s390-stattrib.c b/hw/s390x/s390-stattrib.c
> index a1d2135a60..adf07ef312 100644
> --- a/hw/s390x/s390-stattrib.c
> +++ b/hw/s390x/s390-stattrib.c
> @@ -365,22 +365,22 @@ static inline void s390_stattrib_set_migration_enabled(Object *obj, bool value,
>      s->migration_enabled = value;
>  }
>  
> +static SaveVMHandlers savevm_s390_stattrib_handlers = {
> +    .save_setup = cmma_save_setup,
> +    .save_live_iterate = cmma_save_iterate,
> +    .save_live_complete_precopy = cmma_save_complete,
> +    .save_live_pending = cmma_save_pending,
> +    .save_cleanup = cmma_save_cleanup,
> +    .load_state = cmma_load,
> +    .is_active = cmma_active,
> +};

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Bonus points for the follow-on to make this const.  I realize that
register_savevm_live is not currently prepared for this, so pushing the
qualifier down will take some doing.


r~

Re: [Qemu-devel] [PATCH] s390/stattrib: Make SaveVMHandlers data static
Posted by Cornelia Huck 6 years, 2 months ago
On Mon, 12 Feb 2018 15:49:03 +0000
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:

> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> There's no need for this to be dynamic, make it static.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  hw/s390x/s390-stattrib.c | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)

Thanks, applied.