[PATCH v3 0/7] migration/cpr: support vhost-vsock devices

[PATCH v3 0/7] migration/cpr: support vhost-vsock devices

[Andrey Drobyshev]

v2 -> v3:

Re-design device ownership hand-off (suggested by Dongli).  Do not rely
on SETUP migration notifiers, as this approach forces us to reorder
generic migration code (see v2 discussion).  Instead:

  * .pre_save() releases ownership on the source (RESET_OWNER) once VM
    is stopped;
  * .realize() is adjusted to defer device ownership acquisition for an
    incoming CPR;
  * .post_load() actually claims the device (VHOST_SET_OWNER);
  * FAILED migration event callback re-aquires the ownership on the
    source after re_save released it.

v2: https://lore.kernel.org/qemu-devel/57c9c9b3-d758-489b-95b8-d16c258b9b0c@virtuozzo.com

Andrey Drobyshev (7):
  vhost: add vhost_reset_owner op
  vhost-vsock: don't reset connections during CPR
  vhost-vsock: fix FD leak in realize()
  vhost-vsock: preserve vhost FD during CPR
  vhost: factor out vhost_dev_init_backend()
  vhost: add vhost_dev_set_owner() / vhost_dev_reset_owner() helpers
  vhost-vsock: hand off device ownership across CPR

 hw/virtio/vhost-kernel.c          |   6 +
 hw/virtio/vhost-vsock.c           | 208 +++++++++++++++++++++++++++---
 hw/virtio/vhost.c                 |  55 ++++++--
 include/hw/virtio/vhost-backend.h |   1 +
 include/hw/virtio/vhost-vsock.h   |   4 +
 include/hw/virtio/vhost.h         |  32 +++++
 6 files changed, 275 insertions(+), 31 deletions(-)

-- 
2.47.1

[PATCH v3 1/7] vhost: add vhost_reset_owner op

[Andrey Drobyshev]

Add a VhostOps callback issuing VHOST_RESET_OWNER, wired up for the kernel
backend.  The CPR (checkpoint-restore) path uses it to release device
ownership on the source so the destination can reclaim it later with
VHOST_SET_OWNER.

Originally-by: Mark Kanda <mark.kanda@oracle.com>
Originally-by: Steve Sistare <steven.sistare@oracle.com>
Originally-by: Ben Chaney <bchaney@akamai.com>
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
 hw/virtio/vhost-kernel.c          | 6 ++++++
 include/hw/virtio/vhost-backend.h | 1 +
 2 files changed, 7 insertions(+)

diff --git a/hw/virtio/vhost-kernel.c b/hw/virtio/vhost-kernel.c
index 3390b48c6f1..55d316b88e9 100644
--- a/hw/virtio/vhost-kernel.c
+++ b/hw/virtio/vhost-kernel.c
@@ -261,6 +261,11 @@ static int vhost_kernel_set_owner(struct vhost_dev *dev)
     return vhost_kernel_call(dev, VHOST_SET_OWNER, NULL);
 }
 
+static int vhost_kernel_reset_owner(struct vhost_dev *dev)
+{
+    return vhost_kernel_call(dev, VHOST_RESET_OWNER, NULL);
+}
+
 static int vhost_kernel_get_vq_index(struct vhost_dev *dev, int idx)
 {
     assert(idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs);
@@ -385,6 +390,7 @@ const VhostOps kernel_ops = {
         .vhost_get_features_ex = vhost_kernel_get_features,
         .vhost_set_backend_cap = vhost_kernel_set_backend_cap,
         .vhost_set_owner = vhost_kernel_set_owner,
+        .vhost_reset_owner = vhost_kernel_reset_owner,
         .vhost_get_vq_index = vhost_kernel_get_vq_index,
         .vhost_vsock_set_guest_cid = vhost_kernel_vsock_set_guest_cid,
         .vhost_vsock_set_running = vhost_kernel_vsock_set_running,
diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
index d878d7b733a..6c949e6a38d 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -204,6 +204,7 @@ typedef struct VhostOps {
     vhost_get_features_op vhost_get_features;
     vhost_set_backend_cap_op vhost_set_backend_cap;
     vhost_set_owner_op vhost_set_owner;
+    vhost_set_owner_op vhost_reset_owner;
     vhost_reset_device_op vhost_reset_device;
     vhost_get_vq_index_op vhost_get_vq_index;
     vhost_set_vring_enable_op vhost_set_vring_enable;
-- 
2.47.1

[PATCH v3 2/7] vhost-vsock: don't reset connections during CPR

[Andrey Drobyshev]

Migration target with a vhost-vsock device in the .post_load() hook issues
VIRTIO_VSOCK_EVENT_TRANSPORT_RESET event to the guest.  This results into
the guest tearing down all its vsock connections.  That reset exists
because after a normal migration the cid may change.

However it's not true for CPR-style migration.  In this case cid remains
the same, and we want the connections to persist.  Thus, let's customize
the .post_load() hook to skip the common transport reset part in this case.

Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
 hw/virtio/vhost-vsock.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
index da244eb1657..cdc58d58790 100644
--- a/hw/virtio/vhost-vsock.c
+++ b/hw/virtio/vhost-vsock.c
@@ -20,6 +20,7 @@
 #include "hw/core/qdev-properties.h"
 #include "hw/virtio/vhost-vsock.h"
 #include "monitor/monitor.h"
+#include "migration/cpr.h"
 
 static void vhost_vsock_get_config(VirtIODevice *vdev, uint8_t *config)
 {
@@ -108,6 +109,20 @@ static uint64_t vhost_vsock_get_features(VirtIODevice *vdev,
     return vhost_vsock_common_get_features(vdev, requested_features, errp);
 }
 
+static int vhost_vsock_post_load(void *opaque, int version_id)
+{
+    /*
+     * Only reset vsock connections for non-CPR migration.  For CPR the
+     * guest cid is unchanged, and the cid-change reset would otherwise
+     * tear the vsock connections down.
+     */
+    if (cpr_is_incoming()) {
+        return 0;
+    }
+
+    return vhost_vsock_common_post_load(opaque, version_id);
+}
+
 static const VMStateDescription vmstate_virtio_vhost_vsock = {
     .name = "virtio-vhost_vsock",
     .minimum_version_id = VHOST_VSOCK_SAVEVM_VERSION,
@@ -117,7 +132,7 @@ static const VMStateDescription vmstate_virtio_vhost_vsock = {
         VMSTATE_END_OF_LIST()
     },
     .pre_save = vhost_vsock_common_pre_save,
-    .post_load = vhost_vsock_common_post_load,
+    .post_load = vhost_vsock_post_load,
 };
 
 static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
-- 
2.47.1

[PATCH v3 3/7] vhost-vsock: fix FD leak in realize()

[Andrey Drobyshev]

In vhost_vsock_device_realize(), after unsuccessful call to
qemu_set_blocking(vhostfd, false), vhostfd gets leaked.  Let's close it
explicitly in this case.  CPR-saved FDs don't get automatically closed,
thus it is safe both for vhostfd obtained from cpr_find_fd() and from
open(/dev/vhost-vsock).

Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
 hw/virtio/vhost-vsock.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
index cdc58d58790..fd7ffa88990 100644
--- a/hw/virtio/vhost-vsock.c
+++ b/hw/virtio/vhost-vsock.c
@@ -161,20 +161,17 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
             error_prepend(errp, "vhost-vsock: unable to parse vhostfd: ");
             return;
         }
-
-        if (!qemu_set_blocking(vhostfd, false, errp)) {
-            return;
-        }
     } else {
         vhostfd = open("/dev/vhost-vsock", O_RDWR);
         if (vhostfd < 0) {
             error_setg_file_open(errp, errno, "/dev/vhost-vsock");
             return;
         }
+    }
 
-        if (!qemu_set_blocking(vhostfd, false, errp)) {
-            return;
-        }
+    if (!qemu_set_blocking(vhostfd, false, errp)) {
+        close(vhostfd);
+        return;
     }
 
     vhost_vsock_common_realize(vdev);
-- 
2.47.1

[PATCH v3 4/7] vhost-vsock: preserve vhost FD during CPR

[Andrey Drobyshev]

During CPR (checkpoint-restore) migration the guest keeps running on the
same host, so instead of reopening /dev/vhost-vsock on the destination,
we should reuse the FD from the source.  The FD is saved in the CPR
namespace (hash table) with cpr_save_fd() and then reclaimed on the
target via cpr_find_fd().

Since the key in CPR hash table is device ID, CPR needs a unique ID.
Rather than make it mandatory for every vhost-vsock device, we add CPR
migration blocker which only fires once we attempt CPR with ID-less
vhost-vsock.

vhost_dev_init() (and thus VHOST_SET_OWNER) still runs in realize() here.
Deferring the ownership handoff to pre_save/post_load is done in a
following patch.

Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
 hw/virtio/vhost-vsock.c         | 50 ++++++++++++++++++++++++++++++---
 include/hw/virtio/vhost-vsock.h |  1 +
 2 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
index fd7ffa88990..7eacb608d07 100644
--- a/hw/virtio/vhost-vsock.c
+++ b/hw/virtio/vhost-vsock.c
@@ -21,6 +21,8 @@
 #include "hw/virtio/vhost-vsock.h"
 #include "monitor/monitor.h"
 #include "migration/cpr.h"
+#include "migration/blocker.h"
+#include "migration/misc.h"
 
 static void vhost_vsock_get_config(VirtIODevice *vdev, uint8_t *config)
 {
@@ -141,6 +143,7 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
     VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(dev);
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VHostVSock *vsock = VHOST_VSOCK(dev);
+    DeviceState *proxy = qdev_get_parent_bus(DEVICE(vsock))->parent;
     int vhostfd;
     int ret;
 
@@ -155,23 +158,49 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
         return;
     }
 
-    if (vsock->conf.vhostfd) {
+    /*
+     * Having a unique ID is mandatory for FD preservation during CPR
+     * migration, thus we add migration blockers for CPR modes.
+     */
+    if (!proxy->id) {
+        error_setg(&vsock->migration_blocker,
+                   "vhost-vsock: device ID is required for CPR migration");
+        if (migrate_add_blocker_modes(&vsock->migration_blocker,
+                                      BIT(MIG_MODE_CPR_TRANSFER) |
+                                      BIT(MIG_MODE_CPR_EXEC), errp) < 0) {
+            return;
+        }
+    }
+
+    if (cpr_is_incoming()) {
+        /* Reuse the fd handed over from the source QEMU. */
+        if (!proxy->id) {
+            error_setg(errp, "vhost-vsock: device ID is required for "
+                       "CPR migration");
+            goto err_blocker;
+        }
+        vhostfd = cpr_find_fd(proxy->id, 0);
+        if (vhostfd < 0) {
+            error_setg(errp, "vhost-vsock: could not find restored vhost FD");
+            goto err_blocker;
+        }
+    } else if (vsock->conf.vhostfd) {
         vhostfd = monitor_fd_param(monitor_cur(), vsock->conf.vhostfd, errp);
         if (vhostfd == -1) {
             error_prepend(errp, "vhost-vsock: unable to parse vhostfd: ");
-            return;
+            goto err_blocker;
         }
     } else {
         vhostfd = open("/dev/vhost-vsock", O_RDWR);
         if (vhostfd < 0) {
             error_setg_file_open(errp, errno, "/dev/vhost-vsock");
-            return;
+            goto err_blocker;
         }
     }
 
     if (!qemu_set_blocking(vhostfd, false, errp)) {
         close(vhostfd);
-        return;
+        goto err_blocker;
     }
 
     vhost_vsock_common_realize(vdev);
@@ -192,6 +221,11 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
         goto err_vhost_dev;
     }
 
+    /* Register the fd for a future CPR after a fully successful realize */
+    if (proxy->id) {
+        cpr_save_fd(proxy->id, 0, vhostfd);
+    }
+
     return;
 
 err_vhost_dev:
@@ -199,16 +233,24 @@ err_vhost_dev:
     vhost_dev_cleanup(&vvc->vhost_dev);
 err_virtio:
     vhost_vsock_common_unrealize(vdev);
+err_blocker:
+    migrate_del_blocker(&vsock->migration_blocker);
 }
 
 static void vhost_vsock_device_unrealize(DeviceState *dev)
 {
     VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(dev);
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VHostVSock *vsock = VHOST_VSOCK(dev);
+    DeviceState *proxy = qdev_get_parent_bus(dev)->parent;
 
     /* This will stop vhost backend if appropriate. */
     vhost_vsock_set_status(vdev, 0);
 
+    if (proxy->id) {
+        cpr_delete_fd(proxy->id, 0);
+    }
+    migrate_del_blocker(&vsock->migration_blocker);
     vhost_dev_cleanup(&vvc->vhost_dev);
     vhost_vsock_common_unrealize(vdev);
 }
diff --git a/include/hw/virtio/vhost-vsock.h b/include/hw/virtio/vhost-vsock.h
index 84f4e727c70..5ebc63afc5a 100644
--- a/include/hw/virtio/vhost-vsock.h
+++ b/include/hw/virtio/vhost-vsock.h
@@ -29,6 +29,7 @@ struct VHostVSock {
     /*< private >*/
     VHostVSockCommon parent;
     VHostVSockConf conf;
+    Error *migration_blocker;   /* set when the device has no ID */
 
     /*< public >*/
 };
-- 
2.47.1

[PATCH v3 5/7] vhost: factor out vhost_dev_init_backend()

[Andrey Drobyshev]

Split the first part of vhost_dev_init(): selecting the backend, calling
its .vhost_init() and reading the supported features - into a new
vhost_dev_init_backend() helper, and call it from vhost_dev_init().

This is in preparation for CPR restore of vhost-vsock, which needs to learn
the backend's features at realize time to negotiate them when loading the
incoming virtio state, but also must defer taking ownership of the device
to post_load.  vhost_dev_init_backend() does exactly the pre-ownership part.

As a result VHOST_SET_OWNER now follows the feature query rather than
precedes it.  This should be safe, as no backend requires ownership before
VHOST_GET_FEATURES - the kernel and vdpa backends do not check ownership
for it, and vhost-user already does query features from its .vhost_init()
before set_owner().

Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
 hw/virtio/vhost.c         | 33 +++++++++++++++++++++++----------
 include/hw/virtio/vhost.h | 19 +++++++++++++++++++
 2 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index af41841b529..f11588cc51a 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1667,6 +1667,28 @@ static int vhost_dev_init_features(struct vhost_dev *hdev)
     return r;
 }
 
+int vhost_dev_init_backend(struct vhost_dev *hdev, void *opaque,
+                           VhostBackendType backend_type, Error **errp)
+{
+    int r;
+
+    r = vhost_set_backend_type(hdev, backend_type);
+    assert(r >= 0);
+
+    r = hdev->vhost_ops->vhost_init(hdev, opaque, errp);
+    if (r < 0) {
+        return r;
+    }
+
+    r = vhost_dev_init_features(hdev);
+    if (r < 0) {
+        error_setg_errno(errp, -r, "vhost_init_features failed");
+        return r;
+    }
+
+    return 0;
+}
+
 int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
                    VhostBackendType backend_type, uint32_t busyloop_timeout,
                    Error **errp)
@@ -1679,10 +1701,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
     hdev->vdev = NULL;
     hdev->migration_blocker = NULL;
 
