Re: [Qemu-devel] [PATCH 2/4] vl: move global property, migrate init earlier

[Eduardo Habkost]

On Tue, Jul 04, 2017 at 10:12:45AM +0200, Markus Armbruster wrote:
> Eduardo Habkost <address@hidden> writes:
> 
> > On Mon, Jul 03, 2017 at 10:44:06AM +0800, Peter Xu wrote:
> >> Currently drive_init_func() may call migrate_get_current() while the
> >> migrate object is still not ready yet at that time. Move the migration
> >> object init earlier, along with the global properties, right after
> >> acceleration init.
> >> 
> >> This fixes a breakage for iotest 055, which caused an assertion failure.
> >> 
> >> Reported-by: Max Reitz <address@hidden>
> >> Reported-by: Philippe Mathieu-Daudé <address@hidden>
> >> Fixes: 3df663 ("migration: move only_migratable to MigrationState")
> >> Signed-off-by: Peter Xu <address@hidden>
> >> ---
> >>  vl.c | 24 ++++++++++++------------
> >>  1 file changed, 12 insertions(+), 12 deletions(-)
> >> 
> >> diff --git a/vl.c b/vl.c
> >> index 0c497a3..2ae4313 100644
> >> --- a/vl.c
> >> +++ b/vl.c
> >> @@ -4414,6 +4414,18 @@ int main(int argc, char **argv, char **envp)
> >>  
> >>      configure_accelerator(current_machine);
> >>  
> >> +    /*
> >> +     * Register all the global properties, including accel properties,
> >> +     * machine properties, and user-specified ones.
> >> +     */
> >> +    register_global_properties(current_machine);
> >> +
> >> +    /*
> >> +     * Migration object can only be created after global properties
> >> +     * are applied correctly.
> >> +     */
> >> +    migration_object_init();
> >> +
> >
> > So, things that might introduce bugs here are:
> > 1) Unexpected qdev_prop_register_global() calls between this place and
> >    the original register_global_properties() call (that would now happen
> >    in a different order).
> > 2) register_global_properties() seeing a different global property list
> >    because it is being called earlier.
> >    2.1) AccelClass::global_props is statically defined and will be the
> >         same here. Not a problem.
> >    2.2) MachineClass::compat_props: same as above.
> >    2.3) user-provided global properties: we need to ensure all
> >         properties in the "global" QemuOpts section are already
> >         available at this point.
> 
> This stuff is fragile.  There are lots of dependencies, all implicit,
> and once again we reorder things ever so slightly to keep them
> satisfied.  We could certainly use a more robust solution.

Yes, and we are working on it.  Items 2.1-2.3 are already grouped in
register_global_properties() to make ordering rules explicit.  The rest
of the qdev_prop_register_global() calls need to be either replaced with
something else or moved to register_global_properties() too.

-- 
Eduardo