Gentle ping :)

On 4/27/26 16:15, Alexandr Moshkov wrote:

On 4/24/26 17:52, Raphael Norwitz wrote:


On 4/24/26 4:57 AM, Alexandr Moshkov wrote:
Thanks for reply!

On 4/23/26 19:15, Raphael Norwitz wrote:


On 4/16/26 5:26 AM, Alexandr Moshkov wrote:
Greetings! Thanks for reply!


On 4/15/26 20:21, Raphael Norwitz wrote:
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?

I think we can, but I thought at first that this will be almost a complete copy of GET_VRING_BASE message, with the exception of waiting for drain of requests, so I choose to expand existing message.


I think a new message would be cleaner. Anyone else have thoughts?

If this is not an appropriate approach, is it better to make a new message like GET_VRING_BASE or a separate message used together with default GET_VRING_BASE message (for example, message for setting some kind of status on the server)? What should I name this new message?


Maybe GET_VRING_BASE_SKIP_DRAIN? How would a separate message used together with default GET_VRING_BASE message work?

I was thinking about a message something like SET_VRING_ENABLE - for example SET_SKIP_DRAIN_ENABLE, that would enable/disable some state (skip_drain) in the backend.

I'd need to see the flow in more detail but sounds promising.

On migration vhost-user-blk firstly send SET_SKIP_DRAIN_ENABLE with num = 1 message if `inflight-migration` device parameter and VHOST_USER_PROTOCOL_F_GET_VRING_BASE_INFLIGHT enabled. Then send GET_VRING_BASE and continue work as usual.

On SET_SKIP_DRAIN_ENABLE backend sets inner state (skip_drain) so when GET_VRING_BASE is called and skip_drain is true the drain will skipped.

What do you think?




I also fix other comments, thanks!


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;
    /**




Reply via email to