migration/rdma: Plug memory leaks in qemu_rdma_registration_stop()

qemu_rdma_registration_stop() uses the ERROR() macro to create, report
to stderr, and store an Error object.  The stored Error object is
never used, and its memory is leaked.

Even where ERROR() doesn't leak, it is ill-advised.  The whole point
of passing an Error to the caller is letting the caller handle the
error.  Error handling may report to stderr, to somewhere else, or not
at all.  Also reporting in the callee mixes up concerns that should be
kept separate.  Since I don't know what reporting to stderr is
supposed to accomplish, I'm not touching it.

Commit 2a1bc8bde7 "migration/rdma: rdma_accept_incoming_migration fix
error handling" plugged the same leak in
rdma_accept_incoming_migration().

Plug the memory leak the same way: keep the report part, delete the
store part.

The report part uses fprintf().  If it's truly an error, it should use
error_report() instead.  But I don't know, so I leave it alone, just
like commit 2a1bc8bde7 did.

Fixes: 2da776db48
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Cc: Juan Quintela <quintela@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200630090351.1247703-27-armbru@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
This commit is contained in:
Markus Armbruster 2020-06-30 11:03:51 +02:00
parent 7cd1c981eb
commit 9cde9caa04

View File

@ -3787,7 +3787,6 @@ static int qemu_rdma_registration_start(QEMUFile *f, void *opaque,
static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque, static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque,
uint64_t flags, void *data) uint64_t flags, void *data)
{ {
Error *local_err = NULL, **errp = &local_err;
QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(opaque); QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(opaque);
RDMAContext *rdma; RDMAContext *rdma;
RDMAControlHeader head = { .len = 0, .repeat = 1 }; RDMAControlHeader head = { .len = 0, .repeat = 1 };
@ -3832,7 +3831,7 @@ static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque,
&reg_result_idx, rdma->pin_all ? &reg_result_idx, rdma->pin_all ?
qemu_rdma_reg_whole_ram_blocks : NULL); qemu_rdma_reg_whole_ram_blocks : NULL);
if (ret < 0) { if (ret < 0) {
ERROR(errp, "receiving remote info!"); fprintf(stderr, "receiving remote info!");
return ret; return ret;
} }
@ -3851,7 +3850,7 @@ static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque,
*/ */
if (local->nb_blocks != nb_dest_blocks) { if (local->nb_blocks != nb_dest_blocks) {
ERROR(errp, "ram blocks mismatch (Number of blocks %d vs %d) " fprintf(stderr, "ram blocks mismatch (Number of blocks %d vs %d) "
"Your QEMU command line parameters are probably " "Your QEMU command line parameters are probably "
"not identical on both the source and destination.", "not identical on both the source and destination.",
local->nb_blocks, nb_dest_blocks); local->nb_blocks, nb_dest_blocks);
@ -3867,7 +3866,7 @@ static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque,
/* We require that the blocks are in the same order */ /* We require that the blocks are in the same order */
if (rdma->dest_blocks[i].length != local->block[i].length) { if (rdma->dest_blocks[i].length != local->block[i].length) {
ERROR(errp, "Block %s/%d has a different length %" PRIu64 fprintf(stderr, "Block %s/%d has a different length %" PRIu64
"vs %" PRIu64, local->block[i].block_name, i, "vs %" PRIu64, local->block[i].block_name, i,
local->block[i].length, local->block[i].length,
rdma->dest_blocks[i].length); rdma->dest_blocks[i].length);