Re: [PATCH v1 1/5] block/vhost-user-blk-server: don't expose VHOST_USER_F_PROTOCOL_FEATURES

[Alex Bennée]

Kevin Wolf <kwolf@redhat.com> writes:

> Am 27.07.2022 um 17:56 hat Alex Bennée geschrieben:
>> This bit is unused in actual VirtIO feature negotiation and should
>> only appear in the vhost-user messages between master and slave.
>> 
>> [AJB: experiment, this doesn't break the tests but I'm not super
>> confident of the range of tests]
>> 
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Message-Id: <20220726192150.2435175-6-alex.bennee@linaro.org>
>> ---
>>  block/export/vhost-user-blk-server.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>> 
>> diff --git a/block/export/vhost-user-blk-server.c 
>> b/block/export/vhost-user-blk-server.c
>> index 3409d9e02e..d31436006d 100644
>> --- a/block/export/vhost-user-blk-server.c
>> +++ b/block/export/vhost-user-blk-server.c
>> @@ -125,8 +125,7 @@ static uint64_t vu_blk_get_features(VuDev *dev)
>>                 1ull << VIRTIO_BLK_F_MQ |
>>                 1ull << VIRTIO_F_VERSION_1 |
>>                 1ull << VIRTIO_RING_F_INDIRECT_DESC |
>> -               1ull << VIRTIO_RING_F_EVENT_IDX |
>> -               1ull << VHOST_USER_F_PROTOCOL_FEATURES;
>> +               1ull << VIRTIO_RING_F_EVENT_IDX ;
>
> I didn't see this series yet when I replied to the other series this is
> split off from, but of course, my comments are still relevant for this
> one.
>
> I asked for a changed commit message (the "experiment" part should
> probably go away if we're merging it; instead, it should explain that
> in vu_get_features_exec(), libvhost-user adds the vhost-user protocol
> level VHOST_USER_F_PROTOCOL_FEATURES flag anyway and the device is the
> wrong layer to add it, but the behaviour doesn't change with this patch)
> and noted the extra space before the semicolon.

OK - mst do you want me to respin or can you tweak the commit?

>
> Kevin


-- 
Alex Bennée