vhost_dev_cleanup() closes the backend FD, but that doesn't release the device ownership taken with VHOST_SET_OWNER if the same file is open elsewhere. That's the case on a CPR target: the source keeps the FD, so if the target fails after SET_OWNER, either inside vhost_dev_init() or later in its caller, the ownership stays with the target. Once the source is resumed it can't take the device back, SET_OWNER fails with EBUSY.
Track the ownership with a new 'owner' flag in struct vhost_dev, set and cleared by vhost_dev_set_owner() and vhost_dev_reset_owner(), and issue VHOST_RESET_OWNER in vhost_dev_cleanup() if it's set, as the common teardown for all those paths. That should be safe in any case: the kernel refuses it from a non-owner, and an owner is about to close the FD anyway. Reported-by: Akihiko Odaki <[email protected]> Suggested-by: Stefano Garzarella <[email protected]> Signed-off-by: Andrey Drobyshev <[email protected]> --- hw/virtio/vhost.c | 24 ++++++++++++++++++++++-- include/hw/virtio/vhost.h | 5 +++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index c01ccc800cb..3432b9a43c8 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -1705,20 +1705,32 @@ int vhost_dev_init_backend(struct vhost_dev *hdev, void *opaque, int vhost_dev_set_owner(struct vhost_dev *hdev) { + int r; + assert(hdev->vhost_ops); if (!hdev->vhost_ops->vhost_set_owner) { return -ENOSYS; } - return hdev->vhost_ops->vhost_set_owner(hdev); + r = hdev->vhost_ops->vhost_set_owner(hdev); + if (!r) { + hdev->owner = true; + } + return r; } int vhost_dev_reset_owner(struct vhost_dev *hdev) { + int r; + assert(hdev->vhost_ops); if (!hdev->vhost_ops->vhost_reset_owner) { return -ENOSYS; } - return hdev->vhost_ops->vhost_reset_owner(hdev); + r = hdev->vhost_ops->vhost_reset_owner(hdev); + if (!r) { + hdev->owner = false; + } + return r; } int vhost_dev_init(struct vhost_dev *hdev, void *opaque, @@ -1864,6 +1876,14 @@ void vhost_dev_cleanup(struct vhost_dev *hdev) g_free(hdev->mem); g_free(hdev->mem_sections); if (hdev->vhost_ops) { + /* + * Release the device ownership if we hold it. Needed e.g. for + * failing CPR target, so the source can re-acquire it. + */ + if (hdev->owner && vhost_dev_reset_owner(hdev) < 0) { + error_report("vhost: cannot release device ownership on " + "cleanup"); + } hdev->vhost_ops->vhost_cleanup(hdev); } assert(!hdev->log); diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h index 73ea130d7ee..f6e32b59ab2 100644 --- a/include/hw/virtio/vhost.h +++ b/include/hw/virtio/vhost.h @@ -111,6 +111,11 @@ struct vhost_dev { bool started; /* @initialized: has the full vhost_dev_init() completed? */ bool initialized; + /* + * @owner: has this vhost_dev acquired the device ownership? + * Updated only by vhost_dev_set_owner()/vhost_dev_reset_owner() + */ + bool owner; bool log_enabled; uint64_t log_size; Error *migration_blocker; -- 2.47.1
