From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46935) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZKVjZ-0004c3-72 for qemu-devel@nongnu.org; Wed, 29 Jul 2015 14:05:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZKVjT-0004Pg-VC for qemu-devel@nongnu.org; Wed, 29 Jul 2015 14:05:40 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50713) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZKVjT-0004PJ-Qk for qemu-devel@nongnu.org; Wed, 29 Jul 2015 14:05:35 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (Postfix) with ESMTPS id 0630BB6E7E for ; Wed, 29 Jul 2015 18:05:34 +0000 (UTC) Date: Wed, 29 Jul 2015 21:05:31 +0300 From: "Michael S. Tsirkin" Message-ID: <20150729210414-mutt-send-email-mst@redhat.com> References: <1438189918-2087-1-git-send-email-marcel@redhat.com> <20150729201945-mutt-send-email-mst@redhat.com> <55B910D3.9060103@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <55B910D3.9060103@redhat.com> Subject: Re: [Qemu-devel] [PATCH] vhost-user: sync backend features List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Marcel Apfelbaum Cc: jasowang@redhat.com, famz@redhat.com, qemu-devel@nongnu.org On Wed, Jul 29, 2015 at 08:43:47PM +0300, Marcel Apfelbaum wrote: > On 07/29/2015 08:20 PM, Michael S. Tsirkin wrote: > >On Wed, Jul 29, 2015 at 08:11:58PM +0300, Marcel Apfelbaum wrote: > >>Complete vhost-user negotiation by syncing the features > >>supported by the backend. > >> > >>Signed-off-by: Marcel Apfelbaum > >>--- > >> To be used on top of: > >> [PATCH 0/4] vhost-user: protocol updates > >> https://lists.gnu.org/archive/html/qemu-devel/2015-07/msg03842.html > >> > >> Currently the vhost-user supported features are not evaluated. > >> The way I see it, and please correct me, the best way to do > >> this is to: > >> 1. get the backend features on vhost init > >> 2. Instead of simply copying them during features ack, > >> check that that all backend features are supported by current QEMU > >> 3. All other code should remain the same. > >> > >> Thanks, > >> Marcel > >> > >> hw/net/vhost_net.c | 3 ++- > >> hw/virtio/vhost-user.c | 4 ++-- > >> 2 files changed, 4 insertions(+), 3 deletions(-) > >> > >>diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c > >>index c864237..1ea5866 100644 > >>--- a/hw/net/vhost_net.c > >>+++ b/hw/net/vhost_net.c > >>@@ -118,7 +118,8 @@ uint64_t vhost_net_get_features(struct vhost_net *net, uint64_t features) > >> > >> void vhost_net_ack_features(struct vhost_net *net, uint64_t features) > >> { > >>- net->dev.acked_features = net->dev.backend_features; > >>+ vhost_ack_features(&net->dev, vhost_net_get_feature_bits(net), > >>+ net->dev.backend_features); > >> vhost_ack_features(&net->dev, vhost_net_get_feature_bits(net), features); > > > >So you ack it twice? > Not really, first call to vhost_ack_features acks the net->dev.backend_features, > the second one acks the guest features. > > The first call replaces the previous assignment that assumes that all backend features are > supported by QEMU. > > Thanks, > Marcel I think it's cleaner to whitelist backend features QEMU supports. I thought we did this, will look again. > > > > >> } > >> > >>diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > >>index c4428a1..077457b 100644 > >>--- a/hw/virtio/vhost-user.c > >>+++ b/hw/virtio/vhost-user.c > >>@@ -358,9 +358,9 @@ static int vhost_user_init(struct vhost_dev *dev, void *opaque) > >> return err; > >> } > >> > >>- if (__virtio_has_feature(msg.u64, VHOST_USER_F_PROTOCOL_FEATURES)) { > >>- dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES; > >>+ dev->backend_features = msg.u64; > >> > >>+ if (__virtio_has_feature(msg.u64, VHOST_USER_F_PROTOCOL_FEATURES)) { > >> msg.request = VHOST_USER_GET_PROTOCOL_FEATURES; > >> msg.flags = VHOST_USER_VERSION; > >> msg.size = 0; > >>-- > >>2.1.0