[v2,08/17] migration: Add load_finish handler and associated functions

Commit Message

Maciej S. Szmigiero Aug. 27, 2024, 5:54 p.m. UTC

From: "Maciej S. Szmigiero" <[e-mail]>

load_finish SaveVMHandler allows migration code to poll whether
a device-specific asynchronous device state loading operation had finished.

In order to avoid calling this handler needlessly the device is supposed
to notify the migration code of its possible readiness via a call to
qemu_loadvm_load_finish_ready_broadcast() while holding
qemu_loadvm_load_finish_ready_lock.

Signed-off-by: Maciej S. Szmigiero <[e-mail]>
---
 include/migration/register.h | 21 +++++++++++++++
 migration/migration.c        |  6 +++++
 migration/migration.h        |  3 +++
 migration/savevm.c           | 52 ++++++++++++++++++++++++++++++++++++
 migration/savevm.h           |  4 +++
 5 files changed, 86 insertions(+)

Comments

Fabiano Rosas Aug. 30, 2024, 7:28 p.m. UTC | #1

"Maciej S. Szmigiero" <[e-mail]> writes:

> From: "Maciej S. Szmigiero" <[e-mail]>
>
> load_finish SaveVMHandler allows migration code to poll whether
> a device-specific asynchronous device state loading operation had finished.
>
> In order to avoid calling this handler needlessly the device is supposed
> to notify the migration code of its possible readiness via a call to
> qemu_loadvm_load_finish_ready_broadcast() while holding
> qemu_loadvm_load_finish_ready_lock.
>
> Signed-off-by: Maciej S. Szmigiero <[e-mail]>
> ---
>  include/migration/register.h | 21 +++++++++++++++
>  migration/migration.c        |  6 +++++
>  migration/migration.h        |  3 +++
>  migration/savevm.c           | 52 ++++++++++++++++++++++++++++++++++++
>  migration/savevm.h           |  4 +++
>  5 files changed, 86 insertions(+)
>
> diff --git a/include/migration/register.h b/include/migration/register.h
> index 4a578f140713..44d8cf5192ae 100644
> --- a/include/migration/register.h
> +++ b/include/migration/register.h
> @@ -278,6 +278,27 @@ typedef struct SaveVMHandlers {
>      int (*load_state_buffer)(void *opaque, char *data, size_t data_size,
>                               Error **errp);
>  
> +    /**
> +     * @load_finish
> +     *
> +     * Poll whether all asynchronous device state loading had finished.
> +     * Not called on the load failure path.
> +     *
> +     * Called while holding the qemu_loadvm_load_finish_ready_lock.
> +     *
> +     * If this method signals "not ready" then it might not be called
> +     * again until qemu_loadvm_load_finish_ready_broadcast() is invoked
> +     * while holding qemu_loadvm_load_finish_ready_lock.
> +     *
> +     * @opaque: data pointer passed to register_savevm_live()
> +     * @is_finished: whether the loading had finished (output parameter)
> +     * @errp: pointer to Error*, to store an error if it happens.
> +     *
> +     * Returns zero to indicate success and negative for error
> +     * It's not an error that the loading still hasn't finished.
> +     */
> +    int (*load_finish)(void *opaque, bool *is_finished, Error **errp);
> +
>      /**
>       * @load_setup
>       *
> diff --git a/migration/migration.c b/migration/migration.c
> index 3dea06d57732..d61e7b055e07 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -259,6 +259,9 @@ void migration_object_init(void)
>  
>      current_incoming->exit_on_error = INMIGRATE_DEFAULT_EXIT_ON_ERROR;
>  
> +    qemu_mutex_init(&current_incoming->load_finish_ready_mutex);
> +    qemu_cond_init(&current_incoming->load_finish_ready_cond);
> +
>      migration_object_check(current_migration, &error_fatal);
>  
>      ram_mig_init();
> @@ -410,6 +413,9 @@ void migration_incoming_state_destroy(void)
>          mis->postcopy_qemufile_dst = NULL;
>      }
>  
> +    qemu_mutex_destroy(&mis->load_finish_ready_mutex);
> +    qemu_cond_destroy(&mis->load_finish_ready_cond);
> +
>      yank_unregister_instance(MIGRATION_YANK_INSTANCE);
>  }
>  
> diff --git a/migration/migration.h b/migration/migration.h
> index 38aa1402d516..4e2443e6c8ec 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -230,6 +230,9 @@ struct MigrationIncomingState {
>  
>      /* Do exit on incoming migration failure */
>      bool exit_on_error;
> +
> +    QemuCond load_finish_ready_cond;
> +    QemuMutex load_finish_ready_mutex;

With these moved to MigrationState:

Reviewed-by: Fabiano Rosas <[e-mail]>

Avihai Horon Sept. 5, 2024, 3:13 p.m. UTC | #2

On 27/08/2024 20:54, Maciej S. Szmigiero wrote:
> External email: Use caution opening links or attachments
>
>
> From: "Maciej S. Szmigiero" <[e-mail]>
>
> load_finish SaveVMHandler allows migration code to poll whether
> a device-specific asynchronous device state loading operation had finished.
>
> In order to avoid calling this handler needlessly the device is supposed
> to notify the migration code of its possible readiness via a call to
> qemu_loadvm_load_finish_ready_broadcast() while holding
> qemu_loadvm_load_finish_ready_lock.
>
> Signed-off-by: Maciej S. Szmigiero <[e-mail]>
> ---
>   include/migration/register.h | 21 +++++++++++++++
>   migration/migration.c        |  6 +++++
>   migration/migration.h        |  3 +++
>   migration/savevm.c           | 52 ++++++++++++++++++++++++++++++++++++
>   migration/savevm.h           |  4 +++
>   5 files changed, 86 insertions(+)
>
> diff --git a/include/migration/register.h b/include/migration/register.h
> index 4a578f140713..44d8cf5192ae 100644
> --- a/include/migration/register.h
> +++ b/include/migration/register.h
> @@ -278,6 +278,27 @@ typedef struct SaveVMHandlers {
>       int (*load_state_buffer)(void *opaque, char *data, size_t data_size,
>                                Error **errp);
>
> +    /**
> +     * @load_finish
> +     *
> +     * Poll whether all asynchronous device state loading had finished.
> +     * Not called on the load failure path.
> +     *
> +     * Called while holding the qemu_loadvm_load_finish_ready_lock.
> +     *
> +     * If this method signals "not ready" then it might not be called
> +     * again until qemu_loadvm_load_finish_ready_broadcast() is invoked
> +     * while holding qemu_loadvm_load_finish_ready_lock.
> +     *
> +     * @opaque: data pointer passed to register_savevm_live()
> +     * @is_finished: whether the loading had finished (output parameter)
> +     * @errp: pointer to Error*, to store an error if it happens.
> +     *
> +     * Returns zero to indicate success and negative for error
> +     * It's not an error that the loading still hasn't finished.
> +     */
> +    int (*load_finish)(void *opaque, bool *is_finished, Error **errp);
> +
>       /**
>        * @load_setup
>        *
> diff --git a/migration/migration.c b/migration/migration.c
> index 3dea06d57732..d61e7b055e07 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -259,6 +259,9 @@ void migration_object_init(void)
>
>       current_incoming->exit_on_error = INMIGRATE_DEFAULT_EXIT_ON_ERROR;
>
> +    qemu_mutex_init(&current_incoming->load_finish_ready_mutex);
> +    qemu_cond_init(&current_incoming->load_finish_ready_cond);
> +
>       migration_object_check(current_migration, &error_fatal);
>
>       ram_mig_init();
> @@ -410,6 +413,9 @@ void migration_incoming_state_destroy(void)
>           mis->postcopy_qemufile_dst = NULL;
>       }
>
> +    qemu_mutex_destroy(&mis->load_finish_ready_mutex);
> +    qemu_cond_destroy(&mis->load_finish_ready_cond);
> +
>       yank_unregister_instance(MIGRATION_YANK_INSTANCE);
>   }
>
> diff --git a/migration/migration.h b/migration/migration.h
> index 38aa1402d516..4e2443e6c8ec 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -230,6 +230,9 @@ struct MigrationIncomingState {
>
>       /* Do exit on incoming migration failure */
>       bool exit_on_error;
> +
> +    QemuCond load_finish_ready_cond;
> +    QemuMutex load_finish_ready_mutex;
>   };
>
>   MigrationIncomingState *migration_incoming_get_current(void);
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 3fde5ca8c26b..33c9200d1e78 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -3022,6 +3022,37 @@ int qemu_loadvm_state(QEMUFile *f)
>           return ret;
>       }
>
> +    qemu_loadvm_load_finish_ready_lock();
> +    while (!ret) { /* Don't call load_finish() handlers on the load failure path */
> +        bool all_ready = true;

Nit: Maybe rename all_ready to all_finished to be consistent with 
load_finish() terminology? Same for this_ready.

> +        SaveStateEntry *se = NULL;
> +
> +        QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> +            bool this_ready;
> +
> +            if (!se->ops || !se->ops->load_finish) {
> +                continue;
> +            }
> +
> +            ret = se->ops->load_finish(se->opaque, &this_ready, &local_err);
> +            if (ret) {
> +                error_report_err(local_err);
> +
> +                qemu_loadvm_load_finish_ready_unlock();
> +                return -EINVAL;
> +            } else if (!this_ready) {
> +                all_ready = false;
> +            }
> +        }
> +
> +        if (all_ready) {
> +            break;
> +        }
> +
> +        qemu_cond_wait(&mis->load_finish_ready_cond, &mis->load_finish_ready_mutex);
> +    }
> +    qemu_loadvm_load_finish_ready_unlock();
> +
>       if (ret == 0) {
>           ret = qemu_file_get_error(f);
>       }
> @@ -3126,6 +3157,27 @@ int qemu_loadvm_load_state_buffer(const char *idstr, uint32_t instance_id,
>       return 0;
>   }
>
> +void qemu_loadvm_load_finish_ready_lock(void)
> +{
> +    MigrationIncomingState *mis = migration_incoming_get_current();
> +
> +    qemu_mutex_lock(&mis->load_finish_ready_mutex);
> +}
> +
> +void qemu_loadvm_load_finish_ready_unlock(void)
> +{
> +    MigrationIncomingState *mis = migration_incoming_get_current();
> +
> +    qemu_mutex_unlock(&mis->load_finish_ready_mutex);
> +}
> +
> +void qemu_loadvm_load_finish_ready_broadcast(void)
> +{
> +    MigrationIncomingState *mis = migration_incoming_get_current();
> +
> +    qemu_cond_broadcast(&mis->load_finish_ready_cond);

Do we need a broadcast? isn't signal enough as we only have one waiter 
thread?

Thanks.

> +}
> +
>   bool save_snapshot(const char *name, bool overwrite, const char *vmstate,
>                     bool has_devices, strList *devices, Error **errp)
>   {
> diff --git a/migration/savevm.h b/migration/savevm.h
> index d388f1bfca98..69ae22cded7a 100644
> --- a/migration/savevm.h
> +++ b/migration/savevm.h
> @@ -73,4 +73,8 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
>   int qemu_loadvm_load_state_buffer(const char *idstr, uint32_t instance_id,
>                                     char *buf, size_t len, Error **errp);
>
> +void qemu_loadvm_load_finish_ready_lock(void);
> +void qemu_loadvm_load_finish_ready_unlock(void);
> +void qemu_loadvm_load_finish_ready_broadcast(void);
> +
>   #endif

Maciej S. Szmigiero Sept. 9, 2024, 6:05 p.m. UTC | #3

On 5.09.2024 17:13, Avihai Horon wrote:
> 
> On 27/08/2024 20:54, Maciej S. Szmigiero wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> From: "Maciej S. Szmigiero" <[e-mail]>
>>
>> load_finish SaveVMHandler allows migration code to poll whether
>> a device-specific asynchronous device state loading operation had finished.
>>
>> In order to avoid calling this handler needlessly the device is supposed
>> to notify the migration code of its possible readiness via a call to
>> qemu_loadvm_load_finish_ready_broadcast() while holding
>> qemu_loadvm_load_finish_ready_lock.
>>
>> Signed-off-by: Maciej S. Szmigiero <[e-mail]>
>> ---
>>   include/migration/register.h | 21 +++++++++++++++
>>   migration/migration.c        |  6 +++++
>>   migration/migration.h        |  3 +++
>>   migration/savevm.c           | 52 ++++++++++++++++++++++++++++++++++++
>>   migration/savevm.h           |  4 +++
>>   5 files changed, 86 insertions(+)
>>
>> diff --git a/include/migration/register.h b/include/migration/register.h
>> index 4a578f140713..44d8cf5192ae 100644
>> --- a/include/migration/register.h
>> +++ b/include/migration/register.h
>> @@ -278,6 +278,27 @@ typedef struct SaveVMHandlers {
>>       int (*load_state_buffer)(void *opaque, char *data, size_t data_size,
>>                                Error **errp);
>>
>> +    /**
>> +     * @load_finish
>> +     *
>> +     * Poll whether all asynchronous device state loading had finished.
>> +     * Not called on the load failure path.
>> +     *
>> +     * Called while holding the qemu_loadvm_load_finish_ready_lock.
>> +     *
>> +     * If this method signals "not ready" then it might not be called
>> +     * again until qemu_loadvm_load_finish_ready_broadcast() is invoked
>> +     * while holding qemu_loadvm_load_finish_ready_lock.
>> +     *
>> +     * @opaque: data pointer passed to register_savevm_live()
>> +     * @is_finished: whether the loading had finished (output parameter)
>> +     * @errp: pointer to Error*, to store an error if it happens.
>> +     *
>> +     * Returns zero to indicate success and negative for error
>> +     * It's not an error that the loading still hasn't finished.
>> +     */
>> +    int (*load_finish)(void *opaque, bool *is_finished, Error **errp);
>> +
>>       /**
>>        * @load_setup
>>        *
>> diff --git a/migration/migration.c b/migration/migration.c
>> index 3dea06d57732..d61e7b055e07 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -259,6 +259,9 @@ void migration_object_init(void)
>>
>>       current_incoming->exit_on_error = INMIGRATE_DEFAULT_EXIT_ON_ERROR;
>>
>> +    qemu_mutex_init(&current_incoming->load_finish_ready_mutex);
>> +    qemu_cond_init(&current_incoming->load_finish_ready_cond);
>> +
>>       migration_object_check(current_migration, &error_fatal);
>>
>>       ram_mig_init();
>> @@ -410,6 +413,9 @@ void migration_incoming_state_destroy(void)
>>           mis->postcopy_qemufile_dst = NULL;
>>       }
>>
>> +    qemu_mutex_destroy(&mis->load_finish_ready_mutex);
>> +    qemu_cond_destroy(&mis->load_finish_ready_cond);
>> +
>>       yank_unregister_instance(MIGRATION_YANK_INSTANCE);
>>   }
>>
>> diff --git a/migration/migration.h b/migration/migration.h
>> index 38aa1402d516..4e2443e6c8ec 100644
>> --- a/migration/migration.h
>> +++ b/migration/migration.h
>> @@ -230,6 +230,9 @@ struct MigrationIncomingState {
>>
>>       /* Do exit on incoming migration failure */
>>       bool exit_on_error;
>> +
>> +    QemuCond load_finish_ready_cond;
>> +    QemuMutex load_finish_ready_mutex;
>>   };
>>
>>   MigrationIncomingState *migration_incoming_get_current(void);
>> diff --git a/migration/savevm.c b/migration/savevm.c
>> index 3fde5ca8c26b..33c9200d1e78 100644
>> --- a/migration/savevm.c
>> +++ b/migration/savevm.c
>> @@ -3022,6 +3022,37 @@ int qemu_loadvm_state(QEMUFile *f)
>>           return ret;
>>       }
>>
>> +    qemu_loadvm_load_finish_ready_lock();
>> +    while (!ret) { /* Don't call load_finish() handlers on the load failure path */
>> +        bool all_ready = true;
> 
> Nit: Maybe rename all_ready to all_finished to be consistent with load_finish() terminology? Same for this_ready.

Will rename it accordingly.

>> +        SaveStateEntry *se = NULL;
>> +
>> +        QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>> +            bool this_ready;
>> +
>> +            if (!se->ops || !se->ops->load_finish) {
>> +                continue;
>> +            }
>> +
>> +            ret = se->ops->load_finish(se->opaque, &this_ready, &local_err);
>> +            if (ret) {
>> +                error_report_err(local_err);
>> +
>> +                qemu_loadvm_load_finish_ready_unlock();
>> +                return -EINVAL;
>> +            } else if (!this_ready) {
>> +                all_ready = false;
>> +            }
>> +        }
>> +
>> +        if (all_ready) {
>> +            break;
>> +        }
>> +
>> +        qemu_cond_wait(&mis->load_finish_ready_cond, &mis->load_finish_ready_mutex);
>> +    }
>> +    qemu_loadvm_load_finish_ready_unlock();
>> +
>>       if (ret == 0) {
>>           ret = qemu_file_get_error(f);
>>       }
>> @@ -3126,6 +3157,27 @@ int qemu_loadvm_load_state_buffer(const char *idstr, uint32_t instance_id,
>>       return 0;
>>   }
>>
>> +void qemu_loadvm_load_finish_ready_lock(void)
>> +{
>> +    MigrationIncomingState *mis = migration_incoming_get_current();
>> +
>> +    qemu_mutex_lock(&mis->load_finish_ready_mutex);
>> +}
>> +
>> +void qemu_loadvm_load_finish_ready_unlock(void)
>> +{
>> +    MigrationIncomingState *mis = migration_incoming_get_current();
>> +
>> +    qemu_mutex_unlock(&mis->load_finish_ready_mutex);
>> +}
>> +
>> +void qemu_loadvm_load_finish_ready_broadcast(void)
>> +{
>> +    MigrationIncomingState *mis = migration_incoming_get_current();
>> +
>> +    qemu_cond_broadcast(&mis->load_finish_ready_cond);
> 
> Do we need a broadcast? isn't signal enough as we only have one waiter thread?

Currently, there's just one waiter but looking at the relatively small
implementation difference between pthread_cond_signal() and
pthread_cond_broadcast() I'm not sure whether it is worth changing it
it to _signal() and not having a possibility of signalling multiple
waiters upfront.

> Thanks.

Thanks,
Maciej

Peter Xu Sept. 9, 2024, 8:03 p.m. UTC | #4

On Tue, Aug 27, 2024 at 07:54:27PM +0200, Maciej S. Szmigiero wrote:
> From: "Maciej S. Szmigiero" <[e-mail]>
> 
> load_finish SaveVMHandler allows migration code to poll whether
> a device-specific asynchronous device state loading operation had finished.
> 
> In order to avoid calling this handler needlessly the device is supposed
> to notify the migration code of its possible readiness via a call to
> qemu_loadvm_load_finish_ready_broadcast() while holding
> qemu_loadvm_load_finish_ready_lock.
> 
> Signed-off-by: Maciej S. Szmigiero <[e-mail]>
> ---
>  include/migration/register.h | 21 +++++++++++++++
>  migration/migration.c        |  6 +++++
>  migration/migration.h        |  3 +++
>  migration/savevm.c           | 52 ++++++++++++++++++++++++++++++++++++
>  migration/savevm.h           |  4 +++
>  5 files changed, 86 insertions(+)
> 
> diff --git a/include/migration/register.h b/include/migration/register.h
> index 4a578f140713..44d8cf5192ae 100644
> --- a/include/migration/register.h
> +++ b/include/migration/register.h
> @@ -278,6 +278,27 @@ typedef struct SaveVMHandlers {
>      int (*load_state_buffer)(void *opaque, char *data, size_t data_size,
>                               Error **errp);
>  
> +    /**
> +     * @load_finish
> +     *
> +     * Poll whether all asynchronous device state loading had finished.
> +     * Not called on the load failure path.
> +     *
> +     * Called while holding the qemu_loadvm_load_finish_ready_lock.
> +     *
> +     * If this method signals "not ready" then it might not be called
> +     * again until qemu_loadvm_load_finish_ready_broadcast() is invoked
> +     * while holding qemu_loadvm_load_finish_ready_lock.

[1]

> +     *
> +     * @opaque: data pointer passed to register_savevm_live()
> +     * @is_finished: whether the loading had finished (output parameter)
> +     * @errp: pointer to Error*, to store an error if it happens.
> +     *
> +     * Returns zero to indicate success and negative for error
> +     * It's not an error that the loading still hasn't finished.
> +     */
> +    int (*load_finish)(void *opaque, bool *is_finished, Error **errp);

The load_finish() semantics is a bit weird, especially above [1] on "only
allowed to be called once if ..." and also on the locks.

It looks to me vfio_load_finish() also does the final load of the device.

I wonder whether that final load can be done in the threads, then after
everything loaded the device post a semaphore telling the main thread to
continue.  See e.g.:

    if (migrate_switchover_ack()) {
        qemu_loadvm_state_switchover_ack_needed(mis);
    }

IIUC, VFIO can register load_complete_ack similarly so it only sem_post()
when all things are loaded?  We can then get rid of this slightly awkward
interface.  I had a feeling that things can be simplified (e.g., if the
thread will take care of loading the final vmstate then the mutex is also
not needed? etc.).

Patch

diff --git a/include/migration/register.h b/include/migration/register.h
index 4a578f140713..44d8cf5192ae 100644
--- a/include/migration/register.h
+++ b/include/migration/register.h
@@ -278,6 +278,27 @@  typedef struct SaveVMHandlers {
     int (*load_state_buffer)(void *opaque, char *data, size_t data_size,
                              Error **errp);
 
+    /**
+     * @load_finish
+     *
+     * Poll whether all asynchronous device state loading had finished.
+     * Not called on the load failure path.
+     *
+     * Called while holding the qemu_loadvm_load_finish_ready_lock.
+     *
+     * If this method signals "not ready" then it might not be called
+     * again until qemu_loadvm_load_finish_ready_broadcast() is invoked
+     * while holding qemu_loadvm_load_finish_ready_lock.
+     *
+     * @opaque: data pointer passed to register_savevm_live()
+     * @is_finished: whether the loading had finished (output parameter)
+     * @errp: pointer to Error*, to store an error if it happens.
+     *
+     * Returns zero to indicate success and negative for error
+     * It's not an error that the loading still hasn't finished.
+     */
+    int (*load_finish)(void *opaque, bool *is_finished, Error **errp);
+
     /**
      * @load_setup
      *
diff --git a/migration/migration.c b/migration/migration.c
index 3dea06d57732..d61e7b055e07 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -259,6 +259,9 @@  void migration_object_init(void)
 
     current_incoming->exit_on_error = INMIGRATE_DEFAULT_EXIT_ON_ERROR;
 
+    qemu_mutex_init(&current_incoming->load_finish_ready_mutex);
+    qemu_cond_init(&current_incoming->load_finish_ready_cond);
+
     migration_object_check(current_migration, &error_fatal);
 
     ram_mig_init();
@@ -410,6 +413,9 @@  void migration_incoming_state_destroy(void)
         mis->postcopy_qemufile_dst = NULL;
     }
 
+    qemu_mutex_destroy(&mis->load_finish_ready_mutex);
+    qemu_cond_destroy(&mis->load_finish_ready_cond);
+
     yank_unregister_instance(MIGRATION_YANK_INSTANCE);
 }
 
diff --git a/migration/migration.h b/migration/migration.h
index 38aa1402d516..4e2443e6c8ec 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -230,6 +230,9 @@  struct MigrationIncomingState {
 
     /* Do exit on incoming migration failure */
     bool exit_on_error;
+
+    QemuCond load_finish_ready_cond;
+    QemuMutex load_finish_ready_mutex;
 };
 
 MigrationIncomingState *migration_incoming_get_current(void);
diff --git a/migration/savevm.c b/migration/savevm.c
index 3fde5ca8c26b..33c9200d1e78 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -3022,6 +3022,37 @@  int qemu_loadvm_state(QEMUFile *f)
         return ret;
     }
 
+    qemu_loadvm_load_finish_ready_lock();
+    while (!ret) { /* Don't call load_finish() handlers on the load failure path */
+        bool all_ready = true;
+        SaveStateEntry *se = NULL;
+
+        QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
+            bool this_ready;
+
+            if (!se->ops || !se->ops->load_finish) {
+                continue;
+            }
+
+            ret = se->ops->load_finish(se->opaque, &this_ready, &local_err);
+            if (ret) {
+                error_report_err(local_err);
+
+                qemu_loadvm_load_finish_ready_unlock();
+                return -EINVAL;
+            } else if (!this_ready) {
+                all_ready = false;
+            }
+        }
+
+        if (all_ready) {
+            break;
+        }
+
+        qemu_cond_wait(&mis->load_finish_ready_cond, &mis->load_finish_ready_mutex);
+    }
+    qemu_loadvm_load_finish_ready_unlock();
+
     if (ret == 0) {
         ret = qemu_file_get_error(f);
     }
