[Qemu-devel] [PULL 0/5] Migration pull request

[Qemu-devel] [PULL 0/5] Migration pull request

[Juan Quintela]

Hi

This is the pull request for migration, it includes:

- Liang Li series for improving migration downtime
- Mark Cave-Ayland fix to analyze-migration script

Please, apply

The following changes since commit 79cf9fad341e6e7bd6b55395b71d5c5727d7f5b0:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20151103' into staging (2015-11-03 14:54:40 +0000)

are available in the git repository at:

  git://github.com/juanquintela/qemu.git tags/migration/20151104

for you to fetch changes up to 96e5c9bc77acef8b7b56cbe23a8a2611feff9e34:

  migration: fix analyze-migration.py script (2015-11-04 13:40:13 +0100)

----------------------------------------------------------------
migration/next for 20151104

----------------------------------------------------------------
Liang Li (4):
      migration: defer migration_end & blk_mig_cleanup
      migration: rename qemu_savevm_state_cancel
      migration: rename cancel to cleanup in SaveVMHandles
      migration: code clean up

Mark Cave-Ayland (1):
      migration: fix analyze-migration.py script

 include/migration/vmstate.h  |  2 +-
 include/sysemu/sysemu.h      |  2 +-
 migration/block.c            | 10 ++--------
 migration/migration.c        | 13 ++++++-------
 migration/ram.c              | 10 ++--------
 migration/savevm.c           | 10 +++++-----
 scripts/analyze-migration.py | 13 +++++++++++++
 trace-events                 |  2 +-
 8 files changed, 31 insertions(+), 31 deletions(-)

[Qemu-devel] [PULL 1/5] migration: defer migration_end & blk_mig_cleanup

[Juan Quintela]

From: Liang Li <liang.z.li@intel.com>

Because of the patch 3ea3b7fa9af067982f34b of kvm, which introduces a
lazy collapsing of small sptes into large sptes mechanism, now
migration_end() is a time consuming operation because it calls
memroy_global_dirty_log_stop(), which will trigger the dropping of small
sptes operation and takes about dozens of milliseconds, so call
migration_end() before all the vmsate data has already been transferred
to the destination will prolong VM downtime. This operation should be
deferred after all the data has been transferred to the destination.

blk_mig_cleanup() can be deferred too.

For a VM with 8G RAM, this patch can reduce the VM downtime about 30 ms.

Signed-off-by: Liang Li <liang.z.li@intel.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>al3
Reviewed-by: Amit Shah <amit.shah@redhat.com>al3
Signed-off-by: Juan Quintela <quintela@redhat.com>al3
---
 migration/block.c     |  1 -
 migration/migration.c | 13 ++++++-------
 migration/ram.c       |  1 -
 3 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/migration/block.c b/migration/block.c
index f7bb1e0..8401597 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 b092f38..d5a7304 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -613,12 +613,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);
@@ -1028,6 +1025,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();
@@ -1089,10 +1087,11 @@ static void *migration_thread(void *opaque)

     /* If we enabled cpu throttling for auto-converge, turn it off. */
     cpu_throttle_stop();
+    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 a25bcc7..25e9eeb 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1344,7 +1344,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;
-- 
2.4.3

[Qemu-devel] [PULL 2/5] migration: rename qemu_savevm_state_cancel

[Juan Quintela]

From: Liang Li <liang.z.li@intel.com>

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>
Reviewed-by: Juan Quintela <quintela@redhat.com>al3
Reviewed-by: Amit Shah <amit.shah@redhat.com>al3
Signed-off-by: Juan Quintela <quintela@redhat.com>al3
---
 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 c439975..5cb0f05 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -89,7 +89,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 d5a7304..f99d3ea 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1090,7 +1090,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 dbcc39a..ae8fdda 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -902,11 +902,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);
@@ -943,7 +943,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 72136b9..17fbddf 100644
--- a/trace-events
+++ b/trace-events
@@ -1211,7 +1211,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"
-- 
2.4.3

[Qemu-devel] [PULL 3/5] migration: rename cancel to cleanup in SaveVMHandles

[Juan Quintela]

From: Liang Li <liang.z.li@intel.com>

'cleanup' seems more appropriate than 'cancel'.

