[PATCH v2 02/23] vhost: drop backend_features field

Vladimir Sementsov-Ogievskiy posted 23 patches 1 month ago
Maintainers: "Gonglei (Arei)" <arei.gonglei@huawei.com>, Zhenwei Pi <pizhenwei@bytedance.com>, "Michael S. Tsirkin" <mst@redhat.com>, Stefano Garzarella <sgarzare@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Raphael Norwitz <raphael@enfabrica.net>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Jason Wang <jasowang@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Fam Zheng <fam@euphon.net>, "Alex Bennée" <alex.bennee@linaro.org>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>
There is a newer version of this series
[PATCH v2 02/23] vhost: drop backend_features field
Posted by Vladimir Sementsov-Ogievskiy 1 month ago
This field is mostly unused and sometimes confusing (we even have
a TODO-like comment to drop it). Let's finally do.

The field is used to held VHOST_USER_F_PROTOCOL_FEATURES for vhost-user
and/or VHOST_NET_F_VIRTIO_NET_HDR for vhost-net (which may be
vhost-user-net). But we can simply recalculate these two flags in place
from hdev->features, and from net-client for VHOST_NET_F_VIRTIO_NET_HDR.

Note: removing field from x-query-virtio-status result is incompatible
change. We can do it because the command is unstable.

Cc: qemu-stable@nongnu.org
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 hw/block/vhost-user-blk.c   |  1 -
 hw/net/vhost_net.c          | 26 ++++++++++++++------------
 hw/scsi/vhost-scsi.c        |  1 -
 hw/scsi/vhost-user-scsi.c   |  1 -
 hw/virtio/vdpa-dev.c        |  1 -
 hw/virtio/vhost-user.c      | 17 ++++++++---------
 hw/virtio/virtio-hmp-cmds.c |  2 --
 hw/virtio/virtio-qmp.c      |  4 ----
 include/hw/virtio/vhost.h   |  7 -------
 qapi/virtio.json            |  3 ---
 10 files changed, 22 insertions(+), 41 deletions(-)

diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index c0cc5f6942..de7a810c93 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -348,7 +348,6 @@ static int vhost_user_blk_connect(DeviceState *dev, Error **errp)
     s->dev.nvqs = s->num_queues;
     s->dev.vqs = s->vhost_vqs;
     s->dev.vq_index = 0;
-    s->dev.backend_features = 0;
 
     vhost_dev_set_config_notifier(&s->dev, &blk_ops);
 
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 25e9f1fd24..fda90e231e 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -52,8 +52,14 @@ int vhost_net_set_config(struct vhost_net *net, const uint8_t *data,
 
 void vhost_net_ack_features_ex(struct vhost_net *net, const uint64_t *features)
 {
-    virtio_features_copy(net->dev.acked_features_ex,
-                         net->dev.backend_features_ex);
+    virtio_features_clear(net->dev.acked_features_ex);
+    if (net->backend == -1) {
+        net->dev.acked_features =
+           net->dev.features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES);
+    } else if (!qemu_has_vnet_hdr(net->nc)) {
+        net->dev.acked_features = 1ULL << VHOST_NET_F_VIRTIO_NET_HDR;
+    }
+
     vhost_ack_features_ex(&net->dev, net->feature_bits, features);
 }
 
@@ -258,12 +264,9 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
         if (r < 0) {
             goto fail;
         }
-        net->dev.backend_features = qemu_has_vnet_hdr(options->net_backend)
-            ? 0 : (1ULL << VHOST_NET_F_VIRTIO_NET_HDR);
         net->backend = r;
         net->dev.protocol_features = 0;
     } else {
-        virtio_features_clear(net->dev.backend_features_ex);
         net->dev.protocol_features = 0;
         net->backend = -1;
 
@@ -284,13 +287,12 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
             net->dev.features &= ~(1ULL << VIRTIO_NET_F_MRG_RXBUF);
         }
 
