[PATCH v4 00/10] migration/cpr: support vhost-vsock devices

[PATCH v4 00/10] migration/cpr: support vhost-vsock devices

[Andrey Drobyshev]

v4 is mostly fixes suggested by Stefano.

v3 -> v4:

  * Patch 1 (NEW) ("vhost-vsock: block CPR migration modes"): block the
    CPR modes until the support is fully in place.  Make mid-series CPR
    attempts fail early and gracefully instead of crashing;
  * Patch 2 -> 3 ("vhost-vsock: don't reset connections during CPR"):
    write vhost_vsock_post_load() in its final shape, not defer to the
    last patch;
  * Patch 4 -> 5 ("vhost-vsock: preserve vhost FD during CPR"): reword
    commit message;
  * Patch 5 -> 6 ("vhost: factor out vhost_dev_init_backend()"):
    document vhost_dev_cleanup() on error;
  * Patch 7 (NEW) ("vhost: make vhost_dev_cleanup() safe on a partially
    initialized device"): split the vq->dev NULL check out of the
    ownership handoff patch;
  * Patch 9 (NEW) ("vhost: add vhost_dev_is_initialized() helper"): add
    an explicit 'initialized' field to struct vhost_dev instead of
    relying on hdev->mem;
  * Patch 7 -> 10 ("vhost-vsock: hand off device ownership across CPR"):
    - make the start guard in vhost_vsock_set_status() check
      vhost_dev_is_initialized() and vsock->owner_reset instead of
      vhost_dev.mem;
    - call vhost_dev_cleanup() if setting the guest cid fails in
      post_load;
    - lift the CPR blocker (narrow it to the ID-less case);
    - reword commit message.

v3: https://lore.kernel.org/qemu-devel/20260626164643.2526-1-andrey.drobyshev@virtuozzo.com

Andrey Drobyshev (10):
  vhost-vsock: block CPR migration modes
  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: make vhost_dev_cleanup() safe on a partially initialized device
  vhost: add vhost_dev_set_owner() / vhost_dev_reset_owner() helpers
  vhost: add vhost_dev_is_initialized() helper
  vhost-vsock: hand off device ownership across CPR

 hw/virtio/vhost-kernel.c          |   6 +
 hw/virtio/vhost-vsock.c           | 220 +++++++++++++++++++++++++++---
 hw/virtio/vhost.c                 |  57 ++++++--
 include/hw/virtio/vhost-backend.h |   1 +
 include/hw/virtio/vhost-vsock.h   |   4 +
 include/hw/virtio/vhost.h         |  49 +++++++
 6 files changed, 307 insertions(+), 30 deletions(-)

-- 
2.47.1

[PATCH v4 01/10] vhost-vsock: block CPR migration modes

[Andrey Drobyshev]

CPR migration with a vhost-vsock device currently crashes the target
QEMU.  The device ownership is not handed over, so while the source
QEMU still holds the guest CID hashed in the kernel, the target's freshly
opened vhost-vsock device fails to set the same CID in realize():

  qemu-system-x86_64: -device vhost-vsock-pci,id=vsock0,guest-cid=3:
  vhost-vsock: unable to set guest cid: Address already in use

Add an unconditional migration blocker for the CPR modes, so that such
a migration fails early and gracefully instead.  The following patches
implement the actual vhost-vsock CPR support, and the blocker is lifted
once it's fully working.

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

diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
index da244eb1657..503d248a617 100644
--- a/hw/virtio/vhost-vsock.c
+++ b/hw/virtio/vhost-vsock.c
@@ -20,6 +20,8 @@
 #include "hw/core/qdev-properties.h"
 #include "hw/virtio/vhost-vsock.h"
 #include "monitor/monitor.h"
+#include "migration/blocker.h"
+#include "migration/misc.h"
 
 static void vhost_vsock_get_config(VirtIODevice *vdev, uint8_t *config)
 {
@@ -140,25 +142,38 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
         return;
     }
 
+    /*
+     * CPR migration of a vhost-vsock device is not supported yet: the
+     * device ownership is not handed over, so the target fails to set
+     * up its device.  Fail the migration early and gracefully instead.
+     */
+    error_setg(&vsock->migration_blocker,
+               "vhost-vsock: CPR migration is not supported");
+    if (migrate_add_blocker_modes(&vsock->migration_blocker,
+                                  BIT(MIG_MODE_CPR_TRANSFER) |
+                                  BIT(MIG_MODE_CPR_EXEC), errp) < 0) {
+        return;
+    }
+
     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;
         }
 
         if (!qemu_set_blocking(vhostfd, false, errp)) {
-            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)) {
-            return;
+            goto err_blocker;
         }
     }
 
@@ -187,16 +202,20 @@ 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);
 
     /* This will stop vhost backend if appropriate. */
     vhost_vsock_set_status(vdev, 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..a964d57e1bc 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;   /* CPR migration is not supported */
 
     /*< public >*/
 };
-- 
2.47.1

[PATCH v4 02/10] 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 v4 03/10] 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 503d248a617..c582ac9fcc8 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"
 #include "migration/blocker.h"
 #include "migration/misc.h"
 
@@ -110,6 +111,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 vhost_vsock_common_post_load(opaque, version_id);
+    }
+
+    return 0;
+}
+
 static const VMStateDescription vmstate_virtio_vhost_vsock = {
     .name = "virtio-vhost_vsock",
     .minimum_version_id = VHOST_VSOCK_SAVEVM_VERSION,
@@ -119,7 +134,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 v4 04/10] 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 c582ac9fcc8..3ec20d87606 100644
--- a/hw/virtio/vhost-vsock.c
+++ b/hw/virtio/vhost-vsock.c
@@ -176,20 +176,17 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
             error_prepend(errp, "vhost-vsock: unable to parse vhostfd: ");
             goto err_blocker;
         }
-
-        if (!qemu_set_blocking(vhostfd, false, errp)) {
-            goto err_blocker;
-        }
     } else {
         vhostfd = open("/dev/vhost-vsock", O_RDWR);
         if (vhostfd < 0) {
             error_setg_file_open(errp, errno, "/dev/vhost-vsock");
             goto err_blocker;
         }
+    }
 
-        if (!qemu_set_blocking(vhostfd, false, errp)) {
-            goto err_blocker;
-        }
+    if (!qemu_set_blocking(vhostfd, false, errp)) {
+        close(vhostfd);
+        goto err_blocker;
     }
 
     vhost_vsock_common_realize(vdev);
-- 
2.47.1

[PATCH v4 05/10] 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.
Thus we only preserve the FD with cpr_{save,find} if this ID is present.
In a following patch, when we lift the CPR migration blocker, we're
going to condition it on the presence of ID.

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.  Until then CPR remains blocked, so the restore path
added here is not reachable yet.

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

diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
index 3ec20d87606..cec2c31f0ca 100644
--- a/hw/virtio/vhost-vsock.c
+++ b/hw/virtio/vhost-vsock.c
@@ -143,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;
 
@@ -170,7 +171,19 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
         return;
     }
 
-    if (vsock->conf.vhostfd) {
+    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: ");
@@ -207,6 +220,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:
@@ -223,10 +241,14 @@ 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);
-- 
2.47.1

[PATCH v4 06/10] 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 | 22 ++++++++++++++++++++++
 2 files changed, 45 insertions(+), 10 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 371dca17dd8..f9b54c46f93 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1668,6 +1668,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)
@@ -1680,10 +1702,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;
     }
@@ -1694,12 +1713,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..9b98d34dd04 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -156,6 +156,28 @@ 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).
+ *
+ * On failure the backend may be left partially initialised; the caller must
+ * still call vhost_dev_cleanup() to release it.
+ *
+ * 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 v4 07/10] vhost: make vhost_dev_cleanup() safe on a partially initialized device

[Andrey Drobyshev]

A device which only went through vhost_dev_init_backend() has its VQs
not yet initialized, so vq->dev is NULL and vhost_virtqueue_cleanup()
would crash dereferencing it.  Check vq->dev before use, so that
vhost_dev_cleanup() can be called to release a device whose full
vhost_dev_init() never ran or failed along the way.

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

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index f9b54c46f93..2bb9a23fee4 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1648,7 +1648,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);
     }
