From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42208) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZKVOY-0004fo-2s for qemu-devel@nongnu.org; Wed, 29 Jul 2015 13:44:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZKVOS-0000lU-6t for qemu-devel@nongnu.org; Wed, 29 Jul 2015 13:43:58 -0400 Received: from mx1.redhat.com ([209.132.183.28]:45068) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZKVOS-0000lQ-0m for qemu-devel@nongnu.org; Wed, 29 Jul 2015 13:43:52 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id 2E57483F84 for ; Wed, 29 Jul 2015 17:43:51 +0000 (UTC) References: <1438189918-2087-1-git-send-email-marcel@redhat.com> <20150729201945-mutt-send-email-mst@redhat.com> From: Marcel Apfelbaum Message-ID: <55B910D3.9060103@redhat.com> Date: Wed, 29 Jul 2015 20:43:47 +0300 MIME-Version: 1.0 In-Reply-To: <20150729201945-mutt-send-email-mst@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] vhost-user: sync backend features List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: jasowang@redhat.com, famz@redhat.com, qemu-devel@nongnu.org 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 > >> } >> >> 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