Message ID | 20230918144206.560120-16-armbru@redhat.com |
---|---|
State | New |
Headers | show |
Series | migration/rdma: Error handling fixes | expand |
Markus Armbruster <armbru@redhat.com> writes: > Several error messages include numeric error codes returned by failed > functions: > > * ibv_poll_cq() returns an unspecified negative value. Useless. > > * rdma_accept and rmda_get_cm_event() return -1. Useless. > > * qemu_rdma_poll() returns either -1 or an unspecified negative > value. Useless. > > * qemu_rdma_block_for_wrid(), qemu_rdma_write_flush(), > qemu_rdma_exchange_send(), qemu_rdma_exchange_recv(), > qemu_rdma_write() return a negative value that may or may not be an > errno value. While reporting human-readable errno > information (which a number is not) can be useful, reporting an > error code that may or may not be an errno value is useless. > > Drop these error codes from the error messages. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Fabiano Rosas <farosas@suse.de>
On 18/09/2023 22:41, Markus Armbruster wrote: > Several error messages include numeric error codes returned by failed > functions: > > * ibv_poll_cq() returns an unspecified negative value. Useless. > > * rdma_accept and rmda_get_cm_event() return -1. Useless. s/rmda_get_cm_event/rdma_get_cm_event Otherwise, Reviewed-by: Li Zhijian <lizhijian@fujitsu.com> > > * qemu_rdma_poll() returns either -1 or an unspecified negative > value. Useless. > > * qemu_rdma_block_for_wrid(), qemu_rdma_write_flush(), > qemu_rdma_exchange_send(), qemu_rdma_exchange_recv(), > qemu_rdma_write() return a negative value that may or may not be an > errno value. While reporting human-readable errno > information (which a number is not) can be useful, reporting an > error code that may or may not be an errno value is useless. > > Drop these error codes from the error messages. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > migration/rdma.c | 20 ++++++++++---------- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/migration/rdma.c b/migration/rdma.c > index c02a1c83b2..2173cb076f 100644 > --- a/migration/rdma.c > +++ b/migration/rdma.c > @@ -1460,7 +1460,7 @@ static int qemu_rdma_poll(RDMAContext *rdma, struct ibv_cq *cq, > } > > if (ret < 0) { > - error_report("ibv_poll_cq return %d", ret); > + error_report("ibv_poll_cq failed"); > return ret; > } > > @@ -2194,7 +2194,7 @@ retry: > ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_RDMA_WRITE, NULL); > if (ret < 0) { > error_report("rdma migration: failed to make " > - "room in full send queue! %d", ret); > + "room in full send queue!"); > return ret; > } > > @@ -2770,7 +2770,7 @@ static ssize_t qio_channel_rdma_writev(QIOChannel *ioc, > ret = qemu_rdma_write_flush(f, rdma); > if (ret < 0) { > rdma->error_state = ret; > - error_setg(errp, "qemu_rdma_write_flush returned %d", ret); > + error_setg(errp, "qemu_rdma_write_flush failed"); > return -1; > } > > @@ -2790,7 +2790,7 @@ static ssize_t qio_channel_rdma_writev(QIOChannel *ioc, > > if (ret < 0) { > rdma->error_state = ret; > - error_setg(errp, "qemu_rdma_exchange_send returned %d", ret); > + error_setg(errp, "qemu_rdma_exchange_send failed"); > return -1; > } > > @@ -2880,7 +2880,7 @@ static ssize_t qio_channel_rdma_readv(QIOChannel *ioc, > > if (ret < 0) { > rdma->error_state = ret; > - error_setg(errp, "qemu_rdma_exchange_recv returned %d", ret); > + error_setg(errp, "qemu_rdma_exchange_recv failed"); > return -1; > } > > @@ -3222,7 +3222,7 @@ static size_t qemu_rdma_save_page(QEMUFile *f, > */ > ret = qemu_rdma_write(f, rdma, block_offset, offset, size); > if (ret < 0) { > - error_report("rdma migration: write error! %d", ret); > + error_report("rdma migration: write error"); > goto err; > } > > @@ -3249,7 +3249,7 @@ static size_t qemu_rdma_save_page(QEMUFile *f, > uint64_t wr_id, wr_id_in; > int ret = qemu_rdma_poll(rdma, rdma->recv_cq, &wr_id_in, NULL); > if (ret < 0) { > - error_report("rdma migration: polling error! %d", ret); > + error_report("rdma migration: polling error"); > goto err; > } > > @@ -3264,7 +3264,7 @@ static size_t qemu_rdma_save_page(QEMUFile *f, > uint64_t wr_id, wr_id_in; > int ret = qemu_rdma_poll(rdma, rdma->send_cq, &wr_id_in, NULL); > if (ret < 0) { > - error_report("rdma migration: polling error! %d", ret); > + error_report("rdma migration: polling error"); > goto err; > } > > @@ -3439,13 +3439,13 @@ static int qemu_rdma_accept(RDMAContext *rdma) > > ret = rdma_accept(rdma->cm_id, &conn_param); > if (ret) { > - error_report("rdma_accept returns %d", ret); > + error_report("rdma_accept failed"); > goto err_rdma_dest_wait; > } > > ret = rdma_get_cm_event(rdma->channel, &cm_event); > if (ret) { > - error_report("rdma_accept get_cm_event failed %d", ret); > + error_report("rdma_accept get_cm_event failed"); > goto err_rdma_dest_wait; > } >
"Zhijian Li (Fujitsu)" <lizhijian@fujitsu.com> writes: > On 18/09/2023 22:41, Markus Armbruster wrote: >> Several error messages include numeric error codes returned by failed >> functions: >> >> * ibv_poll_cq() returns an unspecified negative value. Useless. >> >> * rdma_accept and rmda_get_cm_event() return -1. Useless. > > > s/rmda_get_cm_event/rdma_get_cm_event Sharp eyes! Will fix. > Otherwise, > Reviewed-by: Li Zhijian <lizhijian@fujitsu.com> Thanks!
diff --git a/migration/rdma.c b/migration/rdma.c index c02a1c83b2..2173cb076f 100644 --- a/migration/rdma.c +++ b/migration/rdma.c @@ -1460,7 +1460,7 @@ static int qemu_rdma_poll(RDMAContext *rdma, struct ibv_cq *cq, } if (ret < 0) { - error_report("ibv_poll_cq return %d", ret); + error_report("ibv_poll_cq failed"); return ret; } @@ -2194,7 +2194,7 @@ retry: ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_RDMA_WRITE, NULL); if (ret < 0) { error_report("rdma migration: failed to make " - "room in full send queue! %d", ret); + "room in full send queue!"); return ret; } @@ -2770,7 +2770,7 @@ static ssize_t qio_channel_rdma_writev(QIOChannel *ioc, ret = qemu_rdma_write_flush(f, rdma); if (ret < 0) { rdma->error_state = ret; - error_setg(errp, "qemu_rdma_write_flush returned %d", ret); + error_setg(errp, "qemu_rdma_write_flush failed"); return -1; } @@ -2790,7 +2790,7 @@ static ssize_t qio_channel_rdma_writev(QIOChannel *ioc, if (ret < 0) { rdma->error_state = ret; - error_setg(errp, "qemu_rdma_exchange_send returned %d", ret); + error_setg(errp, "qemu_rdma_exchange_send failed"); return -1; } @@ -2880,7 +2880,7 @@ static ssize_t qio_channel_rdma_readv(QIOChannel *ioc, if (ret < 0) { rdma->error_state = ret; - error_setg(errp, "qemu_rdma_exchange_recv returned %d", ret); + error_setg(errp, "qemu_rdma_exchange_recv failed"); return -1; } @@ -3222,7 +3222,7 @@ static size_t qemu_rdma_save_page(QEMUFile *f, */ ret = qemu_rdma_write(f, rdma, block_offset, offset, size); if (ret < 0) { - error_report("rdma migration: write error! %d", ret); + error_report("rdma migration: write error"); goto err; } @@ -3249,7 +3249,7 @@ static size_t qemu_rdma_save_page(QEMUFile *f, uint64_t wr_id, wr_id_in; int ret = qemu_rdma_poll(rdma, rdma->recv_cq, &wr_id_in, NULL); if (ret < 0) { - error_report("rdma migration: polling error! %d", ret); + error_report("rdma migration: polling error"); goto err; } @@ -3264,7 +3264,7 @@ static size_t qemu_rdma_save_page(QEMUFile *f, uint64_t wr_id, wr_id_in; int ret = qemu_rdma_poll(rdma, rdma->send_cq, &wr_id_in, NULL); if (ret < 0) { - error_report("rdma migration: polling error! %d", ret); + error_report("rdma migration: polling error"); goto err; } @@ -3439,13 +3439,13 @@ static int qemu_rdma_accept(RDMAContext *rdma) ret = rdma_accept(rdma->cm_id, &conn_param); if (ret) { - error_report("rdma_accept returns %d", ret); + error_report("rdma_accept failed"); goto err_rdma_dest_wait; } ret = rdma_get_cm_event(rdma->channel, &cm_event); if (ret) { - error_report("rdma_accept get_cm_event failed %d", ret); + error_report("rdma_accept get_cm_event failed"); goto err_rdma_dest_wait; }
Several error messages include numeric error codes returned by failed functions: * ibv_poll_cq() returns an unspecified negative value. Useless. * rdma_accept and rmda_get_cm_event() return -1. Useless. * qemu_rdma_poll() returns either -1 or an unspecified negative value. Useless. * qemu_rdma_block_for_wrid(), qemu_rdma_write_flush(), qemu_rdma_exchange_send(), qemu_rdma_exchange_recv(), qemu_rdma_write() return a negative value that may or may not be an errno value. While reporting human-readable errno information (which a number is not) can be useful, reporting an error code that may or may not be an errno value is useless. Drop these error codes from the error messages. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- migration/rdma.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)