qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Philippe Mathieu-Daudé" <philmd@linaro.org>
To: Peter Maydell <peter.maydell@linaro.org>,
	qemu-devel@nongnu.org, qemu-trivial@nongnu.org,
	Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [PATCH] docs/devel: Mention post_load hook restrictions where we document the hook
Date: Thu, 3 Oct 2024 18:01:50 +0200	[thread overview]
Message-ID: <bc7f9c41-2b69-4536-bc5a-218c1efeba87@linaro.org> (raw)
In-Reply-To: <20241003143401.1676548-1-peter.maydell@linaro.org>

On 3/10/24 16:34, Peter Maydell wrote:
> Accessing another device in a post_load hook is a bad idea, because
> the order of device save/restore is not fixed, and so this
> cross-device access makes the save/restore non-deterministic.
> 
> We previously only flagged up this requirement in the
> record-and-replay developer docs; repeat it in the main migration
> documentation, where a developer trying to implement a post_load hook
> is more likely to see it.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> This came up in an IRC discussion.
> 
>   docs/devel/migration/main.rst | 6 ++++++
>   docs/devel/replay.rst         | 3 +++
>   2 files changed, 9 insertions(+)
> 
> diff --git a/docs/devel/migration/main.rst b/docs/devel/migration/main.rst
> index 784c899dca6..c2857fc2446 100644
> --- a/docs/devel/migration/main.rst
> +++ b/docs/devel/migration/main.rst
> @@ -465,6 +465,12 @@ Examples of such API functions are:
>     - portio_list_set_address()
>     - portio_list_set_enabled()
>   
> +Since the order of device save/restore is not defined, you must
> +avoid accessing or changing any other device's state in one of these
> +callbacks. (For instance, don't do anything that calls ``update_irq()``
> +in a ``post_load`` hook.) Otherwise, restore will not be deterministic,
> +and this will break execution record/replay.
> +
>   Iterative device migration
>   --------------------------
>   
> diff --git a/docs/devel/replay.rst b/docs/devel/replay.rst
> index effd856f0c6..40f58d9d4fc 100644
> --- a/docs/devel/replay.rst
> +++ b/docs/devel/replay.rst
> @@ -202,6 +202,9 @@ into the log.
>   Saving/restoring the VM state
>   -----------------------------
>   
> +Record/replay relies on VM state save and restore being complete and
> +deterministic.
> +
>   All fields in the device state structure (including virtual timers)
>   should be restored by loadvm to the same values they had before savevm.
>   

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>



      reply	other threads:[~2024-10-03 16:03 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-03 14:34 [PATCH] docs/devel: Mention post_load hook restrictions where we document the hook Peter Maydell
2024-10-03 16:01 ` Philippe Mathieu-Daudé [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bc7f9c41-2b69-4536-bc5a-218c1efeba87@linaro.org \
    --to=philmd@linaro.org \
    --cc=pavel.dovgaluk@ispras.ru \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-trivial@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).