-        if (virtio_features_andnot(missing_features,
-                                   net->dev.backend_features_ex,
-                                   net->dev.features_ex)) {
-            fprintf(stderr, "vhost lacks feature mask 0x" VIRTIO_FEATURES_FMT
-                   " for backend\n", VIRTIO_FEATURES_PR(missing_features));
-            goto fail;
-        }
+        if (!qemu_has_vnet_hdr(options->net_backend) &&
+            (~net->dev.features & (1ULL << VHOST_NET_F_VIRTIO_NET_HDR))) {
+            fprintf(stderr, "vhost lacks feature mask 0x%llx for backend\n",
+                    ~net->dev.features & (1ULL << VHOST_NET_F_VIRTIO_NET_HDR));
+             goto fail;
+         }
     }
 
     /* Set sane init value. Override when guest acks. */
diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index cdf405b0f8..d694a25fe2 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -276,7 +276,6 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp)
     vqs = g_new0(struct vhost_virtqueue, vsc->dev.nvqs);
     vsc->dev.vqs = vqs;
     vsc->dev.vq_index = 0;
-    vsc->dev.backend_features = 0;
 
     ret = vhost_dev_init(&vsc->dev, (void *)(uintptr_t)vhostfd,
                          VHOST_BACKEND_TYPE_KERNEL, 0, errp);
diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
index 25f2d894e7..0c80a271d8 100644
--- a/hw/scsi/vhost-user-scsi.c
+++ b/hw/scsi/vhost-user-scsi.c
@@ -159,7 +159,6 @@ static int vhost_user_scsi_connect(DeviceState *dev, Error **errp)
     vsc->dev.nvqs = VIRTIO_SCSI_VQ_NUM_FIXED + vs->conf.num_queues;
     vsc->dev.vqs = s->vhost_vqs;
     vsc->dev.vq_index = 0;
-    vsc->dev.backend_features = 0;
 
     ret = vhost_dev_init(&vsc->dev, &s->vhost_user, VHOST_BACKEND_TYPE_USER, 0,
                          errp);
diff --git a/hw/virtio/vdpa-dev.c b/hw/virtio/vdpa-dev.c
index 4a7b970976..efd9f68420 100644
--- a/hw/virtio/vdpa-dev.c
+++ b/hw/virtio/vdpa-dev.c
@@ -104,7 +104,6 @@ static void vhost_vdpa_device_realize(DeviceState *dev, Error **errp)
     v->dev.vqs = vqs;
     v->dev.vq_index = 0;
     v->dev.vq_index_end = v->dev.nvqs;
