Re: [PATCH] vhost-user: Fix protocol feature bit conflict

[Hanna Czenczek <hreitz@redhat.com>]

On 17.10.23 07:36, Viresh Kumar wrote:
> On 16-10-23, 12:40, Alex Bennée wrote:
>> Viresh Kumar <viresh.kumar@linaro.org> writes:
>>
>>> On 16-10-23, 11:45, Manos Pitsidianakis wrote:
>>>> On Mon, 16 Oct 2023 11:32, Hanna Czenczek <hreitz@redhat.com> wrote:
>>>>> diff --git a/include/hw/virtio/vhost-user.h
>>>>> b/include/hw/virtio/vhost-user.h
>>>>> index 9f9ddf878d..1d4121431b 100644
>>>>> --- a/include/hw/virtio/vhost-user.h
>>>>> +++ b/include/hw/virtio/vhost-user.h
>>>>> @@ -29,7 +29,8 @@ enum VhostUserProtocolFeature {
>>>>>      VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS = 14,
>>>>>      VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS = 15,
>>>>>      VHOST_USER_PROTOCOL_F_STATUS = 16,
>>>>> -    VHOST_USER_PROTOCOL_F_SHARED_OBJECT = 17,
>>>>> +    /* Feature 17 reserved for VHOST_USER_PROTOCOL_F_XEN_MMAP. */
>>>>> +    VHOST_USER_PROTOCOL_F_SHARED_OBJECT = 18,
>>>>>      VHOST_USER_PROTOCOL_F_MAX
>>>>> };
>>>> May I ask, why not define VHOST_USER_PROTOCOL_F_XEN_MMAP as well instead of
>>>> a comment mention?
>>> Perhaps because we will never use it from Qemu code ?
>> Vikram's work on enabling xenpvh support will mean enabling grant
>> support and while I suspect most VirtIO backends will be within QEMU
>> itself if it ever want to off-load something to a vhost-user backend it
>> will need to ensure this flag is set.
> Hanna,
>
> It would be good to define it then in the current commit itself.

Not that I’m really opposed to that, but I don’t see the problem with 
just doing that in the same work that makes qemu actually use this flag, 
exactly because it’s just a -1/+1 change.

I can send a v2, but should I do the same for libvhost-user and define 
the flag there?  Do I have to add a patch to do the same for F_STATUS, 
which so far only got a placeholder comment?

Hanna