@@ -3126,6 +3157,27 @@  int qemu_loadvm_load_state_buffer(const char *idstr, uint32_t instance_id,
     return 0;
 }
 
+void qemu_loadvm_load_finish_ready_lock(void)
+{
+    MigrationIncomingState *mis = migration_incoming_get_current();
+
+    qemu_mutex_lock(&mis->load_finish_ready_mutex);
+}
+
+void qemu_loadvm_load_finish_ready_unlock(void)
+{
+    MigrationIncomingState *mis = migration_incoming_get_current();
+
+    qemu_mutex_unlock(&mis->load_finish_ready_mutex);
+}
+
+void qemu_loadvm_load_finish_ready_broadcast(void)
+{
+    MigrationIncomingState *mis = migration_incoming_get_current();
+
+    qemu_cond_broadcast(&mis->load_finish_ready_cond);
+}
+
 bool save_snapshot(const char *name, bool overwrite, const char *vmstate,
                   bool has_devices, strList *devices, Error **errp)
 {
diff --git a/migration/savevm.h b/migration/savevm.h
index d388f1bfca98..69ae22cded7a 100644
--- a/migration/savevm.h
+++ b/migration/savevm.h
@@ -73,4 +73,8 @@  int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
 int qemu_loadvm_load_state_buffer(const char *idstr, uint32_t instance_id,
                                   char *buf, size_t len, Error **errp);
 
+void qemu_loadvm_load_finish_ready_lock(void);
+void qemu_loadvm_load_finish_ready_unlock(void);
+void qemu_loadvm_load_finish_ready_broadcast(void);
+
 #endif