[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 39/52] migration/rdma: Convert qemu_rdma_write_one() to Error
From: |
Markus Armbruster |
Subject: |
Re: [PATCH 39/52] migration/rdma: Convert qemu_rdma_write_one() to Error |
Date: |
Tue, 26 Sep 2023 08:40:38 +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:
>> Functions that use an Error **errp parameter to return errors should
>> not also report them to the user, because reporting is the caller's
>> job. When the caller does, the error is reported twice. When it
>> doesn't (because it recovered from the error), there is no error to
>> report, i.e. the report is bogus.
>>
>> qemu_rdma_write_flush() violates this principle: it calls
>> error_report() via qemu_rdma_write_one(). I elected not to
>> investigate how callers handle the error, i.e. precise impact is not
>> known.
>>
>> Clean this up by converting qemu_rdma_write_one() to Error.
>>
>> Signed-off-by: Markus Armbruster<armbru@redhat.com>
>> ---
>> migration/rdma.c | 25 +++++++++++--------------
>> 1 file changed, 11 insertions(+), 14 deletions(-)
>>
>> diff --git a/migration/rdma.c b/migration/rdma.c
>> index c3c33fe242..9b8cbadfcd 100644
>> --- a/migration/rdma.c
>> +++ b/migration/rdma.c
>> @@ -2019,9 +2019,8 @@ static int qemu_rdma_exchange_recv(RDMAContext *rdma,
>> RDMAControlHeader *head,
>> */
>> static int qemu_rdma_write_one(QEMUFile *f, RDMAContext *rdma,
>> int current_index, uint64_t current_addr,
>> - uint64_t length)
>> + uint64_t length, Error **errp)
>> {
>> - Error *err = NULL;
>> struct ibv_sge sge;
>> struct ibv_send_wr send_wr = { 0 };
>> struct ibv_send_wr *bad_wr;
>
> [...]
>
>> }
>> @@ -2219,7 +2216,7 @@ retry:
>> goto retry;
>>
>> } else if (ret > 0) {
>> - perror("rdma migration: post rdma write failed");
>> + error_setg(errp, "rdma migration: post rdma write failed");
>
> It reminds that do you miss to use error_setg_errno() instead.
Indeed!
I'll adjust the commit message as well.
>> return -1;
>> }
- Re: [PATCH 50/52] migration/rdma: Silence qemu_rdma_cleanup(), (continued)
- Re: [PATCH 39/52] migration/rdma: Convert qemu_rdma_write_one() to Error,
Markus Armbruster <=
[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
Re: [PATCH 26/52] migration/rdma: Replace int error_state by bool errored, Eric Blake, 2023/09/27