A CPR target that has taken the device ownership in post_load() doesn't always reach vhost_dev_cleanup(): a subsequent load error, e.g. a truncated migration stream footer, makes QEMU exit() right away, and the ownership stays with the dead process. The source still holds the FD, so the kernel never releases the device, and the source can't take it back after being resumed -> SET_OWNER fails with EBUSY.
Releasing the ownership at exit isn't enough either: with the return-path capability the target reports the failure to the source before exiting, and the source resumes right away, racing with the target's exit for the ownership. And with exit-on-error=false the target doesn't exit at all. Add a migration notifier to each vhost_dev, registered for its lifetime for the CPR modes, that releases the ownership if still held on MIG_EVENT_INCOMING_FAILED. It runs before the failure is published in any way. Reported-by: Akihiko Odaki <[email protected]> Signed-off-by: Andrey Drobyshev <[email protected]> --- hw/virtio/vhost.c | 25 +++++++++++++++++++++++++ include/hw/virtio/vhost.h | 6 ++++++ 2 files changed, 31 insertions(+) diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 3432b9a43c8..36e82bc14f0 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -26,6 +26,7 @@ #include "hw/virtio/vhost-user.h" #include "hw/mem/memory-device.h" #include "migration/blocker.h" +#include "migration/misc.h" #include "migration/qemu-file-types.h" #include "system/dma.h" #include "system/memory.h" @@ -53,6 +54,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); +/* + * A CPR target takes the device ownership in post_load(). If the load + * fails afterwards, release it before the source learns about the failure + * and resumes, so that the source can take the device back. + */ +static int vhost_migration_notify(NotifierWithReturn *notifier, + MigrationEvent *e, Error **errp) +{ + struct vhost_dev *hdev = container_of(notifier, struct vhost_dev, + migration_notifier); + + if (e->type == MIG_EVENT_INCOMING_FAILED && hdev->owner && + vhost_dev_reset_owner(hdev) < 0) { + error_report("vhost: cannot release device ownership on " + "incoming migration failure"); + } + return 0; +} + unsigned int vhost_get_max_memslots(void) { unsigned int max = UINT_MAX; @@ -1823,6 +1843,10 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque, hdev->log_enabled = false; hdev->started = false; memory_listener_register(&hdev->memory_listener, &address_space_memory); + migration_add_notifier_modes(&hdev->migration_notifier, + vhost_migration_notify, + BIT(MIG_MODE_CPR_TRANSFER) | + BIT(MIG_MODE_CPR_EXEC)); QLIST_INSERT_HEAD(&vhost_devices, hdev, entry); /* @@ -1872,6 +1896,7 @@ void vhost_dev_cleanup(struct vhost_dev *hdev) memory_listener_unregister(&hdev->memory_listener); QLIST_REMOVE(hdev, entry); } + migration_remove_notifier(&hdev->migration_notifier); migrate_del_blocker(&hdev->migration_blocker); g_free(hdev->mem); g_free(hdev->mem_sections); diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h index f6e32b59ab2..14484b8de24 100644 --- a/include/hw/virtio/vhost.h +++ b/include/hw/virtio/vhost.h @@ -3,6 +3,7 @@ #include "net/vhost_net.h" #include "hw/virtio/vhost-backend.h" +#include "qemu/notify.h" #include "hw/virtio/virtio.h" #include "system/memory.h" @@ -116,6 +117,11 @@ struct vhost_dev { * Updated only by vhost_dev_set_owner()/vhost_dev_reset_owner() */ bool owner; + /* + * @migration_notifier: releases the ownership when an incoming + * migration fails, vhost_migration_notify() + */ + NotifierWithReturn migration_notifier; bool log_enabled; uint64_t log_size; Error *migration_blocker; -- 2.47.1
