qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: qemu-devel@nongnu.org
Cc: quintela@redhat.com, peterx@redhat.com, leobras@redhat.com,
	farosas@suse.de, lizhijian@fujitsu.com, eblake@redhat.com
Subject: [PATCH v2 18/53] migration/rdma: Fix io_writev(), io_readv() methods to obey contract
Date: Thu, 28 Sep 2023 15:19:44 +0200	[thread overview]
Message-ID: <20230928132019.2544702-19-armbru@redhat.com> (raw)
In-Reply-To: <20230928132019.2544702-1-armbru@redhat.com>

QIOChannelClass methods qio_channel_rdma_readv() and
qio_channel_rdma_writev() violate their method contract when
rdma->error_state is non-zero:

1. They return whatever is in rdma->error_state then.  Only -1 will be
   fine.  -2 will be misinterpreted as "would block".  Anything less
   than -2 isn't defined in the contract.  A positive value would be
   misinterpreted as success, but I believe that's not actually
   possible.

2. They neglect to set an error then.  If something up the call stack
   dereferences the error when failure is returned, it will crash.  If
   it ignores the return value and checks the error instead, it will
   miss the error.

Crap like this happens when return statements hide in macros,
especially when their uses are far away from the definition.

I elected not to investigate how callers are impacted.

Expand the two bad macro uses, so we can set an error and return -1.
The next commit will then get rid of the macro altogether.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Li Zhijian <lizhijian@fujitsu.com>
---
 migration/rdma.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index 0d2d119e6a..fb89b89e80 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -2791,7 +2791,11 @@ static ssize_t qio_channel_rdma_writev(QIOChannel *ioc,
         return -1;
     }
 
