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.
The exit notifier is not good enough here, since in case 'return-path' capability is enabled, the target reports the failure to the source before exiting. In this case the source resumes right away and is racing with the target's exit() for the ownership. Add an incoming migration failure notifier to each vhost_dev, registered for its lifetime, that releases the ownership if still held. It runs before the failure is reported to the source and before the target exits. A target killed with SIGKILL or crashing still leaves the device unrecoverable. Reported-by: Akihiko Odaki <[email protected]> Signed-off-by: Andrey Drobyshev <[email protected]> --- hw/virtio/vhost.c | 24 ++++++++++++++++++++++++ include/hw/virtio/vhost.h | 6 ++++++ 2 files changed, 30 insertions(+) diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 1241c5a954d..86e9a234638 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,22 @@ 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 target dies or before the source + * learns about the failure and resumes, so that the source can take the + * device back. + */ +static void vhost_incoming_failure_notify(Notifier *notifier, void *data) +{ + struct vhost_dev *hdev = container_of(notifier, struct vhost_dev, + incoming_failure_notifier); + + if (hdev->owner) { + vhost_dev_reset_owner(hdev); + } +} + unsigned int vhost_get_max_memslots(void) { unsigned int max = UINT_MAX; @@ -1823,6 +1840,8 @@ 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); + hdev->incoming_failure_notifier.notify = vhost_incoming_failure_notify; + migration_incoming_add_failure_notifier(&hdev->incoming_failure_notifier); QLIST_INSERT_HEAD(&vhost_devices, hdev, entry); /* @@ -1872,6 +1891,11 @@ void vhost_dev_cleanup(struct vhost_dev *hdev) memory_listener_unregister(&hdev->memory_listener); QLIST_REMOVE(hdev, entry); } + if (hdev->incoming_failure_notifier.notify) { + migration_incoming_remove_failure_notifier( + &hdev->incoming_failure_notifier); + hdev->incoming_failure_notifier.notify = NULL; + } 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..6df012859a6 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; + /* + * @incoming_failure_notifier: releases the ownership when an incoming + * migration fails, vhost_incoming_failure_notify() + */ + Notifier incoming_failure_notifier; bool log_enabled; uint64_t log_size; Error *migration_blocker; -- 2.47.1
