Skip to content
  • Markus Armbruster's avatar
    811f8652
    Revert "migration: move only_migratable to MigrationState" · 811f8652
    Markus Armbruster authored
    
    
    This reverts commit 3df663e5.
    This reverts commit b605c47b.
    
    Command line option --only-migratable is for disallowing any
    configuration that can block migration.
    
    Initially, --only-migratable set global variable @only_migratable.
    
    Commit 3df663e5 "migration: move only_migratable to MigrationState"
    replaced it by MigrationState member @only_migratable.  That was a
    mistake.
    
    First, it doesn't make sense on the design level.  MigrationState
    captures the state of an individual migration, but --only-migratable
    isn't a property of an individual migration, it's a restriction on
    QEMU configuration.  With fault tolerance, we could have several
    migrations at once.  --only-migratable would certainly protect all of
    them.  Storing it in MigrationState feels inappropriate.
    
    Second, it contributes to a dependency cycle that manifests itself as
    a bug now.
    
    Putting @only_migratable into MigrationState means its available only
    after migration_object_init().
    
    We can't set it before migration_object_init(), so we delay setting it
    with a global property (this is fixup commit b605c47b "migration:
    fix handling for --only-migratable").
    
    We can't get it before migration_object_init(), so anything that uses
    it can only run afterwards.
    
    Since migrate_add_blocker() needs to obey --only-migratable, any code
    adding migration blockers can run only afterwards.  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 cda4aa9a "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.
    
    Moving @only_migratable into MigrationState was a mistake.  Revert it.
    
    This doesn't quite break the "migration_object_init() before
    configure_blockdev() dependency, since migrate_add_blocker() still has
    another dependency on migration_object_init().  To be addressed the
    next commit.
    
    Note that the reverted commit made -only-migratable sugar for -global
    migration.only-migratable=on below the hood.  Documentation has only
    ever mentioned -only-migratable.  This commit removes the arcane &
    undocumented alternative to -only-migratable again.  Nobody should be
    using it.
    
    Conflicts:
    	include/migration/misc.h
    	migration/migration.c
    	migration/migration.h
    	vl.c
    
    Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
    Message-Id: <20190401090827.20793-3-armbru@redhat.com>
    Reviewed-by: default avatarIgor Mammedov <imammedo@redhat.com>
    811f8652
    Revert "migration: move only_migratable to MigrationState"
    Markus Armbruster authored
    
    
    This reverts commit 3df663e5.
    This reverts commit b605c47b.
    
    Command line option --only-migratable is for disallowing any
    configuration that can block migration.
    
    Initially, --only-migratable set global variable @only_migratable.
    
    Commit 3df663e5 "migration: move only_migratable to MigrationState"
    replaced it by MigrationState member @only_migratable.  That was a
    mistake.
    
    First, it doesn't make sense on the design level.  MigrationState
    captures the state of an individual migration, but --only-migratable
    isn't a property of an individual migration, it's a restriction on
    QEMU configuration.  With fault tolerance, we could have several
    migrations at once.  --only-migratable would certainly protect all of
    them.  Storing it in MigrationState feels inappropriate.
    
    Second, it contributes to a dependency cycle that manifests itself as
    a bug now.
    
    Putting @only_migratable into MigrationState means its available only
    after migration_object_init().
    
    We can't set it before migration_object_init(), so we delay setting it
    with a global property (this is fixup commit b605c47b "migration:
    fix handling for --only-migratable").
    
    We can't get it before migration_object_init(), so anything that uses
    it can only run afterwards.
    
    Since migrate_add_blocker() needs to obey --only-migratable, any code
    adding migration blockers can run only afterwards.  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 cda4aa9a "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.
    
    Moving @only_migratable into MigrationState was a mistake.  Revert it.
    
    This doesn't quite break the "migration_object_init() before
    configure_blockdev() dependency, since migrate_add_blocker() still has
    another dependency on migration_object_init().  To be addressed the
    next commit.
    
    Note that the reverted commit made -only-migratable sugar for -global
    migration.only-migratable=on below the hood.  Documentation has only
    ever mentioned -only-migratable.  This commit removes the arcane &
    undocumented alternative to -only-migratable again.  Nobody should be
    using it.
    
    Conflicts:
    	include/migration/misc.h
    	migration/migration.c
    	migration/migration.h
    	vl.c
    
    Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
    Message-Id: <20190401090827.20793-3-armbru@redhat.com>
    Reviewed-by: default avatarIgor Mammedov <imammedo@redhat.com>
Loading