| Message ID | 20230918144206.560120-20-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | migration/rdma: Error handling fixes | expand |
On Mon, Sep 18, 2023 at 04:41:33PM +0200, Markus Armbruster wrote: > qemu_get_cm_event_timeout() neglects to set an error when it fails > because rdma_get_cm_event() fails. Harmless, as its caller > qemu_rdma_connect() substitutes a generic error then. Fix it anyway. > > qemu_rdma_connect() also sets the generic error when its own call of > rdma_get_cm_event() fails. Make the error handling more obvious: set > a specific error right after rdma_get_cm_event() fails. Delete the > generic error. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Peter Xu <peterx@redhat.com>
On 18/09/2023 22:41, Markus Armbruster wrote: > qemu_get_cm_event_timeout() neglects to set an error when it fails > because rdma_get_cm_event() fails. Harmless, as its caller > qemu_rdma_connect() substitutes a generic error then. Fix it anyway. > > qemu_rdma_connect() also sets the generic error when its own call of > rdma_get_cm_event() fails. Make the error handling more obvious: set > a specific error right after rdma_get_cm_event() fails. Delete the > generic error. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Li Zhijian <lizhijian@fujitsu.com> > --- > migration/rdma.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/migration/rdma.c b/migration/rdma.c > index 08cd186385..d3dc162363 100644 > --- a/migration/rdma.c > +++ b/migration/rdma.c > @@ -2509,7 +2509,11 @@ static int qemu_get_cm_event_timeout(RDMAContext *rdma, > ERROR(errp, "failed to poll cm event, errno=%i", errno); > return -1; > } else if (poll_fd.revents & POLLIN) { > - return rdma_get_cm_event(rdma->channel, cm_event); > + if (rdma_get_cm_event(rdma->channel, cm_event) < 0) { > + ERROR(errp, "failed to get cm event"); > + return -1; > + } > + return 0; > } else { > ERROR(errp, "no POLLIN event, revent=%x", poll_fd.revents); > return -1; > @@ -2559,10 +2563,12 @@ static int qemu_rdma_connect(RDMAContext *rdma, bool return_path, > ret = qemu_get_cm_event_timeout(rdma, &cm_event, 5000, errp); > } else { > ret = rdma_get_cm_event(rdma->channel, &cm_event); > + if (ret < 0) { > + ERROR(errp, "failed to get cm event"); > + } > } > if (ret) { > perror("rdma_get_cm_event after rdma_connect"); > - ERROR(errp, "connecting to destination!"); > goto err_rdma_source_connect; > } >
diff --git a/migration/rdma.c b/migration/rdma.c index 08cd186385..d3dc162363 100644 --- a/migration/rdma.c +++ b/migration/rdma.c @@ -2509,7 +2509,11 @@ static int qemu_get_cm_event_timeout(RDMAContext *rdma, ERROR(errp, "failed to poll cm event, errno=%i", errno); return -1; } else if (poll_fd.revents & POLLIN) { - return rdma_get_cm_event(rdma->channel, cm_event); + if (rdma_get_cm_event(rdma->channel, cm_event) < 0) { + ERROR(errp, "failed to get cm event"); + return -1; + } + return 0; } else { ERROR(errp, "no POLLIN event, revent=%x", poll_fd.revents); return -1; @@ -2559,10 +2563,12 @@ static int qemu_rdma_connect(RDMAContext *rdma, bool return_path, ret = qemu_get_cm_event_timeout(rdma, &cm_event, 5000, errp); } else { ret = rdma_get_cm_event(rdma->channel, &cm_event); + if (ret < 0) { + ERROR(errp, "failed to get cm event"); + } } if (ret) { perror("rdma_get_cm_event after rdma_connect"); - ERROR(errp, "connecting to destination!"); goto err_rdma_source_connect; }
qemu_get_cm_event_timeout() neglects to set an error when it fails because rdma_get_cm_event() fails. Harmless, as its caller qemu_rdma_connect() substitutes a generic error then. Fix it anyway. qemu_rdma_connect() also sets the generic error when its own call of rdma_get_cm_event() fails. Make the error handling more obvious: set a specific error right after rdma_get_cm_event() fails. Delete the generic error. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- migration/rdma.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)