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? >> return -1; >> }