diff options
author | John Levon <john.levon@nutanix.com> | 2021-02-11 21:03:02 +0000 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-02-11 21:03:02 +0000 |
commit | 3d60fca4d4b4ef100d23f3470a5456aef1b1a68e (patch) | |
tree | fac268d9848213fea715597f4087c4c77c731784 | |
parent | 0cfb9449dc1469cb8b12369afc2c5881d6b544fd (diff) | |
download | libvfio-user-3d60fca4d4b4ef100d23f3470a5456aef1b1a68e.zip libvfio-user-3d60fca4d4b4ef100d23f3470a5456aef1b1a68e.tar.gz libvfio-user-3d60fca4d4b4ef100d23f3470a5456aef1b1a68e.tar.bz2 |
move exec_command socket handling into the transport (#320)
Also clean up some code surrounding this. In particular, don't play games with
modifying the message header.
Signed-off-by: John Levon <john.levon@nutanix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@nutanix.com>
-rw-r--r-- | lib/libvfio-user.c | 198 | ||||
-rw-r--r-- | lib/private.h | 3 | ||||
-rw-r--r-- | lib/tran_sock.c | 38 |
3 files changed, 135 insertions, 104 deletions
diff --git a/lib/libvfio-user.c b/lib/libvfio-user.c index 4d37a5d..9b87899 100644 --- a/lib/libvfio-user.c +++ b/lib/libvfio-user.c @@ -670,12 +670,13 @@ validate_header(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr, size_t size) } if (hdr->flags.type != VFIO_USER_F_TYPE_COMMAND) { - vfu_log(vfu_ctx, LOG_ERR, "header not a request"); + vfu_log(vfu_ctx, LOG_ERR, "msg%#hx: not a command req", hdr->msg_id); return -EINVAL; } if (hdr->msg_size < sizeof hdr) { - vfu_log(vfu_ctx, LOG_ERR, "bad size in header %d", hdr->msg_size); + vfu_log(vfu_ctx, LOG_ERR, "msg%#hx: bad size %d in header", + hdr->msg_id, hdr->msg_size); return -EINVAL; } @@ -734,6 +735,7 @@ exec_command(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr, size_t size, struct vfio_device_info *dev_info; struct vfio_region_info *dev_region_info_in, *dev_region_info_out = NULL; void *cmd_data = NULL; + size_t cmd_data_size; assert(vfu_ctx != NULL); assert(hdr != NULL); @@ -747,29 +749,13 @@ exec_command(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr, size_t size, return ret; } - /* - * TODO from now on if an error occurs we still need to reply. Move this - * code into a separate function so that we don't have to use goto. - */ + cmd_data_size = hdr->msg_size - sizeof (*hdr); + + if (cmd_data_size > 0) { + ret = vfu_ctx->tran->recv_body(vfu_ctx, hdr, &cmd_data); - hdr->msg_size -= sizeof(struct vfio_user_header); - if (hdr->msg_size > 0) { - cmd_data = malloc(hdr->msg_size); - if (cmd_data == NULL) { - ret = -ENOMEM; - goto reply; - } - // FIXME: should be transport op - ret = recv(vfu_ctx->conn_fd, cmd_data, hdr->msg_size, 0); if (ret < 0) { - ret = -errno; - goto reply; - } - if (ret != (int)hdr->msg_size) { - vfu_log(vfu_ctx, LOG_ERR, "short read, expected=%d, actual=%d", - hdr->msg_size, ret); - ret = -EINVAL; - goto reply; + return ret; } } @@ -778,90 +764,98 @@ exec_command(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr, size_t size, vfu_log(vfu_ctx, LOG_ERR, "bad command %d while device in stop-and-copy state", hdr->cmd); ret = -EINVAL; - goto reply; + goto out; } switch (hdr->cmd) { - case VFIO_USER_DMA_MAP: - case VFIO_USER_DMA_UNMAP: - ret = handle_dma_map_or_unmap(vfu_ctx, hdr->msg_size, - hdr->cmd == VFIO_USER_DMA_MAP, - fds, nr_fds, cmd_data); - break; - case VFIO_USER_DEVICE_GET_INFO: - dev_info = calloc(1, sizeof *dev_info); - if (dev_info == NULL) { - ret = -ENOMEM; - goto reply; - } - ret = handle_device_get_info(vfu_ctx, hdr->msg_size, dev_info); - if (ret >= 0) { - _iovecs[1].iov_base = dev_info; - _iovecs[1].iov_len = dev_info->argsz; - *iovecs = _iovecs; - *nr_iovecs = 2; - } - break; - case VFIO_USER_DEVICE_GET_REGION_INFO: - dev_region_info_in = cmd_data; - ret = handle_device_get_region_info(vfu_ctx, hdr->msg_size, - dev_region_info_in, - &dev_region_info_out, fds_out, - nr_fds_out); - if (ret == 0) { - _iovecs[1].iov_base = dev_region_info_out; - _iovecs[1].iov_len = dev_region_info_in->argsz; - *iovecs = _iovecs; - *nr_iovecs = 2; - } - break; - case VFIO_USER_DEVICE_GET_IRQ_INFO: - irq_info = calloc(1, sizeof *irq_info); - if (irq_info == NULL) { - ret = -ENOMEM; - goto reply; - } - ret = handle_device_get_irq_info(vfu_ctx, hdr->msg_size, cmd_data, - irq_info); - if (ret == 0) { - _iovecs[1].iov_base = irq_info; - _iovecs[1].iov_len = sizeof *irq_info; - *iovecs = _iovecs; - *nr_iovecs = 2; - } - break; - case VFIO_USER_DEVICE_SET_IRQS: - ret = handle_device_set_irqs(vfu_ctx, hdr->msg_size, fds, nr_fds, - cmd_data); - break; - case VFIO_USER_REGION_READ: - case VFIO_USER_REGION_WRITE: - ret = handle_region_access(vfu_ctx, hdr->msg_size, hdr->cmd, - &(_iovecs[1].iov_base), - &(_iovecs[1].iov_len), - cmd_data); - if (ret == 0) { - *iovecs = _iovecs; - *nr_iovecs = 2; - } + case VFIO_USER_DMA_MAP: + case VFIO_USER_DMA_UNMAP: + ret = handle_dma_map_or_unmap(vfu_ctx, cmd_data_size, + hdr->cmd == VFIO_USER_DMA_MAP, + fds, nr_fds, cmd_data); + break; + + case VFIO_USER_DEVICE_GET_INFO: + dev_info = calloc(1, sizeof *dev_info); + if (dev_info == NULL) { + ret = -ENOMEM; break; - case VFIO_USER_DEVICE_RESET: - ret = handle_device_reset(vfu_ctx); + } + ret = handle_device_get_info(vfu_ctx, cmd_data_size, dev_info); + if (ret >= 0) { + _iovecs[1].iov_base = dev_info; + _iovecs[1].iov_len = dev_info->argsz; + *iovecs = _iovecs; + *nr_iovecs = 2; + } + break; + + case VFIO_USER_DEVICE_GET_REGION_INFO: + dev_region_info_in = cmd_data; + ret = handle_device_get_region_info(vfu_ctx, cmd_data_size, + dev_region_info_in, + &dev_region_info_out, fds_out, + nr_fds_out); + if (ret == 0) { + _iovecs[1].iov_base = dev_region_info_out; + _iovecs[1].iov_len = dev_region_info_in->argsz; + *iovecs = _iovecs; + *nr_iovecs = 2; + } + break; + + case VFIO_USER_DEVICE_GET_IRQ_INFO: + irq_info = calloc(1, sizeof *irq_info); + if (irq_info == NULL) { + ret = -ENOMEM; break; - case VFIO_USER_DIRTY_PAGES: - // FIXME: don't allow migration calls if migration == NULL - ret = handle_dirty_pages(vfu_ctx, hdr->msg_size, iovecs, nr_iovecs, + } + ret = handle_device_get_irq_info(vfu_ctx, cmd_data_size, cmd_data, + irq_info); + if (ret == 0) { + _iovecs[1].iov_base = irq_info; + _iovecs[1].iov_len = sizeof *irq_info; + *iovecs = _iovecs; + *nr_iovecs = 2; + } + break; + + case VFIO_USER_DEVICE_SET_IRQS: + ret = handle_device_set_irqs(vfu_ctx, cmd_data_size, fds, nr_fds, cmd_data); - if (ret >= 0) { - *free_iovec_data = false; - } - break; - default: - vfu_log(vfu_ctx, LOG_ERR, "bad command %d", hdr->cmd); - ret = -EINVAL; - goto reply; + break; + + case VFIO_USER_REGION_READ: + case VFIO_USER_REGION_WRITE: + ret = handle_region_access(vfu_ctx, cmd_data_size, hdr->cmd, + &(_iovecs[1].iov_base), + &(_iovecs[1].iov_len), + cmd_data); + if (ret == 0) { + *iovecs = _iovecs; + *nr_iovecs = 2; + } + break; + + case VFIO_USER_DEVICE_RESET: + ret = handle_device_reset(vfu_ctx); + break; + + case VFIO_USER_DIRTY_PAGES: + // FIXME: don't allow migration calls if migration == NULL + ret = handle_dirty_pages(vfu_ctx, cmd_data_size, iovecs, nr_iovecs, + cmd_data); + if (ret >= 0) { + *free_iovec_data = false; + } + break; + default: + vfu_log(vfu_ctx, LOG_ERR, "bad command %d", hdr->cmd); + ret = -EINVAL; + break; } -reply: + +out: free(cmd_data); return ret; } @@ -922,8 +916,8 @@ process_request(vfu_ctx_t *vfu_ctx) */ if (ret < 0) { - vfu_log(vfu_ctx, LOG_ERR, "failed to handle command %d: %s", hdr.cmd, - strerror(-ret)); + vfu_log(vfu_ctx, LOG_ERR, "msg%#hx: cmd %d failed: %s", hdr.msg_id, + hdr.cmd, strerror(-ret)); } else { ret = 0; } diff --git a/lib/private.h b/lib/private.h index be5cf6d..c38cc09 100644 --- a/lib/private.h +++ b/lib/private.h @@ -57,6 +57,9 @@ struct transport_ops { int (*get_request)(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr, int *fds, size_t *nr_fds); + int (*recv_body)(vfu_ctx_t *vfu_ctx, const struct vfio_user_header *hdr, + void **datap); + int (*reply)(vfu_ctx_t *vfu_ctx, uint16_t msg_id, struct iovec *iovecs, size_t nr_iovecs, int *fds, int count, int err); diff --git a/lib/tran_sock.c b/lib/tran_sock.c index 311f480..4762c71 100644 --- a/lib/tran_sock.c +++ b/lib/tran_sock.c @@ -524,7 +524,7 @@ recv_version(vfu_ctx_t *vfu_ctx, int sock, uint16_t *msg_idp, } if (hdr.cmd != VFIO_USER_VERSION) { - vfu_log(vfu_ctx, LOG_ERR, "msg%hx: invalid cmd %hu (expected %hu)", + vfu_log(vfu_ctx, LOG_ERR, "msg%#hx: invalid cmd %hu (expected %hu)", *msg_idp, hdr.cmd, VFIO_USER_VERSION); ret = -EINVAL; goto out; @@ -532,7 +532,7 @@ recv_version(vfu_ctx_t *vfu_ctx, int sock, uint16_t *msg_idp, if (vlen < sizeof (*cversion)) { vfu_log(vfu_ctx, LOG_ERR, - "msg%hx (VFIO_USER_VERSION): invalid size %lu", *msg_idp, vlen); + "msg%#hx: VFIO_USER_VERSION: invalid size %lu", *msg_idp, vlen); ret = -EINVAL; goto out; } @@ -713,6 +713,39 @@ tran_sock_get_request(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr, } static int +tran_sock_recv_body(vfu_ctx_t *vfu_ctx, const struct vfio_user_header *hdr, + void **datap) +{ + size_t body_size = hdr->msg_size - sizeof (*hdr); + void *data; + int ret; + + // FIXME: should check max-msg-size + data = malloc(body_size); + + if (data == NULL) { + return -errno; + } + + ret = recv(vfu_ctx->conn_fd, data, body_size, 0); + if (ret < 0) { + ret = -errno; + free(data); + return ret; + } + + if (ret != (int)body_size) { + vfu_log(vfu_ctx, LOG_ERR, "msg%#hx: short read: expected=%d, actual=%d", + hdr->msg_id, body_size, ret); + free(data); + return -EINVAL; + } + + *datap = data; + return 0; +} + +static int tran_sock_reply(vfu_ctx_t *vfu_ctx, uint16_t msg_id, struct iovec *iovecs, size_t nr_iovecs, int *fds, int count, int err) @@ -755,6 +788,7 @@ struct transport_ops tran_sock_ops = { .init = tran_sock_init, .attach = tran_sock_attach, .get_request = tran_sock_get_request, + .recv_body = tran_sock_recv_body, .reply = tran_sock_reply, .send_msg = tran_sock_send_msg, .detach = tran_sock_detach, |