diff mbox

[PULL,12/25] vhost: use a function for each call

Message ID CABUUfwNtq988Qcvwyhaj=xD6OGp9uJqADR_i4oq0LFNe2iJE5g@mail.gmail.com
State New
Headers show

Commit Message

Thibaut Collet Oct. 9, 2015, 6:40 a.m. UTC
On Thu, Oct 8, 2015 at 11:17 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Replace the generic vhost_call() by specific functions for each
> function call to help with type safety and changing arguments.
>
> While doing this, I found that "unsigned long long" and "uint64_t" were
> used interchangeably and causing compilation warnings, using uint64_t
> instead, as the vhost & protocol specifies.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  include/hw/virtio/vhost-backend.h |  63 ++++-
>  include/hw/virtio/vhost.h         |  12 +-
>  hw/net/vhost_net.c                |  16 +-
>  hw/scsi/vhost-scsi.c              |   7 +-
>  hw/virtio/vhost-backend.c         | 124 ++++++++-
>  hw/virtio/vhost-user.c            | 518 ++++++++++++++++++++++----------------
>  hw/virtio/vhost.c                 |  36 +--
>  7 files changed, 501 insertions(+), 275 deletions(-)
>
> diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
> index 7064a12..e07118c 100644
> --- a/include/hw/virtio/vhost-backend.h
> +++ b/include/hw/virtio/vhost-backend.h
> @@ -22,28 +22,77 @@ typedef enum VhostBackendType {
>
>  struct vhost_dev;
>  struct vhost_log;
> +struct vhost_memory;
> +struct vhost_vring_file;
> +struct vhost_vring_state;
> +struct vhost_vring_addr;
> +struct vhost_scsi_target;
>
> -typedef int (*vhost_call)(struct vhost_dev *dev, unsigned long int request,
> -             void *arg);
>  typedef int (*vhost_backend_init)(struct vhost_dev *dev, void *opaque);
>  typedef int (*vhost_backend_cleanup)(struct vhost_dev *dev);
> -typedef int (*vhost_backend_get_vq_index)(struct vhost_dev *dev, int idx);
> -typedef int (*vhost_backend_set_vring_enable)(struct vhost_dev *dev, int enable);
>
> +typedef int (*vhost_net_set_backend_op)(struct vhost_dev *dev,
> +                                struct vhost_vring_file *file);
> +typedef int (*vhost_scsi_set_endpoint_op)(struct vhost_dev *dev,
> +                                  struct vhost_scsi_target *target);
> +typedef int (*vhost_scsi_clear_endpoint_op)(struct vhost_dev *dev,
> +                                    struct vhost_scsi_target *target);
> +typedef int (*vhost_scsi_get_abi_version_op)(struct vhost_dev *dev,
> +                                             int *version);
>  typedef int (*vhost_set_log_base_op)(struct vhost_dev *dev, uint64_t base,
>                                       struct vhost_log *log);
> +typedef int (*vhost_set_mem_table_op)(struct vhost_dev *dev,
> +                                      struct vhost_memory *mem);
> +typedef int (*vhost_set_vring_addr_op)(struct vhost_dev *dev,
> +                                       struct vhost_vring_addr *addr);
> +typedef int (*vhost_set_vring_endian_op)(struct vhost_dev *dev,
> +                                         struct vhost_vring_state *ring);
> +typedef int (*vhost_set_vring_num_op)(struct vhost_dev *dev,
> +                                      struct vhost_vring_state *ring);
> +typedef int (*vhost_set_vring_base_op)(struct vhost_dev *dev,
> +                                       struct vhost_vring_state *ring);
> +typedef int (*vhost_get_vring_base_op)(struct vhost_dev *dev,
> +                                       struct vhost_vring_state *ring);
> +typedef int (*vhost_set_vring_kick_op)(struct vhost_dev *dev,
> +                                       struct vhost_vring_file *file);
> +typedef int (*vhost_set_vring_call_op)(struct vhost_dev *dev,
> +                                       struct vhost_vring_file *file);
> +typedef int (*vhost_set_features_op)(struct vhost_dev *dev,
> +                                     uint64_t features);
> +typedef int (*vhost_get_features_op)(struct vhost_dev *dev,
> +                                     uint64_t *features);
> +typedef int (*vhost_set_owner_op)(struct vhost_dev *dev);
> +typedef int (*vhost_reset_device_op)(struct vhost_dev *dev);
> +typedef int (*vhost_get_vq_index_op)(struct vhost_dev *dev, int idx);
> +typedef int (*vhost_set_vring_enable_op)(struct vhost_dev *dev,
> +                                         int enable);
>  typedef bool (*vhost_requires_shm_log_op)(struct vhost_dev *dev);
>
>  typedef struct VhostOps {
>      VhostBackendType backend_type;
> -    vhost_call vhost_call;
>
>      vhost_backend_init vhost_backend_init;
>      vhost_backend_cleanup vhost_backend_cleanup;
> -    vhost_backend_get_vq_index vhost_backend_get_vq_index;
> -    vhost_backend_set_vring_enable vhost_backend_set_vring_enable;
>
> +    vhost_net_set_backend_op vhost_net_set_backend;
> +    vhost_scsi_set_endpoint_op vhost_scsi_set_endpoint;
> +    vhost_scsi_clear_endpoint_op vhost_scsi_clear_endpoint;
> +    vhost_scsi_get_abi_version_op vhost_scsi_get_abi_version;
>      vhost_set_log_base_op vhost_set_log_base;
> +    vhost_set_mem_table_op vhost_set_mem_table;
> +    vhost_set_vring_addr_op vhost_set_vring_addr;
> +    vhost_set_vring_endian_op vhost_set_vring_endian;
> +    vhost_set_vring_num_op vhost_set_vring_num;
> +    vhost_set_vring_base_op vhost_set_vring_base;
> +    vhost_get_vring_base_op vhost_get_vring_base;
> +    vhost_set_vring_kick_op vhost_set_vring_kick;
> +    vhost_set_vring_call_op vhost_set_vring_call;
> +    vhost_set_features_op vhost_set_features;
> +    vhost_get_features_op vhost_get_features;
> +    vhost_set_owner_op vhost_set_owner;
> +    vhost_reset_device_op vhost_reset_device;
> +    vhost_get_vq_index_op vhost_get_vq_index;
> +    vhost_set_vring_enable_op vhost_set_vring_enable;
>      vhost_requires_shm_log_op vhost_requires_shm_log;
>  } VhostOps;
>
> diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
> index 7e7dc45..c8c42cc 100644
> --- a/include/hw/virtio/vhost.h
> +++ b/include/hw/virtio/vhost.h
> @@ -45,14 +45,14 @@ struct vhost_dev {
>      int nvqs;
>      /* the first virtqueue which would be used by this vhost dev */
>      int vq_index;
> -    unsigned long long features;
> -    unsigned long long acked_features;
> -    unsigned long long backend_features;
> -    unsigned long long protocol_features;
> -    unsigned long long max_queues;
> +    uint64_t features;
> +    uint64_t acked_features;
> +    uint64_t backend_features;
> +    uint64_t protocol_features;
> +    uint64_t max_queues;
>      bool started;
>      bool log_enabled;
> -    unsigned long long log_size;
> +    uint64_t log_size;
>      Error *migration_blocker;
>      bool memory_changed;
>      hwaddr mem_changed_start_addr;
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index 2bce891..1ab4133 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -252,8 +252,7 @@ static int vhost_net_start_one(struct vhost_net *net,
>          file.fd = net->backend;
>          for (file.index = 0; file.index < net->dev.nvqs; ++file.index) {
>              const VhostOps *vhost_ops = net->dev.vhost_ops;
> -            r = vhost_ops->vhost_call(&net->dev, VHOST_NET_SET_BACKEND,
> -                                      &file);
> +            r = vhost_ops->vhost_net_set_backend(&net->dev, &file);
>              if (r < 0) {
>                  r = -errno;
>                  goto fail;
> @@ -266,8 +265,7 @@ fail:
>      if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_TAP) {
>          while (file.index-- > 0) {
>              const VhostOps *vhost_ops = net->dev.vhost_ops;
> -            int r = vhost_ops->vhost_call(&net->dev, VHOST_NET_SET_BACKEND,
> -                                          &file);
> +            int r = vhost_ops->vhost_net_set_backend(&net->dev, &file);
>              assert(r >= 0);
>          }
>      }
> @@ -289,15 +287,13 @@ static void vhost_net_stop_one(struct vhost_net *net,
>      if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_TAP) {
>          for (file.index = 0; file.index < net->dev.nvqs; ++file.index) {
>              const VhostOps *vhost_ops = net->dev.vhost_ops;
> -            int r = vhost_ops->vhost_call(&net->dev, VHOST_NET_SET_BACKEND,
> -                                          &file);
> +            int r = vhost_ops->vhost_net_set_backend(&net->dev, &file);
>              assert(r >= 0);
>          }
>      } else if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER) {
>          for (file.index = 0; file.index < net->dev.nvqs; ++file.index) {
>              const VhostOps *vhost_ops = net->dev.vhost_ops;
> -            int r = vhost_ops->vhost_call(&net->dev, VHOST_RESET_DEVICE,
> -                                          NULL);
> +            int r = vhost_ops->vhost_reset_device(&net->dev);
>              assert(r >= 0);
>          }
>      }
> @@ -428,8 +424,8 @@ int vhost_set_vring_enable(NetClientState *nc, int enable)
>      VHostNetState *net = get_vhost_net(nc);
>      const VhostOps *vhost_ops = net->dev.vhost_ops;
>
> -    if (vhost_ops->vhost_backend_set_vring_enable) {
> -        return vhost_ops->vhost_backend_set_vring_enable(&net->dev, enable);
> +    if (vhost_ops->vhost_set_vring_enable) {
> +        return vhost_ops->vhost_set_vring_enable(&net->dev, enable);
>      }
>
>      return 0;
> diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
> index fb7983d..00cdac6 100644
> --- a/hw/scsi/vhost-scsi.c
> +++ b/hw/scsi/vhost-scsi.c
> @@ -46,7 +46,7 @@ static int vhost_scsi_set_endpoint(VHostSCSI *s)
>
>      memset(&backend, 0, sizeof(backend));
>      pstrcpy(backend.vhost_wwpn, sizeof(backend.vhost_wwpn), vs->conf.wwpn);
> -    ret = vhost_ops->vhost_call(&s->dev, VHOST_SCSI_SET_ENDPOINT, &backend);
> +    ret = vhost_ops->vhost_scsi_set_endpoint(&s->dev, &backend);
>      if (ret < 0) {
>          return -errno;
>      }
> @@ -61,7 +61,7 @@ static void vhost_scsi_clear_endpoint(VHostSCSI *s)
>
>      memset(&backend, 0, sizeof(backend));
>      pstrcpy(backend.vhost_wwpn, sizeof(backend.vhost_wwpn), vs->conf.wwpn);
> -    vhost_ops->vhost_call(&s->dev, VHOST_SCSI_CLEAR_ENDPOINT, &backend);
> +    vhost_ops->vhost_scsi_clear_endpoint(&s->dev, &backend);
>  }
>
>  static int vhost_scsi_start(VHostSCSI *s)
> @@ -77,8 +77,7 @@ static int vhost_scsi_start(VHostSCSI *s)
>          return -ENOSYS;
>      }
>
> -    ret = vhost_ops->vhost_call(&s->dev,
> -                                VHOST_SCSI_GET_ABI_VERSION, &abi_version);
> +    ret = vhost_ops->vhost_scsi_get_abi_version(&s->dev, &abi_version);
>      if (ret < 0) {
>          return -errno;
>      }
> diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
> index 87ab028..3820af4 100644
> --- a/hw/virtio/vhost-backend.c
> +++ b/hw/virtio/vhost-backend.c
> @@ -43,27 +43,135 @@ static int vhost_kernel_cleanup(struct vhost_dev *dev)
>      return close(fd);
>  }
>
> -static int vhost_kernel_get_vq_index(struct vhost_dev *dev, int idx)
> +static int vhost_kernel_net_set_backend(struct vhost_dev *dev,
> +                                        struct vhost_vring_file *file)
>  {
> -    assert(idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs);
> +    return vhost_kernel_call(dev, VHOST_NET_SET_BACKEND, file);
> +}
>
> -    return idx - dev->vq_index;
> +static int vhost_kernel_scsi_set_endpoint(struct vhost_dev *dev,
> +                                          struct vhost_scsi_target *target)
> +{
> +    return vhost_kernel_call(dev, VHOST_SCSI_SET_ENDPOINT, target);
> +}
> +
> +static int vhost_kernel_scsi_clear_endpoint(struct vhost_dev *dev,
> +                                            struct vhost_scsi_target *target)
> +{
> +    return vhost_kernel_call(dev, VHOST_SCSI_CLEAR_ENDPOINT, target);
> +}
> +
> +static int vhost_kernel_scsi_get_abi_version(struct vhost_dev *dev, int *version)
> +{
> +    return vhost_kernel_call(dev, VHOST_SCSI_GET_ABI_VERSION, version);
>  }
>
> -static int vhost_set_log_base(struct vhost_dev *dev, uint64_t base,
> -                              struct vhost_log *log)
> +static int vhost_kernel_set_log_base(struct vhost_dev *dev, uint64_t base,
> +                                     struct vhost_log *log)
>  {
>      return vhost_kernel_call(dev, VHOST_SET_LOG_BASE, &base);
>  }
>
> +static int vhost_kernel_set_mem_table(struct vhost_dev *dev,
> +                                      struct vhost_memory *mem)
> +{
> +    return vhost_kernel_call(dev, VHOST_SET_MEM_TABLE, mem);
> +}
> +
> +static int vhost_kernel_set_vring_addr(struct vhost_dev *dev,
> +                                       struct vhost_vring_addr *addr)
> +{
> +    return vhost_kernel_call(dev, VHOST_SET_VRING_ADDR, addr);
> +}
> +
> +static int vhost_kernel_set_vring_endian(struct vhost_dev *dev,
> +                                         struct vhost_vring_state *ring)
> +{
> +    return vhost_kernel_call(dev, VHOST_SET_VRING_ENDIAN, ring);
> +}
> +
> +static int vhost_kernel_set_vring_num(struct vhost_dev *dev,
> +                                      struct vhost_vring_state *ring)
> +{
> +    return vhost_kernel_call(dev, VHOST_SET_VRING_NUM, ring);
> +}
> +
> +static int vhost_kernel_set_vring_base(struct vhost_dev *dev,
> +                                       struct vhost_vring_state *ring)
> +{
> +    return vhost_kernel_call(dev, VHOST_SET_VRING_BASE, ring);
> +}
> +
> +static int vhost_kernel_get_vring_base(struct vhost_dev *dev,
> +                                       struct vhost_vring_state *ring)
> +{
> +    return vhost_kernel_call(dev, VHOST_GET_VRING_BASE, ring);
> +}
> +
> +static int vhost_kernel_set_vring_kick(struct vhost_dev *dev,
> +                                       struct vhost_vring_file *file)
> +{
> +    return vhost_kernel_call(dev, VHOST_SET_VRING_KICK, file);
> +}
> +
> +static int vhost_kernel_set_vring_call(struct vhost_dev *dev,
> +                                       struct vhost_vring_file *file)
> +{
> +    return vhost_kernel_call(dev, VHOST_SET_VRING_CALL, file);
> +}
> +
> +static int vhost_kernel_set_features(struct vhost_dev *dev,
> +                                     uint64_t features)
> +{
> +    return vhost_kernel_call(dev, VHOST_SET_FEATURES, &features);
> +}
> +
> +static int vhost_kernel_get_features(struct vhost_dev *dev,
> +                                     uint64_t *features)
> +{
> +    return vhost_kernel_call(dev, VHOST_GET_FEATURES, features);
> +}
> +
> +static int vhost_kernel_set_owner(struct vhost_dev *dev)
> +{
> +    return vhost_kernel_call(dev, VHOST_SET_OWNER, NULL);
> +}
> +
> +static int vhost_kernel_reset_device(struct vhost_dev *dev)
> +{
> +    return vhost_kernel_call(dev, VHOST_RESET_DEVICE, NULL);
> +}
> +
> +static int vhost_kernel_get_vq_index(struct vhost_dev *dev, int idx)
> +{
> +    assert(idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs);
> +
> +    return idx - dev->vq_index;
> +}
> +
>  static const VhostOps kernel_ops = {
>          .backend_type = VHOST_BACKEND_TYPE_KERNEL,
> -        .vhost_call = vhost_kernel_call,
>          .vhost_backend_init = vhost_kernel_init,
>          .vhost_backend_cleanup = vhost_kernel_cleanup,
> -        .vhost_backend_get_vq_index = vhost_kernel_get_vq_index,
>
> -        .vhost_set_log_base = vhost_set_log_base,
> +        .vhost_net_set_backend = vhost_kernel_net_set_backend,
> +        .vhost_scsi_set_endpoint = vhost_kernel_scsi_set_endpoint,
> +        .vhost_scsi_clear_endpoint = vhost_kernel_scsi_clear_endpoint,
> +        .vhost_scsi_get_abi_version = vhost_kernel_scsi_get_abi_version,
> +        .vhost_set_log_base = vhost_kernel_set_log_base,
> +        .vhost_set_mem_table = vhost_kernel_set_mem_table,
> +        .vhost_set_vring_addr = vhost_kernel_set_vring_addr,
> +        .vhost_set_vring_endian = vhost_kernel_set_vring_endian,
> +        .vhost_set_vring_num = vhost_kernel_set_vring_num,
> +        .vhost_set_vring_base = vhost_kernel_set_vring_base,
> +        .vhost_get_vring_base = vhost_kernel_get_vring_base,
> +        .vhost_set_vring_kick = vhost_kernel_set_vring_kick,
> +        .vhost_set_vring_call = vhost_kernel_set_vring_call,
> +        .vhost_set_features = vhost_kernel_set_features,
> +        .vhost_get_features = vhost_kernel_get_features,
> +        .vhost_set_owner = vhost_kernel_set_owner,
> +        .vhost_reset_device = vhost_kernel_reset_device,
> +        .vhost_get_vq_index = vhost_kernel_get_vq_index,
>  };
>
>  int vhost_set_backend_type(struct vhost_dev *dev, VhostBackendType backend_type)
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index f1edd04..cd84f0c 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -99,37 +99,6 @@ static bool ioeventfd_enabled(void)
>      return kvm_enabled() && kvm_eventfds_enabled();
>  }
>
> -static unsigned long int ioctl_to_vhost_user_request[VHOST_USER_MAX] = {
> -    -1,                     /* VHOST_USER_NONE */
> -    VHOST_GET_FEATURES,     /* VHOST_USER_GET_FEATURES */
> -    VHOST_SET_FEATURES,     /* VHOST_USER_SET_FEATURES */
> -    VHOST_SET_OWNER,        /* VHOST_USER_SET_OWNER */
> -    VHOST_RESET_DEVICE,      /* VHOST_USER_RESET_DEVICE */
> -    VHOST_SET_MEM_TABLE,    /* VHOST_USER_SET_MEM_TABLE */
> -    VHOST_SET_LOG_BASE,     /* VHOST_USER_SET_LOG_BASE */
> -    VHOST_SET_LOG_FD,       /* VHOST_USER_SET_LOG_FD */
> -    VHOST_SET_VRING_NUM,    /* VHOST_USER_SET_VRING_NUM */
> -    VHOST_SET_VRING_ADDR,   /* VHOST_USER_SET_VRING_ADDR */
> -    VHOST_SET_VRING_BASE,   /* VHOST_USER_SET_VRING_BASE */
> -    VHOST_GET_VRING_BASE,   /* VHOST_USER_GET_VRING_BASE */
> -    VHOST_SET_VRING_KICK,   /* VHOST_USER_SET_VRING_KICK */
> -    VHOST_SET_VRING_CALL,   /* VHOST_USER_SET_VRING_CALL */
> -    VHOST_SET_VRING_ERR     /* VHOST_USER_SET_VRING_ERR */
> -};
> -
> -static VhostUserRequest vhost_user_request_translate(unsigned long int request)
> -{
> -    VhostUserRequest idx;
> -
> -    for (idx = 0; idx < VHOST_USER_MAX; idx++) {
> -        if (ioctl_to_vhost_user_request[idx] == request) {
> -            break;
> -        }
> -    }
> -
> -    return (idx == VHOST_USER_MAX) ? VHOST_USER_NONE : idx;
> -}
> -
>  static int vhost_user_read(struct vhost_dev *dev, VhostUserMsg *msg)
>  {
>      CharDriverState *chr = dev->opaque;
> @@ -176,12 +145,35 @@ fail:
>      return -1;
>  }
>
> +static bool vhost_user_one_time_request(VhostUserRequest request)
> +{
> +    switch (request) {
> +    case VHOST_USER_SET_OWNER:
> +    case VHOST_USER_RESET_DEVICE:
> +    case VHOST_USER_SET_MEM_TABLE:
> +    case VHOST_USER_GET_QUEUE_NUM:
> +        return true;
> +    default:
> +        return false;
> +    }
> +}
> +
> +/* most non-init callers ignore the error */
>  static int vhost_user_write(struct vhost_dev *dev, VhostUserMsg *msg,
>                              int *fds, int fd_num)
>  {
>      CharDriverState *chr = dev->opaque;
>      int size = VHOST_USER_HDR_SIZE + msg->size;
>
> +    /*
> +     * For non-vring specific requests, like VHOST_USER_SET_MEM_TABLE,
> +     * we just need send it once in the first time. For later such
> +     * request, we just ignore it.
> +     */
> +    if (vhost_user_one_time_request(msg->request) && dev->vq_index != 0) {
> +        return 0;
> +    }
> +
>      if (fd_num) {
>          qemu_chr_fe_set_msgfds(chr, fds, fd_num);
>      }
> @@ -190,231 +182,317 @@ static int vhost_user_write(struct vhost_dev *dev, VhostUserMsg *msg,
>              0 : -1;
>  }
>
> -static bool vhost_user_one_time_request(VhostUserRequest request)
> +static int vhost_user_set_log_base(struct vhost_dev *dev, uint64_t base,
> +                                   struct vhost_log *log)
>  {
> -    switch (request) {
> -    case VHOST_USER_SET_OWNER:
> -    case VHOST_USER_RESET_DEVICE:
> -    case VHOST_USER_SET_MEM_TABLE:
> -    case VHOST_USER_GET_QUEUE_NUM:
> -        return true;
> -    default:
> -        return false;
> +    int fds[VHOST_MEMORY_MAX_NREGIONS];
> +    size_t fd_num = 0;
> +    bool shmfd = virtio_has_feature(dev->protocol_features,
> +                                    VHOST_USER_PROTOCOL_F_LOG_SHMFD);
> +    VhostUserMsg msg = {
> +        .request = VHOST_USER_SET_LOG_BASE,
> +        .flags = VHOST_USER_VERSION,
> +        .u64 = base,
> +        .size = sizeof(m.u64),
> +    };
> +
> +    if (shmfd && log->fd != -1) {
> +        fds[fd_num++] = log->fd;
>      }
> +
> +    vhost_user_write(dev, &msg, fds, fd_num);
> +
> +    if (shmfd) {
> +        msg.size = 0;
> +        if (vhost_user_read(dev, &msg) < 0) {
> +            return 0;
> +        }
> +
> +        if (msg.request != VHOST_USER_SET_LOG_BASE) {
> +            error_report("Received unexpected msg type. "
> +                         "Expected %d received %d",
> +                         VHOST_USER_SET_LOG_BASE, msg.request);
> +            return -1;
> +        }
> +    }
> +
> +    return 0;
>  }
>
> -static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
> -        void *arg)
> +static int vhost_user_set_mem_table(struct vhost_dev *dev,
> +                                    struct vhost_memory *mem)
>  {
> -    VhostUserMsg msg;
> -    VhostUserRequest msg_request;
> -    struct vhost_vring_file *file = 0;
> -    int need_reply = 0;
>      int fds[VHOST_MEMORY_MAX_NREGIONS];
>      int i, fd;
>      size_t fd_num = 0;
> +    VhostUserMsg msg = {
> +        .request = VHOST_USER_SET_MEM_TABLE,
> +        .flags = VHOST_USER_VERSION,
> +    };
>
> -    assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
> -
> -    /* only translate vhost ioctl requests */
> -    if (request > VHOST_USER_MAX) {
> -        msg_request = vhost_user_request_translate(request);
> -    } else {
> -        msg_request = request;
> +    for (i = 0; i < dev->mem->nregions; ++i) {
> +        struct vhost_memory_region *reg = dev->mem->regions + i;
> +        ram_addr_t ram_addr;
> +
> +        assert((uintptr_t)reg->userspace_addr == reg->userspace_addr);
> +        qemu_ram_addr_from_host((void *)(uintptr_t)reg->userspace_addr,
> +                                &ram_addr);
> +        fd = qemu_get_ram_fd(ram_addr);
> +        if (fd > 0) {
> +            msg.memory.regions[fd_num].userspace_addr = reg->userspace_addr;
> +            msg.memory.regions[fd_num].memory_size  = reg->memory_size;
> +            msg.memory.regions[fd_num].guest_phys_addr = reg->guest_phys_addr;
> +            msg.memory.regions[fd_num].mmap_offset = reg->userspace_addr -
> +                (uintptr_t) qemu_get_ram_block_host_ptr(ram_addr);
> +            assert(fd_num < VHOST_MEMORY_MAX_NREGIONS);
> +            fds[fd_num++] = fd;
> +        }
>      }
>
> -    /*
> -     * For non-vring specific requests, like VHOST_USER_SET_MEM_TABLE,
> -     * we just need send it once in the first time. For later such
> -     * request, we just ignore it.
> -     */
> -    if (vhost_user_one_time_request(msg_request) && dev->vq_index != 0) {
> -        return 0;
> +    msg.memory.nregions = fd_num;
> +
> +    if (!fd_num) {
> +        error_report("Failed initializing vhost-user memory map, "
> +                     "consider using -object memory-backend-file share=on");
> +        return -1;
>      }
>
> -    msg.request = msg_request;
> -    msg.flags = VHOST_USER_VERSION;
> -    msg.size = 0;
> +    msg.size = sizeof(m.memory.nregions);
> +    msg.size += sizeof(m.memory.padding);
> +    msg.size += fd_num * sizeof(VhostUserMemoryRegion);
>
> -    switch (msg_request) {
> -    case VHOST_USER_GET_FEATURES:
> -    case VHOST_USER_GET_PROTOCOL_FEATURES:
> -    case VHOST_USER_GET_QUEUE_NUM:
> -        need_reply = 1;
> -        break;
> +    vhost_user_write(dev, &msg, fds, fd_num);
>
> -    case VHOST_USER_SET_FEATURES:
> -    case VHOST_USER_SET_PROTOCOL_FEATURES:
> -        msg.u64 = *((__u64 *) arg);
> -        msg.size = sizeof(m.u64);
> -        break;
> +    return 0;
> +}
>
> -    case VHOST_USER_SET_OWNER:
> -    case VHOST_USER_RESET_DEVICE:
> -        break;
> +static int vhost_user_set_vring_addr(struct vhost_dev *dev,
> +                                     struct vhost_vring_addr *addr)
> +{
> +    VhostUserMsg msg = {
> +        .request = VHOST_USER_SET_VRING_ADDR,
> +        .flags = VHOST_USER_VERSION,
> +        .addr = *addr,
> +        .size = sizeof(*addr),
> +    };
>
> -    case VHOST_USER_SET_MEM_TABLE:
> -        for (i = 0; i < dev->mem->nregions; ++i) {
> -            struct vhost_memory_region *reg = dev->mem->regions + i;
> -            ram_addr_t ram_addr;
> -
> -            assert((uintptr_t)reg->userspace_addr == reg->userspace_addr);
> -            qemu_ram_addr_from_host((void *)(uintptr_t)reg->userspace_addr, &ram_addr);
> -            fd = qemu_get_ram_fd(ram_addr);
> -            if (fd > 0) {
> -                msg.memory.regions[fd_num].userspace_addr = reg->userspace_addr;
> -                msg.memory.regions[fd_num].memory_size  = reg->memory_size;
> -                msg.memory.regions[fd_num].guest_phys_addr = reg->guest_phys_addr;
> -                msg.memory.regions[fd_num].mmap_offset = reg->userspace_addr -
> -                    (uintptr_t) qemu_get_ram_block_host_ptr(ram_addr);
> -                assert(fd_num < VHOST_MEMORY_MAX_NREGIONS);
> -                fds[fd_num++] = fd;
> -            }
> -        }
> +    vhost_user_write(dev, &msg, NULL, 0);
>
> -        msg.memory.nregions = fd_num;
> +    return 0;
> +}
>
> -        if (!fd_num) {
> -            error_report("Failed initializing vhost-user memory map, "
> -                    "consider using -object memory-backend-file share=on");
> -            return -1;
> -        }
> +static int vhost_user_set_vring_endian(struct vhost_dev *dev,
> +                                       struct vhost_vring_state *ring)
> +{
> +    error_report("vhost-user trying to send unhandled ioctl");
> +    return -1;
> +}
>
> -        msg.size = sizeof(m.memory.nregions);
> -        msg.size += sizeof(m.memory.padding);
> -        msg.size += fd_num * sizeof(VhostUserMemoryRegion);
> -
> -        break;
> -
> -    case VHOST_USER_SET_LOG_FD:
> -        fds[fd_num++] = *((int *) arg);
> -        break;
> -
> -    case VHOST_USER_SET_VRING_NUM:
> -    case VHOST_USER_SET_VRING_BASE:
> -    case VHOST_USER_SET_VRING_ENABLE:
> -        memcpy(&msg.state, arg, sizeof(struct vhost_vring_state));
> -        msg.size = sizeof(m.state);
> -        break;
> -
> -    case VHOST_USER_GET_VRING_BASE:
> -        memcpy(&msg.state, arg, sizeof(struct vhost_vring_state));
> -        msg.size = sizeof(m.state);
> -        need_reply = 1;
> -        break;
> -
> -    case VHOST_USER_SET_VRING_ADDR:
> -        memcpy(&msg.addr, arg, sizeof(struct vhost_vring_addr));
> -        msg.size = sizeof(m.addr);
> -        break;
> -
> -    case VHOST_USER_SET_VRING_KICK:
> -    case VHOST_USER_SET_VRING_CALL:
> -    case VHOST_USER_SET_VRING_ERR:
> -        file = arg;
> -        msg.u64 = file->index & VHOST_USER_VRING_IDX_MASK;
> -        msg.size = sizeof(m.u64);
> -        if (ioeventfd_enabled() && file->fd > 0) {
> -            fds[fd_num++] = file->fd;
> -        } else {
> -            msg.u64 |= VHOST_USER_VRING_NOFD_MASK;
> -        }
> -        break;
> -    default:
> -        error_report("vhost-user trying to send unhandled ioctl");
> +static int vhost_set_vring(struct vhost_dev *dev,
> +                           unsigned long int request,
> +                           struct vhost_vring_state *ring)
> +{
> +    VhostUserMsg msg = {
> +        .request = request,
> +        .flags = VHOST_USER_VERSION,
> +        .state = *ring,
> +        .size = sizeof(*ring),
> +    };
> +
> +    vhost_user_write(dev, &msg, NULL, 0);
> +
> +    return 0;
> +}
> +
> +static int vhost_user_set_vring_num(struct vhost_dev *dev,
> +                                    struct vhost_vring_state *ring)
> +{
> +    return vhost_set_vring(dev, VHOST_SET_VRING_NUM, ring);
> +}
> +
> +static int vhost_user_set_vring_base(struct vhost_dev *dev,
> +                                     struct vhost_vring_state *ring)
> +{
> +    return vhost_set_vring(dev, VHOST_SET_VRING_BASE, ring);
> +}
> +
> +static int vhost_user_set_vring_enable(struct vhost_dev *dev, int enable)
> +{
> +    struct vhost_vring_state state = {
> +        .index = dev->vq_index,
> +        .num   = enable,
> +    };
> +
> +    if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_MQ))) {
>          return -1;
> -        break;
>      }
>
> -    if (vhost_user_write(dev, &msg, fds, fd_num) < 0) {
> +    return vhost_set_vring(dev, VHOST_USER_SET_VRING_ENABLE, &state);
> +}
> +
> +
> +static int vhost_user_get_vring_base(struct vhost_dev *dev,
> +                                     struct vhost_vring_state *ring)
> +{
> +    VhostUserMsg msg = {
> +        .request = VHOST_USER_GET_VRING_BASE,
> +        .flags = VHOST_USER_VERSION,
> +        .state = *ring,
> +        .size = sizeof(*ring),
> +    };
> +
> +    vhost_user_write(dev, &msg, NULL, 0);
> +
> +    if (vhost_user_read(dev, &msg) < 0) {
>          return 0;
>      }
>
> -    if (need_reply) {
> -        if (vhost_user_read(dev, &msg) < 0) {
> -            return 0;
> -        }
> -
> -        if (msg_request != msg.request) {
> -            error_report("Received unexpected msg type."
> -                    " Expected %d received %d", msg_request, msg.request);
> -            return -1;
> -        }
> +    if (msg.request != VHOST_USER_GET_VRING_BASE) {
> +        error_report("Received unexpected msg type. Expected %d received %d",
> +                     VHOST_USER_GET_VRING_BASE, msg.request);
> +        return -1;
> +    }
>
> -        switch (msg_request) {
> -        case VHOST_USER_GET_FEATURES:
> -        case VHOST_USER_GET_PROTOCOL_FEATURES:
> -        case VHOST_USER_GET_QUEUE_NUM:
> -            if (msg.size != sizeof(m.u64)) {
> -                error_report("Received bad msg size.");
> -                return -1;
> -            }
> -            *((__u64 *) arg) = msg.u64;
> -            break;
> -        case VHOST_USER_GET_VRING_BASE:
> -            if (msg.size != sizeof(m.state)) {
> -                error_report("Received bad msg size.");
> -                return -1;
> -            }
> -            memcpy(arg, &msg.state, sizeof(struct vhost_vring_state));
> -            break;
> -        default:
> -            error_report("Received unexpected msg type.");
> -            return -1;
> -            break;
> -        }
> +    if (msg.size != sizeof(m.state)) {
> +        error_report("Received bad msg size.");
> +        return -1;
>      }
>
> +    *ring = msg.state;
> +
>      return 0;
>  }
>
> -static int vhost_set_log_base(struct vhost_dev *dev, uint64_t base,
> -                              struct vhost_log *log)
> +static int vhost_set_vring_file(struct vhost_dev *dev,
> +                                VhostUserRequest request,
> +                                struct vhost_vring_file *file)
>  {
>      int fds[VHOST_MEMORY_MAX_NREGIONS];
>      size_t fd_num = 0;
> -    bool shmfd = virtio_has_feature(dev->protocol_features,
> -                                    VHOST_USER_PROTOCOL_F_LOG_SHMFD);
>      VhostUserMsg msg = {
> -        .request = VHOST_USER_SET_LOG_BASE,
> +        .request = request,
>          .flags = VHOST_USER_VERSION,
> -        .u64 = base,
> +        .u64 = file->index & VHOST_USER_VRING_IDX_MASK,
>          .size = sizeof(m.u64),
>      };
>
> -    if (shmfd && log->fd != -1) {
> -        fds[fd_num++] = log->fd;
> +    if (ioeventfd_enabled() && file->fd > 0) {
> +        fds[fd_num++] = file->fd;
> +    } else {
> +        msg.u64 |= VHOST_USER_VRING_NOFD_MASK;
>      }
>
>      vhost_user_write(dev, &msg, fds, fd_num);
>
> -    if (shmfd) {
> -        msg.size = 0;
> -        if (vhost_user_read(dev, &msg) < 0) {
> -            return 0;
> -        }
> +    return 0;
> +}
>
> -        if (msg.request != VHOST_USER_SET_LOG_BASE) {
> -            error_report("Received unexpected msg type. "
> -                         "Expected %d received %d",
> -                         VHOST_USER_SET_LOG_BASE, msg.request);
> -            return -1;
> -        }
> +static int vhost_user_set_vring_kick(struct vhost_dev *dev,
> +                                     struct vhost_vring_file *file)
> +{
> +    return vhost_set_vring_file(dev, VHOST_USER_SET_VRING_KICK, file);
> +}
> +
> +static int vhost_user_set_vring_call(struct vhost_dev *dev,
> +                                     struct vhost_vring_file *file)
> +{
> +    return vhost_set_vring_file(dev, VHOST_USER_SET_VRING_CALL, file);
> +}
> +
> +static int vhost_user_set_u64(struct vhost_dev *dev, int request, uint64_t u64)
> +{
> +    VhostUserMsg msg = {
> +        .request = request,
> +        .flags = VHOST_USER_VERSION,
> +        .u64 = u64,
> +        .size = sizeof(m.u64),
> +    };
> +
> +    vhost_user_write(dev, &msg, NULL, 0);
> +
> +    return 0;
> +}
> +
> +static int vhost_user_set_features(struct vhost_dev *dev,
> +                                   uint64_t features)
> +{
> +    return vhost_user_set_u64(dev, VHOST_USER_SET_FEATURES, features);
> +}
> +
> +static int vhost_user_set_protocol_features(struct vhost_dev *dev,
> +                                            uint64_t features)
> +{
> +    return vhost_user_set_u64(dev, VHOST_USER_SET_PROTOCOL_FEATURES, features);
> +}
> +
> +static int vhost_user_get_u64(struct vhost_dev *dev, int request, uint64_t *u64)
> +{
> +    VhostUserMsg msg = {
> +        .request = request,
> +        .flags = VHOST_USER_VERSION,
> +    };
> +
> +    vhost_user_write(dev, &msg, NULL, 0);
> +
> +    if (vhost_user_read(dev, &msg) < 0) {
> +        return 0;
> +    }
> +
> +    if (msg.request != request) {
> +        error_report("Received unexpected msg type. Expected %d received %d",
> +                     request, msg.request);
> +        return -1;
> +    }
> +
> +    if (msg.size != sizeof(m.u64)) {
> +        error_report("Received bad msg size.");
> +        return -1;
>      }
>
> +    *u64 = msg.u64;
> +
> +    return 0;
> +}
> +
> +static int vhost_user_get_features(struct vhost_dev *dev, uint64_t *features)
> +{
> +    return vhost_user_get_u64(dev, VHOST_USER_GET_FEATURES, features);
> +}
> +
> +static int vhost_user_set_owner(struct vhost_dev *dev)
> +{
> +    VhostUserMsg msg = {
> +        .request = VHOST_USER_SET_OWNER,
> +        .flags = VHOST_USER_VERSION,
> +    };
> +
> +    vhost_user_write(dev, &msg, NULL, 0);
> +
> +    return 0;
> +}
> +
> +static int vhost_user_reset_device(struct vhost_dev *dev)
> +{
> +    VhostUserMsg msg = {
> +        .request = VHOST_USER_RESET_DEVICE,
> +        .flags = VHOST_USER_VERSION,
> +    };
> +
> +    vhost_user_write(dev, &msg, NULL, 0);
> +
>      return 0;
>  }
>
>  static int vhost_user_init(struct vhost_dev *dev, void *opaque)
>  {
> -    unsigned long long features;
> +    uint64_t features;
>      int err;
>
>      assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
>
>      dev->opaque = opaque;
>
> -    err = vhost_user_call(dev, VHOST_USER_GET_FEATURES, &features);
> +    err = vhost_user_get_features(dev, &features);
>      if (err < 0) {
>          return err;
>      }
> @@ -422,21 +500,22 @@ static int vhost_user_init(struct vhost_dev *dev, void *opaque)
>      if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) {
>          dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
>
> -        err = vhost_user_call(dev, VHOST_USER_GET_PROTOCOL_FEATURES, &features);
> +        err = vhost_user_get_u64(dev, VHOST_USER_GET_PROTOCOL_FEATURES,
> +                                 &features);
>          if (err < 0) {
>              return err;
>          }
>
>          dev->protocol_features = features & VHOST_USER_PROTOCOL_FEATURE_MASK;
> -        err = vhost_user_call(dev, VHOST_USER_SET_PROTOCOL_FEATURES,
> -                              &dev->protocol_features);
> +        err = vhost_user_set_protocol_features(dev, dev->protocol_features);
>          if (err < 0) {
>              return err;
>          }
>
>          /* query the max queues we support if backend supports Multiple Queue */
>          if (dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_MQ)) {
> -            err = vhost_user_call(dev, VHOST_USER_GET_QUEUE_NUM, &dev->max_queues);
> +            err = vhost_user_get_u64(dev, VHOST_USER_GET_QUEUE_NUM,
> +                                     &dev->max_queues);
>              if (err < 0) {
>                  return err;
>              }
> @@ -454,22 +533,6 @@ static int vhost_user_init(struct vhost_dev *dev, void *opaque)
>      return 0;
>  }
>
> -static int vhost_user_set_vring_enable(struct vhost_dev *dev, int enable)
> -{
> -    struct vhost_vring_state state = {
> -        .index = dev->vq_index,
> -        .num   = enable,
> -    };
> -
> -    assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
> -
> -    if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_MQ))) {
> -        return -1;
> -    }
> -
> -    return vhost_user_call(dev, VHOST_USER_SET_VRING_ENABLE, &state);
> -}
> -
>  static int vhost_user_cleanup(struct vhost_dev *dev)
>  {
>      assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
> @@ -496,12 +559,23 @@ static bool vhost_user_requires_shm_log(struct vhost_dev *dev)
>
>  const VhostOps user_ops = {
>          .backend_type = VHOST_BACKEND_TYPE_USER,
> -        .vhost_call = vhost_user_call,
>          .vhost_backend_init = vhost_user_init,
>          .vhost_backend_cleanup = vhost_user_cleanup,
> -        .vhost_backend_get_vq_index = vhost_user_get_vq_index,
> -        .vhost_backend_set_vring_enable = vhost_user_set_vring_enable,
>
> -        .vhost_set_log_base = vhost_set_log_base,
> +        .vhost_set_log_base = vhost_user_set_log_base,
> +        .vhost_set_mem_table = vhost_user_set_mem_table,
> +        .vhost_set_vring_addr = vhost_user_set_vring_addr,
> +        .vhost_set_vring_endian = vhost_user_set_vring_endian,
> +        .vhost_set_vring_num = vhost_user_set_vring_num,
> +        .vhost_set_vring_base = vhost_user_set_vring_base,
> +        .vhost_get_vring_base = vhost_user_get_vring_base,
> +        .vhost_set_vring_kick = vhost_user_set_vring_kick,
> +        .vhost_set_vring_call = vhost_user_set_vring_call,
> +        .vhost_set_features = vhost_user_set_features,
> +        .vhost_get_features = vhost_user_get_features,
> +        .vhost_set_owner = vhost_user_set_owner,
> +        .vhost_reset_device = vhost_user_reset_device,
> +        .vhost_get_vq_index = vhost_user_get_vq_index,
> +        .vhost_set_vring_enable = vhost_user_set_vring_enable,
>          .vhost_requires_shm_log = vhost_user_requires_shm_log,
>  };
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 554e49d..1e8ee76 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -538,7 +538,7 @@ static void vhost_commit(MemoryListener *listener)
>      }
>
>      if (!dev->log_enabled) {
> -        r = dev->vhost_ops->vhost_call(dev, VHOST_SET_MEM_TABLE, dev->mem);
> +        r = dev->vhost_ops->vhost_set_mem_table(dev, dev->mem);
>          assert(r >= 0);
>          dev->memory_changed = false;
>          return;
> @@ -551,7 +551,7 @@ static void vhost_commit(MemoryListener *listener)
>      if (dev->log_size < log_size) {
>          vhost_dev_log_resize(dev, log_size + VHOST_LOG_BUFFER);
>      }
> -    r = dev->vhost_ops->vhost_call(dev, VHOST_SET_MEM_TABLE, dev->mem);
> +    r = dev->vhost_ops->vhost_set_mem_table(dev, dev->mem);
>      assert(r >= 0);
>      /* To log less, can only decrease log size after table update. */
>      if (dev->log_size > log_size + VHOST_LOG_BUFFER) {
> @@ -619,7 +619,7 @@ static int vhost_virtqueue_set_addr(struct vhost_dev *dev,
>          .log_guest_addr = vq->used_phys,
>          .flags = enable_log ? (1 << VHOST_VRING_F_LOG) : 0,
>      };
> -    int r = dev->vhost_ops->vhost_call(dev, VHOST_SET_VRING_ADDR, &addr);
> +    int r = dev->vhost_ops->vhost_set_vring_addr(dev, &addr);
>      if (r < 0) {
>          return -errno;
>      }
> @@ -633,7 +633,7 @@ static int vhost_dev_set_features(struct vhost_dev *dev, bool enable_log)
>      if (enable_log) {
>          features |= 0x1ULL << VHOST_F_LOG_ALL;
>      }
> -    r = dev->vhost_ops->vhost_call(dev, VHOST_SET_FEATURES, &features);
> +    r = dev->vhost_ops->vhost_set_features(dev, features);
>      return r < 0 ? -errno : 0;
>  }
>
> @@ -738,7 +738,7 @@ static int vhost_virtqueue_set_vring_endian_legacy(struct vhost_dev *dev,
>          .num = is_big_endian
>      };
>
> -    if (!dev->vhost_ops->vhost_call(dev, VHOST_SET_VRING_ENDIAN, &s)) {
> +    if (!dev->vhost_ops->vhost_set_vring_endian(dev, &s)) {
>          return 0;
>      }
>
> @@ -757,7 +757,7 @@ static int vhost_virtqueue_start(struct vhost_dev *dev,
>  {
>      hwaddr s, l, a;
>      int r;
> -    int vhost_vq_index = dev->vhost_ops->vhost_backend_get_vq_index(dev, idx);
> +    int vhost_vq_index = dev->vhost_ops->vhost_get_vq_index(dev, idx);
>      struct vhost_vring_file file = {
>          .index = vhost_vq_index
>      };
> @@ -768,13 +768,13 @@ static int vhost_virtqueue_start(struct vhost_dev *dev,
>
>
>      vq->num = state.num = virtio_queue_get_num(vdev, idx);
> -    r = dev->vhost_ops->vhost_call(dev, VHOST_SET_VRING_NUM, &state);
> +    r = dev->vhost_ops->vhost_set_vring_num(dev, &state);
>      if (r) {
>          return -errno;
>      }
>
>      state.num = virtio_queue_get_last_avail_idx(vdev, idx);
> -    r = dev->vhost_ops->vhost_call(dev, VHOST_SET_VRING_BASE, &state);
> +    r = dev->vhost_ops->vhost_set_vring_base(dev, &state);
>      if (r) {
>          return -errno;
>      }
> @@ -826,7 +826,7 @@ static int vhost_virtqueue_start(struct vhost_dev *dev,
>      }
>
>      file.fd = event_notifier_get_fd(virtio_queue_get_host_notifier(vvq));
> -    r = dev->vhost_ops->vhost_call(dev, VHOST_SET_VRING_KICK, &file);
> +    r = dev->vhost_ops->vhost_set_vring_kick(dev, &file);
>      if (r) {
>          r = -errno;
>          goto fail_kick;
> @@ -859,13 +859,13 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev,
>                                      struct vhost_virtqueue *vq,
>                                      unsigned idx)
>  {
> -    int vhost_vq_index = dev->vhost_ops->vhost_backend_get_vq_index(dev, idx);
> +    int vhost_vq_index = dev->vhost_ops->vhost_get_vq_index(dev, idx);
>      struct vhost_vring_state state = {
>          .index = vhost_vq_index,
>      };
>      int r;
>
> -    r = dev->vhost_ops->vhost_call(dev, VHOST_GET_VRING_BASE, &state);
> +    r = dev->vhost_ops->vhost_get_vring_base(dev, &state);
>      if (r < 0) {
>          fprintf(stderr, "vhost VQ %d ring restore failed: %d\n", idx, r);
>          fflush(stderr);
> @@ -912,7 +912,7 @@ static void vhost_eventfd_del(MemoryListener *listener,
>  static int vhost_virtqueue_init(struct vhost_dev *dev,
>                                  struct vhost_virtqueue *vq, int n)
>  {
> -    int vhost_vq_index = dev->vhost_ops->vhost_backend_get_vq_index(dev, n);
> +    int vhost_vq_index = dev->vhost_ops->vhost_get_vq_index(dev, n);
>      struct vhost_vring_file file = {
>          .index = vhost_vq_index,
>      };
> @@ -922,7 +922,7 @@ static int vhost_virtqueue_init(struct vhost_dev *dev,
>      }
>
>      file.fd = event_notifier_get_fd(&vq->masked_notifier);
> -    r = dev->vhost_ops->vhost_call(dev, VHOST_SET_VRING_CALL, &file);
> +    r = dev->vhost_ops->vhost_set_vring_call(dev, &file);
>      if (r) {
>          r = -errno;
>          goto fail_call;
> @@ -956,12 +956,12 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
>          return -errno;
>      }
>
> -    r = hdev->vhost_ops->vhost_call(hdev, VHOST_SET_OWNER, NULL);
> +    r = hdev->vhost_ops->vhost_set_owner(hdev);
>      if (r < 0) {
>          goto fail;
>      }
>
> -    r = hdev->vhost_ops->vhost_call(hdev, VHOST_GET_FEATURES, &features);
> +    r = hdev->vhost_ops->vhost_get_features(hdev, &features);
>      if (r < 0) {
>          goto fail;
>      }
> @@ -1120,8 +1120,8 @@ void vhost_virtqueue_mask(struct vhost_dev *hdev, VirtIODevice *vdev, int n,
>          file.fd = event_notifier_get_fd(virtio_queue_get_guest_notifier(vvq));
>      }
>
> -    file.index = hdev->vhost_ops->vhost_backend_get_vq_index(hdev, n);
> -    r = hdev->vhost_ops->vhost_call(hdev, VHOST_SET_VRING_CALL, &file);
> +    file.index = hdev->vhost_ops->vhost_get_vq_index(hdev, n);
> +    r = hdev->vhost_ops->vhost_set_vring_call(hdev, &file);
>      assert(r >= 0);
>  }
>
> @@ -1163,7 +1163,7 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
>      if (r < 0) {
>          goto fail_features;
>      }
> -    r = hdev->vhost_ops->vhost_call(hdev, VHOST_SET_MEM_TABLE, hdev->mem);
> +    r = hdev->vhost_ops->vhost_set_mem_table(hdev, hdev->mem);
>      if (r < 0) {
>          r = -errno;
>          goto fail_mem;
> --
> MST
>
>

Hi Michael,

this patch is not correct. If we apply QEMU enters in a dead lock if
we have several queue pairs for virttio/vhost and vhost backend
crashes during the initialisation sequence
I have sent some comments yesterday to Marc to correct theses issues.
The attached fixup solves the issues (already sent to Marc Andre on
QEMU mailing list):
 - The two first corrections solve the vhost backend crashes (the
message id used for some messages are the kernel IOCTL and not the
VHOST_USER id)
 - the third patchs solves the QEMU dead lock in case of multiqueue
(VHOST_USER_GET_QUEUE_NUM is a one time message. This message is sent
through the new vhost_user_get_u64 function but except for the first
queue a blocking read is sent whereas no write has been sent before
that causes the lock)

If you (and Marc Andre) agree this correction could you update this
patch with the fixup and resend the pull request ?

Thanks

Best regards.

Thibaut.

Comments

Michael S. Tsirkin Oct. 9, 2015, 7:37 a.m. UTC | #1
On Fri, Oct 09, 2015 at 08:40:27AM +0200, Thibaut Collet wrote:
> Hi Michael,
> 
> this patch is not correct. If we apply QEMU enters in a dead lock if
> we have several queue pairs for virttio/vhost and vhost backend
> crashes during the initialisation sequence
> I have sent some comments yesterday to Marc to correct theses issues.
> The attached fixup solves the issues (already sent to Marc Andre on
> QEMU mailing list):
>  - The two first corrections solve the vhost backend crashes (the
> message id used for some messages are the kernel IOCTL and not the
> VHOST_USER id)
>  - the third patchs solves the QEMU dead lock in case of multiqueue
> (VHOST_USER_GET_QUEUE_NUM is a one time message. This message is sent
> through the new vhost_user_get_u64 function but except for the first
> queue a blocking read is sent whereas no write has been sent before
> that causes the lock)
> 
> If you (and Marc Andre) agree this correction could you update this
> patch with the fixup and resend the pull request ?
> 
> Thanks
> 
> Best regards.
> 
> Thibaut.

I dropped this and dependent patches from the tree for now.
Thanks!
diff mbox

Patch

From effefe37d6ffa929657d788873311e026a5c0c80 Mon Sep 17 00:00:00 2001
From: Thibaut Collet <thibaut.collet@6wind.com>
Date: Wed, 7 Oct 2015 17:41:42 +0200
Subject: [PATCH] FIXUP for vhost: use a function for each call

Signed-off-by: Thibaut Collet <thibaut.collet@6wind.com>
---
 hw/virtio/vhost-user.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index bbf5eeb..8b4ea0f 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -315,13 +315,13 @@  static int vhost_set_vring(struct vhost_dev *dev,
 static int vhost_user_set_vring_num(struct vhost_dev *dev,
                                     struct vhost_vring_state *ring)
 {
-    return vhost_set_vring(dev, VHOST_SET_VRING_NUM, ring);
+    return vhost_set_vring(dev, VHOST_USER_SET_VRING_NUM, ring);
 }
 
 static int vhost_user_set_vring_base(struct vhost_dev *dev,
                                      struct vhost_vring_state *ring)
 {
-    return vhost_set_vring(dev, VHOST_SET_VRING_BASE, ring);
+    return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring);
 }
 
 static int vhost_user_set_vring_enable(struct vhost_dev *dev, int enable)
@@ -440,6 +440,10 @@  static int vhost_user_get_u64(struct vhost_dev *dev, int request, uint64_t *u64)
         .flags = VHOST_USER_VERSION,
     };
 
+    if (vhost_user_one_time_request(request) && dev->vq_index != 0) {
+        return 0;
+    }
+
     vhost_user_write(dev, &msg, NULL, 0);
 
     if (vhost_user_read(dev, &msg) < 0) {
-- 
2.1.4