Re: [Qemu-devel] [PATCH 0/2] Fix long vm downtime during live migration

Re: [Qemu-devel] [PATCH 0/2] Fix long vm downtime during live migration

[Paolo Bonzini]

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

Re: [Qemu-devel] [PATCH 1/2] migration: do cleanup operation after completion

[Paolo Bonzini]

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	

Re: [Qemu-devel] [PATCH 1/2] migration: do cleanup operation after completion

[Li, Liang Z]

> 
> 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

[Qemu-devel] [PATCH 0/2] Fix long vm downtime during live migration

[Liang Li]

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(-)

-- 
1.9.1

[Qemu-devel] [PATCH 1/2] migration: do cleanup operation after completion

[Liang Li]

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

[Qemu-devel] [PATCH 2/2] migration: rename qemu_savevm_state_cancel

[Liang Li]

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