Re: [PATCH 39/52] migration/rdma: Convert qemu_rdma_write_one() to Error

[Zhijian Li (Fujitsu)]

On 26/09/2023 13:50, Li Zhijian wrote:
> 
> 
> 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.
> 

Answer it myself:
ibv_post_send(3) says:

RETURN VALUE
        ibv_post_send() returns 0 on success, or the value of errno on failure 
(which indicates the failure reason).


the global error is not defined here.



> 
> 
> 
> 
>>           return -1;
>>       }