qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V4] migration: simplify notifiers
@ 2023-10-18 13:40 Steve Sistare
  2023-10-18 13:41 ` Steven Sistare
  0 siblings, 1 reply; 2+ messages in thread
From: Steve Sistare @ 2023-10-18 13:40 UTC (permalink / raw)
  To: qemu-devel
  Cc: Juan Quintela, Michael S. Tsirkin, Alex Williamson, Fabiano Rosas,
	Leonardo Bras, Steve Sistare

Pass the callback function to add_migration_state_change_notifier so
that migration can initialize the notifier on add and clear it on
delete, which simplifies the call sites.  Shorten the function names
so the extra arg can be added more legibly.  Hide the global notifier
list in a new function migration_call_notifiers, and make it externally
visible so future live update code can call it.

No functional change.

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
Tested-by: Michael Galaxy <mgalaxy@akamai.com>
Reviewed-by: Michael Galaxy <mgalaxy@akamai.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
---
 hw/net/virtio-net.c      |  6 +++---
 hw/vfio/migration.c      |  6 +++---
 include/migration/misc.h |  6 ++++--
 migration/migration.c    | 22 ++++++++++++++++------
 net/vhost-vdpa.c         |  7 ++++---
 ui/spice-core.c          |  3 +--
 6 files changed, 31 insertions(+), 19 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 29e33ea..b85c794 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3624,8 +3624,8 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
         n->primary_listener.hide_device = failover_hide_primary_device;
         qatomic_set(&n->failover_primary_hidden, true);
         device_listener_register(&n->primary_listener);
-        n->migration_state.notify = virtio_net_migration_state_notifier;
-        add_migration_state_change_notifier(&n->migration_state);
+        migration_add_notifier(&n->migration_state,
+                               virtio_net_migration_state_notifier);
         n->host_features |= (1ULL << VIRTIO_NET_F_STANDBY);
     }
 
@@ -3788,7 +3788,7 @@ static void virtio_net_device_unrealize(DeviceState *dev)
     if (n->failover) {
         qobject_unref(n->primary_opts);
         device_listener_unregister(&n->primary_listener);
-        remove_migration_state_change_notifier(&n->migration_state);
+        migration_remove_notifier(&n->migration_state);
     } else {
         assert(n->primary_opts == NULL);
     }
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 0aaad65..28d422b 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -872,8 +872,8 @@ static int vfio_migration_init(VFIODevice *vbasedev)
                      NULL;
     migration->vm_state = qdev_add_vm_change_state_handler_full(
         vbasedev->dev, vfio_vmstate_change, prepare_cb, vbasedev);
-    migration->migration_state.notify = vfio_migration_state_notifier;
-    add_migration_state_change_notifier(&migration->migration_state);
+    migration_add_notifier(&migration->migration_state,
+                           vfio_migration_state_notifier);
 
     return 0;
 }
@@ -882,7 +882,7 @@ static void vfio_migration_deinit(VFIODevice *vbasedev)
 {
     VFIOMigration *migration = vbasedev->migration;
 
-    remove_migration_state_change_notifier(&migration->migration_state);
+    migration_remove_notifier(&migration->migration_state);
     qemu_del_vm_change_state_handler(migration->vm_state);
     unregister_savevm(VMSTATE_IF(vbasedev->dev), "vfio", vbasedev);
     vfio_migration_free(vbasedev);
diff --git a/include/migration/misc.h b/include/migration/misc.h
index 7dcc0b5..673ac49 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -60,8 +60,10 @@ void migration_object_init(void);
 void migration_shutdown(void);
 bool migration_is_idle(void);
 bool migration_is_active(MigrationState *);
-void add_migration_state_change_notifier(Notifier *notify);
-void remove_migration_state_change_notifier(Notifier *notify);
+void migration_add_notifier(Notifier *notify,
+                            void (*func)(Notifier *notifier, void *data));
+void migration_remove_notifier(Notifier *notify);
+void migration_call_notifiers(MigrationState *s);
 bool migration_in_setup(MigrationState *);
 bool migration_has_finished(MigrationState *);
 bool migration_has_failed(MigrationState *);
diff --git a/migration/migration.c b/migration/migration.c
index 65f9791..29a69bf 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1204,7 +1204,7 @@ static void migrate_fd_cleanup(MigrationState *s)
         /* It is used on info migrate.  We can't free it */
         error_report_err(error_copy(s->error));
     }
-    notifier_list_notify(&migration_state_notifiers, s);
+    migration_call_notifiers(s);
     block_cleanup_parameters();
     yank_unregister_instance(MIGRATION_YANK_INSTANCE);
 }
@@ -1308,14 +1308,24 @@ static void migrate_fd_cancel(MigrationState *s)
     }
 }
 
-void add_migration_state_change_notifier(Notifier *notify)
+void migration_add_notifier(Notifier *notify,
+                            void (*func)(Notifier *notifier, void *data))
 {
+    notify->notify = func;
     notifier_list_add(&migration_state_notifiers, notify);
 }
 
-void remove_migration_state_change_notifier(Notifier *notify)
+void migration_remove_notifier(Notifier *notify)
 {
-    notifier_remove(notify);
+    if (notify->notify) {
+        notifier_remove(notify);
+        notify->notify = NULL;
+    }
+}
+
+void migration_call_notifiers(MigrationState *s)
+{
+    notifier_list_notify(&migration_state_notifiers, s);
 }
 
 bool migration_in_setup(MigrationState *s)
@@ -2231,7 +2241,7 @@ static int postcopy_start(MigrationState *ms, Error **errp)
      * spice needs to trigger a transition now
      */
     ms->postcopy_after_devices = true;
-    notifier_list_notify(&migration_state_notifiers, ms);
+    migration_call_notifiers(ms);
 
     ms->downtime =  qemu_clock_get_ms(QEMU_CLOCK_REALTIME) - time_at_stop;
 
@@ -3265,7 +3275,7 @@ void migrate_fd_connect(MigrationState *s, Error *error_in)
         rate_limit = migrate_max_bandwidth();
 
         /* Notify before starting migration thread */
-        notifier_list_notify(&migration_state_notifiers, s);
+        migration_call_notifiers(s);
     }
 
     migration_rate_set(rate_limit);
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 939c984..0f2e6fc 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -339,7 +339,8 @@ static void vhost_vdpa_net_data_start_first(VhostVDPAState *s)
 {
     struct vhost_vdpa *v = &s->vhost_vdpa;
 
-    add_migration_state_change_notifier(&s->migration_state);
+    migration_add_notifier(&s->migration_state,
+                           vdpa_net_migration_state_notifier);
     if (v->shadow_vqs_enabled) {
         v->iova_tree = vhost_iova_tree_new(v->iova_range.first,
                                            v->iova_range.last);
@@ -399,7 +400,7 @@ static void vhost_vdpa_net_client_stop(NetClientState *nc)
     assert(nc->info->type == NET_CLIENT_DRIVER_VHOST_VDPA);
 
     if (s->vhost_vdpa.index == 0) {
-        remove_migration_state_change_notifier(&s->migration_state);
+        migration_remove_notifier(&s->migration_state);
     }
 
     dev = s->vhost_vdpa.dev;
@@ -1456,7 +1457,7 @@ static NetClientState *net_vhost_vdpa_init(NetClientState *peer,
     s->vhost_vdpa.device_fd = vdpa_device_fd;
     s->vhost_vdpa.index = queue_pair_index;
     s->always_svq = svq;
-    s->migration_state.notify = vdpa_net_migration_state_notifier;
+    s->migration_state.notify = NULL;
     s->vhost_vdpa.shadow_vqs_enabled = svq;
     s->vhost_vdpa.iova_range = iova_range;
     s->vhost_vdpa.shadow_data = svq;
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 52a5938..db21db2 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -821,8 +821,7 @@ static void qemu_spice_init(void)
     };
     using_spice = 1;
 
-    migration_state.notify = migration_state_notifier;
-    add_migration_state_change_notifier(&migration_state);
+    migration_add_notifier(&migration_state, migration_state_notifier);
     spice_migrate.base.sif = &migrate_interface.base;
     qemu_spice.add_interface(&spice_migrate.base);
 
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH V4] migration: simplify notifiers
  2023-10-18 13:40 [PATCH V4] migration: simplify notifiers Steve Sistare
@ 2023-10-18 13:41 ` Steven Sistare
  0 siblings, 0 replies; 2+ messages in thread
