Re: [Qemu-devel] [PATCH v3] Enable vhost with vhostforce, vhost options for guests without MSI-X support

[Jason Wang]

On 06/16/2015 04:18 PM, Pankaj Gupta wrote:
> We use vhostforce to enable vhost even if Guests don't have MSI-X support
> and we fall back to QEMU virtio-net. This patch will enable vhost 
> unconditionally 
> whenever we have vhostforce='ON' or vhost='ON'. 
>
> Initially, I wanted to remove vhostforce completely as an additional 
> argument. 
> But after discussing this in mailing list found that some programs are using 
> vhostforce
> and some vhost. So, we want to keep semantics of both the options.
>
> Changes from v2->v3
> Jason Wang : Remove functions which are not being used.
>
> Changes from v1->v2
> MST : Keep comamnd line semantic for options vhost= 'ON' ||
>       vhostforce = 'ON', vhost should be ON. 
>
> Signed-off-by: Pankaj Gupta <address@hidden>

Subject could be more compact like "start vhost without querying guest
notifiers" and some lines in commit log exceeds 80 characters.

Except from these:

Reviewed-by: Jason Wang <address@hidden>

> ---
>  hw/net/vhost_net.c        | 12 +-----------
>  hw/net/virtio-net.c       |  4 ----
>  hw/scsi/vhost-scsi.c      |  2 +-
>  hw/virtio/vhost.c         | 14 +-------------
>  include/hw/virtio/vhost.h |  3 +--
>  include/net/vhost_net.h   |  2 --
>  net/tap.c                 |  1 -
>  net/vhost-user.c          |  1 -
>  8 files changed, 4 insertions(+), 35 deletions(-)
>
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index 1c55517..0a4577d 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -162,7 +162,7 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
>      net->dev.vq_index = net->nc->queue_index;
>  
>      r = vhost_dev_init(&net->dev, options->opaque,
> -                       options->backend_type, options->force);
> +                       options->backend_type);
>      if (r < 0) {
>          goto fail;
>      }
> @@ -187,11 +187,6 @@ fail:
>      return NULL;
>  }
>  
> -bool vhost_net_query(VHostNetState *net, VirtIODevice *dev)
> -{
> -    return vhost_dev_query(&net->dev, dev);
> -}
> -
>  static void vhost_net_set_vq_index(struct vhost_net *net, int vq_index)
>  {
>      net->dev.vq_index = vq_index;
> @@ -412,11 +407,6 @@ struct vhost_net *vhost_net_init(VhostNetOptions 
> *options)
>      return NULL;
>  }
>  
> -bool vhost_net_query(VHostNetState *net, VirtIODevice *dev)
> -{
> -    return false;
> -}
> -
>  int vhost_net_start(VirtIODevice *dev,
>                      NetClientState *ncs,
>                      int total_queues)
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 9281aa1..d728233 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -128,10 +128,6 @@ static void virtio_net_vhost_status(VirtIONet *n, 
> uint8_t status)
>      if (!n->vhost_started) {
>          int r, i;
>  
> -        if (!vhost_net_query(get_vhost_net(nc->peer), vdev)) {
> -            return;
> -        }
> -
>          /* Any packets outstanding? Purge them to avoid touching rings
>           * when vhost is running.
>           */
> diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
> index 1941aa1..1c389c4 100644
> --- a/hw/scsi/vhost-scsi.c
> +++ b/hw/scsi/vhost-scsi.c
> @@ -246,7 +246,7 @@ static void vhost_scsi_realize(DeviceState *dev, Error 
> **errp)
>      s->dev.backend_features = 0;
>  
>      ret = vhost_dev_init(&s->dev, (void *)(uintptr_t)vhostfd,
> -                         VHOST_BACKEND_TYPE_KERNEL, true);
> +                         VHOST_BACKEND_TYPE_KERNEL);
>      if (ret < 0) {
>          error_setg(errp, "vhost-scsi: vhost initialization failed: %s",
>                     strerror(-ret));
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 2d6c27a..4f5bb15 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -853,7 +853,7 @@ static void vhost_virtqueue_cleanup(struct 
> vhost_virtqueue *vq)
>  }
>  
>  int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
> -                   VhostBackendType backend_type, bool force)
> +                   VhostBackendType backend_type)
>  {
>      uint64_t features;
>      int i, r;
> @@ -916,7 +916,6 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
>      hdev->started = false;
>      hdev->memory_changed = false;
>      memory_listener_register(&hdev->memory_listener, &address_space_memory);
> -    hdev->force = force;
>      return 0;
>  fail_vq:
>      while (--i >= 0) {
> @@ -944,17 +943,6 @@ void vhost_dev_cleanup(struct vhost_dev *hdev)
>      hdev->vhost_ops->vhost_backend_cleanup(hdev);
>  }
>  
> -bool vhost_dev_query(struct vhost_dev *hdev, VirtIODevice *vdev)
> -{
> -    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
> -    VirtioBusState *vbus = VIRTIO_BUS(qbus);
> -    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
> -
> -    return !k->query_guest_notifiers ||
> -           k->query_guest_notifiers(qbus->parent) ||
> -           hdev->force;
> -}
> -
>  /* Stop processing guest IO notifications in qemu.
>   * Start processing them in vhost in kernel.
>   */
> diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
> index 84f170e..dd51050 100644
> --- a/include/hw/virtio/vhost.h
> +++ b/include/hw/virtio/vhost.h
> @@ -51,7 +51,6 @@ struct vhost_dev {
>      bool log_enabled;
>      unsigned long long log_size;
>      Error *migration_blocker;
> -    bool force;
>      bool memory_changed;
>      hwaddr mem_changed_start_addr;
>      hwaddr mem_changed_end_addr;
> @@ -61,7 +60,7 @@ struct vhost_dev {
>  };
>  
>  int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
> -                   VhostBackendType backend_type, bool force);
> +                   VhostBackendType backend_type);
>  void vhost_dev_cleanup(struct vhost_dev *hdev);
>  bool vhost_dev_query(struct vhost_dev *hdev, VirtIODevice *vdev);
>  int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev);
> diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
> index 9eb493e..840d4b1 100644
> --- a/include/net/vhost_net.h
> +++ b/include/net/vhost_net.h
> @@ -11,12 +11,10 @@ typedef struct VhostNetOptions {
>      VhostBackendType backend_type;
>      NetClientState *net_backend;
>      void *opaque;
> -    bool force;
>  } VhostNetOptions;
>  
>  struct vhost_net *vhost_net_init(VhostNetOptions *options);
>  
> -bool vhost_net_query(VHostNetState *net, VirtIODevice *dev);
>  int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, int 
> total_queues);
>  void vhost_net_stop(VirtIODevice *dev, NetClientState *ncs, int 
> total_queues);
>  
> diff --git a/net/tap.c b/net/tap.c
> index aa8b3f5..bb67c6e 100644
> --- a/net/tap.c
> +++ b/net/tap.c
> @@ -646,7 +646,6 @@ static void net_init_tap_one(const NetdevTapOptions *tap, 
> NetClientState *peer,
>  
>          options.backend_type = VHOST_BACKEND_TYPE_KERNEL;
>          options.net_backend = &s->nc;
> -        options.force = tap->has_vhostforce && tap->vhostforce;
>  
>          if (tap->has_vhostfd || tap->has_vhostfds) {
>              vhostfd = monitor_fd_param(cur_mon, vhostfdname, &err);
> diff --git a/net/vhost-user.c b/net/vhost-user.c
> index 3930741..b51bc04 100644
> --- a/net/vhost-user.c
> +++ b/net/vhost-user.c
> @@ -50,7 +50,6 @@ static int vhost_user_start(VhostUserState *s)
>      options.backend_type = VHOST_BACKEND_TYPE_USER;
>      options.net_backend = &s->nc;
>      options.opaque = s->chr;
> -    options.force = true;
>  
>      s->vhost_net = vhost_net_init(&options);
>