Signed-off-by: Liang Li <liang.z.li@intel.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>al3
Reviewed-by: Amit Shah <amit.shah@redhat.com>al3
Signed-off-by: Juan Quintela <quintela@redhat.com>al3
---
 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 9a65522..d173b56 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 8401597..ecfe005 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -884,7 +884,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 25e9eeb..0a51473 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1685,7 +1685,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 ae8fdda..e05158d 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -908,8 +908,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);
         }
     }
 }
-- 
2.4.3

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

[Juan Quintela]

From: Liang Li <liang.z.li@intel.com>

Just clean up code, no behavior change.

Signed-off-by: Liang Li <liang.z.li@intel.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>al3
Reviewed-by: Amit Shah <amit.shah@redhat.com>al3
Signed-off-by: Juan Quintela <quintela@redhat.com>al3
---
 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 ecfe005..cf9d9f8 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;
@@ -884,7 +879,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 0a51473..df3df9e 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1100,7 +1100,7 @@ static void migration_bitmap_free(struct BitmapRcu *bmap)
     g_free(bmap);
 }

-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
@@ -1124,11 +1124,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;
@@ -1685,7 +1680,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)
-- 
2.4.3

[Qemu-devel] [PULL 5/5] migration: fix analyze-migration.py script

[Juan Quintela]

From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Commit 61964 "Add configuration section" broke the analyze-migration.py script
which terminates due to the unrecognised section. Fix the script by parsing
the contents of the configuration section directly into a new
ConfigurationSection object (although nothing is done with it yet).

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Juan Quintela <quintela@redhat.com>al3
Signed-off-by: Juan Quintela <quintela@redhat.com>al3
---
 scripts/analyze-migration.py | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/scripts/analyze-migration.py b/scripts/analyze-migration.py
index f6894be..1455387 100755
--- a/scripts/analyze-migration.py
+++ b/scripts/analyze-migration.py
@@ -252,6 +252,15 @@ class HTABSection(object):
     def getDict(self):
         return ""

+
+class ConfigurationSection(object):
+    def __init__(self, file):
+        self.file = file
+
+    def read(self):
+        name_len = self.file.read32()
+        name = self.file.readstr(len = name_len)
+
 class VMSDFieldGeneric(object):
     def __init__(self, desc, file):
         self.file = file
@@ -474,6 +483,7 @@ class MigrationDump(object):
     QEMU_VM_SECTION_FULL  = 0x04
     QEMU_VM_SUBSECTION    = 0x05
     QEMU_VM_VMDESCRIPTION = 0x06
+    QEMU_VM_CONFIGURATION = 0x07
     QEMU_VM_SECTION_FOOTER= 0x7e

     def __init__(self, filename):
@@ -514,6 +524,9 @@ class MigrationDump(object):
             section_type = file.read8()
             if section_type == self.QEMU_VM_EOF:
                 break
+            elif section_type == self.QEMU_VM_CONFIGURATION:
+                section = ConfigurationSection(file)
+                section.read()
             elif section_type == self.QEMU_VM_SECTION_START or section_type == self.QEMU_VM_SECTION_FULL:
                 section_id = file.read32()
                 name = file.readstr()
-- 
2.4.3

Re: [Qemu-devel] [PULL 0/5] Migration pull request

[Peter Maydell]

On 4 November 2015 at 12:48, Juan Quintela <quintela@redhat.com> wrote:
> Hi
>
> This is the pull request for migration, it includes:
>
> - Liang Li series for improving migration downtime
> - Mark Cave-Ayland fix to analyze-migration script
>
> Please, apply
>
> The following changes since commit 79cf9fad341e6e7bd6b55395b71d5c5727d7f5b0:
>
>   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20151103' into staging (2015-11-03 14:54:40 +0000)
>
> are available in the git repository at:
>
>   git://github.com/juanquintela/qemu.git tags/migration/20151104
>
> for you to fetch changes up to 96e5c9bc77acef8b7b56cbe23a8a2611feff9e34:
>
>   migration: fix analyze-migration.py script (2015-11-04 13:40:13 +0100)
>
> ----------------------------------------------------------------
> migration/next for 20151104
>
> ----------------------------------------------------------------

Applied, thanks.

-- PMM