Re: [PATCH v2 52/53] migration/rdma: Use error_report() & friends instead of stderr

[Fabiano Rosas]

Markus Armbruster <armbru@redhat.com> writes:

> error_report() obeys -msg, reports the current error location if any,
> and reports to the current monitor if any.  Reporting to stderr
> directly with fprintf() or perror() is wrong, because it loses all
> this.
>
> Fix the offenders.  Bonus: resolves a FIXME about problematic use of
> errno.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  migration/rdma.c | 44 +++++++++++++++++++++-----------------------
>  1 file changed, 21 insertions(+), 23 deletions(-)
>
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 54b59d12b1..dba0802fca 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -877,12 +877,12 @@ static int qemu_rdma_broken_ipv6_kernel(struct 
> ibv_context *verbs, Error **errp)
>  
>          if (roce_found) {
>              if (ib_found) {
> -                fprintf(stderr, "WARN: migrations may fail:"
> -                                " IPv6 over RoCE / iWARP in linux"
> -                                " is broken. But since you appear to have a"
> -                                " mixed RoCE / IB environment, be sure to 
> only"
> -                                " migrate over the IB fabric until the 
> kernel "
> -                                " fixes the bug.\n");
> +                warn_report("WARN: migrations may fail:"
> +                            " IPv6 over RoCE / iWARP in linux"
> +                            " is broken. But since you appear to have a"
> +                            " mixed RoCE / IB environment, be sure to only"
> +                            " migrate over the IB fabric until the kernel "
> +                            " fixes the bug.");

Won't this become "warning: WARN:"?

>              } else {
>                  error_setg(errp, "RDMA ERROR: "
>                             "You only have RoCE / iWARP devices in your 
> systems"
> @@ -1418,12 +1418,8 @@ static int qemu_rdma_unregister_waiting(RDMAContext 
> *rdma)
>          block->remote_keys[chunk] = 0;
>  
>          if (ret != 0) {
> -            /*
> -             * FIXME perror() is problematic, bcause ibv_dereg_mr() is
> -             * not documented to set errno.  Will go away later in
> -             * this series.
> -             */
> -            perror("unregistration chunk failed");
> +            error_report("unregistration chunk failed: %s",
> +                         strerror(ret));

Doesn't seem to fix the issue, ret might still not be an errno. Am I
missing something?

>              return -1;
>          }