| Message ID | 20241003143401.1676548-1-peter.maydell@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | docs/devel: Mention post_load hook restrictions where we document the hook | expand |
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>
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.
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(+)