On 9/22/26 12:26 PM, Stefano Garzarella wrote: > On Fri, Sep 18, 2026 at 04:47:05PM +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 restart: >> >> - .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. >> >> - .set_status() refuses to start a device whose init hasn't completed >> yet (incoming CPR before post_load). >> >> - .set_status() re-acquires ownership before starting the device if >> pre_save released it and this QEMU is about to run the guest again: >> the migration failed, or it completed and the management resumed the >> source (e.g. cpr-transfer target died). >> >> With the handoff in place, lift the CPR blocker. >> >> Note: this requires kernel side support of VHOST_RESET_OWNER operation >> for vhost-vsock. Otherwise CPR attempt fails in .pre_save(). >> >> Suggested-by: Dongli Zhang <[email protected]> >> Signed-off-by: Andrey Drobyshev <[email protected]> >> --- >> hw/virtio/vhost-vsock.c | 157 >> ++++++++++++++++++++++++++++++++-------- >> include/hw/virtio/vhost-vsock.h | 2 +- >> 2 files changed, 128 insertions(+), 31 deletions(-) >> >> diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c >> index 6e89007b2ce..65ba573da4d 100644 >> --- a/hw/virtio/vhost-vsock.c >> +++ b/hw/virtio/vhost-vsock.c >> @@ -21,7 +21,6 @@ >> #include "hw/virtio/vhost-vsock.h" >> #include "monitor/monitor.h" >> #include "migration/cpr.h" >> -#include "migration/blocker.h" >> #include "migration/misc.h" >> >> static void vhost_vsock_get_config(VirtIODevice *vdev, uint8_t *config) >> @@ -85,14 +84,46 @@ static char *vhost_vsock_cpr_name(DeviceState *dev) >> 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); >> int ret; >> >> + /* >> + * During CPR, on target the full vhost_dev_init() is deferred to >> + * post_load. Refuse to start a half-initialised device rather than >> + * issue vhost ioctls on it. >> + */ >> + if (should_start && !vhost_dev_is_initialized(&vvc->vhost_dev)) { >> + error_report("vhost-vsock: refusing to start, device init >> incomplete"); >> + return 0; >> + } >> + >> if (vhost_dev_is_started(&vvc->vhost_dev) == should_start) { >> return 0; >> } >> >> if (should_start) { >> + /* >> + * vsock->owner_reset is only set in .pre_save() on CPR migration >> + * source, after a successful RESET_OWNER op. If we end up here >> + * with this flag set - that means that either CPR migration failed, >> + * or we got 'cont' cmd from the management (e.g. cpr-transfer >> + * target died). In either case, we need to re-acquire device >> + * ownership. >> + * >> + * On failure leave the device stopped. The next start attempt >> + * (guest driver reset, VM stop/cont) retries. >> + */ >> + if (vsock->owner_reset) { >> + ret = vhost_dev_set_owner(&vvc->vhost_dev); >> + if (ret < 0) { >> + error_report("vhost-vsock: cannot re-acquire device " >> + "ownership: %s", strerror(-ret)); >> + return 0; >> + } >> + vsock->owner_reset = false; >> + } >> + >> ret = vhost_vsock_common_start(vdev); >> if (ret < 0) { >> return 0; >> @@ -123,8 +154,44 @@ static uint64_t vhost_vsock_get_features(VirtIODevice >> *vdev, >> return vhost_vsock_common_get_features(vdev, requested_features, errp); >> } >> >> +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) && migration_is_running() && >> + !vsock->owner_reset) { >> + ret = vhost_dev_reset_owner(&vvc->vhost_dev); >> + if (ret < 0) { >> + error_report("vhost-vsock: vhost_reset_owner failed: %s", >> + strerror(-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); >> + g_autofree char *cpr_name = vhost_vsock_cpr_name(DEVICE(opaque)); >> + 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 >> @@ -134,6 +201,35 @@ 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(cpr_name, 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); >> + cpr_delete_fd(cpr_name, 0); >> + return ret; >> + } >> + >> + ret = vhost_vsock_set_guest_cid(vdev); > > Why we need to set the CID? Isn't it preserved? > > That said, I don't think it is wrong, it seems just superfluous, but > maybe there is a reason. > > The rest LGTM: > > Reviewed-by: Stefano Garzarella <[email protected]>
You're right that the CID itself is preserved across CPR, the kernel keeps it hashed. But the ownership of the device is NOT preserved. The ownership must be claimed by the new QEMU. After we issue RESET_OWNER in .pre_save() (old QEMU), it must be followed by SET_OWNER in .post_load() (new QEMU). So setting the same CID value in this call is indeed redundant, but that's not the main goal here. Thanks, Andrey
