qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 07/24] migration: Document the BQL behavior of load SaveVM


From: Peter Xu
Subject: Re: [PATCH v3 07/24] migration: Document the BQL behavior of load SaveVMHandlers
Date: Wed, 4 Dec 2024 16:38:11 -0500

On Sun, Nov 17, 2024 at 08:20:02PM +0100, Maciej S. Szmigiero wrote:
> From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
> 
> Some of these SaveVMHandlers were missing the BQL behavior annotation,
> making people wonder what it exactly is.
> 
> Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
> ---
>  include/migration/register.h | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/include/migration/register.h b/include/migration/register.h
> index 39991f3cc5d0..761e4e4d8bcb 100644
> --- a/include/migration/register.h
> +++ b/include/migration/register.h
> @@ -212,6 +212,8 @@ typedef struct SaveVMHandlers {
>      void (*state_pending_exact)(void *opaque, uint64_t *must_precopy,
>                                  uint64_t *can_postcopy);
>  
> +    /* This runs inside the BQL. */
> +
>      /**
>       * @load_state
>       *
> @@ -246,6 +248,8 @@ typedef struct SaveVMHandlers {
>      int (*load_state_buffer)(void *opaque, char *buf, size_t len,
>                               Error **errp);
>  
> +    /* The following handlers run inside the BQL. */
> +
>      /**
>       * @load_setup
>       *
> @@ -272,6 +276,9 @@ typedef struct SaveVMHandlers {
>       */
>      int (*load_cleanup)(void *opaque);
>  
> +
> +    /* This runs outside the BQL. */
> +
>      /**
>       * @resume_prepare
>       *
> @@ -284,6 +291,8 @@ typedef struct SaveVMHandlers {
>       */
>      int (*resume_prepare)(MigrationState *s, void *opaque);
>  
> +    /* The following handlers run inside the BQL. */
> +
>      /**
>       * @switchover_ack_needed
>       *
> 

Such change is not only error prone when adding new hooks, it's also hard
to review..

If we do care about that, I suggest we attach that info to every command.
For example, changing from:

    /**
     * @save_state
     * ...

To:

    /**
     * @save_state (invoked with BQL)
     * ...

Or somewhere in the doc lines of each hook.

-- 
Peter Xu




reply via email to

[Prev in Thread] Current Thread [Next in Thread]