On Tue, Aug 27, 2024 at 07:54:27PM +0200, Maciej S. Szmigiero wrote:
> From: "Maciej S. Szmigiero" <maciej.szmigi...@oracle.com>
>
> 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 <maciej.szmigi...@oracle.com>
> ---
> 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.).
--
Peter Xu
