On Mon, 1 Apr 2019 11:08:25 +0200
Markus Armbruster <arm...@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 <arm...@redhat.com>
Reviewed-by: Igor Mammedov <imamm...@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:
