+rdma-core
Is global variable *errno* reliable when the documentation only states
"returns 0 on success, or the value of errno on failure (which indicates the failure reason)."
Someone read it as "assign error code to errno and return it.", I used to think the same way.
but ibv_post_send() doesn't always follow this rule. see ibv_post_send() -> mana_post_send()
Actually, QEMU are using errno after calling libibverbs APIs, so we hope the man page can be
more clear. like posix does:
RETURN VALUE
Upon successful completion fopen(), fdopen() and freopen() return a FILE pointer. Otherwise, NULL is returned and errno is set to indicate the error
Thanks
Zhijian
On 27/09/2023 19:46, Markus Armbruster wrote:
> 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
>
