mirrors/qemu
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Revert "migration: move only_migratable to MigrationState"

Browse Source 
This reverts commit 3df663e575.
This reverts commit b605c47b57.

Command line option --only-migratable is for disallowing any
configuration that can block migration.

Initially, --only-migratable set global variable @only_migratable.

Commit 3df663e575 "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 b605c47b57 "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 cda4aa9a5a "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: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190401090827.20793-3-armbru@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
This commit is contained in:
Markus Armbruster 2019-04-01 11:08:24 +02:00
parent 2fa23277d5
commit 811f865271
5 changed files with 6 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
include/sysemu/sysemu.h
View File

@ -14,6 +14,7 @@
/* vl.c */
extern const char *bios_name;
extern int only_migratable;
extern const char *qemu_name;
extern QemuUUID qemu_uuid;
extern bool qemu_uuid_set;

5
migration/migration.c
View File

@ -1707,7 +1707,7 @@ static GSList *migration_blockers;
int migrate_add_blocker(Error *reason, Error **errp)
{
if (migrate_get_current()->only_migratable) {
if (only_migratable) {
error_propagate_prepend(errp, error_copy(reason),
"disallowing migration blocker "
"(--only_migratable) for: ");
@ -3337,7 +3337,7 @@ void migration_global_dump(Monitor *mon)
monitor_printf(mon, "store-global-state: %s\n",
ms->store_global_state ? "on" : "off");
monitor_printf(mon, "only-migratable: %s\n",
ms->only_migratable ? "on" : "off");
only_migratable ? "on" : "off");
monitor_printf(mon, "send-configuration: %s\n",
ms->send_configuration ? "on" : "off");
monitor_printf(mon, "send-section-footer: %s\n",
@ -3352,7 +3352,6 @@ void migration_global_dump(Monitor *mon)
static Property migration_properties[] = {
DEFINE_PROP_BOOL("store-global-state", MigrationState,
store_global_state, true),
DEFINE_PROP_BOOL("only-migratable", MigrationState, only_migratable, false),
DEFINE_PROP_BOOL("send-configuration", MigrationState,
send_configuration, true),
DEFINE_PROP_BOOL("send-section-footer", MigrationState,

3
migration/migration.h
View File

@ -219,9 +219,6 @@ struct MigrationState
*/
bool store_global_state;
/* Whether the VM is only allowing for migratable devices */
bool only_migratable;
/* Whether we send QEMU_VM_CONFIGURATION during migration */
bool send_configuration;
/* Whether we send section footer during migration */

2
migration/savevm.c
View File

@ -2844,7 +2844,7 @@ void vmstate_register_ram_global(MemoryRegion *mr)
bool vmstate_check_only_migratable(const VMStateDescription *vmsd)
{
/* check needed if --only-migratable is specified */
if (!migrate_get_current()->only_migratable) {
if (!only_migratable) {
return true;
}

9
vl.c
View File

@ -185,6 +185,7 @@ const char *prom_envs[MAX_PROM_ENVS];
int boot_menu;
bool boot_strict;
uint8_t *boot_splash_filedata;
int only_migratable; /* turn it off unless user states otherwise */
bool wakeup_suspend_enabled;
int icount_align_option;
@ -3800,13 +3801,7 @@ int main(int argc, char **argv, char **envp)
incoming = optarg;
break;
case QEMU_OPTION_only_migratable:
/*
* TODO: we can remove this option one day, and we
* should all use:
*
* "-global migration.only-migratable=true"
*/
qemu_global_option("migration.only-migratable=true");
only_migratable = 1;
break;
case QEMU_OPTION_nodefaults:
has_defaults = 0;