-    v->dev.backend_features = 0;
     v->started = false;
 
     ret = vhost_vdpa_get_iova_range(v->vhostfd, &iova_range);
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index f367ce06ce..b80174f489 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1448,14 +1448,15 @@ static int vhost_user_set_features(struct vhost_dev *dev,
     int ret;
 
     /*
-     * We need to include any extra backend only feature bits that
-     * might be needed by our device. Currently this includes the
-     * VHOST_USER_F_PROTOCOL_FEATURES bit for enabling protocol
-     * features.
+     * Don't lose VHOST_USER_F_PROTOCOL_FEATURES, which is vhost-user
+     * specific.
      */
-    ret = vhost_user_set_u64(dev, VHOST_USER_SET_FEATURES,
-                              features | dev->backend_features,
-                              log_enabled);
+    if (virtio_has_feature(dev->features, VHOST_USER_F_PROTOCOL_FEATURES)) {
+        features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
+    }
+
+    ret = vhost_user_set_u64(dev, VHOST_USER_SET_FEATURES, features,
+                             log_enabled);
 
     if (virtio_has_feature(dev->protocol_features,
                            VHOST_USER_PROTOCOL_F_STATUS)) {
@@ -2189,8 +2190,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
             (dev->config_ops && dev->config_ops->vhost_dev_config_notifier);
         uint64_t protocol_features;
 
-        dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
-
         err = vhost_user_get_u64(dev, VHOST_USER_GET_PROTOCOL_FEATURES,
                                  &protocol_features);
         if (err < 0) {
diff --git a/hw/virtio/virtio-hmp-cmds.c b/hw/virtio/virtio-hmp-cmds.c
index 1daae482d3..4bf9a3109d 100644
--- a/hw/virtio/virtio-hmp-cmds.c
+++ b/hw/virtio/virtio-hmp-cmds.c
@@ -176,8 +176,6 @@ void hmp_virtio_status(Monitor *mon, const QDict *qdict)
         hmp_virtio_dump_features(mon, s->vhost_dev->features);
         monitor_printf(mon, "    Acked features:\n");
         hmp_virtio_dump_features(mon, s->vhost_dev->acked_features);
-        monitor_printf(mon, "    Backend features:\n");
-        hmp_virtio_dump_features(mon, s->vhost_dev->backend_features);
         monitor_printf(mon, "    Protocol features:\n");
         hmp_virtio_dump_protocols(mon, s->vhost_dev->protocol_features);
     }
diff --git a/hw/virtio/virtio-qmp.c b/hw/virtio/virtio-qmp.c
index b338344c6c..dd1a38e792 100644
--- a/hw/virtio/virtio-qmp.c
+++ b/hw/virtio/virtio-qmp.c
@@ -780,8 +780,6 @@ VirtioStatus *qmp_x_query_virtio_status(const char *path, Error **errp)
                                                  vdev->guest_features_ex);
     status->host_features = qmp_decode_features(vdev->device_id,
                                                 vdev->host_features_ex);
-    status->backend_features = qmp_decode_features(vdev->device_id,
-                                                 vdev->backend_features_ex);
 
     switch (vdev->device_endian) {
     case VIRTIO_DEVICE_ENDIAN_LITTLE:
@@ -822,8 +820,6 @@ VirtioStatus *qmp_x_query_virtio_status(const char *path, Error **errp)
             qmp_decode_features(vdev->device_id, hdev->features_ex);
         status->vhost_dev->acked_features =
             qmp_decode_features(vdev->device_id, hdev->acked_features_ex);
-        status->vhost_dev->backend_features =
-            qmp_decode_features(vdev->device_id, hdev->backend_features_ex);
 
         status->vhost_dev->protocol_features =
             qmp_decode_protocols(hdev->protocol_features);
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 1ee639dd7e..3e69e47833 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -100,16 +100,9 @@ struct vhost_dev {
      *
      * @features: available features provided by the backend
      * @acked_features: final negotiated features with front-end driver
-     *
-     * @backend_features: this is used in a couple of places to either
-     * store VHOST_USER_F_PROTOCOL_FEATURES to apply to
-     * VHOST_USER_SET_FEATURES or VHOST_NET_F_VIRTIO_NET_HDR. Its
-     * future use should be discouraged and the variable retired as
-     * its easy to confuse with the VirtIO backend_features.
      */
     VIRTIO_DECLARE_FEATURES(features);
     VIRTIO_DECLARE_FEATURES(acked_features);
-    VIRTIO_DECLARE_FEATURES(backend_features);
 
     /**
      * @protocol_features: is the vhost-user only feature set by
diff --git a/qapi/virtio.json b/qapi/virtio.json
index 05295ab665..b995a5bb6d 100644
--- a/qapi/virtio.json
+++ b/qapi/virtio.json
@@ -85,8 +85,6 @@
 #
 # @acked-features: vhost_dev acked_features
 #
-# @backend-features: vhost_dev backend_features
-#
 # @protocol-features: vhost_dev protocol_features
 #
 # @max-queues: vhost_dev max_queues
@@ -106,7 +104,6 @@
             'vq-index': 'int',
             'features': 'VirtioDeviceFeatures',
             'acked-features': 'VirtioDeviceFeatures',
-            'backend-features': 'VirtioDeviceFeatures',
             'protocol-features': 'VhostDeviceProtocols',
             'max-queues': 'uint64',
             'backend-cap': 'uint64',
-- 
2.48.1
Re: [PATCH v2 02/23] vhost: drop backend_features field
Posted by Michael Tokarev 1 month ago
On 10/12/25 02:23, 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.
> 
> The field is used to held VHOST_USER_F_PROTOCOL_FEATURES for vhost-user
> and/or VHOST_NET_F_VIRTIO_NET_HDR for vhost-net (which may be
> vhost-user-net). But we can simply recalculate these two flags in place
> from hdev->features, and from net-client for VHOST_NET_F_VIRTIO_NET_HDR.
> 
> Note: removing field from x-query-virtio-status result is incompatible
> change. We can do it because the command is unstable.
> 
> Cc: qemu-stable@nongnu.org

Why do you think this change deserves qemu-stable?  Does it fix a bug or
is it just a clean-up?

Thanks,

/mjt
Re: [PATCH v2 02/23] vhost: drop backend_features field
Posted by Vladimir Sementsov-Ogievskiy 1 month ago
On 14.10.25 18:28, Michael Tokarev wrote:
> On 10/12/25 02:23, 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.
>>
>> The field is used to held VHOST_USER_F_PROTOCOL_FEATURES for vhost-user
>> and/or VHOST_NET_F_VIRTIO_NET_HDR for vhost-net (which may be
>> vhost-user-net). But we can simply recalculate these two flags in place
>> from hdev->features, and from net-client for VHOST_NET_F_VIRTIO_NET_HDR.
>>
>> Note: removing field from x-query-virtio-status result is incompatible
>> change. We can do it because the command is unstable.
>>
>> Cc: qemu-stable@nongnu.org
> 
> Why do you think this change deserves qemu-stable?  Does it fix a bug or
> is it just a clean-up?
> 

Oops, it's a mistake (working too late).

I wanted to write

Cc: devel@lists.libvirt.org

:)

Seems, v3 is needed anyway, will fix it.

-- 
Best regards,
Vladimir

Re: [PATCH v2 02/23] vhost: drop backend_features field
Posted by Raphael Norwitz 1 month ago
Acked-by: Raphael Norwitz <raphael.s.norwitz@gmail.com>

On Sat, Oct 11, 2025 at 7:24 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.
>
> The field is used to held VHOST_USER_F_PROTOCOL_FEATURES for vhost-user
> and/or VHOST_NET_F_VIRTIO_NET_HDR for vhost-net (which may be
> vhost-user-net). But we can simply recalculate these two flags in place
> from hdev->features, and from net-client for VHOST_NET_F_VIRTIO_NET_HDR.
>
> Note: removing field from x-query-virtio-status result is incompatible
> change. We can do it because the command is unstable.
>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  hw/block/vhost-user-blk.c   |  1 -
>  hw/net/vhost_net.c          | 26 ++++++++++++++------------
>  hw/scsi/vhost-scsi.c        |  1 -
>  hw/scsi/vhost-user-scsi.c   |  1 -
>  hw/virtio/vdpa-dev.c        |  1 -
>  hw/virtio/vhost-user.c      | 17 ++++++++---------
>  hw/virtio/virtio-hmp-cmds.c |  2 --
>  hw/virtio/virtio-qmp.c      |  4 ----
>  include/hw/virtio/vhost.h   |  7 -------
>  qapi/virtio.json            |  3 ---
>  10 files changed, 22 insertions(+), 41 deletions(-)
>
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> index c0cc5f6942..de7a810c93 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -348,7 +348,6 @@ static int vhost_user_blk_connect(DeviceState *dev, Error **errp)
>      s->dev.nvqs = s->num_queues;
>      s->dev.vqs = s->vhost_vqs;
>      s->dev.vq_index = 0;
> -    s->dev.backend_features = 0;
>
>      vhost_dev_set_config_notifier(&s->dev, &blk_ops);
>
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index 25e9f1fd24..fda90e231e 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -52,8 +52,14 @@ int vhost_net_set_config(struct vhost_net *net, const uint8_t *data,
>
>  void vhost_net_ack_features_ex(struct vhost_net *net, const uint64_t *features)
>  {
> -    virtio_features_copy(net->dev.acked_features_ex,
> -                         net->dev.backend_features_ex);
> +    virtio_features_clear(net->dev.acked_features_ex);
> +    if (net->backend == -1) {
> +        net->dev.acked_features =
> +           net->dev.features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES);
> +    } else if (!qemu_has_vnet_hdr(net->nc)) {
> +        net->dev.acked_features = 1ULL << VHOST_NET_F_VIRTIO_NET_HDR;
> +    }
> +
>      vhost_ack_features_ex(&net->dev, net->feature_bits, features);
>  }
>
> @@ -258,12 +264,9 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
>          if (r < 0) {
>              goto fail;
>          }
> -        net->dev.backend_features = qemu_has_vnet_hdr(options->net_backend)
> -            ? 0 : (1ULL << VHOST_NET_F_VIRTIO_NET_HDR);
>          net->backend = r;
>          net->dev.protocol_features = 0;
>      } else {
> -        virtio_features_clear(net->dev.backend_features_ex);
>          net->dev.protocol_features = 0;
>          net->backend = -1;
>
> @@ -284,13 +287,12 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
>              net->dev.features &= ~(1ULL << VIRTIO_NET_F_MRG_RXBUF);
>          }
>
> -        if (virtio_features_andnot(missing_features,
> -                                   net->dev.backend_features_ex,
> -                                   net->dev.features_ex)) {
> -            fprintf(stderr, "vhost lacks feature mask 0x" VIRTIO_FEATURES_FMT
> -                   " for backend\n", VIRTIO_FEATURES_PR(missing_features));
> -            goto fail;
> -        }
> +        if (!qemu_has_vnet_hdr(options->net_backend) &&
> +            (~net->dev.features & (1ULL << VHOST_NET_F_VIRTIO_NET_HDR))) {
> +            fprintf(stderr, "vhost lacks feature mask 0x%llx for backend\n",
> +                    ~net->dev.features & (1ULL << VHOST_NET_F_VIRTIO_NET_HDR));
> +             goto fail;
> +         }
>      }
>
>      /* Set sane init value. Override when guest acks. */
> diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
> index cdf405b0f8..d694a25fe2 100644
> --- a/hw/scsi/vhost-scsi.c
> +++ b/hw/scsi/vhost-scsi.c
> @@ -276,7 +276,6 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp)
>      vqs = g_new0(struct vhost_virtqueue, vsc->dev.nvqs);
>      vsc->dev.vqs = vqs;
>      vsc->dev.vq_index = 0;
> -    vsc->dev.backend_features = 0;
>
>      ret = vhost_dev_init(&vsc->dev, (void *)(uintptr_t)vhostfd,
>                           VHOST_BACKEND_TYPE_KERNEL, 0, errp);
> diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
> index 25f2d894e7..0c80a271d8 100644
> --- a/hw/scsi/vhost-user-scsi.c
> +++ b/hw/scsi/vhost-user-scsi.c
> @@ -159,7 +159,6 @@ static int vhost_user_scsi_connect(DeviceState *dev, Error **errp)
>      vsc->dev.nvqs = VIRTIO_SCSI_VQ_NUM_FIXED + vs->conf.num_queues;
>      vsc->dev.vqs = s->vhost_vqs;
>      vsc->dev.vq_index = 0;
> -    vsc->dev.backend_features = 0;
>
>      ret = vhost_dev_init(&vsc->dev, &s->vhost_user, VHOST_BACKEND_TYPE_USER, 0,
>                           errp);
> diff --git a/hw/virtio/vdpa-dev.c b/hw/virtio/vdpa-dev.c
> index 4a7b970976..efd9f68420 100644
> --- a/hw/virtio/vdpa-dev.c
> +++ b/hw/virtio/vdpa-dev.c
> @@ -104,7 +104,6 @@ static void vhost_vdpa_device_realize(DeviceState *dev, Error **errp)
>      v->dev.vqs = vqs;
>      v->dev.vq_index = 0;
>      v->dev.vq_index_end = v->dev.nvqs;
> -    v->dev.backend_features = 0;
>      v->started = false;
>
>      ret = vhost_vdpa_get_iova_range(v->vhostfd, &iova_range);
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index f367ce06ce..b80174f489 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -1448,14 +1448,15 @@ static int vhost_user_set_features(struct vhost_dev *dev,
>      int ret;
>
>      /*
> -     * We need to include any extra backend only feature bits that
> -     * might be needed by our device. Currently this includes the
> -     * VHOST_USER_F_PROTOCOL_FEATURES bit for enabling protocol
> -     * features.
> +     * Don't lose VHOST_USER_F_PROTOCOL_FEATURES, which is vhost-user
> +     * specific.
>       */
> -    ret = vhost_user_set_u64(dev, VHOST_USER_SET_FEATURES,
> -                              features | dev->backend_features,
> -                              log_enabled);
> +    if (virtio_has_feature(dev->features, VHOST_USER_F_PROTOCOL_FEATURES)) {
> +        features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
> +    }
> +
> +    ret = vhost_user_set_u64(dev, VHOST_USER_SET_FEATURES, features,
> +                             log_enabled);
>
>      if (virtio_has_feature(dev->protocol_features,
>                             VHOST_USER_PROTOCOL_F_STATUS)) {
> @@ -2189,8 +2190,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
>              (dev->config_ops && dev->config_ops->vhost_dev_config_notifier);
>          uint64_t protocol_features;
>
> -        dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
> -
>          err = vhost_user_get_u64(dev, VHOST_USER_GET_PROTOCOL_FEATURES,
>                                   &protocol_features);
>          if (err < 0) {
> diff --git a/hw/virtio/virtio-hmp-cmds.c b/hw/virtio/virtio-hmp-cmds.c
> index 1daae482d3..4bf9a3109d 100644
> --- a/hw/virtio/virtio-hmp-cmds.c
> +++ b/hw/virtio/virtio-hmp-cmds.c
> @@ -176,8 +176,6 @@ void hmp_virtio_status(Monitor *mon, const QDict *qdict)
>          hmp_virtio_dump_features(mon, s->vhost_dev->features);
>          monitor_printf(mon, "    Acked features:\n");
>          hmp_virtio_dump_features(mon, s->vhost_dev->acked_features);
> -        monitor_printf(mon, "    Backend features:\n");
> -        hmp_virtio_dump_features(mon, s->vhost_dev->backend_features);
>          monitor_printf(mon, "    Protocol features:\n");
>          hmp_virtio_dump_protocols(mon, s->vhost_dev->protocol_features);
>      }
> diff --git a/hw/virtio/virtio-qmp.c b/hw/virtio/virtio-qmp.c
> index b338344c6c..dd1a38e792 100644
> --- a/hw/virtio/virtio-qmp.c
> +++ b/hw/virtio/virtio-qmp.c
> @@ -780,8 +780,6 @@ VirtioStatus *qmp_x_query_virtio_status(const char *path, Error **errp)
>                                                   vdev->guest_features_ex);
>      status->host_features = qmp_decode_features(vdev->device_id,
>                                                  vdev->host_features_ex);
> -    status->backend_features = qmp_decode_features(vdev->device_id,
> -                                                 vdev->backend_features_ex);
>
>      switch (vdev->device_endian) {
>      case VIRTIO_DEVICE_ENDIAN_LITTLE:
> @@ -822,8 +820,6 @@ VirtioStatus *qmp_x_query_virtio_status(const char *path, Error **errp)
>              qmp_decode_features(vdev->device_id, hdev->features_ex);
>          status->vhost_dev->acked_features =
>              qmp_decode_features(vdev->device_id, hdev->acked_features_ex);
> -        status->vhost_dev->backend_features =
> -            qmp_decode_features(vdev->device_id, hdev->backend_features_ex);
>
>          status->vhost_dev->protocol_features =
>              qmp_decode_protocols(hdev->protocol_features);
> diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
> index 1ee639dd7e..3e69e47833 100644
> --- a/include/hw/virtio/vhost.h
> +++ b/include/hw/virtio/vhost.h
> @@ -100,16 +100,9 @@ struct vhost_dev {
>       *
>       * @features: available features provided by the backend
>       * @acked_features: final negotiated features with front-end driver
> -     *
> -     * @backend_features: this is used in a couple of places to either
> -     * store VHOST_USER_F_PROTOCOL_FEATURES to apply to
> -     * VHOST_USER_SET_FEATURES or VHOST_NET_F_VIRTIO_NET_HDR. Its
> -     * future use should be discouraged and the variable retired as
> -     * its easy to confuse with the VirtIO backend_features.
>       */
>      VIRTIO_DECLARE_FEATURES(features);
>      VIRTIO_DECLARE_FEATURES(acked_features);
> -    VIRTIO_DECLARE_FEATURES(backend_features);
>
>      /**
>       * @protocol_features: is the vhost-user only feature set by
> diff --git a/qapi/virtio.json b/qapi/virtio.json
> index 05295ab665..b995a5bb6d 100644
> --- a/qapi/virtio.json
> +++ b/qapi/virtio.json
> @@ -85,8 +85,6 @@
>  #
>  # @acked-features: vhost_dev acked_features
>  #
> -# @backend-features: vhost_dev backend_features
> -#
>  # @protocol-features: vhost_dev protocol_features
>  #
>  # @max-queues: vhost_dev max_queues
> @@ -106,7 +104,6 @@
>              'vq-index': 'int',
>              'features': 'VirtioDeviceFeatures',
>              'acked-features': 'VirtioDeviceFeatures',
> -            'backend-features': 'VirtioDeviceFeatures',
>              'protocol-features': 'VhostDeviceProtocols',
>              'max-queues': 'uint64',
>              'backend-cap': 'uint64',
> --
> 2.48.1
>
Re: [PATCH v2 02/23] vhost: drop backend_features field
Posted by Markus Armbruster 1 month ago
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:

> This field is mostly unused and sometimes confusing (we even have
> a TODO-like comment to drop it). Let's finally do.
>
> The field is used to held VHOST_USER_F_PROTOCOL_FEATURES for vhost-user
> and/or VHOST_NET_F_VIRTIO_NET_HDR for vhost-net (which may be
> vhost-user-net). But we can simply recalculate these two flags in place
> from hdev->features, and from net-client for VHOST_NET_F_VIRTIO_NET_HDR.
>
> Note: removing field from x-query-virtio-status result is incompatible
> change. We can do it because the command is unstable.
>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

You improved the commit message like I suggested, but lost my
Acked-by: Markus Armbruster <armbru@redhat.com>
:)

[...]

> diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
> index 1ee639dd7e..3e69e47833 100644
> --- a/include/hw/virtio/vhost.h
> +++ b/include/hw/virtio/vhost.h
> @@ -100,16 +100,9 @@ struct vhost_dev {
>       *
>       * @features: available features provided by the backend
>       * @acked_features: final negotiated features with front-end driver
> -     *
> -     * @backend_features: this is used in a couple of places to either
> -     * store VHOST_USER_F_PROTOCOL_FEATURES to apply to
> -     * VHOST_USER_SET_FEATURES or VHOST_NET_F_VIRTIO_NET_HDR. Its
> -     * future use should be discouraged and the variable retired as
> -     * its easy to confuse with the VirtIO backend_features.
>       */

I guess this is the TODO-like comment mentioned in the commit message.

>      VIRTIO_DECLARE_FEATURES(features);
>      VIRTIO_DECLARE_FEATURES(acked_features);
> -    VIRTIO_DECLARE_FEATURES(backend_features);
>  
>      /**
>       * @protocol_features: is the vhost-user only feature set by
> diff --git a/qapi/virtio.json b/qapi/virtio.json
> index 05295ab665..b995a5bb6d 100644
> --- a/qapi/virtio.json
> +++ b/qapi/virtio.json
> @@ -85,8 +85,6 @@
>  #
>  # @acked-features: vhost_dev acked_features
>  #
> -# @backend-features: vhost_dev backend_features
> -#
>  # @protocol-features: vhost_dev protocol_features
>  #
>  # @max-queues: vhost_dev max_queues
> @@ -106,7 +104,6 @@
>              'vq-index': 'int',
>              'features': 'VirtioDeviceFeatures',
>              'acked-features': 'VirtioDeviceFeatures',
> -            'backend-features': 'VirtioDeviceFeatures',
>              'protocol-features': 'VhostDeviceProtocols',
>              'max-queues': 'uint64',
>              'backend-cap': 'uint64',
Re: [PATCH v2 02/23] vhost: drop backend_features field
Posted by Vladimir Sementsov-Ogievskiy 1 month ago
On 13.10.25 07:42, Markus Armbruster wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
> 
>> This field is mostly unused and sometimes confusing (we even have
>> a TODO-like comment to drop it). Let's finally do.
>>
>> The field is used to held VHOST_USER_F_PROTOCOL_FEATURES for vhost-user
>> and/or VHOST_NET_F_VIRTIO_NET_HDR for vhost-net (which may be
>> vhost-user-net). But we can simply recalculate these two flags in place
>> from hdev->features, and from net-client for VHOST_NET_F_VIRTIO_NET_HDR.
>>
>> Note: removing field from x-query-virtio-status result is incompatible
>> change. We can do it because the command is unstable.
>>
>> Cc: qemu-stable@nongnu.org
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> 
> You improved the commit message like I suggested, but lost my

I decided that commit is changed enough, so that it's not safe to save any r-b/a-b marks.

> Acked-by: Markus Armbruster <armbru@redhat.com>
> :)

Thanks!

> 
> [...]
> 
>> diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
>> index 1ee639dd7e..3e69e47833 100644
>> --- a/include/hw/virtio/vhost.h
>> +++ b/include/hw/virtio/vhost.h
>> @@ -100,16 +100,9 @@ struct vhost_dev {
>>        *
>>        * @features: available features provided by the backend
>>        * @acked_features: final negotiated features with front-end driver
>> -     *
>> -     * @backend_features: this is used in a couple of places to either
>> -     * store VHOST_USER_F_PROTOCOL_FEATURES to apply to
>> -     * VHOST_USER_SET_FEATURES or VHOST_NET_F_VIRTIO_NET_HDR. Its
>> -     * future use should be discouraged and the variable retired as
>> -     * its easy to confuse with the VirtIO backend_features.
>>        */
> 
> I guess this is the TODO-like comment mentioned in the commit message.

yes)

> 
>>       VIRTIO_DECLARE_FEATURES(features);
>>       VIRTIO_DECLARE_FEATURES(acked_features);
>> -    VIRTIO_DECLARE_FEATURES(backend_features);
>>   
>>       /**
>>        * @protocol_features: is the vhost-user only feature set by
>> diff --git a/qapi/virtio.json b/qapi/virtio.json
>> index 05295ab665..b995a5bb6d 100644
>> --- a/qapi/virtio.json
>> +++ b/qapi/virtio.json
>> @@ -85,8 +85,6 @@
>>   #
>>   # @acked-features: vhost_dev acked_features
>>   #
>> -# @backend-features: vhost_dev backend_features
>> -#
>>   # @protocol-features: vhost_dev protocol_features
>>   #
>>   # @max-queues: vhost_dev max_queues
>> @@ -106,7 +104,6 @@
>>               'vq-index': 'int',
>>               'features': 'VirtioDeviceFeatures',
>>               'acked-features': 'VirtioDeviceFeatures',
>> -            'backend-features': 'VirtioDeviceFeatures',
>>               'protocol-features': 'VhostDeviceProtocols',
>>               'max-queues': 'uint64',
>>               'backend-cap': 'uint64',
> 


-- 
Best regards,
Vladimir