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>