[Qemu-devel] [PATCH] docs/migration: Clarify pre_load in subsections

[Qemu-devel] [PATCH] docs/migration: Clarify pre_load in subsections

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Clarify that the pre_load function in a subsection is only called if
the subsection is found; to handle a missing subsection you may
set values in the pre_load of the parent vmsd.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 docs/devel/migration.rst | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/docs/devel/migration.rst b/docs/devel/migration.rst
index 6ed3fce061..687570754d 100644
--- a/docs/devel/migration.rst
+++ b/docs/devel/migration.rst
@@ -240,10 +240,13 @@ should succeed even with the data missing.  To support this the
 subsection can be connected to a device property and from there
 to a versioned machine type.
 
-One important note is that the post_load() function is called "after"
-loading all subsections, because a newer subsection could change same
-value that it uses.  A flag, and the combination of pre_load and post_load
-can be used to detect whether a subsection was loaded, and to
+The 'pre_load' and 'post_load' functions on subsections are only
+called if the subsection is loaded.
+
+One important note is that the outer post_load() function is called "after"
+loading all subsections, because a newer subsection could change the same
+value that it uses.  A flag, and the combination of outer pre_load and
+post_load can be used to detect whether a subsection was loaded, and to
 fall back on default behaviour when the subsection isn't present.
 
 Example:
@@ -315,8 +318,8 @@ For example:
       the property to false.
    c) Add a static bool  support_foo function that tests the property.
    d) Add a subsection with a .needed set to the support_foo function
-   e) (potentially) Add a pre_load that sets up a default value for 'foo'
-      to be used if the subsection isn't loaded.
+   e) (potentially) Add an outer pre_load that sets up a default value
+      for 'foo' to be used if the subsection isn't loaded.
 
 Now that subsection will not be generated when using an older
 machine type and the migration stream will be accepted by older
-- 
2.17.1

Re: [Qemu-devel] [PATCH] docs/migration: Clarify pre_load in subsections

[Peter Maydell]

On 6 August 2018 at 13:13, Dr. David Alan Gilbert (git)
<dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Clarify that the pre_load function in a subsection is only called if
> the subsection is found; to handle a missing subsection you may
> set values in the pre_load of the parent vmsd.

NB: I did a quick audit of all the uses of pre_load in the tree:

These uses rely on the hook not being called unless the
subsection is found:
hw/acpi/ich9.c
hw/acpi/piix4.c
hw/net/vmxnet3.c

These are buggy (and I have a patch I'm testing to fix it)
hw/intc/arm_gicv3_common.c:    .pre_load = vmstate_gicv3_cpu_pre_load,
hw/intc/arm_gicv3_common.c:    .pre_load = gicv3_pre_load,

and all the rest are not in VMSDs for subsections.

(I didn't audit the post_load uses; gicv3 has a wrong
use of that too.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH] docs/migration: Clarify pre_load in subsections

[Juan Quintela]

"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Clarify that the pre_load function in a subsection is only called if
> the subsection is found; to handle a missing subsection you may
> set values in the pre_load of the parent vmsd.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>



> ---
>  docs/devel/migration.rst | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/docs/devel/migration.rst b/docs/devel/migration.rst
> index 6ed3fce061..687570754d 100644
> --- a/docs/devel/migration.rst
> +++ b/docs/devel/migration.rst
> @@ -240,10 +240,13 @@ should succeed even with the data missing.  To support this the
>  subsection can be connected to a device property and from there
>  to a versioned machine type.
>  
> -One important note is that the post_load() function is called "after"
> -loading all subsections, because a newer subsection could change same
> -value that it uses.  A flag, and the combination of pre_load and post_load
> -can be used to detect whether a subsection was loaded, and to
> +The 'pre_load' and 'post_load' functions on subsections are only
> +called if the subsection is loaded.
> +
> +One important note is that the outer post_load() function is called "after"
> +loading all subsections, because a newer subsection could change the same
> +value that it uses.  A flag, and the combination of outer pre_load and
> +post_load can be used to detect whether a subsection was loaded, and to
>  fall back on default behaviour when the subsection isn't present.
>  
>  Example:
> @@ -315,8 +318,8 @@ For example:
>        the property to false.
>     c) Add a static bool  support_foo function that tests the property.
>     d) Add a subsection with a .needed set to the support_foo function
> -   e) (potentially) Add a pre_load that sets up a default value for 'foo'
> -      to be used if the subsection isn't loaded.
> +   e) (potentially) Add an outer pre_load that sets up a default value
> +      for 'foo' to be used if the subsection isn't loaded.
>  
>  Now that subsection will not be generated when using an older
>  machine type and the migration stream will be accepted by older