-    r = vhost_set_backend_type(hdev, backend_type);
-    assert(r >= 0);
-
-    r = hdev->vhost_ops->vhost_init(hdev, opaque, errp);
+    r = vhost_dev_init_backend(hdev, opaque, backend_type, errp);
     if (r < 0) {
         goto fail;
     }
@@ -1693,12 +1712,6 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
         goto fail;
     }
 
-    r = vhost_dev_init_features(hdev);
-    if (r < 0) {
-        error_setg_errno(errp, -r, "vhost_init_features failed");
-        goto fail;
-    }
-
     limit = hdev->vhost_ops->vhost_memslots_limit(hdev);
     if (limit < MEMORY_DEVICES_SAFE_MAX_MEMSLOTS &&
         memory_devices_memslot_auto_decision_active()) {
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 684bafcaadd..bc81e09663e 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -156,6 +156,25 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
                    VhostBackendType backend_type,
                    uint32_t busyloop_timeout, Error **errp);
 
+/**
+ * vhost_dev_init_backend() - set up the backend and query its features
+ * @hdev: the common vhost_dev structure
+ * @opaque: opaque ptr passed to backend (vhost/vhost-user/vdpa)
+ * @backend_type: type of backend
+ * @errp: error handle
+ *
+ * Select the backend, initialise the backend instance and read its supported
+ * features into @hdev, without issuing VHOST_SET_OWNER, setting up the
+ * virtqueues or registering the memory listener.  This is the part of
+ * vhost_dev_init() that precedes taking ownership; it can be used on its own
+ * so feature negotiation can happen before ownership is acquired (e.g. by CPR
+ * restore).
+ *
+ * Return: 0 on success, non-zero on error while setting errp.
+ */
+int vhost_dev_init_backend(struct vhost_dev *hdev, void *opaque,
+                           VhostBackendType backend_type, Error **errp);
+
 /**
  * vhost_dev_cleanup() - tear down and cleanup vhost interface
  * @hdev: the common vhost_dev structure
-- 
2.47.1

[PATCH v3 6/7] vhost: add vhost_dev_set_owner() / vhost_dev_reset_owner() helpers

[Andrey Drobyshev]

Wrap the set_owner/reset_owner backend ops in dev-level helpers, matching
other vhost_dev_* wrappers, so device code can take or release ownership
without reaching into vhost_ops directly.  vhost_dev_init() now uses
vhost_dev_set_owner(). Both return -ENOSYS if the backend has no such op.

No functional change.  These are used by the following vhost-vsock patch to
hand a device between owners during CPR.

Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
 hw/virtio/vhost.c         | 20 +++++++++++++++++++-
 include/hw/virtio/vhost.h | 13 +++++++++++++
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index f11588cc51a..7ae2abe33cd 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1689,6 +1689,24 @@ int vhost_dev_init_backend(struct vhost_dev *hdev, void *opaque,
     return 0;
 }
 
+int vhost_dev_set_owner(struct vhost_dev *hdev)
+{
+    assert(hdev->vhost_ops);
+    if (!hdev->vhost_ops->vhost_set_owner) {
+        return -ENOSYS;
+    }
+    return hdev->vhost_ops->vhost_set_owner(hdev);
+}
+
+int vhost_dev_reset_owner(struct vhost_dev *hdev)
+{
+    assert(hdev->vhost_ops);
+    if (!hdev->vhost_ops->vhost_reset_owner) {
+        return -ENOSYS;
+    }
+    return hdev->vhost_ops->vhost_reset_owner(hdev);
+}
+
 int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
                    VhostBackendType backend_type, uint32_t busyloop_timeout,
                    Error **errp)
@@ -1706,7 +1724,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
         goto fail;
     }
 
-    r = hdev->vhost_ops->vhost_set_owner(hdev);
+    r = vhost_dev_set_owner(hdev);
     if (r < 0) {
         error_setg_errno(errp, -r, "vhost_set_owner failed");
         goto fail;
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index bc81e09663e..a56a2b66bd5 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -175,6 +175,19 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
 int vhost_dev_init_backend(struct vhost_dev *hdev, void *opaque,
                            VhostBackendType backend_type, Error **errp);
 
+/**
+ * vhost_dev_set_owner() / vhost_dev_reset_owner() - take / release ownership
+ * @hdev: the common vhost_dev structure
+ *
+ * Take (VHOST_SET_OWNER) or release (VHOST_RESET_OWNER) ownership of a
+ * device that has already been set up.  Used to hand a device over during
+ * CPR.  Returns -ENOSYS if the backend has no such op.
+ *
+ * Return: 0 on success, negative errno on failure.
+ */
+int vhost_dev_set_owner(struct vhost_dev *hdev);
+int vhost_dev_reset_owner(struct vhost_dev *hdev);
+
 /**
  * vhost_dev_cleanup() - tear down and cleanup vhost interface
  * @hdev: the common vhost_dev structure
-- 
2.47.1

[PATCH v3 7/7] vhost-vsock: hand off device ownership across CPR

[Andrey Drobyshev]

The previous patches reuse the source vhost FD on the destination,
however both source and targe still call vhost_dev_init() (VHOST_SET_OWNER)
in realize().  For cpr-transfer the destination realizes while the source
still owns the shared FD, so its SET_OWNER would fail - ownership has to be
handed over explicitly.

Do this through the device's CPR vmstate hooks.  Namely, release device
ownership in pre_save, reclaim in post_load, re-acquire on failure:

  - .pre_save() releases ownership on the source (VHOST_RESET_OWNER) once
    the VM is stopped, for the FD-preserving CPR modes (cpr-transfer and
    cpr-exec).

  - .realize(), for an incoming CPR, only sets up the virtio device and
    queries the backend features by calling vhost_dev_init_backend().
    It doesn't take device ownership and doesn't touch the VQs which
    still-running source might use.  The full init is deferred to
    .post_load().

  - .post_load() reclaims it on the destination: the full vhost_dev_init()
    (VHOST_SET_OWNER) on the preserved FD, plus sets the guest cid, before
    the device is started at vm_start.

  - A MIG_EVENT_FAILED notifier re-acquires ownership if the migration
    fails after pre_save released it and the source VM is resumed.

Also harden vhost_virtqueue_cleanup() against a NULL vq->dev which can
now happen if an incoming CPR is aborted after realize set up the backend
but before post_load initialised the VQs.

Suggested-by: Dongli Zhang <dongli.zhang@oracle.com>
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
 hw/virtio/vhost-vsock.c         | 147 +++++++++++++++++++++++++++++---
 hw/virtio/vhost.c               |   2 +-
 include/hw/virtio/vhost-vsock.h |   3 +
 3 files changed, 140 insertions(+), 12 deletions(-)

diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
index 7eacb608d07..b02b3f9cc03 100644
--- a/hw/virtio/vhost-vsock.c
+++ b/hw/virtio/vhost-vsock.c
@@ -76,6 +76,17 @@ static int vhost_vsock_set_status(VirtIODevice *vdev, uint8_t status)
     bool should_start = virtio_device_should_start(vdev, status);
     int ret;
 
+    /*
+     * On an incoming CPR the full vhost_dev_init() is deferred to post_load
+     * (realize only ran vhost_dev_init_backend()).  hdev->mem is set only by
+     * the full init, so refuse to start a device whose handoff never
+     * completed rather than dereference a half-initialised vhost_dev.
+     */
+    if (should_start && !vvc->vhost_dev.mem) {
+        error_report("vhost-vsock: refusing to start, device init incomplete");
+        return 0;
+    }
+
     if (vhost_dev_is_started(&vvc->vhost_dev) == should_start) {
         return 0;
     }
@@ -111,18 +122,102 @@ static uint64_t vhost_vsock_get_features(VirtIODevice *vdev,
     return vhost_vsock_common_get_features(vdev, requested_features, errp);
 }
 
+/*
+ * Re-acquire device ownership if a CPR migration that released it (in
+ * vhost_vsock_cpr_pre_save()) failed and the source VM is about to resume.
+ * This runs before vm_start(), so the device is owned again before it is
+ * restarted.
+ */
+static int vhost_vsock_cpr_notifier(NotifierWithReturn *notifier,
+                                    MigrationEvent *e, Error **errp)
+{
+    VHostVSock *vsock = container_of(notifier, VHostVSock, cpr_notifier);
+    VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(vsock);
+    int ret;
+
+    if (e->type == MIG_EVENT_FAILED && vsock->owner_reset) {
+        ret = vhost_dev_set_owner(&vvc->vhost_dev);
+        if (ret < 0) {
+            error_report("vhost-vsock: failed to re-acquire owner: %d", ret);
+        } else {
+            vsock->owner_reset = false;
+        }
+    }
+
+    return 0;
+}
+
+static int vhost_vsock_pre_save(void *opaque)
+{
+    VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(opaque);
+    VHostVSock *vsock = VHOST_VSOCK(opaque);
+    int ret;
+
+    ret = vhost_vsock_common_pre_save(opaque);
+    if (ret) {
+        return ret;
+    }
+
+    /*
+     * Release the device ownership now for CPR migration.  The device is
+     * already stopped at pre_save, and destination reclaims it by calling
+     * VHOST_SET_OWNER in post_load.
+     */
+    if (cpr_incoming_needed(NULL)) {
+        ret = vhost_dev_reset_owner(&vvc->vhost_dev);
+        if (ret < 0) {
+            error_report("vhost-vsock: vhost_reset_owner failed: %d", ret);
+            return ret;
+        }
+        vsock->owner_reset = true;
+    }
+
+    return 0;
+}
+
 static int vhost_vsock_post_load(void *opaque, int version_id)
 {
+    VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(opaque);
+    VirtIODevice *vdev = VIRTIO_DEVICE(opaque);
+    DeviceState *proxy = qdev_get_parent_bus(DEVICE(vdev))->parent;
+    Error *local_err = NULL;
+    int vhostfd, ret;
+
     /*
      * Only reset vsock connections for non-CPR migration.  For CPR the
      * guest cid is unchanged, and the cid-change reset would otherwise
      * tear the vsock connections down.
      */
-    if (cpr_is_incoming()) {
-        return 0;
+    if (!cpr_is_incoming()) {
+        return vhost_vsock_common_post_load(opaque, version_id);
+    }
+
+    /*
+     * CPR restore case.  The source released device ownership in its
+     * pre_save.  Complete the handoff here, before the device is started
+     * at vm_start.  Init vhost device on preserved FD, issue
+     * VHOST_SET_OWNER on it, and restore the guest cid.
+     */
+    vhostfd = cpr_find_fd(proxy->id, 0);
+    if (vhostfd < 0) {
+        error_report("vhost-vsock: could not find restored vhost FD");
+        return -1;
     }
 
-    return vhost_vsock_common_post_load(opaque, version_id);
+    ret = vhost_dev_init(&vvc->vhost_dev, (void *)(uintptr_t)vhostfd,
+                         VHOST_BACKEND_TYPE_KERNEL, 0, &local_err);
+    if (ret < 0) {
+        error_report_err(local_err);
+        return ret;
+    }
+
+    ret = vhost_vsock_set_guest_cid(vdev);
+    if (ret < 0) {
+        error_report("vhost-vsock: unable to set guest cid: %d", ret);
+        return ret;
+    }
+
+    return 0;
 }
 
 static const VMStateDescription vmstate_virtio_vhost_vsock = {
@@ -133,7 +228,7 @@ static const VMStateDescription vmstate_virtio_vhost_vsock = {
         VMSTATE_VIRTIO_DEVICE,
         VMSTATE_END_OF_LIST()
     },
-    .pre_save = vhost_vsock_common_pre_save,
+    .pre_save = vhost_vsock_pre_save,
     .post_load = vhost_vsock_post_load,
 };
 
@@ -144,6 +239,7 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VHostVSock *vsock = VHOST_VSOCK(dev);
     DeviceState *proxy = qdev_get_parent_bus(DEVICE(vsock))->parent;
+    bool cpr_incoming = cpr_is_incoming();
     int vhostfd;
     int ret;
 
@@ -172,7 +268,16 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
         }
     }
 
