On 28/09/2023 21:20, Markus Armbruster wrote:
> 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>
Reviewed-by: Li Zhijian <lizhij...@fujitsu.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.");
> } 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));
> return -1;
> }
> rdma->total_registrations--;
> @@ -3767,7 +3763,8 @@ static int qemu_rdma_registration_handle(QEMUFile *f)
> block->pmr[reg->key.chunk] = NULL;
>
> if (ret != 0) {
> - perror("rdma unregistration chunk failed");
> + error_report("rdma unregistration chunk failed: %s",
> + strerror(errno));
> goto err;
> }
>
> @@ -3956,10 +3953,10 @@ static int qemu_rdma_registration_stop(QEMUFile *f,
> */
>
> if (local->nb_blocks != nb_dest_blocks) {
> - fprintf(stderr, "ram blocks mismatch (Number of blocks %d vs %d)
> "
> - "Your QEMU command line parameters are probably "
> - "not identical on both the source and destination.",
> - local->nb_blocks, nb_dest_blocks);
> + error_report("ram blocks mismatch (Number of blocks %d vs %d)",
> + local->nb_blocks, nb_dest_blocks);
> + error_printf("Your QEMU command line parameters are probably "
> + "not identical on both the source and
> destination.");
> rdma->errored = true;
> return -1;
> }
> @@ -3972,10 +3969,11 @@ static int qemu_rdma_registration_stop(QEMUFile *f,
>
> /* We require that the blocks are in the same order */
> if (rdma->dest_blocks[i].length != local->block[i].length) {
> - fprintf(stderr, "Block %s/%d has a different length %" PRIu64
> - "vs %" PRIu64, local->block[i].block_name, i,
> - local->block[i].length,
> - rdma->dest_blocks[i].length);
> + error_report("Block %s/%d has a different length %" PRIu64
> + "vs %" PRIu64,
> + local->block[i].block_name, i,
> + local->block[i].length,
> + rdma->dest_blocks[i].length);
> rdma->errored = true;
> return -1;
> }
> @@ -4091,7 +4089,7 @@ static void rdma_accept_incoming_migration(void *opaque)
> ret = qemu_rdma_accept(rdma);
>
> if (ret < 0) {
> - fprintf(stderr, "RDMA ERROR: Migration initialization failed\n");
> + error_report("RDMA ERROR: Migration initialization failed");
> return;
> }
>
> @@ -4103,7 +4101,7 @@ static void rdma_accept_incoming_migration(void *opaque)
>
> f = rdma_new_input(rdma);
> if (f == NULL) {
> - fprintf(stderr, "RDMA ERROR: could not open RDMA for input\n");
> + error_report("RDMA ERROR: could not open RDMA for input");
> qemu_rdma_cleanup(rdma);
> return;
> }
