[Qemu-devel] [v2 0/4] Fix long vm downtime during live migration

[Qemu-devel] [v2 0/4] 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 operations after
the completion of live migration can help to reduce VM downtime.

Ony the first patch changes the behavior, the rest 3 patches are for code
cleanup.

Changes:
  * Remove qemu_savevm_sate_cancel() in migrate_fd_cleanup()
  * Add 2 more patches for code clean up

Liang Li (4):
  migration: do cleanup operation after completion
  migration: rename qemu_savevm_state_cancel
  migration: rename cancel to cleanup in SaveVMHandles
  migration: code clean up

 include/migration/vmstate.h |  2 +-
 include/sysemu/sysemu.h     |  2 +-
 migration/block.c           | 10 ++--------
 migration/migration.c       | 14 +++++++-------
 migration/ram.c             | 10 ++--------
 migration/savevm.c          | 10 +++++-----
 trace-events                |  2 +-
 7 files changed, 19 insertions(+), 31 deletions(-)

-- 
1.9.1

[Qemu-devel] [v2 1/4] 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>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 migration/block.c     |  1 -
 migration/migration.c | 14 +++++++-------
 migration/ram.c       |  1 -
 3 files changed, 7 insertions(+), 9 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..4ddb9ad 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -560,12 +560,9 @@ static void migrate_fd_cleanup(void *opaque)
 
     assert(s->state != MIGRATION_STATUS_ACTIVE);
 
-    if (s->state != MIGRATION_STATUS_COMPLETED) {
-        qemu_savevm_state_cancel();
-        if (s->state == MIGRATION_STATUS_CANCELLING) {
-            migrate_set_state(s, MIGRATION_STATUS_CANCELLING,
-                              MIGRATION_STATUS_CANCELLED);
-        }
+    if (s->state == MIGRATION_STATUS_CANCELLING) {
+        migrate_set_state(s, MIGRATION_STATUS_CANCELLING,
+                          MIGRATION_STATUS_CANCELLED);
     }
 
     notifier_list_notify(&migration_state_notifiers, s);
@@ -923,6 +920,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();
@@ -1007,9 +1005,11 @@ static void *migration_thread(void *opaque)
         }
     }
 
+    end_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
+
     qemu_mutex_lock_iothread();
+    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] [v2 2/4] 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   | 2 +-
 migration/savevm.c      | 6 +++---
 trace-events            | 2 +-
 4 files changed, 6 insertions(+), 6 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 4ddb9ad..1ff30fe 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1008,7 +1008,7 @@ static void *migration_thread(void *opaque)
     end_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
 
     qemu_mutex_lock_iothread();
