From fd8cec932c2ddc687e2da954978954b46a926f90 Mon Sep 17 00:00:00 2001 From: Gonglei Date: Tue, 4 Mar 2014 21:29:21 +0800 Subject: [PATCH 1/4] XBZRLE: Fix qemu crash when resize the xbzrle cache Resizing the xbzrle cache during migration causes qemu-crash, because the main-thread and migration-thread modify the xbzrle cache size concurrently without lock-protection. Signed-off-by: ChenLiang Signed-off-by: Gonglei Reviewed-by: Dr. David Alan Gilbert Signed-off-by: Juan Quintela --- arch_init.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 49 insertions(+), 3 deletions(-) diff --git a/arch_init.c b/arch_init.c index fe1727922c..60c975db2b 100644 --- a/arch_init.c +++ b/arch_init.c @@ -164,8 +164,9 @@ static struct { uint8_t *encoded_buf; /* buffer for storing page content */ uint8_t *current_buf; - /* Cache for XBZRLE */ + /* Cache for XBZRLE, Protected by lock. */ PageCache *cache; + QemuMutex lock; } XBZRLE = { .encoded_buf = NULL, .current_buf = NULL, @@ -174,16 +175,52 @@ static struct { /* buffer used for XBZRLE decoding */ static uint8_t *xbzrle_decoded_buf; +static void XBZRLE_cache_lock(void) +{ + if (migrate_use_xbzrle()) + qemu_mutex_lock(&XBZRLE.lock); +} + +static void XBZRLE_cache_unlock(void) +{ + if (migrate_use_xbzrle()) + qemu_mutex_unlock(&XBZRLE.lock); +} + int64_t xbzrle_cache_resize(int64_t new_size) { + PageCache *new_cache, *cache_to_free; + if (new_size < TARGET_PAGE_SIZE) { return -1; } + /* no need to lock, the current thread holds qemu big lock */ if (XBZRLE.cache != NULL) { - return cache_resize(XBZRLE.cache, new_size / TARGET_PAGE_SIZE) * - TARGET_PAGE_SIZE; + /* check XBZRLE.cache again later */ + if (pow2floor(new_size) == migrate_xbzrle_cache_size()) { + return pow2floor(new_size); + } + new_cache = cache_init(new_size / TARGET_PAGE_SIZE, + TARGET_PAGE_SIZE); + if (!new_cache) { + DPRINTF("Error creating cache\n"); + return -1; + } + + XBZRLE_cache_lock(); + /* the XBZRLE.cache may have be destroyed, check it again */ + if (XBZRLE.cache != NULL) { + cache_to_free = XBZRLE.cache; + XBZRLE.cache = new_cache; + } else { + cache_to_free = new_cache; + } + XBZRLE_cache_unlock(); + + cache_fini(cache_to_free); } + return pow2floor(new_size); } @@ -539,6 +576,8 @@ static int ram_save_block(QEMUFile *f, bool last_stage) ret = ram_control_save_page(f, block->offset, offset, TARGET_PAGE_SIZE, &bytes_sent); + XBZRLE_cache_lock(); + current_addr = block->offset + offset; if (ret != RAM_SAVE_CONTROL_NOT_SUPP) { if (ret != RAM_SAVE_CONTROL_DELAYED) { @@ -587,6 +626,7 @@ static int ram_save_block(QEMUFile *f, bool last_stage) acct_info.norm_pages++; } + XBZRLE_cache_unlock(); /* if page is unmodified, continue to the next */ if (bytes_sent > 0) { last_sent_block = block; @@ -654,6 +694,7 @@ static void migration_end(void) migration_bitmap = NULL; } + XBZRLE_cache_lock(); if (XBZRLE.cache) { cache_fini(XBZRLE.cache); g_free(XBZRLE.cache); @@ -663,6 +704,7 @@ static void migration_end(void) XBZRLE.encoded_buf = NULL; XBZRLE.current_buf = NULL; } + XBZRLE_cache_unlock(); } static void ram_migration_cancel(void *opaque) @@ -693,13 +735,17 @@ static int ram_save_setup(QEMUFile *f, void *opaque) dirty_rate_high_cnt = 0; if (migrate_use_xbzrle()) { + qemu_mutex_lock_iothread(); XBZRLE.cache = cache_init(migrate_xbzrle_cache_size() / TARGET_PAGE_SIZE, TARGET_PAGE_SIZE); if (!XBZRLE.cache) { + qemu_mutex_unlock_iothread(); DPRINTF("Error creating cache\n"); return -1; } + qemu_mutex_init(&XBZRLE.lock); + qemu_mutex_unlock_iothread(); /* We prefer not to abort if there is no memory */ XBZRLE.encoded_buf = g_try_malloc0(TARGET_PAGE_SIZE); From ac4df4e608e84da135eacecd7bba7c6e9e9a63b7 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 7 Mar 2014 01:33:35 +0530 Subject: [PATCH 2/4] qemu_file: Fix mismerge of "use fwrite() correctly" Reviewers accepted v2 of the patch, but what got committed was v1, with the R-bys for v2. This is the v1->v2 followup fix. [Amit: This fixes commit aded6539d983280212e08d09f14157b1cb4d58cc ] Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Reviewed-by: Amit Shah Signed-off-by: Amit Shah Signed-off-by: Juan Quintela --- qemu-file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qemu-file.c b/qemu-file.c index f074af15c3..e5ec798e0b 100644 --- a/qemu-file.c +++ b/qemu-file.c @@ -105,7 +105,7 @@ static int stdio_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, res = fwrite(buf, 1, size, s->stdio_file); if (res != size) { - return -EIO; /* fake errno value */ + return -errno; } return res; } From 4fed9421e931128bd3c86a4f1c90a5989beb88eb Mon Sep 17 00:00:00 2001 From: Alexey Kardashevskiy Date: Fri, 7 Mar 2014 01:33:36 +0530 Subject: [PATCH 3/4] vl: add system_wakeup_request tracepoint It might be useful for tracing migration. Signed-off-by: Alexey Kardashevskiy Signed-off-by: Amit Shah Signed-off-by: Juan Quintela --- trace-events | 1 + vl.c | 2 ++ 2 files changed, 3 insertions(+) diff --git a/trace-events b/trace-events index aec420292c..466c27efdd 100644 --- a/trace-events +++ b/trace-events @@ -486,6 +486,7 @@ runstate_set(int new_state) "new state %d" g_malloc(size_t size, void *ptr) "size %zu ptr %p" g_realloc(void *ptr, size_t size, void *newptr) "ptr %p size %zu newptr %p" g_free(void *ptr) "ptr %p" +system_wakeup_request(int reason) "reason=%d" # block/qcow2.c qcow2_writev_start_req(void *co, int64_t sector, int nb_sectors) "co %p sector %" PRIx64 " nb_sectors %d" diff --git a/vl.c b/vl.c index 41581c1c23..50693e6efd 100644 --- a/vl.c +++ b/vl.c @@ -1837,6 +1837,8 @@ void qemu_register_suspend_notifier(Notifier *notifier) void qemu_system_wakeup_request(WakeupReason reason) { + trace_system_wakeup_request(reason); + if (!runstate_check(RUN_STATE_SUSPENDED)) { return; } From 464400f6a5583eafb466595add435a3a33ea980f Mon Sep 17 00:00:00 2001 From: Alexey Kardashevskiy Date: Fri, 7 Mar 2014 01:33:37 +0530 Subject: [PATCH 4/4] migration: extend section_start/end traces This adds @idstr to savevm_section_start and savevm_section_end tracepoints. Signed-off-by: Alexey Kardashevskiy Signed-off-by: Amit Shah Signed-off-by: Juan Quintela --- savevm.c | 12 ++++++------ trace-events | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/savevm.c b/savevm.c index 7329fc58de..d094fbb854 100644 --- a/savevm.c +++ b/savevm.c @@ -527,13 +527,13 @@ int qemu_savevm_state_iterate(QEMUFile *f) if (qemu_file_rate_limit(f)) { return 0; } - trace_savevm_section_start(); + trace_savevm_section_start(se->idstr, se->section_id); /* Section type */ qemu_put_byte(f, QEMU_VM_SECTION_PART); qemu_put_be32(f, se->section_id); ret = se->ops->save_live_iterate(f, se->opaque); - trace_savevm_section_end(se->section_id); + trace_savevm_section_end(se->idstr, se->section_id); if (ret < 0) { qemu_file_set_error(f, ret); @@ -565,13 +565,13 @@ void qemu_savevm_state_complete(QEMUFile *f) continue; } } - trace_savevm_section_start(); + trace_savevm_section_start(se->idstr, se->section_id); /* Section type */ qemu_put_byte(f, QEMU_VM_SECTION_END); qemu_put_be32(f, se->section_id); ret = se->ops->save_live_complete(f, se->opaque); - trace_savevm_section_end(se->section_id); + trace_savevm_section_end(se->idstr, se->section_id); if (ret < 0) { qemu_file_set_error(f, ret); return; @@ -584,7 +584,7 @@ void qemu_savevm_state_complete(QEMUFile *f) if ((!se->ops || !se->ops->save_state) && !se->vmsd) { continue; } - trace_savevm_section_start(); + trace_savevm_section_start(se->idstr, se->section_id); /* Section type */ qemu_put_byte(f, QEMU_VM_SECTION_FULL); qemu_put_be32(f, se->section_id); @@ -598,7 +598,7 @@ void qemu_savevm_state_complete(QEMUFile *f) qemu_put_be32(f, se->version_id); vmstate_save(f, se); - trace_savevm_section_end(se->section_id); + trace_savevm_section_end(se->idstr, se->section_id); } qemu_put_byte(f, QEMU_VM_EOF); diff --git a/trace-events b/trace-events index 466c27efdd..002c2604d8 100644 --- a/trace-events +++ b/trace-events @@ -1040,8 +1040,8 @@ vmware_scratch_write(uint32_t index, uint32_t value) "index %d, value 0x%x" vmware_setmode(uint32_t w, uint32_t h, uint32_t bpp) "%dx%d @ %d bpp" # savevm.c -savevm_section_start(void) "" -savevm_section_end(unsigned int section_id) "section_id %u" +savevm_section_start(const char *id, unsigned int section_id) "%s, section_id %u" +savevm_section_end(const char *id, unsigned int section_id) "%s, section_id %u" # arch_init.c migration_bitmap_sync_start(void) ""