From cce8040bb0ea6ff56d8882aeb0a0435a61901d93 Mon Sep 17 00:00:00 2001 From: "Dr. David Alan Gilbert" Date: Fri, 15 Dec 2017 17:16:54 +0000 Subject: [PATCH 01/14] migration: Allow migrate_fd_connect to take an Error * Allow whatever is performing the connection to pass migrate_fd_connect an error to indicate there was a problem during connection, an allow us to clean up. The caller must free the error. Signed-off-by: Dr. David Alan Gilbert Reviewed-by: Juan Quintela Signed-off-by: Juan Quintela Signed-off-by: Dr. David Alan Gilbert --- migration/channel.c | 2 +- migration/migration.c | 7 ++++++- migration/migration.h | 2 +- migration/rdma.c | 2 +- 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/migration/channel.c b/migration/channel.c index 70ec7ea3b7..fdb7ddbd17 100644 --- a/migration/channel.c +++ b/migration/channel.c @@ -78,6 +78,6 @@ void migration_channel_connect(MigrationState *s, s->to_dst_file = f; - migrate_fd_connect(s); + migrate_fd_connect(s, NULL); } } diff --git a/migration/migration.c b/migration/migration.c index c99a4e62d7..1fbd304c66 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -2378,10 +2378,15 @@ static void *migration_thread(void *opaque) return NULL; } -void migrate_fd_connect(MigrationState *s) +void migrate_fd_connect(MigrationState *s, Error *error_in) { s->expected_downtime = s->parameters.downtime_limit; s->cleanup_bh = qemu_bh_new(migrate_fd_cleanup, s); + if (error_in) { + migrate_fd_error(s, error_in); + migrate_fd_cleanup(s); + return; + } qemu_file_set_blocking(s->to_dst_file, true); qemu_file_set_rate_limit(s->to_dst_file, diff --git a/migration/migration.h b/migration/migration.h index 786d971ce2..d3b214e5ba 100644 --- a/migration/migration.h +++ b/migration/migration.h @@ -190,7 +190,7 @@ uint64_t migrate_max_downtime(void); void migrate_set_error(MigrationState *s, const Error *error); void migrate_fd_error(MigrationState *s, const Error *error); -void migrate_fd_connect(MigrationState *s); +void migrate_fd_connect(MigrationState *s, Error *error_in); MigrationState *migrate_init(void); bool migration_is_blocked(Error **errp); diff --git a/migration/rdma.c b/migration/rdma.c index 9d5a424011..da474fc19f 100644 --- a/migration/rdma.c +++ b/migration/rdma.c @@ -3758,7 +3758,7 @@ void rdma_start_outgoing_migration(void *opaque, trace_rdma_start_outgoing_migration_after_rdma_connect(); s->to_dst_file = qemu_fopen_rdma(rdma, "wb"); - migrate_fd_connect(s); + migrate_fd_connect(s, NULL); return; err: g_free(rdma); From 688a3dcba980bf01344a1ae2bc37fea44c6014ac Mon Sep 17 00:00:00 2001 From: "Dr. David Alan Gilbert" Date: Fri, 15 Dec 2017 17:16:55 +0000 Subject: [PATCH 02/14] migration: Route errors down through migration_channel_connect Route async errors (especially from sockets) down through migration_channel_connect and on to migrate_fd_connect where they can be cleaned up. Signed-off-by: Dr. David Alan Gilbert Reviewed-by: Juan Quintela Signed-off-by: Juan Quintela Signed-off-by: Dr. David Alan Gilbert --- migration/channel.c | 34 +++++++++++++++++----------------- migration/channel.h | 3 ++- migration/exec.c | 2 +- migration/fd.c | 2 +- migration/socket.c | 4 +--- migration/tls.c | 3 +-- migration/trace-events | 2 +- 7 files changed, 24 insertions(+), 26 deletions(-) diff --git a/migration/channel.c b/migration/channel.c index fdb7ddbd17..c5eaf0fa0e 100644 --- a/migration/channel.c +++ b/migration/channel.c @@ -55,29 +55,29 @@ void migration_channel_process_incoming(QIOChannel *ioc) * @s: Current migration state * @ioc: Channel to which we are connecting * @hostname: Where we want to connect + * @error: Error indicating failure to connect, free'd here */ void migration_channel_connect(MigrationState *s, QIOChannel *ioc, - const char *hostname) + const char *hostname, + Error *error) { trace_migration_set_outgoing_channel( - ioc, object_get_typename(OBJECT(ioc)), hostname); + ioc, object_get_typename(OBJECT(ioc)), hostname, error); + + if (!error) { + if (s->parameters.tls_creds && + *s->parameters.tls_creds && + !object_dynamic_cast(OBJECT(ioc), + TYPE_QIO_CHANNEL_TLS)) { + migration_tls_channel_connect(s, ioc, hostname, &error); + } else { + QEMUFile *f = qemu_fopen_channel_output(ioc); + + s->to_dst_file = f; - if (s->parameters.tls_creds && - *s->parameters.tls_creds && - !object_dynamic_cast(OBJECT(ioc), - TYPE_QIO_CHANNEL_TLS)) { - Error *local_err = NULL; - migration_tls_channel_connect(s, ioc, hostname, &local_err); - if (local_err) { - migrate_fd_error(s, local_err); - error_free(local_err); } - } else { - QEMUFile *f = qemu_fopen_channel_output(ioc); - - s->to_dst_file = f; - - migrate_fd_connect(s, NULL); } + migrate_fd_connect(s, error); + error_free(error); } diff --git a/migration/channel.h b/migration/channel.h index e4b40579a1..67a461c28a 100644 --- a/migration/channel.h +++ b/migration/channel.h @@ -22,5 +22,6 @@ void migration_channel_process_incoming(QIOChannel *ioc); void migration_channel_connect(MigrationState *s, QIOChannel *ioc, - const char *hostname); + const char *hostname, + Error *error_in); #endif diff --git a/migration/exec.c b/migration/exec.c index f3be1baf2e..c9537974ad 100644 --- a/migration/exec.c +++ b/migration/exec.c @@ -39,7 +39,7 @@ void exec_start_outgoing_migration(MigrationState *s, const char *command, Error } qio_channel_set_name(ioc, "migration-exec-outgoing"); - migration_channel_connect(s, ioc, NULL); + migration_channel_connect(s, ioc, NULL, NULL); object_unref(OBJECT(ioc)); } diff --git a/migration/fd.c b/migration/fd.c index 30de4b9847..6284a97cba 100644 --- a/migration/fd.c +++ b/migration/fd.c @@ -39,7 +39,7 @@ void fd_start_outgoing_migration(MigrationState *s, const char *fdname, Error ** } qio_channel_set_name(QIO_CHANNEL(ioc), "migration-fd-outgoing"); - migration_channel_connect(s, ioc, NULL); + migration_channel_connect(s, ioc, NULL, NULL); object_unref(OBJECT(ioc)); } diff --git a/migration/socket.c b/migration/socket.c index 3a8232dd2d..e090097077 100644 --- a/migration/socket.c +++ b/migration/socket.c @@ -79,12 +79,10 @@ static void socket_outgoing_migration(QIOTask *task, if (qio_task_propagate_error(task, &err)) { trace_migration_socket_outgoing_error(error_get_pretty(err)); - migrate_fd_error(data->s, err); - error_free(err); } else { trace_migration_socket_outgoing_connected(data->hostname); - migration_channel_connect(data->s, sioc, data->hostname); } + migration_channel_connect(data->s, sioc, data->hostname, err); object_unref(OBJECT(sioc)); } diff --git a/migration/tls.c b/migration/tls.c index 026a008667..a29b35b33c 100644 --- a/migration/tls.c +++ b/migration/tls.c @@ -118,11 +118,10 @@ static void migration_tls_outgoing_handshake(QIOTask *task, if (qio_task_propagate_error(task, &err)) { trace_migration_tls_outgoing_handshake_error(error_get_pretty(err)); - migrate_fd_error(s, err); } else { trace_migration_tls_outgoing_handshake_complete(); - migration_channel_connect(s, ioc, NULL); } + migration_channel_connect(s, ioc, NULL, err); object_unref(OBJECT(ioc)); } diff --git a/migration/trace-events b/migration/trace-events index 6f29fcc686..93961dea16 100644 --- a/migration/trace-events +++ b/migration/trace-events @@ -114,7 +114,7 @@ migrate_transferred(uint64_t tranferred, uint64_t time_spent, double bandwidth, process_incoming_migration_co_end(int ret, int ps) "ret=%d postcopy-state=%d" process_incoming_migration_co_postcopy_end_main(void) "" migration_set_incoming_channel(void *ioc, const char *ioctype) "ioc=%p ioctype=%s" -migration_set_outgoing_channel(void *ioc, const char *ioctype, const char *hostname) "ioc=%p ioctype=%s hostname=%s" +migration_set_outgoing_channel(void *ioc, const char *ioctype, const char *hostname, void *err) "ioc=%p ioctype=%s hostname=%s err=%p" # migration/rdma.c qemu_rdma_accept_incoming_migration(void) "" From ee555cdf4d495ddd83633406e3099c5d1ef22e0a Mon Sep 17 00:00:00 2001 From: Daniel Henrique Barboza Date: Fri, 26 Jan 2018 13:59:40 -0200 Subject: [PATCH 03/14] migration/savevm.c: set MAX_VM_CMD_PACKAGED_SIZE to 1ul << 32 MAX_VM_CMD_PACKAGED_SIZE is a constant used in qemu_savevm_send_packaged and loadvm_handle_cmd_packaged to determine whether a package is too big to be sent or received. qemu_savevm_send_packaged is called inside postcopy_start (migration/migration.c) to send the MigrationState in a single blob to the destination, using the MIG_CMD_PACKAGED subcommand, which will read it up using loadvm_handle_cmd_packaged. If the blob is larger than MAX_VM_CMD_PACKAGED_SIZE, an error is thrown and the postcopy migration is aborted. Both MAX_VM_CMD_PACKAGED_SIZE and MIG_CMD_PACKAGED were introduced by commit 11cf1d984b ("MIG_CMD_PACKAGED: Send a packaged chunk ..."). The constant has its original value of 1ul << 24 (16MB). The current MAX_VM_CMD_PACKAGED_SIZE value is not enough to support postcopy migration of bigger pseries guests. The blob size for a postcopy migration of a pseries guest with the following setup: qemu-system-ppc64 --nographic -vga none -machine pseries,accel=kvm -m 64G \ -smp 1,maxcpus=32 -device virtio-blk-pci,drive=rootdisk \ -drive file=f27.qcow2,if=none,cache=none,format=qcow2,id=rootdisk \ -netdev user,id=u1 -net nic,netdev=u1 Goes around 12MB. Bumping the RAM to 128G makes the blob sizes goes to 20MB. With 256G the blob goes to 37MB - more than twice the current maximum size. At this moment the pseries machine can handle guests with up to 1TB of RAM, making this postcopy blob goes to 128MB of size approximately. Following the discussions made in [1], there is a need to understand what devices are aggressively consuming the blob in that manner and see if that can be mitigated. Until then, we can set MAX_VM_CMD_PACKAGED_SIZE to the maximum value allowed. Since the size is a 32 bit int variable, we can set it as 1ul << 32, giving a maximum blob size of 4G that is enough to support postcopy migration of 32TB RAM guests given the above constraints. [1] https://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg06313.html Signed-off-by: Daniel Henrique Barboza Reported-by: Balamuruhan S Reviewed-by: Juan Quintela Signed-off-by: Juan Quintela Signed-off-by: Dr. David Alan Gilbert --- migration/savevm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/migration/savevm.c b/migration/savevm.c index b7908f62be..b8e9c532af 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -81,7 +81,7 @@ enum qemu_vm_cmd { MIG_CMD_MAX }; -#define MAX_VM_CMD_PACKAGED_SIZE (1ul << 24) +#define MAX_VM_CMD_PACKAGED_SIZE UINT32_MAX static struct mig_cmd_args { ssize_t len; /* -1 = variable */ const char *name; From 0781c1ed1cbe1361b45f8fddfc85d202a517a88c Mon Sep 17 00:00:00 2001 From: Wei Wang Date: Mon, 22 Jan 2018 19:36:39 +0800 Subject: [PATCH 04/14] migration: use s->threshold_size inside migration_update_counters Fixes: b15df1ae50 ("migration: cleanup stats update into function") The threshold size is changed to be recorded in s->threshold_size. Signed-off-by: Wei Wang Reviewed-by: Peter Xu Reviewed-by: Juan Quintela Signed-off-by: Juan Quintela Signed-off-by: Dr. David Alan Gilbert --- migration/migration.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/migration/migration.c b/migration/migration.c index 1fbd304c66..44cbfb0ddd 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -2169,7 +2169,6 @@ static void migration_update_counters(MigrationState *s, int64_t current_time) { uint64_t transferred, time_spent; - int64_t threshold_size; double bandwidth; if (current_time < s->iteration_start_time + BUFFER_DELAY) { @@ -2179,7 +2178,7 @@ static void migration_update_counters(MigrationState *s, transferred = qemu_ftell(s->to_dst_file) - s->iteration_initial_bytes; time_spent = current_time - s->iteration_start_time; bandwidth = (double)transferred / time_spent; - threshold_size = bandwidth * s->parameters.downtime_limit; + s->threshold_size = bandwidth * s->parameters.downtime_limit; s->mbps = (((double) transferred * 8.0) / ((double) time_spent / 1000.0)) / 1000.0 / 1000.0; @@ -2199,7 +2198,7 @@ static void migration_update_counters(MigrationState *s, s->iteration_initial_bytes = qemu_ftell(s->to_dst_file); trace_migrate_transferred(transferred, time_spent, - bandwidth, threshold_size); + bandwidth, s->threshold_size); } /* Migration thread iteration status */ From 7faccdc3e761ce44d77f986065d4a0e1df5c8a01 Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Mon, 8 Jan 2018 18:58:17 +0100 Subject: [PATCH 05/14] migration: Drop current address parameter from save_zero_page() It already has RAMBlock and offset, it can calculate it itself. Signed-off-by: Juan Quintela Reviewed-by: Dr. David Alan Gilbert Signed-off-by: Dr. David Alan Gilbert --- migration/ram.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/migration/ram.c b/migration/ram.c index cb1950f3eb..5a109efeda 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -907,11 +907,10 @@ static void migration_bitmap_sync(RAMState *rs) * @rs: current RAM state * @block: block that contains the page we want to send * @offset: offset inside the block for the page - * @p: pointer to the page */ -static int save_zero_page(RAMState *rs, RAMBlock *block, ram_addr_t offset, - uint8_t *p) +static int save_zero_page(RAMState *rs, RAMBlock *block, ram_addr_t offset) { + uint8_t *p = block->host + offset; int pages = -1; if (is_zero_range(p, TARGET_PAGE_SIZE)) { @@ -984,7 +983,7 @@ static int ram_save_page(RAMState *rs, PageSearchStatus *pss, bool last_stage) } } } else { - pages = save_zero_page(rs, block, offset, p); + pages = save_zero_page(rs, block, offset); if (pages > 0) { /* Must let xbzrle know, otherwise a previous (now 0'd) cached * page would be stale @@ -1160,7 +1159,7 @@ static int ram_save_compressed_page(RAMState *rs, PageSearchStatus *pss, */ if (block != rs->last_sent_block) { flush_compressed_data(rs); - pages = save_zero_page(rs, block, offset, p); + pages = save_zero_page(rs, block, offset); if (pages == -1) { /* Make sure the first page is sent out before other pages */ bytes_xmit = save_page_header(rs, rs->f, block, offset | @@ -1180,7 +1179,7 @@ static int ram_save_compressed_page(RAMState *rs, PageSearchStatus *pss, ram_release_pages(block->idstr, offset, pages); } } else { - pages = save_zero_page(rs, block, offset, p); + pages = save_zero_page(rs, block, offset); if (pages == -1) { pages = compress_page_with_multi_thread(rs, block, offset); } else { From 1f90d797110a1e1800ba21cd79b6cd15318cb36a Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Thu, 30 Nov 2017 21:12:03 +0100 Subject: [PATCH 06/14] tests: Remove deprecated migration tests commands We move to use migration_set_parameter() for everything. Signed-off-by: Juan Quintela Reviewed-by: Dr. David Alan Gilbert Signed-off-by: Dr. David Alan Gilbert --- tests/migration-test.c | 33 ++++++++------------------------- 1 file changed, 8 insertions(+), 25 deletions(-) diff --git a/tests/migration-test.c b/tests/migration-test.c index 799e24ebc6..0428d450df 100644 --- a/tests/migration-test.c +++ b/tests/migration-test.c @@ -369,37 +369,20 @@ static void migrate_check_parameter(QTestState *who, const char *parameter, QDECREF(rsp); } -static void migrate_set_downtime(QTestState *who, const double value) -{ - QDict *rsp; - gchar *cmd; - char *expected; - int64_t result_int; - - cmd = g_strdup_printf("{ 'execute': 'migrate_set_downtime'," - "'arguments': { 'value': %g } }", value); - rsp = qtest_qmp(who, cmd); - g_free(cmd); - g_assert(qdict_haskey(rsp, "return")); - QDECREF(rsp); - result_int = value * 1000L; - expected = g_strdup_printf("%" PRId64, result_int); - migrate_check_parameter(who, "downtime-limit", expected); - g_free(expected); -} - -static void migrate_set_speed(QTestState *who, const char *value) +static void migrate_set_parameter(QTestState *who, const char *parameter, + const char *value) { QDict *rsp; gchar *cmd; - cmd = g_strdup_printf("{ 'execute': 'migrate_set_speed'," - "'arguments': { 'value': %s } }", value); + cmd = g_strdup_printf("{ 'execute': 'migrate-set-parameters'," + "'arguments': { '%s': %s } }", + parameter, value); rsp = qtest_qmp(who, cmd); g_free(cmd); g_assert(qdict_haskey(rsp, "return")); QDECREF(rsp); - migrate_check_parameter(who, "max-bandwidth", value); + migrate_check_parameter(who, parameter, value); } static void migrate_set_capability(QTestState *who, const char *capability, @@ -530,8 +513,8 @@ static void test_migrate(void) * quickly, but that it doesn't complete precopy even on a slow * machine, so also set the downtime. */ - migrate_set_speed(from, "100000000"); - migrate_set_downtime(from, 0.001); + migrate_set_parameter(from, "max-bandwidth", "100000000"); + migrate_set_parameter(from, "downtime-limit", "1"); /* Wait for the first serial output from the source */ wait_for_serial("src_serial"); From 63b2d935f7f9b09c75380d1ffb37a8f1fa23fdcb Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Fri, 5 Jan 2018 12:49:59 +0100 Subject: [PATCH 07/14] tests: Consolidate accelerators declaration Signed-off-by: Juan Quintela Reviewed-by: Dr. David Alan Gilbert Reviewed-by: Peter Xu Signed-off-by: Dr. David Alan Gilbert --- tests/migration-test.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/migration-test.c b/tests/migration-test.c index 0428d450df..3732682a29 100644 --- a/tests/migration-test.c +++ b/tests/migration-test.c @@ -422,27 +422,29 @@ static void test_migrate_start(QTestState **from, QTestState **to, gchar *cmd_src, *cmd_dst; char *bootpath = g_strdup_printf("%s/bootsect", tmpfs); const char *arch = qtest_get_arch(); + const char *accel = "kvm:tcg"; got_stop = false; if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { init_bootfile_x86(bootpath); - cmd_src = g_strdup_printf("-machine accel=kvm:tcg -m 150M" + cmd_src = g_strdup_printf("-machine accel=%s -m 150M" " -name pcsource,debug-threads=on" " -serial file:%s/src_serial" " -drive file=%s,format=raw", - tmpfs, bootpath); - cmd_dst = g_strdup_printf("-machine accel=kvm:tcg -m 150M" + accel, tmpfs, bootpath); + cmd_dst = g_strdup_printf("-machine accel=%s -m 150M" " -name pcdest,debug-threads=on" " -serial file:%s/dest_serial" " -drive file=%s,format=raw" " -incoming %s", - tmpfs, bootpath, uri); + accel, tmpfs, bootpath, uri); } else if (strcmp(arch, "ppc64") == 0) { - const char *accel; /* On ppc64, the test only works with kvm-hv, but not with kvm-pr */ - accel = access("/sys/module/kvm_hv", F_OK) ? "tcg" : "kvm:tcg"; + if (access("/sys/module/kvm_hv", F_OK)) { + accel = "tcg"; + } init_bootfile_ppc(bootpath); cmd_src = g_strdup_printf("-machine accel=%s -m 256M" " -name pcsource,debug-threads=on" From 31a6bb74fa5383192c010f87d079993c99dd5bf8 Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Fri, 5 Jan 2018 12:51:49 +0100 Subject: [PATCH 08/14] tests: Use consistent names for migration Signed-off-by: Juan Quintela Reviewed-by: Dr. David Alan Gilbert Reviewed-by: Peter Xu Signed-off-by: Dr. David Alan Gilbert --- tests/migration-test.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/migration-test.c b/tests/migration-test.c index 3732682a29..ac4a916169 100644 --- a/tests/migration-test.c +++ b/tests/migration-test.c @@ -429,12 +429,12 @@ static void test_migrate_start(QTestState **from, QTestState **to, if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { init_bootfile_x86(bootpath); cmd_src = g_strdup_printf("-machine accel=%s -m 150M" - " -name pcsource,debug-threads=on" + " -name source,debug-threads=on" " -serial file:%s/src_serial" " -drive file=%s,format=raw", accel, tmpfs, bootpath); cmd_dst = g_strdup_printf("-machine accel=%s -m 150M" - " -name pcdest,debug-threads=on" + " -name target,debug-threads=on" " -serial file:%s/dest_serial" " -drive file=%s,format=raw" " -incoming %s", @@ -447,12 +447,12 @@ static void test_migrate_start(QTestState **from, QTestState **to, } init_bootfile_ppc(bootpath); cmd_src = g_strdup_printf("-machine accel=%s -m 256M" - " -name pcsource,debug-threads=on" + " -name source,debug-threads=on" " -serial file:%s/src_serial" " -drive file=%s,if=pflash,format=raw", accel, tmpfs, bootpath); cmd_dst = g_strdup_printf("-machine accel=%s -m 256M" - " -name pcdest,debug-threads=on" + " -name target,debug-threads=on" " -serial file:%s/dest_serial" " -incoming %s", accel, tmpfs, uri); From 4c27486dc75b803ba8fe9eb9375cc9c075d3f127 Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Fri, 5 Jan 2018 13:18:49 +0100 Subject: [PATCH 09/14] tests: Add deprecated commands migration test We add deprecated commands on a new test, so we don't have to add it on normal tests. Signed-off-by: Juan Quintela Reviewed-by: Peter Xu Signed-off-by: Dr. David Alan Gilbert --- tests/migration-test.c | 46 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/tests/migration-test.c b/tests/migration-test.c index ac4a916169..2a0b651cd1 100644 --- a/tests/migration-test.c +++ b/tests/migration-test.c @@ -500,6 +500,51 @@ static void test_migrate_end(QTestState *from, QTestState *to) cleanup("dest_serial"); } +static void deprecated_set_downtime(QTestState *who, const double value) +{ + QDict *rsp; + gchar *cmd; + char *expected; + int64_t result_int; + + cmd = g_strdup_printf("{ 'execute': 'migrate_set_downtime'," + "'arguments': { 'value': %g } }", value); + rsp = qtest_qmp(who, cmd); + g_free(cmd); + g_assert(qdict_haskey(rsp, "return")); + QDECREF(rsp); + result_int = value * 1000L; + expected = g_strdup_printf("%" PRId64, result_int); + migrate_check_parameter(who, "downtime-limit", expected); + g_free(expected); +} + +static void deprecated_set_speed(QTestState *who, const char *value) +{ + QDict *rsp; + gchar *cmd; + + cmd = g_strdup_printf("{ 'execute': 'migrate_set_speed'," + "'arguments': { 'value': %s } }", value); + rsp = qtest_qmp(who, cmd); + g_free(cmd); + g_assert(qdict_haskey(rsp, "return")); + QDECREF(rsp); + migrate_check_parameter(who, "max-bandwidth", value); +} + +static void test_deprecated(void) +{ + QTestState *from; + + from = qtest_start(""); + + deprecated_set_downtime(from, 0.12345); + deprecated_set_speed(from, "12345"); + + qtest_quit(from); +} + static void test_migrate(void) { char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); @@ -563,6 +608,7 @@ int main(int argc, char **argv) module_call_init(MODULE_INIT_QOM); qtest_add_func("/migration/postcopy/unix", test_migrate); + qtest_add_func("/migration/deprecated", test_deprecated); ret = g_test_run(); From eb665d7d92200d948238f67b827d604856ac061e Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Fri, 5 Jan 2018 13:35:56 +0100 Subject: [PATCH 10/14] tests: Create migrate-start-postcopy command This way, it is like the rest of commands Signed-off-by: Juan Quintela Reviewed-by: Dr. David Alan Gilbert Reviewed-by: Peter Xu Signed-off-by: Dr. David Alan Gilbert --- tests/migration-test.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/migration-test.c b/tests/migration-test.c index 2a0b651cd1..26a792c3aa 100644 --- a/tests/migration-test.c +++ b/tests/migration-test.c @@ -416,6 +416,15 @@ static void migrate(QTestState *who, const char *uri) QDECREF(rsp); } +static void migrate_start_postcopy(QTestState *who) +{ + QDict *rsp; + + rsp = wait_command(who, "{ 'execute': 'migrate-start-postcopy' }"); + g_assert(qdict_haskey(rsp, "return")); + QDECREF(rsp); +} + static void test_migrate_start(QTestState **from, QTestState **to, const char *uri) { @@ -549,7 +558,6 @@ static void test_migrate(void) { char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); QTestState *from, *to; - QDict *rsp; test_migrate_start(&from, &to, uri); @@ -570,9 +578,7 @@ static void test_migrate(void) wait_for_migration_pass(from); - rsp = wait_command(from, "{ 'execute': 'migrate-start-postcopy' }"); - g_assert(qdict_haskey(rsp, "return")); - QDECREF(rsp); + migrate_start_postcopy(from); if (!got_stop) { qtest_qmp_eventwait(from, "STOP"); From 6a7724e9a239b5f1342df00deedab06f3d360083 Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Fri, 5 Jan 2018 13:56:48 +0100 Subject: [PATCH 11/14] tests: Adjust sleeps for migration test Also reorder code to not sleep when event already happened. Signed-off-by: Juan Quintela Reviewed-by: Dr. David Alan Gilbert Reviewed-by: Peter Xu Signed-off-by: Dr. David Alan Gilbert --- tests/migration-test.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/tests/migration-test.c b/tests/migration-test.c index 26a792c3aa..9efad95749 100644 --- a/tests/migration-test.c +++ b/tests/migration-test.c @@ -268,10 +268,9 @@ static uint64_t get_migration_pass(QTestState *who) static void wait_for_migration_complete(QTestState *who) { - QDict *rsp, *rsp_return; - bool completed; - - do { + while (true) { + QDict *rsp, *rsp_return; + bool completed; const char *status; rsp = wait_command(who, "{ 'execute': 'query-migrate' }"); @@ -280,8 +279,11 @@ static void wait_for_migration_complete(QTestState *who) completed = strcmp(status, "completed") == 0; g_assert_cmpstr(status, !=, "failed"); QDECREF(rsp); - usleep(1000 * 100); - } while (!completed); + if (completed) { + return; + } + usleep(1000); + } } static void wait_for_migration_pass(QTestState *who) @@ -290,16 +292,13 @@ static void wait_for_migration_pass(QTestState *who) uint64_t pass; /* Wait for the 1st sync */ - do { + while (!got_stop && !initial_pass) { + usleep(1000); initial_pass = get_migration_pass(who); - if (got_stop || initial_pass) { - break; - } - usleep(1000 * 100); - } while (true); + } do { - usleep(1000 * 100); + usleep(1000); pass = get_migration_pass(who); } while (pass == initial_pass && !got_stop); } @@ -489,13 +488,13 @@ static void test_migrate_end(QTestState *from, QTestState *to) /* Destination still running, wait for a byte to change */ do { qtest_memread(to, start_address, &dest_byte_b, 1); - usleep(10 * 1000); + usleep(1000 * 10); } while (dest_byte_a == dest_byte_b); qtest_qmp_discard_response(to, "{ 'execute' : 'stop'}"); /* With it stopped, check nothing changes */ qtest_memread(to, start_address, &dest_byte_c, 1); - sleep(1); + usleep(1000 * 200); qtest_memread(to, start_address, &dest_byte_d, 1); g_assert_cmpint(dest_byte_c, ==, dest_byte_d); From 6039dd5b1c45d76403b9dcadd2afd7efd8f42330 Mon Sep 17 00:00:00 2001 From: "Dr. David Alan Gilbert" Date: Mon, 5 Feb 2018 09:13:37 +0000 Subject: [PATCH 12/14] migration: Recover block devices if failure in device state In e91d895 I added the new pause-before-switchover mechanism to allow migration completion to be delayed; this changes the last state prior to completion to MIGRATE_STATUS_DEVICE rather than MIGRATE_STATUS_ACTIVE. Fix the failure path in migration_completion to recover the block devices if it fails in MIGRATE_STATUS_DEVICE, not just the MIGRATE_STATUS_ACTIVE that it previously had. This corresponds to rh bz: https://bugzilla.redhat.com/show_bug.cgi?id=1538494 whose symptom is an occasional source crash on a failed migration. Fixes: e91d8951d59d483f085f Signed-off-by: Dr. David Alan Gilbert Reviewed-by: Peter Xu Signed-off-by: Dr. David Alan Gilbert --- migration/migration.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/migration/migration.c b/migration/migration.c index 44cbfb0ddd..0fdb2e410d 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -2122,7 +2122,8 @@ fail_invalidate: /* If not doing postcopy, vm_start() will be called: let's regain * control on images. */ - if (s->state == MIGRATION_STATUS_ACTIVE) { + if (s->state == MIGRATION_STATUS_ACTIVE || + s->state == MIGRATION_STATUS_DEVICE) { Error *local_err = NULL; qemu_mutex_lock_iothread(); From 032b79f7173051e7f8742a43d106c7fc526856f9 Mon Sep 17 00:00:00 2001 From: Ross Lagerwall Date: Wed, 1 Nov 2017 14:25:23 +0000 Subject: [PATCH 13/14] migration: Don't leak IO channels Since qemu_fopen_channel_{in,out}put take references on the underlying IO channels, make sure to release our references to them. Signed-off-by: Ross Lagerwall Message-Id: <20171101142526.1006-2-ross.lagerwall@citrix.com> Reviewed-by: Daniel P. Berrange Signed-off-by: Dr. David Alan Gilbert --- migration/savevm.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/migration/savevm.c b/migration/savevm.c index b8e9c532af..b024ee3b22 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -2266,6 +2266,7 @@ void qmp_xen_save_devices_state(const char *filename, bool has_live, bool live, } qio_channel_set_name(QIO_CHANNEL(ioc), "migration-xen-save-state"); f = qemu_fopen_channel_output(QIO_CHANNEL(ioc)); + object_unref(OBJECT(ioc)); ret = qemu_save_device_state(f); qemu_fclose(f); if (ret < 0) { @@ -2313,6 +2314,7 @@ void qmp_xen_load_devices_state(const char *filename, Error **errp) } qio_channel_set_name(QIO_CHANNEL(ioc), "migration-xen-load-state"); f = qemu_fopen_channel_input(QIO_CHANNEL(ioc)); + object_unref(OBJECT(ioc)); ret = qemu_loadvm_state(f); qemu_fclose(f); From 875fcd013ab68c64802998b22f54f0184479d21b Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Tue, 6 Feb 2018 12:23:30 +0100 Subject: [PATCH 14/14] migration: incoming postcopy advise sanity checks If postcopy-ram was set on the source but not on the destination, migration doesn't occur, the destination prints an error and boots the guest: qemu-system-ppc64: Expected vmdescription section, but got 0 We end up with two running instances. This behaviour was introduced in 2.11 by commit 58110f0acb1a "migration: split common postcopy out of ram postcopy" to prepare ground for the upcoming dirty bitmap postcopy support. It adds a new case where the source may send an empty postcopy advise because dirty bitmap doesn't need to check page sizes like RAM postcopy does. If the source has enabled postcopy-ram, then it sends an advise with the page size values. If the destination hasn't enabled postcopy-ram, then loadvm_postcopy_handle_advise() leaves the page size values on the stream and returns. This confuses qemu_loadvm_state() later on and causes the destination to start execution. As discussed several times, postcopy-ram should be enabled both sides to be functional. This patch changes the destination to perform some extra checks on the advise length to ensure this is the case. Otherwise an error is returned and migration is aborted. Reported-by: Balamuruhan S Signed-off-by: Greg Kurz Reviewed-by: Daniel Henrique Barboza Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Dr. David Alan Gilbert Message-Id: <151791621042.19120.3103118434734245776.stgit@bahia> Signed-off-by: Dr. David Alan Gilbert --- migration/savevm.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/migration/savevm.c b/migration/savevm.c index b024ee3b22..f202c3de3a 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -1376,7 +1376,8 @@ static int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis); * *might* happen - it might be skipped if precopy transferred everything * quickly. */ -static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis) +static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis, + uint16_t len) { PostcopyState ps = postcopy_state_set(POSTCOPY_INCOMING_ADVISE); uint64_t remote_pagesize_summary, local_pagesize_summary, remote_tps; @@ -1387,8 +1388,22 @@ static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis) return -1; } - if (!migrate_postcopy_ram()) { + switch (len) { + case 0: + if (migrate_postcopy_ram()) { + error_report("RAM postcopy is enabled but have 0 byte advise"); + return -EINVAL; + } return 0; + case 8 + 8: + if (!migrate_postcopy_ram()) { + error_report("RAM postcopy is disabled but have 16 byte advise"); + return -EINVAL; + } + break; + default: + error_report("CMD_POSTCOPY_ADVISE invalid length (%d)", len); + return -EINVAL; } if (!postcopy_ram_supported_by_host(mis)) { @@ -1807,7 +1822,7 @@ static int loadvm_process_command(QEMUFile *f) return loadvm_handle_cmd_packaged(mis); case MIG_CMD_POSTCOPY_ADVISE: - return loadvm_postcopy_handle_advise(mis); + return loadvm_postcopy_handle_advise(mis, len); case MIG_CMD_POSTCOPY_LISTEN: return loadvm_postcopy_handle_listen(mis);