Re: [PATCH v3 5/5] vhost-user: add skip_drain param to GET_VRING_BASE

I’m not sure I like using the num field in vhost_vring_state to set 
magic values which affect protocol behavior for GET_VRING_BASE. It feels 
like a hack to me. I would think the proper solution if we want to 
support migration from new to old would be to have new use a different 
new message entirely. Can we do that?

On 3/30/26 11:52 AM, Alexandr Moshkov wrote:
> In case of migration of QEMU from the new version (where the
> inllight-migration parameter is present), to the old one (where it is

nit: spelling inllight-migration

> absent) there is no way to disable this feature on the backend during
> runtime.
> 
> This commit slightly changes the semantics of the protocol feature
> VHOST_USER_PROTOCOL_F_GET_VRING_BASE_INFLIGHT. Enabling this feature
> adds a new parameter for GET_VRING_BASE, which allows to control the
> drain in-flight requests on the backend.
> Thus, QEMU will be able to turn this feature on GET_VRING_BASE off and
> on anytime.
> 
> In vhost-user-blk use inflight_migration param to enable skip_drain to
> suspend in-flight I/O requests, and then migrate them throught inflight
> subsection.
> 
> Also now QEMU will always try to setup
> VHOST_USER_PROTOCOL_F_GET_VRING_BASE_INFLIGHT protocol featrue with
> backend. This will allow to use skip_drain parameter on GET_VRING_BASE
> message.
> 
> Reviewed-by: Vladimir Sementsov-Ogievskiy <[email protected]>
> Signed-off-by: Alexandr Moshkov <[email protected]>
> ---
>   docs/interop/vhost-user.rst    |  6 ++----
>   hw/block/vhost-user-blk.c      | 30 ++++++++++++++++++++++++------
>   hw/virtio/vhost-user.c         |  3 +--
>   hw/virtio/vhost.c              |  1 +
>   include/hw/virtio/vhost-user.h |  1 -
>   5 files changed, 28 insertions(+), 13 deletions(-)
> 
> diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
> index bfa75ff9a3..63efc87264 100644
> --- a/docs/interop/vhost-user.rst
> +++ b/docs/interop/vhost-user.rst
> @@ -1255,14 +1255,12 @@ Front-end message types
>     *suspended*, see :ref:`Suspended device state
>     <suspended_device_state>`.
>   
> -  The request payload's *num* field is currently reserved and must be
> -  set to 0.
> -
>     By default, the back-end must complete all inflight I/O requests for the
>     specified vring before stopping it.
>   
>     If the ``VHOST_USER_PROTOCOL_F_GET_VRING_BASE_INFLIGHT`` protocol
> -  feature has been negotiated, the back-end may suspend in-flight I/O
> +  feature has been negotiated, using request payload's *num* field,
> +  when *num* is set to 1, QEMU can tell the back-end to suspend in-flight I/O
>     requests and record them as described in :ref:`Inflight I/O tracking
>     <inflight_io_tracking>` instead of completing them before stopping the vring.
>     How to suspend an in-flight request depends on the implementation of the back-end
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> index a3ecd83f54..522e9d34b5 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -134,10 +134,7 @@ static bool vhost_user_blk_inflight_needed(void *opaque)
>   {
>       struct VHostUserBlk *s = opaque;
>   
> -    bool inflight_migration = virtio_has_feature(s->dev.protocol_features,
> -                               VHOST_USER_PROTOCOL_F_GET_VRING_BASE_INFLIGHT);
> -
> -    return inflight_migration;
> +    return s->inflight_migration;
>   }
>   
>   
> @@ -232,11 +229,14 @@ static int vhost_user_blk_stop(VirtIODevice *vdev)
>           return 0;
>       }
>   
> +    bool skip_drain = vhost_user_blk_inflight_needed(s) &&

nit: declarations on top

> +                      runstate_check(RUN_STATE_FINISH_MIGRATE);
> +
>       force_stop = s->skip_get_vring_base_on_force_shutdown &&
>                    qemu_force_shutdown_requested();
>   
>       ret = force_stop ? vhost_dev_force_stop(&s->dev, vdev, true) :
> -                       vhost_dev_stop(&s->dev, vdev, true, false);
> +                       vhost_dev_stop(&s->dev, vdev, true, skip_drain);
>   
>       if (k->set_guest_notifiers(qbus->parent, s->dev.nvqs, false) < 0) {
>           error_report("vhost guest notifier cleanup failed: %d", ret);
> @@ -364,7 +364,6 @@ static int vhost_user_blk_connect(DeviceState *dev, Error **errp)
>       vhost_dev_set_config_notifier(&s->dev, &blk_ops);
>   
>       s->vhost_user.supports_config = true;
> -    s->vhost_user.supports_inflight_migration = s->inflight_migration;
>       ret = vhost_dev_init(&s->dev, &s->vhost_user, VHOST_BACKEND_TYPE_USER, 0,
>                            errp);
>       if (ret < 0) {
> @@ -580,10 +579,29 @@ static struct vhost_dev *vhost_user_blk_get_vhost(VirtIODevice *vdev)
>       return &s->dev;
>   }
>   
> +static bool vhost_user_blk_pre_save(void *opaque, Error **errp)
> +{
> +    VHostUserBlk *s = VHOST_USER_BLK(opaque);
> +
> +    bool inflight_migration_enabled = virtio_has_feature(
> +        s->dev.protocol_features,
> +        VHOST_USER_PROTOCOL_F_GET_VRING_BASE_INFLIGHT);
> +    if (vhost_user_blk_inflight_needed(s) && !inflight_migration_enabled) {
> +        error_setg(errp, "can't migrate vhost-user-blk device: "
> +                         "backend doesn't support "
> +                         "VHOST_USER_PROTOCOL_F_GET_VRING_BASE_INFLIGHT "
> +                         "protocol feature");
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
>   static const VMStateDescription vmstate_vhost_user_blk_inflight = {
>       .name = "vhost-user-blk/inflight",
>       .version_id = 1,
>       .needed = vhost_user_blk_inflight_needed,
> +    .pre_save_errp = vhost_user_blk_pre_save,
>       .fields = (const VMStateField[]) {
>           VMSTATE_VHOST_INFLIGHT_REGION(inflight, VHostUserBlk),
>           VMSTATE_END_OF_LIST()
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index bb8f8eab77..ed95ec7523 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -2225,8 +2225,7 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
>               }
>           }
>   
> -        if (!u->user->supports_inflight_migration ||
> -            !virtio_has_feature(protocol_features,
> +        if (!virtio_has_feature(protocol_features,
>                                   VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD)) {
>               protocol_features &= ~(1ULL <<
>                                  VHOST_USER_PROTOCOL_F_GET_VRING_BASE_INFLIGHT);
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index ac4d6fca73..54e4ce2660 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1398,6 +1398,7 @@ static int do_vhost_virtqueue_stop(struct vhost_dev *dev,
>       int vhost_vq_index = dev->vhost_ops->vhost_get_vq_index(dev, idx);
>       struct vhost_vring_state state = {
>           .index = vhost_vq_index,
> +        .num = skip_drain,
>       };
>       int r = 0;
>   
> diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h
> index 53fe996686..c95bad5ddc 100644
> --- a/include/hw/virtio/vhost-user.h
> +++ b/include/hw/virtio/vhost-user.h
> @@ -69,7 +69,6 @@ typedef struct VhostUserState {
>       GPtrArray *notifiers;
>       int memory_slots;
>       bool supports_config;
> -    bool supports_inflight_migration;
>   } VhostUserState;
>   
>   /**