-    if (cpr_is_incoming()) {
+    /*
+     * Re-acquire ownership if a CPR migration releases it (in pre_save) but
+     * then fails.
+     */
+    migration_add_notifier_modes(&vsock->cpr_notifier,
+                                 vhost_vsock_cpr_notifier,
+                                 BIT(MIG_MODE_CPR_TRANSFER) |
+                                 BIT(MIG_MODE_CPR_EXEC));
+
+    if (cpr_incoming) {
         /* Reuse the fd handed over from the source QEMU. */
         if (!proxy->id) {
             error_setg(errp, "vhost-vsock: device ID is required for "
@@ -205,14 +310,32 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
 
     vhost_vsock_common_realize(vdev);
 
-    ret = vhost_dev_init(&vvc->vhost_dev, (void *)(uintptr_t)vhostfd,
-                         VHOST_BACKEND_TYPE_KERNEL, 0, errp);
-    if (ret < 0) {
+    if (!cpr_incoming) {
+        ret = vhost_dev_init(&vvc->vhost_dev, (void *)(uintptr_t)vhostfd,
+                             VHOST_BACKEND_TYPE_KERNEL, 0, errp);
+        if (ret < 0) {
+            /*
+             * vhostfd is closed by vhost_dev_cleanup, which is called
+             * by vhost_dev_init on initialization error.
+             */
+            goto err_virtio;
+        }
+    } else {
         /*
-         * vhostfd is closed by vhost_dev_cleanup, which is called
-         * by vhost_dev_init on initialization error.
+         * CPR restore case: only learn the backend feature set now, but
+         * defer taking ownership or touching VQs (the still-running source
+         * might be using them).  The full vhost_dev_init()/VHOST_SET_OWNER
+         * is done later in post_load.
          */
-        goto err_virtio;
+        ret = vhost_dev_init_backend(&vvc->vhost_dev,
+                                     (void *)(uintptr_t)vhostfd,
+                                     VHOST_BACKEND_TYPE_KERNEL, errp);
+        if (ret < 0) {
+            /* vhost_dev_init_backend() does not close the fd on error */
+            goto err_vhost_dev;
+        }
+
+        return;
     }
 
     ret = vhost_vsock_set_guest_cid(vdev);
@@ -234,6 +357,7 @@ err_vhost_dev:
 err_virtio:
     vhost_vsock_common_unrealize(vdev);
 err_blocker:
+    migration_remove_notifier(&vsock->cpr_notifier);
     migrate_del_blocker(&vsock->migration_blocker);
 }
 
@@ -250,6 +374,7 @@ static void vhost_vsock_device_unrealize(DeviceState *dev)
     if (proxy->id) {
         cpr_delete_fd(proxy->id, 0);
     }
+    migration_remove_notifier(&vsock->cpr_notifier);
     migrate_del_blocker(&vsock->migration_blocker);
     vhost_dev_cleanup(&vvc->vhost_dev);
     vhost_vsock_common_unrealize(vdev);
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 7ae2abe33cd..3734e3f20b5 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1647,7 +1647,7 @@ fail_call:
 static void vhost_virtqueue_cleanup(struct vhost_virtqueue *vq)
 {
     event_notifier_cleanup(&vq->masked_notifier);
-    if (vq->dev->vhost_ops->vhost_set_vring_err) {
+    if (vq->dev && vq->dev->vhost_ops->vhost_set_vring_err) {
         event_notifier_set_handler(&vq->error_notifier, NULL);
         event_notifier_cleanup(&vq->error_notifier);
     }
diff --git a/include/hw/virtio/vhost-vsock.h b/include/hw/virtio/vhost-vsock.h
index 5ebc63afc5a..6d0cff4fb93 100644
--- a/include/hw/virtio/vhost-vsock.h
+++ b/include/hw/virtio/vhost-vsock.h
@@ -15,6 +15,7 @@
 #define QEMU_VHOST_VSOCK_H
 
 #include "hw/virtio/vhost-vsock-common.h"
+#include "qemu/notify.h"
 #include "qom/object.h"
 
 #define TYPE_VHOST_VSOCK "vhost-vsock-device"
@@ -30,6 +31,8 @@ struct VHostVSock {
     VHostVSockCommon parent;
     VHostVSockConf conf;
     Error *migration_blocker;   /* set when the device has no ID */
+    bool owner_reset;           /* CPR released ownership; needs re-acquire */
+    NotifierWithReturn cpr_notifier;   /* re-acquires ownership if CPR fails */
 
     /*< public >*/
 };
-- 
2.47.1

Re: [PATCH v3 0/7] migration/cpr: support vhost-vsock devices

[Maciej S. Szmigiero]

On 26.06.2026 18:46, Andrey Drobyshev wrote:
> v2 -> v3:
> 
> Re-design device ownership hand-off (suggested by Dongli).  Do not rely
> on SETUP migration notifiers, as this approach forces us to reorder
> generic migration code (see v2 discussion).  Instead:
> 
>    * .pre_save() releases ownership on the source (RESET_OWNER) once VM
>      is stopped;
>    * .realize() is adjusted to defer device ownership acquisition for an
>      incoming CPR;
>    * .post_load() actually claims the device (VHOST_SET_OWNER);
>    * FAILED migration event callback re-aquires the ownership on the
>      source after re_save released it.
> 
> v2: https://lore.kernel.org/qemu-devel/57c9c9b3-d758-489b-95b8-d16c258b9b0c@virtuozzo.com
> 
> Andrey Drobyshev (7):
>    vhost: add vhost_reset_owner op
>    vhost-vsock: don't reset connections during CPR
>    vhost-vsock: fix FD leak in realize()
>    vhost-vsock: preserve vhost FD during CPR
>    vhost: factor out vhost_dev_init_backend()
>    vhost: add vhost_dev_set_owner() / vhost_dev_reset_owner() helpers
>    vhost-vsock: hand off device ownership across CPR
> 
These patches look okay CPR-wise, so:
Acked-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com> # for CPR

But the vsock parts still need review by their maintainers/reviewers.

Thanks,
Maciej

Re: [PATCH v3 0/7] migration/cpr: support vhost-vsock devices

[Vladimir Sementsov-Ogievskiy]

On 26.06.26 19:46, Andrey Drobyshev wrote:
> v2 -> v3:
> 
> Re-design device ownership hand-off (suggested by Dongli).  Do not rely
> on SETUP migration notifiers, as this approach forces us to reorder
> generic migration code (see v2 discussion).  Instead:
> 
>    * .pre_save() releases ownership on the source (RESET_OWNER) once VM
>      is stopped;
>    * .realize() is adjusted to defer device ownership acquisition for an
>      incoming CPR;
>    * .post_load() actually claims the device (VHOST_SET_OWNER);
>    * FAILED migration event callback re-aquires the ownership on the
>      source after re_save released it.
> 
> v2:https://lore.kernel.org/qemu-devel/57c9c9b3-d758-489b-95b8-d16c258b9b0c@virtuozzo.com

Hi!

Two notes:

1. Did you consider migrating needed FDs through main migration channel, without
use of CPR, like I do in (not yet landed) "[PATCH v19 00/15] virtio-net: live-TAP local migration" [1]

2. I don't know how much vhost-vsock differs from vhost-net, but for vhost-net I remember
that RESET_OWNER + SET_OWNER is effectively equal to simply recreating vhost device on target,
because in kernel vhost device is hardly bound to the process itself, and can't be passed to
another process. So, we can pass only "empty" FD, not the initialized vhost device.
That's why in [1] I only pass tap-fds (and some additional state), but vhost fds are simply
reopened on target (or passed by mgmt app).
Does vhost-vsock work in a different way? Is there real sense in passing vhost fds here?


[1] https://lore.kernel.org/qemu-devel/20260714154246.1242856-1-vsementsov@yandex-team.ru/

-- 
Best regards,
Vladimir

Re: [PATCH v3 0/7] migration/cpr: support vhost-vsock devices

[Andrey Drobyshev]

On 7/14/26 6:03 PM, Vladimir Sementsov-Ogievskiy wrote:
> On 26.06.26 19:46, Andrey Drobyshev wrote:
>> v2 -> v3:
>>
>> Re-design device ownership hand-off (suggested by Dongli).  Do not rely
>> on SETUP migration notifiers, as this approach forces us to reorder
>> generic migration code (see v2 discussion).  Instead:
>>
>>    * .pre_save() releases ownership on the source (RESET_OWNER) once VM
>>      is stopped;
>>    * .realize() is adjusted to defer device ownership acquisition for an
>>      incoming CPR;
>>    * .post_load() actually claims the device (VHOST_SET_OWNER);
>>    * FAILED migration event callback re-aquires the ownership on the
>>      source after re_save released it.
>>
>> v2:https://lore.kernel.org/qemu-devel/57c9c9b3-d758-489b-95b8-d16c258b9b0c@virtuozzo.com
> 
> Hi!
>

Hello Vladimir!

> Two notes:
> 
> 1. Did you consider migrating needed FDs through main migration channel, without
> use of CPR, like I do in (not yet landed) "[PATCH v19 00/15] virtio-net: live-TAP local migration" [1]
>

AFAIU you're doing local same-host migration as a way to update QEMU
binary.  With this approach transferring FDs through the main channel is
indeed cleaner.  However in our own downstream we use enhanced in-place
memory preservation which is based entirely on cpr-exec migration mode.

So, to answer your question: yes, we considered it, and it might work
for cpr-transfer where we're also doing same-host migration.  But in
cpr-exec case there's only one QEMU process involved => we can't use
UNIX socket for migration transport => no SCM_RIGHTS FDs passing => FDs
should be passed via additional transport, no way around it.


> 2. I don't know how much vhost-vsock differs from vhost-net, but for vhost-net I remember
> that RESET_OWNER + SET_OWNER is effectively equal to simply recreating vhost device on target,
> because in kernel vhost device is hardly bound to the process itself, and can't be passed to
> another process. So, we can pass only "empty" FD, not the initialized vhost device.
> That's why in [1] I only pass tap-fds (and some additional state), but vhost fds are simply
> reopened on target (or passed by mgmt app).
> Does vhost-vsock work in a different way? Is there real sense in passing vhost fds here?
> 
> 
> [1] https://lore.kernel.org/qemu-devel/20260714154246.1242856-1-vsementsov@yandex-team.ru/
> 

I'm also not extremely keen on vhost-net internals, but as I understand
with vhost-net we have 2 FDs: TAP FD for the network endpoint and vhost
FD for the memory tables, vring addresses etc.  The TAP FD is
process-independent state - and we pass it via migration channel.  The
vhost FD is entirely per owning process - and we're able to close it on
source, reopen on target and bind to the passed TAP FD.

However with vhost-vsock there's only one FD.  It's used both for
plumbing and the endpoint.  At the very least, vhost device state holds
guest CID registration in the global vhost_vsock_hash.  If we go the
"recreate/reopen the device" path - CID is removed from the hash in
vhost_vsock_dev_release(), and then we're gonna get ECONNRESET for any
incoming packets => connections are dead.  We can't afford that.

Andrey

Re: [PATCH v3 0/7] migration/cpr: support vhost-vsock devices

[Vladimir Sementsov-Ogievskiy]

On 15.07.26 13:25, Andrey Drobyshev wrote:
> On 7/14/26 6:03 PM, Vladimir Sementsov-Ogievskiy wrote:
>> On 26.06.26 19:46, Andrey Drobyshev wrote:
>>> v2 -> v3:
>>>
>>> Re-design device ownership hand-off (suggested by Dongli).  Do not rely
>>> on SETUP migration notifiers, as this approach forces us to reorder
>>> generic migration code (see v2 discussion).  Instead:
>>>
>>>     * .pre_save() releases ownership on the source (RESET_OWNER) once VM
>>>       is stopped;
>>>     * .realize() is adjusted to defer device ownership acquisition for an
>>>       incoming CPR;
>>>     * .post_load() actually claims the device (VHOST_SET_OWNER);
>>>     * FAILED migration event callback re-aquires the ownership on the
>>>       source after re_save released it.
>>>
>>> v2:https://lore.kernel.org/qemu-devel/57c9c9b3-d758-489b-95b8-d16c258b9b0c@virtuozzo.com
>>
>> Hi!
>>
> 
> Hello Vladimir!
> 
>> Two notes:
>>
>> 1. Did you consider migrating needed FDs through main migration channel, without
>> use of CPR, like I do in (not yet landed) "[PATCH v19 00/15] virtio-net: live-TAP local migration" [1]
>>
> 
> AFAIU you're doing local same-host migration as a way to update QEMU
> binary.  With this approach transferring FDs through the main channel is
> indeed cleaner.  However in our own downstream we use enhanced in-place
> memory preservation which is based entirely on cpr-exec migration mode.
> 
> So, to answer your question: yes, we considered it, and it might work
> for cpr-transfer where we're also doing same-host migration.  But in
> cpr-exec case there's only one QEMU process involved => we can't use
> UNIX socket for migration transport => no SCM_RIGHTS FDs passing => FDs
> should be passed via additional transport, no way around it.


Hmm. Looking at code for fd-passing through migration channel:

static bool load_fd(QEMUFile *f, void *pv, size_t size,
                     const VMStateField *field, Error **errp)
{
     int32_t *v = pv;

     if (migrate_mode() == MIG_MODE_CPR_EXEC) {
         qemu_get_sbe32s(f, v);
         return true;
     }
...


Seems like passing FDs through main channel (like in my TAP-series) would
work both for local migration (with two processes and UNIX socket) and for
CPR_EXEC mode. So the FD is passed as simple number in case of cpr-exec.

So, if simply pass FDs through migration state, it should work both for
CPR_EXEC and local migration.

(hmm, still, keeping in mind that TAP-series API still not negotiated,
it's simpler to start from pure CPR approach, and add local-migration
support in future)

> 
> 
>> 2. I don't know how much vhost-vsock differs from vhost-net, but for vhost-net I remember
>> that RESET_OWNER + SET_OWNER is effectively equal to simply recreating vhost device on target,
>> because in kernel vhost device is hardly bound to the process itself, and can't be passed to
>> another process. So, we can pass only "empty" FD, not the initialized vhost device.
>> That's why in [1] I only pass tap-fds (and some additional state), but vhost fds are simply
>> reopened on target (or passed by mgmt app).
>> Does vhost-vsock work in a different way? Is there real sense in passing vhost fds here?
>>
>>
>> [1] https://lore.kernel.org/qemu-devel/20260714154246.1242856-1-vsementsov@yandex-team.ru/
>>
> 
> I'm also not extremely keen on vhost-net internals, but as I understand
> with vhost-net we have 2 FDs: TAP FD for the network endpoint and vhost
> FD for the memory tables, vring addresses etc.  The TAP FD is
> process-independent state - and we pass it via migration channel.  The
> vhost FD is entirely per owning process - and we're able to close it on
> source, reopen on target and bind to the passed TAP FD.

Yes, my understanding matches.

> 
> However with vhost-vsock there's only one FD.  It's used both for
> plumbing and the endpoint.  At the very least, vhost device state holds
> guest CID registration in the global vhost_vsock_hash.  If we go the
> "recreate/reopen the device" path - CID is removed from the hash in
> vhost_vsock_dev_release(), and then we're gonna get ECONNRESET for any
> incoming packets => connections are dead.  We can't afford that.
> 

OK, thanks for explanation!


-- 
Best regards,
Vladimir

Re: [PATCH v3 0/7] migration/cpr: support vhost-vsock devices

[Andrey Drobyshev]

On 7/15/26 12:48 PM, Vladimir Sementsov-Ogievskiy wrote:
> On 15.07.26 13:25, Andrey Drobyshev wrote:
>> On 7/14/26 6:03 PM, Vladimir Sementsov-Ogievskiy wrote:
>>> On 26.06.26 19:46, Andrey Drobyshev wrote:
>>>> v2 -> v3:
>>>>
>>>> Re-design device ownership hand-off (suggested by Dongli).  Do not rely
>>>> on SETUP migration notifiers, as this approach forces us to reorder
>>>> generic migration code (see v2 discussion).  Instead:
>>>>
>>>>     * .pre_save() releases ownership on the source (RESET_OWNER) once VM
>>>>       is stopped;
>>>>     * .realize() is adjusted to defer device ownership acquisition for an
>>>>       incoming CPR;
>>>>     * .post_load() actually claims the device (VHOST_SET_OWNER);
>>>>     * FAILED migration event callback re-aquires the ownership on the
>>>>       source after re_save released it.
>>>>
>>>> v2:https://lore.kernel.org/qemu-devel/57c9c9b3-d758-489b-95b8-d16c258b9b0c@virtuozzo.com
>>>
>>> Hi!
>>>
>>
>> Hello Vladimir!
>>
>>> Two notes:
>>>
>>> 1. Did you consider migrating needed FDs through main migration channel, without
>>> use of CPR, like I do in (not yet landed) "[PATCH v19 00/15] virtio-net: live-TAP local migration" [1]
>>>
>>
>> AFAIU you're doing local same-host migration as a way to update QEMU
>> binary.  With this approach transferring FDs through the main channel is
>> indeed cleaner.  However in our own downstream we use enhanced in-place
>> memory preservation which is based entirely on cpr-exec migration mode.
>>
>> So, to answer your question: yes, we considered it, and it might work
>> for cpr-transfer where we're also doing same-host migration.  But in
>> cpr-exec case there's only one QEMU process involved => we can't use
>> UNIX socket for migration transport => no SCM_RIGHTS FDs passing => FDs
>> should be passed via additional transport, no way around it.
> 
> 
> Hmm. Looking at code for fd-passing through migration channel:
> 
> static bool load_fd(QEMUFile *f, void *pv, size_t size,
>                      const VMStateField *field, Error **errp)
> {
>      int32_t *v = pv;
> 
>      if (migrate_mode() == MIG_MODE_CPR_EXEC) {
>          qemu_get_sbe32s(f, v);
>          return true;
>      }
> ...
> 
> 
> Seems like passing FDs through main channel (like in my TAP-series) would
> work both for local migration (with two processes and UNIX socket) and for
> CPR_EXEC mode. So the FD is passed as simple number in case of cpr-exec.
> 
> So, if simply pass FDs through migration state, it should work both for
> CPR_EXEC and local migration.
> 
> (hmm, still, keeping in mind that TAP-series API still not negotiated,
> it's simpler to start from pure CPR approach, and add local-migration
> support in future)
>

Yes, my claim was too harsh - the FD passing mechanism itself probably
can be implemented via the main migration channel as well, even for
cpr-exec.  E.g. if it's not a UNIX socket but a plain file on file
system - we should be good.

But then the issue is timing: VMSTATE_FD delivers the FD during the
device's vmstate load, i.e. around post_load.  And cpr_state is loaded
early, before device realization, so the FD is available at realize
time.  And for instance our vhost-vsock case does need FD already during
realize to read the backend feature set:

vhost_vsock_device_realize() ->
  vhost_dev_init_backend() ->
    vhost_dev_init_features()

So for me the exact transport used underneath for FDs passing is not
that important.  I'm just exploiting the CPR API to get the passed FD at
an early .realize() stage.  If we can somehow guarantee an early FDs
delivery through the main channel - that'll work.

Andrey

>>
>>
>>> 2. I don't know how much vhost-vsock differs from vhost-net, but for vhost-net I remember
>>> that RESET_OWNER + SET_OWNER is effectively equal to simply recreating vhost device on target,
>>> because in kernel vhost device is hardly bound to the process itself, and can't be passed to
>>> another process. So, we can pass only "empty" FD, not the initialized vhost device.
>>> That's why in [1] I only pass tap-fds (and some additional state), but vhost fds are simply
>>> reopened on target (or passed by mgmt app).
>>> Does vhost-vsock work in a different way? Is there real sense in passing vhost fds here?
>>>
>>>
>>> [1] https://lore.kernel.org/qemu-devel/20260714154246.1242856-1-vsementsov@yandex-team.ru/
>>>
>>
>> I'm also not extremely keen on vhost-net internals, but as I understand
>> with vhost-net we have 2 FDs: TAP FD for the network endpoint and vhost
>> FD for the memory tables, vring addresses etc.  The TAP FD is
>> process-independent state - and we pass it via migration channel.  The
>> vhost FD is entirely per owning process - and we're able to close it on
>> source, reopen on target and bind to the passed TAP FD.
> 
> Yes, my understanding matches.
> 
>>
>> However with vhost-vsock there's only one FD.  It's used both for
>> plumbing and the endpoint.  At the very least, vhost device state holds
>> guest CID registration in the global vhost_vsock_hash.  If we go the
>> "recreate/reopen the device" path - CID is removed from the hash in
>> vhost_vsock_dev_release(), and then we're gonna get ECONNRESET for any
>> incoming packets => connections are dead.  We can't afford that.
>>
> 
> OK, thanks for explanation!
> 
> 

Re: [PATCH v3 0/7] migration/cpr: support vhost-vsock devices

[Andrey Drobyshev]

On 6/30/26 9:35 PM, Maciej S. Szmigiero wrote:
> On 26.06.2026 18:46, Andrey Drobyshev wrote:
>> v2 -> v3:
>>
>> Re-design device ownership hand-off (suggested by Dongli).  Do not rely
>> on SETUP migration notifiers, as this approach forces us to reorder
>> generic migration code (see v2 discussion).  Instead:
>>
>>    * .pre_save() releases ownership on the source (RESET_OWNER) once VM
>>      is stopped;
>>    * .realize() is adjusted to defer device ownership acquisition for an
>>      incoming CPR;
>>    * .post_load() actually claims the device (VHOST_SET_OWNER);
>>    * FAILED migration event callback re-aquires the ownership on the
>>      source after re_save released it.
>>
>> v2: https://lore.kernel.org/qemu-devel/57c9c9b3-d758-489b-95b8-d16c258b9b0c@virtuozzo.com
>>
>> Andrey Drobyshev (7):
>>    vhost: add vhost_reset_owner op
>>    vhost-vsock: don't reset connections during CPR
>>    vhost-vsock: fix FD leak in realize()
>>    vhost-vsock: preserve vhost FD during CPR
>>    vhost: factor out vhost_dev_init_backend()
>>    vhost: add vhost_dev_set_owner() / vhost_dev_reset_owner() helpers
>>    vhost-vsock: hand off device ownership across CPR
>>
> These patches look okay CPR-wise, so:
> Acked-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com> # for CPR
> 
> But the vsock parts still need review by their maintainers/reviewers.
> 
> Thanks,
> Maciej
> 

FYI the kernel counterpart series adding VHOST_RESET_OWNER ioctl to
vsock ended up in Michael's vhost tree ([1], commits
6236e765f16d^..c7bfb8815d83).

Therefore - friendly ping for this series.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git

Andrey

Re: [PATCH v3 0/7] migration/cpr: support vhost-vsock devices

[Andrey Drobyshev]

On 8/11/26 5:12 PM, Andrey Drobyshev wrote:
> On 6/30/26 9:35 PM, Maciej S. Szmigiero wrote:
>> On 26.06.2026 18:46, Andrey Drobyshev wrote:
>>> v2 -> v3:
>>>
>>> Re-design device ownership hand-off (suggested by Dongli).  Do not rely
>>> on SETUP migration notifiers, as this approach forces us to reorder
>>> generic migration code (see v2 discussion).  Instead:
>>>
>>>    * .pre_save() releases ownership on the source (RESET_OWNER) once VM
>>>      is stopped;
>>>    * .realize() is adjusted to defer device ownership acquisition for an
>>>      incoming CPR;
>>>    * .post_load() actually claims the device (VHOST_SET_OWNER);
>>>    * FAILED migration event callback re-aquires the ownership on the
>>>      source after re_save released it.
>>>
>>> v2: https://lore.kernel.org/qemu-devel/57c9c9b3-d758-489b-95b8-d16c258b9b0c@virtuozzo.com
>>>
>>> Andrey Drobyshev (7):
>>>    vhost: add vhost_reset_owner op
>>>    vhost-vsock: don't reset connections during CPR
>>>    vhost-vsock: fix FD leak in realize()
>>>    vhost-vsock: preserve vhost FD during CPR
>>>    vhost: factor out vhost_dev_init_backend()
>>>    vhost: add vhost_dev_set_owner() / vhost_dev_reset_owner() helpers
>>>    vhost-vsock: hand off device ownership across CPR
>>>
>> These patches look okay CPR-wise, so:
>> Acked-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com> # for CPR
>>
>> But the vsock parts still need review by their maintainers/reviewers.
>>
>> Thanks,
>> Maciej
>>
> 
> FYI the kernel counterpart series adding VHOST_RESET_OWNER ioctl to
> vsock ended up in Michael's vhost tree ([1], commits
> 6236e765f16d^..c7bfb8815d83).
> 
> Therefore - friendly ping for this series.
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git
> 
> Andrey

Another friendly ping

Re: [PATCH v3 4/7] vhost-vsock: preserve vhost FD during CPR

[Stefano Garzarella]

On Fri, Jun 26, 2026 at 07:46:40PM +0300, Andrey Drobyshev wrote:
>During CPR (checkpoint-restore) migration the guest keeps running on the
>same host, so instead of reopening /dev/vhost-vsock on the destination,
>we should reuse the FD from the source.  The FD is saved in the CPR
>namespace (hash table) with cpr_save_fd() and then reclaimed on the
>target via cpr_find_fd().
>
>Since the key in CPR hash table is device ID, CPR needs a unique ID.
>Rather than make it mandatory for every vhost-vsock device, we add CPR
>migration blocker which only fires once we attempt CPR with ID-less
>vhost-vsock.
>
>vhost_dev_init() (and thus VHOST_SET_OWNER) still runs in realize() here.
>Deferring the ownership handoff to pre_save/post_load is done in a
>following patch.

So will this patch be bisectabale?

Thanks,
Stefano

>
>Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>---
> hw/virtio/vhost-vsock.c         | 50 ++++++++++++++++++++++++++++++---
> include/hw/virtio/vhost-vsock.h |  1 +
> 2 files changed, 47 insertions(+), 4 deletions(-)
>
>diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
>index fd7ffa88990..7eacb608d07 100644
>--- a/hw/virtio/vhost-vsock.c
>+++ b/hw/virtio/vhost-vsock.c
>@@ -21,6 +21,8 @@
> #include "hw/virtio/vhost-vsock.h"
> #include "monitor/monitor.h"
> #include "migration/cpr.h"
>+#include "migration/blocker.h"
>+#include "migration/misc.h"
>
> static void vhost_vsock_get_config(VirtIODevice *vdev, uint8_t *config)
> {
>@@ -141,6 +143,7 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
>     VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(dev);
>     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>     VHostVSock *vsock = VHOST_VSOCK(dev);
>+    DeviceState *proxy = qdev_get_parent_bus(DEVICE(vsock))->parent;
>     int vhostfd;
>     int ret;
>
>@@ -155,23 +158,49 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
>         return;
>     }
>
>-    if (vsock->conf.vhostfd) {
>+    /*
>+     * Having a unique ID is mandatory for FD preservation during CPR
>+     * migration, thus we add migration blockers for CPR modes.
>+     */
>+    if (!proxy->id) {
>+        error_setg(&vsock->migration_blocker,
>+                   "vhost-vsock: device ID is required for CPR migration");
>+        if (migrate_add_blocker_modes(&vsock->migration_blocker,
>+                                      BIT(MIG_MODE_CPR_TRANSFER) |
>+                                      BIT(MIG_MODE_CPR_EXEC), errp) < 0) {
>+            return;
>+        }
>+    }
>+
>+    if (cpr_is_incoming()) {
>+        /* Reuse the fd handed over from the source QEMU. */
>+        if (!proxy->id) {
>+            error_setg(errp, "vhost-vsock: device ID is required for "
>+                       "CPR migration");
>+            goto err_blocker;
>+        }
>+        vhostfd = cpr_find_fd(proxy->id, 0);
>+        if (vhostfd < 0) {
>+            error_setg(errp, "vhost-vsock: could not find restored vhost FD");
>+            goto err_blocker;
>+        }
>+    } else if (vsock->conf.vhostfd) {
>         vhostfd = monitor_fd_param(monitor_cur(), vsock->conf.vhostfd, errp);
>         if (vhostfd == -1) {
>             error_prepend(errp, "vhost-vsock: unable to parse vhostfd: ");
>-            return;
>+            goto err_blocker;
>         }
>     } else {
>         vhostfd = open("/dev/vhost-vsock", O_RDWR);
>         if (vhostfd < 0) {
>             error_setg_file_open(errp, errno, "/dev/vhost-vsock");
>-            return;
>+            goto err_blocker;
>         }
>     }
>
>     if (!qemu_set_blocking(vhostfd, false, errp)) {
>         close(vhostfd);
>-        return;
>+        goto err_blocker;
>     }
>
>     vhost_vsock_common_realize(vdev);
>@@ -192,6 +221,11 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
>         goto err_vhost_dev;
>     }
>
>+    /* Register the fd for a future CPR after a fully successful realize */
>+    if (proxy->id) {
>+        cpr_save_fd(proxy->id, 0, vhostfd);
>+    }
>+
>     return;
>
> err_vhost_dev:
>@@ -199,16 +233,24 @@ err_vhost_dev:
>     vhost_dev_cleanup(&vvc->vhost_dev);
> err_virtio:
>     vhost_vsock_common_unrealize(vdev);
>+err_blocker:
>+    migrate_del_blocker(&vsock->migration_blocker);
> }
>
> static void vhost_vsock_device_unrealize(DeviceState *dev)
> {
>     VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(dev);
>     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>+    VHostVSock *vsock = VHOST_VSOCK(dev);
>+    DeviceState *proxy = qdev_get_parent_bus(dev)->parent;
>
>     /* This will stop vhost backend if appropriate. */
>     vhost_vsock_set_status(vdev, 0);
>
>+    if (proxy->id) {
>+        cpr_delete_fd(proxy->id, 0);
>+    }
>+    migrate_del_blocker(&vsock->migration_blocker);
>     vhost_dev_cleanup(&vvc->vhost_dev);
>     vhost_vsock_common_unrealize(vdev);
> }
>diff --git a/include/hw/virtio/vhost-vsock.h b/include/hw/virtio/vhost-vsock.h
>index 84f4e727c70..5ebc63afc5a 100644
>--- a/include/hw/virtio/vhost-vsock.h
>+++ b/include/hw/virtio/vhost-vsock.h
>@@ -29,6 +29,7 @@ struct VHostVSock {
>     /*< private >*/
>     VHostVSockCommon parent;
>     VHostVSockConf conf;
>+    Error *migration_blocker;   /* set when the device has no ID */
>
>     /*< public >*/
> };
>-- 
>2.47.1
>

Re: [PATCH v3 5/7] vhost: factor out vhost_dev_init_backend()

[Stefano Garzarella]

On Fri, Jun 26, 2026 at 07:46:41PM +0300, Andrey Drobyshev wrote:
>Split the first part of vhost_dev_init(): selecting the backend, calling
>its .vhost_init() and reading the supported features - into a new
>vhost_dev_init_backend() helper, and call it from vhost_dev_init().
>
>This is in preparation for CPR restore of vhost-vsock, which needs to learn
>the backend's features at realize time to negotiate them when loading the
>incoming virtio state, but also must defer taking ownership of the device
>to post_load.  vhost_dev_init_backend() does exactly the pre-ownership part.
>
>As a result VHOST_SET_OWNER now follows the feature query rather than
>precedes it.  This should be safe, as no backend requires ownership before
>VHOST_GET_FEATURES - the kernel and vdpa backends do not check ownership
>for it, and vhost-user already does query features from its .vhost_init()
>before set_owner().
>
>Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>---
> hw/virtio/vhost.c         | 33 +++++++++++++++++++++++----------
> include/hw/virtio/vhost.h | 19 +++++++++++++++++++
> 2 files changed, 42 insertions(+), 10 deletions(-)
>
>diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>index af41841b529..f11588cc51a 100644
>--- a/hw/virtio/vhost.c
>+++ b/hw/virtio/vhost.c
>@@ -1667,6 +1667,28 @@ static int vhost_dev_init_features(struct vhost_dev *hdev)
>     return r;
> }
>
>+int vhost_dev_init_backend(struct vhost_dev *hdev, void *opaque,
>+                           VhostBackendType backend_type, Error **errp)
>+{
>+    int r;
>+
>+    r = vhost_set_backend_type(hdev, backend_type);
>+    assert(r >= 0);
>+
>+    r = hdev->vhost_ops->vhost_init(hdev, opaque, errp);
>+    if (r < 0) {
>+        return r;
>+    }
>+
>+    r = vhost_dev_init_features(hdev);
>+    if (r < 0) {
>+        error_setg_errno(errp, -r, "vhost_init_features failed");
>+        return r;
>+    }
>+
>+    return 0;
>+}
>+
> int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
>                    VhostBackendType backend_type, uint32_t busyloop_timeout,
>                    Error **errp)
>@@ -1679,10 +1701,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
>     hdev->vdev = NULL;
>     hdev->migration_blocker = NULL;
>
>-    r = vhost_set_backend_type(hdev, backend_type);
>-    assert(r >= 0);
>-
>-    r = hdev->vhost_ops->vhost_init(hdev, opaque, errp);
>+    r = vhost_dev_init_backend(hdev, opaque, backend_type, errp);
>     if (r < 0) {
>         goto fail;
>     }
>@@ -1693,12 +1712,6 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
>         goto fail;
>     }
>
>-    r = vhost_dev_init_features(hdev);
>-    if (r < 0) {
>-        error_setg_errno(errp, -r, "vhost_init_features failed");
>-        goto fail;
>-    }
>-
>     limit = hdev->vhost_ops->vhost_memslots_limit(hdev);
>     if (limit < MEMORY_DEVICES_SAFE_MAX_MEMSLOTS &&
>         memory_devices_memslot_auto_decision_active()) {
>diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
>index 684bafcaadd..bc81e09663e 100644
>--- a/include/hw/virtio/vhost.h
>+++ b/include/hw/virtio/vhost.h
>@@ -156,6 +156,25 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
>                    VhostBackendType backend_type,
>                    uint32_t busyloop_timeout, Error **errp);
>
>+/**
>+ * vhost_dev_init_backend() - set up the backend and query its features
>+ * @hdev: the common vhost_dev structure
>+ * @opaque: opaque ptr passed to backend (vhost/vhost-user/vdpa)
>+ * @backend_type: type of backend
>+ * @errp: error handle
>+ *
>+ * Select the backend, initialise the backend instance and read its supported
>+ * features into @hdev, without issuing VHOST_SET_OWNER, setting up the
>+ * virtqueues or registering the memory listener.  This is the part of
>+ * vhost_dev_init() that precedes taking ownership; it can be used on its own
>+ * so feature negotiation can happen before ownership is acquired (e.g. by CPR
>+ * restore).

Should we mention that on error `vhost_dev_cleanup()` must be called, or 
maybe should we call it in the new function if vhost_dev_init_features() 
fails?

Thanks,
Stefano

>+ *
>+ * Return: 0 on success, non-zero on error while setting errp.
>+ */
>+int vhost_dev_init_backend(struct vhost_dev *hdev, void *opaque,
>+                           VhostBackendType backend_type, Error **errp);
>+
> /**
>  * vhost_dev_cleanup() - tear down and cleanup vhost interface
>  * @hdev: the common vhost_dev structure
>-- 
>2.47.1
>

Re: [PATCH v3 7/7] vhost-vsock: hand off device ownership across CPR

[Stefano Garzarella]

On Fri, Jun 26, 2026 at 07:46:43PM +0300, Andrey Drobyshev wrote:
>The previous patches reuse the source vhost FD on the destination,
>however both source and targe still call vhost_dev_init() (VHOST_SET_OWNER)
>in realize().  For cpr-transfer the destination realizes while the source
>still owns the shared FD, so its SET_OWNER would fail - ownership has to be
>handed over explicitly.
>
>Do this through the device's CPR vmstate hooks.  Namely, release device
>ownership in pre_save, reclaim in post_load, re-acquire on failure:
>
>  - .pre_save() releases ownership on the source (VHOST_RESET_OWNER) once
>    the VM is stopped, for the FD-preserving CPR modes (cpr-transfer and
>    cpr-exec).
>
>  - .realize(), for an incoming CPR, only sets up the virtio device and
>    queries the backend features by calling vhost_dev_init_backend().
>    It doesn't take device ownership and doesn't touch the VQs which
>    still-running source might use.  The full init is deferred to
>    .post_load().
>
>  - .post_load() reclaims it on the destination: the full vhost_dev_init()
>    (VHOST_SET_OWNER) on the preserved FD, plus sets the guest cid, before
>    the device is started at vm_start.
>
>  - A MIG_EVENT_FAILED notifier re-acquires ownership if the migration
>    fails after pre_save released it and the source VM is resumed.
>
>Also harden vhost_virtqueue_cleanup() against a NULL vq->dev which can
>now happen if an incoming CPR is aborted after realize set up the backend
>but before post_load initialised the VQs.
>
>Suggested-by: Dongli Zhang <dongli.zhang@oracle.com>
>Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>---
> hw/virtio/vhost-vsock.c         | 147 +++++++++++++++++++++++++++++---
> hw/virtio/vhost.c               |   2 +-
> include/hw/virtio/vhost-vsock.h |   3 +
> 3 files changed, 140 insertions(+), 12 deletions(-)
>
>diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
>index 7eacb608d07..b02b3f9cc03 100644
>--- a/hw/virtio/vhost-vsock.c
>+++ b/hw/virtio/vhost-vsock.c
>@@ -76,6 +76,17 @@ static int vhost_vsock_set_status(VirtIODevice *vdev, uint8_t status)
>     bool should_start = virtio_device_should_start(vdev, status);
>     int ret;
>
>+    /*
>+     * On an incoming CPR the full vhost_dev_init() is deferred to post_load
>+     * (realize only ran vhost_dev_init_backend()).  hdev->mem is set only by
>+     * the full init, so refuse to start a device whose handoff never
>+     * completed rather than dereference a half-initialised vhost_dev.
>+     */
>+    if (should_start && !vvc->vhost_dev.mem) {

Should we also check `vsock->owner_reset`?
IIUC it can be left set if for example vhost_dev_set_owner() failed.

About using `vvc->vhost_dev.mem`, I'm a bit worried if it can be a bit 
fragile for future changes.  What about adding a new field (e.g.  
`initialized`) set by vhost_dev_init() when everything is fine?

>+        error_report("vhost-vsock: refusing to start, device init incomplete");
>+        return 0;
>+    }
>+
>     if (vhost_dev_is_started(&vvc->vhost_dev) == should_start) {
>         return 0;
>     }
>@@ -111,18 +122,102 @@ static uint64_t vhost_vsock_get_features(VirtIODevice *vdev,
>     return vhost_vsock_common_get_features(vdev, requested_features, errp);
> }
>
>+/*
>+ * Re-acquire device ownership if a CPR migration that released it (in
>+ * vhost_vsock_cpr_pre_save()) failed and the source VM is about to resume.
>+ * This runs before vm_start(), so the device is owned again before it is
>+ * restarted.
>+ */
>+static int vhost_vsock_cpr_notifier(NotifierWithReturn *notifier,
>+                                    MigrationEvent *e, Error **errp)
>+{
>+    VHostVSock *vsock = container_of(notifier, VHostVSock, cpr_notifier);
>+    VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(vsock);
>+    int ret;
>+
>+    if (e->type == MIG_EVENT_FAILED && vsock->owner_reset) {
>+        ret = vhost_dev_set_owner(&vvc->vhost_dev);
>+        if (ret < 0) {
>+            error_report("vhost-vsock: failed to re-acquire owner: %d", ret);
>+        } else {
>+            vsock->owner_reset = false;
>+        }
>+    }
>+
>+    return 0;
>+}
>+
>+static int vhost_vsock_pre_save(void *opaque)
>+{
>+    VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(opaque);
>+    VHostVSock *vsock = VHOST_VSOCK(opaque);
>+    int ret;
>+
>+    ret = vhost_vsock_common_pre_save(opaque);
>+    if (ret) {
>+        return ret;
>+    }
>+
>+    /*
>+     * Release the device ownership now for CPR migration.  The device is
>+     * already stopped at pre_save, and destination reclaims it by calling
>+     * VHOST_SET_OWNER in post_load.
>+     */
>+    if (cpr_incoming_needed(NULL)) {
>+        ret = vhost_dev_reset_owner(&vvc->vhost_dev);
>+        if (ret < 0) {
>+            error_report("vhost-vsock: vhost_reset_owner failed: %d", ret);
>+            return ret;
>+        }
>+        vsock->owner_reset = true;
>+    }
>+
>+    return 0;
>+}
>+
> static int vhost_vsock_post_load(void *opaque, int version_id)
> {
>+    VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(opaque);
>+    VirtIODevice *vdev = VIRTIO_DEVICE(opaque);
>+    DeviceState *proxy = qdev_get_parent_bus(DEVICE(vdev))->parent;
>+    Error *local_err = NULL;
>+    int vhostfd, ret;
>+
>     /*
>      * Only reset vsock connections for non-CPR migration.  For CPR the
>      * guest cid is unchanged, and the cid-change reset would otherwise
>      * tear the vsock connections down.
>      */
>-    if (cpr_is_incoming()) {
>-        return 0;
>+    if (!cpr_is_incoming()) {
>+        return vhost_vsock_common_post_load(opaque, version_id);
>+    }

nit: maybe we can do this in the patch where we introduced it.

>+
>+    /*
>+     * CPR restore case.  The source released device ownership in its
>+     * pre_save.  Complete the handoff here, before the device is started
>+     * at vm_start.  Init vhost device on preserved FD, issue
>+     * VHOST_SET_OWNER on it, and restore the guest cid.
>+     */
>+    vhostfd = cpr_find_fd(proxy->id, 0);
>+    if (vhostfd < 0) {
>+        error_report("vhost-vsock: could not find restored vhost FD");
>+        return -1;
>     }
>
>-    return vhost_vsock_common_post_load(opaque, version_id);
>+    ret = vhost_dev_init(&vvc->vhost_dev, (void *)(uintptr_t)vhostfd,
>+                         VHOST_BACKEND_TYPE_KERNEL, 0, &local_err);
>+    if (ret < 0) {
>+        error_report_err(local_err);
>+        return ret;
>+    }
>+
>+    ret = vhost_vsock_set_guest_cid(vdev);
>+    if (ret < 0) {

Should we call vhost_dev_cleanup() here?

>+        error_report("vhost-vsock: unable to set guest cid: %d", ret);
>+        return ret;
>+    }
>+
>+    return 0;
> }
>
> static const VMStateDescription vmstate_virtio_vhost_vsock = {
>@@ -133,7 +228,7 @@ static const VMStateDescription vmstate_virtio_vhost_vsock = {
>         VMSTATE_VIRTIO_DEVICE,
>         VMSTATE_END_OF_LIST()
>     },
>-    .pre_save = vhost_vsock_common_pre_save,
>+    .pre_save = vhost_vsock_pre_save,
>     .post_load = vhost_vsock_post_load,
> };
>
>@@ -144,6 +239,7 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
>     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>     VHostVSock *vsock = VHOST_VSOCK(dev);
>     DeviceState *proxy = qdev_get_parent_bus(DEVICE(vsock))->parent;
>+    bool cpr_incoming = cpr_is_incoming();
>     int vhostfd;
>     int ret;
>
>@@ -172,7 +268,16 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
>         }
>     }
>
>-    if (cpr_is_incoming()) {
>+    /*
>+     * Re-acquire ownership if a CPR migration releases it (in pre_save) but
>+     * then fails.
>+     */
>+    migration_add_notifier_modes(&vsock->cpr_notifier,
>+                                 vhost_vsock_cpr_notifier,
>+                                 BIT(MIG_MODE_CPR_TRANSFER) |
>+                                 BIT(MIG_MODE_CPR_EXEC));
>+
>+    if (cpr_incoming) {
>         /* Reuse the fd handed over from the source QEMU. */
>         if (!proxy->id) {
>             error_setg(errp, "vhost-vsock: device ID is required for "
>@@ -205,14 +310,32 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
>
>     vhost_vsock_common_realize(vdev);
>
>-    ret = vhost_dev_init(&vvc->vhost_dev, (void *)(uintptr_t)vhostfd,
>-                         VHOST_BACKEND_TYPE_KERNEL, 0, errp);
>-    if (ret < 0) {
>+    if (!cpr_incoming) {
>+        ret = vhost_dev_init(&vvc->vhost_dev, (void *)(uintptr_t)vhostfd,
>+                             VHOST_BACKEND_TYPE_KERNEL, 0, errp);
>+        if (ret < 0) {
>+            /*
>+             * vhostfd is closed by vhost_dev_cleanup, which is called
>+             * by vhost_dev_init on initialization error.
>+             */
>+            goto err_virtio;
>+        }
>+    } else {
>         /*
>-         * vhostfd is closed by vhost_dev_cleanup, which is called
>-         * by vhost_dev_init on initialization error.
>+         * CPR restore case: only learn the backend feature set now, but
>+         * defer taking ownership or touching VQs (the still-running source
>+         * might be using them).  The full vhost_dev_init()/VHOST_SET_OWNER
>+         * is done later in post_load.
>          */
>-        goto err_virtio;
>+        ret = vhost_dev_init_backend(&vvc->vhost_dev,
>+                                     (void *)(uintptr_t)vhostfd,
>+                                     VHOST_BACKEND_TYPE_KERNEL, errp);
>+        if (ret < 0) {
>+            /* vhost_dev_init_backend() does not close the fd on error */
>+            goto err_vhost_dev;
>+        }
>+
>+        return;
>     }
>
>     ret = vhost_vsock_set_guest_cid(vdev);
>@@ -234,6 +357,7 @@ err_vhost_dev:
> err_virtio:
>     vhost_vsock_common_unrealize(vdev);
> err_blocker:
>+    migration_remove_notifier(&vsock->cpr_notifier);
>     migrate_del_blocker(&vsock->migration_blocker);
> }
>
>@@ -250,6 +374,7 @@ static void vhost_vsock_device_unrealize(DeviceState *dev)
>     if (proxy->id) {
>         cpr_delete_fd(proxy->id, 0);
>     }
>+    migration_remove_notifier(&vsock->cpr_notifier);
>     migrate_del_blocker(&vsock->migration_blocker);
>     vhost_dev_cleanup(&vvc->vhost_dev);
>     vhost_vsock_common_unrealize(vdev);
>diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>index 7ae2abe33cd..3734e3f20b5 100644
>--- a/hw/virtio/vhost.c
>+++ b/hw/virtio/vhost.c
>@@ -1647,7 +1647,7 @@ fail_call:
> static void vhost_virtqueue_cleanup(struct vhost_virtqueue *vq)
> {
>     event_notifier_cleanup(&vq->masked_notifier);
>-    if (vq->dev->vhost_ops->vhost_set_vring_err) {
>+    if (vq->dev && vq->dev->vhost_ops->vhost_set_vring_err) {

Is this change related?

Oh yeah, I saw your comment in the commit description, thanks for that.
I'm just thinking if it makes sense to move to a preparation patch in 
this series.

Thanks,
Stefano

>         event_notifier_set_handler(&vq->error_notifier, NULL);
>         event_notifier_cleanup(&vq->error_notifier);
>     }
>diff --git a/include/hw/virtio/vhost-vsock.h b/include/hw/virtio/vhost-vsock.h
>index 5ebc63afc5a..6d0cff4fb93 100644
>--- a/include/hw/virtio/vhost-vsock.h
>+++ b/include/hw/virtio/vhost-vsock.h
>@@ -15,6 +15,7 @@
> #define QEMU_VHOST_VSOCK_H
>
> #include "hw/virtio/vhost-vsock-common.h"
>+#include "qemu/notify.h"
> #include "qom/object.h"
>
> #define TYPE_VHOST_VSOCK "vhost-vsock-device"
>@@ -30,6 +31,8 @@ struct VHostVSock {
>     VHostVSockCommon parent;
>     VHostVSockConf conf;
>     Error *migration_blocker;   /* set when the device has no ID */
>+    bool owner_reset;           /* CPR released ownership; needs re-acquire */
>+    NotifierWithReturn cpr_notifier;   /* re-acquires ownership if CPR fails */
>
>     /*< public >*/
> };
>-- 
>2.47.1
>

Re: [PATCH v3 0/7] migration/cpr: support vhost-vsock devices

[Stefano Garzarella]

On Tue, Aug 18, 2026 at 02:02:10PM +0300, Andrey Drobyshev wrote:
>On 8/11/26 5:12 PM, Andrey Drobyshev wrote:
>> On 6/30/26 9:35 PM, Maciej S. Szmigiero wrote:
>>> On 26.06.2026 18:46, Andrey Drobyshev wrote:
>>>> v2 -> v3:
>>>>
>>>> Re-design device ownership hand-off (suggested by Dongli).  Do not rely
>>>> on SETUP migration notifiers, as this approach forces us to reorder
>>>> generic migration code (see v2 discussion).  Instead:
>>>>
>>>>    * .pre_save() releases ownership on the source (RESET_OWNER) once VM
>>>>      is stopped;
>>>>    * .realize() is adjusted to defer device ownership acquisition for an
>>>>      incoming CPR;
>>>>    * .post_load() actually claims the device (VHOST_SET_OWNER);
>>>>    * FAILED migration event callback re-aquires the ownership on the
>>>>      source after re_save released it.
>>>>
>>>> v2: https://lore.kernel.org/qemu-devel/57c9c9b3-d758-489b-95b8-d16c258b9b0c@virtuozzo.com
>>>>
>>>> Andrey Drobyshev (7):
>>>>    vhost: add vhost_reset_owner op
>>>>    vhost-vsock: don't reset connections during CPR
>>>>    vhost-vsock: fix FD leak in realize()
>>>>    vhost-vsock: preserve vhost FD during CPR
>>>>    vhost: factor out vhost_dev_init_backend()
>>>>    vhost: add vhost_dev_set_owner() / vhost_dev_reset_owner() helpers
>>>>    vhost-vsock: hand off device ownership across CPR
>>>>
>>> These patches look okay CPR-wise, so:
>>> Acked-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com> # for CPR
>>>
>>> But the vsock parts still need review by their maintainers/reviewers.
>>>
>>> Thanks,
>>> Maciej
>>>
>>
>> FYI the kernel counterpart series adding VHOST_RESET_OWNER ioctl to
>> vsock ended up in Michael's vhost tree ([1], commits
>> 6236e765f16d^..c7bfb8815d83).
>>
>> Therefore - friendly ping for this series.
>>
>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git
>>
>> Andrey
>
>Another friendly ping
>

Sorry for the long time on my side, and thanks for pings :-)

I left some comments, but overall LGTM.
I'm not a CPR/migration expert, so my comments are more related to 
vhost/vsock.

Thanks,
Stefano

Re: [PATCH v3 4/7] vhost-vsock: preserve vhost FD during CPR

[Andrey Drobyshev]

On 8/18/26 6:29 PM, Stefano Garzarella wrote:
> On Fri, Jun 26, 2026 at 07:46:40PM +0300, Andrey Drobyshev wrote:
>> During CPR (checkpoint-restore) migration the guest keeps running on the
>> same host, so instead of reopening /dev/vhost-vsock on the destination,
>> we should reuse the FD from the source.  The FD is saved in the CPR
>> namespace (hash table) with cpr_save_fd() and then reclaimed on the
>> target via cpr_find_fd().
>>
>> Since the key in CPR hash table is device ID, CPR needs a unique ID.
>> Rather than make it mandatory for every vhost-vsock device, we add CPR
>> migration blocker which only fires once we attempt CPR with ID-less
>> vhost-vsock.
>>
>> vhost_dev_init() (and thus VHOST_SET_OWNER) still runs in realize() here.
>> Deferring the ownership handoff to pre_save/post_load is done in a
>> following patch.
> 
> So will this patch be bisectabale?
> 
> Thanks,
> Stefano
> 

The issue is that cpr-exec already is broken on the current master
branch (i.e. before the series is applied).  The only difference is
which exact error we fail with: -EBUSY or -EADDRINUSE.

In this case I don't think it's feasible to make each and every commit
bisectable - it only becomes fully working after the last one applied.
If you want, I can prepend the series with a migration blocker which
would fail early and gracefully instead of crashing, and lift it after
it's working.

Andrey

>>
>> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>> ---
>> hw/virtio/vhost-vsock.c         | 50 ++++++++++++++++++++++++++++++---
>> include/hw/virtio/vhost-vsock.h |  1 +
>> 2 files changed, 47 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
>> index fd7ffa88990..7eacb608d07 100644
>> --- a/hw/virtio/vhost-vsock.c
>> +++ b/hw/virtio/vhost-vsock.c
>> @@ -21,6 +21,8 @@
>> #include "hw/virtio/vhost-vsock.h"
>> #include "monitor/monitor.h"
>> #include "migration/cpr.h"
>> +#include "migration/blocker.h"
>> +#include "migration/misc.h"
>>
>> static void vhost_vsock_get_config(VirtIODevice *vdev, uint8_t *config)
>> {
>> @@ -141,6 +143,7 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
>>     VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(dev);
>>     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>>     VHostVSock *vsock = VHOST_VSOCK(dev);
>> +    DeviceState *proxy = qdev_get_parent_bus(DEVICE(vsock))->parent;
>>     int vhostfd;
>>     int ret;
>>
>> @@ -155,23 +158,49 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
>>         return;
>>     }
>>
>> -    if (vsock->conf.vhostfd) {
>> +    /*
>> +     * Having a unique ID is mandatory for FD preservation during CPR
>> +     * migration, thus we add migration blockers for CPR modes.
>> +     */
>> +    if (!proxy->id) {
>> +        error_setg(&vsock->migration_blocker,
>> +                   "vhost-vsock: device ID is required for CPR migration");
>> +        if (migrate_add_blocker_modes(&vsock->migration_blocker,
>> +                                      BIT(MIG_MODE_CPR_TRANSFER) |
>> +                                      BIT(MIG_MODE_CPR_EXEC), errp) < 0) {
>> +            return;
>> +        }
>> +    }
>> +
>> +    if (cpr_is_incoming()) {
>> +        /* Reuse the fd handed over from the source QEMU. */
>> +        if (!proxy->id) {
>> +            error_setg(errp, "vhost-vsock: device ID is required for "
>> +                       "CPR migration");
>> +            goto err_blocker;
>> +        }
>> +        vhostfd = cpr_find_fd(proxy->id, 0);
>> +        if (vhostfd < 0) {
>> +            error_setg(errp, "vhost-vsock: could not find restored vhost FD");
>> +            goto err_blocker;
>> +        }
>> +    } else if (vsock->conf.vhostfd) {
>>         vhostfd = monitor_fd_param(monitor_cur(), vsock->conf.vhostfd, errp);
>>         if (vhostfd == -1) {
>>             error_prepend(errp, "vhost-vsock: unable to parse vhostfd: ");
>> -            return;
>> +            goto err_blocker;
>>         }
>>     } else {
>>         vhostfd = open("/dev/vhost-vsock", O_RDWR);
>>         if (vhostfd < 0) {
>>             error_setg_file_open(errp, errno, "/dev/vhost-vsock");
>> -            return;
>> +            goto err_blocker;
>>         }
>>     }
>>
>>     if (!qemu_set_blocking(vhostfd, false, errp)) {
>>         close(vhostfd);
>> -        return;
>> +        goto err_blocker;
>>     }
>>
>>     vhost_vsock_common_realize(vdev);
>> @@ -192,6 +221,11 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
>>         goto err_vhost_dev;
>>     }
>>
>> +    /* Register the fd for a future CPR after a fully successful realize */
>> +    if (proxy->id) {
>> +        cpr_save_fd(proxy->id, 0, vhostfd);
>> +    }
>> +
>>     return;
>>
>> err_vhost_dev:
>> @@ -199,16 +233,24 @@ err_vhost_dev:
>>     vhost_dev_cleanup(&vvc->vhost_dev);
>> err_virtio:
>>     vhost_vsock_common_unrealize(vdev);
>> +err_blocker:
>> +    migrate_del_blocker(&vsock->migration_blocker);
>> }
>>
>> static void vhost_vsock_device_unrealize(DeviceState *dev)
>> {
>>     VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(dev);
>>     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>> +    VHostVSock *vsock = VHOST_VSOCK(dev);
>> +    DeviceState *proxy = qdev_get_parent_bus(dev)->parent;
>>
>>     /* This will stop vhost backend if appropriate. */
>>     vhost_vsock_set_status(vdev, 0);
>>
>> +    if (proxy->id) {
>> +        cpr_delete_fd(proxy->id, 0);
>> +    }
>> +    migrate_del_blocker(&vsock->migration_blocker);
>>     vhost_dev_cleanup(&vvc->vhost_dev);
>>     vhost_vsock_common_unrealize(vdev);
>> }
>> diff --git a/include/hw/virtio/vhost-vsock.h b/include/hw/virtio/vhost-vsock.h
>> index 84f4e727c70..5ebc63afc5a 100644
>> --- a/include/hw/virtio/vhost-vsock.h
>> +++ b/include/hw/virtio/vhost-vsock.h
>> @@ -29,6 +29,7 @@ struct VHostVSock {
>>     /*< private >*/
>>     VHostVSockCommon parent;
>>     VHostVSockConf conf;
>> +    Error *migration_blocker;   /* set when the device has no ID */
>>
>>     /*< public >*/
>> };
>> -- 
>> 2.47.1
>>
> 

Re: [PATCH v3 5/7] vhost: factor out vhost_dev_init_backend()

[Andrey Drobyshev]

On 8/18/26 6:29 PM, Stefano Garzarella wrote:
> On Fri, Jun 26, 2026 at 07:46:41PM +0300, Andrey Drobyshev wrote:
>> Split the first part of vhost_dev_init(): selecting the backend, calling
>> its .vhost_init() and reading the supported features - into a new
>> vhost_dev_init_backend() helper, and call it from vhost_dev_init().
>>
>> This is in preparation for CPR restore of vhost-vsock, which needs to learn
>> the backend's features at realize time to negotiate them when loading the
>> incoming virtio state, but also must defer taking ownership of the device
>> to post_load.  vhost_dev_init_backend() does exactly the pre-ownership part.
>>
>> As a result VHOST_SET_OWNER now follows the feature query rather than
>> precedes it.  This should be safe, as no backend requires ownership before
>> VHOST_GET_FEATURES - the kernel and vdpa backends do not check ownership
>> for it, and vhost-user already does query features from its .vhost_init()
>> before set_owner().
>>
>> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>> ---
>> hw/virtio/vhost.c         | 33 +++++++++++++++++++++++----------
>> include/hw/virtio/vhost.h | 19 +++++++++++++++++++
>> 2 files changed, 42 insertions(+), 10 deletions(-)
>>
>> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>> index af41841b529..f11588cc51a 100644
>> --- a/hw/virtio/vhost.c
>> +++ b/hw/virtio/vhost.c
>> @@ -1667,6 +1667,28 @@ static int vhost_dev_init_features(struct vhost_dev *hdev)
>>     return r;
>> }
>>
>> +int vhost_dev_init_backend(struct vhost_dev *hdev, void *opaque,
>> +                           VhostBackendType backend_type, Error **errp)
>> +{
>> +    int r;
>> +
>> +    r = vhost_set_backend_type(hdev, backend_type);
>> +    assert(r >= 0);
>> +
>> +    r = hdev->vhost_ops->vhost_init(hdev, opaque, errp);
>> +    if (r < 0) {
>> +        return r;
>> +    }
>> +
>> +    r = vhost_dev_init_features(hdev);
>> +    if (r < 0) {
>> +        error_setg_errno(errp, -r, "vhost_init_features failed");
>> +        return r;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
>>                    VhostBackendType backend_type, uint32_t busyloop_timeout,
>>                    Error **errp)
>> @@ -1679,10 +1701,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
>>     hdev->vdev = NULL;
>>     hdev->migration_blocker = NULL;
>>
>> -    r = vhost_set_backend_type(hdev, backend_type);
>> -    assert(r >= 0);
>> -
>> -    r = hdev->vhost_ops->vhost_init(hdev, opaque, errp);
>> +    r = vhost_dev_init_backend(hdev, opaque, backend_type, errp);
>>     if (r < 0) {
>>         goto fail;
>>     }
>> @@ -1693,12 +1712,6 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
>>         goto fail;
>>     }
>>
>> -    r = vhost_dev_init_features(hdev);
>> -    if (r < 0) {
>> -        error_setg_errno(errp, -r, "vhost_init_features failed");
>> -        goto fail;
>> -    }
>> -
>>     limit = hdev->vhost_ops->vhost_memslots_limit(hdev);
>>     if (limit < MEMORY_DEVICES_SAFE_MAX_MEMSLOTS &&
>>         memory_devices_memslot_auto_decision_active()) {
>> diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
>> index 684bafcaadd..bc81e09663e 100644
>> --- a/include/hw/virtio/vhost.h
>> +++ b/include/hw/virtio/vhost.h
>> @@ -156,6 +156,25 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
>>                    VhostBackendType backend_type,
>>                    uint32_t busyloop_timeout, Error **errp);
>>
>> +/**
>> + * vhost_dev_init_backend() - set up the backend and query its features
>> + * @hdev: the common vhost_dev structure
>> + * @opaque: opaque ptr passed to backend (vhost/vhost-user/vdpa)
>> + * @backend_type: type of backend
>> + * @errp: error handle
>> + *
>> + * Select the backend, initialise the backend instance and read its supported
>> + * features into @hdev, without issuing VHOST_SET_OWNER, setting up the
>> + * virtqueues or registering the memory listener.  This is the part of
>> + * vhost_dev_init() that precedes taking ownership; it can be used on its own
>> + * so feature negotiation can happen before ownership is acquired (e.g. by CPR
>> + * restore).
> 
> Should we mention that on error `vhost_dev_cleanup()` must be called, or 
> maybe should we call it in the new function if vhost_dev_init_features() 
> fails?
> 
> Thanks,
> Stefano

Good catch, thanks.  In the entire series we have 2 callers for
vhost_dev_init_backend(): vhost_dev_init_backend() in this patch and
vhost_vsock_device_realize() in the last patch.  Both of the callers do
'goto err' where vhost_dev_cleanup() is called.  So let's indeed mention
in the doc that cleanup responsibility lies on the caller, as our
callers already comply.

Andrey

> 
>> + *
>> + * Return: 0 on success, non-zero on error while setting errp.
>> + */
>> +int vhost_dev_init_backend(struct vhost_dev *hdev, void *opaque,
>> +                           VhostBackendType backend_type, Error **errp);
>> +
>> /**
>>  * vhost_dev_cleanup() - tear down and cleanup vhost interface
>>  * @hdev: the common vhost_dev structure
>> -- 
>> 2.47.1
>>
> 

Re: [PATCH v3 7/7] vhost-vsock: hand off device ownership across CPR

[Andrey Drobyshev]

On 8/18/26 6:30 PM, Stefano Garzarella wrote:
> On Fri, Jun 26, 2026 at 07:46:43PM +0300, Andrey Drobyshev wrote:
>> The previous patches reuse the source vhost FD on the destination,
>> however both source and targe still call vhost_dev_init() (VHOST_SET_OWNER)
>> in realize().  For cpr-transfer the destination realizes while the source
>> still owns the shared FD, so its SET_OWNER would fail - ownership has to be
>> handed over explicitly.
>>
>> Do this through the device's CPR vmstate hooks.  Namely, release device
>> ownership in pre_save, reclaim in post_load, re-acquire on failure:
>>
>>  - .pre_save() releases ownership on the source (VHOST_RESET_OWNER) once
>>    the VM is stopped, for the FD-preserving CPR modes (cpr-transfer and
>>    cpr-exec).
>>
>>  - .realize(), for an incoming CPR, only sets up the virtio device and
>>    queries the backend features by calling vhost_dev_init_backend().
>>    It doesn't take device ownership and doesn't touch the VQs which
>>    still-running source might use.  The full init is deferred to
>>    .post_load().
>>
>>  - .post_load() reclaims it on the destination: the full vhost_dev_init()
>>    (VHOST_SET_OWNER) on the preserved FD, plus sets the guest cid, before
>>    the device is started at vm_start.
>>
>>  - A MIG_EVENT_FAILED notifier re-acquires ownership if the migration
>>    fails after pre_save released it and the source VM is resumed.
>>
>> Also harden vhost_virtqueue_cleanup() against a NULL vq->dev which can
>> now happen if an incoming CPR is aborted after realize set up the backend
>> but before post_load initialised the VQs.
>>
>> Suggested-by: Dongli Zhang <dongli.zhang@oracle.com>
>> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>> ---
>> hw/virtio/vhost-vsock.c         | 147 +++++++++++++++++++++++++++++---
>> hw/virtio/vhost.c               |   2 +-
>> include/hw/virtio/vhost-vsock.h |   3 +
>> 3 files changed, 140 insertions(+), 12 deletions(-)
>>
>> diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
>> index 7eacb608d07..b02b3f9cc03 100644
>> --- a/hw/virtio/vhost-vsock.c
>> +++ b/hw/virtio/vhost-vsock.c
>> @@ -76,6 +76,17 @@ static int vhost_vsock_set_status(VirtIODevice *vdev, uint8_t status)
>>     bool should_start = virtio_device_should_start(vdev, status);
>>     int ret;
>>
>> +    /*
>> +     * On an incoming CPR the full vhost_dev_init() is deferred to post_load
>> +     * (realize only ran vhost_dev_init_backend()).  hdev->mem is set only by
>> +     * the full init, so refuse to start a device whose handoff never
>> +     * completed rather than dereference a half-initialised vhost_dev.
>> +     */
>> +    if (should_start && !vvc->vhost_dev.mem) {
> 
> Should we also check `vsock->owner_reset`?
> IIUC it can be left set if for example vhost_dev_set_owner() failed.
>

You're right, we should.
> About using `vvc->vhost_dev.mem`, I'm a bit worried if it can be a bit 
> fragile for future changes.  What about adding a new field (e.g.  
> `initialized`) set by vhost_dev_init() when everything is fine?
>

Agreed, let's add 'bool initialized' field.
>> +        error_report("vhost-vsock: refusing to start, device init incomplete");
>> +        return 0;
>> +    }
>> +
>>     if (vhost_dev_is_started(&vvc->vhost_dev) == should_start) {
>>         return 0;
>>     }
>> @@ -111,18 +122,102 @@ static uint64_t vhost_vsock_get_features(VirtIODevice *vdev,
>>     return vhost_vsock_common_get_features(vdev, requested_features, errp);
>> }
>>
>> +/*
>> + * Re-acquire device ownership if a CPR migration that released it (in
>> + * vhost_vsock_cpr_pre_save()) failed and the source VM is about to resume.
>> + * This runs before vm_start(), so the device is owned again before it is
>> + * restarted.
>> + */
>> +static int vhost_vsock_cpr_notifier(NotifierWithReturn *notifier,
>> +                                    MigrationEvent *e, Error **errp)
>> +{
>> +    VHostVSock *vsock = container_of(notifier, VHostVSock, cpr_notifier);
>> +    VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(vsock);
>> +    int ret;
>> +
>> +    if (e->type == MIG_EVENT_FAILED && vsock->owner_reset) {
>> +        ret = vhost_dev_set_owner(&vvc->vhost_dev);
>> +        if (ret < 0) {
>> +            error_report("vhost-vsock: failed to re-acquire owner: %d", ret);
>> +        } else {
>> +            vsock->owner_reset = false;
>> +        }
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int vhost_vsock_pre_save(void *opaque)
>> +{
>> +    VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(opaque);
>> +    VHostVSock *vsock = VHOST_VSOCK(opaque);
>> +    int ret;
>> +
>> +    ret = vhost_vsock_common_pre_save(opaque);
>> +    if (ret) {
>> +        return ret;
>> +    }
>> +
>> +    /*
>> +     * Release the device ownership now for CPR migration.  The device is
>> +     * already stopped at pre_save, and destination reclaims it by calling
>> +     * VHOST_SET_OWNER in post_load.
>> +     */
>> +    if (cpr_incoming_needed(NULL)) {
>> +        ret = vhost_dev_reset_owner(&vvc->vhost_dev);
>> +        if (ret < 0) {
>> +            error_report("vhost-vsock: vhost_reset_owner failed: %d", ret);
>> +            return ret;
>> +        }
>> +        vsock->owner_reset = true;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> static int vhost_vsock_post_load(void *opaque, int version_id)
>> {
>> +    VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(opaque);
>> +    VirtIODevice *vdev = VIRTIO_DEVICE(opaque);
>> +    DeviceState *proxy = qdev_get_parent_bus(DEVICE(vdev))->parent;
>> +    Error *local_err = NULL;
>> +    int vhostfd, ret;
>> +
>>     /*
>>      * Only reset vsock connections for non-CPR migration.  For CPR the
>>      * guest cid is unchanged, and the cid-change reset would otherwise
>>      * tear the vsock connections down.
>>      */
>> -    if (cpr_is_incoming()) {
>> -        return 0;
>> +    if (!cpr_is_incoming()) {
>> +        return vhost_vsock_common_post_load(opaque, version_id);
>> +    }
> 
> nit: maybe we can do this in the patch where we introduced it.
>

Agreed, I'll move it to patch 2.
>> +
>> +    /*
>> +     * CPR restore case.  The source released device ownership in its
>> +     * pre_save.  Complete the handoff here, before the device is started
>> +     * at vm_start.  Init vhost device on preserved FD, issue
>> +     * VHOST_SET_OWNER on it, and restore the guest cid.
>> +     */
>> +    vhostfd = cpr_find_fd(proxy->id, 0);
>> +    if (vhostfd < 0) {
>> +        error_report("vhost-vsock: could not find restored vhost FD");
>> +        return -1;
>>     }
>>
>> -    return vhost_vsock_common_post_load(opaque, version_id);
>> +    ret = vhost_dev_init(&vvc->vhost_dev, (void *)(uintptr_t)vhostfd,
>> +                         VHOST_BACKEND_TYPE_KERNEL, 0, &local_err);
>> +    if (ret < 0) {
>> +        error_report_err(local_err);
>> +        return ret;
>> +    }
>> +
>> +    ret = vhost_vsock_set_guest_cid(vdev);
>> +    if (ret < 0) {
> 
> Should we call vhost_dev_cleanup() here?
>

Yes, we should, thank you.
>> +        error_report("vhost-vsock: unable to set guest cid: %d", ret);
>> +        return ret;
>> +    }
>> +
>> +    return 0;
>> }
>>
>> static const VMStateDescription vmstate_virtio_vhost_vsock = {
>> @@ -133,7 +228,7 @@ static const VMStateDescription vmstate_virtio_vhost_vsock = {
>>         VMSTATE_VIRTIO_DEVICE,
>>         VMSTATE_END_OF_LIST()
>>     },
>> -    .pre_save = vhost_vsock_common_pre_save,
>> +    .pre_save = vhost_vsock_pre_save,
>>     .post_load = vhost_vsock_post_load,
>> };
>>
>> @@ -144,6 +239,7 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
>>     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>>     VHostVSock *vsock = VHOST_VSOCK(dev);
>>     DeviceState *proxy = qdev_get_parent_bus(DEVICE(vsock))->parent;
>> +    bool cpr_incoming = cpr_is_incoming();
>>     int vhostfd;
>>     int ret;
>>
>> @@ -172,7 +268,16 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
>>         }
>>     }
>>
>> -    if (cpr_is_incoming()) {
>> +    /*
>> +     * Re-acquire ownership if a CPR migration releases it (in pre_save) but
>> +     * then fails.
>> +     */
>> +    migration_add_notifier_modes(&vsock->cpr_notifier,
>> +                                 vhost_vsock_cpr_notifier,
>> +                                 BIT(MIG_MODE_CPR_TRANSFER) |
>> +                                 BIT(MIG_MODE_CPR_EXEC));
>> +
>> +    if (cpr_incoming) {
>>         /* Reuse the fd handed over from the source QEMU. */
>>         if (!proxy->id) {
>>             error_setg(errp, "vhost-vsock: device ID is required for "
>> @@ -205,14 +310,32 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
>>
>>     vhost_vsock_common_realize(vdev);
>>
>> -    ret = vhost_dev_init(&vvc->vhost_dev, (void *)(uintptr_t)vhostfd,
>> -                         VHOST_BACKEND_TYPE_KERNEL, 0, errp);
>> -    if (ret < 0) {
>> +    if (!cpr_incoming) {
>> +        ret = vhost_dev_init(&vvc->vhost_dev, (void *)(uintptr_t)vhostfd,
>> +                             VHOST_BACKEND_TYPE_KERNEL, 0, errp);
>> +        if (ret < 0) {
>> +            /*
>> +             * vhostfd is closed by vhost_dev_cleanup, which is called
>> +             * by vhost_dev_init on initialization error.
>> +             */
>> +            goto err_virtio;
>> +        }
>> +    } else {
>>         /*
>> -         * vhostfd is closed by vhost_dev_cleanup, which is called
>> -         * by vhost_dev_init on initialization error.
>> +         * CPR restore case: only learn the backend feature set now, but
>> +         * defer taking ownership or touching VQs (the still-running source
>> +         * might be using them).  The full vhost_dev_init()/VHOST_SET_OWNER
>> +         * is done later in post_load.
>>          */
>> -        goto err_virtio;
>> +        ret = vhost_dev_init_backend(&vvc->vhost_dev,
>> +                                     (void *)(uintptr_t)vhostfd,
>> +                                     VHOST_BACKEND_TYPE_KERNEL, errp);
>> +        if (ret < 0) {
>> +            /* vhost_dev_init_backend() does not close the fd on error */
>> +            goto err_vhost_dev;
>> +        }
>> +
>> +        return;
>>     }
>>
>>     ret = vhost_vsock_set_guest_cid(vdev);
>> @@ -234,6 +357,7 @@ err_vhost_dev:
>> err_virtio:
>>     vhost_vsock_common_unrealize(vdev);
>> err_blocker:
>> +    migration_remove_notifier(&vsock->cpr_notifier);
>>     migrate_del_blocker(&vsock->migration_blocker);
>> }
>>
>> @@ -250,6 +374,7 @@ static void vhost_vsock_device_unrealize(DeviceState *dev)
>>     if (proxy->id) {
>>         cpr_delete_fd(proxy->id, 0);
>>     }
>> +    migration_remove_notifier(&vsock->cpr_notifier);
>>     migrate_del_blocker(&vsock->migration_blocker);
>>     vhost_dev_cleanup(&vvc->vhost_dev);
>>     vhost_vsock_common_unrealize(vdev);
>> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>> index 7ae2abe33cd..3734e3f20b5 100644
>> --- a/hw/virtio/vhost.c
>> +++ b/hw/virtio/vhost.c
>> @@ -1647,7 +1647,7 @@ fail_call:
>> static void vhost_virtqueue_cleanup(struct vhost_virtqueue *vq)
>> {
>>     event_notifier_cleanup(&vq->masked_notifier);
>> -    if (vq->dev->vhost_ops->vhost_set_vring_err) {
>> +    if (vq->dev && vq->dev->vhost_ops->vhost_set_vring_err) {
> 
> Is this change related?
> 
> Oh yeah, I saw your comment in the commit description, thanks for that.
> I'm just thinking if it makes sense to move to a preparation patch in 
> this series.
>

Sure, let's commit it separately.

Andrey
> Thanks,
> Stefano
> 
>>         event_notifier_set_handler(&vq->error_notifier, NULL);
>>         event_notifier_cleanup(&vq->error_notifier);
>>     }
>> diff --git a/include/hw/virtio/vhost-vsock.h b/include/hw/virtio/vhost-vsock.h
>> index 5ebc63afc5a..6d0cff4fb93 100644
>> --- a/include/hw/virtio/vhost-vsock.h
>> +++ b/include/hw/virtio/vhost-vsock.h
>> @@ -15,6 +15,7 @@
>> #define QEMU_VHOST_VSOCK_H
>>
>> #include "hw/virtio/vhost-vsock-common.h"
>> +#include "qemu/notify.h"
>> #include "qom/object.h"
>>
>> #define TYPE_VHOST_VSOCK "vhost-vsock-device"
>> @@ -30,6 +31,8 @@ struct VHostVSock {
>>     VHostVSockCommon parent;
>>     VHostVSockConf conf;
>>     Error *migration_blocker;   /* set when the device has no ID */
>> +    bool owner_reset;           /* CPR released ownership; needs re-acquire */
>> +    NotifierWithReturn cpr_notifier;   /* re-acquires ownership if CPR fails */
>>
>>     /*< public >*/
>> };
>> -- 
>> 2.47.1
>>
> 

Re: [PATCH v3 0/7] migration/cpr: support vhost-vsock devices

[Andrey Drobyshev]

On 8/18/26 6:33 PM, Stefano Garzarella wrote:
> On Tue, Aug 18, 2026 at 02:02:10PM +0300, Andrey Drobyshev wrote:
>> On 8/11/26 5:12 PM, Andrey Drobyshev wrote:
>>> On 6/30/26 9:35 PM, Maciej S. Szmigiero wrote:
>>>> On 26.06.2026 18:46, Andrey Drobyshev wrote:
>>>>> v2 -> v3:
>>>>>
>>>>> Re-design device ownership hand-off (suggested by Dongli).  Do not rely
>>>>> on SETUP migration notifiers, as this approach forces us to reorder
>>>>> generic migration code (see v2 discussion).  Instead:
>>>>>
>>>>>    * .pre_save() releases ownership on the source (RESET_OWNER) once VM
>>>>>      is stopped;
>>>>>    * .realize() is adjusted to defer device ownership acquisition for an
>>>>>      incoming CPR;
>>>>>    * .post_load() actually claims the device (VHOST_SET_OWNER);
>>>>>    * FAILED migration event callback re-aquires the ownership on the
>>>>>      source after re_save released it.
>>>>>
>>>>> v2: https://lore.kernel.org/qemu-devel/57c9c9b3-d758-489b-95b8-d16c258b9b0c@virtuozzo.com
>>>>>
>>>>> Andrey Drobyshev (7):
>>>>>    vhost: add vhost_reset_owner op
>>>>>    vhost-vsock: don't reset connections during CPR
>>>>>    vhost-vsock: fix FD leak in realize()
>>>>>    vhost-vsock: preserve vhost FD during CPR
>>>>>    vhost: factor out vhost_dev_init_backend()
>>>>>    vhost: add vhost_dev_set_owner() / vhost_dev_reset_owner() helpers
>>>>>    vhost-vsock: hand off device ownership across CPR
>>>>>
>>>> These patches look okay CPR-wise, so:
>>>> Acked-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com> # for CPR
>>>>
>>>> But the vsock parts still need review by their maintainers/reviewers.
>>>>
>>>> Thanks,
>>>> Maciej
>>>>
>>>
>>> FYI the kernel counterpart series adding VHOST_RESET_OWNER ioctl to
>>> vsock ended up in Michael's vhost tree ([1], commits
>>> 6236e765f16d^..c7bfb8815d83).
>>>
>>> Therefore - friendly ping for this series.
>>>
>>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git
>>>
>>> Andrey
>>
>> Another friendly ping
>>
> 
> Sorry for the long time on my side, and thanks for pings :-)
> 
> I left some comments, but overall LGTM.
> I'm not a CPR/migration expert, so my comments are more related to 
> vhost/vsock.
> 
> Thanks,
> Stefano
> 

Thanks for the review, most of the issues you commented on deserve
fixing.  I'll send an adjusted v4.

Thanks,
Andrey

Re: [PATCH v3 4/7] vhost-vsock: preserve vhost FD during CPR

[Michael S. Tsirkin]

On Wed, Aug 19, 2026 at 05:38:46PM +0300, Andrey Drobyshev wrote:
> On 8/18/26 6:29 PM, Stefano Garzarella wrote:
> > On Fri, Jun 26, 2026 at 07:46:40PM +0300, Andrey Drobyshev wrote:
> >> During CPR (checkpoint-restore) migration the guest keeps running on the
> >> same host, so instead of reopening /dev/vhost-vsock on the destination,
> >> we should reuse the FD from the source.  The FD is saved in the CPR
> >> namespace (hash table) with cpr_save_fd() and then reclaimed on the
> >> target via cpr_find_fd().
> >>
> >> Since the key in CPR hash table is device ID, CPR needs a unique ID.
> >> Rather than make it mandatory for every vhost-vsock device, we add CPR
> >> migration blocker which only fires once we attempt CPR with ID-less
> >> vhost-vsock.
> >>
> >> vhost_dev_init() (and thus VHOST_SET_OWNER) still runs in realize() here.
> >> Deferring the ownership handoff to pre_save/post_load is done in a
> >> following patch.
> > 
> > So will this patch be bisectabale?
> > 
> > Thanks,
> > Stefano
> > 
> 
> The issue is that cpr-exec already is broken on the current master
> branch (i.e. before the series is applied).  The only difference is
> which exact error we fail with: -EBUSY or -EADDRINUSE.
> 
> In this case I don't think it's feasible to make each and every commit
> bisectable - it only becomes fully working after the last one applied.

Surely just bypassing cpr things until the last commit
is feasible?

> If you want, I can prepend the series with a migration blocker which
> would fail early and gracefully instead of crashing, and lift it after
> it's working.
> 
> Andrey
> 
> >>
> >> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> >> ---
> >> hw/virtio/vhost-vsock.c         | 50 ++++++++++++++++++++++++++++++---
> >> include/hw/virtio/vhost-vsock.h |  1 +
> >> 2 files changed, 47 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
> >> index fd7ffa88990..7eacb608d07 100644
> >> --- a/hw/virtio/vhost-vsock.c
> >> +++ b/hw/virtio/vhost-vsock.c
> >> @@ -21,6 +21,8 @@
> >> #include "hw/virtio/vhost-vsock.h"
> >> #include "monitor/monitor.h"
> >> #include "migration/cpr.h"
> >> +#include "migration/blocker.h"
> >> +#include "migration/misc.h"
> >>
> >> static void vhost_vsock_get_config(VirtIODevice *vdev, uint8_t *config)
> >> {
> >> @@ -141,6 +143,7 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
> >>     VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(dev);
> >>     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> >>     VHostVSock *vsock = VHOST_VSOCK(dev);
> >> +    DeviceState *proxy = qdev_get_parent_bus(DEVICE(vsock))->parent;
> >>     int vhostfd;
> >>     int ret;
> >>
> >> @@ -155,23 +158,49 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
> >>         return;
> >>     }
> >>
> >> -    if (vsock->conf.vhostfd) {
> >> +    /*
> >> +     * Having a unique ID is mandatory for FD preservation during CPR
> >> +     * migration, thus we add migration blockers for CPR modes.
> >> +     */
> >> +    if (!proxy->id) {
> >> +        error_setg(&vsock->migration_blocker,
> >> +                   "vhost-vsock: device ID is required for CPR migration");
> >> +        if (migrate_add_blocker_modes(&vsock->migration_blocker,
> >> +                                      BIT(MIG_MODE_CPR_TRANSFER) |
> >> +                                      BIT(MIG_MODE_CPR_EXEC), errp) < 0) {
> >> +            return;
> >> +        }
> >> +    }
> >> +
> >> +    if (cpr_is_incoming()) {
> >> +        /* Reuse the fd handed over from the source QEMU. */
> >> +        if (!proxy->id) {
> >> +            error_setg(errp, "vhost-vsock: device ID is required for "
> >> +                       "CPR migration");
> >> +            goto err_blocker;
> >> +        }
> >> +        vhostfd = cpr_find_fd(proxy->id, 0);
> >> +        if (vhostfd < 0) {
> >> +            error_setg(errp, "vhost-vsock: could not find restored vhost FD");
> >> +            goto err_blocker;
> >> +        }
> >> +    } else if (vsock->conf.vhostfd) {
> >>         vhostfd = monitor_fd_param(monitor_cur(), vsock->conf.vhostfd, errp);
> >>         if (vhostfd == -1) {
> >>             error_prepend(errp, "vhost-vsock: unable to parse vhostfd: ");
> >> -            return;
> >> +            goto err_blocker;
> >>         }
> >>     } else {
> >>         vhostfd = open("/dev/vhost-vsock", O_RDWR);
> >>         if (vhostfd < 0) {
> >>             error_setg_file_open(errp, errno, "/dev/vhost-vsock");
> >> -            return;
> >> +            goto err_blocker;
> >>         }
> >>     }
> >>
> >>     if (!qemu_set_blocking(vhostfd, false, errp)) {
> >>         close(vhostfd);
> >> -        return;
> >> +        goto err_blocker;
> >>     }
> >>
> >>     vhost_vsock_common_realize(vdev);
> >> @@ -192,6 +221,11 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
> >>         goto err_vhost_dev;
> >>     }
> >>
> >> +    /* Register the fd for a future CPR after a fully successful realize */
> >> +    if (proxy->id) {
> >> +        cpr_save_fd(proxy->id, 0, vhostfd);
> >> +    }
> >> +
> >>     return;
> >>
> >> err_vhost_dev:
> >> @@ -199,16 +233,24 @@ err_vhost_dev:
> >>     vhost_dev_cleanup(&vvc->vhost_dev);
> >> err_virtio:
> >>     vhost_vsock_common_unrealize(vdev);
> >> +err_blocker:
> >> +    migrate_del_blocker(&vsock->migration_blocker);
> >> }
> >>
> >> static void vhost_vsock_device_unrealize(DeviceState *dev)
> >> {
> >>     VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(dev);
> >>     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> >> +    VHostVSock *vsock = VHOST_VSOCK(dev);
> >> +    DeviceState *proxy = qdev_get_parent_bus(dev)->parent;
> >>
> >>     /* This will stop vhost backend if appropriate. */
> >>     vhost_vsock_set_status(vdev, 0);
> >>
> >> +    if (proxy->id) {
> >> +        cpr_delete_fd(proxy->id, 0);
> >> +    }
> >> +    migrate_del_blocker(&vsock->migration_blocker);
> >>     vhost_dev_cleanup(&vvc->vhost_dev);
> >>     vhost_vsock_common_unrealize(vdev);
> >> }
> >> diff --git a/include/hw/virtio/vhost-vsock.h b/include/hw/virtio/vhost-vsock.h
> >> index 84f4e727c70..5ebc63afc5a 100644
> >> --- a/include/hw/virtio/vhost-vsock.h
> >> +++ b/include/hw/virtio/vhost-vsock.h
> >> @@ -29,6 +29,7 @@ struct VHostVSock {
> >>     /*< private >*/
> >>     VHostVSockCommon parent;
> >>     VHostVSockConf conf;
> >> +    Error *migration_blocker;   /* set when the device has no ID */
> >>
> >>     /*< public >*/
> >> };
> >> -- 
> >> 2.47.1
> >>
> > 

Re: [PATCH v3 4/7] vhost-vsock: preserve vhost FD during CPR

[Andrey Drobyshev]

On 8/19/26 6:11 PM, Michael S. Tsirkin wrote:
> On Wed, Aug 19, 2026 at 05:38:46PM +0300, Andrey Drobyshev wrote:
>> On 8/18/26 6:29 PM, Stefano Garzarella wrote:
>>> On Fri, Jun 26, 2026 at 07:46:40PM +0300, Andrey Drobyshev wrote:
>>>> During CPR (checkpoint-restore) migration the guest keeps running on the
>>>> same host, so instead of reopening /dev/vhost-vsock on the destination,
>>>> we should reuse the FD from the source.  The FD is saved in the CPR
>>>> namespace (hash table) with cpr_save_fd() and then reclaimed on the
>>>> target via cpr_find_fd().
>>>>
>>>> Since the key in CPR hash table is device ID, CPR needs a unique ID.
>>>> Rather than make it mandatory for every vhost-vsock device, we add CPR
>>>> migration blocker which only fires once we attempt CPR with ID-less
>>>> vhost-vsock.
>>>>
>>>> vhost_dev_init() (and thus VHOST_SET_OWNER) still runs in realize() here.
>>>> Deferring the ownership handoff to pre_save/post_load is done in a
>>>> following patch.
>>>
>>> So will this patch be bisectabale?
>>>
>>> Thanks,
>>> Stefano
>>>
>>
>> The issue is that cpr-exec already is broken on the current master
>> branch (i.e. before the series is applied).  The only difference is
>> which exact error we fail with: -EBUSY or -EADDRINUSE.
>>
>> In this case I don't think it's feasible to make each and every commit
>> bisectable - it only becomes fully working after the last one applied.
> 
> Surely just bypassing cpr things until the last commit
> is feasible?
>

Sure, that's what I meant suggesting the migration blocker.  Currently
we add the blocker only on 'if (!proxy->id)' condition.  I suggest
adding an unconditional blocker for CPR_TRANSFER|CPR_EXEC before the
series (new patch 1), and then only make it conditional in the last
patch.  That way each patch of the series would fail early on that
blocker instead of crashing QEMU.
>> If you want, I can prepend the series with a migration blocker which
>> would fail early and gracefully instead of crashing, and lift it after
>> it's working.
>>
>> Andrey
>>
>>>>
>>>> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>>>> ---
>>>> hw/virtio/vhost-vsock.c         | 50 ++++++++++++++++++++++++++++++---
>>>> include/hw/virtio/vhost-vsock.h |  1 +
>>>> 2 files changed, 47 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
>>>> index fd7ffa88990..7eacb608d07 100644
>>>> --- a/hw/virtio/vhost-vsock.c
>>>> +++ b/hw/virtio/vhost-vsock.c
>>>> @@ -21,6 +21,8 @@
>>>> #include "hw/virtio/vhost-vsock.h"
>>>> #include "monitor/monitor.h"
>>>> #include "migration/cpr.h"
>>>> +#include "migration/blocker.h"
>>>> +#include "migration/misc.h"
>>>>
>>>> static void vhost_vsock_get_config(VirtIODevice *vdev, uint8_t *config)
>>>> {
>>>> @@ -141,6 +143,7 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
>>>>     VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(dev);
>>>>     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>>>>     VHostVSock *vsock = VHOST_VSOCK(dev);
>>>> +    DeviceState *proxy = qdev_get_parent_bus(DEVICE(vsock))->parent;
>>>>     int vhostfd;
>>>>     int ret;
>>>>
>>>> @@ -155,23 +158,49 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
>>>>         return;
>>>>     }
>>>>
>>>> -    if (vsock->conf.vhostfd) {
>>>> +    /*
>>>> +     * Having a unique ID is mandatory for FD preservation during CPR
>>>> +     * migration, thus we add migration blockers for CPR modes.
>>>> +     */
>>>> +    if (!proxy->id) {
>>>> +        error_setg(&vsock->migration_blocker,
>>>> +                   "vhost-vsock: device ID is required for CPR migration");
>>>> +        if (migrate_add_blocker_modes(&vsock->migration_blocker,
>>>> +                                      BIT(MIG_MODE_CPR_TRANSFER) |
>>>> +                                      BIT(MIG_MODE_CPR_EXEC), errp) < 0) {
>>>> +            return;
>>>> +        }
>>>> +    }
>>>> +
>>>> +    if (cpr_is_incoming()) {
>>>> +        /* Reuse the fd handed over from the source QEMU. */
>>>> +        if (!proxy->id) {
>>>> +            error_setg(errp, "vhost-vsock: device ID is required for "
>>>> +                       "CPR migration");
>>>> +            goto err_blocker;
>>>> +        }
>>>> +        vhostfd = cpr_find_fd(proxy->id, 0);
>>>> +        if (vhostfd < 0) {
>>>> +            error_setg(errp, "vhost-vsock: could not find restored vhost FD");
>>>> +            goto err_blocker;
>>>> +        }
>>>> +    } else if (vsock->conf.vhostfd) {
>>>>         vhostfd = monitor_fd_param(monitor_cur(), vsock->conf.vhostfd, errp);
>>>>         if (vhostfd == -1) {
>>>>             error_prepend(errp, "vhost-vsock: unable to parse vhostfd: ");
>>>> -            return;
>>>> +            goto err_blocker;
>>>>         }
>>>>     } else {
>>>>         vhostfd = open("/dev/vhost-vsock", O_RDWR);
>>>>         if (vhostfd < 0) {
>>>>             error_setg_file_open(errp, errno, "/dev/vhost-vsock");
>>>> -            return;
>>>> +            goto err_blocker;
>>>>         }
>>>>     }
>>>>
>>>>     if (!qemu_set_blocking(vhostfd, false, errp)) {
>>>>         close(vhostfd);
>>>> -        return;
>>>> +        goto err_blocker;
>>>>     }
>>>>
>>>>     vhost_vsock_common_realize(vdev);
>>>> @@ -192,6 +221,11 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
>>>>         goto err_vhost_dev;
>>>>     }
>>>>
>>>> +    /* Register the fd for a future CPR after a fully successful realize */
>>>> +    if (proxy->id) {
>>>> +        cpr_save_fd(proxy->id, 0, vhostfd);
>>>> +    }
>>>> +
>>>>     return;
>>>>
>>>> err_vhost_dev:
>>>> @@ -199,16 +233,24 @@ err_vhost_dev:
>>>>     vhost_dev_cleanup(&vvc->vhost_dev);
>>>> err_virtio:
>>>>     vhost_vsock_common_unrealize(vdev);
>>>> +err_blocker:
>>>> +    migrate_del_blocker(&vsock->migration_blocker);
>>>> }
>>>>
>>>> static void vhost_vsock_device_unrealize(DeviceState *dev)
>>>> {
>>>>     VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(dev);
>>>>     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>>>> +    VHostVSock *vsock = VHOST_VSOCK(dev);
>>>> +    DeviceState *proxy = qdev_get_parent_bus(dev)->parent;
>>>>
>>>>     /* This will stop vhost backend if appropriate. */
>>>>     vhost_vsock_set_status(vdev, 0);
>>>>
>>>> +    if (proxy->id) {
>>>> +        cpr_delete_fd(proxy->id, 0);
>>>> +    }
>>>> +    migrate_del_blocker(&vsock->migration_blocker);
>>>>     vhost_dev_cleanup(&vvc->vhost_dev);
>>>>     vhost_vsock_common_unrealize(vdev);
>>>> }
>>>> diff --git a/include/hw/virtio/vhost-vsock.h b/include/hw/virtio/vhost-vsock.h
>>>> index 84f4e727c70..5ebc63afc5a 100644
>>>> --- a/include/hw/virtio/vhost-vsock.h
>>>> +++ b/include/hw/virtio/vhost-vsock.h
>>>> @@ -29,6 +29,7 @@ struct VHostVSock {
>>>>     /*< private >*/
>>>>     VHostVSockCommon parent;
>>>>     VHostVSockConf conf;
>>>> +    Error *migration_blocker;   /* set when the device has no ID */
>>>>
>>>>     /*< public >*/
>>>> };
>>>> -- 
>>>> 2.47.1
>>>>
>>>
>