Reviewed-by: Raphael Norwitz <[email protected]>

On Sat, Oct 11, 2025 at 7:24 PM Vladimir Sementsov-Ogievskiy
<[email protected]> wrote:
>
> We call the handler almost the same way in three places:
>
>  - cryptodev-vhost.c
>  - vhost_net.c
>  - vhost.c
>
> The only difference, is that in vhost.c we don't try to call the handler
> for old vhost-user (when VHOST_USER_F_PROTOCOL_FEATURES is not supported).
>
> cryptodev-vhost and vhost_net code will just fail in this case. Probably
> they were developed only for newer vhost-user. Anyway, it doesn't seem
> correct to rely on this error path, if these devices want to check,
> that they don't communicate to old vhost-user protocol, they should
> do that earlier.
>
> Let's create the common helper, to call .vhost_set_vring_enable and
> use in all three places. For vhost-user let's just always skip
> enable/disable if it's unsupported.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]>
> ---
>  backends/cryptodev-vhost.c |  8 +-------
>  hw/net/vhost_net.c         |  7 +------
>  hw/virtio/vhost-user.c     |  7 ++++++-
>  hw/virtio/vhost.c          | 21 ---------------------
>  include/hw/virtio/vhost.h  |  9 +++++++++
>  5 files changed, 17 insertions(+), 35 deletions(-)
>
> diff --git a/backends/cryptodev-vhost.c b/backends/cryptodev-vhost.c
> index 943680a23a..abdfce33af 100644
> --- a/backends/cryptodev-vhost.c
> +++ b/backends/cryptodev-vhost.c
> @@ -152,7 +152,6 @@ vhost_set_vring_enable(CryptoDevBackendClient *cc,
>  {
>      CryptoDevBackendVhost *crypto =
>                         cryptodev_get_vhost(cc, b, queue);
> -    const VhostOps *vhost_ops;
>
>      cc->vring_enable = enable;
>
> @@ -160,12 +159,7 @@ vhost_set_vring_enable(CryptoDevBackendClient *cc,
>          return 0;
>      }
>
> -    vhost_ops = crypto->dev.vhost_ops;
> -    if (vhost_ops->vhost_set_vring_enable) {
> -        return vhost_ops->vhost_set_vring_enable(&crypto->dev, enable);
> -    }
> -
> -    return 0;
> +    return vhost_dev_set_vring_enable(&crypto->dev, enable);
>  }
>
>  int cryptodev_vhost_start(VirtIODevice *dev, int total_queues)
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index a8ee18a912..25e9f1fd24 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -587,7 +587,6 @@ VHostNetState *get_vhost_net(NetClientState *nc)
>  int vhost_net_set_vring_enable(NetClientState *nc, int enable)
>  {
>      VHostNetState *net = get_vhost_net(nc);
> -    const VhostOps *vhost_ops = net->dev.vhost_ops;
>
>      /*
>       * vhost-vdpa network devices need to enable dataplane virtqueues after
> @@ -601,11 +600,7 @@ int vhost_net_set_vring_enable(NetClientState *nc, int 
> enable)
>
>      nc->vring_enable = enable;
>
> -    if (vhost_ops && vhost_ops->vhost_set_vring_enable) {
> -        return vhost_ops->vhost_set_vring_enable(&net->dev, enable);
> -    }
> -
> -    return 0;
> +    return vhost_dev_set_vring_enable(&net->dev, enable);
>  }
>
>  int vhost_net_set_mtu(struct vhost_net *net, uint16_t mtu)
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 36c9c2e04d..f367ce06ce 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -1235,7 +1235,12 @@ static int vhost_user_set_vring_enable(struct 
> vhost_dev *dev, int enable)
>      int i;
>
>      if (!virtio_has_feature(dev->features, VHOST_USER_F_PROTOCOL_FEATURES)) {
> -        return -EINVAL;
> +        /*
> +         * For vhost-user devices, if VHOST_USER_F_PROTOCOL_FEATURES has not
> +         * been negotiated, the rings start directly in the enabled state,
> +         * and can't be disabled.
> +         */
> +        return 0;
>      }
>
>      for (i = 0; i < dev->nvqs; ++i) {
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 266a11514a..414a48a218 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -2013,27 +2013,6 @@ int vhost_dev_get_inflight(struct vhost_dev *dev, 
> uint16_t queue_size,
>      return 0;
>  }
>
> -static int vhost_dev_set_vring_enable(struct vhost_dev *hdev, int enable)
> -{
> -    if (!hdev->vhost_ops->vhost_set_vring_enable) {
> -        return 0;
> -    }
> -
> -    /*
> -     * For vhost-user devices, if VHOST_USER_F_PROTOCOL_FEATURES has not
> -     * been negotiated, the rings start directly in the enabled state, and
> -     * .vhost_set_vring_enable callback will fail since
> -     * VHOST_USER_SET_VRING_ENABLE is not supported.
> -     */
> -    if (hdev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER &&
> -        !virtio_has_feature(hdev->backend_features,
> -                            VHOST_USER_F_PROTOCOL_FEATURES)) {
> -        return 0;
> -    }
> -
> -    return hdev->vhost_ops->vhost_set_vring_enable(hdev, enable);
> -}
> -
>  /*
>   * Host notifiers must be enabled at this point.
>   *
> diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
> index 08bbb4dfe9..1ee639dd7e 100644
> --- a/include/hw/virtio/vhost.h
> +++ b/include/hw/virtio/vhost.h
> @@ -215,6 +215,15 @@ static inline bool vhost_dev_is_started(struct vhost_dev 
> *hdev)
>      return hdev->started;
>  }
>
> +static inline int vhost_dev_set_vring_enable(struct vhost_dev *hdev, int 
> enable)
> +{
> +    if (!hdev->vhost_ops->vhost_set_vring_enable) {
> +        return 0;
> +    }
> +
> +    return hdev->vhost_ops->vhost_set_vring_enable(hdev, enable);
> +}
> +
>  /**
>   * vhost_dev_start() - start the vhost device
>   * @hdev: common vhost_dev structure
> --
> 2.48.1
>

Reply via email to