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

[Markus Armbruster <armbru@redhat.com>]

Eduardo Habkost <ehabkost@redhat.com> 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 <mreitz@redhat.com>
>> Reported-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> Fixes: 3df663 ("migration: move only_migratable to MigrationState")
>> Signed-off-by: Peter Xu <peterx@redhat.com>
>> ---
>>  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.

[...]