-    CHECK_ERROR_STATE();
+    if (rdma->error_state) {
+        error_setg(errp,
+                   "RDMA is in an error state waiting migration to abort!");
+        return -1;
+    }
 
     /*
      * Push out any writes that
@@ -2877,7 +2881,11 @@ static ssize_t qio_channel_rdma_readv(QIOChannel *ioc,
         return -1;
     }
 
-    CHECK_ERROR_STATE();
+    if (rdma->error_state) {
+        error_setg(errp,
+                   "RDMA is in an error state waiting migration to abort!");
+        return -1;
+    }
 
     for (i = 0; i < niov; i++) {
         size_t want = iov[i].iov_len;
-- 
2.41.0



  parent reply	other threads:[~2023-09-28 13:21 UTC|newest]

Thread overview: 121+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-28 13:19 [PATCH v2 00/53] migration/rdma: Error handling fixes Markus Armbruster
2023-09-28 13:19 ` [PATCH v2 01/53] migration/rdma: Clean up qemu_rdma_poll()'s return type Markus Armbruster
2023-10-04 14:26   ` Juan Quintela
2023-09-28 13:19 ` [PATCH v2 02/53] migration/rdma: Clean up qemu_rdma_data_init()'s " Markus Armbruster
2023-10-04 14:35   ` Juan Quintela
2023-09-28 13:19 ` [PATCH v2 03/53] migration/rdma: Clean up rdma_delete_block()'s " Markus Armbruster
2023-10-04 14:36   ` Juan Quintela
2023-09-28 13:19 ` [PATCH v2 04/53] migration/rdma: Drop fragile wr_id formatting Markus Armbruster
2023-10-04 14:38   ` Juan Quintela
2023-09-28 13:19 ` [PATCH v2 05/53] migration/rdma: Consistently use uint64_t for work request IDs Markus Armbruster
2023-10-04 14:39   ` Juan Quintela
2023-09-28 13:19 ` [PATCH v2 06/53] migration/rdma: Fix unwanted integer truncation Markus Armbruster
2023-09-28 14:20   ` Fabiano Rosas
2023-10-04 14:41   ` Juan Quintela
2023-10-07  1:53   ` Zhijian Li (Fujitsu)
2023-09-28 13:19 ` [PATCH v2 07/53] migration/rdma: Clean up two more harmless signed vs. unsigned issues Markus Armbruster
2023-10-04 14:44   ` Juan Quintela
2023-10-07  2:38   ` Zhijian Li (Fujitsu)
2023-09-28 13:19 ` [PATCH v2 08/53] migration/rdma: Give qio_channel_rdma_source_funcs internal linkage Markus Armbruster
2023-10-04 14:50   ` Juan Quintela
2023-09-28 13:19 ` [PATCH v2 09/53] migration/rdma: Fix qemu_rdma_accept() to return failure on errors Markus Armbruster
2023-10-04 14:51   ` Juan Quintela
2023-09-28 13:19 ` [PATCH v2 10/53] migration/rdma: Put @errp parameter last Markus Armbruster
2023-10-04 14:54   ` Juan Quintela
2023-09-28 13:19 ` [PATCH v2 11/53] migration/rdma: Eliminate error_propagate() Markus Armbruster
2023-10-04 14:58   ` Juan Quintela
2023-09-28 13:19 ` [PATCH v2 12/53] migration/rdma: Drop rdma_add_block() error handling Markus Armbruster
2023-10-04 14:58   ` Juan Quintela
2023-09-28 13:19 ` [PATCH v2 13/53] migration/rdma: Drop qemu_rdma_search_ram_block() " Markus Armbruster
2023-10-04 15:00   ` Juan Quintela
2023-09-28 13:19 ` [PATCH v2 14/53] migration/rdma: Make qemu_rdma_buffer_mergeable() return bool Markus Armbruster
2023-10-04 15:01   ` Juan Quintela
2023-09-28 13:19 ` [PATCH v2 15/53] migration/rdma: Use bool for two RDMAContext flags Markus Armbruster
2023-10-04 15:56   ` Juan Quintela
2023-09-28 13:19 ` [PATCH v2 16/53] migration/rdma: Fix or document problematic uses of errno Markus Armbruster
2023-09-29 15:09   ` Fabiano Rosas
2023-10-04 11:12     ` Markus Armbruster
2023-10-05  6:46   ` Juan Quintela
2023-10-07  5:34   ` Zhijian Li (Fujitsu)
2023-09-28 13:19 ` [PATCH v2 17/53] migration/rdma: Ditch useless numeric error codes in error messages Markus Armbruster
2023-10-04 15:06   ` Juan Quintela
2023-09-28 13:19 ` Markus Armbruster [this message]
2023-10-04 15:09   ` [PATCH v2 18/53] migration/rdma: Fix io_writev(), io_readv() methods to obey contract Juan Quintela
2023-09-28 13:19 ` [PATCH v2 19/53] migration/rdma: Replace dangerous macro CHECK_ERROR_STATE() Markus Armbruster
2023-10-04 15:10   ` Juan Quintela
2023-09-28 13:19 ` [PATCH v2 20/53] migration/rdma: Fix qemu_rdma_broken_ipv6_kernel() to set error Markus Armbruster
2023-10-04 15:10   ` Juan Quintela
2023-09-28 13:19 ` [PATCH v2 21/53] migration/rdma: Fix qemu_get_cm_event_timeout() to always " Markus Armbruster
2023-10-04 15:25   ` Juan Quintela
2023-09-28 13:19 ` [PATCH v2 22/53] migration/rdma: Drop dead qemu_rdma_data_init() code for !@host_port Markus Armbruster
2023-09-29 15:10   ` Fabiano Rosas
2023-10-04 15:24   ` Juan Quintela
2023-10-07  5:36   ` Zhijian Li (Fujitsu)
2023-09-28 13:19 ` [PATCH v2 23/53] migration/rdma: Fix QEMUFileHooks method return values Markus Armbruster
2023-10-04 15:28   ` Juan Quintela
2023-10-04 16:22   ` Juan Quintela
2023-10-04 16:37     ` Markus Armbruster
2023-09-28 13:19 ` [PATCH v2 24/53] migration/rdma: Fix rdma_getaddrinfo() error checking Markus Armbruster
2023-10-04 15:30   ` Juan Quintela
2023-09-28 13:19 ` [PATCH v2 25/53] migration/rdma: Return -1 instead of negative errno code Markus Armbruster
2023-10-04 16:19   ` Juan Quintela
2023-10-04 16:23   ` Juan Quintela
2023-09-28 13:19 ` [PATCH v2 26/53] migration/rdma: Dumb down remaining int error values to -1 Markus Armbruster
2023-10-04 16:25   ` Juan Quintela
2023-09-28 13:19 ` [PATCH v2 27/53] migration/rdma: Replace int error_state by bool errored Markus Armbruster
2023-10-04 16:25   ` Juan Quintela
2023-09-28 13:19 ` [PATCH v2 28/53] migration/rdma: Drop superfluous assignments to @ret Markus Armbruster
2023-10-04 16:27   ` Juan Quintela
2023-09-28 13:19 ` [PATCH v2 29/53] migration/rdma: Check negative error values the same way everywhere Markus Armbruster
2023-09-29 15:28   ` Fabiano Rosas
2023-10-04 16:33   ` Juan Quintela
2023-09-28 13:19 ` [PATCH v2 30/53] migration/rdma: Plug a memory leak and improve a message Markus Armbruster
2023-10-04 16:27   ` Juan Quintela
2023-09-28 13:19 ` [PATCH v2 31/53] migration/rdma: Delete inappropriate error_report() in macro ERROR() Markus Armbruster
2023-10-04 16:50   ` Juan Quintela
2023-09-28 13:19 ` [PATCH v2 32/53] migration/rdma: Retire " Markus Armbruster
2023-10-04 16:50   ` Juan Quintela
2023-09-28 13:19 ` [PATCH v2 33/53] migration/rdma: Fix error handling around rdma_getaddrinfo() Markus Armbruster
2023-10-04 16:51   ` Juan Quintela
2023-09-28 13:20 ` [PATCH v2 34/53] migration/rdma: Drop "@errp is clear" guards around error_setg() Markus Armbruster
2023-10-04 16:52   ` Juan Quintela
2023-09-28 13:20 ` [PATCH v2 35/53] migration/rdma: Convert qemu_rdma_exchange_recv() to Error Markus Armbruster
2023-10-04 16:53   ` Juan Quintela
2023-09-28 13:20 ` [PATCH v2 36/53] migration/rdma: Convert qemu_rdma_exchange_send() " Markus Armbruster
2023-10-04 16:55   ` Juan Quintela
2023-09-28 13:20 ` [PATCH v2 37/53] migration/rdma: Convert qemu_rdma_exchange_get_response() " Markus Armbruster
2023-10-04 16:55   ` Juan Quintela
2023-09-28 13:20 ` [PATCH v2 38/53] migration/rdma: Convert qemu_rdma_reg_whole_ram_blocks() " Markus Armbruster
2023-10-04 16:56   ` Juan Quintela
2023-09-28 13:20 ` [PATCH v2 39/53] migration/rdma: Convert qemu_rdma_write_flush() " Markus Armbruster
2023-10-04 16:56   ` Juan Quintela
2023-09-28 13:20 ` [PATCH v2 40/53] migration/rdma: Convert qemu_rdma_write_one() " Markus Armbruster
2023-10-04 16:56   ` Juan Quintela
2023-09-28 13:20 ` [PATCH v2 41/53] migration/rdma: Convert qemu_rdma_write() " Markus Armbruster
2023-10-04 17:23   ` Juan Quintela
2023-09-28 13:20 ` [PATCH v2 42/53] migration/rdma: Convert qemu_rdma_post_send_control() " Markus Armbruster
2023-09-28 13:20 ` [PATCH v2 43/53] migration/rdma: Convert qemu_rdma_post_recv_control() " Markus Armbruster
2023-09-28 13:20 ` [PATCH v2 44/53] migration/rdma: Convert qemu_rdma_alloc_pd_cq() " Markus Armbruster
2023-09-28 13:20 ` [PATCH v2 45/53] migration/rdma: Silence qemu_rdma_resolve_host() Markus Armbruster
2023-09-28 13:20 ` [PATCH v2 46/53] migration/rdma: Silence qemu_rdma_connect() Markus Armbruster
2023-09-28 13:20 ` [PATCH v2 47/53] migration/rdma: Silence qemu_rdma_reg_control() Markus Armbruster
2023-09-28 13:20 ` [PATCH v2 48/53] migration/rdma: Don't report received completion events as error Markus Armbruster
2023-09-28 13:20 ` [PATCH v2 49/53] migration/rdma: Silence qemu_rdma_block_for_wrid() Markus Armbruster
2023-09-28 13:20 ` [PATCH v2 50/53] migration/rdma: Silence qemu_rdma_register_and_get_keys() Markus Armbruster
2023-09-28 13:20 ` [PATCH v2 51/53] migration/rdma: Downgrade qemu_rdma_cleanup() errors to warnings Markus Armbruster
2023-09-29 15:29   ` Fabiano Rosas
2023-10-04 17:47   ` Juan Quintela
2023-10-07  3:50   ` Zhijian Li (Fujitsu)
2023-09-28 13:20 ` [PATCH v2 52/53] migration/rdma: Use error_report() & friends instead of stderr Markus Armbruster
2023-09-29 15:36   ` Fabiano Rosas
2023-10-04 11:15     ` Markus Armbruster
2023-10-04 13:52       ` Fabiano Rosas
2023-10-05  7:24   ` Juan Quintela
2023-10-07  3:56   ` Zhijian Li (Fujitsu)
2023-09-28 13:20 ` [PATCH v2 53/53] migration/rdma: Replace flawed device detail dump by tracing Markus Armbruster
2023-09-29 17:05   ` Fabiano Rosas
2023-10-04 17:50   ` Juan Quintela
2023-10-07  3:57   ` Zhijian Li (Fujitsu)
2023-10-04 17:52 ` [PATCH v2 00/53] migration/rdma: Error handling fixes Juan Quintela
2023-10-05  5:07   ` Markus Armbruster
2023-10-05  6:37     ` Juan Quintela

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230928132019.2544702-19-armbru@redhat.com \
    --to=armbru@redhat.com \
    --cc=eblake@redhat.com \
    --cc=farosas@suse.de \
    --cc=leobras@redhat.com \
    --cc=lizhijian@fujitsu.com \
    --cc=peterx@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).