[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 23/52] migration/rdma: Clean up qemu_rdma_wait_comp_channel()
From: |
Markus Armbruster |
Subject: |
Re: [PATCH 23/52] migration/rdma: Clean up qemu_rdma_wait_comp_channel()'s error value |
Date: |
Mon, 25 Sep 2023 08:55:19 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) |
"Zhijian Li (Fujitsu)" <lizhijian@fujitsu.com> 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 <armbru@redhat.com>
>> ---
>> 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 39/52] migration/rdma: Convert qemu_rdma_write_one() to Error, (continued)
- [PATCH 04/52] migration/rdma: Drop fragile wr_id formatting, Markus Armbruster, 2023/09/18
- [PATCH 25/52] migration/rdma: Dumb down remaining int error values to -1, Markus Armbruster, 2023/09/18
- [PATCH 26/52] migration/rdma: Replace int error_state by bool errored, Markus Armbruster, 2023/09/18
- [PATCH 23/52] migration/rdma: Clean up qemu_rdma_wait_comp_channel()'s error value, Markus Armbruster, 2023/09/18
- [PATCH 15/52] migration/rdma: Ditch useless numeric error codes in error messages, Markus Armbruster, 2023/09/18
- [PATCH 41/52] migration/rdma: Convert qemu_rdma_post_send_control() to Error, Markus Armbruster, 2023/09/18
- [PATCH 17/52] migration/rdma: Replace dangerous macro CHECK_ERROR_STATE(), Markus Armbruster, 2023/09/18
- [PATCH 38/52] migration/rdma: Convert qemu_rdma_write_flush() to Error, Markus Armbruster, 2023/09/18