-- 
2.47.1

[PATCH v4 08/10] 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 2bb9a23fee4..ea17bf01080 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1690,6 +1690,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)
@@ -1707,7 +1725,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 9b98d34dd04..44e65968d9b 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -178,6 +178,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 v4 09/10] vhost: add vhost_dev_is_initialized() helper

[Andrey Drobyshev]

The upcoming CPR support for vhost-vsock defers the full vhost_dev_init()
to .post_load(). On an incoming CPR .realize() only sets up the backend,
so we need a way to tell a fully initialized vhost device from a partially
initialized one.

Add an explicit 'bool initialized' field, set once vhost_dev_init()
succeeds, and cleared only in vhost_dev_cleanup() (by memset(0)). Also add
a public vhost_dev_is_initialized() helper.

Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
 hw/virtio/vhost.c         |  2 ++
 include/hw/virtio/vhost.h | 14 ++++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index ea17bf01080..a81cd94f900 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1821,6 +1821,8 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
         goto fail;
     }
 
+    hdev->initialized = true;
+
     trace_vhost_dev_init_out(hdev);
 
     return 0;
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 44e65968d9b..7c4f032996c 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -109,6 +109,8 @@ struct vhost_dev {
     uint64_t backend_cap;
     /* @started: is the vhost device started? */
     bool started;
+    /* @initialized: has the full vhost_dev_init() completed? */
+    bool initialized;
     bool log_enabled;
     uint64_t log_size;
     Error *migration_blocker;
@@ -236,6 +238,18 @@ static inline bool vhost_dev_is_started(struct vhost_dev *hdev)
     return hdev->started;
 }
 
+/**
+ * vhost_dev_is_initialized() - report init status of vhost device
+ * @hdev: common vhost_dev structure
+ *
+ * Return true if the full vhost_dev_init() has completed successfully
+ * (and vhost_dev_cleanup() has not run since).
+ */
+static inline bool vhost_dev_is_initialized(struct vhost_dev *hdev)
+{
+    return hdev->initialized;
+}
+
 static inline int vhost_dev_set_vring_enable(struct vhost_dev *hdev, int enable)
 {
     if (!hdev->vhost_ops->vhost_set_vring_enable) {
-- 
2.47.1

[PATCH v4 10/10] 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 target 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.

  - .set_status() refuses to start a device whose handoff hasn't
    completed: not fully initialized yet, or left ownerless after a
    failed CPR.

With the handoff in place, lift the CPR blocker: only ID-less devices,
which can't preserve their FDs, remain blocked.

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

diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
index cec2c31f0ca..e25e5350905 100644
--- a/hw/virtio/vhost-vsock.c
+++ b/hw/virtio/vhost-vsock.c
@@ -73,9 +73,23 @@ static int vhost_vsock_set_running(VirtIODevice *vdev, int start)
 static int vhost_vsock_set_status(VirtIODevice *vdev, uint8_t status)
 {
     VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(vdev);
+    VHostVSock *vsock = VHOST_VSOCK(vdev);
     bool should_start = virtio_device_should_start(vdev, status);
+    bool initialized = vhost_dev_is_initialized(&vvc->vhost_dev);
     int ret;
 
+    /*
+     * During CPR, on target the full vhost_dev_init() is deferred to
+     * post_load.  On source the device is left without an owner if a
+     * failed CPR couldn't re-acquire it.  Refuse to start in both cases
+     * rather than issue vhost ioctls on a half-initialised or ownerless
+     * device.
+     */
+    if (should_start && (!initialized || vsock->owner_reset)) {
+        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,8 +125,67 @@ 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_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
@@ -122,6 +195,32 @@ static int vhost_vsock_post_load(void *opaque, int version_id)
         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;
+    }
+
+    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);
+        vhost_dev_cleanup(&vvc->vhost_dev);
+        return ret;
+    }
+
     return 0;
 }
 
@@ -133,7 +232,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 +243,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;
 
@@ -159,19 +259,30 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
     }
 
     /*
-     * CPR migration of a vhost-vsock device is not supported yet: the
-     * device ownership is not handed over, so the target fails to set
-     * up its device.  Fail the migration early and gracefully instead.
+     * With the ownership handoff in place CPR migration is now supported.
+     * Having a unique ID is mandatory for FD preservation during it, thus
+     * we only keep the migration blocker for the ID-less case.
      */
-    error_setg(&vsock->migration_blocker,
-               "vhost-vsock: CPR migration is not supported");
-    if (migrate_add_blocker_modes(&vsock->migration_blocker,
-                                  BIT(MIG_MODE_CPR_TRANSFER) |
-                                  BIT(MIG_MODE_CPR_EXEC), errp) < 0) {
-        return;
+    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()) {
+    /*
+     * 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 "
@@ -204,14 +315,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);
@@ -233,6 +362,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);
 }
 
@@ -249,6 +379,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/include/hw/virtio/vhost-vsock.h b/include/hw/virtio/vhost-vsock.h
index a964d57e1bc..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"
@@ -29,7 +30,9 @@ struct VHostVSock {
     /*< private >*/
     VHostVSockCommon parent;
     VHostVSockConf conf;
-    Error *migration_blocker;   /* CPR migration is not supported */
+    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 v4 00/10] migration/cpr: support vhost-vsock devices

[Andrey Drobyshev]

