[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH V4] migration: simplify notifiers
From: |
Steven Sistare |
Subject: |
Re: [PATCH V4] migration: simplify notifiers |
Date: |
Wed, 18 Oct 2023 09:41:49 -0400 |
User-agent: |
Mozilla Thunderbird |
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);
>