From: Andrey Drobyshev <[email protected]> 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() as the common teardown for all those paths. Only do it if the ownership was handed over to us by another process, which is marked by the device with a new bool flag 'owner_handed_over'. Upon ownership release failure, we have nothing to unwind at this point (CPR target teardown), so we just report the failure. Reported-by: Akihiko Odaki <[email protected]> Suggested-by: Stefano Garzarella <[email protected]> Signed-off-by: Andrey Drobyshev <[email protected]> Reviewed-by: Stefano Garzarella <[email protected]> Reviewed-by: Michael S. Tsirkin <[email protected]> Reviewed-by: Akihiko Odaki <[email protected]> Signed-off-by: Michael S. Tsirkin <[email protected]> Message-ID: <[email protected]> --- include/hw/virtio/vhost.h | 11 +++++++++++ hw/virtio/vhost.c | 40 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h index 73ea130d7e..efe1eed260 100644 --- a/include/hw/virtio/vhost.h +++ b/include/hw/virtio/vhost.h @@ -111,6 +111,17 @@ 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; + /* + * @owner_handed_over: CPR target side. The ownership was handed over + * to us by another process (the CPR source) and must be given back + * when this vhost_dev goes away. Set by the device. + */ + bool owner_handed_over; bool log_enabled; uint64_t log_size; Error *migration_blocker; diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index c01ccc800c..085e2ef6f1 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -53,6 +53,25 @@ static QLIST_HEAD(, vhost_dev) vhost_log_devs[VHOST_BACKEND_TYPE_MAX]; static QLIST_HEAD(, vhost_dev) vhost_devices = QLIST_HEAD_INITIALIZER(vhost_devices); +/* + * Release the device ownership if it was handed over to us by another + * process, e.g. the CPR source. + */ +static void vhost_dev_try_release_owner(struct vhost_dev *hdev, + const char *when) +{ + int r; + + if (!hdev->owner || !hdev->owner_handed_over) { + return; + } + r = vhost_dev_reset_owner(hdev); + if (r < 0) { + error_report("vhost: cannot release device ownership on %s: %s", + when, strerror(-r)); + } +} + unsigned int vhost_get_max_memslots(void) { unsigned int max = UINT_MAX; @@ -1705,20 +1724,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 +1895,11 @@ 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. + */ + vhost_dev_try_release_owner(hdev, "cleanup"); hdev->vhost_ops->vhost_cleanup(hdev); } assert(!hdev->log); -- MST