On 8/20/26 2:39 PM, Andrey Drobyshev wrote:
> v4 is mostly fixes suggested by Stefano.
> 
> v3 -> v4:
> 
>   * Patch 1 (NEW) ("vhost-vsock: block CPR migration modes"): block the
>     CPR modes until the support is fully in place.  Make mid-series CPR
>     attempts fail early and gracefully instead of crashing;
>   * Patch 2 -> 3 ("vhost-vsock: don't reset connections during CPR"):
>     write vhost_vsock_post_load() in its final shape, not defer to the
>     last patch;
>   * Patch 4 -> 5 ("vhost-vsock: preserve vhost FD during CPR"): reword
>     commit message;
>   * Patch 5 -> 6 ("vhost: factor out vhost_dev_init_backend()"):
>     document vhost_dev_cleanup() on error;
>   * Patch 7 (NEW) ("vhost: make vhost_dev_cleanup() safe on a partially
>     initialized device"): split the vq->dev NULL check out of the
>     ownership handoff patch;
>   * Patch 9 (NEW) ("vhost: add vhost_dev_is_initialized() helper"): add
>     an explicit 'initialized' field to struct vhost_dev instead of
>     relying on hdev->mem;
>   * Patch 7 -> 10 ("vhost-vsock: hand off device ownership across CPR"):
>     - make the start guard in vhost_vsock_set_status() check
>       vhost_dev_is_initialized() and vsock->owner_reset instead of
>       vhost_dev.mem;
>     - call vhost_dev_cleanup() if setting the guest cid fails in
>       post_load;
>     - lift the CPR blocker (narrow it to the ID-less case);
>     - reword commit message.
> 
> v3: https://lore.kernel.org/qemu-devel/20260626164643.2526-1-andrey.drobyshev@virtuozzo.com
> 
> Andrey Drobyshev (10):
>   vhost-vsock: block CPR migration modes
>   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: make vhost_dev_cleanup() safe on a partially initialized device
>   vhost: add vhost_dev_set_owner() / vhost_dev_reset_owner() helpers
>   vhost: add vhost_dev_is_initialized() helper
>   vhost-vsock: hand off device ownership across CPR
> 
>  hw/virtio/vhost-kernel.c          |   6 +
>  hw/virtio/vhost-vsock.c           | 220 +++++++++++++++++++++++++++---
>  hw/virtio/vhost.c                 |  57 ++++++--
>  include/hw/virtio/vhost-backend.h |   1 +
>  include/hw/virtio/vhost-vsock.h   |   4 +
>  include/hw/virtio/vhost.h         |  49 +++++++
>  6 files changed, 307 insertions(+), 30 deletions(-)
> 

Friendly ping

Re: [PATCH v4 00/10] migration/cpr: support vhost-vsock devices

[Stefano Garzarella]

On Mon, Aug 31, 2026 at 02:16:13PM +0300, Andrey Drobyshev wrote:
>On 8/20/26 2:39 PM, Andrey Drobyshev wrote:
>> v4 is mostly fixes suggested by Stefano.
>>
>> v3 -> v4:
>>
>>   * Patch 1 (NEW) ("vhost-vsock: block CPR migration modes"): block the
>>     CPR modes until the support is fully in place.  Make mid-series CPR
>>     attempts fail early and gracefully instead of crashing;
>>   * Patch 2 -> 3 ("vhost-vsock: don't reset connections during CPR"):
>>     write vhost_vsock_post_load() in its final shape, not defer to the
>>     last patch;
>>   * Patch 4 -> 5 ("vhost-vsock: preserve vhost FD during CPR"): reword
>>     commit message;
>>   * Patch 5 -> 6 ("vhost: factor out vhost_dev_init_backend()"):
>>     document vhost_dev_cleanup() on error;
>>   * Patch 7 (NEW) ("vhost: make vhost_dev_cleanup() safe on a partially
>>     initialized device"): split the vq->dev NULL check out of the
>>     ownership handoff patch;
>>   * Patch 9 (NEW) ("vhost: add vhost_dev_is_initialized() helper"): add
>>     an explicit 'initialized' field to struct vhost_dev instead of
>>     relying on hdev->mem;
>>   * Patch 7 -> 10 ("vhost-vsock: hand off device ownership across CPR"):
>>     - make the start guard in vhost_vsock_set_status() check
>>       vhost_dev_is_initialized() and vsock->owner_reset instead of
>>       vhost_dev.mem;
>>     - call vhost_dev_cleanup() if setting the guest cid fails in
>>       post_load;
>>     - lift the CPR blocker (narrow it to the ID-less case);
>>     - reword commit message.
>>
>> v3: https://lore.kernel.org/qemu-devel/20260626164643.2526-1-andrey.drobyshev@virtuozzo.com
>>
>> Andrey Drobyshev (10):
>>   vhost-vsock: block CPR migration modes
>>   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: make vhost_dev_cleanup() safe on a partially initialized device
>>   vhost: add vhost_dev_set_owner() / vhost_dev_reset_owner() helpers
>>   vhost: add vhost_dev_is_initialized() helper
>>   vhost-vsock: hand off device ownership across CPR
>>
>>  hw/virtio/vhost-kernel.c          |   6 +
>>  hw/virtio/vhost-vsock.c           | 220 +++++++++++++++++++++++++++---
>>  hw/virtio/vhost.c                 |  57 ++++++--
>>  include/hw/virtio/vhost-backend.h |   1 +
>>  include/hw/virtio/vhost-vsock.h   |   4 +
>>  include/hw/virtio/vhost.h         |  49 +++++++
>>  6 files changed, 307 insertions(+), 30 deletions(-)
>>
>
>Friendly ping
>

Sorry, I was on PTO, I'm going to review this week (hopefully today).

Stefano

Re: [PATCH v4 02/10] vhost: add vhost_reset_owner op

[Stefano Garzarella]

On Thu, Aug 20, 2026 at 02:39:47PM +0300, Andrey Drobyshev wrote:
>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;

Should we introduce `vhost_reset_owner_op` ?

Stefano

>     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
>

Re: [PATCH v4 01/10] vhost-vsock: block CPR migration modes

[Stefano Garzarella]

On Thu, Aug 20, 2026 at 02:39:46PM +0300, Andrey Drobyshev wrote:
>CPR migration with a vhost-vsock device currently crashes the target
>QEMU.  The device ownership is not handed over, so while the source
>QEMU still holds the guest CID hashed in the kernel, the target's freshly
>opened vhost-vsock device fails to set the same CID in realize():
>
>  qemu-system-x86_64: -device vhost-vsock-pci,id=vsock0,guest-cid=3:
>  vhost-vsock: unable to set guest cid: Address already in use
>
>Add an unconditional migration blocker for the CPR modes, so that such
>a migration fails early and gracefully instead.  The following patches
>implement the actual vhost-vsock CPR support, and the blocker is lifted
>once it's fully working.
>
>Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>---
> hw/virtio/vhost-vsock.c         | 27 +++++++++++++++++++++++----
> include/hw/virtio/vhost-vsock.h |  1 +
> 2 files changed, 24 insertions(+), 4 deletions(-)

Acked-by: Stefano Garzarella <sgarzare@redhat.com>

Re: [PATCH v4 03/10] vhost-vsock: don't reset connections during CPR

[Stefano Garzarella]

On Thu, Aug 20, 2026 at 02:39:48PM +0300, Andrey Drobyshev wrote:
>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(-)

Acked-by: Stefano Garzarella <sgarzare@redhat.com>

Re: [PATCH v4 04/10] vhost-vsock: fix FD leak in realize()

[Stefano Garzarella]

On Thu, Aug 20, 2026 at 02:39:49PM +0300, Andrey Drobyshev wrote:
>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).
>

Fixes tag?

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

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

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

Re: [PATCH v4 05/10] vhost-vsock: preserve vhost FD during CPR

[Stefano Garzarella]

On Thu, Aug 20, 2026 at 02:39:50PM +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.

mm, are we sure the device ID is unique?

I just tried this whitout receiving any error:
qemu-system-x86_64 -smp 2 -M q35,accel=kvm,memory-backend=vsock0 \
     -object memory-backend-memfd,id=vsock0,size=512M \
     -device vhost-vsock-pci,id=vsock0,guest-cid=3

Stefano

>Thus we only preserve the FD with cpr_{save,find} if this ID is present.
>In a following patch, when we lift the CPR migration blocker, we're
>going to condition it on the presence of ID.
>
>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.  Until then CPR remains blocked, so the restore path
>added here is not reachable yet.
>
>Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>---
> hw/virtio/vhost-vsock.c | 24 +++++++++++++++++++++++-
> 1 file changed, 23 insertions(+), 1 deletion(-)
>
>diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
>index 3ec20d87606..cec2c31f0ca 100644
>--- a/hw/virtio/vhost-vsock.c
>+++ b/hw/virtio/vhost-vsock.c
>@@ -143,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;
>
>@@ -170,7 +171,19 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
>         return;
>     }
>
>-    if (vsock->conf.vhostfd) {
>+    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: ");
>@@ -207,6 +220,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:
>@@ -223,10 +241,14 @@ 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);
>-- 
>2.47.1
>

Re: [PATCH v4 06/10] vhost: factor out vhost_dev_init_backend()

[Stefano Garzarella]

On Thu, Aug 20, 2026 at 02:39:51PM +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 | 22 ++++++++++++++++++++++
> 2 files changed, 45 insertions(+), 10 deletions(-)

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

Re: [PATCH v4 07/10] vhost: make vhost_dev_cleanup() safe on a partially initialized device

[Stefano Garzarella]

On Thu, Aug 20, 2026 at 02:39:52PM +0300, Andrey Drobyshev wrote:
>A device which only went through vhost_dev_init_backend() has its VQs
>not yet initialized, so vq->dev is NULL and vhost_virtqueue_cleanup()
>would crash dereferencing it.  Check vq->dev before use, so that
>vhost_dev_cleanup() can be called to release a device whose full
>vhost_dev_init() never ran or failed along the way.

Is this a fix for a current issue, a defensive programming measure, or 
something that might happen with future patches?

Stefano

>
>Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>---
> hw/virtio/vhost.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>index f9b54c46f93..2bb9a23fee4 100644
>--- a/hw/virtio/vhost.c
>+++ b/hw/virtio/vhost.c
>@@ -1648,7 +1648,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);
>     }
>-- 
>2.47.1
>

Re: [PATCH v4 08/10] vhost: add vhost_dev_set_owner() / vhost_dev_reset_owner() helpers

[Stefano Garzarella]

