Re: [PATCH 14/20] migration: Add migration_incoming_transport_cleanup()

[Dr. David Alan Gilbert]

* Peter Xu (peterx@redhat.com) wrote:
> Add a helper to cleanup the transport listener.
> 
> When do it, we should also null-ify the cleanup hook and the data, then it's
> even safe to call it multiple times.
> 
> Move the socket_address_list cleanup altogether, because that's a mirror of 
> the
> listener channels and only for the purpose of query-migrate.  Hence when
> someone wants to cleanup the listener transport, it should also want to 
> cleanup
> the socket list too, always.
> 
> No functional change intended.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>

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

> ---
>  migration/migration.c | 22 ++++++++++++++--------
>  migration/migration.h |  1 +
>  2 files changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index b2e6446457..6bb321cdd3 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -279,6 +279,19 @@ MigrationIncomingState 
> *migration_incoming_get_current(void)
>      return current_incoming;
>  }
>  
> +void migration_incoming_transport_cleanup(MigrationIncomingState *mis)
> +{
> +    if (mis->socket_address_list) {
> +        qapi_free_SocketAddressList(mis->socket_address_list);
> +        mis->socket_address_list = NULL;
> +    }
> +
> +    if (mis->transport_cleanup) {
> +        mis->transport_cleanup(mis->transport_data);
> +        mis->transport_data = mis->transport_cleanup = NULL;
> +    }
> +}
> +
>  void migration_incoming_state_destroy(void)
>  {
>      struct MigrationIncomingState *mis = migration_incoming_get_current();
> @@ -299,10 +312,8 @@ void migration_incoming_state_destroy(void)
>          g_array_free(mis->postcopy_remote_fds, TRUE);
>          mis->postcopy_remote_fds = NULL;
>      }
> -    if (mis->transport_cleanup) {
> -        mis->transport_cleanup(mis->transport_data);
> -    }
>  
> +    migration_incoming_transport_cleanup(mis);
>      qemu_event_reset(&mis->main_thread_load_event);
>  
>      if (mis->page_requested) {
> @@ -310,11 +321,6 @@ void migration_incoming_state_destroy(void)
>          mis->page_requested = NULL;
>      }
>  
> -    if (mis->socket_address_list) {
> -        qapi_free_SocketAddressList(mis->socket_address_list);
> -        mis->socket_address_list = NULL;
> -    }
> -
>      yank_unregister_instance(MIGRATION_YANK_INSTANCE);
>  }
>  
> diff --git a/migration/migration.h b/migration/migration.h
> index d677a750c9..f17ccc657c 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -166,6 +166,7 @@ struct MigrationIncomingState {
>  
>  MigrationIncomingState *migration_incoming_get_current(void);
>  void migration_incoming_state_destroy(void);
> +void migration_incoming_transport_cleanup(MigrationIncomingState *mis);
>  /*
>   * Functions to work with blocktime context
>   */
> -- 
> 2.32.0
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK