On 10/2/26 6:33 AM, Akihiko Odaki wrote: > On 2026/10/01 17:25, 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. > > I'm not requesting changes to this patch, but there is a case where > the source cannot reacquire ownership: > > 1. The destination successfully completes vhost_vsock_post_load(). > > 2. A subsequent load error, such as a truncated VMState section > footer, causes the destination to call exit() without reaching > vhost_dev_cleanup(). > > 3. The source still holds an FD for the same open file, so closing > the destination's FD does not destroy the kernel backend or > release its retained owner. > > 4. Resuming the source with cont calls VHOST_SET_OWNER, which fails > with -EBUSY. Repeated attempts do not resolve the stale ownership. > > An atexit() hook could cover this exit() path, but would not run after > SIGKILL or abort(). This appears to be a broader limitation of the > kernel vhost UAPI when sharing an open file between processes: recovery > cannot rely solely on the destination releasing ownership before it exits. > > Regards, > Akihiko Odaki That's an interesting finding. True that we can't do much for SIGKILL or abort(). However a truncated migstream footer with a subsequent exit() is a legitimate case which we should cover. So if I add the ownership flag into 'struct vhost_dev' as discussed under the previous patch - this should become really easy: we just loop over the vhost_dev instances and issue RESET_OWNER for those which hold the ownership.
Thanks, Andrey
