From 172c4356f38fbf91675256447a3bedd08220214f Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Wed, 8 Jul 2015 13:56:26 +0200 Subject: [PATCH 1/6] migration: Only change state after migration has finished On previous change, we changed state at post load time if it was not running, special casing the "running" change. Now, we change any states at the end of the migration. Signed-off-by: Juan Quintela Tested-by: Christian Borntraeger --- migration/migration.c | 48 ++++++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/migration/migration.c b/migration/migration.c index 45719a0f5f..ede432eae6 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -104,6 +104,8 @@ typedef struct { bool optional; uint32_t size; uint8_t runstate[100]; + RunState state; + bool received; } GlobalState; static GlobalState global_state; @@ -119,9 +121,14 @@ static int global_state_store(void) return 0; } -static char *global_state_get_runstate(void) +static bool global_state_received(void) { - return (char *)global_state.runstate; + return global_state.received; +} + +static RunState global_state_get_runstate(void) +{ + return global_state.state; } void global_state_set_optional(void) @@ -154,26 +161,25 @@ static bool global_state_needed(void *opaque) static int global_state_post_load(void *opaque, int version_id) { GlobalState *s = opaque; - int ret = 0; + Error *local_err = NULL; + int r; char *runstate = (char *)s->runstate; + s->received = true; trace_migrate_global_state_post_load(runstate); - if (strcmp(runstate, "running") != 0) { - Error *local_err = NULL; - int r = qapi_enum_parse(RunState_lookup, runstate, RUN_STATE_MAX, + r = qapi_enum_parse(RunState_lookup, runstate, RUN_STATE_MAX, -1, &local_err); - if (r == -1) { - if (local_err) { - error_report_err(local_err); - } - return -EINVAL; + if (r == -1) { + if (local_err) { + error_report_err(local_err); } - ret = vm_stop_force_state(r); + return -EINVAL; } + s->state = r; - return ret; + return 0; } static void global_state_pre_save(void *opaque) @@ -202,6 +208,7 @@ void register_global_state(void) { /* We would use it independently that we receive it */ strcpy((char *)&global_state.runstate, ""); + global_state.received = false; vmstate_register(NULL, 0, &vmstate_globalstate, &global_state); } @@ -283,20 +290,19 @@ static void process_incoming_migration_co(void *opaque) exit(EXIT_FAILURE); } - /* runstate == "" means that we haven't received it through the - * wire, so we obey autostart. runstate == runing means that we - * need to run it, we need to make sure that we do it after - * everything else has finished. Every other state change is done - * at the post_load function */ + /* If global state section was not received or we are in running + state, we need to obey autostart. Any other state is set with + runstate_set. */ - if (strcmp(global_state_get_runstate(), "running") == 0) { - vm_start(); - } else if (strcmp(global_state_get_runstate(), "") == 0) { + if (!global_state_received() || + global_state_get_runstate() == RUN_STATE_RUNNING) { if (autostart) { vm_start(); } else { runstate_set(RUN_STATE_PAUSED); } + } else { + runstate_set(global_state_get_runstate()); } migrate_decompress_threads_join(); } From 4ba4bc5e9bfab457a96ac56dc470730a330aded8 Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Wed, 8 Jul 2015 13:58:27 +0200 Subject: [PATCH 2/6] migration: Trace event and migration event are different things We can want the trace event even without migration events enabled. Reported-by: Wen Congyang Signed-off-by: Juan Quintela Reviewed-by: Dr. David Alan Gilbert --- migration/migration.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/migration/migration.c b/migration/migration.c index ede432eae6..ba82ff6bd1 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -216,7 +216,6 @@ static void migrate_generate_event(int new_state) { if (migrate_use_events()) { qapi_event_send_migration(new_state, &error_abort); - trace_migrate_set_state(new_state); } } @@ -528,6 +527,7 @@ void qmp_migrate_set_parameters(bool has_compress_level, static void migrate_set_state(MigrationState *s, int old_state, int new_state) { if (atomic_cmpxchg(&s->state, old_state, new_state) == old_state) { + trace_migrate_set_state(new_state); migrate_generate_event(new_state); } } From 72e72e1a71e5e67a11204606a5c09f6cc3089a53 Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Wed, 8 Jul 2015 14:13:10 +0200 Subject: [PATCH 3/6] migration: Write documetation for events capabilites Reported-by: Jiri Denemark Signed-off-by: Juan Quintela --- qmp-commands.hx | 1 + 1 file changed, 1 insertion(+) diff --git a/qmp-commands.hx b/qmp-commands.hx index e1bcc60380..ba630b1e7f 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -3406,6 +3406,7 @@ Enable/Disable migration capabilities - "rdma-pin-all": pin all pages when using RDMA during migration - "auto-converge": throttle down guest to help convergence of migration - "zero-blocks": compress zero blocks during block migration +- "events": generate events for each migration state change Arguments: From 48212d87d6655b029231d830a77983c21552fe49 Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Fri, 10 Jul 2015 14:51:58 +0200 Subject: [PATCH 4/6] migration: Register global state section before loadvm Otherwise, it is not found Signed-off-by: Juan Quintela --- vl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vl.c b/vl.c index 3f269dc58d..5856396d46 100644 --- a/vl.c +++ b/vl.c @@ -4615,6 +4615,7 @@ int main(int argc, char **argv, char **envp) } qemu_system_reset(VMRESET_SILENT); + register_global_state(); if (loadvm) { if (load_vmstate(loadvm) < 0) { autostart = 0; @@ -4628,7 +4629,6 @@ int main(int argc, char **argv, char **envp) return 0; } - register_global_state(); if (incoming) { Error *local_err = NULL; qemu_start_incoming_migration(incoming, &local_err); From 9f5f380b54d6ad80cf35d93c8cd71c8d7a1b52b7 Mon Sep 17 00:00:00 2001 From: Liang Li Date: Mon, 13 Jul 2015 17:34:10 +0800 Subject: [PATCH 5/6] migration: reduce the count of strlen call 'strlen' is called three times in 'save_page_header', it's inefficient. Signed-off-by: Liang Li Reviewed-by: Juan Quintela Reviewed-by: Amit Shah Signed-off-by: Juan Quintela --- migration/ram.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/migration/ram.c b/migration/ram.c index 1e58cd3924..7f007e6432 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -382,16 +382,16 @@ void migrate_compress_threads_create(void) */ static size_t save_page_header(QEMUFile *f, RAMBlock *block, ram_addr_t offset) { - size_t size; + size_t size, len; qemu_put_be64(f, offset); size = 8; if (!(offset & RAM_SAVE_FLAG_CONTINUE)) { - qemu_put_byte(f, strlen(block->idstr)); - qemu_put_buffer(f, (uint8_t *)block->idstr, - strlen(block->idstr)); - size += 1 + strlen(block->idstr); + len = strlen(block->idstr); + qemu_put_byte(f, len); + qemu_put_buffer(f, (uint8_t *)block->idstr, len); + size += 1 + len; } return size; } From 560d027b54067ffa4e79c6f7c0a499abb0d749a3 Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Wed, 15 Jul 2015 09:53:46 +0200 Subject: [PATCH 6/6] migration: We also want to store the global state for savevm Commit df4b1024526cae3479da3492d6371fd4a7324a03 introduced global_state section. But it only filled the state while doing migration. While doing a savevm, we stored an empty string as state. So when we did a loadvm, it complained that state was invalid. Fedora 21, 4.1.1, qemu 2.4.0-rc0 > ../../configure --target-list="x86_64-softmmu" 068 2s ... - output mismatch (see 068.out.bad) --- /home/bos/jhuston/src/qemu/tests/qemu-iotests/068.out 2015-07-08 17:56:18.588164979 -0400 +++ 068.out.bad 2015-07-09 17:39:58.636651317 -0400 @@ -6,6 +6,8 @@ QEMU X.Y.Z monitor - type 'help' for more information (qemu) savevm 0 (qemu) quit +qemu-system-x86_64: Unknown savevm section or instance 'globalstate' 0 +qemu-system-x86_64: Error -22 while loading VM state QEMU X.Y.Z monitor - type 'help' for more information (qemu) quit *** done Failures: 068 Failed 1 of 1 tests Actually, there were two problems here: - we registered global_state too late for load_vm (fixed on another patch on the list) - we didn't store a valid state for savevm (fixed by this patch). Reported-by: John Snow Signed-off-by: Juan Quintela Reviewed-by: Dr. David Alan Gilbert Reviewed-by: Amit Shah Tested-by: Christian Borntraeger --- include/migration/migration.h | 1 + migration/migration.c | 2 +- migration/savevm.c | 6 ++++++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/include/migration/migration.h b/include/migration/migration.h index b2711ef305..a2f8ed093c 100644 --- a/include/migration/migration.h +++ b/include/migration/migration.h @@ -202,4 +202,5 @@ void savevm_skip_section_footers(void); void register_global_state(void); void global_state_set_optional(void); void savevm_skip_configuration(void); +int global_state_store(void); #endif diff --git a/migration/migration.c b/migration/migration.c index ba82ff6bd1..86ca099ac4 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -110,7 +110,7 @@ typedef struct { static GlobalState global_state; -static int global_state_store(void) +int global_state_store(void) { if (!runstate_store((char *)global_state.runstate, sizeof(global_state.runstate))) { diff --git a/migration/savevm.c b/migration/savevm.c index 86735fc53a..81dbe5879f 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -1315,6 +1315,12 @@ void hmp_savevm(Monitor *mon, const QDict *qdict) } saved_vm_running = runstate_is_running(); + + ret = global_state_store(); + if (ret) { + monitor_printf(mon, "Error saving global state\n"); + return; + } vm_stop(RUN_STATE_SAVE_VM); memset(sn, 0, sizeof(*sn));