Re: [Qemu-devel] [PATCH v2 3/5] migration: Support adding migration blockers earlier

[Igor Mammedov <imammedo@redhat.com>]

On Mon,  1 Apr 2019 11:08:25 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> migrate_add_blocker() asserts we have a current_migration object, in
> migrate_get_current().  We do only after migration_object_init().
> 
> This contributes to the following dependency cycle:
> 
> * configure_blockdev() must run before machine_set_property()
>   so machine properties can refer to block backends
> 
> * machine_set_property() before configure_accelerator()
>   so machine properties like kvm-irqchip get applied
> 
> * configure_accelerator() before migration_object_init()
>   so that Xen's accelerator compat properties get applied.
> 
> * migration_object_init() before configure_blockdev()
>   so configure_blockdev() can add migration blockers
> 
> The cycle was closed when recent commit cda4aa9a5a0 "Create block
> backends before setting machine properties" added the first
> dependency, and satisfied it by violating the last one.  Broke block
> backends that add migration blockers, as demonstrated by qemu-iotests
> 055.
> 
> To fix it, break the last dependency: make migrate_add_blocker()
> usable before migration_object_init().
> 
> The previous commit already removed the use of migrate_get_current()
> from migrate_add_blocker() itself.  Didn't quite do the trick, as
> there's another one hiding in migration_is_idle().
> 
> The use there isn't actually necessary: when no migration object has
> been created yet, migration is surely idle.  Make migration_is_idle()
> return true then.
> 
> Fixes: cda4aa9a5a08777cf13e164c0543bd4888b8adce
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  migration/migration.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index f6076e5295..609e0df5d0 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1646,7 +1646,11 @@ bool migration_in_postcopy_after_devices(MigrationState *s)
>  
>  bool migration_is_idle(void)
>  {
> -    MigrationState *s = migrate_get_current();
> +    MigrationState *s = current_migration;
> +
> +    if (!s) {
> +        return true;
> +    }
>  
>      switch (s->state) {
>      case MIGRATION_STATUS_NONE: