On 6/16/26 4:48 PM, Stefano Garzarella wrote: > On Fri, Jun 12, 2026 at 07:57:16PM +0300, Andrey Drobyshev wrote: >> From: Pavel Tikhomirov <[email protected]> >> >> This ioctl is needed for QEMU's CPR (checkpoint-restore) migration of >> the guest with vhost-vsock device. For this to work, we need to reset >> the device ownership on the source side by calling RESET_OWNER, and then >> claim it on the dest side by calling SET_OWNER. We expect not to lose any >> AF_VSOCK connection while this happens. >> >> Signed-off-by: Pavel Tikhomirov <[email protected]> >> --- >> drivers/vhost/vsock.c | 28 ++++++++++++++++++++++++++++ >> 1 file changed, 28 insertions(+) >> >> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c >> index b12221ce6faf..e629886e5cf8 100644 >> --- a/drivers/vhost/vsock.c >> +++ b/drivers/vhost/vsock.c >> @@ -894,6 +894,32 @@ static int vhost_vsock_set_features(struct vhost_vsock >> *vsock, u64 features) >> return -EFAULT; >> } >> >> +static int vhost_vsock_reset_owner(struct vhost_vsock *vsock) >> +{ >> + struct vhost_iotlb *umem; >> + long err; >> + >> + mutex_lock(&vsock->dev.mutex); >> + err = vhost_dev_check_owner(&vsock->dev); >> + if (err) >> + goto done; >> + umem = vhost_dev_reset_owner_prepare(); >> + if (!umem) { >> + err = -ENOMEM; >> + goto done; >> + } >> + /* Follows vhost_vsock_dev_release closely except for guest_cid drop */ >> + vsock_for_each_connected_socket(&vhost_transport.transport, >> + vhost_vsock_reset_orphans); > > In vhost_vsock_reset_orphans() we have: > > rcu_read_lock(); > > /* If the peer is still valid, no need to reset connection */ > if (vhost_vsock_get(vsk->remote_addr.svm_cid, sock_net(sk))) { > rcu_read_unlock(); > return; > } > > IIUC we are not removing the guest cid from the hash table, so this > check will be always true, and nothing is done. > > So, is this call really useful? >
You're right, and it's most probably an artifact from mimicking the vhost_vsock_dev_release() implementation, as mentioned in the comment. In our case this whole iteration is a no-op, we better remove it. BTW earlier I received some feedback from Sashiko AI reviewer, which also spotted that same issue (and some more interesting races): https://sashiko.dev/#/patchset/[email protected] Apparently it only CC's its reviews to [email protected] so you can't see them right away. Just wanted to let you know to save your time here. I'll send a v2 with respect to Sashiko remarks. But of course would be great if you spot some more issues here. >> + vhost_vsock_drop_backends(vsock); >> + vhost_vsock_flush(vsock); >> + vhost_dev_stop(&vsock->dev); >> + vhost_dev_reset_owner(&vsock->dev, umem); >> +done: >> + mutex_unlock(&vsock->dev.mutex); >> + return err; >> +} >> + >> static long vhost_vsock_dev_ioctl(struct file *f, unsigned int ioctl, >> unsigned long arg) >> { >> @@ -937,6 +963,8 @@ static long vhost_vsock_dev_ioctl(struct file *f, >> unsigned int ioctl, >> return -EOPNOTSUPP; >> vhost_set_backend_features(&vsock->dev, features); >> return 0; >> + case VHOST_RESET_OWNER: >> + return vhost_vsock_reset_owner(vsock); >> default: >> mutex_lock(&vsock->dev.mutex); >> r = vhost_dev_ioctl(&vsock->dev, ioctl, argp); >> -- >> 2.47.1 >> >
