On 6/25/26 2:38 AM, Dongli Zhang wrote: > I was trying to understand the error paths and why these calls need to move > here > and there. I think I now see where most of the complications come from :) > > My understanding is that the current design relies *very heavily on > vhost_cpr_notifier()* as the mechanism for releasing vhost ownership before > the > CPR fd handoff. Since the target may receive the vhost fd from > cpr_state_save() > and call VHOST_SET_OWNER before the main migration thread is started, > MIG_EVENT_SETUP is moved before cpr_state_save(). Once SETUP performs > RESET_OWNER, migration_stop_vm() also has to move before SETUP. Otherwise the > source VM could keep running after the vhost owner has been released. > > This is reason to make the fix migration(cpr)-wide rather than > vhost-vsock-local: it changes the SETUP/stop timing for all CPR notifiers, > which > may affect other devices such as VFIO. > > How about to borrow the idea from VFIO? That said, transfer the fd early, but > defer the ownership handoff until the source is stopped and the target is > loading the final device state? > > 1. cpr_state_save() transfers only the CPR fd state. > 2. The target receives the fd in cpr_state_load(). > 3. During target realize, vhost-vsock restores the fd and initializes only the > parts that do not require VHOST_SET_OWNER. > 4. After the source VM is stopped, source vhost-vsock releases ownership from > pre_save with VHOST_RESET_OWNER. > 5. During target post_load, vhost-vsock completes the handoff with > VHOST_SET_OWNER and the owner-dependent initialization. > > Of course, the decision is left to the maintainers :) > > Thank you very much! > > Dongli Zhang >
Hello Dongli, I think the design you suggested might actually work. And I especially like the fact that we don't have to touch the generic migration code at all, moving around the SETUP event and VM stop for our convenience, with potential unwanted side effects. One note though: we can get rid of the SETUP notifier callback, but pre_save will now have a side effect (RESET_OWNER), so in case we couldn't complete CPR migration, we'll have to rely on the FAILED event to do the SET_OWNER and restore the source VM. The bulk of the work will have to be done in the vhost-vsock initialization part, i.e. separating the owner-independent and owner-dependent initialization, but that shouldn't be a big deal. I'll try implementing it and will send in v3 if it works. Andrey > On 2026-06-19 3:55 AM, Andrey Drobyshev wrote: >> v1 -> v2: >> >> Adjustments on the issues pointed out by Dongli: >> >> * Patch 1: avoid emitting SETUP event in resume (postcopy recovery) case; >> * Patch 2: >> - gate vm_resume() op by checking that block graph is activated already; >> - reword commit message; >> - adjust the comment. >> >> Also: >> >> * Patch 4: use more generic proxy object qdev_get_parent_bus()->parent >> instead of container_of(struct VHostVSockPCI) >> >> v1: >> https://urldefense.com/v3/__https://lore.kernel.org/qemu-devel/[email protected]/__;!!ACWV5N9M2RV99hQ!LRVJYpDrzTKttLBQmMWczpaOwMB-ftzvLd8PYJRzKs_s5bvmx0kTjHl6pR0yA5Mb0Nv6YSoWkXTzmwMbLepUOy0bHXFGVenJ7Q$ >> >> >> Andrey Drobyshev (5): >> migration: emit SETUP notification before CPR >> migration: stop VM earlier for CPR >> vhost: reset vhost devices for CPR >> vhost-vsock: preserve vhost FD during CPR >> vhost-vsock: don't reset connections during CPR >> >> hw/virtio/vhost-kernel.c | 6 ++ >> hw/virtio/vhost-vsock.c | 40 +++++++++++-- >> hw/virtio/vhost.c | 33 +++++++++++ >> include/hw/virtio/vhost-backend.h | 1 + >> include/hw/virtio/vhost.h | 1 + >> migration/migration.c | 98 ++++++++++++++++++++++++------- >> 6 files changed, 152 insertions(+), 27 deletions(-) >> >> -- >> 2.47.1 >> >
