On 18/09/2023 22:42, 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_one() violates this principle: it reports errors to
> stderr via qemu_rdma_register_and_get_keys(). I elected not to
> investigate how callers handle the error, i.e. precise impact is not
> known.
>
> Clean this up: silence qemu_rdma_register_and_get_keys(). I believe
> the caller's error reports suffice. If they don't, we need to convert
> to Error instead.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Li Zhijian <lizhijian@fujitsu.com>
> ---
> migration/rdma.c | 9 ---------
> 1 file changed, 9 deletions(-)
>
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 99dccdeae5..d9f80ef390 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -1314,15 +1314,6 @@ static int qemu_rdma_register_and_get_keys(RDMAContext *rdma,
> }
> }
> if (!block->pmr[chunk]) {
> - perror("Failed to register chunk!");
> - fprintf(stderr, "Chunk details: block: %d chunk index %d"
> - " start %" PRIuPTR " end %" PRIuPTR
> - " host %" PRIuPTR
> - " local %" PRIuPTR " registrations: %d\n",
> - block->index, chunk, (uintptr_t)chunk_start,
> - (uintptr_t)chunk_end, host_addr,
> - (uintptr_t)block->local_host_addr,
> - rdma->total_registrations);
> return -1;
> }
> rdma->total_registrations++;