diff mbox series

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

Message ID 20220727155653.3974426-2-alex.bennee@linaro.org
State New
Headers show
Series virtio fixes (split from virtio-gpio series) | expand

Commit Message

Alex Bennée July 27, 2022, 3:56 p.m. UTC 
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(-)

Comments

Kevin Wolf July 28, 2022, 8:46 a.m. UTC | #1 
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.

Kevin
Alex Bennée July 28, 2022, 10:27 a.m. UTC | #2 
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
diff mbox series

Patch

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 ;
 
     if (!vexp->handler.writable) {
         features |= 1ull << VIRTIO_BLK_F_RO;