-    qemu_savevm_state_cancel();
+    qemu_savevm_state_cleanup();
     if (s->state == MIGRATION_STATUS_COMPLETED) {
         uint64_t transferred_bytes = qemu_ftell(s->file);
         s->total_time = end_time - s->total_time;
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

[Qemu-devel] [v2 3/4] migration: rename cancel to cleanup in SaveVMHandles

[Liang Li]

'cleanup' seems more appropriate than 'cancel'.

Signed-off-by: Liang Li <liang.z.li@intel.com>
---
 include/migration/vmstate.h | 2 +-
 migration/block.c           | 2 +-
 migration/ram.c             | 2 +-
 migration/savevm.c          | 4 ++--
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 2e5a97d..24b939c 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -39,7 +39,7 @@ typedef struct SaveVMHandlers {
     void (*set_params)(const MigrationParams *params, void * opaque);
     SaveStateHandler *save_state;
 
-    void (*cancel)(void *opaque);
+    void (*cleanup)(void *opaque);
     int (*save_live_complete)(QEMUFile *f, void *opaque);
 
     /* This runs both outside and inside the iothread lock.  */
diff --git a/migration/block.c b/migration/block.c
index 85496fd..55442bd 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -879,7 +879,7 @@ static SaveVMHandlers savevm_block_handlers = {
     .save_live_complete = block_save_complete,
     .save_live_pending = block_save_pending,
     .load_state = block_load,
-    .cancel = block_migration_cancel,
+    .cleanup = block_migration_cancel,
     .is_active = block_is_active,
 };
 
diff --git a/migration/ram.c b/migration/ram.c
index 6249f6e..e7f711e 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1610,7 +1610,7 @@ static SaveVMHandlers savevm_ram_handlers = {
     .save_live_complete = ram_save_complete,
     .save_live_pending = ram_save_pending,
     .load_state = ram_load,
-    .cancel = ram_migration_cancel,
+    .cleanup = ram_migration_cancel,
 };
 
 void ram_mig_init(void)
diff --git a/migration/savevm.c b/migration/savevm.c
index b141b17..96d0aab 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -911,8 +911,8 @@ void qemu_savevm_state_cleanup(void)
 
     trace_savevm_state_cleanup();
     QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
-        if (se->ops && se->ops->cancel) {
-            se->ops->cancel(se->opaque);
+        if (se->ops && se->ops->cleanup) {
+            se->ops->cleanup(se->opaque);
         }
     }
 }
-- 
1.9.1

[Qemu-devel] [v2 4/4] migration: code clean up

[Liang Li]

Just clean up code, no behavior change.

Signed-off-by: Liang Li <liang.z.li@intel.com>
---
 migration/block.c | 9 ++-------
 migration/ram.c   | 9 ++-------
 2 files changed, 4 insertions(+), 14 deletions(-)

diff --git a/migration/block.c b/migration/block.c
index 55442bd..869798c 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -591,7 +591,7 @@ static int64_t get_remaining_dirty(void)
 
 /* Called with iothread lock taken.  */
 
-static void blk_mig_cleanup(void)
+static void block_migration_cleanup(void *opaque)
 {
     BlkMigDevState *bmds;
     BlkMigBlock *blk;
@@ -618,11 +618,6 @@ static void blk_mig_cleanup(void)
     blk_mig_unlock();
 }
 
-static void block_migration_cancel(void *opaque)
-{
-    blk_mig_cleanup();
-}
-
 static int block_save_setup(QEMUFile *f, void *opaque)
 {
     int ret;
@@ -879,7 +874,7 @@ static SaveVMHandlers savevm_block_handlers = {
     .save_live_complete = block_save_complete,
     .save_live_pending = block_save_pending,
     .load_state = block_load,
-    .cleanup = block_migration_cancel,
+    .cleanup = block_migration_cleanup,
     .is_active = block_is_active,
 };
 
diff --git a/migration/ram.c b/migration/ram.c
index e7f711e..8b045fe 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1024,7 +1024,7 @@ void free_xbzrle_decoded_buf(void)
     xbzrle_decoded_buf = NULL;
 }
 
-static void migration_end(void)
+static void ram_migration_cleanup(void *opaque)
 {
     /* caller have hold iothread lock or is in a bh, so there is
      * no writing race against this migration_bitmap
@@ -1049,11 +1049,6 @@ static void migration_end(void)
     XBZRLE_cache_unlock();
 }
 
-static void ram_migration_cancel(void *opaque)
-{
-    migration_end();
-}
-
 static void reset_ram_globals(void)
 {
     last_seen_block = NULL;
@@ -1610,7 +1605,7 @@ static SaveVMHandlers savevm_ram_handlers = {
     .save_live_complete = ram_save_complete,
     .save_live_pending = ram_save_pending,
     .load_state = ram_load,
-    .cleanup = ram_migration_cancel,
+    .cleanup = ram_migration_cleanup,
 };
 
 void ram_mig_init(void)
-- 
1.9.1

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

[Paolo Bonzini]

On 13/08/2015 07:46, Liang Li wrote:
> Some cleanup operations take long time during the pause and copy stage,
> especially with the KVM patch 3ea3b7fa9af067, do these operations after
> the completion of live migration can help to reduce VM downtime.
> 
> Ony the first patch changes the behavior, the rest 3 patches are for code
> cleanup.
> 
> Changes:
>   * Remove qemu_savevm_sate_cancel() in migrate_fd_cleanup()
>   * Add 2 more patches for code clean up

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

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

[Li, Liang Z]

> > Some cleanup operations take long time during the pause and copy
> > stage, especially with the KVM patch 3ea3b7fa9af067, do these
> > operations after the completion of live migration can help to reduce VM
> downtime.
> >
> > Ony the first patch changes the behavior, the rest 3 patches are for
> > code cleanup.
> >
> > Changes:
> >   * Remove qemu_savevm_sate_cancel() in migrate_fd_cleanup()
> >   * Add 2 more patches for code clean up
> 
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Hi Juan & Amit,

Could you help to review this serial of patches  and give some comments when you have time? 

The link to the thread is: https://lists.nongnu.org/archive/html/qemu-devel/2015-08/msg01516.html

Liang

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

[Li, Liang Z]

> > Some cleanup operations take long time during the pause and copy 
> > stage, especially with the KVM patch 3ea3b7fa9af067, do these 
> > operations after the completion of live migration can help to reduce 
> > VM
> downtime.
> >
> > Ony the first patch changes the behavior, the rest 3 patches are for 
> > code cleanup.
> >
> > Changes:
> >   * Remove qemu_savevm_sate_cancel() in migrate_fd_cleanup()
> >   * Add 2 more patches for code clean up
> 
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Resend this mail.

Hi Juan & Amit,

Could you help to review this serial of patches  and give some comments when you have time? 

The link to the thread is: https://lists.nongnu.org/archive/html/qemu-devel/2015-08/msg01516.html

Liang

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

[Amit Shah]

On (Wed) 21 Oct 2015 [09:00:31], Li, Liang Z wrote:
> > > Some cleanup operations take long time during the pause and copy 
> > > stage, especially with the KVM patch 3ea3b7fa9af067, do these 
> > > operations after the completion of live migration can help to reduce 
> > > VM
> > downtime.
> > >
> > > Ony the first patch changes the behavior, the rest 3 patches are for 
> > > code cleanup.
> > >
> > > Changes:
> > >   * Remove qemu_savevm_sate_cancel() in migrate_fd_cleanup()
> > >   * Add 2 more patches for code clean up
> > 
> > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> Resend this mail.

Hi Liang,

I'm looking at this patchset; I did take a look at it in the past and
I couldn't map everything; I need more time to look at everything
here.

Definitely a more detailed commit log or comments would've helped.

But there's already a reviewed-by from Paolo, so I fully expect this
patch to be merged in 2.5.

		Amit

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

[Li, Liang Z]

Hi Amit, 

I am very glad that you are looking at this patchset.  Looking forward to your comments. :)


Liang

> -----Original Message-----
> From: Amit Shah [mailto:amit.shah@redhat.com]
> Sent: Monday, October 26, 2015 2:10 PM
> To: Li, Liang Z
> Cc: 'qemu-devel@nongnu.org'; 'quintela@redhat.com'; 'qemu-
> block@nongnu.org'; Wang, Yong Y; 'qemu-stable@nongnu.org';
> 'stefanha@redhat.com'; Zhang, Yang Z; 'Paolo Bonzini'
> Subject: Re: [Qemu-devel] [v2 0/4] Fix long vm downtime during live migration
> 
> On (Wed) 21 Oct 2015 [09:00:31], Li, Liang Z wrote:
> > > > Some cleanup operations take long time during the pause and copy
> > > > stage, especially with the KVM patch 3ea3b7fa9af067, do these
> > > > operations after the completion of live migration can help to
> > > > reduce VM
> > > downtime.
> > > >
> > > > Ony the first patch changes the behavior, the rest 3 patches are
> > > > for code cleanup.
> > > >
> > > > Changes:
> > > >   * Remove qemu_savevm_sate_cancel() in migrate_fd_cleanup()
> > > >   * Add 2 more patches for code clean up
> > >
> > > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> >
> > Resend this mail.
> 
> Hi Liang,
> 
> I'm looking at this patchset; I did take a look at it in the past and I couldn't map
> everything; I need more time to look at everything here.
> 
> Definitely a more detailed commit log or comments would've helped.
> 
> But there's already a reviewed-by from Paolo, so I fully expect this patch to be
> merged in 2.5.
> 
> 		Amit