Re: [Qemu-devel] [PATCH] vhost-user: sync backend features

[Marcel Apfelbaum <marcel@redhat.com>]

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 <marcel@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


>
>>   }
>>
>> 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