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
