From 8f8d528e73144c063deafc534df0a04c67f6f98e Mon Sep 17 00:00:00 2001 From: Wei Yang Date: Wed, 17 Jul 2019 08:53:41 +0800 Subject: migration: use migration_is_active to represent active state Wrap the check into a function to make it easy to read. Signed-off-by: Wei Yang Message-Id: <20190717005341.14140-1-richardw.yang@linux.intel.com> Reviewed-by: Dr. David Alan Gilbert Signed-off-by: Dr. David Alan Gilbert --- migration/migration.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'migration') diff --git a/migration/migration.c b/migration/migration.c index 5f7e4d1..0c51aa6 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -1533,8 +1533,7 @@ static void migrate_fd_cleanup(MigrationState *s) qemu_fclose(tmp); } - assert((s->state != MIGRATION_STATUS_ACTIVE) && - (s->state != MIGRATION_STATUS_POSTCOPY_ACTIVE)); + assert(!migration_is_active(s)); if (s->state == MIGRATION_STATUS_CANCELLING) { migrate_set_state(&s->state, MIGRATION_STATUS_CANCELLING, @@ -1703,6 +1702,12 @@ bool migration_is_idle(void) return false; } +bool migration_is_active(MigrationState *s) +{ + return (s->state == MIGRATION_STATUS_ACTIVE || + s->state == MIGRATION_STATUS_POSTCOPY_ACTIVE); +} + void migrate_init(MigrationState *s) { /* @@ -3266,8 +3271,7 @@ static void *migration_thread(void *opaque) trace_migration_thread_setup_complete(); - while (s->state == MIGRATION_STATUS_ACTIVE || - s->state == MIGRATION_STATUS_POSTCOPY_ACTIVE) { + while (migration_is_active(s)) { int64_t current_time; if (urgent || !qemu_file_rate_limit(s->to_dst_file)) { -- cgit v1.1 From 0e6ebd487743b1c79ad8d8a9005ce989bbaddb6c Mon Sep 17 00:00:00 2001 From: "Dr. David Alan Gilbert" Date: Mon, 7 Oct 2019 15:36:38 +0100 Subject: migration: Fix missing rcu_read_unlock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use the automatic rcu_read unlocker to fix a missing unlock. Signed-off-by: Dr. David Alan Gilbert Reviewed-by: Daniel P. Berrangé Message-Id: <20191007143642.301445-3-dgilbert@redhat.com> Signed-off-by: Dr. David Alan Gilbert --- migration/ram.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) (limited to 'migration') diff --git a/migration/ram.c b/migration/ram.c index 22423f0..484d663 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -3373,24 +3373,23 @@ static int ram_save_setup(QEMUFile *f, void *opaque) } (*rsp)->f = f; - rcu_read_lock(); - - qemu_put_be64(f, ram_bytes_total_common(true) | RAM_SAVE_FLAG_MEM_SIZE); + WITH_RCU_READ_LOCK_GUARD() { + qemu_put_be64(f, ram_bytes_total_common(true) | RAM_SAVE_FLAG_MEM_SIZE); - RAMBLOCK_FOREACH_MIGRATABLE(block) { - qemu_put_byte(f, strlen(block->idstr)); - qemu_put_buffer(f, (uint8_t *)block->idstr, strlen(block->idstr)); - qemu_put_be64(f, block->used_length); - if (migrate_postcopy_ram() && block->page_size != qemu_host_page_size) { - qemu_put_be64(f, block->page_size); - } - if (migrate_ignore_shared()) { - qemu_put_be64(f, block->mr->addr); + RAMBLOCK_FOREACH_MIGRATABLE(block) { + qemu_put_byte(f, strlen(block->idstr)); + qemu_put_buffer(f, (uint8_t *)block->idstr, strlen(block->idstr)); + qemu_put_be64(f, block->used_length); + if (migrate_postcopy_ram() && block->page_size != + qemu_host_page_size) { + qemu_put_be64(f, block->page_size); + } + if (migrate_ignore_shared()) { + qemu_put_be64(f, block->mr->addr); + } } } - rcu_read_unlock(); - ram_control_before_iterate(f, RAM_CONTROL_SETUP); ram_control_after_iterate(f, RAM_CONTROL_SETUP); -- cgit v1.1 From 89ac5a1d2a32e9cc520ed5a8a2ff217e11a962eb Mon Sep 17 00:00:00 2001 From: "Dr. David Alan Gilbert" Date: Mon, 7 Oct 2019 15:36:39 +0100 Subject: migration: Use automatic rcu_read unlock in ram.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use the automatic read unlocker in migration/ram.c Signed-off-by: Dr. David Alan Gilbert Reviewed-by: Daniel P. Berrangé Message-Id: <20191007143642.301445-4-dgilbert@redhat.com> Signed-off-by: Dr. David Alan Gilbert --- migration/ram.c | 259 ++++++++++++++++++++++++++------------------------------ 1 file changed, 121 insertions(+), 138 deletions(-) (limited to 'migration') diff --git a/migration/ram.c b/migration/ram.c index 484d663..e29c8b3 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -181,14 +181,14 @@ int foreach_not_ignored_block(RAMBlockIterFunc func, void *opaque) RAMBlock *block; int ret = 0; - rcu_read_lock(); + RCU_READ_LOCK_GUARD(); + RAMBLOCK_FOREACH_NOT_IGNORED(block) { ret = func(block, opaque); if (ret) { break; } } - rcu_read_unlock(); return ret; } @@ -1848,12 +1848,12 @@ static void migration_bitmap_sync(RAMState *rs) memory_global_dirty_log_sync(); qemu_mutex_lock(&rs->bitmap_mutex); - rcu_read_lock(); - RAMBLOCK_FOREACH_NOT_IGNORED(block) { - ramblock_sync_dirty_bitmap(rs, block); + WITH_RCU_READ_LOCK_GUARD() { + RAMBLOCK_FOREACH_NOT_IGNORED(block) { + ramblock_sync_dirty_bitmap(rs, block); + } + ram_counters.remaining = ram_bytes_remaining(); } - ram_counters.remaining = ram_bytes_remaining(); - rcu_read_unlock(); qemu_mutex_unlock(&rs->bitmap_mutex); memory_global_after_dirty_log_sync(); @@ -2397,13 +2397,12 @@ static void migration_page_queue_free(RAMState *rs) /* This queue generally should be empty - but in the case of a failed * migration might have some droppings in. */ - rcu_read_lock(); + RCU_READ_LOCK_GUARD(); QSIMPLEQ_FOREACH_SAFE(mspr, &rs->src_page_requests, next_req, next_mspr) { memory_region_unref(mspr->rb->mr); QSIMPLEQ_REMOVE_HEAD(&rs->src_page_requests, next_req); g_free(mspr); } - rcu_read_unlock(); } /** @@ -2424,7 +2423,8 @@ int ram_save_queue_pages(const char *rbname, ram_addr_t start, ram_addr_t len) RAMState *rs = ram_state; ram_counters.postcopy_requests++; - rcu_read_lock(); + RCU_READ_LOCK_GUARD(); + if (!rbname) { /* Reuse last RAMBlock */ ramblock = rs->last_req_rb; @@ -2466,12 +2466,10 @@ int ram_save_queue_pages(const char *rbname, ram_addr_t start, ram_addr_t len) QSIMPLEQ_INSERT_TAIL(&rs->src_page_requests, new_entry, next_req); migration_make_urgent_request(); qemu_mutex_unlock(&rs->src_page_req_mutex); - rcu_read_unlock(); return 0; err: - rcu_read_unlock(); return -1; } @@ -2700,7 +2698,8 @@ static uint64_t ram_bytes_total_common(bool count_ignored) RAMBlock *block; uint64_t total = 0; - rcu_read_lock(); + RCU_READ_LOCK_GUARD(); + if (count_ignored) { RAMBLOCK_FOREACH_MIGRATABLE(block) { total += block->used_length; @@ -2710,7 +2709,6 @@ static uint64_t ram_bytes_total_common(bool count_ignored) total += block->used_length; } } - rcu_read_unlock(); return total; } @@ -3034,7 +3032,7 @@ int ram_postcopy_send_discard_bitmap(MigrationState *ms) RAMBlock *block; int ret; - rcu_read_lock(); + RCU_READ_LOCK_GUARD(); /* This should be our last sync, the src is now paused */ migration_bitmap_sync(rs); @@ -3048,7 +3046,6 @@ int ram_postcopy_send_discard_bitmap(MigrationState *ms) /* Deal with TPS != HPS and huge pages */ ret = postcopy_chunk_hostpages(ms, block); if (ret) { - rcu_read_unlock(); return ret; } @@ -3060,7 +3057,6 @@ int ram_postcopy_send_discard_bitmap(MigrationState *ms) trace_ram_postcopy_send_discard_bitmap(); ret = postcopy_each_ram_send_discard(ms); - rcu_read_unlock(); return ret; } @@ -3081,7 +3077,7 @@ int ram_discard_range(const char *rbname, uint64_t start, size_t length) trace_ram_discard_range(rbname, start, length); - rcu_read_lock(); + RCU_READ_LOCK_GUARD(); RAMBlock *rb = qemu_ram_block_by_name(rbname); if (!rb) { @@ -3101,8 +3097,6 @@ int ram_discard_range(const char *rbname, uint64_t start, size_t length) ret = ram_block_discard_range(rb, start, length); err: - rcu_read_unlock(); - return ret; } @@ -3231,13 +3225,12 @@ static void ram_init_bitmaps(RAMState *rs) /* For memory_global_dirty_log_start below. */ qemu_mutex_lock_iothread(); qemu_mutex_lock_ramlist(); - rcu_read_lock(); - ram_list_init_bitmaps(); - memory_global_dirty_log_start(); - migration_bitmap_sync_precopy(rs); - - rcu_read_unlock(); + WITH_RCU_READ_LOCK_GUARD() { + ram_list_init_bitmaps(); + memory_global_dirty_log_start(); + migration_bitmap_sync_precopy(rs); + } qemu_mutex_unlock_ramlist(); qemu_mutex_unlock_iothread(); } @@ -3424,55 +3417,57 @@ static int ram_save_iterate(QEMUFile *f, void *opaque) goto out; } - rcu_read_lock(); - if (ram_list.version != rs->last_version) { - ram_state_reset(rs); - } - - /* Read version before ram_list.blocks */ - smp_rmb(); + WITH_RCU_READ_LOCK_GUARD() { + if (ram_list.version != rs->last_version) { + ram_state_reset(rs); + } - ram_control_before_iterate(f, RAM_CONTROL_ROUND); + /* Read version before ram_list.blocks */ + smp_rmb(); - t0 = qemu_clock_get_ns(QEMU_CLOCK_REALTIME); - i = 0; - while ((ret = qemu_file_rate_limit(f)) == 0 || - !QSIMPLEQ_EMPTY(&rs->src_page_requests)) { - int pages; + ram_control_before_iterate(f, RAM_CONTROL_ROUND); - if (qemu_file_get_error(f)) { - break; - } + t0 = qemu_clock_get_ns(QEMU_CLOCK_REALTIME); + i = 0; + while ((ret = qemu_file_rate_limit(f)) == 0 || + !QSIMPLEQ_EMPTY(&rs->src_page_requests)) { + int pages; - pages = ram_find_and_save_block(rs, false); - /* no more pages to sent */ - if (pages == 0) { - done = 1; - break; - } + if (qemu_file_get_error(f)) { + break; + } - if (pages < 0) { - qemu_file_set_error(f, pages); - break; - } + pages = ram_find_and_save_block(rs, false); + /* no more pages to sent */ + if (pages == 0) { + done = 1; + break; + } - rs->target_page_count += pages; - - /* we want to check in the 1st loop, just in case it was the 1st time - and we had to sync the dirty bitmap. - qemu_clock_get_ns() is a bit expensive, so we only check each some - iterations - */ - if ((i & 63) == 0) { - uint64_t t1 = (qemu_clock_get_ns(QEMU_CLOCK_REALTIME) - t0) / 1000000; - if (t1 > MAX_WAIT) { - trace_ram_save_iterate_big_wait(t1, i); + if (pages < 0) { + qemu_file_set_error(f, pages); break; } + + rs->target_page_count += pages; + + /* + * we want to check in the 1st loop, just in case it was the 1st + * time and we had to sync the dirty bitmap. + * qemu_clock_get_ns() is a bit expensive, so we only check each + * some iterations + */ + if ((i & 63) == 0) { + uint64_t t1 = (qemu_clock_get_ns(QEMU_CLOCK_REALTIME) - t0) / + 1000000; + if (t1 > MAX_WAIT) { + trace_ram_save_iterate_big_wait(t1, i); + break; + } + } + i++; } - i++; } - rcu_read_unlock(); /* * Must occur before EOS (or any QEMUFile operation) @@ -3510,35 +3505,33 @@ static int ram_save_complete(QEMUFile *f, void *opaque) RAMState *rs = *temp; int ret = 0; - rcu_read_lock(); - - if (!migration_in_postcopy()) { - migration_bitmap_sync_precopy(rs); - } + WITH_RCU_READ_LOCK_GUARD() { + if (!migration_in_postcopy()) { + migration_bitmap_sync_precopy(rs); + } - ram_control_before_iterate(f, RAM_CONTROL_FINISH); + ram_control_before_iterate(f, RAM_CONTROL_FINISH); - /* try transferring iterative blocks of memory */ + /* try transferring iterative blocks of memory */ - /* flush all remaining blocks regardless of rate limiting */ - while (true) { - int pages; + /* flush all remaining blocks regardless of rate limiting */ + while (true) { + int pages; - pages = ram_find_and_save_block(rs, !migration_in_colo_state()); - /* no more blocks to sent */ - if (pages == 0) { - break; - } - if (pages < 0) { - ret = pages; - break; + pages = ram_find_and_save_block(rs, !migration_in_colo_state()); + /* no more blocks to sent */ + if (pages == 0) { + break; + } + if (pages < 0) { + ret = pages; + break; + } } - } - - flush_compressed_data(rs); - ram_control_after_iterate(f, RAM_CONTROL_FINISH); - rcu_read_unlock(); + flush_compressed_data(rs); + ram_control_after_iterate(f, RAM_CONTROL_FINISH); + } multifd_send_sync_main(rs); qemu_put_be64(f, RAM_SAVE_FLAG_EOS); @@ -3561,9 +3554,9 @@ static void ram_save_pending(QEMUFile *f, void *opaque, uint64_t max_size, if (!migration_in_postcopy() && remaining_size < max_size) { qemu_mutex_lock_iothread(); - rcu_read_lock(); - migration_bitmap_sync_precopy(rs); - rcu_read_unlock(); + WITH_RCU_READ_LOCK_GUARD() { + migration_bitmap_sync_precopy(rs); + } qemu_mutex_unlock_iothread(); remaining_size = rs->migration_dirty_pages * TARGET_PAGE_SIZE; } @@ -3907,7 +3900,13 @@ int colo_init_ram_cache(void) error_report("%s: Can't alloc memory for COLO cache of block %s," "size 0x" RAM_ADDR_FMT, __func__, block->idstr, block->used_length); - goto out_locked; + RAMBLOCK_FOREACH_NOT_IGNORED(block) { + if (block->colo_cache) { + qemu_anon_ram_free(block->colo_cache, block->used_length); + block->colo_cache = NULL; + } + } + return -errno; } memcpy(block->colo_cache, block->host, block->used_length); } @@ -3933,18 +3932,6 @@ int colo_init_ram_cache(void) memory_global_dirty_log_start(); return 0; - -out_locked: - - RAMBLOCK_FOREACH_NOT_IGNORED(block) { - if (block->colo_cache) { - qemu_anon_ram_free(block->colo_cache, block->used_length); - block->colo_cache = NULL; - } - } - - rcu_read_unlock(); - return -errno; } /* It is need to hold the global lock to call this helper */ @@ -3958,16 +3945,14 @@ void colo_release_ram_cache(void) block->bmap = NULL; } - rcu_read_lock(); - - RAMBLOCK_FOREACH_NOT_IGNORED(block) { - if (block->colo_cache) { - qemu_anon_ram_free(block->colo_cache, block->used_length); - block->colo_cache = NULL; + WITH_RCU_READ_LOCK_GUARD() { + RAMBLOCK_FOREACH_NOT_IGNORED(block) { + if (block->colo_cache) { + qemu_anon_ram_free(block->colo_cache, block->used_length); + block->colo_cache = NULL; + } } } - - rcu_read_unlock(); qemu_mutex_destroy(&ram_state->bitmap_mutex); g_free(ram_state); ram_state = NULL; @@ -4205,31 +4190,30 @@ static void colo_flush_ram_cache(void) unsigned long offset = 0; memory_global_dirty_log_sync(); - rcu_read_lock(); - RAMBLOCK_FOREACH_NOT_IGNORED(block) { - ramblock_sync_dirty_bitmap(ram_state, block); + WITH_RCU_READ_LOCK_GUARD() { + RAMBLOCK_FOREACH_NOT_IGNORED(block) { + ramblock_sync_dirty_bitmap(ram_state, block); + } } - rcu_read_unlock(); trace_colo_flush_ram_cache_begin(ram_state->migration_dirty_pages); - rcu_read_lock(); - block = QLIST_FIRST_RCU(&ram_list.blocks); + WITH_RCU_READ_LOCK_GUARD() { + block = QLIST_FIRST_RCU(&ram_list.blocks); - while (block) { - offset = migration_bitmap_find_dirty(ram_state, block, offset); + while (block) { + offset = migration_bitmap_find_dirty(ram_state, block, offset); - if (offset << TARGET_PAGE_BITS >= block->used_length) { - offset = 0; - block = QLIST_NEXT_RCU(block, next); - } else { - migration_bitmap_clear_dirty(ram_state, block, offset); - dst_host = block->host + (offset << TARGET_PAGE_BITS); - src_host = block->colo_cache + (offset << TARGET_PAGE_BITS); - memcpy(dst_host, src_host, TARGET_PAGE_SIZE); + if (offset << TARGET_PAGE_BITS >= block->used_length) { + offset = 0; + block = QLIST_NEXT_RCU(block, next); + } else { + migration_bitmap_clear_dirty(ram_state, block, offset); + dst_host = block->host + (offset << TARGET_PAGE_BITS); + src_host = block->colo_cache + (offset << TARGET_PAGE_BITS); + memcpy(dst_host, src_host, TARGET_PAGE_SIZE); + } } } - - rcu_read_unlock(); trace_colo_flush_ram_cache_end(); } @@ -4428,16 +4412,15 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id) * it will be necessary to reduce the granularity of this * critical section. */ - rcu_read_lock(); + WITH_RCU_READ_LOCK_GUARD() { + if (postcopy_running) { + ret = ram_load_postcopy(f); + } else { + ret = ram_load_precopy(f); + } - if (postcopy_running) { - ret = ram_load_postcopy(f); - } else { - ret = ram_load_precopy(f); + ret |= wait_for_decompress_done(); } - - ret |= wait_for_decompress_done(); - rcu_read_unlock(); trace_ram_load_complete(ret, seq_iter); if (!ret && migration_incoming_in_colo_state()) { -- cgit v1.1 From 987ab2a54927fe43b93787b7a49734c9f59d3264 Mon Sep 17 00:00:00 2001 From: "Dr. David Alan Gilbert" Date: Mon, 7 Oct 2019 15:36:40 +0100 Subject: migration: Use automatic rcu_read unlock in rdma.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use the automatic read unlocker in migration/rdma.c. Signed-off-by: Dr. David Alan Gilbert Reviewed-by: Daniel P. Berrangé Message-Id: <20191007143642.301445-5-dgilbert@redhat.com> Signed-off-by: Dr. David Alan Gilbert --- migration/rdma.c | 57 +++++++++++--------------------------------------------- 1 file changed, 11 insertions(+), 46 deletions(-) (limited to 'migration') diff --git a/migration/rdma.c b/migration/rdma.c index 4c74e88..e241dcb 100644 --- a/migration/rdma.c +++ b/migration/rdma.c @@ -88,7 +88,6 @@ static uint32_t known_capabilities = RDMA_CAPABILITY_PIN_ALL; " to abort!"); \ rdma->error_reported = 1; \ } \ - rcu_read_unlock(); \ return rdma->error_state; \ } \ } while (0) @@ -2678,11 +2677,10 @@ static ssize_t qio_channel_rdma_writev(QIOChannel *ioc, size_t i; size_t len = 0; - rcu_read_lock(); + RCU_READ_LOCK_GUARD(); rdma = atomic_rcu_read(&rioc->rdmaout); if (!rdma) { - rcu_read_unlock(); return -EIO; } @@ -2695,7 +2693,6 @@ static ssize_t qio_channel_rdma_writev(QIOChannel *ioc, ret = qemu_rdma_write_flush(f, rdma); if (ret < 0) { rdma->error_state = ret; - rcu_read_unlock(); return ret; } @@ -2715,7 +2712,6 @@ static ssize_t qio_channel_rdma_writev(QIOChannel *ioc, if (ret < 0) { rdma->error_state = ret; - rcu_read_unlock(); return ret; } @@ -2724,7 +2720,6 @@ static ssize_t qio_channel_rdma_writev(QIOChannel *ioc, } } - rcu_read_unlock(); return done; } @@ -2764,11 +2759,10 @@ static ssize_t qio_channel_rdma_readv(QIOChannel *ioc, ssize_t i; size_t done = 0; - rcu_read_lock(); + RCU_READ_LOCK_GUARD(); rdma = atomic_rcu_read(&rioc->rdmain); if (!rdma) { - rcu_read_unlock(); return -EIO; } @@ -2805,7 +2799,6 @@ static ssize_t qio_channel_rdma_readv(QIOChannel *ioc, if (ret < 0) { rdma->error_state = ret; - rcu_read_unlock(); return ret; } @@ -2819,14 +2812,12 @@ static ssize_t qio_channel_rdma_readv(QIOChannel *ioc, /* Still didn't get enough, so lets just return */ if (want) { if (done == 0) { - rcu_read_unlock(); return QIO_CHANNEL_ERR_BLOCK; } else { break; } } } - rcu_read_unlock(); return done; } @@ -2882,7 +2873,7 @@ qio_channel_rdma_source_prepare(GSource *source, GIOCondition cond = 0; *timeout = -1; - rcu_read_lock(); + RCU_READ_LOCK_GUARD(); if (rsource->condition == G_IO_IN) { rdma = atomic_rcu_read(&rsource->rioc->rdmain); } else { @@ -2891,7 +2882,6 @@ qio_channel_rdma_source_prepare(GSource *source, if (!rdma) { error_report("RDMAContext is NULL when prepare Gsource"); - rcu_read_unlock(); return FALSE; } @@ -2900,7 +2890,6 @@ qio_channel_rdma_source_prepare(GSource *source, } cond |= G_IO_OUT; - rcu_read_unlock(); return cond & rsource->condition; } @@ -2911,7 +2900,7 @@ qio_channel_rdma_source_check(GSource *source) RDMAContext *rdma; GIOCondition cond = 0; - rcu_read_lock(); + RCU_READ_LOCK_GUARD(); if (rsource->condition == G_IO_IN) { rdma = atomic_rcu_read(&rsource->rioc->rdmain); } else { @@ -2920,7 +2909,6 @@ qio_channel_rdma_source_check(GSource *source) if (!rdma) { error_report("RDMAContext is NULL when check Gsource"); - rcu_read_unlock(); return FALSE; } @@ -2929,7 +2917,6 @@ qio_channel_rdma_source_check(GSource *source) } cond |= G_IO_OUT; - rcu_read_unlock(); return cond & rsource->condition; } @@ -2943,7 +2930,7 @@ qio_channel_rdma_source_dispatch(GSource *source, RDMAContext *rdma; GIOCondition cond = 0; - rcu_read_lock(); + RCU_READ_LOCK_GUARD(); if (rsource->condition == G_IO_IN) { rdma = atomic_rcu_read(&rsource->rioc->rdmain); } else { @@ -2952,7 +2939,6 @@ qio_channel_rdma_source_dispatch(GSource *source, if (!rdma) { error_report("RDMAContext is NULL when dispatch Gsource"); - rcu_read_unlock(); return FALSE; } @@ -2961,7 +2947,6 @@ qio_channel_rdma_source_dispatch(GSource *source, } cond |= G_IO_OUT; - rcu_read_unlock(); return (*func)(QIO_CHANNEL(rsource->rioc), (cond & rsource->condition), user_data); @@ -3073,7 +3058,7 @@ qio_channel_rdma_shutdown(QIOChannel *ioc, QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(ioc); RDMAContext *rdmain, *rdmaout; - rcu_read_lock(); + RCU_READ_LOCK_GUARD(); rdmain = atomic_rcu_read(&rioc->rdmain); rdmaout = atomic_rcu_read(&rioc->rdmain); @@ -3100,7 +3085,6 @@ qio_channel_rdma_shutdown(QIOChannel *ioc, break; } - rcu_read_unlock(); return 0; } @@ -3146,18 +3130,16 @@ static size_t qemu_rdma_save_page(QEMUFile *f, void *opaque, RDMAContext *rdma; int ret; - rcu_read_lock(); + RCU_READ_LOCK_GUARD(); rdma = atomic_rcu_read(&rioc->rdmaout); if (!rdma) { - rcu_read_unlock(); return -EIO; } CHECK_ERROR_STATE(); if (migration_in_postcopy()) { - rcu_read_unlock(); return RAM_SAVE_CONTROL_NOT_SUPP; } @@ -3242,11 +3224,9 @@ static size_t qemu_rdma_save_page(QEMUFile *f, void *opaque, } } - rcu_read_unlock(); return RAM_SAVE_CONTROL_DELAYED; err: rdma->error_state = ret; - rcu_read_unlock(); return ret; } @@ -3470,11 +3450,10 @@ static int qemu_rdma_registration_handle(QEMUFile *f, void *opaque) int count = 0; int i = 0; - rcu_read_lock(); + RCU_READ_LOCK_GUARD(); rdma = atomic_rcu_read(&rioc->rdmain); if (!rdma) { - rcu_read_unlock(); return -EIO; } @@ -3717,7 +3696,6 @@ out: if (ret < 0) { rdma->error_state = ret; } - rcu_read_unlock(); return ret; } @@ -3735,11 +3713,10 @@ rdma_block_notification_handle(QIOChannelRDMA *rioc, const char *name) int curr; int found = -1; - rcu_read_lock(); + RCU_READ_LOCK_GUARD(); rdma = atomic_rcu_read(&rioc->rdmain); if (!rdma) { - rcu_read_unlock(); return -EIO; } @@ -3753,7 +3730,6 @@ rdma_block_notification_handle(QIOChannelRDMA *rioc, const char *name) if (found == -1) { error_report("RAMBlock '%s' not found on destination", name); - rcu_read_unlock(); return -ENOENT; } @@ -3761,7 +3737,6 @@ rdma_block_notification_handle(QIOChannelRDMA *rioc, const char *name) trace_rdma_block_notification_handle(name, rdma->next_src_index); rdma->next_src_index++; - rcu_read_unlock(); return 0; } @@ -3786,17 +3761,15 @@ static int qemu_rdma_registration_start(QEMUFile *f, void *opaque, QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(opaque); RDMAContext *rdma; - rcu_read_lock(); + RCU_READ_LOCK_GUARD(); rdma = atomic_rcu_read(&rioc->rdmaout); if (!rdma) { - rcu_read_unlock(); return -EIO; } CHECK_ERROR_STATE(); if (migration_in_postcopy()) { - rcu_read_unlock(); return 0; } @@ -3804,7 +3777,6 @@ static int qemu_rdma_registration_start(QEMUFile *f, void *opaque, qemu_put_be64(f, RAM_SAVE_FLAG_HOOK); qemu_fflush(f); - rcu_read_unlock(); return 0; } @@ -3821,17 +3793,15 @@ static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque, RDMAControlHeader head = { .len = 0, .repeat = 1 }; int ret = 0; - rcu_read_lock(); + RCU_READ_LOCK_GUARD(); rdma = atomic_rcu_read(&rioc->rdmaout); if (!rdma) { - rcu_read_unlock(); return -EIO; } CHECK_ERROR_STATE(); if (migration_in_postcopy()) { - rcu_read_unlock(); return 0; } @@ -3863,7 +3833,6 @@ static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque, qemu_rdma_reg_whole_ram_blocks : NULL); if (ret < 0) { ERROR(errp, "receiving remote info!"); - rcu_read_unlock(); return ret; } @@ -3887,7 +3856,6 @@ static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque, "not identical on both the source and destination.", local->nb_blocks, nb_dest_blocks); rdma->error_state = -EINVAL; - rcu_read_unlock(); return -EINVAL; } @@ -3904,7 +3872,6 @@ static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque, local->block[i].length, rdma->dest_blocks[i].length); rdma->error_state = -EINVAL; - rcu_read_unlock(); return -EINVAL; } local->block[i].remote_host_addr = @@ -3922,11 +3889,9 @@ static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque, goto err; } - rcu_read_unlock(); return 0; err: rdma->error_state = ret; - rcu_read_unlock(); return ret; } -- cgit v1.1 From fb14a42ade228e2d6123af56c0015262fd83250d Mon Sep 17 00:00:00 2001 From: "Dr. David Alan Gilbert" Date: Mon, 7 Oct 2019 11:35:07 +0100 Subject: migration: Don't try and recover return path in non-postcopy In normal precopy we can't do reconnection recovery - but we also don't need to, since you can just rerun migration. At the moment if the 'return-path' capability is on, we use the return path in precopy to give a positive 'OK' to the end of migration; however if migration fails then we fall into the postcopy recovery path and hang. This fixes it by only running the return path in the postcopy case. Reported-by: Greg Kurz Tested-by: Greg Kurz Reviewed-by: Peter Xu Signed-off-by: Dr. David Alan Gilbert --- migration/migration.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'migration') diff --git a/migration/migration.c b/migration/migration.c index 0c51aa6..d7f8b42 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -2486,7 +2486,7 @@ retry: out: res = qemu_file_get_error(rp); if (res) { - if (res == -EIO) { + if (res == -EIO && migration_in_postcopy()) { /* * Maybe there is something we can do: it looks like a * network down issue, and we pause for a recovery. -- cgit v1.1 From 3414322a83be11b86166d2a53432615092bdcbb8 Mon Sep 17 00:00:00 2001 From: Wei Yang Date: Sat, 5 Oct 2019 21:50:20 +0800 Subject: migration/postcopy: allocate tmp_page in setup stage During migration, a tmp page is allocated so that we could place a whole host page during postcopy. Currently the page is allocated during load stage, this is a little bit late. And more important, if we failed to allocate it, the error is not checked properly. Even it is NULL, we would still use it. This patch moves the allocation to setup stage and if failed error message would be printed and caller would notice it. Signed-off-by: Wei Yang Reviewed-by: Dr. David Alan Gilbert Signed-off-by: Dr. David Alan Gilbert --- migration/postcopy-ram.c | 40 ++++++++++------------------------------ migration/postcopy-ram.h | 7 ------- migration/ram.c | 2 +- 3 files changed, 11 insertions(+), 38 deletions(-) (limited to 'migration') diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c index 1f63e65..9a16f52 100644 --- a/migration/postcopy-ram.c +++ b/migration/postcopy-ram.c @@ -1134,6 +1134,16 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis) return -1; } + mis->postcopy_tmp_page = mmap(NULL, mis->largest_page_size, + PROT_READ | PROT_WRITE, MAP_PRIVATE | + MAP_ANONYMOUS, -1, 0); + if (mis->postcopy_tmp_page == MAP_FAILED) { + mis->postcopy_tmp_page = NULL; + error_report("%s: Failed to map postcopy_tmp_page %s", + __func__, strerror(errno)); + return -1; + } + /* * Ballooning can mark pages as absent while we're postcopying * that would cause false userfaults. @@ -1260,30 +1270,6 @@ int postcopy_place_page_zero(MigrationIncomingState *mis, void *host, } } -/* - * Returns a target page of memory that can be mapped at a later point in time - * using postcopy_place_page - * The same address is used repeatedly, postcopy_place_page just takes the - * backing page away. - * Returns: Pointer to allocated page - * - */ -void *postcopy_get_tmp_page(MigrationIncomingState *mis) -{ - if (!mis->postcopy_tmp_page) { - mis->postcopy_tmp_page = mmap(NULL, mis->largest_page_size, - PROT_READ | PROT_WRITE, MAP_PRIVATE | - MAP_ANONYMOUS, -1, 0); - if (mis->postcopy_tmp_page == MAP_FAILED) { - mis->postcopy_tmp_page = NULL; - error_report("%s: %s", __func__, strerror(errno)); - return NULL; - } - } - - return mis->postcopy_tmp_page; -} - #else /* No target OS support, stubs just fail */ void fill_destination_postcopy_migration_info(MigrationInfo *info) @@ -1341,12 +1327,6 @@ int postcopy_place_page_zero(MigrationIncomingState *mis, void *host, return -1; } -void *postcopy_get_tmp_page(MigrationIncomingState *mis) -{ - assert(0); - return NULL; -} - int postcopy_wake_shared(struct PostCopyFD *pcfd, uint64_t client_addr, RAMBlock *rb) diff --git a/migration/postcopy-ram.h b/migration/postcopy-ram.h index 9c8bd2b..fb4cd11 100644 --- a/migration/postcopy-ram.h +++ b/migration/postcopy-ram.h @@ -100,13 +100,6 @@ typedef enum { POSTCOPY_INCOMING_END } PostcopyState; -/* - * Allocate a page of memory that can be mapped at a later point in time - * using postcopy_place_page - * Returns: Pointer to allocated page - */ -void *postcopy_get_tmp_page(MigrationIncomingState *mis); - PostcopyState postcopy_state_get(void); /* Set the state and return the old state */ PostcopyState postcopy_state_set(PostcopyState new_state); diff --git a/migration/ram.c b/migration/ram.c index e29c8b3..071687e 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -4032,7 +4032,7 @@ static int ram_load_postcopy(QEMUFile *f) bool matches_target_page_size = false; MigrationIncomingState *mis = migration_incoming_get_current(); /* Temporary page that is later 'placed' */ - void *postcopy_host_page = postcopy_get_tmp_page(mis); + void *postcopy_host_page = mis->postcopy_tmp_page; void *last_host = NULL; bool all_zero = false; -- cgit v1.1 From 6629890d55e8faca765c7dba29c659ad7d9efae9 Mon Sep 17 00:00:00 2001 From: Wei Yang Date: Sat, 5 Oct 2019 21:50:21 +0800 Subject: migration/postcopy: map large zero page in postcopy_ram_incoming_setup() postcopy_ram_incoming_setup() and postcopy_ram_incoming_cleanup() are counterpart. It is reasonable to map/unmap large zero page in these two functions respectively. Signed-off-by: Wei Yang Message-Id: <20191005135021.21721-3-richardw.yang@linux.intel.com> Reviewed-by: Dr. David Alan Gilbert Signed-off-by: Dr. David Alan Gilbert --- migration/postcopy-ram.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) (limited to 'migration') diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c index 9a16f52..08a3ed5 100644 --- a/migration/postcopy-ram.c +++ b/migration/postcopy-ram.c @@ -1145,6 +1145,22 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis) } /* + * Map large zero page when kernel can't use UFFDIO_ZEROPAGE for hugepages + */ + mis->postcopy_tmp_zero_page = mmap(NULL, mis->largest_page_size, + PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS, + -1, 0); + if (mis->postcopy_tmp_zero_page == MAP_FAILED) { + int e = errno; + mis->postcopy_tmp_zero_page = NULL; + error_report("%s: Failed to map large zero page %s", + __func__, strerror(e)); + return -e; + } + memset(mis->postcopy_tmp_zero_page, '\0', mis->largest_page_size); + + /* * Ballooning can mark pages as absent while we're postcopying * that would cause false userfaults. */ @@ -1250,23 +1266,7 @@ int postcopy_place_page_zero(MigrationIncomingState *mis, void *host, qemu_ram_block_host_offset(rb, host)); } else { - /* The kernel can't use UFFDIO_ZEROPAGE for hugepages */ - if (!mis->postcopy_tmp_zero_page) { - mis->postcopy_tmp_zero_page = mmap(NULL, mis->largest_page_size, - PROT_READ | PROT_WRITE, - MAP_PRIVATE | MAP_ANONYMOUS, - -1, 0); - if (mis->postcopy_tmp_zero_page == MAP_FAILED) { - int e = errno; - mis->postcopy_tmp_zero_page = NULL; - error_report("%s: %s mapping large zero page", - __func__, strerror(e)); - return -e; - } - memset(mis->postcopy_tmp_zero_page, '\0', mis->largest_page_size); - } - return postcopy_place_page(mis, host, mis->postcopy_tmp_zero_page, - rb); + return postcopy_place_page(mis, host, mis->postcopy_tmp_zero_page, rb); } } -- cgit v1.1 From da1725d3f9723668a61a3b83f1bf94ce14fc4e4b Mon Sep 17 00:00:00 2001 From: Wei Yang Date: Sun, 6 Oct 2019 06:05:15 +0800 Subject: migration/postcopy: fix typo in mark_postcopy_blocktime_begin's comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Wei Yang Message-Id: <20191005220517.24029-3-richardw.yang@linux.intel.com> Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Dr. David Alan Gilbert --- migration/postcopy-ram.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'migration') diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c index 08a3ed5..3a72f7b 100644 --- a/migration/postcopy-ram.c +++ b/migration/postcopy-ram.c @@ -768,9 +768,11 @@ static void mark_postcopy_blocktime_begin(uintptr_t addr, uint32_t ptid, atomic_xchg(&dc->page_fault_vcpu_time[cpu], low_time_offset); atomic_xchg(&dc->vcpu_addr[cpu], addr); - /* check it here, not at the begining of the function, - * due to, check could accur early than bitmap_set in - * qemu_ufd_copy_ioctl */ + /* + * check it here, not at the beginning of the function, + * due to, check could occur early than bitmap_set in + * qemu_ufd_copy_ioctl + */ already_received = ramblock_recv_bitmap_test(rb, (void *)addr); if (already_received) { atomic_xchg(&dc->vcpu_addr[cpu], 0); -- cgit v1.1 From 17d9351bf2415fb66500575665a22fac15b80d10 Mon Sep 17 00:00:00 2001 From: Wei Yang Date: Sun, 6 Oct 2019 06:05:16 +0800 Subject: migration: pass in_postcopy instead of check state again Not necessary to do the check again. Signed-off-by: Wei Yang Message-Id: <20191005220517.24029-4-richardw.yang@linux.intel.com> Reviewed-by: Dr. David Alan Gilbert Signed-off-by: Dr. David Alan Gilbert --- migration/migration.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'migration') diff --git a/migration/migration.c b/migration/migration.c index d7f8b42..3febd0f 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -3149,8 +3149,7 @@ static MigIterateState migration_iteration_run(MigrationState *s) return MIG_ITERATE_SKIP; } /* Just another iteration step */ - qemu_savevm_state_iterate(s->to_dst_file, - s->state == MIGRATION_STATUS_POSTCOPY_ACTIVE); + qemu_savevm_state_iterate(s->to_dst_file, in_postcopy); } else { trace_migration_thread_low_pending(pending_size); migration_completion(s); -- cgit v1.1 From 4991f3091ede9d05312447c12e914481032185c0 Mon Sep 17 00:00:00 2001 From: Wei Yang Date: Sun, 6 Oct 2019 06:05:17 +0800 Subject: migration: report SaveStateEntry id and name on failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This provides helpful information on which entry failed. Signed-off-by: Wei Yang Message-Id: <20191005220517.24029-5-richardw.yang@linux.intel.com> Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Dr. David Alan Gilbert --- migration/savevm.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'migration') diff --git a/migration/savevm.c b/migration/savevm.c index bb9462a..241c5dd 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -1215,6 +1215,8 @@ int qemu_savevm_state_iterate(QEMUFile *f, bool postcopy) save_section_footer(f, se); if (ret < 0) { + error_report("failed to save SaveStateEntry with id(name): %d(%s)", + se->section_id, se->idstr); qemu_file_set_error(f, ret); } if (ret <= 0) { -- cgit v1.1 From 2a461c2467bb58e034c857e47a4a530bdca22227 Mon Sep 17 00:00:00 2001 From: Wei Yang Date: Sun, 6 Oct 2019 08:02:47 +0800 Subject: migration/postcopy: mis->have_listen_thread check will never be touched If mis->have_listen_thread is true, this means current PostcopyState must be LISTENING or RUNNING. While the check at the beginning of the function makes sure the state transaction happens when its previous PostcopyState is ADVISE or DISCARD. This means we would never touch this check. Signed-off-by: Wei Yang Message-Id: <20191006000249.29926-2-richardw.yang@linux.intel.com> Reviewed-by: Dr. David Alan Gilbert Signed-off-by: Dr. David Alan Gilbert --- migration/savevm.c | 5 ----- 1 file changed, 5 deletions(-) (limited to 'migration') diff --git a/migration/savevm.c b/migration/savevm.c index 241c5dd..c62687a 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -1878,11 +1878,6 @@ static int loadvm_postcopy_handle_listen(MigrationIncomingState *mis) return -1; } - if (mis->have_listen_thread) { - error_report("CMD_POSTCOPY_RAM_LISTEN already has a listen thread"); - return -1; - } - mis->have_listen_thread = true; /* Start up the listening thread and wait for it to signal ready */ qemu_sem_init(&mis->listen_thread_sem, 0); -- cgit v1.1 From 2d49bacda00876736c746ce1fcea006a128bef6b Mon Sep 17 00:00:00 2001 From: Wei Yang Date: Sun, 6 Oct 2019 08:02:48 +0800 Subject: migration/postcopy: postpone setting PostcopyState to END There are two places to call function postcopy_ram_incoming_cleanup() postcopy_ram_listen_thread on migration success loadvm_postcopy_handle_listen one setup failure On success, the vm will never accept another migration. On failure, PostcopyState is transited from LISTENING to END and would be checked in qemu_loadvm_state_main(). If PostcopyState is RUNNING, migration would be paused and retried. Currently PostcopyState is set to END in function postcopy_ram_incoming_cleanup(). With above analysis, we can take this step out and postpone this till the end of listen thread to indicate the listen thread is done. This is a preparation patch for later cleanup. Signed-off-by: Wei Yang Message-Id: <20191006000249.29926-3-richardw.yang@linux.intel.com> Reviewed-by: Dr. David Alan Gilbert Signed-off-by: Dr. David Alan Gilbert Fixed up in merge to the 1 parameter postcopy_state_set --- migration/postcopy-ram.c | 2 -- migration/savevm.c | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'migration') diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c index 3a72f7b..a793bad 100644 --- a/migration/postcopy-ram.c +++ b/migration/postcopy-ram.c @@ -577,8 +577,6 @@ int postcopy_ram_incoming_cleanup(MigrationIncomingState *mis) } } - postcopy_state_set(POSTCOPY_INCOMING_END); - if (mis->postcopy_tmp_page) { munmap(mis->postcopy_tmp_page, mis->largest_page_size); mis->postcopy_tmp_page = NULL; diff --git a/migration/savevm.c b/migration/savevm.c index c62687a..bf3da58 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -1837,6 +1837,8 @@ static void *postcopy_ram_listen_thread(void *opaque) rcu_unregister_thread(); mis->have_listen_thread = false; + postcopy_state_set(POSTCOPY_INCOMING_END); + return NULL; } -- cgit v1.1 From 2a7eb14844c1b71df58aa980f7253bddc6f11676 Mon Sep 17 00:00:00 2001 From: Wei Yang Date: Thu, 10 Oct 2019 09:13:15 +0800 Subject: migration/postcopy: rename postcopy_ram_enable_notify to postcopy_ram_incoming_setup Function postcopy_ram_incoming_setup and postcopy_ram_incoming_cleanup is a pair. Rename to make it clear for audience. Signed-off-by: Wei Yang Reviewed-by: Dr. David Alan Gilbert Message-Id: <20191010011316.31363-2-richardw.yang@linux.intel.com> Reviewed-by: Peter Xu Signed-off-by: Dr. David Alan Gilbert --- migration/postcopy-ram.c | 4 ++-- migration/postcopy-ram.h | 2 +- migration/savevm.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) (limited to 'migration') diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c index a793bad..abccafc 100644 --- a/migration/postcopy-ram.c +++ b/migration/postcopy-ram.c @@ -1094,7 +1094,7 @@ retry: return NULL; } -int postcopy_ram_enable_notify(MigrationIncomingState *mis) +int postcopy_ram_incoming_setup(MigrationIncomingState *mis) { /* Open the fd for the kernel to give us userfaults */ mis->userfault_fd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK); @@ -1307,7 +1307,7 @@ int postcopy_request_shared_page(struct PostCopyFD *pcfd, RAMBlock *rb, return -1; } -int postcopy_ram_enable_notify(MigrationIncomingState *mis) +int postcopy_ram_incoming_setup(MigrationIncomingState *mis) { assert(0); return -1; diff --git a/migration/postcopy-ram.h b/migration/postcopy-ram.h index fb4cd11..9941feb 100644 --- a/migration/postcopy-ram.h +++ b/migration/postcopy-ram.h @@ -20,7 +20,7 @@ bool postcopy_ram_supported_by_host(MigrationIncomingState *mis); * Make all of RAM sensitive to accesses to areas that haven't yet been written * and wire up anything necessary to deal with it. */ -int postcopy_ram_enable_notify(MigrationIncomingState *mis); +int postcopy_ram_incoming_setup(MigrationIncomingState *mis); /* * Initialise postcopy-ram, setting the RAM to a state where we can go into diff --git a/migration/savevm.c b/migration/savevm.c index bf3da58..95574c4 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -1869,7 +1869,7 @@ static int loadvm_postcopy_handle_listen(MigrationIncomingState *mis) * shouldn't be doing anything yet so don't actually expect requests */ if (migrate_postcopy_ram()) { - if (postcopy_ram_enable_notify(mis)) { + if (postcopy_ram_incoming_setup(mis)) { postcopy_ram_incoming_cleanup(mis); return -1; } -- cgit v1.1 From 0197d89025d768f3a2505ea50816fbc6b161b992 Mon Sep 17 00:00:00 2001 From: Wei Yang Date: Thu, 10 Oct 2019 09:13:16 +0800 Subject: migration/postcopy: check PostcopyState before setting to POSTCOPY_INCOMING_RUNNING Currently, we set PostcopyState blindly to RUNNING, even we found the previous state is not LISTENING. This will lead to a corner case. First let's look at the code flow: qemu_loadvm_state_main() ret = loadvm_process_command() loadvm_postcopy_handle_run() return -1; if (ret < 0) { if (postcopy_state_get() == POSTCOPY_INCOMING_RUNNING) ... } >From above snippet, the corner case is loadvm_postcopy_handle_run() always sets state to RUNNING. And then it checks the previous state. If the previous state is not LISTENING, it will return -1. But at this moment, PostcopyState is already been set to RUNNING. Then ret is checked in qemu_loadvm_state_main(), when it is -1 PostcopyState is checked. Current logic would pause postcopy and retry if PostcopyState is RUNNING. This is not what we expect, because postcopy is not active yet. This patch makes sure state is set to RUNNING only previous state is LISTENING by checking the state first. Signed-off-by: Wei Yang Suggested by: Peter Xu Message-Id: <20191010011316.31363-3-richardw.yang@linux.intel.com> Reviewed-by: Peter Xu Signed-off-by: Dr. David Alan Gilbert --- migration/savevm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'migration') diff --git a/migration/savevm.c b/migration/savevm.c index 95574c4..8d95e26 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -1933,7 +1933,7 @@ static void loadvm_postcopy_handle_run_bh(void *opaque) /* After all discards we can start running and asking for pages */ static int loadvm_postcopy_handle_run(MigrationIncomingState *mis) { - PostcopyState ps = postcopy_state_set(POSTCOPY_INCOMING_RUNNING); + PostcopyState ps = postcopy_state_get(); trace_loadvm_postcopy_handle_run(); if (ps != POSTCOPY_INCOMING_LISTENING) { @@ -1941,6 +1941,7 @@ static int loadvm_postcopy_handle_run(MigrationIncomingState *mis) return -1; } + postcopy_state_set(POSTCOPY_INCOMING_RUNNING); mis->bh = qemu_bh_new(loadvm_postcopy_handle_run_bh, mis); qemu_bh_schedule(mis->bh); -- cgit v1.1 From d884e77bfe384000a56ea13f448df314add84889 Mon Sep 17 00:00:00 2001 From: Wei Yang Date: Fri, 11 Oct 2019 16:50:47 +0800 Subject: migration/multifd: fix a typo in comment of multifd_recv_unfill_packet() Signed-off-by: Wei Yang Message-Id: <20191011085050.17622-2-richardw.yang@linux.intel.com> Reviewed-by: Juan Quintela Signed-off-by: Dr. David Alan Gilbert --- migration/ram.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'migration') diff --git a/migration/ram.c b/migration/ram.c index 071687e..698ba4d 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -838,7 +838,7 @@ static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp) packet->pages_alloc = be32_to_cpu(packet->pages_alloc); /* - * If we recevied a packet that is 100 times bigger than expected + * If we received a packet that is 100 times bigger than expected * just stop migration. It is a magic number. */ if (packet->pages_alloc > pages_max * 100) { -- cgit v1.1 From f2148c4c793f67e24ff97ddf12c8eeccdba9fb8e Mon Sep 17 00:00:00 2001 From: Wei Yang Date: Fri, 11 Oct 2019 16:50:48 +0800 Subject: migration/multifd: use pages->allocated instead of the static max multifd_send_fill_packet() prepares meta data for following pages to transfer. It would be more proper to fill pages->allocated instead of static max value, especially we want to support flexible packet size. Signed-off-by: Wei Yang Message-Id: <20191011085050.17622-3-richardw.yang@linux.intel.com> Reviewed-by: Juan Quintela Signed-off-by: Dr. David Alan Gilbert --- migration/ram.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'migration') diff --git a/migration/ram.c b/migration/ram.c index 698ba4d..84c5953 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -791,13 +791,12 @@ static void multifd_pages_clear(MultiFDPages_t *pages) static void multifd_send_fill_packet(MultiFDSendParams *p) { MultiFDPacket_t *packet = p->packet; - uint32_t page_max = MULTIFD_PACKET_SIZE / qemu_target_page_size(); int i; packet->magic = cpu_to_be32(MULTIFD_MAGIC); packet->version = cpu_to_be32(MULTIFD_VERSION); packet->flags = cpu_to_be32(p->flags); - packet->pages_alloc = cpu_to_be32(page_max); + packet->pages_alloc = cpu_to_be32(p->pages->allocated); packet->pages_used = cpu_to_be32(p->pages->used); packet->next_packet_size = cpu_to_be32(p->next_packet_size); packet->packet_num = cpu_to_be64(p->packet_num); -- cgit v1.1 From 9985e1f48db7449421dafdc22d275551072f0a06 Mon Sep 17 00:00:00 2001 From: Wei Yang Date: Fri, 11 Oct 2019 16:50:49 +0800 Subject: migration/multifd: initialize packet->magic/version once at setup stage MultiFDPacket_t's magic and version field never changes during migration, so move these two fields in setup stage. Signed-off-by: Wei Yang Message-Id: <20191011085050.17622-4-richardw.yang@linux.intel.com> Reviewed-by: Juan Quintela Signed-off-by: Dr. David Alan Gilbert --- migration/ram.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'migration') diff --git a/migration/ram.c b/migration/ram.c index 84c5953..963e795 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -793,8 +793,6 @@ static void multifd_send_fill_packet(MultiFDSendParams *p) MultiFDPacket_t *packet = p->packet; int i; - packet->magic = cpu_to_be32(MULTIFD_MAGIC); - packet->version = cpu_to_be32(MULTIFD_VERSION); packet->flags = cpu_to_be32(p->flags); packet->pages_alloc = cpu_to_be32(p->pages->allocated); packet->pages_used = cpu_to_be32(p->pages->used); @@ -1240,6 +1238,8 @@ int multifd_save_setup(void) p->packet_len = sizeof(MultiFDPacket_t) + sizeof(ram_addr_t) * page_count; p->packet = g_malloc0(p->packet_len); + p->packet->magic = cpu_to_be32(MULTIFD_MAGIC); + p->packet->version = cpu_to_be32(MULTIFD_VERSION); p->name = g_strdup_printf("multifdsend_%d", i); socket_send_channel_create(multifd_new_send_channel_async, p); } -- cgit v1.1 From aff66d2ef024372a27761959b46a306a6ab92f47 Mon Sep 17 00:00:00 2001 From: Wei Yang Date: Fri, 11 Oct 2019 16:50:50 +0800 Subject: migration/multifd: pages->used would be cleared when attach to multifd_send_state When we found an available channel in multifd_send_pages(), its pages->used is cleared and then attached to multifd_send_state. It is not necessary to do this twice. Signed-off-by: Wei Yang Message-Id: <20191011085050.17622-5-richardw.yang@linux.intel.com> Reviewed-by: Juan Quintela Signed-off-by: Dr. David Alan Gilbert --- migration/ram.c | 1 - 1 file changed, 1 deletion(-) (limited to 'migration') diff --git a/migration/ram.c b/migration/ram.c index 963e795..5078f94 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -1129,7 +1129,6 @@ static void *multifd_send_thread(void *opaque) p->flags = 0; p->num_packets++; p->num_pages += used; - p->pages->used = 0; qemu_mutex_unlock(&p->mutex); trace_multifd_send(p->id, packet_num, used, flags, -- cgit v1.1 From 9a85e4b8f672016adbf7b7d5beaab2a99b9b5615 Mon Sep 17 00:00:00 2001 From: Eric Auger Date: Fri, 11 Oct 2019 14:17:24 +0200 Subject: migration: Support gtree migration Introduce support for GTree migration. A custom save/restore is implemented. Each item is made of a key and a data. If the key is a pointer to an object, 2 VMSDs are passed into the GTree VMStateField. When putting the items, the tree is traversed in sorted order by g_tree_foreach. On the get() path, gtrees must be allocated using the proper key compare, key destroy and value destroy. This must be handled beforehand, for example in a pre_load method. Tests are added to test save/dump of structs containing gtrees including the virtio-iommu domain/mappings scenario. Signed-off-by: Eric Auger Message-Id: <20191011121724.433-1-eric.auger@redhat.com> Reviewed-by: Dr. David Alan Gilbert Reviewed-by: Juan Quintela Signed-off-by: Dr. David Alan Gilbert uintptr_t fixup for test on 32bit --- migration/trace-events | 5 ++ migration/vmstate-types.c | 152 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 157 insertions(+) (limited to 'migration') diff --git a/migration/trace-events b/migration/trace-events index 858d415..6dee7b5 100644 --- a/migration/trace-events +++ b/migration/trace-events @@ -71,6 +71,11 @@ get_qtailq_end(const char *name, const char *reason, int val) "%s %s/%d" put_qtailq(const char *name, int version_id) "%s v%d" put_qtailq_end(const char *name, const char *reason) "%s %s" +get_gtree(const char *field_name, const char *key_vmsd_name, const char *val_vmsd_name, uint32_t nnodes) "%s(%s/%s) nnodes=%d" +get_gtree_end(const char *field_name, const char *key_vmsd_name, const char *val_vmsd_name, int ret) "%s(%s/%s) %d" +put_gtree(const char *field_name, const char *key_vmsd_name, const char *val_vmsd_name, uint32_t nnodes) "%s(%s/%s) nnodes=%d" +put_gtree_end(const char *field_name, const char *key_vmsd_name, const char *val_vmsd_name, int ret) "%s(%s/%s) %d" + # qemu-file.c qemu_file_fclose(void) "" diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c index bee658a..7236cf9 100644 --- a/migration/vmstate-types.c +++ b/migration/vmstate-types.c @@ -691,3 +691,155 @@ const VMStateInfo vmstate_info_qtailq = { .get = get_qtailq, .put = put_qtailq, }; + +struct put_gtree_data { + QEMUFile *f; + const VMStateDescription *key_vmsd; + const VMStateDescription *val_vmsd; + QJSON *vmdesc; + int ret; +}; + +static gboolean put_gtree_elem(gpointer key, gpointer value, gpointer data) +{ + struct put_gtree_data *capsule = (struct put_gtree_data *)data; + QEMUFile *f = capsule->f; + int ret; + + qemu_put_byte(f, true); + + /* put the key */ + if (!capsule->key_vmsd) { + qemu_put_be64(f, (uint64_t)(uintptr_t)(key)); /* direct key */ + } else { + ret = vmstate_save_state(f, capsule->key_vmsd, key, capsule->vmdesc); + if (ret) { + capsule->ret = ret; + return true; + } + } + + /* put the data */ + ret = vmstate_save_state(f, capsule->val_vmsd, value, capsule->vmdesc); + if (ret) { + capsule->ret = ret; + return true; + } + return false; +} + +static int put_gtree(QEMUFile *f, void *pv, size_t unused_size, + const VMStateField *field, QJSON *vmdesc) +{ + bool direct_key = (!field->start); + const VMStateDescription *key_vmsd = direct_key ? NULL : &field->vmsd[1]; + const VMStateDescription *val_vmsd = &field->vmsd[0]; + const char *key_vmsd_name = direct_key ? "direct" : key_vmsd->name; + struct put_gtree_data capsule = { + .f = f, + .key_vmsd = key_vmsd, + .val_vmsd = val_vmsd, + .vmdesc = vmdesc, + .ret = 0}; + GTree **pval = pv; + GTree *tree = *pval; + uint32_t nnodes = g_tree_nnodes(tree); + int ret; + + trace_put_gtree(field->name, key_vmsd_name, val_vmsd->name, nnodes); + qemu_put_be32(f, nnodes); + g_tree_foreach(tree, put_gtree_elem, (gpointer)&capsule); + qemu_put_byte(f, false); + ret = capsule.ret; + if (ret) { + error_report("%s : failed to save gtree (%d)", field->name, ret); + } + trace_put_gtree_end(field->name, key_vmsd_name, val_vmsd->name, ret); + return ret; +} + +static int get_gtree(QEMUFile *f, void *pv, size_t unused_size, + const VMStateField *field) +{ + bool direct_key = (!field->start); + const VMStateDescription *key_vmsd = direct_key ? NULL : &field->vmsd[1]; + const VMStateDescription *val_vmsd = &field->vmsd[0]; + const char *key_vmsd_name = direct_key ? "direct" : key_vmsd->name; + int version_id = field->version_id; + size_t key_size = field->start; + size_t val_size = field->size; + int nnodes, count = 0; + GTree **pval = pv; + GTree *tree = *pval; + void *key, *val; + int ret = 0; + + /* in case of direct key, the key vmsd can be {}, ie. check fields */ + if (!direct_key && version_id > key_vmsd->version_id) { + error_report("%s %s", key_vmsd->name, "too new"); + return -EINVAL; + } + if (!direct_key && version_id < key_vmsd->minimum_version_id) { + error_report("%s %s", key_vmsd->name, "too old"); + return -EINVAL; + } + if (version_id > val_vmsd->version_id) { + error_report("%s %s", val_vmsd->name, "too new"); + return -EINVAL; + } + if (version_id < val_vmsd->minimum_version_id) { + error_report("%s %s", val_vmsd->name, "too old"); + return -EINVAL; + } + + nnodes = qemu_get_be32(f); + trace_get_gtree(field->name, key_vmsd_name, val_vmsd->name, nnodes); + + while (qemu_get_byte(f)) { + if ((++count) > nnodes) { + ret = -EINVAL; + break; + } + if (direct_key) { + key = (void *)(uintptr_t)qemu_get_be64(f); + } else { + key = g_malloc0(key_size); + ret = vmstate_load_state(f, key_vmsd, key, version_id); + if (ret) { + error_report("%s : failed to load %s (%d)", + field->name, key_vmsd->name, ret); + goto key_error; + } + } + val = g_malloc0(val_size); + ret = vmstate_load_state(f, val_vmsd, val, version_id); + if (ret) { + error_report("%s : failed to load %s (%d)", + field->name, val_vmsd->name, ret); + goto val_error; + } + g_tree_insert(tree, key, val); + } + if (count != nnodes) { + error_report("%s inconsistent stream when loading the gtree", + field->name); + return -EINVAL; + } + trace_get_gtree_end(field->name, key_vmsd_name, val_vmsd->name, ret); + return ret; +val_error: + g_free(val); +key_error: + if (!direct_key) { + g_free(key); + } + trace_get_gtree_end(field->name, key_vmsd_name, val_vmsd->name, ret); + return ret; +} + + +const VMStateInfo vmstate_info_gtree = { + .name = "gtree", + .get = get_gtree, + .put = put_gtree, +}; -- cgit v1.1