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

Dr. David Alan Gilbert (git) posted 1 patch 7 years, 3 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180806121304.9440-1-dgilbert@redhat.com
Test checkpatch passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test docker-quick@centos7 passed
docs/devel/migration.rst | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
[Qemu-devel] [PATCH] docs/migration: Clarify pre_load in subsections
Posted by Dr. David Alan Gilbert (git) 7 years, 3 months ago
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
Posted by Juan Quintela 7 years, 2 months ago
"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

Re: [Qemu-devel] [PATCH] docs/migration: Clarify pre_load in subsections
Posted by Peter Maydell 7 years, 3 months ago
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