On Thu, Aug 20, 2026 at 02:39:53PM +0300, Andrey Drobyshev wrote:
>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 2bb9a23fee4..ea17bf01080 100644
>--- a/hw/virtio/vhost.c
>+++ b/hw/virtio/vhost.c
>@@ -1690,6 +1690,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)
>@@ -1707,7 +1725,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 9b98d34dd04..44e65968d9b 100644
>--- a/include/hw/virtio/vhost.h
>+++ b/include/hw/virtio/vhost.h
>@@ -178,6 +178,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

I'm not sure this is a common practice in kernel-doc format.
I think we should add 2 differnt blocks, one for each function.

Stefano

>+ * @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
>

Re: [PATCH v4 09/10] vhost: add vhost_dev_is_initialized() helper

[Stefano Garzarella]

On Thu, Aug 20, 2026 at 02:39:54PM +0300, Andrey Drobyshev wrote:
>The upcoming CPR support for vhost-vsock defers the full vhost_dev_init()
>to .post_load(). On an incoming CPR .realize() only sets up the backend,
>so we need a way to tell a fully initialized vhost device from a partially
>initialized one.
>
>Add an explicit 'bool initialized' field, set once vhost_dev_init()
>succeeds, and cleared only in vhost_dev_cleanup() (by memset(0)). Also add
>a public vhost_dev_is_initialized() helper.
>
>Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
>Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>---
> hw/virtio/vhost.c         |  2 ++
> include/hw/virtio/vhost.h | 14 ++++++++++++++
> 2 files changed, 16 insertions(+)

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

Re: [PATCH v4 10/10] vhost-vsock: hand off device ownership across CPR

[Stefano Garzarella]

