On 9/30/26 8:55 AM, Akihiko Odaki wrote: > On 2026/09/30 2:23, 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]> >> Reviewed-by: Stefano Garzarella <[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_dev_init() can successfully issue SET_OWNER and then fail during > virtqueue initialization or memory-slot validation. Its cleanup only > closes the destination FD; the source still holds the same underlying > file, so the kernel retains the destination owner. Resuming the source > then makes vhost_vsock_set_status() repeatedly fail SET_OWNER with > EBUSY, leaving vsock unusable.
That's indeed a real bug. I suppose the fix should be calling RESET_OWNER from vhost_dev_init() on the cleanup path. And it deserves a separate commit predating this one. Thanks for noticing! > I also suggest using GINT_TO_POINTER(). I'd prefer sticking to plain cast here - only for the sake of consistency. GINT_TO_POINTER() can be done later in a separate commit for all such occurrences. Thanks, Andrey > Regards, > Akihiko Odaki
