Re: [PATCH v3 01/24] migration: Clarify that {load,save}_cleanup handlers can run without setup

[Cédric Le Goater]

On 11/17/24 20:19, Maciej S. Szmigiero wrote:
> From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
>
> It's possible for {load,save}_cleanup SaveVMHandlers to get called without
> the corresponding {load,save}_setup handler being called first.
>
> One such example is if {load,save}_setup handler of a proceeding device
> returns error.
> In this case the migration core cleanup code will call all corresponding
> cleanup handlers, even for these devices which haven't had its setup
> handler called.
>
> Since this behavior can generate some surprises let's clearly document it
> in these SaveVMHandlers description.

I think we should spend some time analyzing the issues too. I would prefer
to avoid the changes in patch 18 ("vfio/migration: Don't run load cleanup
if load setup didn't run") if possible.

> Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> ---
>   include/migration/register.h | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/include/migration/register.h b/include/migration/register.h
> index f60e797894e5..0b0292738320 100644
> --- a/include/migration/register.h
> +++ b/include/migration/register.h
> @@ -69,7 +69,9 @@ typedef struct SaveVMHandlers {
>       /**
>        * @save_cleanup
>        *
> -     * Uninitializes the data structures on the source
> +     * Uninitializes the data structures on the source.
> +     * Note that this handler can be called even if save_setup
> +     * wasn't called earlier.
>        *
>        * @opaque: data pointer passed to register_savevm_live()
>        */
> @@ -244,6 +246,8 @@ typedef struct SaveVMHandlers {
>        * @load_cleanup
>        *
>        * Uninitializes the data structures on the destination.
> +     * Note that this handler can be called even if load_setup
> +     * wasn't called earlier.
>        *
>        * @opaque: data pointer passed to register_savevm_live()
>        *