From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33822) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XP5ur-0008W2-Dn for qemu-devel@nongnu.org; Wed, 03 Sep 2014 04:27:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XP5un-0006Wp-4e for qemu-devel@nongnu.org; Wed, 03 Sep 2014 04:27:45 -0400 Date: Wed, 3 Sep 2014 11:30:52 +0300 From: "Michael S. Tsirkin" Message-ID: <20140903083052.GA6468@redhat.com> References: <1409725530-1425-1-git-send-email-jasowang@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1409725530-1425-1-git-send-email-jasowang@redhat.com> Subject: Re: [Qemu-devel] [PATCH] vhost_net: initialize acked_features to a safe value during ack List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Wang Cc: qemu-stable@nongnu.org, Andrey Korolyov , qemu-devel@nongnu.org, Nikolay Nikolaev , Michael Roth On Wed, Sep 03, 2014 at 02:25:30PM +0800, Jason Wang wrote: > commit 2e6d46d77ed328d34a94688da8371bcbe243479b (vhost: add > vhost_get_features and vhost_ack_features) removes the step that > initializes the acked_features to backend_features. But acked features are set in vhost_ack_features. why would we need to initialize to backend_features? 0 is a better default. > This will result an > unexpected value of acked_features which may fail the features setting of > vhost. This patch fixes it by recover this step. > > Cc: Nikolay Nikolaev > Cc: Andrey Korolyov > Cc: Michael S. Tsirkin > Cc: Michael Roth > Cc: qemu-stable@nongnu.org > Signed-off-by: Jason Wang > --- > hw/net/vhost_net.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c > index f87c798..b1d4b1f 100644 > --- a/hw/net/vhost_net.c > +++ b/hw/net/vhost_net.c > @@ -115,6 +115,7 @@ unsigned vhost_net_get_features(struct vhost_net *net, unsigned features) > > void vhost_net_ack_features(struct vhost_net *net, unsigned features) > { > + net->dev.acked_features = net->dev.backend_features; I think it's wrong: you don't want to set all features. > vhost_ack_features(&net->dev, vhost_net_get_feature_bits(net), features); > } > > -- > 1.8.3.1