* [Qemu-devel] [PATCH 1/5] XBZRLE: Fix qemu crash when resize the xbzrle cache
2014-03-08 1:23 [Qemu-devel] [PULL 0/5] migration queue Juan Quintela
@ 2014-03-08 1:23 ` Juan Quintela
2014-03-08 1:23 ` [Qemu-devel] [PATCH 2/5] qemu_file: Fix mismerge of "use fwrite() correctly" Juan Quintela
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Juan Quintela @ 2014-03-08 1:23 UTC (permalink / raw)
To: qemu-devel; +Cc: ChenLiang, Gonglei
From: Gonglei <arei.gonglei@huawei.com>
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 <chenliang88@huawei.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
arch_init.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 49 insertions(+), 3 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index fe17279..60c975d 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);
--
1.8.5.3
^ permalink raw reply related [flat|nested] 7+ messages in thread* [Qemu-devel] [PATCH 4/5] migration: extend section_start/end traces
2014-03-08 1:23 [Qemu-devel] [PULL 0/5] migration queue Juan Quintela
` (2 preceding siblings ...)
2014-03-08 1:23 ` [Qemu-devel] [PATCH 3/5] vl: add system_wakeup_request tracepoint Juan Quintela
@ 2014-03-08 1:23 ` Juan Quintela
2014-03-08 1:23 ` [Qemu-devel] [PATCH 5/5] migration: add more traces Juan Quintela
2014-03-08 13:23 ` [Qemu-devel] [PULL 0/5] migration queue Peter Maydell
5 siblings, 0 replies; 7+ messages in thread
From: Juan Quintela @ 2014-03-08 1:23 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexey Kardashevskiy, Amit Shah
From: Alexey Kardashevskiy <aik@ozlabs.ru>
This adds @idstr to savevm_section_start and savevm_section_end
tracepoints.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
savevm.c | 12 ++++++------
trace-events | 4 ++--
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/savevm.c b/savevm.c
index 7329fc5..d094fbb 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 466c27e..002c260 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) ""
--
1.8.5.3
^ permalink raw reply related [flat|nested] 7+ messages in thread* [Qemu-devel] [PATCH 5/5] migration: add more traces
2014-03-08 1:23 [Qemu-devel] [PULL 0/5] migration queue Juan Quintela
` (3 preceding siblings ...)
2014-03-08 1:23 ` [Qemu-devel] [PATCH 4/5] migration: extend section_start/end traces Juan Quintela
@ 2014-03-08 1:23 ` Juan Quintela
2014-03-08 13:23 ` [Qemu-devel] [PULL 0/5] migration queue Peter Maydell
5 siblings, 0 replies; 7+ messages in thread
From: Juan Quintela @ 2014-03-08 1:23 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexey Kardashevskiy, Amit Shah
From: Alexey Kardashevskiy <aik@ozlabs.ru>
This replaces DPRINTF macro with tracepoints.
This moves some messages from migration.c to savevm.c.
This adds tracepoint to signal about fields failed to migrate.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration.c | 30 ++++++------------------------
qemu-file.c | 2 ++
savevm.c | 10 ++++++++++
trace-events | 16 ++++++++++++++++
vmstate.c | 2 ++
5 files changed, 36 insertions(+), 24 deletions(-)
diff --git a/migration.c b/migration.c
index 14235b2..2497c5d 100644
--- a/migration.c
+++ b/migration.c
@@ -26,16 +26,6 @@
#include "qmp-commands.h"
#include "trace.h"
-//#define DEBUG_MIGRATION
-
-#ifdef DEBUG_MIGRATION
-#define DPRINTF(fmt, ...) \
- do { printf("migration: " fmt, ## __VA_ARGS__); } while (0)
-#else
-#define DPRINTF(fmt, ...) \
- do { } while (0)
-#endif
-
enum {
MIG_STATE_ERROR = -1,
MIG_STATE_NONE,
@@ -111,7 +101,6 @@ static void process_incoming_migration_co(void *opaque)
exit(EXIT_FAILURE);
}
qemu_announce_self();
- DPRINTF("successfully loaded vm state\n");
bdrv_clear_incoming_migration_all();
/* Make sure all file formats flush their mutable metadata */
@@ -300,7 +289,7 @@ static void migrate_fd_cleanup(void *opaque)
s->cleanup_bh = NULL;
if (s->file) {
- DPRINTF("closing file\n");
+ trace_migrate_fd_cleanup();
qemu_mutex_unlock_iothread();
qemu_thread_join(&s->thread);
qemu_mutex_lock_iothread();
@@ -323,7 +312,7 @@ static void migrate_fd_cleanup(void *opaque)
void migrate_fd_error(MigrationState *s)
{
- DPRINTF("setting error state\n");
+ trace_migrate_fd_error();
assert(s->file == NULL);
s->state = MIG_STATE_ERROR;
trace_migrate_set_state(MIG_STATE_ERROR);
@@ -333,7 +322,7 @@ void migrate_fd_error(MigrationState *s)
static void migrate_fd_cancel(MigrationState *s)
{
int old_state ;
- DPRINTF("cancelling migration\n");
+ trace_migrate_fd_cancel();
do {
old_state = s->state;
@@ -583,29 +572,23 @@ static void *migration_thread(void *opaque)
int64_t start_time = initial_time;
bool old_vm_running = false;
- DPRINTF("beginning savevm\n");
qemu_savevm_state_begin(s->file, &s->params);
s->setup_time = qemu_clock_get_ms(QEMU_CLOCK_HOST) - setup_start;
migrate_set_state(s, MIG_STATE_SETUP, MIG_STATE_ACTIVE);
- DPRINTF("setup complete\n");
-
while (s->state == MIG_STATE_ACTIVE) {
int64_t current_time;
uint64_t pending_size;
if (!qemu_file_rate_limit(s->file)) {
- DPRINTF("iterate\n");
pending_size = qemu_savevm_state_pending(s->file, max_size);
- DPRINTF("pending size %" PRIu64 " max %" PRIu64 "\n",
- pending_size, max_size);
+ trace_migrate_pending(pending_size, max_size);
if (pending_size && pending_size >= max_size) {
qemu_savevm_state_iterate(s->file);
} else {
int ret;
- DPRINTF("done iterating\n");
qemu_mutex_lock_iothread();
start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
@@ -644,9 +627,8 @@ static void *migration_thread(void *opaque)
s->mbps = time_spent ? (((double) transferred_bytes * 8.0) /
((double) time_spent / 1000.0)) / 1000.0 / 1000.0 : -1;
- DPRINTF("transferred %" PRIu64 " time_spent %" PRIu64
- " bandwidth %g max_size %" PRId64 "\n",
- transferred_bytes, time_spent, bandwidth, max_size);
+ trace_migrate_transferred(transferred_bytes, time_spent,
+ bandwidth, max_size);
/* if we haven't sent anything, we don't want to recalculate
10000 is a small enough number for our purposes */
if (s->dirty_bytes_rate && transferred_bytes > 10000) {
diff --git a/qemu-file.c b/qemu-file.c
index e5ec798..8d5f45d 100644
--- a/qemu-file.c
+++ b/qemu-file.c
@@ -4,6 +4,7 @@
#include "block/coroutine.h"
#include "migration/migration.h"
#include "migration/qemu-file.h"
+#include "trace.h"
#define IO_BUF_SIZE 32768
#define MAX_IOV_SIZE MIN(IOV_MAX, 64)
@@ -595,6 +596,7 @@ int qemu_fclose(QEMUFile *f)
ret = f->last_error;
}
g_free(f);
+ trace_qemu_file_fclose();
return ret;
}
diff --git a/savevm.c b/savevm.c
index d094fbb..ef7d9ce 100644
--- a/savevm.c
+++ b/savevm.c
@@ -41,6 +41,7 @@
#include "qemu/iov.h"
#include "block/snapshot.h"
#include "block/qapi.h"
+#include <netinet/ether.h>
#define SELF_ANNOUNCE_ROUNDS 5
@@ -81,6 +82,8 @@ static void qemu_announce_self_iter(NICState *nic, void *opaque)
uint8_t buf[60];
int len;
+ trace_qemu_announce_self_iter(ether_ntoa((struct ether_addr *)
+ &nic->conf->macaddr));
len = announce_self_create(buf, nic->conf->macaddr.a);
qemu_send_packet_raw(qemu_get_queue(nic), buf, len);
@@ -429,6 +432,7 @@ void vmstate_unregister(DeviceState *dev, const VMStateDescription *vmsd,
static int vmstate_load(QEMUFile *f, SaveStateEntry *se, int version_id)
{
+ trace_vmstate_load(se->idstr, se->vmsd ? se->vmsd->name : "(old)");
if (!se->vmsd) { /* Old style */
return se->ops->load_state(f, se->opaque, version_id);
}
@@ -437,6 +441,7 @@ static int vmstate_load(QEMUFile *f, SaveStateEntry *se, int version_id)
static void vmstate_save(QEMUFile *f, SaveStateEntry *se)
{
+ trace_vmstate_save(se->idstr, se->vmsd ? se->vmsd->name : "(old)");
if (!se->vmsd) { /* Old style */
se->ops->save_state(f, se->opaque);
return;
@@ -463,6 +468,7 @@ void qemu_savevm_state_begin(QEMUFile *f,
SaveStateEntry *se;
int ret;
+ trace_savevm_state_begin();
QTAILQ_FOREACH(se, &savevm_handlers, entry) {
if (!se->ops || !se->ops->set_params) {
continue;
@@ -515,6 +521,7 @@ int qemu_savevm_state_iterate(QEMUFile *f)
SaveStateEntry *se;
int ret = 1;
+ trace_savevm_state_iterate();
QTAILQ_FOREACH(se, &savevm_handlers, entry) {
if (!se->ops || !se->ops->save_live_iterate) {
continue;
@@ -554,6 +561,8 @@ void qemu_savevm_state_complete(QEMUFile *f)
SaveStateEntry *se;
int ret;
+ trace_savevm_state_complete();
+
cpu_synchronize_all_states();
QTAILQ_FOREACH(se, &savevm_handlers, entry) {
@@ -628,6 +637,7 @@ void qemu_savevm_state_cancel(void)
{
SaveStateEntry *se;
+ trace_savevm_state_cancel();
QTAILQ_FOREACH(se, &savevm_handlers, entry) {
if (se->ops && se->ops->cancel) {
se->ops->cancel(se->opaque);
diff --git a/trace-events b/trace-events
index 002c260..657d476 100644
--- a/trace-events
+++ b/trace-events
@@ -1042,6 +1042,17 @@ vmware_setmode(uint32_t w, uint32_t h, uint32_t bpp) "%dx%d @ %d bpp"
# savevm.c
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"
+savevm_state_begin(void) ""
+savevm_state_iterate(void) ""
+savevm_state_complete(void) ""
+savevm_state_cancel(void) ""
+vmstate_save(const char *idstr, const char *vmsd_name) "%s, %s"
+vmstate_load(const char *idstr, const char *vmsd_name) "%s, %s"
+vmstate_load_field_error(const char *field, int ret) "field \"%s\" load failed, ret = %d"
+qemu_announce_self_iter(const char *mac) "%s"
+
+# qemu-file.c
+qemu_file_fclose(void) ""
# arch_init.c
migration_bitmap_sync_start(void) ""
@@ -1181,6 +1192,11 @@ flic_reset_failed(int err) "flic: reset failed %d"
# migration.c
migrate_set_state(int new_state) "new state %d"
+migrate_fd_cleanup(void) ""
+migrate_fd_error(void) ""
+migrate_fd_cancel(void) ""
+migrate_pending(uint64_t size, uint64_t max) "pending size %" PRIu64 " max %" PRIu64
+migrate_transferred(uint64_t tranferred, uint64_t time_spent, double bandwidth, uint64_t size) "transferred %" PRIu64 " time_spent %" PRIu64 " bandwidth %g max_size %" PRId64
# kvm-all.c
kvm_ioctl(int type, void *arg) "type 0x%x, arg %p"
diff --git a/vmstate.c b/vmstate.c
index d1f5eb0..b689f2f 100644
--- a/vmstate.c
+++ b/vmstate.c
@@ -3,6 +3,7 @@
#include "migration/qemu-file.h"
#include "migration/vmstate.h"
#include "qemu/bitops.h"
+#include "trace.h"
static void vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
void *opaque);
@@ -73,6 +74,7 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
}
if (ret < 0) {
+ trace_vmstate_load_field_error(field->name, ret);
return ret;
}
}
--
1.8.5.3
^ permalink raw reply related [flat|nested] 7+ messages in thread