* Re: [Qemu-devel] [PATCH 0/2] Fix long vm downtime during live migration
2015-08-12 21:04 [Qemu-devel] [PATCH 0/2] Fix long vm downtime during live migration Liang Li
@ 2015-08-12 9:52 ` Paolo Bonzini
2015-08-12 21:04 ` [Qemu-devel] [PATCH 1/2] migration: do cleanup operation after completion Liang Li
2015-08-12 21:04 ` [Qemu-devel] [PATCH 2/2] migration: rename qemu_savevm_state_cancel Liang Li
2 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2015-08-12 9:52 UTC (permalink / raw)
To: Liang Li, qemu-devel
Cc: qemu-block, quintela, qemu-stable, amit.shah, stefanha,
yang.z.zhang
On 12/08/2015 23:04, Liang Li wrote:
> Some cleanup operations take long time during the pause and copy stage,
> especially with the KVM patch 3ea3b7fa9af067, do these operation after
> the completion of live migration can help to reduce VM downtime.
>
>
> Liang Li (2):
> migration: do cleanup operation after completion
> migration: rename qemu_savevm_state_cancel
>
> include/sysemu/sysemu.h | 2 +-
> migration/block.c | 1 -
> migration/migration.c | 7 +++++--
> migration/ram.c | 1 -
> migration/savevm.c | 6 +++---
> trace-events | 2 +-
> 6 files changed, 10 insertions(+), 9 deletions(-)
>
That's really a clever solution!
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-stable@nongnu.org
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH 1/2] migration: do cleanup operation after completion
2015-08-12 21:04 [Qemu-devel] [PATCH 0/2] Fix long vm downtime during live migration Liang Li
2015-08-12 9:52 ` Paolo Bonzini
@ 2015-08-12 21:04 ` Liang Li
2015-08-12 9:55 ` Paolo Bonzini
2015-08-12 21:04 ` [Qemu-devel] [PATCH 2/2] migration: rename qemu_savevm_state_cancel Liang Li
2 siblings, 1 reply; 6+ messages in thread
From: Liang Li @ 2015-08-12 21:04 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, quintela, Liang Li, yang.z.zhang, stefanha, amit.shah
Because of the patch 3ea3b7fa9af067982f34b of kvm, now the migration_end()
is a time consuming operation, which takes about dozens of milliseconds, and
will prolong VM downtime. Such an operation should be done after migration
completion.
For a VM with 8G RAM, this patch can reduce the VM downtime about 32 ms during
live migration.
Signed-off-by: Liang Li <liang.z.li@intel.com>
---
migration/block.c | 1 -
migration/migration.c | 5 ++++-
migration/ram.c | 1 -
3 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/migration/block.c b/migration/block.c
index ed865ed..85496fd 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -750,7 +750,6 @@ static int block_save_complete(QEMUFile *f, void *opaque)
qemu_put_be64(f, BLK_MIG_FLAG_EOS);
- blk_mig_cleanup();
return 0;
}
diff --git a/migration/migration.c b/migration/migration.c
index 662e77e..c22095e 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -923,6 +923,7 @@ static void *migration_thread(void *opaque)
int64_t initial_bytes = 0;
int64_t max_size = 0;
int64_t start_time = initial_time;
+ int64_t end_time;
bool old_vm_running = false;
rcu_register_thread();
@@ -1008,8 +1009,10 @@ static void *migration_thread(void *opaque)
}
qemu_mutex_lock_iothread();
+ end_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
+ qemu_savevm_state_cancel();
+
if (s->state == MIGRATION_STATUS_COMPLETED) {
- int64_t end_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
uint64_t transferred_bytes = qemu_ftell(s->file);
s->total_time = end_time - s->total_time;
s->downtime = end_time - start_time;
diff --git a/migration/ram.c b/migration/ram.c
index 7f007e6..6249f6e 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1269,7 +1269,6 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
rcu_read_unlock();
- migration_end();
qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
return 0;
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] migration: do cleanup operation after completion
2015-08-12 21:04 ` [Qemu-devel] [PATCH 1/2] migration: do cleanup operation after completion Liang Li
@ 2015-08-12 9:55 ` Paolo Bonzini
2015-08-12 11:11 ` Li, Liang Z
0 siblings, 1 reply; 6+ messages in thread
From: Paolo Bonzini @ 2015-08-12 9:55 UTC (permalink / raw)
To: Liang Li, qemu-devel
Cc: yang.z.zhang, amit.shah, stefanha, qemu-block, quintela
On 12/08/2015 23:04, Liang Li wrote:
> @@ -1008,8 +1009,10 @@ static void *migration_thread(void *opaque)
> }
>
> qemu_mutex_lock_iothread();
> + end_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
> + qemu_savevm_state_cancel();
> +
You can remove the qemu_savevm_state_cancel() call from
migrate_fd_cleanup, too. Probably best to post a v2 with that change as
well.
Paolo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] migration: do cleanup operation after completion
2015-08-12 9:55 ` Paolo Bonzini
@ 2015-08-12 11:11 ` Li, Liang Z
0 siblings, 0 replies; 6+ messages in thread
From: Li, Liang Z @ 2015-08-12 11:11 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel@nongnu.org
Cc: Zhang, Yang Z, amit.shah@redhat.com, stefanha@redhat.com,
qemu-block@nongnu.org, quintela@redhat.com
>
> On 12/08/2015 23:04, Liang Li wrote:
> > @@ -1008,8 +1009,10 @@ static void *migration_thread(void *opaque)
> > }
> >
> > qemu_mutex_lock_iothread();
> > + end_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
> > + qemu_savevm_state_cancel();
> > +
>
> You can remove the qemu_savevm_state_cancel() call from
> migrate_fd_cleanup, too. Probably best to post a v2 with that change as well.
>
> Paolo
You are right. Done.
Liang
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH 2/2] migration: rename qemu_savevm_state_cancel
2015-08-12 21:04 [Qemu-devel] [PATCH 0/2] Fix long vm downtime during live migration Liang Li
2015-08-12 9:52 ` Paolo Bonzini
2015-08-12 21:04 ` [Qemu-devel] [PATCH 1/2] migration: do cleanup operation after completion Liang Li
@ 2015-08-12 21:04 ` Liang Li
2 siblings, 0 replies; 6+ messages in thread
From: Liang Li @ 2015-08-12 21:04 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, quintela, Liang Li, yang.z.zhang, stefanha, amit.shah
The function qemu_savevm_state_cancel is called after the migration
in migration_thread, it seems strange to 'cancel' it after completion,
rename it to qemu_savevm_state_cleanup looks better.
Signed-off-by: Liang Li <liang.z.li@intel.com>
---
include/sysemu/sysemu.h | 2 +-
migration/migration.c | 4 ++--
migration/savevm.c | 6 +++---
trace-events | 2 +-
4 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 44570d1..9cc0240 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -88,7 +88,7 @@ void qemu_savevm_state_begin(QEMUFile *f,
void qemu_savevm_state_header(QEMUFile *f);
int qemu_savevm_state_iterate(QEMUFile *f);
void qemu_savevm_state_complete(QEMUFile *f);
-void qemu_savevm_state_cancel(void);
+void qemu_savevm_state_cleanup(void);
uint64_t qemu_savevm_state_pending(QEMUFile *f, uint64_t max_size);
int qemu_loadvm_state(QEMUFile *f);
diff --git a/migration/migration.c b/migration/migration.c
index c22095e..e7fe50f 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -561,7 +561,7 @@ static void migrate_fd_cleanup(void *opaque)
assert(s->state != MIGRATION_STATUS_ACTIVE);
if (s->state != MIGRATION_STATUS_COMPLETED) {
- qemu_savevm_state_cancel();
+ qemu_savevm_state_cleanup();
if (s->state == MIGRATION_STATUS_CANCELLING) {
migrate_set_state(s, MIGRATION_STATUS_CANCELLING,
MIGRATION_STATUS_CANCELLED);
@@ -1010,7 +1010,7 @@ static void *migration_thread(void *opaque)
qemu_mutex_lock_iothread();
end_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
- qemu_savevm_state_cancel();
+ qemu_savevm_state_cleanup();
if (s->state == MIGRATION_STATUS_COMPLETED) {
uint64_t transferred_bytes = qemu_ftell(s->file);
diff --git a/migration/savevm.c b/migration/savevm.c
index 6071215..b141b17 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -905,11 +905,11 @@ uint64_t qemu_savevm_state_pending(QEMUFile *f, uint64_t max_size)
return ret;
}
-void qemu_savevm_state_cancel(void)
+void qemu_savevm_state_cleanup(void)
{
SaveStateEntry *se;
- trace_savevm_state_cancel();
+ trace_savevm_state_cleanup();
QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
if (se->ops && se->ops->cancel) {
se->ops->cancel(se->opaque);
@@ -946,7 +946,7 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp)
ret = qemu_file_get_error(f);
}
if (ret != 0) {
- qemu_savevm_state_cancel();
+ qemu_savevm_state_cleanup();
error_setg_errno(errp, -ret, "Error while writing VM state");
}
return ret;
diff --git a/trace-events b/trace-events
index 94bf3bb..102373c 100644
--- a/trace-events
+++ b/trace-events
@@ -1195,7 +1195,7 @@ savevm_state_begin(void) ""
savevm_state_header(void) ""
savevm_state_iterate(void) ""
savevm_state_complete(void) ""
-savevm_state_cancel(void) ""
+savevm_state_cleanup(void) ""
vmstate_save(const char *idstr, const char *vmsd_name) "%s, %s"
vmstate_load(const char *idstr, const char *vmsd_name) "%s, %s"
qemu_announce_self_iter(const char *mac) "%s"
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread