Re: [Qemu-devel] qemu_rdma_cleanup seg - related to 5a91337?

["Michael R. Hines" <mrhines@linux.vnet.ibm.com>]

On 02/06/2014 08:26 PM, Dr. David Alan Gilbert wrote:
> Hi Isaku,
>     I hit a seg in qemu_rdma_cleanup in the code changed by your
> '[PATCH] rdma: clean up of qemu_rdma_cleanup()'
>
> migration-rdma.c ~ 2241
>
>      if (rdma->qp) {
>          rdma_destroy_qp(rdma->cm_id);
>          rdma->qp = NULL;
>      }
>
> Your patch changed that to free cm_id at that point rather than
> qp; but in my case cm_id is NULL and so rdma_destroy_qp segs.
>
> given that there is a :
>
>      if (rdma->cm_id) {
>          rdma_destroy_id(rdma->cm_id);
>          rdma->cm_id = NULL;
>      }
>
> later down, and there is now no longer any destroy of rdma->qp
> I don't understand your change.
>
> Your change text says:
>    '- RDMAContext::qp is created by rdma_create_qp() so that it should be destroyed
>     by rdma_destroy_qp(). not ibv_destroy_qp()'
>
> but the diff is:
>        if (rdma->qp) {
> -        ibv_destroy_qp(rdma->qp);
> +        rdma_destroy_qp(rdma->cm_id);
>            rdma->qp = NULL;
>
> should that have been rdma_destroy_qp(rdma->qp)?
>
> Dave (who doesn't yet know enough RDMA to be dangerous)
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
Responding for Isaku..... Thanks for reporting the bug, but I need some help
in tracking down the cause of the bug, see below.

Actually, the parameter "rdma->cm_id" to the function is correct, it's just
that the variable never got initialized in the first place, which means 
that either
the connection never got established or an early error happened during
the migration that required cleaning up the identifier.

Can you describe the conditions of the migration and the environment?
1. Did you migrate only one VM? Was the host under heavy load?
2. Did your migration lose connectivity? Did one of the hosts crash?
3. Was the connection abruptly broken for some reason?
4. Did you ever cancel the migration at some point and restart?
5. Did you use libvirt?

A simple fix would be to surround the "rdma_destroy_qp()" call with a check
to see if rdma->cm_id is valid, but that doesn't answer why rdma->cm_id 
would be invalid
in the first place.

I need some additional information to try to reproduce the conditions of 
the bug.

Thanks!
- Michael Hines