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 <mar...@redhat.com>
> >>---
> >> 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
