[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH v2 16/53] migration/rdma: Fix or document problematic uses of err
From: |
Markus Armbruster |
Subject: |
[PATCH v2 16/53] migration/rdma: Fix or document problematic uses of errno |
Date: |
Thu, 28 Sep 2023 15:19:42 +0200 |
We use errno after calling Libibverbs functions that are not
documented to set errno (manual page does not mention errno), or where
the documentation is unclear ("returns [...] the value of errno on
failure"). While this could be read as "sets errno and returns it",
a glance at the source code[*] kills that hope:
static inline int ibv_post_send(struct ibv_qp *qp, struct ibv_send_wr *wr,
struct ibv_send_wr **bad_wr)
{
return qp->context->ops.post_send(qp, wr, bad_wr);
}
The callback can be
static int mana_post_send(struct ibv_qp *ibqp, struct ibv_send_wr *wr,
struct ibv_send_wr **bad)
{
/* This version of driver supports RAW QP only.
* Posting WR is done directly in the application.
*/
return EOPNOTSUPP;
}
Neither of them touches errno.
One of these errno uses is easy to fix, so do that now. Several more
will go away later in the series; add temporary FIXME commments.
Three will remain; add TODO comments. TODO, not FIXME, because the
bug might be in Libibverbs documentation.
[*] https://github.com/linux-rdma/rdma-core.git
commit 55fa316b4b18f258d8ac1ceb4aa5a7a35b094dcf
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
migration/rdma.c | 45 +++++++++++++++++++++++++++++++++++++++------
1 file changed, 39 insertions(+), 6 deletions(-)
diff --git a/migration/rdma.c b/migration/rdma.c
index 28097ce604..bba8c99fa9 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -853,6 +853,12 @@ static int qemu_rdma_broken_ipv6_kernel(struct ibv_context
*verbs, Error **errp)
for (x = 0; x < num_devices; x++) {
verbs = ibv_open_device(dev_list[x]);
+ /*
+ * ibv_open_device() is not documented to set errno. If
+ * it does, it's somebody else's doc bug. If it doesn't,
+ * the use of errno below is wrong.
+ * TODO Find out whether ibv_open_device() sets errno.
+ */
if (!verbs) {
if (errno == EPERM) {
continue;
@@ -1162,11 +1168,7 @@ static void qemu_rdma_advise_prefetch_mr(struct ibv_pd
*pd, uint64_t addr,
ret = ibv_advise_mr(pd, advice,
IBV_ADVISE_MR_FLAG_FLUSH, &sg_list, 1);
/* ignore the error */
- if (ret) {
- trace_qemu_rdma_advise_mr(name, len, addr, strerror(errno));
- } else {
- trace_qemu_rdma_advise_mr(name, len, addr, "successed");
- }
+ trace_qemu_rdma_advise_mr(name, len, addr, strerror(ret));
#endif
}
@@ -1183,7 +1185,12 @@ static int qemu_rdma_reg_whole_ram_blocks(RDMAContext
*rdma)
local->block[i].local_host_addr,
local->block[i].length, access
);
-
+ /*
+ * ibv_reg_mr() is not documented to set errno. If it does,
+ * it's somebody else's doc bug. If it doesn't, the use of
+ * errno below is wrong.
+ * TODO Find out whether ibv_reg_mr() sets errno.
+ */
if (!local->block[i].mr &&
errno == ENOTSUP && rdma_support_odp(rdma->verbs)) {
access |= IBV_ACCESS_ON_DEMAND;
@@ -1291,6 +1298,12 @@ static int qemu_rdma_register_and_get_keys(RDMAContext
*rdma,
trace_qemu_rdma_register_and_get_keys(len, chunk_start);
block->pmr[chunk] = ibv_reg_mr(rdma->pd, chunk_start, len, access);
+ /*
+ * ibv_reg_mr() is not documented to set errno. If it does,
+ * it's somebody else's doc bug. If it doesn't, the use of
+ * errno below is wrong.
+ * TODO Find out whether ibv_reg_mr() sets errno.
+ */
if (!block->pmr[chunk] &&
errno == ENOTSUP && rdma_support_odp(rdma->verbs)) {
access |= IBV_ACCESS_ON_DEMAND;
@@ -1408,6 +1421,11 @@ 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");
return -ret;
}
@@ -1658,6 +1676,11 @@ static int qemu_rdma_block_for_wrid(RDMAContext *rdma,
ret = ibv_get_cq_event(ch, &cq, &cq_ctx);
if (ret) {
+ /*
+ * FIXME perror() is problematic, because ibv_reg_mr() is
+ * not documented to set errno. Will go away later in
+ * this series.
+ */
perror("ibv_get_cq_event");
goto err_block_for_wrid;
}
@@ -2199,6 +2222,11 @@ retry:
goto retry;
} else if (ret > 0) {
+ /*
+ * FIXME perror() is problematic, because whether
+ * ibv_post_send() sets errno is unclear. Will go away later
+ * in this series.
+ */
perror("rdma migration: post rdma write failed");
return -ret;
}
@@ -2559,6 +2587,11 @@ static int qemu_rdma_connect(RDMAContext *rdma, bool
return_path,
ret = rdma_get_cm_event(rdma->channel, &cm_event);
}
if (ret) {
+ /*
+ * FIXME perror() is wrong, because
+ * qemu_get_cm_event_timeout() can fail without setting errno.
+ * Will go away later in this series.
+ */
perror("rdma_get_cm_event after rdma_connect");
ERROR(errp, "connecting to destination!");
goto err_rdma_source_connect;
--
2.41.0
- [PATCH v2 35/53] migration/rdma: Convert qemu_rdma_exchange_recv() to Error, (continued)
- [PATCH v2 35/53] migration/rdma: Convert qemu_rdma_exchange_recv() to Error, Markus Armbruster, 2023/09/28
- [PATCH v2 11/53] migration/rdma: Eliminate error_propagate(), Markus Armbruster, 2023/09/28
- [PATCH v2 52/53] migration/rdma: Use error_report() & friends instead of stderr, Markus Armbruster, 2023/09/28
- [PATCH v2 22/53] migration/rdma: Drop dead qemu_rdma_data_init() code for !@host_port, Markus Armbruster, 2023/09/28
- [PATCH v2 45/53] migration/rdma: Silence qemu_rdma_resolve_host(), Markus Armbruster, 2023/09/28
- [PATCH v2 40/53] migration/rdma: Convert qemu_rdma_write_one() to Error, Markus Armbruster, 2023/09/28
- [PATCH v2 50/53] migration/rdma: Silence qemu_rdma_register_and_get_keys(), Markus Armbruster, 2023/09/28
- [PATCH v2 27/53] migration/rdma: Replace int error_state by bool errored, Markus Armbruster, 2023/09/28
- [PATCH v2 16/53] migration/rdma: Fix or document problematic uses of errno,
Markus Armbruster <=
- [PATCH v2 14/53] migration/rdma: Make qemu_rdma_buffer_mergeable() return bool, Markus Armbruster, 2023/09/28
- [PATCH v2 44/53] migration/rdma: Convert qemu_rdma_alloc_pd_cq() to Error, Markus Armbruster, 2023/09/28
- [PATCH v2 48/53] migration/rdma: Don't report received completion events as error, Markus Armbruster, 2023/09/28
- [PATCH v2 37/53] migration/rdma: Convert qemu_rdma_exchange_get_response() to Error, Markus Armbruster, 2023/09/28
- [PATCH v2 51/53] migration/rdma: Downgrade qemu_rdma_cleanup() errors to warnings, Markus Armbruster, 2023/09/28
- [PATCH v2 53/53] migration/rdma: Replace flawed device detail dump by tracing, Markus Armbruster, 2023/09/28
- [PATCH v2 49/53] migration/rdma: Silence qemu_rdma_block_for_wrid(), Markus Armbruster, 2023/09/28