Re: [PATCH 2/4] vhost/vsock: add VHOST_RESET_OWNER ioctl

[Andrey Drobyshev]

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 <ptikhomirov@xxxxxxxxxxxxx>
>>
>> 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 <ptikhomirov@xxxxxxxxxxxxx>
>> ---
>> 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/20260612165718.433546-1-andrey.drobyshev@xxxxxxxxxxxxx

Apparently it only CC's its reviews to kvm@xxxxxxxxxxxxxxx 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
>>
>