From 94f5a43704129ca4995aa3385303c5ae225bde42 Mon Sep 17 00:00:00 2001 From: Liang Li Date: Mon, 2 Nov 2015 15:37:00 +0800 Subject: [PATCH 1/5] migration: defer migration_end & blk_mig_cleanup Because of the patch 3ea3b7fa9af067982f34b of kvm, which introduces a lazy collapsing of small sptes into large sptes mechanism, now migration_end() is a time consuming operation because it calls memroy_global_dirty_log_stop(), which will trigger the dropping of small sptes operation and takes about dozens of milliseconds, so call migration_end() before all the vmsate data has already been transferred to the destination will prolong VM downtime. This operation should be deferred after all the data has been transferred to the destination. blk_mig_cleanup() can be deferred too. For a VM with 8G RAM, this patch can reduce the VM downtime about 30 ms. Signed-off-by: Liang Li Reviewed-by: Paolo Bonzini Reviewed-by: Juan Quintela al3 Reviewed-by: Amit Shah al3 Signed-off-by: Juan Quintela al3 --- migration/block.c | 1 - migration/migration.c | 13 ++++++------- migration/ram.c | 1 - 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/migration/block.c b/migration/block.c index f7bb1e0126..8401597bcc 100644 --- a/migration/block.c +++ b/migration/block.c @@ -750,7 +750,6 @@ static int block_save_complete(QEMUFile *f, void *opaque) qemu_put_be64(f, BLK_MIG_FLAG_EOS); - blk_mig_cleanup(); return 0; } diff --git a/migration/migration.c b/migration/migration.c index b092f386b4..d5a7304e76 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -613,12 +613,9 @@ static void migrate_fd_cleanup(void *opaque) assert(s->state != MIGRATION_STATUS_ACTIVE); - if (s->state != MIGRATION_STATUS_COMPLETED) { - qemu_savevm_state_cancel(); - if (s->state == MIGRATION_STATUS_CANCELLING) { - migrate_set_state(s, MIGRATION_STATUS_CANCELLING, - MIGRATION_STATUS_CANCELLED); - } + if (s->state == MIGRATION_STATUS_CANCELLING) { + migrate_set_state(s, MIGRATION_STATUS_CANCELLING, + MIGRATION_STATUS_CANCELLED); } notifier_list_notify(&migration_state_notifiers, s); @@ -1028,6 +1025,7 @@ static void *migration_thread(void *opaque) int64_t initial_bytes = 0; int64_t max_size = 0; int64_t start_time = initial_time; + int64_t end_time; bool old_vm_running = false; rcu_register_thread(); @@ -1089,10 +1087,11 @@ static void *migration_thread(void *opaque) /* If we enabled cpu throttling for auto-converge, turn it off. */ cpu_throttle_stop(); + end_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME); qemu_mutex_lock_iothread(); + qemu_savevm_state_cancel(); if (s->state == MIGRATION_STATUS_COMPLETED) { - int64_t end_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME); uint64_t transferred_bytes = qemu_ftell(s->file); s->total_time = end_time - s->total_time; s->downtime = end_time - start_time; diff --git a/migration/ram.c b/migration/ram.c index a25bcc7db1..25e9eebea6 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -1344,7 +1344,6 @@ static int ram_save_complete(QEMUFile *f, void *opaque) rcu_read_unlock(); - migration_end(); qemu_put_be64(f, RAM_SAVE_FLAG_EOS); return 0; From ea7415fac677c5c1599214ee226ab4a3a438fdd6 Mon Sep 17 00:00:00 2001 From: Liang Li Date: Mon, 2 Nov 2015 15:37:01 +0800 Subject: [PATCH 2/5] migration: rename qemu_savevm_state_cancel The function qemu_savevm_state_cancel is called after the migration in migration_thread, it seems strange to 'cancel' it after completion, rename it to qemu_savevm_state_cleanup looks better. Signed-off-by: Liang Li Reviewed-by: Juan Quintela al3 Reviewed-by: Amit Shah al3 Signed-off-by: Juan Quintela al3 --- include/sysemu/sysemu.h | 2 +- migration/migration.c | 2 +- migration/savevm.c | 6 +++--- trace-events | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index c439975139..5cb0f05068 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -89,7 +89,7 @@ void qemu_savevm_state_begin(QEMUFile *f, void qemu_savevm_state_header(QEMUFile *f); int qemu_savevm_state_iterate(QEMUFile *f); void qemu_savevm_state_complete(QEMUFile *f); -void qemu_savevm_state_cancel(void); +void qemu_savevm_state_cleanup(void); uint64_t qemu_savevm_state_pending(QEMUFile *f, uint64_t max_size); int qemu_loadvm_state(QEMUFile *f); diff --git a/migration/migration.c b/migration/migration.c index d5a7304e76..f99d3eabf7 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -1090,7 +1090,7 @@ static void *migration_thread(void *opaque) end_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME); qemu_mutex_lock_iothread(); - qemu_savevm_state_cancel(); + qemu_savevm_state_cleanup(); if (s->state == MIGRATION_STATUS_COMPLETED) { uint64_t transferred_bytes = qemu_ftell(s->file); s->total_time = end_time - s->total_time; diff --git a/migration/savevm.c b/migration/savevm.c index dbcc39a617..ae8fdda875 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -902,11 +902,11 @@ uint64_t qemu_savevm_state_pending(QEMUFile *f, uint64_t max_size) return ret; } -void qemu_savevm_state_cancel(void) +void qemu_savevm_state_cleanup(void) { SaveStateEntry *se; - trace_savevm_state_cancel(); + trace_savevm_state_cleanup(); QTAILQ_FOREACH(se, &savevm_state.handlers, entry) { if (se->ops && se->ops->cancel) { se->ops->cancel(se->opaque); @@ -943,7 +943,7 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp) ret = qemu_file_get_error(f); } if (ret != 0) { - qemu_savevm_state_cancel(); + qemu_savevm_state_cleanup(); error_setg_errno(errp, -ret, "Error while writing VM state"); } return ret; diff --git a/trace-events b/trace-events index 72136b9846..17fbddf1ec 100644 --- a/trace-events +++ b/trace-events @@ -1211,7 +1211,7 @@ savevm_state_begin(void) "" savevm_state_header(void) "" savevm_state_iterate(void) "" savevm_state_complete(void) "" -savevm_state_cancel(void) "" +savevm_state_cleanup(void) "" vmstate_save(const char *idstr, const char *vmsd_name) "%s, %s" vmstate_load(const char *idstr, const char *vmsd_name) "%s, %s" qemu_announce_self_iter(const char *mac) "%s" From d1a8548c10bf6d24160ec2aafa4881a3f50a8373 Mon Sep 17 00:00:00 2001 From: Liang Li Date: Mon, 2 Nov 2015 15:37:02 +0800 Subject: [PATCH 3/5] migration: rename cancel to cleanup in SaveVMHandles 'cleanup' seems more appropriate than 'cancel'. Signed-off-by: Liang Li Reviewed-by: Juan Quintela al3 Reviewed-by: Amit Shah al3 Signed-off-by: Juan Quintela al3 --- include/migration/vmstate.h | 2 +- migration/block.c | 2 +- migration/ram.c | 2 +- migration/savevm.c | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h index 9a65522da1..d173b565f5 100644 --- a/include/migration/vmstate.h +++ b/include/migration/vmstate.h @@ -39,7 +39,7 @@ typedef struct SaveVMHandlers { void (*set_params)(const MigrationParams *params, void * opaque); SaveStateHandler *save_state; - void (*cancel)(void *opaque); + void (*cleanup)(void *opaque); int (*save_live_complete)(QEMUFile *f, void *opaque); /* This runs both outside and inside the iothread lock. */ diff --git a/migration/block.c b/migration/block.c index 8401597bcc..ecfe005691 100644 --- a/migration/block.c +++ b/migration/block.c @@ -884,7 +884,7 @@ static SaveVMHandlers savevm_block_handlers = { .save_live_complete = block_save_complete, .save_live_pending = block_save_pending, .load_state = block_load, - .cancel = block_migration_cancel, + .cleanup = block_migration_cancel, .is_active = block_is_active, }; diff --git a/migration/ram.c b/migration/ram.c index 25e9eebea6..0a51473cd6 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -1685,7 +1685,7 @@ static SaveVMHandlers savevm_ram_handlers = { .save_live_complete = ram_save_complete, .save_live_pending = ram_save_pending, .load_state = ram_load, - .cancel = ram_migration_cancel, + .cleanup = ram_migration_cancel, }; void ram_mig_init(void) diff --git a/migration/savevm.c b/migration/savevm.c index ae8fdda875..e05158d7ba 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -908,8 +908,8 @@ void qemu_savevm_state_cleanup(void) trace_savevm_state_cleanup(); QTAILQ_FOREACH(se, &savevm_state.handlers, entry) { - if (se->ops && se->ops->cancel) { - se->ops->cancel(se->opaque); + if (se->ops && se->ops->cleanup) { + se->ops->cleanup(se->opaque); } } } From 6ad2a215e7170350430adfda02eb8ef47c1acd8e Mon Sep 17 00:00:00 2001 From: Liang Li Date: Mon, 2 Nov 2015 15:37:03 +0800 Subject: [PATCH 4/5] migration: code clean up Just clean up code, no behavior change. Signed-off-by: Liang Li Reviewed-by: Juan Quintela al3 Reviewed-by: Amit Shah al3 Signed-off-by: Juan Quintela al3 --- migration/block.c | 9 ++------- migration/ram.c | 9 ++------- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/migration/block.c b/migration/block.c index ecfe005691..cf9d9f8999 100644 --- a/migration/block.c +++ b/migration/block.c @@ -591,7 +591,7 @@ static int64_t get_remaining_dirty(void) /* Called with iothread lock taken. */ -static void blk_mig_cleanup(void) +static void block_migration_cleanup(void *opaque) { BlkMigDevState *bmds; BlkMigBlock *blk; @@ -618,11 +618,6 @@ static void blk_mig_cleanup(void) blk_mig_unlock(); } -static void block_migration_cancel(void *opaque) -{ - blk_mig_cleanup(); -} - static int block_save_setup(QEMUFile *f, void *opaque) { int ret; @@ -884,7 +879,7 @@ static SaveVMHandlers savevm_block_handlers = { .save_live_complete = block_save_complete, .save_live_pending = block_save_pending, .load_state = block_load, - .cleanup = block_migration_cancel, + .cleanup = block_migration_cleanup, .is_active = block_is_active, }; diff --git a/migration/ram.c b/migration/ram.c index 0a51473cd6..df3df9e3bf 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -1100,7 +1100,7 @@ static void migration_bitmap_free(struct BitmapRcu *bmap) g_free(bmap); } -static void migration_end(void) +static void ram_migration_cleanup(void *opaque) { /* caller have hold iothread lock or is in a bh, so there is * no writing race against this migration_bitmap @@ -1124,11 +1124,6 @@ static void migration_end(void) XBZRLE_cache_unlock(); } -static void ram_migration_cancel(void *opaque) -{ - migration_end(); -} - static void reset_ram_globals(void) { last_seen_block = NULL; @@ -1685,7 +1680,7 @@ static SaveVMHandlers savevm_ram_handlers = { .save_live_complete = ram_save_complete, .save_live_pending = ram_save_pending, .load_state = ram_load, - .cleanup = ram_migration_cancel, + .cleanup = ram_migration_cleanup, }; void ram_mig_init(void) From 96e5c9bc77acef8b7b56cbe23a8a2611feff9e34 Mon Sep 17 00:00:00 2001 From: Mark Cave-Ayland Date: Sat, 5 Sep 2015 20:51:48 +0100 Subject: [PATCH 5/5] migration: fix analyze-migration.py script Commit 61964 "Add configuration section" broke the analyze-migration.py script which terminates due to the unrecognised section. Fix the script by parsing the contents of the configuration section directly into a new ConfigurationSection object (although nothing is done with it yet). Signed-off-by: Mark Cave-Ayland Reviewed-by: Juan Quintela al3 Signed-off-by: Juan Quintela al3 --- scripts/analyze-migration.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/scripts/analyze-migration.py b/scripts/analyze-migration.py index f6894bece9..14553876a2 100755 --- a/scripts/analyze-migration.py +++ b/scripts/analyze-migration.py @@ -252,6 +252,15 @@ class HTABSection(object): def getDict(self): return "" + +class ConfigurationSection(object): + def __init__(self, file): + self.file = file + + def read(self): + name_len = self.file.read32() + name = self.file.readstr(len = name_len) + class VMSDFieldGeneric(object): def __init__(self, desc, file): self.file = file @@ -474,6 +483,7 @@ class MigrationDump(object): QEMU_VM_SECTION_FULL = 0x04 QEMU_VM_SUBSECTION = 0x05 QEMU_VM_VMDESCRIPTION = 0x06 + QEMU_VM_CONFIGURATION = 0x07 QEMU_VM_SECTION_FOOTER= 0x7e def __init__(self, filename): @@ -514,6 +524,9 @@ class MigrationDump(object): section_type = file.read8() if section_type == self.QEMU_VM_EOF: break + elif section_type == self.QEMU_VM_CONFIGURATION: + section = ConfigurationSection(file) + section.read() elif section_type == self.QEMU_VM_SECTION_START or section_type == self.QEMU_VM_SECTION_FULL: section_id = file.read32() name = file.readstr()