* [Qemu-devel] [PULL 1/9] ram: Split host_from_stream_offset() into two helper functions
2016-02-05 13:56 [Qemu-devel] [PULL 0/9] migration: small fixes Amit Shah
@ 2016-02-05 13:56 ` Amit Shah
2016-02-05 13:56 ` [Qemu-devel] [PULL 2/9] migration: rename 'file' in MigrationState to 'to_dst_file' Amit Shah
` (8 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Amit Shah @ 2016-02-05 13:56 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu list, Amit Shah, zhanghailiang, Dr. David Alan Gilbert,
Juan Quintela
From: zhanghailiang <zhang.zhanghailiang@huawei.com>
Split host_from_stream_offset() into two parts:
One is to get ram block, which the block idstr may be get from migration
stream, the other is to get hva (host) address from block and the offset.
Besides, we will do the check working in a new helper offset_in_ramblock().
Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Amit Shah <amit.shah@redhat.com>
Message-Id: <1452829066-9764-2-git-send-email-zhang.zhanghailiang@huawei.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
include/exec/ram_addr.h | 8 ++++++--
migration/ram.c | 40 +++++++++++++++++++++++++---------------
2 files changed, 31 insertions(+), 17 deletions(-)
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index ef1489d..606e277 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -38,10 +38,14 @@ struct RAMBlock {
int fd;
};
+static inline bool offset_in_ramblock(RAMBlock *b, ram_addr_t offset)
+{
+ return (b && b->host && offset < b->used_length) ? true : false;
+}
+
static inline void *ramblock_ptr(RAMBlock *block, ram_addr_t offset)
{
- assert(offset < block->used_length);
- assert(block->host);
+ assert(offset_in_ramblock(block, offset));
return (char *)block->host + offset;
}
diff --git a/migration/ram.c b/migration/ram.c
index 40d0533..b541ceb 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2124,28 +2124,24 @@ static int load_xbzrle(QEMUFile *f, ram_addr_t addr, void *host)
* Returns a pointer from within the RCU-protected ram_list.
*/
/*
- * Read a RAMBlock ID from the stream f, find the host address of the
- * start of that block and add on 'offset'
+ * Read a RAMBlock ID from the stream f.
*
* f: Stream to read from
- * offset: Offset within the block
* flags: Page flags (mostly to see if it's a continuation of previous block)
*/
-static inline void *host_from_stream_offset(QEMUFile *f,
- ram_addr_t offset,
- int flags)
+static inline RAMBlock *ram_block_from_stream(QEMUFile *f,
+ int flags)
{
static RAMBlock *block = NULL;
char id[256];
uint8_t len;
if (flags & RAM_SAVE_FLAG_CONTINUE) {
- if (!block || block->max_length <= offset) {
+ if (!block) {
error_report("Ack, bad migration stream!");
return NULL;
}
-
- return block->host + offset;
+ return block;
}
len = qemu_get_byte(f);
@@ -2153,12 +2149,22 @@ static inline void *host_from_stream_offset(QEMUFile *f,
id[len] = 0;
block = qemu_ram_block_by_name(id);
- if (block && block->max_length > offset) {
- return block->host + offset;
+ if (!block) {
+ error_report("Can't find block %s", id);
+ return NULL;
}
- error_report("Can't find block %s", id);
- return NULL;
+ return block;
+}
+
+static inline void *host_from_ram_block_offset(RAMBlock *block,
+ ram_addr_t offset)
+{
+ if (!offset_in_ramblock(block, offset)) {
+ return NULL;
+ }
+
+ return block->host + offset;
}
/*
@@ -2302,7 +2308,9 @@ static int ram_load_postcopy(QEMUFile *f)
trace_ram_load_postcopy_loop((uint64_t)addr, flags);
place_needed = false;
if (flags & (RAM_SAVE_FLAG_COMPRESS | RAM_SAVE_FLAG_PAGE)) {
- host = host_from_stream_offset(f, addr, flags);
+ RAMBlock *block = ram_block_from_stream(f, flags);
+
+ host = host_from_ram_block_offset(block, addr);
if (!host) {
error_report("Illegal RAM offset " RAM_ADDR_FMT, addr);
ret = -EINVAL;
@@ -2433,7 +2441,9 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
if (flags & (RAM_SAVE_FLAG_COMPRESS | RAM_SAVE_FLAG_PAGE |
RAM_SAVE_FLAG_COMPRESS_PAGE | RAM_SAVE_FLAG_XBZRLE)) {
- host = host_from_stream_offset(f, addr, flags);
+ RAMBlock *block = ram_block_from_stream(f, flags);
+
+ host = host_from_ram_block_offset(block, addr);
if (!host) {
error_report("Illegal RAM offset " RAM_ADDR_FMT, addr);
ret = -EINVAL;
--
2.5.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PULL 2/9] migration: rename 'file' in MigrationState to 'to_dst_file'
2016-02-05 13:56 [Qemu-devel] [PULL 0/9] migration: small fixes Amit Shah
2016-02-05 13:56 ` [Qemu-devel] [PULL 1/9] ram: Split host_from_stream_offset() into two helper functions Amit Shah
@ 2016-02-05 13:56 ` Amit Shah
2016-02-05 13:56 ` [Qemu-devel] [PULL 3/9] savevm: Split load vm state function qemu_loadvm_state Amit Shah
` (7 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Amit Shah @ 2016-02-05 13:56 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu list, Amit Shah, zhanghailiang, Dr. David Alan Gilbert,
Juan Quintela
From: zhanghailiang <zhang.zhanghailiang@huawei.com>
Rename the 'file' member of MigrationState to 'to_dst_file' to
be consistent with to_src_file, from_src_file and from_dst_file.
Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Amit Shah <amit.shah@redhat.com>
Message-Id: <1452829066-9764-3-git-send-email-zhang.zhanghailiang@huawei.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
include/migration/migration.h | 2 +-
migration/exec.c | 4 +--
migration/fd.c | 4 +--
migration/migration.c | 72 ++++++++++++++++++++++---------------------
migration/postcopy-ram.c | 6 ++--
migration/rdma.c | 2 +-
migration/savevm.c | 2 +-
migration/tcp.c | 4 +--
migration/unix.c | 4 +--
9 files changed, 52 insertions(+), 48 deletions(-)
diff --git a/include/migration/migration.h b/include/migration/migration.h
index 0fc1ffa..74684ad 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -133,7 +133,7 @@ struct MigrationState
size_t xfer_limit;
QemuThread thread;
QEMUBH *cleanup_bh;
- QEMUFile *file;
+ QEMUFile *to_dst_file;
int parameters[MIGRATION_PARAMETER__MAX];
int state;
diff --git a/migration/exec.c b/migration/exec.c
index f1b5e47..62f892d 100644
--- a/migration/exec.c
+++ b/migration/exec.c
@@ -36,8 +36,8 @@
void exec_start_outgoing_migration(MigrationState *s, const char *command, Error **errp)
{
- s->file = qemu_popen_cmd(command, "w");
- if (s->file == NULL) {
+ s->to_dst_file = qemu_popen_cmd(command, "w");
+ if (s->to_dst_file == NULL) {
error_setg_errno(errp, errno, "failed to popen the migration target");
return;
}
diff --git a/migration/fd.c b/migration/fd.c
index c0afc96..085dd7c 100644
--- a/migration/fd.c
+++ b/migration/fd.c
@@ -51,9 +51,9 @@ void fd_start_outgoing_migration(MigrationState *s, const char *fdname, Error **
}
if (fd_is_socket(fd)) {
- s->file = qemu_fopen_socket(fd, "wb");
+ s->to_dst_file = qemu_fopen_socket(fd, "wb");
} else {
- s->file = qemu_fdopen(fd, "wb");
+ s->to_dst_file = qemu_fdopen(fd, "wb");
}
migrate_fd_connect(s);
diff --git a/migration/migration.c b/migration/migration.c
index 2c76998..4971722 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -809,7 +809,7 @@ static void migrate_fd_cleanup(void *opaque)
flush_page_queue(s);
- if (s->file) {
+ if (s->to_dst_file) {
trace_migrate_fd_cleanup();
qemu_mutex_unlock_iothread();
if (s->migration_thread_running) {
@@ -819,8 +819,8 @@ static void migrate_fd_cleanup(void *opaque)
qemu_mutex_lock_iothread();
migrate_compress_threads_join();
- qemu_fclose(s->file);
- s->file = NULL;
+ qemu_fclose(s->to_dst_file);
+ s->to_dst_file = NULL;
}
assert((s->state != MIGRATION_STATUS_ACTIVE) &&
@@ -837,7 +837,7 @@ static void migrate_fd_cleanup(void *opaque)
void migrate_fd_error(MigrationState *s)
{
trace_migrate_fd_error();
- assert(s->file == NULL);
+ assert(s->to_dst_file == NULL);
migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
MIGRATION_STATUS_FAILED);
notifier_list_notify(&migration_state_notifiers, s);
@@ -846,7 +846,7 @@ void migrate_fd_error(MigrationState *s)
static void migrate_fd_cancel(MigrationState *s)
{
int old_state ;
- QEMUFile *f = migrate_get_current()->file;
+ QEMUFile *f = migrate_get_current()->to_dst_file;
trace_migrate_fd_cancel();
if (s->rp_state.from_dst_file) {
@@ -917,7 +917,7 @@ MigrationState *migrate_init(const MigrationParams *params)
s->bytes_xfer = 0;
s->xfer_limit = 0;
s->cleanup_bh = 0;
- s->file = NULL;
+ s->to_dst_file = NULL;
s->state = MIGRATION_STATUS_NONE;
s->params = *params;
s->rp_state.from_dst_file = NULL;
@@ -1096,8 +1096,9 @@ void qmp_migrate_set_speed(int64_t value, Error **errp)
s = migrate_get_current();
s->bandwidth_limit = value;
- if (s->file) {
- qemu_file_set_rate_limit(s->file, s->bandwidth_limit / XFER_LIMIT_RATIO);
+ if (s->to_dst_file) {
+ qemu_file_set_rate_limit(s->to_dst_file,
+ s->bandwidth_limit / XFER_LIMIT_RATIO);
}
}
@@ -1367,7 +1368,7 @@ out:
static int open_return_path_on_source(MigrationState *ms)
{
- ms->rp_state.from_dst_file = qemu_file_get_return_path(ms->file);
+ ms->rp_state.from_dst_file = qemu_file_get_return_path(ms->to_dst_file);
if (!ms->rp_state.from_dst_file) {
return -1;
}
@@ -1389,7 +1390,7 @@ static int await_return_path_close_on_source(MigrationState *ms)
* rp_thread will exit, however if there's an error we need to cause
* it to exit.
*/
- if (qemu_file_get_error(ms->file) && ms->rp_state.from_dst_file) {
+ if (qemu_file_get_error(ms->to_dst_file) && ms->rp_state.from_dst_file) {
/*
* shutdown(2), if we have it, will cause it to unblock if it's stuck
* waiting for the destination.
@@ -1436,7 +1437,7 @@ static int postcopy_start(MigrationState *ms, bool *old_vm_running)
* Cause any non-postcopiable, but iterative devices to
* send out their final data.
*/
- qemu_savevm_state_complete_precopy(ms->file, true);
+ qemu_savevm_state_complete_precopy(ms->to_dst_file, true);
/*
* in Finish migrate and with the io-lock held everything should
@@ -1454,9 +1455,9 @@ static int postcopy_start(MigrationState *ms, bool *old_vm_running)
* will notice we're in POSTCOPY_ACTIVE and not actually
* wrap their state up here
*/
- qemu_file_set_rate_limit(ms->file, INT64_MAX);
+ qemu_file_set_rate_limit(ms->to_dst_file, INT64_MAX);
/* Ping just for debugging, helps line traces up */
- qemu_savevm_send_ping(ms->file, 2);
+ qemu_savevm_send_ping(ms->to_dst_file, 2);
/*
* While loading the device state we may trigger page transfer
@@ -1490,7 +1491,7 @@ static int postcopy_start(MigrationState *ms, bool *old_vm_running)
qsb = qemu_buf_get(fb);
/* Now send that blob */
- if (qemu_savevm_send_packaged(ms->file, qsb)) {
+ if (qemu_savevm_send_packaged(ms->to_dst_file, qsb)) {
goto fail_closefb;
}
qemu_fclose(fb);
@@ -1502,9 +1503,9 @@ static int postcopy_start(MigrationState *ms, bool *old_vm_running)
* Although this ping is just for debug, it could potentially be
* used for getting a better measurement of downtime at the source.
*/
- qemu_savevm_send_ping(ms->file, 4);
+ qemu_savevm_send_ping(ms->to_dst_file, 4);
- ret = qemu_file_get_error(ms->file);
+ ret = qemu_file_get_error(ms->to_dst_file);
if (ret) {
error_report("postcopy_start: Migration stream errored");
migrate_set_state(&ms->state, MIGRATION_STATUS_POSTCOPY_ACTIVE,
@@ -1550,8 +1551,8 @@ static void migration_completion(MigrationState *s, int current_active_state,
ret = bdrv_inactivate_all();
}
if (ret >= 0) {
- qemu_file_set_rate_limit(s->file, INT64_MAX);
- qemu_savevm_state_complete_precopy(s->file, false);
+ qemu_file_set_rate_limit(s->to_dst_file, INT64_MAX);
+ qemu_savevm_state_complete_precopy(s->to_dst_file, false);
}
}
qemu_mutex_unlock_iothread();
@@ -1562,7 +1563,7 @@ static void migration_completion(MigrationState *s, int current_active_state,
} else if (s->state == MIGRATION_STATUS_POSTCOPY_ACTIVE) {
trace_migration_completion_postcopy_end();
- qemu_savevm_state_complete_postcopy(s->file);
+ qemu_savevm_state_complete_postcopy(s->to_dst_file);
trace_migration_completion_postcopy_end_after_complete();
}
@@ -1583,7 +1584,7 @@ static void migration_completion(MigrationState *s, int current_active_state,
}
}
- if (qemu_file_get_error(s->file)) {
+ if (qemu_file_get_error(s->to_dst_file)) {
trace_migration_completion_file_err();
goto fail;
}
@@ -1618,24 +1619,24 @@ static void *migration_thread(void *opaque)
rcu_register_thread();
- qemu_savevm_state_header(s->file);
+ qemu_savevm_state_header(s->to_dst_file);
if (migrate_postcopy_ram()) {
/* Now tell the dest that it should open its end so it can reply */
- qemu_savevm_send_open_return_path(s->file);
+ qemu_savevm_send_open_return_path(s->to_dst_file);
/* And do a ping that will make stuff easier to debug */
- qemu_savevm_send_ping(s->file, 1);
+ qemu_savevm_send_ping(s->to_dst_file, 1);
/*
* Tell the destination that we *might* want to do postcopy later;
* if the other end can't do postcopy it should fail now, nice and
* early.
*/
- qemu_savevm_send_postcopy_advise(s->file);
+ qemu_savevm_send_postcopy_advise(s->to_dst_file);
}
- qemu_savevm_state_begin(s->file, &s->params);
+ qemu_savevm_state_begin(s->to_dst_file, &s->params);
s->setup_time = qemu_clock_get_ms(QEMU_CLOCK_HOST) - setup_start;
current_active_state = MIGRATION_STATUS_ACTIVE;
@@ -1649,10 +1650,10 @@ static void *migration_thread(void *opaque)
int64_t current_time;
uint64_t pending_size;
- if (!qemu_file_rate_limit(s->file)) {
+ if (!qemu_file_rate_limit(s->to_dst_file)) {
uint64_t pend_post, pend_nonpost;
- qemu_savevm_state_pending(s->file, max_size, &pend_nonpost,
+ qemu_savevm_state_pending(s->to_dst_file, max_size, &pend_nonpost,
&pend_post);
pending_size = pend_nonpost + pend_post;
trace_migrate_pending(pending_size, max_size,
@@ -1673,7 +1674,7 @@ static void *migration_thread(void *opaque)
continue;
}
/* Just another iteration step */
- qemu_savevm_state_iterate(s->file, entered_postcopy);
+ qemu_savevm_state_iterate(s->to_dst_file, entered_postcopy);
} else {
trace_migration_thread_low_pending(pending_size);
migration_completion(s, current_active_state,
@@ -1682,7 +1683,7 @@ static void *migration_thread(void *opaque)
}
}
- if (qemu_file_get_error(s->file)) {
+ if (qemu_file_get_error(s->to_dst_file)) {
migrate_set_state(&s->state, current_active_state,
MIGRATION_STATUS_FAILED);
trace_migration_thread_file_err();
@@ -1690,7 +1691,8 @@ static void *migration_thread(void *opaque)
}
current_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
if (current_time >= initial_time + BUFFER_DELAY) {
- uint64_t transferred_bytes = qemu_ftell(s->file) - initial_bytes;
+ uint64_t transferred_bytes = qemu_ftell(s->to_dst_file) -
+ initial_bytes;
uint64_t time_spent = current_time - initial_time;
double bandwidth = (double)transferred_bytes / time_spent;
max_size = bandwidth * migrate_max_downtime() / 1000000;
@@ -1706,11 +1708,11 @@ static void *migration_thread(void *opaque)
s->expected_downtime = s->dirty_bytes_rate / bandwidth;
}
- qemu_file_reset_rate_limit(s->file);
+ qemu_file_reset_rate_limit(s->to_dst_file);
initial_time = current_time;
- initial_bytes = qemu_ftell(s->file);
+ initial_bytes = qemu_ftell(s->to_dst_file);
}
- if (qemu_file_rate_limit(s->file)) {
+ if (qemu_file_rate_limit(s->to_dst_file)) {
/* usleep expects microseconds */
g_usleep((initial_time + BUFFER_DELAY - current_time)*1000);
}
@@ -1724,7 +1726,7 @@ static void *migration_thread(void *opaque)
qemu_mutex_lock_iothread();
qemu_savevm_state_cleanup();
if (s->state == MIGRATION_STATUS_COMPLETED) {
- uint64_t transferred_bytes = qemu_ftell(s->file);
+ uint64_t transferred_bytes = qemu_ftell(s->to_dst_file);
s->total_time = end_time - s->total_time;
if (!entered_postcopy) {
s->downtime = end_time - start_time;
@@ -1752,7 +1754,7 @@ void migrate_fd_connect(MigrationState *s)
s->expected_downtime = max_downtime/1000000;
s->cleanup_bh = qemu_bh_new(migrate_fd_cleanup, s);
- qemu_file_set_rate_limit(s->file,
+ qemu_file_set_rate_limit(s->to_dst_file,
s->bandwidth_limit / XFER_LIMIT_RATIO);
/* Notify before starting migration thread */
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 02b28ed..254c629 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -725,7 +725,8 @@ void postcopy_discard_send_range(MigrationState *ms, PostcopyDiscardState *pds,
if (pds->cur_entry == MAX_DISCARDS_PER_COMMAND) {
/* Full set, ship it! */
- qemu_savevm_send_postcopy_ram_discard(ms->file, pds->ramblock_name,
+ qemu_savevm_send_postcopy_ram_discard(ms->to_dst_file,
+ pds->ramblock_name,
pds->cur_entry,
pds->start_list,
pds->length_list);
@@ -745,7 +746,8 @@ void postcopy_discard_send_finish(MigrationState *ms, PostcopyDiscardState *pds)
{
/* Anything unsent? */
if (pds->cur_entry) {
- qemu_savevm_send_postcopy_ram_discard(ms->file, pds->ramblock_name,
+ qemu_savevm_send_postcopy_ram_discard(ms->to_dst_file,
+ pds->ramblock_name,
pds->cur_entry,
pds->start_list,
pds->length_list);
diff --git a/migration/rdma.c b/migration/rdma.c
index 86a13b8..bcae1e8 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -3504,7 +3504,7 @@ void rdma_start_outgoing_migration(void *opaque,
trace_rdma_start_outgoing_migration_after_rdma_connect();
- s->file = qemu_fopen_rdma(rdma, "wb");
+ s->to_dst_file = qemu_fopen_rdma(rdma, "wb");
migrate_fd_connect(s);
return;
err:
diff --git a/migration/savevm.c b/migration/savevm.c
index 954988d..8a2cddc 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1163,7 +1163,7 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp)
.shared = 0
};
MigrationState *ms = migrate_init(¶ms);
- ms->file = f;
+ ms->to_dst_file = f;
if (qemu_savevm_state_blocked(errp)) {
return -EINVAL;
diff --git a/migration/tcp.c b/migration/tcp.c
index ae80e94..e888a4e 100644
--- a/migration/tcp.c
+++ b/migration/tcp.c
@@ -39,11 +39,11 @@ static void tcp_wait_for_connect(int fd, Error *err, void *opaque)
if (fd < 0) {
DPRINTF("migrate connect error: %s\n", error_get_pretty(err));
- s->file = NULL;
+ s->to_dst_file = NULL;
migrate_fd_error(s);
} else {
DPRINTF("migrate connect success\n");
- s->file = qemu_fopen_socket(fd, "wb");
+ s->to_dst_file = qemu_fopen_socket(fd, "wb");
migrate_fd_connect(s);
}
}
diff --git a/migration/unix.c b/migration/unix.c
index a721c4a..d9aac36 100644
--- a/migration/unix.c
+++ b/migration/unix.c
@@ -39,11 +39,11 @@ static void unix_wait_for_connect(int fd, Error *err, void *opaque)
if (fd < 0) {
DPRINTF("migrate connect error: %s\n", error_get_pretty(err));
- s->file = NULL;
+ s->to_dst_file = NULL;
migrate_fd_error(s);
} else {
DPRINTF("migrate connect success\n");
- s->file = qemu_fopen_socket(fd, "wb");
+ s->to_dst_file = qemu_fopen_socket(fd, "wb");
migrate_fd_connect(s);
}
}
--
2.5.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PULL 3/9] savevm: Split load vm state function qemu_loadvm_state
2016-02-05 13:56 [Qemu-devel] [PULL 0/9] migration: small fixes Amit Shah
2016-02-05 13:56 ` [Qemu-devel] [PULL 1/9] ram: Split host_from_stream_offset() into two helper functions Amit Shah
2016-02-05 13:56 ` [Qemu-devel] [PULL 2/9] migration: rename 'file' in MigrationState to 'to_dst_file' Amit Shah
@ 2016-02-05 13:56 ` Amit Shah
2016-02-05 13:56 ` [Qemu-devel] [PULL 4/9] migration/ram: Fix some helper functions' parameter to use PageSearchStatus Amit Shah
` (6 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Amit Shah @ 2016-02-05 13:56 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu list, Amit Shah, zhanghailiang, Dr. David Alan Gilbert,
Juan Quintela
From: zhanghailiang <zhang.zhanghailiang@huawei.com>
qemu_loadvm_state is too long, and we can simplify it by splitting up
with three helper functions.
Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Amit Shah <amit.shah@redhat.com>
Message-Id: <1452829066-9764-4-git-send-email-zhang.zhanghailiang@huawei.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
migration/savevm.c | 156 +++++++++++++++++++++++++++++++----------------------
1 file changed, 92 insertions(+), 64 deletions(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index 8a2cddc..00be5fe 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1718,90 +1718,118 @@ void loadvm_free_handlers(MigrationIncomingState *mis)
}
}
+static int
+qemu_loadvm_section_start_full(QEMUFile *f, MigrationIncomingState *mis)
+{
+ uint32_t instance_id, version_id, section_id;
+ SaveStateEntry *se;
+ LoadStateEntry *le;
+ char idstr[256];
+ int ret;
+
+ /* Read section start */
+ section_id = qemu_get_be32(f);
+ if (!qemu_get_counted_string(f, idstr)) {
+ error_report("Unable to read ID string for section %u",
+ section_id);
+ return -EINVAL;
+ }
+ instance_id = qemu_get_be32(f);
+ version_id = qemu_get_be32(f);
+
+ trace_qemu_loadvm_state_section_startfull(section_id, idstr,
+ instance_id, version_id);
+ /* Find savevm section */
+ se = find_se(idstr, instance_id);
+ if (se == NULL) {
+ error_report("Unknown savevm section or instance '%s' %d",
+ idstr, instance_id);
+ return -EINVAL;
+ }
+
+ /* Validate version */
+ if (version_id > se->version_id) {
+ error_report("savevm: unsupported version %d for '%s' v%d",
+ version_id, idstr, se->version_id);
+ return -EINVAL;
+ }
+
+ /* Add entry */
+ le = g_malloc0(sizeof(*le));
+
+ le->se = se;
+ le->section_id = section_id;
+ le->version_id = version_id;
+ QLIST_INSERT_HEAD(&mis->loadvm_handlers, le, entry);
+
+ ret = vmstate_load(f, le->se, le->version_id);
+ if (ret < 0) {
+ error_report("error while loading state for instance 0x%x of"
+ " device '%s'", instance_id, idstr);
+ return ret;
+ }
+ if (!check_section_footer(f, le)) {
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int
+qemu_loadvm_section_part_end(QEMUFile *f, MigrationIncomingState *mis)
+{
+ uint32_t section_id;
+ LoadStateEntry *le;
+ int ret;
+
+ section_id = qemu_get_be32(f);
+
+ trace_qemu_loadvm_state_section_partend(section_id);
+ QLIST_FOREACH(le, &mis->loadvm_handlers, entry) {
+ if (le->section_id == section_id) {
+ break;
+ }
+ }
+ if (le == NULL) {
+ error_report("Unknown savevm section %d", section_id);
+ return -EINVAL;
+ }
+
+ ret = vmstate_load(f, le->se, le->version_id);
+ if (ret < 0) {
+ error_report("error while loading state section id %d(%s)",
+ section_id, le->se->idstr);
+ return ret;
+ }
+ if (!check_section_footer(f, le)) {
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis)
{
uint8_t section_type;
int ret;
while ((section_type = qemu_get_byte(f)) != QEMU_VM_EOF) {
- uint32_t instance_id, version_id, section_id;
- SaveStateEntry *se;
- LoadStateEntry *le;
- char idstr[256];
trace_qemu_loadvm_state_section(section_type);
switch (section_type) {
case QEMU_VM_SECTION_START:
case QEMU_VM_SECTION_FULL:
- /* Read section start */
- section_id = qemu_get_be32(f);
- if (!qemu_get_counted_string(f, idstr)) {
- error_report("Unable to read ID string for section %u",
- section_id);
- return -EINVAL;
- }
- instance_id = qemu_get_be32(f);
- version_id = qemu_get_be32(f);
-
- trace_qemu_loadvm_state_section_startfull(section_id, idstr,
- instance_id, version_id);
- /* Find savevm section */
- se = find_se(idstr, instance_id);
- if (se == NULL) {
- error_report("Unknown savevm section or instance '%s' %d",
- idstr, instance_id);
- return -EINVAL;
- }
-
- /* Validate version */
- if (version_id > se->version_id) {
- error_report("savevm: unsupported version %d for '%s' v%d",
- version_id, idstr, se->version_id);
- return -EINVAL;
- }
-
- /* Add entry */
- le = g_malloc0(sizeof(*le));
-
- le->se = se;
- le->section_id = section_id;
- le->version_id = version_id;
- QLIST_INSERT_HEAD(&mis->loadvm_handlers, le, entry);
-
- ret = vmstate_load(f, le->se, le->version_id);
+ ret = qemu_loadvm_section_start_full(f, mis);
if (ret < 0) {
- error_report("error while loading state for instance 0x%x of"
- " device '%s'", instance_id, idstr);
return ret;
}
- if (!check_section_footer(f, le)) {
- return -EINVAL;
- }
break;
case QEMU_VM_SECTION_PART:
case QEMU_VM_SECTION_END:
- section_id = qemu_get_be32(f);
-
- trace_qemu_loadvm_state_section_partend(section_id);
- QLIST_FOREACH(le, &mis->loadvm_handlers, entry) {
- if (le->section_id == section_id) {
- break;
- }
- }
- if (le == NULL) {
- error_report("Unknown savevm section %d", section_id);
- return -EINVAL;
- }
-
- ret = vmstate_load(f, le->se, le->version_id);
+ ret = qemu_loadvm_section_part_end(f, mis);
if (ret < 0) {
- error_report("error while loading state section id %d(%s)",
- section_id, le->se->idstr);
return ret;
}
- if (!check_section_footer(f, le)) {
- return -EINVAL;
- }
break;
case QEMU_VM_COMMAND:
ret = loadvm_process_command(f);
--
2.5.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PULL 4/9] migration/ram: Fix some helper functions' parameter to use PageSearchStatus
2016-02-05 13:56 [Qemu-devel] [PULL 0/9] migration: small fixes Amit Shah
` (2 preceding siblings ...)
2016-02-05 13:56 ` [Qemu-devel] [PULL 3/9] savevm: Split load vm state function qemu_loadvm_state Amit Shah
@ 2016-02-05 13:56 ` Amit Shah
2016-02-05 13:56 ` [Qemu-devel] [PULL 5/9] qmp-commands.hx: Fix the missing options for migration parameters commands Amit Shah
` (5 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Amit Shah @ 2016-02-05 13:56 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu list, Amit Shah, zhanghailiang, Dr. David Alan Gilbert,
Juan Quintela
From: zhanghailiang <zhang.zhanghailiang@huawei.com>
Some helper functions use parameters 'RAMBlock *block' and 'ram_addr_t *offset',
We can use 'PageSearchStatus *pss' directly instead, with this change, we
can reduce the number of parameters for these helper function, also
it is easily to add new parameters for these helper functions.
Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Amit Shah <amit.shah@redhat.com>
Message-Id: <1452829066-9764-5-git-send-email-zhang.zhanghailiang@huawei.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
migration/ram.c | 33 +++++++++++++++++++--------------
1 file changed, 19 insertions(+), 14 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index b541ceb..3cdfea4 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -729,7 +729,7 @@ static int save_zero_page(QEMUFile *f, RAMBlock *block, ram_addr_t offset,
* @last_stage: if we are at the completion stage
* @bytes_transferred: increase it with the number of transferred bytes
*/
-static int ram_save_page(QEMUFile *f, RAMBlock* block, ram_addr_t offset,
+static int ram_save_page(QEMUFile *f, PageSearchStatus *pss,
bool last_stage, uint64_t *bytes_transferred)
{
int pages = -1;
@@ -738,6 +738,8 @@ static int ram_save_page(QEMUFile *f, RAMBlock* block, ram_addr_t offset,
uint8_t *p;
int ret;
bool send_async = true;
+ RAMBlock *block = pss->block;
+ ram_addr_t offset = pss->offset;
p = block->host + offset;
@@ -912,14 +914,16 @@ static int compress_page_with_multi_thread(QEMUFile *f, RAMBlock *block,
* @last_stage: if we are at the completion stage
* @bytes_transferred: increase it with the number of transferred bytes
*/
-static int ram_save_compressed_page(QEMUFile *f, RAMBlock *block,
- ram_addr_t offset, bool last_stage,
+static int ram_save_compressed_page(QEMUFile *f, PageSearchStatus *pss,
+ bool last_stage,
uint64_t *bytes_transferred)
{
int pages = -1;
uint64_t bytes_xmit;
uint8_t *p;
int ret;
+ RAMBlock *block = pss->block;
+ ram_addr_t offset = pss->offset;
p = block->host + offset;
@@ -1229,7 +1233,7 @@ err:
* Returns: Number of pages written.
*/
static int ram_save_target_page(MigrationState *ms, QEMUFile *f,
- RAMBlock *block, ram_addr_t offset,
+ PageSearchStatus *pss,
bool last_stage,
uint64_t *bytes_transferred,
ram_addr_t dirty_ram_abs)
@@ -1240,11 +1244,11 @@ static int ram_save_target_page(MigrationState *ms, QEMUFile *f,
if (migration_bitmap_clear_dirty(dirty_ram_abs)) {
unsigned long *unsentmap;
if (compression_switch && migrate_use_compression()) {
- res = ram_save_compressed_page(f, block, offset,
+ res = ram_save_compressed_page(f, pss,
last_stage,
bytes_transferred);
} else {
- res = ram_save_page(f, block, offset, last_stage,
+ res = ram_save_page(f, pss, last_stage,
bytes_transferred);
}
@@ -1260,7 +1264,7 @@ static int ram_save_target_page(MigrationState *ms, QEMUFile *f,
* to the stream.
*/
if (res > 0) {
- last_sent_block = block;
+ last_sent_block = pss->block;
}
}
@@ -1284,26 +1288,27 @@ static int ram_save_target_page(MigrationState *ms, QEMUFile *f,
* @bytes_transferred: increase it with the number of transferred bytes
* @dirty_ram_abs: Address of the start of the dirty page in ram_addr_t space
*/
-static int ram_save_host_page(MigrationState *ms, QEMUFile *f, RAMBlock *block,
- ram_addr_t *offset, bool last_stage,
+static int ram_save_host_page(MigrationState *ms, QEMUFile *f,
+ PageSearchStatus *pss,
+ bool last_stage,
uint64_t *bytes_transferred,
ram_addr_t dirty_ram_abs)
{
int tmppages, pages = 0;
do {
- tmppages = ram_save_target_page(ms, f, block, *offset, last_stage,
+ tmppages = ram_save_target_page(ms, f, pss, last_stage,
bytes_transferred, dirty_ram_abs);
if (tmppages < 0) {
return tmppages;
}
pages += tmppages;
- *offset += TARGET_PAGE_SIZE;
+ pss->offset += TARGET_PAGE_SIZE;
dirty_ram_abs += TARGET_PAGE_SIZE;
- } while (*offset & (qemu_host_page_size - 1));
+ } while (pss->offset & (qemu_host_page_size - 1));
/* The offset we leave with is the last one we looked at */
- *offset -= TARGET_PAGE_SIZE;
+ pss->offset -= TARGET_PAGE_SIZE;
return pages;
}
@@ -1351,7 +1356,7 @@ static int ram_find_and_save_block(QEMUFile *f, bool last_stage,
}
if (found) {
- pages = ram_save_host_page(ms, f, pss.block, &pss.offset,
+ pages = ram_save_host_page(ms, f, &pss,
last_stage, bytes_transferred,
dirty_ram_abs);
}
--
2.5.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PULL 5/9] qmp-commands.hx: Fix the missing options for migration parameters commands
2016-02-05 13:56 [Qemu-devel] [PULL 0/9] migration: small fixes Amit Shah
` (3 preceding siblings ...)
2016-02-05 13:56 ` [Qemu-devel] [PULL 4/9] migration/ram: Fix some helper functions' parameter to use PageSearchStatus Amit Shah
@ 2016-02-05 13:56 ` Amit Shah
2016-02-05 13:56 ` [Qemu-devel] [PULL 6/9] qmp-commands.hx: Document the missing options for migration capability commands Amit Shah
` (4 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Amit Shah @ 2016-02-05 13:56 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu list, Amit Shah, zhanghailiang, Dr. David Alan Gilbert,
Juan Quintela
From: zhanghailiang <zhang.zhanghailiang@huawei.com>
We didn't document x-cpu-throttle-initial/x-cpu-throttle-increment for
commands migrate-set-parameters and query-migrate-parameters.
Here we add the descriptions for these two options and fix the wrong example
for query-migrate-parameters qmp commands.
Besides, this will also fix the bug that we can't set x-cpu-throttle-initial
and x-cpu-throttle-increment through migrate-set-parameters qmp command.
Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Amit Shah <amit.shah@redhat.com>
Message-Id: <1452829066-9764-6-git-send-email-zhang.zhanghailiang@huawei.com>
[Amit: fix typo in 'auto-converge']
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
qmp-commands.hx | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/qmp-commands.hx b/qmp-commands.hx
index db072a6..b25b97c 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3700,6 +3700,10 @@ Set migration parameters
- "compress-level": set compression level during migration (json-int)
- "compress-threads": set compression thread count for migration (json-int)
- "decompress-threads": set decompression thread count for migration (json-int)
+- "x-cpu-throttle-initial": set initial percentage of time guest cpus are
+ throttled for auto-converge (json-int)
+- "x-cpu-throttle-increment": set throttle increasing percentage for
+ auto-converge (json-int)
Arguments:
@@ -3713,7 +3717,7 @@ EQMP
{
.name = "migrate-set-parameters",
.args_type =
- "compress-level:i?,compress-threads:i?,decompress-threads:i?",
+ "compress-level:i?,compress-threads:i?,decompress-threads:i?,x-cpu-throttle-initial:i?,x-cpu-throttle-increment:i?",
.mhandler.cmd_new = qmp_marshal_migrate_set_parameters,
},
SQMP
@@ -3726,6 +3730,10 @@ Query current migration parameters
- "compress-level" : compression level value (json-int)
- "compress-threads" : compression thread count value (json-int)
- "decompress-threads" : decompression thread count value (json-int)
+ - "x-cpu-throttle-initial" : initial percentage of time guest cpus are
+ throttled (json-int)
+ - "x-cpu-throttle-increment" : throttle increasing percentage for
+ auto-converge (json-int)
Arguments:
@@ -3734,9 +3742,11 @@ Example:
-> { "execute": "query-migrate-parameters" }
<- {
"return": {
- "decompress-threads", 2,
- "compress-threads", 8,
- "compress-level", 1
+ "decompress-threads": 2,
+ "x-cpu-throttle-increment": 10,
+ "compress-threads": 8,
+ "compress-level": 1,
+ "x-cpu-throttle-initial": 20
}
}
--
2.5.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PULL 6/9] qmp-commands.hx: Document the missing options for migration capability commands
2016-02-05 13:56 [Qemu-devel] [PULL 0/9] migration: small fixes Amit Shah
` (4 preceding siblings ...)
2016-02-05 13:56 ` [Qemu-devel] [PULL 5/9] qmp-commands.hx: Fix the missing options for migration parameters commands Amit Shah
@ 2016-02-05 13:56 ` Amit Shah
2016-02-05 13:56 ` [Qemu-devel] [PULL 7/9] migration: remove useless code Amit Shah
` (3 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Amit Shah @ 2016-02-05 13:56 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu list, Amit Shah, zhanghailiang, Dr. David Alan Gilbert,
Juan Quintela
From: zhanghailiang <zhang.zhanghailiang@huawei.com>
Add the missing descriptions for the options of migration capability commands,
and fix the example for query-migrate-capabilities command.
Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Amit Shah <amit.shah@redhat.com>
Message-Id: <1452829066-9764-7-git-send-email-zhang.zhanghailiang@huawei.com>
[Amit: Strip whitespace]
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
qmp-commands.hx | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/qmp-commands.hx b/qmp-commands.hx
index b25b97c..020e5ee 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3647,7 +3647,9 @@ Enable/Disable migration capabilities
- "rdma-pin-all": pin all pages when using RDMA during migration
- "auto-converge": throttle down guest to help convergence of migration
- "zero-blocks": compress zero blocks during block migration
+- "compress": use multiple compression threads to accelerate live migration
- "events": generate events for each migration state change
+- "x-postcopy-ram": postcopy mode for live migration
Arguments:
@@ -3675,13 +3677,24 @@ Query current migration capabilities
- "rdma-pin-all" : RDMA Pin Page state (json-bool)
- "auto-converge" : Auto Converge state (json-bool)
- "zero-blocks" : Zero Blocks state (json-bool)
+ - "compress": Multiple compression threads state (json-bool)
+ - "events": Migration state change event state (json-bool)
+ - "x-postcopy-ram": postcopy ram state (json-bool)
Arguments:
Example:
-> { "execute": "query-migrate-capabilities" }
-<- { "return": [ { "state": false, "capability": "xbzrle" } ] }
+<- {"return": [
+ {"state": false, "capability": "xbzrle"},
+ {"state": false, "capability": "rdma-pin-all"},
+ {"state": false, "capability": "auto-converge"},
+ {"state": false, "capability": "zero-blocks"},
+ {"state": false, "capability": "compress"},
+ {"state": true, "capability": "events"},
+ {"state": false, "capability": "x-postcopy-ram"}
+ ]}
EQMP
--
2.5.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PULL 7/9] migration: remove useless code.
2016-02-05 13:56 [Qemu-devel] [PULL 0/9] migration: small fixes Amit Shah
` (5 preceding siblings ...)
2016-02-05 13:56 ` [Qemu-devel] [PULL 6/9] qmp-commands.hx: Document the missing options for migration capability commands Amit Shah
@ 2016-02-05 13:56 ` Amit Shah
2016-02-05 13:56 ` [Qemu-devel] [PULL 8/9] static checker: e1000-82540em got aliased to e1000 Amit Shah
` (2 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Amit Shah @ 2016-02-05 13:56 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu list, Amit Shah, Liang Li, Dr. David Alan Gilbert,
Juan Quintela
From: Liang Li <liang.z.li@intel.com>
Since 's->state' will be set in migrate_init(), there is no
need to set it before calling migrate_init(). The code and
the related comments can be removed.
Signed-off-by: Liang Li <liang.z.li@intel.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Amit Shah <amit.shah@redhat.com>
Message-Id: <1453875065-24326-1-git-send-email-liang.z.li@intel.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
migration/migration.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 4971722..82604d2 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1007,12 +1007,6 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
return;
}
- /* We are starting a new migration, so we want to start in a clean
- state. This change is only needed if previous migration
- failed/was cancelled. We don't use migrate_set_state() because
- we are setting the initial state, not changing it. */
- s->state = MIGRATION_STATUS_NONE;
-
s = migrate_init(¶ms);
if (strstart(uri, "tcp:", &p)) {
--
2.5.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PULL 8/9] static checker: e1000-82540em got aliased to e1000
2016-02-05 13:56 [Qemu-devel] [PULL 0/9] migration: small fixes Amit Shah
` (6 preceding siblings ...)
2016-02-05 13:56 ` [Qemu-devel] [PULL 7/9] migration: remove useless code Amit Shah
@ 2016-02-05 13:56 ` Amit Shah
2016-02-11 13:04 ` Paolo Bonzini
2016-02-05 13:56 ` [Qemu-devel] [PULL 9/9] migration: fix bad string passed to error_report() Amit Shah
2016-02-05 15:06 ` [Qemu-devel] [PULL 0/9] migration: small fixes Peter Maydell
9 siblings, 1 reply; 19+ messages in thread
From: Amit Shah @ 2016-02-05 13:56 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu list, Amit Shah, Dr. David Alan Gilbert, Juan Quintela
Commit 8304402033e8dbe8e379017d51ed1dd8344f1dce changed the name of the
e1000-82540em device to e1000. This was flagged:
Section "e1000-82540em" does not exist in dest
Add the mapping to the changed section names dictionary so the checker
can proceed.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
Acked-by: Jason Wang <jasowang@redhat.com>
Message-Id: <7ccfe834c897142dceaa4da87c13b7059fa12aa8.1450416947.git.amit.shah@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
scripts/vmstate-static-checker.py | 1 +
1 file changed, 1 insertion(+)
diff --git a/scripts/vmstate-static-checker.py b/scripts/vmstate-static-checker.py
index b6c0bbe..b5ecaf6 100755
--- a/scripts/vmstate-static-checker.py
+++ b/scripts/vmstate-static-checker.py
@@ -99,6 +99,7 @@ def get_changed_sec_name(sec):
# Section names can change -- see commit 292b1634 for an example.
changes = {
"ICH9 LPC": "ICH9-LPC",
+ "e1000-82540em": "e1000",
}
for item in changes:
--
2.5.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [Qemu-devel] [PULL 8/9] static checker: e1000-82540em got aliased to e1000
2016-02-05 13:56 ` [Qemu-devel] [PULL 8/9] static checker: e1000-82540em got aliased to e1000 Amit Shah
@ 2016-02-11 13:04 ` Paolo Bonzini
2016-02-22 12:39 ` Amit Shah
0 siblings, 1 reply; 19+ messages in thread
From: Paolo Bonzini @ 2016-02-11 13:04 UTC (permalink / raw)
To: Amit Shah, Jason Wang (jasowang@redhat.com)
Cc: Juan Quintela, qemu list, Dr. David Alan Gilbert
On 05/02/2016 14:56, Amit Shah wrote:
> Commit 8304402033e8dbe8e379017d51ed1dd8344f1dce changed the name of the
> e1000-82540em device to e1000. This was flagged:
>
> Section "e1000-82540em" does not exist in dest
>
> Add the mapping to the changed section names dictionary so the checker
> can proceed.
>
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> Acked-by: Jason Wang <jasowang@redhat.com>
> Message-Id: <7ccfe834c897142dceaa4da87c13b7059fa12aa8.1450416947.git.amit.shah@redhat.com>
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---
> scripts/vmstate-static-checker.py | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/scripts/vmstate-static-checker.py b/scripts/vmstate-static-checker.py
> index b6c0bbe..b5ecaf6 100755
> --- a/scripts/vmstate-static-checker.py
> +++ b/scripts/vmstate-static-checker.py
> @@ -99,6 +99,7 @@ def get_changed_sec_name(sec):
> # Section names can change -- see commit 292b1634 for an example.
> changes = {
> "ICH9 LPC": "ICH9-LPC",
> + "e1000-82540em": "e1000",
> }
>
> for item in changes:
>
This means that 2.5 cannot migrate 2.4 virtual machines, right? Is that
something we want to rectify in 2.6 by making e1000-82540em an alias of
e1000 (instead of the other way round)?
Paolo
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [Qemu-devel] [PULL 8/9] static checker: e1000-82540em got aliased to e1000
2016-02-11 13:04 ` Paolo Bonzini
@ 2016-02-22 12:39 ` Amit Shah
2016-02-23 7:02 ` Jason Wang
0 siblings, 1 reply; 19+ messages in thread
From: Amit Shah @ 2016-02-22 12:39 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Jason Wang (jasowang@redhat.com), Juan Quintela, qemu list,
Dr. David Alan Gilbert
On (Thu) 11 Feb 2016 [14:04:06], Paolo Bonzini wrote:
>
>
> On 05/02/2016 14:56, Amit Shah wrote:
> > Commit 8304402033e8dbe8e379017d51ed1dd8344f1dce changed the name of the
> > e1000-82540em device to e1000. This was flagged:
> >
> > Section "e1000-82540em" does not exist in dest
> >
> > Add the mapping to the changed section names dictionary so the checker
> > can proceed.
> >
> > Signed-off-by: Amit Shah <amit.shah@redhat.com>
> > Acked-by: Jason Wang <jasowang@redhat.com>
> > Message-Id: <7ccfe834c897142dceaa4da87c13b7059fa12aa8.1450416947.git.amit.shah@redhat.com>
> > Signed-off-by: Amit Shah <amit.shah@redhat.com>
> > ---
> > scripts/vmstate-static-checker.py | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/scripts/vmstate-static-checker.py b/scripts/vmstate-static-checker.py
> > index b6c0bbe..b5ecaf6 100755
> > --- a/scripts/vmstate-static-checker.py
> > +++ b/scripts/vmstate-static-checker.py
> > @@ -99,6 +99,7 @@ def get_changed_sec_name(sec):
> > # Section names can change -- see commit 292b1634 for an example.
> > changes = {
> > "ICH9 LPC": "ICH9-LPC",
> > + "e1000-82540em": "e1000",
> > }
> >
> > for item in changes:
> >
>
> This means that 2.5 cannot migrate 2.4 virtual machines, right? Is that
> something we want to rectify in 2.6 by making e1000-82540em an alias of
> e1000 (instead of the other way round)?
You're right; I misread it. With that commit (8304402033):
2.4 with e1000-82540em will not migrate to 2.5 with e1000-82540em.
This is despite they're aliased (so the cmdline is backward
compatible), but the migration device name actually changed.
Of course, 2.5->2.4 will also not work.
Since 2.4 emits 'e1000-82540em' as the device name in the migration
stream, and 2.5 emits just 'e1000', we have two different names for
the same device in two versions.
To fix this, we'll need a hack on the dest side to allow e1000 and
e1000-82540em in the migration stream for the device, and this can be
done for 2.6 and 2.5.stable.
Jason, can you attempt this?
Amit
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [Qemu-devel] [PULL 8/9] static checker: e1000-82540em got aliased to e1000
2016-02-22 12:39 ` Amit Shah
@ 2016-02-23 7:02 ` Jason Wang
2016-02-23 7:41 ` Amit Shah
0 siblings, 1 reply; 19+ messages in thread
From: Jason Wang @ 2016-02-23 7:02 UTC (permalink / raw)
To: Amit Shah, Paolo Bonzini; +Cc: Juan Quintela, qemu list, Dr. David Alan Gilbert
On 02/22/2016 08:39 PM, Amit Shah wrote:
> On (Thu) 11 Feb 2016 [14:04:06], Paolo Bonzini wrote:
>>
>> On 05/02/2016 14:56, Amit Shah wrote:
>>> Commit 8304402033e8dbe8e379017d51ed1dd8344f1dce changed the name of the
>>> e1000-82540em device to e1000. This was flagged:
>>>
>>> Section "e1000-82540em" does not exist in dest
>>>
>>> Add the mapping to the changed section names dictionary so the checker
>>> can proceed.
>>>
>>> Signed-off-by: Amit Shah <amit.shah@redhat.com>
>>> Acked-by: Jason Wang <jasowang@redhat.com>
>>> Message-Id: <7ccfe834c897142dceaa4da87c13b7059fa12aa8.1450416947.git.amit.shah@redhat.com>
>>> Signed-off-by: Amit Shah <amit.shah@redhat.com>
>>> ---
>>> scripts/vmstate-static-checker.py | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/scripts/vmstate-static-checker.py b/scripts/vmstate-static-checker.py
>>> index b6c0bbe..b5ecaf6 100755
>>> --- a/scripts/vmstate-static-checker.py
>>> +++ b/scripts/vmstate-static-checker.py
>>> @@ -99,6 +99,7 @@ def get_changed_sec_name(sec):
>>> # Section names can change -- see commit 292b1634 for an example.
>>> changes = {
>>> "ICH9 LPC": "ICH9-LPC",
>>> + "e1000-82540em": "e1000",
>>> }
>>>
>>> for item in changes:
>>>
>> This means that 2.5 cannot migrate 2.4 virtual machines, right? Is that
>> something we want to rectify in 2.6 by making e1000-82540em an alias of
>> e1000 (instead of the other way round)?
> You're right; I misread it. With that commit (8304402033):
>
> 2.4 with e1000-82540em will not migrate to 2.5 with e1000-82540em.
>
> This is despite they're aliased (so the cmdline is backward
> compatible), but the migration device name actually changed.
>
> Of course, 2.5->2.4 will also not work.
>
> Since 2.4 emits 'e1000-82540em' as the device name in the migration
> stream, and 2.5 emits just 'e1000', we have two different names for
> the same device in two versions.
>
> To fix this, we'll need a hack on the dest side to allow e1000 and
> e1000-82540em in the migration stream for the device, and this can be
> done for 2.6 and 2.5.stable.
>
> Jason, can you attempt this?
>
>
> Amit
Sure, but just need to understand the "problem". If I understand this
correctly, the issue only happen for JSON description at the end of
migration stream, and it won't break migration in fact?
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [Qemu-devel] [PULL 8/9] static checker: e1000-82540em got aliased to e1000
2016-02-23 7:02 ` Jason Wang
@ 2016-02-23 7:41 ` Amit Shah
2016-04-05 13:32 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 19+ messages in thread
From: Amit Shah @ 2016-02-23 7:41 UTC (permalink / raw)
To: Jason Wang
Cc: Paolo Bonzini, Juan Quintela, qemu list, Dr. David Alan Gilbert
On (Tue) 23 Feb 2016 [15:02:58], Jason Wang wrote:
> >> This means that 2.5 cannot migrate 2.4 virtual machines, right? Is that
> >> something we want to rectify in 2.6 by making e1000-82540em an alias of
> >> e1000 (instead of the other way round)?
> > You're right; I misread it. With that commit (8304402033):
> >
> > 2.4 with e1000-82540em will not migrate to 2.5 with e1000-82540em.
> >
> > This is despite they're aliased (so the cmdline is backward
> > compatible), but the migration device name actually changed.
> >
> > Of course, 2.5->2.4 will also not work.
> >
> > Since 2.4 emits 'e1000-82540em' as the device name in the migration
> > stream, and 2.5 emits just 'e1000', we have two different names for
> > the same device in two versions.
> >
> > To fix this, we'll need a hack on the dest side to allow e1000 and
> > e1000-82540em in the migration stream for the device, and this can be
> > done for 2.6 and 2.5.stable.
> >
> > Jason, can you attempt this?
> >
> Sure, but just need to understand the "problem". If I understand this
> correctly, the issue only happen for JSON description at the end of
> migration stream, and it won't break migration in fact?
No, this does break migration.
The stream contained 'e1000-82540em' as the section header for the
device earlier. It now only has 'e1000'. So a newer qemu will only
accept 'e1000', but not 'e1000-82540em' (from an older release).
Amit
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PULL 8/9] static checker: e1000-82540em got aliased to e1000
2016-02-23 7:41 ` Amit Shah
@ 2016-04-05 13:32 ` Dr. David Alan Gilbert
2016-04-06 1:48 ` Jason Wang
0 siblings, 1 reply; 19+ messages in thread
From: Dr. David Alan Gilbert @ 2016-04-05 13:32 UTC (permalink / raw)
To: Amit Shah; +Cc: Paolo Bonzini, Jason Wang, qemu list, Juan Quintela
* Amit Shah (amit.shah@redhat.com) wrote:
> On (Tue) 23 Feb 2016 [15:02:58], Jason Wang wrote:
> > >> This means that 2.5 cannot migrate 2.4 virtual machines, right? Is that
> > >> something we want to rectify in 2.6 by making e1000-82540em an alias of
> > >> e1000 (instead of the other way round)?
> > > You're right; I misread it. With that commit (8304402033):
> > >
> > > 2.4 with e1000-82540em will not migrate to 2.5 with e1000-82540em.
> > >
> > > This is despite they're aliased (so the cmdline is backward
> > > compatible), but the migration device name actually changed.
> > >
> > > Of course, 2.5->2.4 will also not work.
> > >
> > > Since 2.4 emits 'e1000-82540em' as the device name in the migration
> > > stream, and 2.5 emits just 'e1000', we have two different names for
> > > the same device in two versions.
> > >
> > > To fix this, we'll need a hack on the dest side to allow e1000 and
> > > e1000-82540em in the migration stream for the device, and this can be
> > > done for 2.6 and 2.5.stable.
> > >
> > > Jason, can you attempt this?
> > >
> > Sure, but just need to understand the "problem". If I understand this
> > correctly, the issue only happen for JSON description at the end of
> > migration stream, and it won't break migration in fact?
>
> No, this does break migration.
>
> The stream contained 'e1000-82540em' as the section header for the
> device earlier. It now only has 'e1000'. So a newer qemu will only
> accept 'e1000', but not 'e1000-82540em' (from an older release).
OK, so do we need to get this fixed for 2.6 - i.e. now.
Dave
>
> Amit
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PULL 8/9] static checker: e1000-82540em got aliased to e1000
2016-04-05 13:32 ` Dr. David Alan Gilbert
@ 2016-04-06 1:48 ` Jason Wang
2016-04-06 13:52 ` Amit Shah
0 siblings, 1 reply; 19+ messages in thread
From: Jason Wang @ 2016-04-06 1:48 UTC (permalink / raw)
To: Dr. David Alan Gilbert, Amit Shah; +Cc: Paolo Bonzini, qemu list, Juan Quintela
On 04/05/2016 09:32 PM, Dr. David Alan Gilbert wrote:
> * Amit Shah (amit.shah@redhat.com) wrote:
>> On (Tue) 23 Feb 2016 [15:02:58], Jason Wang wrote:
>>>>> This means that 2.5 cannot migrate 2.4 virtual machines, right? Is that
>>>>> something we want to rectify in 2.6 by making e1000-82540em an alias of
>>>>> e1000 (instead of the other way round)?
>>>> You're right; I misread it. With that commit (8304402033):
>>>>
>>>> 2.4 with e1000-82540em will not migrate to 2.5 with e1000-82540em.
>>>>
>>>> This is despite they're aliased (so the cmdline is backward
>>>> compatible), but the migration device name actually changed.
>>>>
>>>> Of course, 2.5->2.4 will also not work.
>>>>
>>>> Since 2.4 emits 'e1000-82540em' as the device name in the migration
>>>> stream, and 2.5 emits just 'e1000', we have two different names for
>>>> the same device in two versions.
>>>>
>>>> To fix this, we'll need a hack on the dest side to allow e1000 and
>>>> e1000-82540em in the migration stream for the device, and this can be
>>>> done for 2.6 and 2.5.stable.
>>>>
>>>> Jason, can you attempt this?
>>>>
>>> Sure, but just need to understand the "problem". If I understand this
>>> correctly, the issue only happen for JSON description at the end of
>>> migration stream, and it won't break migration in fact?
>> No, this does break migration.
>>
>> The stream contained 'e1000-82540em' as the section header for the
>> device earlier. It now only has 'e1000'. So a newer qemu will only
>> accept 'e1000', but not 'e1000-82540em' (from an older release).
> OK, so do we need to get this fixed for 2.6 - i.e. now.
>
> Dave
Sorry for the late response.
Have a try with 2.4 -> 2.5 migration and it works. Looking at
save_section_header(), it will save se->idstr which seems always be
"e1000" even if e1000-82540em is used in cli, or is there anything I missed?
>
>> Amit
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PULL 8/9] static checker: e1000-82540em got aliased to e1000
2016-04-06 1:48 ` Jason Wang
@ 2016-04-06 13:52 ` Amit Shah
2016-04-07 7:25 ` Jason Wang
0 siblings, 1 reply; 19+ messages in thread
From: Amit Shah @ 2016-04-06 13:52 UTC (permalink / raw)
To: Jason Wang
Cc: Paolo Bonzini, Juan Quintela, Dr. David Alan Gilbert, qemu list
On (Wed) 06 Apr 2016 [09:48:19], Jason Wang wrote:
>
>
> On 04/05/2016 09:32 PM, Dr. David Alan Gilbert wrote:
> > * Amit Shah (amit.shah@redhat.com) wrote:
> >> On (Tue) 23 Feb 2016 [15:02:58], Jason Wang wrote:
> >>>>> This means that 2.5 cannot migrate 2.4 virtual machines, right? Is that
> >>>>> something we want to rectify in 2.6 by making e1000-82540em an alias of
> >>>>> e1000 (instead of the other way round)?
> >>>> You're right; I misread it. With that commit (8304402033):
> >>>>
> >>>> 2.4 with e1000-82540em will not migrate to 2.5 with e1000-82540em.
> >>>>
> >>>> This is despite they're aliased (so the cmdline is backward
> >>>> compatible), but the migration device name actually changed.
> >>>>
> >>>> Of course, 2.5->2.4 will also not work.
> >>>>
> >>>> Since 2.4 emits 'e1000-82540em' as the device name in the migration
> >>>> stream, and 2.5 emits just 'e1000', we have two different names for
> >>>> the same device in two versions.
> >>>>
> >>>> To fix this, we'll need a hack on the dest side to allow e1000 and
> >>>> e1000-82540em in the migration stream for the device, and this can be
> >>>> done for 2.6 and 2.5.stable.
> >>>>
> >>>> Jason, can you attempt this?
> >>>>
> >>> Sure, but just need to understand the "problem". If I understand this
> >>> correctly, the issue only happen for JSON description at the end of
> >>> migration stream, and it won't break migration in fact?
> >> No, this does break migration.
> >>
> >> The stream contained 'e1000-82540em' as the section header for the
> >> device earlier. It now only has 'e1000'. So a newer qemu will only
> >> accept 'e1000', but not 'e1000-82540em' (from an older release).
> > OK, so do we need to get this fixed for 2.6 - i.e. now.
> >
> > Dave
>
> Sorry for the late response.
>
> Have a try with 2.4 -> 2.5 migration and it works. Looking at
> save_section_header(), it will save se->idstr which seems always be
> "e1000" even if e1000-82540em is used in cli, or is there anything I missed?
OK, sorry for the wrong alarm.
The VMStateDescription's 'name' field has remained the same; but the
section name has changed, which is fine (that isn't transmitted over
the wire). So my initial reading was correct - and the whitelist I
added in 1483e0d74dcfd183ff46dd63cc57e1fe8b775bf8 is fine.
So nothing needed for the migration stream.
Thanks, Jason.
Amit
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PULL 8/9] static checker: e1000-82540em got aliased to e1000
2016-04-06 13:52 ` Amit Shah
@ 2016-04-07 7:25 ` Jason Wang
0 siblings, 0 replies; 19+ messages in thread
From: Jason Wang @ 2016-04-07 7:25 UTC (permalink / raw)
To: Amit Shah; +Cc: qemu list, Paolo Bonzini, Dr. David Alan Gilbert, Juan Quintela
On 04/06/2016 09:52 PM, Amit Shah wrote:
> On (Wed) 06 Apr 2016 [09:48:19], Jason Wang wrote:
>>
>> On 04/05/2016 09:32 PM, Dr. David Alan Gilbert wrote:
>>> * Amit Shah (amit.shah@redhat.com) wrote:
>>>> On (Tue) 23 Feb 2016 [15:02:58], Jason Wang wrote:
>>>>>>> This means that 2.5 cannot migrate 2.4 virtual machines, right? Is that
>>>>>>> something we want to rectify in 2.6 by making e1000-82540em an alias of
>>>>>>> e1000 (instead of the other way round)?
>>>>>> You're right; I misread it. With that commit (8304402033):
>>>>>>
>>>>>> 2.4 with e1000-82540em will not migrate to 2.5 with e1000-82540em.
>>>>>>
>>>>>> This is despite they're aliased (so the cmdline is backward
>>>>>> compatible), but the migration device name actually changed.
>>>>>>
>>>>>> Of course, 2.5->2.4 will also not work.
>>>>>>
>>>>>> Since 2.4 emits 'e1000-82540em' as the device name in the migration
>>>>>> stream, and 2.5 emits just 'e1000', we have two different names for
>>>>>> the same device in two versions.
>>>>>>
>>>>>> To fix this, we'll need a hack on the dest side to allow e1000 and
>>>>>> e1000-82540em in the migration stream for the device, and this can be
>>>>>> done for 2.6 and 2.5.stable.
>>>>>>
>>>>>> Jason, can you attempt this?
>>>>>>
>>>>> Sure, but just need to understand the "problem". If I understand this
>>>>> correctly, the issue only happen for JSON description at the end of
>>>>> migration stream, and it won't break migration in fact?
>>>> No, this does break migration.
>>>>
>>>> The stream contained 'e1000-82540em' as the section header for the
>>>> device earlier. It now only has 'e1000'. So a newer qemu will only
>>>> accept 'e1000', but not 'e1000-82540em' (from an older release).
>>> OK, so do we need to get this fixed for 2.6 - i.e. now.
>>>
>>> Dave
>> Sorry for the late response.
>>
>> Have a try with 2.4 -> 2.5 migration and it works. Looking at
>> save_section_header(), it will save se->idstr which seems always be
>> "e1000" even if e1000-82540em is used in cli, or is there anything I missed?
> OK, sorry for the wrong alarm.
>
> The VMStateDescription's 'name' field has remained the same; but the
> section name has changed, which is fine (that isn't transmitted over
> the wire). So my initial reading was correct - and the whitelist I
> added in 1483e0d74dcfd183ff46dd63cc57e1fe8b775bf8 is fine.
>
> So nothing needed for the migration stream.
>
> Thanks, Jason.
>
> Amit
>
Thanks for the confirmation.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PULL 9/9] migration: fix bad string passed to error_report()
2016-02-05 13:56 [Qemu-devel] [PULL 0/9] migration: small fixes Amit Shah
` (7 preceding siblings ...)
2016-02-05 13:56 ` [Qemu-devel] [PULL 8/9] static checker: e1000-82540em got aliased to e1000 Amit Shah
@ 2016-02-05 13:56 ` Amit Shah
2016-02-05 15:06 ` [Qemu-devel] [PULL 0/9] migration: small fixes Peter Maydell
9 siblings, 0 replies; 19+ messages in thread
From: Amit Shah @ 2016-02-05 13:56 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu list, Amit Shah, Greg Kurz, Dr. David Alan Gilbert,
Juan Quintela
From: Greg Kurz <gkurz@linux.vnet.ibm.com>
state->name does not contain a terminating '\0' and you may get:
Machine type received is 'pseries-2.3y�?' and local is 'pseries-2.4'
load of migration failed: Invalid argument
Let's add a precision modifier to fix this.
Reviewed-by: Amit Shah <amit.shah@redhat.com>
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
Message-Id: <20160205083201.2201.76109.stgit@bahia.huguette.org>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
migration/savevm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index 00be5fe..94f2894 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -299,8 +299,8 @@ static int configuration_post_load(void *opaque, int version_id)
const char *current_name = MACHINE_GET_CLASS(current_machine)->name;
if (strncmp(state->name, current_name, state->len) != 0) {
- error_report("Machine type received is '%s' and local is '%s'",
- state->name, current_name);
+ error_report("Machine type received is '%.*s' and local is '%s'",
+ (int) state->len, state->name, current_name);
return -EINVAL;
}
return 0;
--
2.5.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [Qemu-devel] [PULL 0/9] migration: small fixes
2016-02-05 13:56 [Qemu-devel] [PULL 0/9] migration: small fixes Amit Shah
` (8 preceding siblings ...)
2016-02-05 13:56 ` [Qemu-devel] [PULL 9/9] migration: fix bad string passed to error_report() Amit Shah
@ 2016-02-05 15:06 ` Peter Maydell
9 siblings, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2016-02-05 15:06 UTC (permalink / raw)
To: Amit Shah; +Cc: qemu list, Dr. David Alan Gilbert, Juan Quintela
On 5 February 2016 at 13:56, Amit Shah <amit.shah@redhat.com> wrote:
> The following changes since commit d38ea87ac54af64ef611de434d07c12dc0399216:
>
> all: Clean up includes (2016-02-04 17:41:30 +0000)
>
> are available in the git repository at:
>
> https://git.kernel.org/pub/scm/virt/qemu/amit/migration.git tags/migration-for-2.6-2
>
> for you to fetch changes up to 15d61692da651fc79b3fc40050b986c5a73055c0:
>
> migration: fix bad string passed to error_report() (2016-02-05 19:09:51 +0530)
>
> ----------------------------------------------------------------
> Migration pull req.
>
> Small fixes, nothing major.
>
> ----------------------------------------------------------------
>
Applied, thanks.
-- PMM
^ permalink raw reply [flat|nested] 19+ messages in thread