* Chuan Zheng (zhengch...@huawei.com) wrote:
> Signed-off-by: Chuan Zheng <zhengch...@huawei.com>
> ---
> migration/multifd.c | 6 ++++++
> migration/multifd.h | 1 +
> migration/rdma.c | 16 +++++++++++++++-
> 3 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/migration/multifd.c b/migration/multifd.c
> index 1186246..4031648 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -577,6 +577,9 @@ void multifd_save_cleanup(void)
> p->packet_len = 0;
> g_free(p->packet);
> p->packet = NULL;
> +#ifdef CONFIG_RDMA
> + multifd_rdma_cleanup(p->rdma);
> +#endif
You may find it easier to add an entry into stubs/ for
multifd_rdma_cleanup; it then avoids the need for the ifdef.
> multifd_send_state->ops->send_cleanup(p, &local_err);
> if (local_err) {
> migrate_set_error(migrate_get_current(), local_err);
> @@ -1039,6 +1042,9 @@ int multifd_load_cleanup(Error **errp)
> p->packet_len = 0;
> g_free(p->packet);
> p->packet = NULL;
> +#ifdef CONFIG_RDMA
> + multifd_rdma_cleanup(p->rdma);
> +#endif
> multifd_recv_state->ops->recv_cleanup(p);
> }
> qemu_sem_destroy(&multifd_recv_state->sem_sync);
> diff --git a/migration/multifd.h b/migration/multifd.h
> index 26d4489..0ecec5e 100644
> --- a/migration/multifd.h
> +++ b/migration/multifd.h
> @@ -183,6 +183,7 @@ typedef struct {
>
> #ifdef CONFIG_RDMA
> extern MultiFDSetup multifd_rdma_ops;
> +void multifd_rdma_cleanup(void *opaque);
> #endif
> void multifd_send_terminate_threads(Error *err);
> int multifd_send_initial_packet(MultiFDSendParams *p, Error **errp);
> diff --git a/migration/rdma.c b/migration/rdma.c
> index c19a91f..f14357f 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -2369,7 +2369,7 @@ static void qemu_rdma_cleanup(RDMAContext *rdma)
> {
> int idx;
>
> - if (rdma->cm_id && rdma->connected) {
> + if (rdma->channel && rdma->cm_id && rdma->connected) {
> if ((rdma->error_state ||
> migrate_get_current()->state == MIGRATION_STATUS_CANCELLING) &&
> !rdma->received_error) {
> @@ -4599,6 +4599,20 @@ static void multifd_rdma_recv_channel_setup(QIOChannel
> *ioc,
> return;
> }
>
> +void multifd_rdma_cleanup(void *opaque)
I think you need to make it clear that this is only to cleanup one
channel, rather than the whole multifd-rdma connection;
multifd_load_cleanup for example cleans up all the channels, where as I
think this is only doing one?
Don't use a 'void *opaque' except for something that's called via
a registration/callback scheme that's designed to be generic
(e.g. multifd_send_thread does it because it's called from
qemu_thread_create that doesn't know the type). Where you know
the type, use it!
> +{
> + RDMAContext *rdma = (RDMAContext *)opaque;
> +
> + if (!migrate_use_rdma()) {
> + return;
> + }
> +
> + rdma->listen_id = NULL;
> + rdma->channel = NULL;
> + qemu_rdma_cleanup(rdma);
> + g_free(rdma);
> +}
> +
> MultiFDSetup multifd_rdma_ops = {
> .send_thread = multifd_rdma_send_thread,
> .recv_thread = multifd_rdma_recv_thread,
> --
> 1.8.3.1
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
