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 <arm...@redhat.com>


Reviewed-by: Li Zhijian <lizhij...@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++;

Reply via email to