On Thu, Aug 20, 2026 at 02:39:55PM +0300, Andrey Drobyshev wrote:
>The previous patches reuse the source vhost FD on the destination,
>however both source and target 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.
>
>  - .set_status() refuses to start a device whose handoff hasn't
>    completed: not fully initialized yet, or left ownerless after a
>    failed CPR.
>
>With the handoff in place, lift the CPR blocker: only ID-less devices,
>which can't preserve their FDs, remain blocked.
>
>Suggested-by: Dongli Zhang <dongli.zhang@oracle.com>
>Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>---
> hw/virtio/vhost-vsock.c         | 165 ++++++++++++++++++++++++++++----
> include/hw/virtio/vhost-vsock.h |   5 +-
> 2 files changed, 152 insertions(+), 18 deletions(-)
>
>diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
>index cec2c31f0ca..e25e5350905 100644
>--- a/hw/virtio/vhost-vsock.c
>+++ b/hw/virtio/vhost-vsock.c
>@@ -73,9 +73,23 @@ static int vhost_vsock_set_running(VirtIODevice *vdev, int start)
> static int vhost_vsock_set_status(VirtIODevice *vdev, uint8_t status)
> {
>     VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(vdev);
>+    VHostVSock *vsock = VHOST_VSOCK(vdev);
>     bool should_start = virtio_device_should_start(vdev, status);
>+    bool initialized = vhost_dev_is_initialized(&vvc->vhost_dev);
>     int ret;
>
>+    /*
>+     * During CPR, on target the full vhost_dev_init() is deferred to
>+     * post_load.  On source the device is left without an owner if a
>+     * failed CPR couldn't re-acquire it.  Refuse to start in both cases
>+     * rather than issue vhost ioctls on a half-initialised or ownerless
>+     * device.
>+     */
>+    if (should_start && (!initialized || vsock->owner_reset)) {
>+        error_report("vhost-vsock: refusing to start, device init incomplete");
>+        return 0;

Why returning 0?

>+    }
>+
>     if (vhost_dev_is_started(&vvc->vhost_dev) == should_start) {
>         return 0;
>     }
>@@ -111,8 +125,67 @@ 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_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) {

IIUC MIG_EVENT_FAILED only covers the source failing. If the source 
completes it successfully and something goes wrong afterwards (e.g. in 
the destination), what will happen?

Thinking if we can try to call vhost_dev_set_owner() in 
vhost_vsock_set_status(), expecting also -EBUSY, instead of relying on 
`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);

Is it okay to return 0 also in this case?

>+        } 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)) {

Am I missing something that keeps pre_save from running outside an 
actual CPR migration (savevm, or any save path taken while mode is still 
set from an earlier attempt), or should the condition be tied to a CPR 
migration actually being in flight rather than to the mode parameter 
alone?

>+        ret = vhost_dev_reset_owner(&vvc->vhost_dev);
>+        if (ret < 0) {
>+            error_report("vhost-vsock: vhost_reset_owner failed: %d", ret);

vhost-vsock devices that don't support `reset_owner` will fail here, 
right? Is that what we expect?

>+            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
>@@ -122,6 +195,32 @@ static int vhost_vsock_post_load(void *opaque, int version_id)
>         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;
>+    }
>+
>+    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);
>+        vhost_dev_cleanup(&vvc->vhost_dev);
>+        return ret;
>+    }
>+
>     return 0;
> }
>
>@@ -133,7 +232,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 +243,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;
>
>@@ -159,19 +259,30 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
>     }
>
>     /*
>-     * CPR migration of a vhost-vsock device is not supported yet: the
>-     * device ownership is not handed over, so the target fails to set
>-     * up its device.  Fail the migration early and gracefully instead.
>+     * With the ownership handoff in place CPR migration is now supported.
>+     * Having a unique ID is mandatory for FD preservation during it, thus
>+     * we only keep the migration blocker for the ID-less case.
>      */
>-    error_setg(&vsock->migration_blocker,
>-               "vhost-vsock: CPR migration is not supported");
>-    if (migrate_add_blocker_modes(&vsock->migration_blocker,
>-                                  BIT(MIG_MODE_CPR_TRANSFER) |
>-                                  BIT(MIG_MODE_CPR_EXEC), errp) < 0) {
>-        return;
>+    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()) {
>+    /*
>+     * 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 "
>@@ -204,14 +315,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 */

Okay, but IIUC this is returned by cpr_find_fd(), is that funtion 
passing the ownership of the fd, even that the fd is not removed from 
the hash table?

Should we call cpr_delete_fd() when we take it?

Thanks,
Stefano

>+            goto err_vhost_dev;
>+        }
>+
>+        return;
>     }
>
>     ret = vhost_vsock_set_guest_cid(vdev);
>@@ -233,6 +362,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);
> }
>
>@@ -249,6 +379,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/include/hw/virtio/vhost-vsock.h b/include/hw/virtio/vhost-vsock.h
>index a964d57e1bc..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"
>@@ -29,7 +30,9 @@ struct VHostVSock {
>     /*< private >*/
>     VHostVSockCommon parent;
>     VHostVSockConf conf;
>-    Error *migration_blocker;   /* CPR migration is not supported */
>+    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 v4 02/10] vhost: add vhost_reset_owner op

[Andrey Drobyshev]

On 9/7/26 4:58 PM, Stefano Garzarella wrote:
> On Thu, Aug 20, 2026 at 02:39:47PM +0300, Andrey Drobyshev wrote:
>> 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;
> 
> Should we introduce `vhost_reset_owner_op` ?
> 
> Stefano

You're right of course, I overlooked it.  Thanks for noticing!

Andrey

Re: [PATCH v4 04/10] vhost-vsock: fix FD leak in realize()

[Andrey Drobyshev]

On 9/7/26 5:54 PM, Stefano Garzarella wrote:
> On Thu, Aug 20, 2026 at 02:39:49PM +0300, Andrey Drobyshev wrote:
>> 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).
>>
> 
> Fixes tag?

Sure, should probably be 2 tags:

Fixes: 384c2561bddf ("vhost-vsock: set vhostfd to non-blocking mode")
Fixes: 701544cfaf46 ("hw: replace qemu_set_nonblock()")

Will add them on the next respin.

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

Re: [PATCH v4 05/10] vhost-vsock: preserve vhost FD during CPR

[Andrey Drobyshev]

On 9/7/26 5:55 PM, Stefano Garzarella wrote:
> On Thu, Aug 20, 2026 at 02:39:50PM +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.
> 
> mm, are we sure the device ID is unique?
> 
> I just tried this whitout receiving any error:
> qemu-system-x86_64 -smp 2 -M q35,accel=kvm,memory-backend=vsock0 \
>      -object memory-backend-memfd,id=vsock0,size=512M \
>      -device vhost-vsock-pci,id=vsock0,guest-cid=3
> 
> Stefano 
You're right, that seems to be the a real issue.  In fact there's even a
UAF bug IIUC, stemming from the fact that we: 1) provide key destroy
function when creating the CPR hash table; 2) use same CprFd for both
key and value.

That means that when we have several devices supporting CPR with
repeating IDs, we do:

  g_hash_table_insert(table, fd1, fd1);    // table has fd1 -> fd1
  g_hash_table_insert(table, fd2, fd2);    // table has fd1 -> fd2

Here fd1 and fd2 have same hash of course. According to GLib docs [1],
value is renewed in this case, but the new key is destroyed and the old
one is used.  As a result, fd2 is freed.

And, mind you, this all happens long before actual CPR, cause the
devices with CPR support usually call cpr_save_fd() somewhere in
.realize().  Thus we get memory corruption right at boot time, plus
subsequent CPR will likely crash.

In these circumstances I'd personally prefer just asserting on repeating
key somewhere in cpr_fd_hash_insert().  The only downside is that it'll
crash when booting 2 CPR supporting devices having identical IDs, even
if the user isn't going to perform CPR.  I'd say it's acceptable, what
do you think?

[1] https://docs.gtk.org/glib/type_func.HashTable.insert.html


Andrey

Re: [PATCH v4 08/10] vhost: add vhost_dev_set_owner() / vhost_dev_reset_owner() helpers

[Andrey Drobyshev]

On 9/7/26 5:56 PM, Stefano Garzarella wrote:
> On Thu, Aug 20, 2026 at 02:39:53PM +0300, Andrey Drobyshev wrote:
>> 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 2bb9a23fee4..ea17bf01080 100644
>> --- a/hw/virtio/vhost.c
>> +++ b/hw/virtio/vhost.c
>> @@ -1690,6 +1690,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)
>> @@ -1707,7 +1725,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 9b98d34dd04..44e65968d9b 100644
>> --- a/include/hw/virtio/vhost.h
>> +++ b/include/hw/virtio/vhost.h
>> @@ -178,6 +178,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
> 
> I'm not sure this is a common practice in kernel-doc format.
> I think we should add 2 differnt blocks, one for each function.
> 
> Stefano

Agreed, will do.

Andrey
>> + * @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
>>
> 

Re: [PATCH v4 07/10] vhost: make vhost_dev_cleanup() safe on a partially initialized device

[Andrey Drobyshev]

On 9/7/26 5:56 PM, Stefano Garzarella wrote:
> On Thu, Aug 20, 2026 at 02:39:52PM +0300, Andrey Drobyshev wrote:
>> A device which only went through vhost_dev_init_backend() has its VQs
>> not yet initialized, so vq->dev is NULL and vhost_virtqueue_cleanup()
>> would crash dereferencing it.  Check vq->dev before use, so that
>> vhost_dev_cleanup() can be called to release a device whose full
>> vhost_dev_init() never ran or failed along the way.
> 
> Is this a fix for a current issue, a defensive programming measure, or 
> something that might happen with future patches?
> 
> Stefano
It's the latter.  IIUC right now there's no way to reach the cleanup
code with vq->dev uninitialized, but after a subsequent patch this
becomes a possibility.  I'll add this note to commit message.

Andrey

Re: [PATCH v4 10/10] vhost-vsock: hand off device ownership across CPR

[Andrey Drobyshev]

On 9/7/26 6:13 PM, Stefano Garzarella wrote:
> On Thu, Aug 20, 2026 at 02:39:55PM +0300, Andrey Drobyshev wrote:
>> The previous patches reuse the source vhost FD on the destination,
>> however both source and target 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.
>>
>>  - .set_status() refuses to start a device whose handoff hasn't
>>    completed: not fully initialized yet, or left ownerless after a
>>    failed CPR.
>>
>> With the handoff in place, lift the CPR blocker: only ID-less devices,
>> which can't preserve their FDs, remain blocked.
>>
>> Suggested-by: Dongli Zhang <dongli.zhang@oracle.com>
>> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>> ---
>> hw/virtio/vhost-vsock.c         | 165 ++++++++++++++++++++++++++++----
>> include/hw/virtio/vhost-vsock.h |   5 +-
>> 2 files changed, 152 insertions(+), 18 deletions(-)
>>
>> diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
>> index cec2c31f0ca..e25e5350905 100644
>> --- a/hw/virtio/vhost-vsock.c
>> +++ b/hw/virtio/vhost-vsock.c
>> @@ -73,9 +73,23 @@ static int vhost_vsock_set_running(VirtIODevice *vdev, int start)
>> static int vhost_vsock_set_status(VirtIODevice *vdev, uint8_t status)
>> {
>>     VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(vdev);
>> +    VHostVSock *vsock = VHOST_VSOCK(vdev);
>>     bool should_start = virtio_device_should_start(vdev, status);
>> +    bool initialized = vhost_dev_is_initialized(&vvc->vhost_dev);
>>     int ret;
>>
>> +    /*
>> +     * During CPR, on target the full vhost_dev_init() is deferred to
>> +     * post_load.  On source the device is left without an owner if a
>> +     * failed CPR couldn't re-acquire it.  Refuse to start in both cases
>> +     * rather than issue vhost ioctls on a half-initialised or ownerless
>> +     * device.
>> +     */
>> +    if (should_start && (!initialized || vsock->owner_reset)) {
>> +        error_report("vhost-vsock: refusing to start, device init incomplete");
>> +        return 0;
> 
> Why returning 0?

That's just what every other error path does here, because return value
for vhost_vsock_set_status() goes nowhere.  The only thing it'd
influence is having one more line in the QEMU log when we're called from
virtio_set_status().
>> +    }
>> +
>>     if (vhost_dev_is_started(&vvc->vhost_dev) == should_start) {
>>         return 0;
>>     }
>> @@ -111,8 +125,67 @@ 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_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) {
> 
> IIUC MIG_EVENT_FAILED only covers the source failing. If the source 
> completes it successfully and something goes wrong afterwards (e.g. in 
> the destination), what will happen?

Hmm that indeed might be a problem for cpr-transfer.  E.g. target fails,
management (libvirt) sends qmp-cont to source, and we bail out with
error in vhost_vsock_set_status().
> Thinking if we can try to call vhost_dev_set_owner() in 
> vhost_vsock_set_status(), expecting also -EBUSY, instead of relying on 
> `vsock->owner_reset`.

Moving .set_owner() to .set_status() actually looks promising, I'll try
it out and see how that works.  Although I'd still keep the flag and
restrain from tolerating -EBUSY - we shouldn't proceed to realize the
device if we don't own it.


>> +        ret = vhost_dev_set_owner(&vvc->vhost_dev);
>> +        if (ret < 0) {
>> +            error_report("vhost-vsock: failed to re-acquire owner: %d", ret);
> 
> Is it okay to return 0 also in this case?

As of now it's mandatory cause migration_call_notifiers() asserts on
type == MIG_EVENT_SETUP whenever notifier returns non-null.
>> +        } 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)) {
> 
> Am I missing something that keeps pre_save from running outside an 
> actual CPR migration (savevm, or any save path taken while mode is still 
> set from an earlier attempt), or should the condition be tied to a CPR 
> migration actually being in flight rather than to the mode parameter 
> alone?

You're right, that's a real bug.  Should probably guard it with
cpr_incoming_needed(NULL) && migration_is_running().
>> +        ret = vhost_dev_reset_owner(&vvc->vhost_dev);
>> +        if (ret < 0) {
>> +            error_report("vhost-vsock: vhost_reset_owner failed: %d", ret);
> 
> vhost-vsock devices that don't support `reset_owner` will fail here, 
> right? Is that what we expect?

Which devices you mean?  No other vhost-vsock devices should ever end up
here, e.g. vhost-user-vsock has .unmigratable = 1 set, so .pre_save() is
never run for it.  If you mean the kernels which don't support
RESET_OWNER - then yes, we'll fail here.
>> +            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
>> @@ -122,6 +195,32 @@ static int vhost_vsock_post_load(void *opaque, int version_id)
>>         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;
>> +    }
>> +
>> +    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);
>> +        vhost_dev_cleanup(&vvc->vhost_dev);
>> +        return ret;
>> +    }
>> +
>>     return 0;
>> }
>>
>> @@ -133,7 +232,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 +243,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;
>>
>> @@ -159,19 +259,30 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
>>     }
>>
>>     /*
>> -     * CPR migration of a vhost-vsock device is not supported yet: the
>> -     * device ownership is not handed over, so the target fails to set
>> -     * up its device.  Fail the migration early and gracefully instead.
>> +     * With the ownership handoff in place CPR migration is now supported.
>> +     * Having a unique ID is mandatory for FD preservation during it, thus
>> +     * we only keep the migration blocker for the ID-less case.
>>      */
>> -    error_setg(&vsock->migration_blocker,
>> -               "vhost-vsock: CPR migration is not supported");
>> -    if (migrate_add_blocker_modes(&vsock->migration_blocker,
>> -                                  BIT(MIG_MODE_CPR_TRANSFER) |
>> -                                  BIT(MIG_MODE_CPR_EXEC), errp) < 0) {
>> -        return;
>> +    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()) {
>> +    /*
>> +     * 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 "
>> @@ -204,14 +315,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 */
> 
> Okay, but IIUC this is returned by cpr_find_fd(), is that funtion 
> passing the ownership of the fd, even that the fd is not removed from 
> the hash table?
> 
> Should we call cpr_delete_fd() when we take it?

You're right, that should be a proper cleanup.  Every path that closes
the fd after a successful cpr_find_fd() should also drop the entry.
Will add this.

Andrey
> Thanks,
> Stefano

Re: [PATCH v4 05/10] vhost-vsock: preserve vhost FD during CPR

[Stefano Garzarella]

On Wed, Sep 09, 2026 at 07:59:55PM +0300, Andrey Drobyshev wrote:
>On 9/7/26 5:55 PM, Stefano Garzarella wrote:
>> On Thu, Aug 20, 2026 at 02:39:50PM +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.
>>
>> mm, are we sure the device ID is unique?
>>
>> I just tried this whitout receiving any error:
>> qemu-system-x86_64 -smp 2 -M q35,accel=kvm,memory-backend=vsock0 \
>>      -object memory-backend-memfd,id=vsock0,size=512M \
>>      -device vhost-vsock-pci,id=vsock0,guest-cid=3
>>
>> Stefano
>You're right, that seems to be the a real issue.  In fact there's even a
>UAF bug IIUC, stemming from the fact that we: 1) provide key destroy
>function when creating the CPR hash table; 2) use same CprFd for both
>key and value.
>
>That means that when we have several devices supporting CPR with
>repeating IDs, we do:
>
>  g_hash_table_insert(table, fd1, fd1);    // table has fd1 -> fd1
>  g_hash_table_insert(table, fd2, fd2);    // table has fd1 -> fd2
>
>Here fd1 and fd2 have same hash of course. According to GLib docs [1],
>value is renewed in this case, but the new key is destroyed and the old
>one is used.  As a result, fd2 is freed.
>
>And, mind you, this all happens long before actual CPR, cause the
>devices with CPR support usually call cpr_save_fd() somewhere in
>.realize().  Thus we get memory corruption right at boot time, plus
>subsequent CPR will likely crash.
>
>In these circumstances I'd personally prefer just asserting on repeating
>key somewhere in cpr_fd_hash_insert().  The only downside is that it'll
>crash when booting 2 CPR supporting devices having identical IDs, even
>if the user isn't going to perform CPR.  I'd say it's acceptable, what
>do you think?

I'd prefer an error if possible.

That said, can we append a prefix to the key (e.g. `vhost-vsock`)?
In this way we reduce the chance to have a conflict. Of course the user 
can still assign `vhost-vsock-something` to the memory backend and 
`something` to the vhost-vsock device, but yeah xD

Thanks,
Stefano

Re: [PATCH v4 10/10] vhost-vsock: hand off device ownership across CPR

[Stefano Garzarella]

On Wed, Sep 09, 2026 at 08:45:22PM +0300, Andrey Drobyshev wrote:
>On 9/7/26 6:13 PM, Stefano Garzarella wrote:
>> On Thu, Aug 20, 2026 at 02:39:55PM +0300, Andrey Drobyshev wrote:
>>> The previous patches reuse the source vhost FD on the destination,
>>> however both source and target 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.
>>>
>>>  - .set_status() refuses to start a device whose handoff hasn't
>>>    completed: not fully initialized yet, or left ownerless after a
>>>    failed CPR.
>>>
>>> With the handoff in place, lift the CPR blocker: only ID-less devices,
>>> which can't preserve their FDs, remain blocked.
>>>
>>> Suggested-by: Dongli Zhang <dongli.zhang@oracle.com>
>>> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>>> ---
>>> hw/virtio/vhost-vsock.c         | 165 ++++++++++++++++++++++++++++----
>>> include/hw/virtio/vhost-vsock.h |   5 +-
>>> 2 files changed, 152 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
>>> index cec2c31f0ca..e25e5350905 100644
>>> --- a/hw/virtio/vhost-vsock.c
>>> +++ b/hw/virtio/vhost-vsock.c
>>> @@ -73,9 +73,23 @@ static int vhost_vsock_set_running(VirtIODevice *vdev, int start)
>>> static int vhost_vsock_set_status(VirtIODevice *vdev, uint8_t status)
>>> {
>>>     VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(vdev);
>>> +    VHostVSock *vsock = VHOST_VSOCK(vdev);
>>>     bool should_start = virtio_device_should_start(vdev, status);
>>> +    bool initialized = vhost_dev_is_initialized(&vvc->vhost_dev);
>>>     int ret;
>>>
>>> +    /*
>>> +     * During CPR, on target the full vhost_dev_init() is deferred to
>>> +     * post_load.  On source the device is left without an owner if a
>>> +     * failed CPR couldn't re-acquire it.  Refuse to start in both cases
>>> +     * rather than issue vhost ioctls on a half-initialised or ownerless
>>> +     * device.
>>> +     */
>>> +    if (should_start && (!initialized || vsock->owner_reset)) {
>>> +        error_report("vhost-vsock: refusing to start, device init incomplete");
>>> +        return 0;
>>
>> Why returning 0?
>
>That's just what every other error path does here, because return value
>for vhost_vsock_set_status() goes nowhere.  The only thing it'd
>influence is having one more line in the QEMU log when we're called from
>virtio_set_status().

Ah okay, thanks for pointing out :-)

>>> +    }
>>> +
>>>     if (vhost_dev_is_started(&vvc->vhost_dev) == should_start) {
>>>         return 0;
>>>     }
>>> @@ -111,8 +125,67 @@ 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_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) {
>>
>> IIUC MIG_EVENT_FAILED only covers the source failing. If the source
>> completes it successfully and something goes wrong afterwards (e.g. in
>> the destination), what will happen?
>
>Hmm that indeed might be a problem for cpr-transfer.  E.g. target fails,
>management (libvirt) sends qmp-cont to source, and we bail out with
>error in vhost_vsock_set_status().
>> Thinking if we can try to call vhost_dev_set_owner() in
>> vhost_vsock_set_status(), expecting also -EBUSY, instead of relying on
>> `vsock->owner_reset`.
>
>Moving .set_owner() to .set_status() actually looks promising, I'll try
>it out and see how that works.  Although I'd still keep the flag and
>restrain from tolerating -EBUSY - we shouldn't proceed to realize the
>device if we don't own it.

Yeah, agree.

>
>
>>> +        ret = vhost_dev_set_owner(&vvc->vhost_dev);
>>> +        if (ret < 0) {
>>> +            error_report("vhost-vsock: failed to re-acquire owner: %d", ret);
>>
>> Is it okay to return 0 also in this case?
>
>As of now it's mandatory cause migration_call_notifiers() asserts on
>type == MIG_EVENT_SETUP whenever notifier returns non-null.

I see, thanks.

>>> +        } 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)) {
>>
>> Am I missing something that keeps pre_save from running outside an
>> actual CPR migration (savevm, or any save path taken while mode is still
>> set from an earlier attempt), or should the condition be tied to a CPR
>> migration actually being in flight rather than to the mode parameter
>> alone?
>
>You're right, that's a real bug.  Should probably guard it with
>cpr_incoming_needed(NULL) && migration_is_running().

Yeah, that looks better to me too.

>>> +        ret = vhost_dev_reset_owner(&vvc->vhost_dev);
>>> +        if (ret < 0) {
>>> +            error_report("vhost-vsock: vhost_reset_owner failed: %d", ret);
>>
>> vhost-vsock devices that don't support `reset_owner` will fail here,
>> right? Is that what we expect?
>
>Which devices you mean?  No other vhost-vsock devices should ever end up
>here, e.g. vhost-user-vsock has .unmigratable = 1 set, so .pre_save() is
>never run for it.  If you mean the kernels which don't support
>RESET_OWNER - then yes, we'll fail here.

Yep, I meant the kernel.

I'd like to better understand what happens when we perform CPR with a 
vhost-vsock device and a kernel that doesn't support RESET_OWNER. Both 
before and after this series.

>>> +            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
>>> @@ -122,6 +195,32 @@ static int vhost_vsock_post_load(void *opaque, int version_id)
>>>         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;
>>> +    }
>>> +
>>> +    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);
>>> +        vhost_dev_cleanup(&vvc->vhost_dev);
>>> +        return ret;
>>> +    }
>>> +
>>>     return 0;
>>> }
>>>
>>> @@ -133,7 +232,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 +243,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;
>>>
>>> @@ -159,19 +259,30 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
>>>     }
>>>
>>>     /*
>>> -     * CPR migration of a vhost-vsock device is not supported yet: the
>>> -     * device ownership is not handed over, so the target fails to set
>>> -     * up its device.  Fail the migration early and gracefully instead.
>>> +     * With the ownership handoff in place CPR migration is now supported.
>>> +     * Having a unique ID is mandatory for FD preservation during it, thus
>>> +     * we only keep the migration blocker for the ID-less case.
>>>      */
>>> -    error_setg(&vsock->migration_blocker,
>>> -               "vhost-vsock: CPR migration is not supported");
>>> -    if (migrate_add_blocker_modes(&vsock->migration_blocker,
>>> -                                  BIT(MIG_MODE_CPR_TRANSFER) |
>>> -                                  BIT(MIG_MODE_CPR_EXEC), errp) < 0) {
>>> -        return;
>>> +    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()) {
>>> +    /*
>>> +     * 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 "
>>> @@ -204,14 +315,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 */
>>
>> Okay, but IIUC this is returned by cpr_find_fd(), is that funtion
>> passing the ownership of the fd, even that the fd is not removed from
>> the hash table?
>>
>> Should we call cpr_delete_fd() when we take it?
>
>You're right, that should be a proper cleanup.  Every path that closes
>the fd after a successful cpr_find_fd() should also drop the entry.
>Will add this.

I don't know how CPR works, but if we don't close the FD here, will it 
be closed later? I mean, if we leave it on the map, will someone else 
close it?

That way, we don't have a different situation here.

That said, I'm fine also with your solution.

Thanks,
Stefano

Re: [PATCH v4 05/10] vhost-vsock: preserve vhost FD during CPR

[Andrey Drobyshev]

On 9/11/26 10:37 AM, Stefano Garzarella wrote:
> On Wed, Sep 09, 2026 at 07:59:55PM +0300, Andrey Drobyshev wrote:
>> On 9/7/26 5:55 PM, Stefano Garzarella wrote:
>>> On Thu, Aug 20, 2026 at 02:39:50PM +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.
>>>
>>> mm, are we sure the device ID is unique?
>>>
>>> I just tried this whitout receiving any error:
>>> qemu-system-x86_64 -smp 2 -M q35,accel=kvm,memory-backend=vsock0 \
>>>      -object memory-backend-memfd,id=vsock0,size=512M \
>>>      -device vhost-vsock-pci,id=vsock0,guest-cid=3
>>>
>>> Stefano
>> You're right, that seems to be the a real issue.  In fact there's even a
>> UAF bug IIUC, stemming from the fact that we: 1) provide key destroy
>> function when creating the CPR hash table; 2) use same CprFd for both
>> key and value.
>>
>> That means that when we have several devices supporting CPR with
>> repeating IDs, we do:
>>
>>  g_hash_table_insert(table, fd1, fd1);    // table has fd1 -> fd1
>>  g_hash_table_insert(table, fd2, fd2);    // table has fd1 -> fd2
>>
>> Here fd1 and fd2 have same hash of course. According to GLib docs [1],
>> value is renewed in this case, but the new key is destroyed and the old
>> one is used.  As a result, fd2 is freed.
>>
>> And, mind you, this all happens long before actual CPR, cause the
>> devices with CPR support usually call cpr_save_fd() somewhere in
>> .realize().  Thus we get memory corruption right at boot time, plus
>> subsequent CPR will likely crash.
>>
>> In these circumstances I'd personally prefer just asserting on repeating
>> key somewhere in cpr_fd_hash_insert().  The only downside is that it'll
>> crash when booting 2 CPR supporting devices having identical IDs, even
>> if the user isn't going to perform CPR.  I'd say it's acceptable, what
>> do you think?
> 
> I'd prefer an error if possible.
> 
> That said, can we append a prefix to the key (e.g. `vhost-vsock`)?
> In this way we reduce the chance to have a conflict. Of course the user 
> can still assign `vhost-vsock-something` to the memory backend and 
> `something` to the vhost-vsock device, but yeah xD
> 
> Thanks,
> Stefano
>

If you mean erroring out directly at startup whenever we encounter a
duplicate CPR key, with a clear error message instead of abort/crash - I
agree that it's a better solution.  That'll likely require changing the
CPR FD saving API, but shouldn't be too intrusive.  As for the prefix -
once we forbid duplicate keys entirely, there's no need for it.

Andrey

Re: [PATCH v4 10/10] vhost-vsock: hand off device ownership across CPR

[Andrey Drobyshev]

On 9/11/26 10:45 AM, Stefano Garzarella wrote:
> On Wed, Sep 09, 2026 at 08:45:22PM +0300, Andrey Drobyshev wrote:
>> On 9/7/26 6:13 PM, Stefano Garzarella wrote:
>>> On Thu, Aug 20, 2026 at 02:39:55PM +0300, Andrey Drobyshev wrote:
>>>> The previous patches reuse the source vhost FD on the destination,
>>>> however both source and target 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.
>>>>
>>>>  - .set_status() refuses to start a device whose handoff hasn't
>>>>    completed: not fully initialized yet, or left ownerless after a
>>>>    failed CPR.
>>>>
>>>> With the handoff in place, lift the CPR blocker: only ID-less devices,
>>>> which can't preserve their FDs, remain blocked.
>>>>
>>>> Suggested-by: Dongli Zhang <dongli.zhang@oracle.com>
>>>> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>>>> ---
>>>> hw/virtio/vhost-vsock.c         | 165 ++++++++++++++++++++++++++++----
>>>> include/hw/virtio/vhost-vsock.h |   5 +-
>>>> 2 files changed, 152 insertions(+), 18 deletions(-)
>>>>
>>>> diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
>>>> index cec2c31f0ca..e25e5350905 100644
>>>> --- a/hw/virtio/vhost-vsock.c
>>>> +++ b/hw/virtio/vhost-vsock.c
>>>> @@ -73,9 +73,23 @@ static int vhost_vsock_set_running(VirtIODevice *vdev, int start)
>>>> static int vhost_vsock_set_status(VirtIODevice *vdev, uint8_t status)
>>>> {
>>>>     VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(vdev);
>>>> +    VHostVSock *vsock = VHOST_VSOCK(vdev);
>>>>     bool should_start = virtio_device_should_start(vdev, status);
>>>> +    bool initialized = vhost_dev_is_initialized(&vvc->vhost_dev);
>>>>     int ret;
>>>>
>>>> +    /*
>>>> +     * During CPR, on target the full vhost_dev_init() is deferred to
>>>> +     * post_load.  On source the device is left without an owner if a
>>>> +     * failed CPR couldn't re-acquire it.  Refuse to start in both cases
>>>> +     * rather than issue vhost ioctls on a half-initialised or ownerless
>>>> +     * device.
>>>> +     */
>>>> +    if (should_start && (!initialized || vsock->owner_reset)) {
>>>> +        error_report("vhost-vsock: refusing to start, device init incomplete");
>>>> +        return 0;
>>>
>>> Why returning 0?
>>
>> That's just what every other error path does here, because return value
>> for vhost_vsock_set_status() goes nowhere.  The only thing it'd
>> influence is having one more line in the QEMU log when we're called from
>> virtio_set_status().
> 
> Ah okay, thanks for pointing out :-)
> 
>>>> +    }
>>>> +
>>>>     if (vhost_dev_is_started(&vvc->vhost_dev) == should_start) {
>>>>         return 0;
>>>>     }
>>>> @@ -111,8 +125,67 @@ 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_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) {
>>>
>>> IIUC MIG_EVENT_FAILED only covers the source failing. If the source
>>> completes it successfully and something goes wrong afterwards (e.g. in
>>> the destination), what will happen?
>>
>> Hmm that indeed might be a problem for cpr-transfer.  E.g. target fails,
>> management (libvirt) sends qmp-cont to source, and we bail out with
>> error in vhost_vsock_set_status().
>>> Thinking if we can try to call vhost_dev_set_owner() in
>>> vhost_vsock_set_status(), expecting also -EBUSY, instead of relying on
>>> `vsock->owner_reset`.
>>
>> Moving .set_owner() to .set_status() actually looks promising, I'll try
>> it out and see how that works.  Although I'd still keep the flag and
>> restrain from tolerating -EBUSY - we shouldn't proceed to realize the
>> device if we don't own it.
> 
> Yeah, agree.
> 
>>
>>
>>>> +        ret = vhost_dev_set_owner(&vvc->vhost_dev);
>>>> +        if (ret < 0) {
>>>> +            error_report("vhost-vsock: failed to re-acquire owner: %d", ret);
>>>
>>> Is it okay to return 0 also in this case?
>>
>> As of now it's mandatory cause migration_call_notifiers() asserts on
>> type == MIG_EVENT_SETUP whenever notifier returns non-null.
> 
> I see, thanks.
> 
>>>> +        } 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)) {
>>>
>>> Am I missing something that keeps pre_save from running outside an
>>> actual CPR migration (savevm, or any save path taken while mode is still
>>> set from an earlier attempt), or should the condition be tied to a CPR
>>> migration actually being in flight rather than to the mode parameter
>>> alone?
>>
>> You're right, that's a real bug.  Should probably guard it with
>> cpr_incoming_needed(NULL) && migration_is_running().
> 
> Yeah, that looks better to me too.

Actually I checked, and this is not enough:

  qemu_savevm_state() ->
    migrate_init() ->
      migrate_set_state(MIGRATION_STATUS_SETUP)

And migration_is_running() returns true when in MIGRATION_STATUS_SETUP.
So on savevm we still encounter the same bug.

Now, the savevm/loadvm operation doesn't make any sense at all in
cpr-exec or cpr-transfer mode.  That's because savevm/loadvm suggests
saving VM state, which implies saving its RAM entirely.  And the whole
point ot CPR is avoiding saving entire RAM (see migrate_ram_is_ignored()
in migration/ram.c).

So I'm guessing the proper fix would be patching migrate_can_snapshot(),
which guards 'save/load-snapshot' and 'savevm'/'loadvm' commands, to
make sure they can never fire with any of CPR modes set.  I'll add this
as a separate patch.

migration_is_running() should probably be kept here to guard from things
like qemu_save_device_state().
>>>> +        ret = vhost_dev_reset_owner(&vvc->vhost_dev);
>>>> +        if (ret < 0) {
>>>> +            error_report("vhost-vsock: vhost_reset_owner failed: %d", ret);
>>>
>>> vhost-vsock devices that don't support `reset_owner` will fail here,
>>> right? Is that what we expect?
>>
>> Which devices you mean?  No other vhost-vsock devices should ever end up
>> here, e.g. vhost-user-vsock has .unmigratable = 1 set, so .pre_save() is
>> never run for it.  If you mean the kernels which don't support
>> RESET_OWNER - then yes, we'll fail here.
> 
> Yep, I meant the kernel.
> 
> I'd like to better understand what happens when we perform CPR with a 
> vhost-vsock device and a kernel that doesn't support RESET_OWNER. Both 
> before and after this series.

Before the series obviously nobody was issuing RESET_OWNER.  And without
the RESET_OWNER, AFAICT, cpr-transfer fails on the target at
SET_GUEST_CID with EADDRINUSE because the source still holds the CID.

After the series, whenever RESET_OWNER operation fails, and it fails on
the migration source in .pre_save(), we get:

  - 'migrate' command itself returns success;
  - 'query-migrate' reports status=failed with "pre-save failed...";
  - QEMU's stderr ot log contains "vhost-vsock: vhost_reset_owner
    failed: %d".

That's what happens on source.  If it's cpr-transfer - target dies
reading a truncated stream.
>>>> +            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
>>>> @@ -122,6 +195,32 @@ static int vhost_vsock_post_load(void *opaque, int version_id)
>>>>         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;
>>>> +    }
>>>> +
>>>> +    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);
>>>> +        vhost_dev_cleanup(&vvc->vhost_dev);
>>>> +        return ret;
>>>> +    }
>>>> +
>>>>     return 0;
>>>> }
>>>>
>>>> @@ -133,7 +232,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 +243,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;
>>>>
>>>> @@ -159,19 +259,30 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp)
>>>>     }
>>>>
>>>>     /*
>>>> -     * CPR migration of a vhost-vsock device is not supported yet: the
>>>> -     * device ownership is not handed over, so the target fails to set
>>>> -     * up its device.  Fail the migration early and gracefully instead.
>>>> +     * With the ownership handoff in place CPR migration is now supported.
>>>> +     * Having a unique ID is mandatory for FD preservation during it, thus
>>>> +     * we only keep the migration blocker for the ID-less case.
>>>>      */
>>>> -    error_setg(&vsock->migration_blocker,
>>>> -               "vhost-vsock: CPR migration is not supported");
>>>> -    if (migrate_add_blocker_modes(&vsock->migration_blocker,
>>>> -                                  BIT(MIG_MODE_CPR_TRANSFER) |
>>>> -                                  BIT(MIG_MODE_CPR_EXEC), errp) < 0) {
>>>> -        return;
>>>> +    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()) {
>>>> +    /*
>>>> +     * 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 "
>>>> @@ -204,14 +315,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 */
>>>
>>> Okay, but IIUC this is returned by cpr_find_fd(), is that funtion
>>> passing the ownership of the fd, even that the fd is not removed from
>>> the hash table?
>>>
>>> Should we call cpr_delete_fd() when we take it?
>>
>> You're right, that should be a proper cleanup.  Every path that closes
>> the fd after a successful cpr_find_fd() should also drop the entry.
>> Will add this.
> 
> I don't know how CPR works, but if we don't close the FD here, will it 
> be closed later? I mean, if we leave it on the map, will someone else 
> close it?

CPR code doesn't close any FDs on its own.  Those FDs are owned by the
devices and we only account them so that they stay opened and their
numbers get transfered to the CPR target - that's it.  If the caller
doesn't close them on a failed .realize(), they get leaked.  Or, if we
close them but keep the entry in CPR registry - target gets a stale FD
number.
> That way, we don't have a different situation here.
> 
> That said, I'm fine also with your solution.

Yes, closing fds on the caller side still looks ultimately better.

Andrey
> Thanks,
> Stefano
>