From: Steven Sistare @ 2023-10-18 13:41 UTC (permalink / raw)
  To: qemu-devel
  Cc: Juan Quintela, Michael S. Tsirkin, Alex Williamson, Fabiano Rosas,
	Leonardo Bras

I rebased the V3 patch and added the new maintainers and reviewers.
Peter has already reviewed it.

- Steve

On 10/18/2023 9:40 AM, Steve Sistare wrote:
> Pass the callback function to add_migration_state_change_notifier so
> that migration can initialize the notifier on add and clear it on
> delete, which simplifies the call sites.  Shorten the function names
> so the extra arg can be added more legibly.  Hide the global notifier
> list in a new function migration_call_notifiers, and make it externally
> visible so future live update code can call it.
> 
> No functional change.
> 
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> Tested-by: Michael Galaxy <mgalaxy@akamai.com>
> Reviewed-by: Michael Galaxy <mgalaxy@akamai.com>
> Reviewed-by: Peter Xu <peterx@redhat.com>
> ---
>  hw/net/virtio-net.c      |  6 +++---
>  hw/vfio/migration.c      |  6 +++---
>  include/migration/misc.h |  6 ++++--
>  migration/migration.c    | 22 ++++++++++++++++------
>  net/vhost-vdpa.c         |  7 ++++---
>  ui/spice-core.c          |  3 +--
>  6 files changed, 31 insertions(+), 19 deletions(-)
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 29e33ea..b85c794 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -3624,8 +3624,8 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
>          n->primary_listener.hide_device = failover_hide_primary_device;
>          qatomic_set(&n->failover_primary_hidden, true);
>          device_listener_register(&n->primary_listener);
> -        n->migration_state.notify = virtio_net_migration_state_notifier;
> -        add_migration_state_change_notifier(&n->migration_state);
> +        migration_add_notifier(&n->migration_state,
> +                               virtio_net_migration_state_notifier);
>          n->host_features |= (1ULL << VIRTIO_NET_F_STANDBY);
>      }
>  
> @@ -3788,7 +3788,7 @@ static void virtio_net_device_unrealize(DeviceState *dev)
>      if (n->failover) {
>          qobject_unref(n->primary_opts);
>          device_listener_unregister(&n->primary_listener);
> -        remove_migration_state_change_notifier(&n->migration_state);
> +        migration_remove_notifier(&n->migration_state);
>      } else {
>          assert(n->primary_opts == NULL);
>      }
> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> index 0aaad65..28d422b 100644
> --- a/hw/vfio/migration.c
> +++ b/hw/vfio/migration.c
> @@ -872,8 +872,8 @@ static int vfio_migration_init(VFIODevice *vbasedev)
>                       NULL;
>      migration->vm_state = qdev_add_vm_change_state_handler_full(
>          vbasedev->dev, vfio_vmstate_change, prepare_cb, vbasedev);
> -    migration->migration_state.notify = vfio_migration_state_notifier;
> -    add_migration_state_change_notifier(&migration->migration_state);
> +    migration_add_notifier(&migration->migration_state,
> +                           vfio_migration_state_notifier);
>  
>      return 0;
>  }
> @@ -882,7 +882,7 @@ static void vfio_migration_deinit(VFIODevice *vbasedev)
>  {
>      VFIOMigration *migration = vbasedev->migration;
>  
> -    remove_migration_state_change_notifier(&migration->migration_state);
> +    migration_remove_notifier(&migration->migration_state);
>      qemu_del_vm_change_state_handler(migration->vm_state);
>      unregister_savevm(VMSTATE_IF(vbasedev->dev), "vfio", vbasedev);
>      vfio_migration_free(vbasedev);
> diff --git a/include/migration/misc.h b/include/migration/misc.h
> index 7dcc0b5..673ac49 100644
> --- a/include/migration/misc.h
> +++ b/include/migration/misc.h
> @@ -60,8 +60,10 @@ void migration_object_init(void);
>  void migration_shutdown(void);
>  bool migration_is_idle(void);
>  bool migration_is_active(MigrationState *);
> -void add_migration_state_change_notifier(Notifier *notify);
> -void remove_migration_state_change_notifier(Notifier *notify);
> +void migration_add_notifier(Notifier *notify,
> +                            void (*func)(Notifier *notifier, void *data));
> +void migration_remove_notifier(Notifier *notify);
> +void migration_call_notifiers(MigrationState *s);
>  bool migration_in_setup(MigrationState *);
>  bool migration_has_finished(MigrationState *);
>  bool migration_has_failed(MigrationState *);
> diff --git a/migration/migration.c b/migration/migration.c
> index 65f9791..29a69bf 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1204,7 +1204,7 @@ static void migrate_fd_cleanup(MigrationState *s)
>          /* It is used on info migrate.  We can't free it */
>          error_report_err(error_copy(s->error));
>      }
> -    notifier_list_notify(&migration_state_notifiers, s);
> +    migration_call_notifiers(s);
>      block_cleanup_parameters();
>      yank_unregister_instance(MIGRATION_YANK_INSTANCE);
>  }
> @@ -1308,14 +1308,24 @@ static void migrate_fd_cancel(MigrationState *s)
>      }
>  }
>  
> -void add_migration_state_change_notifier(Notifier *notify)
> +void migration_add_notifier(Notifier *notify,
> +                            void (*func)(Notifier *notifier, void *data))
>  {
> +    notify->notify = func;
>      notifier_list_add(&migration_state_notifiers, notify);
>  }
>  
> -void remove_migration_state_change_notifier(Notifier *notify)
> +void migration_remove_notifier(Notifier *notify)
>  {
> -    notifier_remove(notify);
> +    if (notify->notify) {
> +        notifier_remove(notify);
> +        notify->notify = NULL;
> +    }
> +}
> +
> +void migration_call_notifiers(MigrationState *s)
> +{
> +    notifier_list_notify(&migration_state_notifiers, s);
>  }
>  
>  bool migration_in_setup(MigrationState *s)
> @@ -2231,7 +2241,7 @@ static int postcopy_start(MigrationState *ms, Error **errp)
>       * spice needs to trigger a transition now
>       */
>      ms->postcopy_after_devices = true;
> -    notifier_list_notify(&migration_state_notifiers, ms);
> +    migration_call_notifiers(ms);
>  
>      ms->downtime =  qemu_clock_get_ms(QEMU_CLOCK_REALTIME) - time_at_stop;
>  
> @@ -3265,7 +3275,7 @@ void migrate_fd_connect(MigrationState *s, Error *error_in)
>          rate_limit = migrate_max_bandwidth();
>  
>          /* Notify before starting migration thread */
> -        notifier_list_notify(&migration_state_notifiers, s);
> +        migration_call_notifiers(s);
>      }
>  
>      migration_rate_set(rate_limit);
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index 939c984..0f2e6fc 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -339,7 +339,8 @@ static void vhost_vdpa_net_data_start_first(VhostVDPAState *s)
>  {
>      struct vhost_vdpa *v = &s->vhost_vdpa;
>  
> -    add_migration_state_change_notifier(&s->migration_state);
> +    migration_add_notifier(&s->migration_state,
> +                           vdpa_net_migration_state_notifier);
>      if (v->shadow_vqs_enabled) {
>          v->iova_tree = vhost_iova_tree_new(v->iova_range.first,
>                                             v->iova_range.last);
> @@ -399,7 +400,7 @@ static void vhost_vdpa_net_client_stop(NetClientState *nc)
>      assert(nc->info->type == NET_CLIENT_DRIVER_VHOST_VDPA);
>  
>      if (s->vhost_vdpa.index == 0) {
> -        remove_migration_state_change_notifier(&s->migration_state);
> +        migration_remove_notifier(&s->migration_state);
>      }
>  
>      dev = s->vhost_vdpa.dev;
> @@ -1456,7 +1457,7 @@ static NetClientState *net_vhost_vdpa_init(NetClientState *peer,
>      s->vhost_vdpa.device_fd = vdpa_device_fd;
>      s->vhost_vdpa.index = queue_pair_index;
>      s->always_svq = svq;
> -    s->migration_state.notify = vdpa_net_migration_state_notifier;
> +    s->migration_state.notify = NULL;
>      s->vhost_vdpa.shadow_vqs_enabled = svq;
>      s->vhost_vdpa.iova_range = iova_range;
>      s->vhost_vdpa.shadow_data = svq;
> diff --git a/ui/spice-core.c b/ui/spice-core.c
> index 52a5938..db21db2 100644
> --- a/ui/spice-core.c
> +++ b/ui/spice-core.c
> @@ -821,8 +821,7 @@ static void qemu_spice_init(void)
>      };
>      using_spice = 1;
>  
> -    migration_state.notify = migration_state_notifier;
> -    add_migration_state_change_notifier(&migration_state);
> +    migration_add_notifier(&migration_state, migration_state_notifier);
>      spice_migrate.base.sif = &migrate_interface.base;
>      qemu_spice.add_interface(&spice_migrate.base);
>  


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-10-18 13:42 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-18 13:40 [PATCH V4] migration: simplify notifiers Steve Sistare
2023-10-18 13:41 ` Steven Sistare

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).