On Wed, Sep 30, 2026 at 01:13:14PM +0300, Andrey Drobyshev wrote:
vhost_dev_init() takes the device ownership with VHOST_SET_OWNER and
then may still fail, e.g. on virtqueue init. Its cleanup only closes
the backend fd, which doesn't release the ownership if the same file
is open elsewhere. That's the case on a CPR target: the source keeps
the fd, and once it's resumed after the failed migration it can't take
the device back, SET_OWNER fails with EBUSY.
Release the ownership explicitly on the failure paths past SET_OWNER.
Reported-by: Akihiko Odaki <[email protected]>
Signed-off-by: Andrey Drobyshev <[email protected]>
---
hw/virtio/vhost.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
If you have to send another version, maybe better to move the
introduction of vhost_dev_reset_owner() in this patch since IIUC this is
the first place where we use it. (not a strong opinion)
That said, what happen if the caller of vhost_dev_init() fails after
calling it? Who will call vhost_dev_reset_owner()?
E.g. in vhost_vsock_post_load(), if vhost_vsock_set_guest_cid() fails,
should we call vhost_dev_reset_owner()?
Since IIUC all error paths call vhost_dev_cleanup(), might it make sense
to call vhost_dev_reset_owner() there?
Thanks,
Stefano
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index c01ccc800cb..3ff54879068 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1754,7 +1754,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
error_append_hint(errp, "Try plugging this vhost backend before"
" plugging such memory devices.\n");
r = -EINVAL;
- goto fail;
+ goto fail_owner;
}
for (i = 0; i < hdev->nvqs; ++i, ++n_initialized_vqs) {
@@ -1762,7 +1762,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
busyloop_timeout);
if (r < 0) {
error_setg_errno(errp, -r, "Failed to initialize virtqueue %d", i);
- goto fail;
+ goto fail_owner;
}
}
@@ -1799,7 +1799,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
if (hdev->migration_blocker != NULL) {
r = migrate_add_blocker_normal(&hdev->migration_blocker, errp);
if (r < 0) {
- goto fail;
+ goto fail_owner;
}
}
@@ -1831,7 +1831,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
" than current number of used (%d) and reserved (%d)"
" memory slots for memory devices.", limit, used, reserved);
r = -EINVAL;
- goto fail;
+ goto fail_owner;
}
hdev->initialized = true;
@@ -1840,6 +1840,8 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
return 0;
+fail_owner:
+ vhost_dev_reset_owner(hdev);
fail:
hdev->nvqs = n_initialized_vqs;
vhost_dev_cleanup(hdev);
--
2.47.1