Markus Armbruster <arm...@redhat.com> writes:
> Fabiano Rosas <faro...@suse.de> writes:
>
>> Markus Armbruster <arm...@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 <arm...@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:"?
>
> It will. I'll drop the "WARN: " prefix.
>
>>> } 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?
>
> Yes :)
>
> ibv_dereg_mr(3) section RETURN VALUE has:
>
> ibv_dereg_mr() returns 0 on success, or the value of errno on failure
> (which indicates the failure reason).
>
> Clearer now?
Yep, thank you.
