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<arm...@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.
> return -1;
> }
