From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39112) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zoqne-0005Nk-8G for qemu-devel@nongnu.org; Wed, 21 Oct 2015 06:39:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zoqna-0001zk-7B for qemu-devel@nongnu.org; Wed, 21 Oct 2015 06:39:18 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60768) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zoqna-0001zV-2c for qemu-devel@nongnu.org; Wed, 21 Oct 2015 06:39:14 -0400 Date: Wed, 21 Oct 2015 13:39:11 +0300 From: "Michael S. Tsirkin" Message-ID: <20151021133634-mutt-send-email-mst@redhat.com> References: <1445418438-24244-1-git-send-email-yuanhan.liu@linux.intel.com> <1445418438-24244-5-git-send-email-yuanhan.liu@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1445418438-24244-5-git-send-email-yuanhan.liu@linux.intel.com> Subject: Re: [Qemu-devel] [PATCH v2 5/5] vhost: send VHOST_USER_SET_VRING_ENABLE at start/stop List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Yuanhan Liu Cc: qemu-devel@nongnu.org On Wed, Oct 21, 2015 at 05:07:18PM +0800, Yuanhan Liu wrote: > Send VHOST_USER_SET_VRING_ENABLE at start/stop when multiple queue > is negotiated, to inform the backend that we are ready or not. OK but that's only if MQ is set. If now, we need to do RESET_OWNER followed by SET_OWNER. > And exclude VHOST_USER_GET_QUEUE_NUM as one time request, as we need > to get max_queues for each vhost_dev. Pls split this part out. > Suggested-by: Michael S. Tsirkin > Signed-off-by: Yuanhan Liu > --- > hw/virtio/vhost-user.c | 1 - > hw/virtio/vhost.c | 18 ++++++++++++++++++ > 2 files changed, 18 insertions(+), 1 deletion(-) > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > index 12a9104..6532a73 100644 > --- a/hw/virtio/vhost-user.c > +++ b/hw/virtio/vhost-user.c > @@ -194,7 +194,6 @@ static bool vhost_user_one_time_request(VhostUserRequest request) > case VHOST_USER_SET_OWNER: > case VHOST_USER_RESET_OWNER: > case VHOST_USER_SET_MEM_TABLE: > - case VHOST_USER_GET_QUEUE_NUM: > return true; > default: > return false; > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c > index c0ed5b2..54a4633 100644 > --- a/hw/virtio/vhost.c > +++ b/hw/virtio/vhost.c > @@ -1146,6 +1146,19 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev) > } > } > > + /* > + * Send VHOST_USER_SET_VRING_ENABLE message when multiple queue > + * is negotiated to inform the back end that we are ready. > + * > + * Only set enable to 1 for first queue pair, as we enable one > + * queue pair by default. > + */ > + if (hdev->max_queues > 1 && this makes no sense in the generic code. This is a work around for a protocol bug - belongs in vhost user internally. And that's not the way to test this. MQ could be negotiated even if there is a single pair of queues. > + hdev->vhost_ops->vhost_backend_set_vring_enable) { > + hdev->vhost_ops->vhost_backend_set_vring_enable(hdev, > + hdev->vq_index == 0); > + } > + > return 0; > fail_log: > vhost_log_put(hdev, false); > @@ -1180,5 +1193,10 @@ void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev) > hdev->started = false; > hdev->log = NULL; > hdev->log_size = 0; > + > + if (hdev->max_queues > 1 && > + hdev->vhost_ops->vhost_backend_set_vring_enable) { > + hdev->vhost_ops->vhost_backend_set_vring_enable(hdev, 0); > + } > } > > -- > 1.9.0 >