From ab105cc138159b1f829025bbb45b63ad24bc0aaa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Tue, 6 Mar 2018 18:09:59 +0100 Subject: [PATCH 1/6] migration: fix minor finalize leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Spotted thanks to ASAN: QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 tests/migration-test -p /x86_64/migration/bad_dest ==30302==ERROR: LeakSanitizer: detected memory leaks Direct leak of 48 byte(s) in 1 object(s) allocated from: #0 0x7f60efba1a38 in __interceptor_calloc (/lib64/libasan.so.4+0xdea38) #1 0x7f60eef3cf75 in g_malloc0 ../glib/gmem.c:124 #2 0x55ca9094702c in error_copy /home/elmarco/src/qemu/util/error.c:203 #3 0x55ca9037a30f in migrate_set_error /home/elmarco/src/qemu/migration/migration.c:1139 #4 0x55ca9037a462 in migrate_fd_error /home/elmarco/src/qemu/migration/migration.c:1150 #5 0x55ca9038162b in migrate_fd_connect /home/elmarco/src/qemu/migration/migration.c:2411 #6 0x55ca90386e41 in migration_channel_connect /home/elmarco/src/qemu/migration/channel.c:81 #7 0x55ca9038335e in socket_outgoing_migration /home/elmarco/src/qemu/migration/socket.c:85 #8 0x55ca9083dd3a in qio_task_complete /home/elmarco/src/qemu/io/task.c:142 #9 0x55ca9083d6cc in gio_task_thread_result /home/elmarco/src/qemu/io/task.c:88 #10 0x7f60eef37317 in g_idle_dispatch ../glib/gmain.c:5552 #11 0x7f60eef3490b in g_main_dispatch ../glib/gmain.c:3182 #12 0x7f60eef357ac in g_main_context_dispatch ../glib/gmain.c:3847 #13 0x55ca90927231 in glib_pollfds_poll /home/elmarco/src/qemu/util/main-loop.c:214 #14 0x55ca90927420 in os_host_main_loop_wait /home/elmarco/src/qemu/util/main-loop.c:261 #15 0x55ca909275fa in main_loop_wait /home/elmarco/src/qemu/util/main-loop.c:515 #16 0x55ca8fc1c2a4 in main_loop /home/elmarco/src/qemu/vl.c:1942 #17 0x55ca8fc2eb3a in main /home/elmarco/src/qemu/vl.c:4724 #18 0x7f60e4082009 in __libc_start_main (/lib64/libc.so.6+0x21009) Indirect leak of 45 byte(s) in 1 object(s) allocated from: #0 0x7f60efba1850 in malloc (/lib64/libasan.so.4+0xde850) #1 0x7f60eef3cf0c in g_malloc ../glib/gmem.c:94 #2 0x7f60eef3d1cf in g_malloc_n ../glib/gmem.c:331 #3 0x7f60eef596eb in g_strdup ../glib/gstrfuncs.c:363 #4 0x55ca90947085 in error_copy /home/elmarco/src/qemu/util/error.c:204 #5 0x55ca9037a30f in migrate_set_error /home/elmarco/src/qemu/migration/migration.c:1139 #6 0x55ca9037a462 in migrate_fd_error /home/elmarco/src/qemu/migration/migration.c:1150 #7 0x55ca9038162b in migrate_fd_connect /home/elmarco/src/qemu/migration/migration.c:2411 #8 0x55ca90386e41 in migration_channel_connect /home/elmarco/src/qemu/migration/channel.c:81 #9 0x55ca9038335e in socket_outgoing_migration /home/elmarco/src/qemu/migration/socket.c:85 #10 0x55ca9083dd3a in qio_task_complete /home/elmarco/src/qemu/io/task.c:142 #11 0x55ca9083d6cc in gio_task_thread_result /home/elmarco/src/qemu/io/task.c:88 #12 0x7f60eef37317 in g_idle_dispatch ../glib/gmain.c:5552 #13 0x7f60eef3490b in g_main_dispatch ../glib/gmain.c:3182 #14 0x7f60eef357ac in g_main_context_dispatch ../glib/gmain.c:3847 #15 0x55ca90927231 in glib_pollfds_poll /home/elmarco/src/qemu/util/main-loop.c:214 #16 0x55ca90927420 in os_host_main_loop_wait /home/elmarco/src/qemu/util/main-loop.c:261 #17 0x55ca909275fa in main_loop_wait /home/elmarco/src/qemu/util/main-loop.c:515 #18 0x55ca8fc1c2a4 in main_loop /home/elmarco/src/qemu/vl.c:1942 #19 0x55ca8fc2eb3a in main /home/elmarco/src/qemu/vl.c:4724 #20 0x7f60e4082009 in __libc_start_main (/lib64/libc.so.6+0x21009) Signed-off-by: Marc-André Lureau Message-Id: <20180306170959.3921-1-marcandre.lureau@redhat.com> Reviewed-by: Dr. David Alan Gilbert Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Dr. David Alan Gilbert --- migration/migration.c | 1 + 1 file changed, 1 insertion(+) diff --git a/migration/migration.c b/migration/migration.c index e345d0cc7e..62c243d2d4 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -2541,6 +2541,7 @@ static void migration_instance_finalize(Object *obj) g_free(params->tls_hostname); g_free(params->tls_creds); qemu_sem_destroy(&ms->pause_sem); + error_free(ms->error); } static void migration_instance_init(Object *obj) From b255734531ea2853d1a22b3e64c49084bb304e60 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Thu, 8 Mar 2018 12:18:24 +0100 Subject: [PATCH 2/6] migration: do not transfer ram during bulk storage migration this patch makes the bulk phase of a block migration to take place before we start transferring ram. As the bulk block migration can take a long time its pointless to transfer ram during that phase. Signed-off-by: Peter Lieven Reviewed-by: Stefan Hajnoczi Message-Id: <1520507908-16743-2-git-send-email-pl@kamp.de> Reviewed-by: Juan Quintela Signed-off-by: Dr. David Alan Gilbert --- migration/ram.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/migration/ram.c b/migration/ram.c index 3b6c077964..7266351fd0 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -2258,6 +2258,13 @@ static int ram_save_iterate(QEMUFile *f, void *opaque) int64_t t0; int done = 0; + if (blk_mig_bulk_active()) { + /* Avoid transferring ram during bulk phase of block migration as + * the bulk phase will usually take a long time and transferring + * ram updates during that time is pointless. */ + goto out; + } + rcu_read_lock(); if (ram_list.version != rs->last_version) { ram_state_reset(rs); @@ -2304,6 +2311,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque) */ ram_control_after_iterate(f, RAM_CONTROL_ROUND); +out: qemu_put_be64(f, RAM_SAVE_FLAG_EOS); ram_counters.transferred += 8; From 86b124bc76bd7137d0fb20696c4e349571b8533d Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Thu, 8 Mar 2018 12:18:25 +0100 Subject: [PATCH 3/6] migration/block: reset dirty bitmap before read in bulk phase Reset the dirty bitmap before reading to make sure we don't miss any new data. Cc: qemu-stable@nongnu.org Signed-off-by: Peter Lieven Message-Id: <1520507908-16743-3-git-send-email-pl@kamp.de> Reviewed-by: Juan Quintela Signed-off-by: Dr. David Alan Gilbert --- migration/block.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/migration/block.c b/migration/block.c index 1f03946797..87bb35ce63 100644 --- a/migration/block.c +++ b/migration/block.c @@ -331,11 +331,10 @@ static int mig_save_device_bulk(QEMUFile *f, BlkMigDevState *bmds) */ qemu_mutex_lock_iothread(); aio_context_acquire(blk_get_aio_context(bmds->blk)); - blk->aiocb = blk_aio_preadv(bb, cur_sector * BDRV_SECTOR_SIZE, &blk->qiov, - 0, blk_mig_read_cb, blk); - bdrv_reset_dirty_bitmap(bmds->dirty_bitmap, cur_sector * BDRV_SECTOR_SIZE, nr_sectors * BDRV_SECTOR_SIZE); + blk->aiocb = blk_aio_preadv(bb, cur_sector * BDRV_SECTOR_SIZE, &blk->qiov, + 0, blk_mig_read_cb, blk); aio_context_release(blk_get_aio_context(bmds->blk)); qemu_mutex_unlock_iothread(); From ef9c5160a18f4d0eee3cbd878fab766b22797269 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Thu, 8 Mar 2018 12:18:26 +0100 Subject: [PATCH 4/6] migration/block: rename MAX_INFLIGHT_IO to MAX_IO_BUFFERS this actually limits (as the original commit mesage suggests) the number of I/O buffers that can be allocated and not the number of parallel (inflight) I/O requests. Signed-off-by: Peter Lieven Message-Id: <1520507908-16743-4-git-send-email-pl@kamp.de> Reviewed-by: Juan Quintela Signed-off-by: Dr. David Alan Gilbert --- migration/block.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/migration/block.c b/migration/block.c index 87bb35ce63..41b95d1dd8 100644 --- a/migration/block.c +++ b/migration/block.c @@ -36,7 +36,7 @@ #define MAX_IS_ALLOCATED_SEARCH (65536 * BDRV_SECTOR_SIZE) -#define MAX_INFLIGHT_IO 512 +#define MAX_IO_BUFFERS 512 //#define DEBUG_BLK_MIGRATION @@ -775,9 +775,8 @@ static int block_save_iterate(QEMUFile *f, void *opaque) while ((block_mig_state.submitted + block_mig_state.read_done) * BLOCK_SIZE < qemu_file_get_rate_limit(f) && - (block_mig_state.submitted + - block_mig_state.read_done) < - MAX_INFLIGHT_IO) { + (block_mig_state.submitted + block_mig_state.read_done) < + MAX_IO_BUFFERS) { blk_mig_unlock(); if (block_mig_state.bulk_completed == 0) { /* first finish the bulk phase */ From dd0ee30caeebbd25a80d8e985f3ee1b9343c05f1 Mon Sep 17 00:00:00 2001 From: Peter Xu Date: Mon, 5 Mar 2018 17:49:38 +0800 Subject: [PATCH 5/6] migration: fix applying wrong capabilities When setting migration capabilities via QMP/HMP, we'll apply them even if the capability check failed. Fix it. Fixes: 4a84214ebe ("migration: provide migrate_caps_check()", 2017-07-18) Signed-off-by: Peter Xu Message-Id: <20180305094938.31374-1-peterx@redhat.com> Reviewed-by: Dr. David Alan Gilbert Signed-off-by: Dr. David Alan Gilbert --- migration/migration.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/migration/migration.c b/migration/migration.c index 62c243d2d4..6a4780ef6f 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -747,13 +747,15 @@ void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params, { MigrationState *s = migrate_get_current(); MigrationCapabilityStatusList *cap; + bool cap_list[MIGRATION_CAPABILITY__MAX]; if (migration_is_setup_or_active(s->state)) { error_setg(errp, QERR_MIGRATION_ACTIVE); return; } - if (!migrate_caps_check(s->enabled_capabilities, params, errp)) { + memcpy(cap_list, s->enabled_capabilities, sizeof(cap_list)); + if (!migrate_caps_check(cap_list, params, errp)) { return; } From f96d6651e4b7cb8a8e91774b9330d82c333171d6 Mon Sep 17 00:00:00 2001 From: "Dr. David Alan Gilbert" Date: Tue, 6 Mar 2018 17:30:42 +0000 Subject: [PATCH 6/6] tests: Silence migration-test 'bad' test In 2c9bb29703c I added a migration test that purposely fails; unfortunately it prints a copy of the failure message to stderr which makes the output a bit messy. Hide stderr for that test. Signed-off-by: Dr. David Alan Gilbert Message-Id: <20180306173042.24572-1-dgilbert@redhat.com> Reviewed-by: Peter Xu Tested-by: Peter Xu Signed-off-by: Dr. David Alan Gilbert --- tests/migration-test.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/tests/migration-test.c b/tests/migration-test.c index 74f9361bdd..422bf1afdf 100644 --- a/tests/migration-test.c +++ b/tests/migration-test.c @@ -382,7 +382,7 @@ static void migrate_start_postcopy(QTestState *who) } static void test_migrate_start(QTestState **from, QTestState **to, - const char *uri) + const char *uri, bool hide_stderr) { gchar *cmd_src, *cmd_dst; char *bootpath = g_strdup_printf("%s/bootsect", tmpfs); @@ -427,6 +427,17 @@ static void test_migrate_start(QTestState **from, QTestState **to, g_free(bootpath); + if (hide_stderr) { + gchar *tmp; + tmp = g_strdup_printf("%s 2>/dev/null", cmd_src); + g_free(cmd_src); + cmd_src = tmp; + + tmp = g_strdup_printf("%s 2>/dev/null", cmd_dst); + g_free(cmd_dst); + cmd_dst = tmp; + } + *from = qtest_start(cmd_src); g_free(cmd_src); @@ -518,7 +529,7 @@ static void test_migrate(void) char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); QTestState *from, *to; - test_migrate_start(&from, &to, uri); + test_migrate_start(&from, &to, uri, false); migrate_set_capability(from, "postcopy-ram", "true"); migrate_set_capability(to, "postcopy-ram", "true"); @@ -560,7 +571,7 @@ static void test_baddest(void) const char *status; bool failed; - test_migrate_start(&from, &to, "tcp:0:0"); + test_migrate_start(&from, &to, "tcp:0:0", true); migrate(from, "tcp:0:0"); do { rsp = wait_command(from, "{ 'execute': 'query-migrate' }");