Re: [PATCH 23/52] migration/rdma: Clean up qemu_rdma_wait_comp_channel()'s error value
"Zhijian Li (Fujitsu)" writes: > On 18/09/2023 22:41, Markus Armbruster wrote: >> qemu_rdma_wait_comp_channel() returns 0 on success, and either -1 or >> rdma->error_state on failure. Callers actually expect a negative >> error value. > > I don't see the only one callers expect a negative error code. > migration/rdma.c:1654:ret = qemu_rdma_wait_comp_channel(rdma, ch); > migration/rdma.c-1655-if (ret) { > migration/rdma.c-1656-goto err_block_for_wrid; You're right. I want the change anyway, to let me simplify the code some. I'll adjust the commit message. > I believe rdma->error_state can't be positive, but let's >> make things more obvious by simply returning -1 on any failure. >> >> Signed-off-by: Markus Armbruster >> --- >> migration/rdma.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/migration/rdma.c b/migration/rdma.c >> index 3421ae0796..efbb3c7754 100644 >> --- a/migration/rdma.c >> +++ b/migration/rdma.c >> @@ -1588,7 +1588,7 @@ static int qemu_rdma_wait_comp_channel(RDMAContext >> *rdma, >> if (rdma->received_error) { >> return -EPIPE; >> } >> -return rdma->error_state; >> +return -!!rdma->error_state; > > And i rarely see such things, below would be more clear. > > return rdma->error_state ? -1 : 0: Goes away in PATCH 26: @@ -1588,7 +1588,7 @@ static int qemu_rdma_wait_comp_channel(RDMAContext *rdma, if (rdma->received_error) { return -1; } -return -!!rdma->error_state; +return -rdma->errored; } static struct ibv_comp_channel *to_channel(RDMAContext *rdma, uint64_t wrid) > >> } >> >> static struct ibv_comp_channel *to_channel(RDMAContext *rdma, uint64_t >> wrid)
Re: [PATCH 23/52] migration/rdma: Clean up qemu_rdma_wait_comp_channel()'s error value
On 18/09/2023 22:41, Markus Armbruster wrote: > qemu_rdma_wait_comp_channel() returns 0 on success, and either -1 or > rdma->error_state on failure. Callers actually expect a negative > error value. I don't see the only one callers expect a negative error code. migration/rdma.c:1654:ret = qemu_rdma_wait_comp_channel(rdma, ch); migration/rdma.c-1655-if (ret) { migration/rdma.c-1656-goto err_block_for_wrid; I believe rdma->error_state can't be positive, but let's > make things more obvious by simply returning -1 on any failure. > > Signed-off-by: Markus Armbruster > --- > migration/rdma.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/migration/rdma.c b/migration/rdma.c > index 3421ae0796..efbb3c7754 100644 > --- a/migration/rdma.c > +++ b/migration/rdma.c > @@ -1588,7 +1588,7 @@ static int qemu_rdma_wait_comp_channel(RDMAContext > *rdma, > if (rdma->received_error) { > return -EPIPE; > } > -return rdma->error_state; > +return -!!rdma->error_state; And i rarely see such things, below would be more clear. return rdma->error_state ? -1 : 0: > } > > static struct ibv_comp_channel *to_channel(RDMAContext *rdma, uint64_t wrid)
[PATCH 23/52] migration/rdma: Clean up qemu_rdma_wait_comp_channel()'s error value
qemu_rdma_wait_comp_channel() returns 0 on success, and either -1 or rdma->error_state on failure. Callers actually expect a negative error value. I believe rdma->error_state can't be positive, but let's make things more obvious by simply returning -1 on any failure. Signed-off-by: Markus Armbruster --- migration/rdma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/migration/rdma.c b/migration/rdma.c index 3421ae0796..efbb3c7754 100644 --- a/migration/rdma.c +++ b/migration/rdma.c @@ -1588,7 +1588,7 @@ static int qemu_rdma_wait_comp_channel(RDMAContext *rdma, if (rdma->received_error) { return -EPIPE; } -return rdma->error_state; +return -!!rdma->error_state; } static struct ibv_comp_channel *to_channel(RDMAContext *rdma, uint64_t wrid) -- 2.41.0