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.

Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
---
 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()
      *

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()
>        *
> 

"Maciej S. Szmigiero" <mail@maciej.szmigiero.name> writes:

> 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.
>
> Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>

Reviewed-by: Fabiano Rosas <farosas@suse.de>