hw/block/vhost-user-blk.c | 1 - hw/net/vhost_net.c | 14 ++++++-------- hw/scsi/vhost-scsi.c | 1 - hw/scsi/vhost-user-scsi.c | 1 - hw/virtio/vdpa-dev.c | 1 - hw/virtio/vhost-user.c | 25 ++++++++++++++++--------- hw/virtio/vhost.c | 15 ++++++--------- hw/virtio/virtio-qmp.c | 2 -- include/hw/virtio/vhost-backend.h | 2 ++ include/hw/virtio/vhost.h | 7 ------- 10 files changed, 30 insertions(+), 39 deletions(-)
This field is mostly unused and sometimes confusing (we even have a TODO-like comment to drop it). Let's finally do. Vladimir Sementsov-Ogievskiy (4): vhost: introduce vhost_ops->vhost_set_vring_enable_supported method vhost-user: stop use backend_features vhost_net: stop use backend_features hw/vhost: finally drop vhost_dev.backend_features field hw/block/vhost-user-blk.c | 1 - hw/net/vhost_net.c | 14 ++++++-------- hw/scsi/vhost-scsi.c | 1 - hw/scsi/vhost-user-scsi.c | 1 - hw/virtio/vdpa-dev.c | 1 - hw/virtio/vhost-user.c | 25 ++++++++++++++++--------- hw/virtio/vhost.c | 15 ++++++--------- hw/virtio/virtio-qmp.c | 2 -- include/hw/virtio/vhost-backend.h | 2 ++ include/hw/virtio/vhost.h | 7 ------- 10 files changed, 30 insertions(+), 39 deletions(-) -- 2.48.1
ping:) On 03.07.25 15:47, Vladimir Sementsov-Ogievskiy wrote: > This field is mostly unused and sometimes confusing (we even have > a TODO-like comment to drop it). Let's finally do. > > Vladimir Sementsov-Ogievskiy (4): > vhost: introduce vhost_ops->vhost_set_vring_enable_supported method > vhost-user: stop use backend_features > vhost_net: stop use backend_features > hw/vhost: finally drop vhost_dev.backend_features field > > hw/block/vhost-user-blk.c | 1 - > hw/net/vhost_net.c | 14 ++++++-------- > hw/scsi/vhost-scsi.c | 1 - > hw/scsi/vhost-user-scsi.c | 1 - > hw/virtio/vdpa-dev.c | 1 - > hw/virtio/vhost-user.c | 25 ++++++++++++++++--------- > hw/virtio/vhost.c | 15 ++++++--------- > hw/virtio/virtio-qmp.c | 2 -- > include/hw/virtio/vhost-backend.h | 2 ++ > include/hw/virtio/vhost.h | 7 ------- > 10 files changed, 30 insertions(+), 39 deletions(-) > -- Best regards, Vladimir
I tested this series of patches with virtio-net regression tests which were triggered because the virtio-net code was changed. Everything works fine. Tested-by: Lei Yang <leiyang@redhat.com> On Thu, Jul 3, 2025 at 8:55 PM Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> wrote: > > This field is mostly unused and sometimes confusing (we even have > a TODO-like comment to drop it). Let's finally do. > > Vladimir Sementsov-Ogievskiy (4): > vhost: introduce vhost_ops->vhost_set_vring_enable_supported method > vhost-user: stop use backend_features > vhost_net: stop use backend_features > hw/vhost: finally drop vhost_dev.backend_features field > > hw/block/vhost-user-blk.c | 1 - > hw/net/vhost_net.c | 14 ++++++-------- > hw/scsi/vhost-scsi.c | 1 - > hw/scsi/vhost-user-scsi.c | 1 - > hw/virtio/vdpa-dev.c | 1 - > hw/virtio/vhost-user.c | 25 ++++++++++++++++--------- > hw/virtio/vhost.c | 15 ++++++--------- > hw/virtio/virtio-qmp.c | 2 -- > include/hw/virtio/vhost-backend.h | 2 ++ > include/hw/virtio/vhost.h | 7 ------- > 10 files changed, 30 insertions(+), 39 deletions(-) > > -- > 2.48.1 > >
On Thu, Jul 03, 2025 at 03:47:08PM +0300, Vladimir Sementsov-Ogievskiy wrote: > This field is mostly unused and sometimes confusing (we even have > a TODO-like comment to drop it). Let's finally do. Breaks make check with UBSAN enabled: 32/109 /arm/virt/virtio-mmio/virtio-bus/virtio-net-device/virtio-net/virtio-net-tests/vhost-user/flags-mismatch - ERROR:../tests/qtest/qos-test.c:189:subprocess_run_one_test: child process (/arm/virt/virtio-mmio/virtio-bus/virtio-net-device/virtio-net/virtio-net-tests/vhost-user/flags-mismatch/subprocess [9701]) failed unexpectedly FAIL https://gitlab.com/mstredhat/qemu/-/jobs/10668177755 To trigger, configure with: ./configure '--cc=clang' '--cxx=clang++' '--enable-ubsan' '--extra-cflags=-fno-sanitize-recover=undefined -fno-sanitize=pointer-overflow' '--target-list=arm-softmmu' make make check > Vladimir Sementsov-Ogievskiy (4): > vhost: introduce vhost_ops->vhost_set_vring_enable_supported method > vhost-user: stop use backend_features > vhost_net: stop use backend_features > hw/vhost: finally drop vhost_dev.backend_features field > > hw/block/vhost-user-blk.c | 1 - > hw/net/vhost_net.c | 14 ++++++-------- > hw/scsi/vhost-scsi.c | 1 - > hw/scsi/vhost-user-scsi.c | 1 - > hw/virtio/vdpa-dev.c | 1 - > hw/virtio/vhost-user.c | 25 ++++++++++++++++--------- > hw/virtio/vhost.c | 15 ++++++--------- > hw/virtio/virtio-qmp.c | 2 -- > include/hw/virtio/vhost-backend.h | 2 ++ > include/hw/virtio/vhost.h | 7 ------- > 10 files changed, 30 insertions(+), 39 deletions(-) > > -- > 2.48.1
On 14.07.25 13:41, Michael S. Tsirkin wrote:
> On Thu, Jul 03, 2025 at 03:47:08PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> This field is mostly unused and sometimes confusing (we even have
>> a TODO-like comment to drop it). Let's finally do.
>
> Breaks make check with UBSAN enabled:
> 32/109 /arm/virt/virtio-mmio/virtio-bus/virtio-net-device/virtio-net/virtio-net-tests/vhost-user/flags-mismatch - ERROR:../tests/qtest/qos-test.c:189:subprocess_run_one_test: child process (/arm/virt/virtio-mmio/virtio-bus/virtio-net-device/virtio-net/virtio-net-tests/vhost-user/flags-mismatch/subprocess [9701]) failed unexpectedly FAIL
>
>
>
> https://gitlab.com/mstredhat/qemu/-/jobs/10668177755
>
>
> To trigger, configure with:
>
> ./configure '--cc=clang' '--cxx=clang++' '--enable-ubsan' '--extra-cflags=-fno-sanitize-recover=undefined -fno-sanitize=pointer-overflow' '--target-list=arm-softmmu'
>
> make
> make check
Thanks for reproducer, it works for me.
Hm. Seems, that because I miss that I've changed the behavior for vhost_net_ack_features():
with may patches it doesn't include VHOST_USER_F_PROTOCOL_FEATURES (taken from backend_features) into acked_features..
And with such change:
void vhost_net_ack_features(struct vhost_net *net, uint64_t features)
{
- net->dev.acked_features = qemu_has_vnet_hdr(net->nc)
- ? 0 : (1ULL << VHOST_NET_F_VIRTIO_NET_HDR);
+ net->dev.acked_features =
+ (qemu_has_vnet_hdr(net->nc) ? 0 :
+ (1ULL << VHOST_NET_F_VIRTIO_NET_HDR)) |
+ (net->dev.features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES));
vhost_ack_features(&net->dev, vhost_net_get_feature_bits(net), features);
}
test starts to work again.
But I can't understand, what is the relation with VHOST_USER_F_PROTOCOL_FEATURES and acked_features.
I though, VHOST_USER_F_PROTOCOL_FEATURES make sense only for vhost <-> QEMU communication, and should
not present in guest-negotiated acked_features..
>
>
>> Vladimir Sementsov-Ogievskiy (4):
>> vhost: introduce vhost_ops->vhost_set_vring_enable_supported method
>> vhost-user: stop use backend_features
>> vhost_net: stop use backend_features
>> hw/vhost: finally drop vhost_dev.backend_features field
>>
>> hw/block/vhost-user-blk.c | 1 -
>> hw/net/vhost_net.c | 14 ++++++--------
>> hw/scsi/vhost-scsi.c | 1 -
>> hw/scsi/vhost-user-scsi.c | 1 -
>> hw/virtio/vdpa-dev.c | 1 -
>> hw/virtio/vhost-user.c | 25 ++++++++++++++++---------
>> hw/virtio/vhost.c | 15 ++++++---------
>> hw/virtio/virtio-qmp.c | 2 --
>> include/hw/virtio/vhost-backend.h | 2 ++
>> include/hw/virtio/vhost.h | 7 -------
>> 10 files changed, 30 insertions(+), 39 deletions(-)
>>
>> --
>> 2.48.1
>
--
Best regards,
Vladimir
On 14.07.25 22:23, Vladimir Sementsov-Ogievskiy wrote:
> On 14.07.25 13:41, Michael S. Tsirkin wrote:
>> On Thu, Jul 03, 2025 at 03:47:08PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>>> This field is mostly unused and sometimes confusing (we even have
>>> a TODO-like comment to drop it). Let's finally do.
>>
>> Breaks make check with UBSAN enabled:
>> 32/109 /arm/virt/virtio-mmio/virtio-bus/virtio-net-device/virtio-net/virtio-net-tests/vhost-user/flags-mismatch - ERROR:../tests/qtest/qos-test.c:189:subprocess_run_one_test: child process (/arm/virt/virtio-mmio/virtio-bus/virtio-net-device/virtio-net/virtio-net-tests/vhost-user/flags-mismatch/subprocess [9701]) failed unexpectedly FAIL
>>
>>
>>
>> https://gitlab.com/mstredhat/qemu/-/jobs/10668177755
>>
>>
>> To trigger, configure with:
>>
>> ./configure '--cc=clang' '--cxx=clang++' '--enable-ubsan' '--extra-cflags=-fno-sanitize-recover=undefined -fno-sanitize=pointer-overflow' '--target-list=arm-softmmu'
>>
>> make
>> make check
>
>
> Thanks for reproducer, it works for me.
>
>
> Hm. Seems, that because I miss that I've changed the behavior for vhost_net_ack_features():
> with may patches it doesn't include VHOST_USER_F_PROTOCOL_FEATURES (taken from backend_features) into acked_features..
>
> And with such change:
>
> void vhost_net_ack_features(struct vhost_net *net, uint64_t features)
> {
> - net->dev.acked_features = qemu_has_vnet_hdr(net->nc)
> - ? 0 : (1ULL << VHOST_NET_F_VIRTIO_NET_HDR);
> + net->dev.acked_features =
> + (qemu_has_vnet_hdr(net->nc) ? 0 :
> + (1ULL << VHOST_NET_F_VIRTIO_NET_HDR)) |
> + (net->dev.features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES));
> vhost_ack_features(&net->dev, vhost_net_get_feature_bits(net), features);
> }
>
>
> test starts to work again.
>
> But I can't understand, what is the relation with VHOST_USER_F_PROTOCOL_FEATURES and acked_features.
> I though, VHOST_USER_F_PROTOCOL_FEATURES make sense only for vhost <-> QEMU communication, and should
> not present in guest-negotiated acked_features..
>
>
UPD: undestand.
the test wants to check features mismatch, and on this code in vhost_net_init():
#ifdef CONFIG_VHOST_NET_USER
if (net->nc->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
features = vhost_user_get_acked_features(net->nc);
if (~net->dev.features & features) {
fprintf(stderr, "vhost lacks feature mask 0x%" PRIx64
" for backend\n",
(uint64_t)(~net->dev.features & features));
goto fail;
}
}
#endif
still the only mismatching feature is VHOST_USER_F_PROTOCOL_FEATURES, which vhost_net code (before my changes) passes to acked features.
Instead of exploiting acked_features this way, we may improve this check to also check VHOST_USER_F_PROTOCOL_FEATURES, which is vhost-user-only.
I'll resend
>>
>>
>>> Vladimir Sementsov-Ogievskiy (4):
>>> vhost: introduce vhost_ops->vhost_set_vring_enable_supported method
>>> vhost-user: stop use backend_features
>>> vhost_net: stop use backend_features
>>> hw/vhost: finally drop vhost_dev.backend_features field
>>>
>>> hw/block/vhost-user-blk.c | 1 -
>>> hw/net/vhost_net.c | 14 ++++++--------
>>> hw/scsi/vhost-scsi.c | 1 -
>>> hw/scsi/vhost-user-scsi.c | 1 -
>>> hw/virtio/vdpa-dev.c | 1 -
>>> hw/virtio/vhost-user.c | 25 ++++++++++++++++---------
>>> hw/virtio/vhost.c | 15 ++++++---------
>>> hw/virtio/virtio-qmp.c | 2 --
>>> include/hw/virtio/vhost-backend.h | 2 ++
>>> include/hw/virtio/vhost.h | 7 -------
>>> 10 files changed, 30 insertions(+), 39 deletions(-)
>>>
>>> --
>>> 2.48.1
>>
>
--
Best regards,
Vladimir
© 2016 - 2025 Red Hat, Inc.