[PATCH 15/52] migration/rdma: Ditch useless numeric error codes in error messages

Markus Armbruster posted 52 patches 11 months, 3 weeks ago
Maintainers: Juan Quintela <quintela@redhat.com>, Peter Xu <peterx@redhat.com>, Leonardo Bras <leobras@redhat.com>
There is a newer version of this series
[PATCH 15/52] migration/rdma: Ditch useless numeric error codes in error messages
Posted by Markus Armbruster 11 months, 3 weeks ago
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(-)

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;
     }
 
-- 
2.41.0
Re: [PATCH 15/52] migration/rdma: Ditch useless numeric error codes in error messages
Posted by Zhijian Li (Fujitsu) 11 months, 3 weeks ago

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;
>       }
>   
Re: [PATCH 15/52] migration/rdma: Ditch useless numeric error codes in error messages
Posted by Markus Armbruster 11 months, 3 weeks ago
"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!
Re: [PATCH 15/52] migration/rdma: Ditch useless numeric error codes in error messages
Posted by Fabiano Rosas 11 months, 3 weeks ago
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>