[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 39/52] migration/rdma: Convert qemu_rdma_write_one() to Error
From: |
Markus Armbruster |
Subject: |
Re: [PATCH 39/52] migration/rdma: Convert qemu_rdma_write_one() to Error |
Date: |
Wed, 27 Sep 2023 13:46:40 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) |
migration/rdma.c uses errno directly or via perror() after the following
functions:
* poll()
POSIX specifies errno is set on error. Good.
* rdma_get_cm_event(), rdma_connect(), rdma_get_cm_event()
Manual page promises "if an error occurs, errno will be set". Good.
* ibv_open_device()
Manual page does not mention errno. Using it seems ill-advised.
qemu_rdma_broken_ipv6_kernel() recovers from EPERM by trying the next
device. Wrong if ibv_open_device() doesn't actually set errno.
What is to be done here?
* ibv_reg_mr()
Manual page does not mention errno. Using it seems ill-advised.
qemu_rdma_reg_whole_ram_blocks() and qemu_rdma_register_and_get_keys()
recover from errno = ENOTSUP by retrying with modified @access
argument. Wrong if ibv_reg_mr() doesn't actually set errno.
What is to be done here?
* ibv_get_cq_event()
Manual page does not mention errno. Using it seems ill-advised.
qemu_rdma_block_for_wrid() calls perror(). Removed in PATCH 48. Good
enough.
* ibv_post_send()
Manual page has the function return "the value of errno on failure".
Sounds like it sets errno to the value it returns. However, the
rdma-core repository defines it as
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);
}
and at least one of the methods fails without setting errno:
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;
}
qemu_rdma_write_one() calls perror(). PATCH 39 (this one) replaces it
by error_setg(), not error_setg_errno(). Seems prudent, but should be
called out in the commit message.
* ibv_advise_mr()
Manual page has the function return "the value of errno on failure".
Sounds like it sets errno to the value it returns, but my findings for
ibv_post_send() make me doubt it.
qemu_rdma_advise_prefetch_mr() traces strerror(errno). Could be
misleading. Drop that part?
* ibv_dereg_mr()
Manual page has the function return "the value of errno on failure".
Sounds like it sets errno to the value it returns, but my findings for
ibv_post_send() make me doubt it.
qemu_rdma_unregister_waiting() calls perror(). Removed in PATCH 51.
Good enough.
* qemu_get_cm_event_timeout()
Can fail without setting errno.
qemu_rdma_connect() calls perror(). Removed in PATCH 45. Good
enough.
Thoughts?
[...]
[*] https://github.com/linux-rdma/rdma-core.git
commit 55fa316b4b18f258d8ac1ceb4aa5a7a35b094dcf
- Re: [PATCH 50/52] migration/rdma: Silence qemu_rdma_cleanup(), (continued)
[PATCH 36/52] migration/rdma: Convert qemu_rdma_exchange_get_response() to Error, Markus Armbruster, 2023/09/18
[PATCH 39/52] migration/rdma: Convert qemu_rdma_write_one() to Error, Markus Armbruster, 2023/09/18
Re: [PATCH 39/52] migration/rdma: Convert qemu_rdma_write_one() to Error, Markus Armbruster, 2023/09/26
[PATCH 04/52] migration/rdma: Drop fragile wr_id formatting, Markus Armbruster, 2023/09/18
[PATCH 25/52] migration/rdma: Dumb down remaining int error values to -1, Markus Armbruster, 2023/09/18
[PATCH 26/52] migration/rdma: Replace int error_state by bool errored, Markus Armbruster, 2023/09/18