Re: [Qemu-devel] [PATCH] migration/rdma: unegister fd handler

[Dr. David Alan Gilbert]

* Peter Maydell (address@hidden) wrote:
> On Tue, 22 Jan 2019 at 19:08, Dr. David Alan Gilbert (git)
> <address@hidden> wrote:
> >
> > From: "Dr. David Alan Gilbert" <address@hidden>
> >
> > Unregister the fd handler before we destroy the channel,
> > otherwise we've got a race where we might land in the
> > fd handler just as we're closing the device.
> >
> > (The race is quite data dependent, you just have to have
> > the right set of devices for it to trigger).
> >
> > Corresponds to RH bz: https://bugzilla.redhat.com/show_bug.cgi?id=1666601
> >
> > Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> > ---
> >  migration/rdma.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/migration/rdma.c b/migration/rdma.c
> > index 9b2e7e10aa..54a3c11540 100644
> > --- a/migration/rdma.c
> > +++ b/migration/rdma.c
> > @@ -2321,6 +2321,7 @@ static void qemu_rdma_cleanup(RDMAContext *rdma)
> >          rdma->connected = false;
> >      }
> >
> > +    qemu_set_fd_handler(rdma->channel->fd, NULL, NULL, NULL);
> >      g_free(rdma->dest_blocks);
> >      rdma->dest_blocks = NULL;
> 
> Hi -- this patch makes coverity complain (CID 1398634),
> because here we use rdma->channel without checking that it is NULL,
> but later in the function we have an "if (rdma->channel)" test.
> Should this code be conditional on rmda->channel being non-NULL,
> or is the later test incorrect?

Yes, it's got a point - I can seg that.

I'll post a fix.

Dave

> thanks
> -- PMM
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK