[PATCH V2 0/8] Optimize VM's downtime while do checkpoint in COLO

[PATCH V2 0/8] Optimize VM's downtime while do checkpoint in COLO

[zhanghailiang]

This series try to  tries to reduce VM's pause time while do checkpoint in COLO state.

Here, we use two methods to reduce the downtime during COLO stage:
The first one is to reduce the time of backup PVM's memory into cache,
Instread of doing this once time backup all PVM's memory when VM is stopped, we backup
them during the live migration time.

Secondly, we reduced the total number of dirty pages while do checkpoint with VM been paused,
instead of sending all dirty pages while VM been pause, it sends part of dirty pages
during the gap time of two checkpoints when SVM and PVM are running.

V1 -> V2:
- Fix tested problem found by Daniel Cho
- Fix a degradation after rebase to master (first patch)

Please review, thanks.

Hailiang Zhang (8):
  migration: fix COLO broken caused by a previous commit
  migration/colo: wrap incoming checkpoint process into new helper
  savevm: Don't call colo_init_ram_cache twice
  COLO: Optimize memory back-up process
  ram/colo: only record bitmap of dirty pages in COLO stage
  migration: recognize COLO as part of activating process
  COLO: Migrate dirty pages during the gap of checkpointing
  migration/colo: Only flush ram cache while do checkpoint

 migration/colo.c       | 337 +++++++++++++++++++++++++----------------
 migration/migration.c  |   7 +-
 migration/migration.h  |   1 +
 migration/ram.c        |  78 +++++++---
 migration/ram.h        |   2 +
 migration/trace-events |   1 +
 qapi/migration.json    |   4 +-
 7 files changed, 269 insertions(+), 161 deletions(-)

--
2.21.0

[PATCH V2 1/8] migration: fix COLO broken caused by a previous commit

[zhanghailiang]

This commit "migration: Create migration_is_running()" broke
COLO. Becuase there is a process broken by this commit.

colo_process_checkpoint
 ->colo_do_checkpoint_transaction
   ->migrate_set_block_enabled
     ->qmp_migrate_set_capabilities

It can be fixed by make COLO process as an exception,
Maybe we need a better way to fix it.

Cc: Juan Quintela <quintela@redhat.com>
Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
---
 migration/migration.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/migration/migration.c b/migration/migration.c
index 8fb68795dc..06d1ff9d56 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -865,7 +865,6 @@ bool migration_is_running(int state)
     case MIGRATION_STATUS_DEVICE:
     case MIGRATION_STATUS_WAIT_UNPLUG:
     case MIGRATION_STATUS_CANCELLING:
-    case MIGRATION_STATUS_COLO:
         return true;
 
     default:
-- 
2.21.0

[PATCH V2 2/8] migration/colo: wrap incoming checkpoint process into new helper

[zhanghailiang]

Split checkpoint incoming process into a helper.

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/colo.c | 260 ++++++++++++++++++++++++-----------------------
 1 file changed, 133 insertions(+), 127 deletions(-)

diff --git a/migration/colo.c b/migration/colo.c
index 2c88aa57a2..93c5a452fb 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -664,13 +664,138 @@ void migrate_start_colo_process(MigrationState *s)
     qemu_mutex_lock_iothread();
 }
 
-static void colo_wait_handle_message(QEMUFile *f, int *checkpoint_request,
-                                     Error **errp)
+static void colo_incoming_process_checkpoint(MigrationIncomingState *mis,
+                      QEMUFile *fb, QIOChannelBuffer *bioc, Error **errp)
+{
+    uint64_t total_size;
+    uint64_t value;
+    Error *local_err = NULL;
+    int ret;
+
+    qemu_mutex_lock_iothread();
+    vm_stop_force_state(RUN_STATE_COLO);
+    trace_colo_vm_state_change("run", "stop");
+    qemu_mutex_unlock_iothread();
+
+    /* FIXME: This is unnecessary for periodic checkpoint mode */
+    colo_send_message(mis->to_src_file, COLO_MESSAGE_CHECKPOINT_REPLY,
+                 &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    colo_receive_check_message(mis->from_src_file,
+                       COLO_MESSAGE_VMSTATE_SEND, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    qemu_mutex_lock_iothread();
+    cpu_synchronize_all_pre_loadvm();
+    ret = qemu_loadvm_state_main(mis->from_src_file, mis);
+    qemu_mutex_unlock_iothread();
+
+    if (ret < 0) {
+        error_setg(errp, "Load VM's live state (ram) error");
+        return;
+    }
+
+    value = colo_receive_message_value(mis->from_src_file,
+                             COLO_MESSAGE_VMSTATE_SIZE, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    /*
+     * Read VM device state data into channel buffer,
+     * It's better to re-use the memory allocated.
+     * Here we need to handle the channel buffer directly.
+     */
+    if (value > bioc->capacity) {
+        bioc->capacity = value;
+        bioc->data = g_realloc(bioc->data, bioc->capacity);
+    }
+    total_size = qemu_get_buffer(mis->from_src_file, bioc->data, value);
+    if (total_size != value) {
+        error_setg(errp, "Got %" PRIu64 " VMState data, less than expected"
+                    " %" PRIu64, total_size, value);
+        return;
+    }
+    bioc->usage = total_size;
+    qio_channel_io_seek(QIO_CHANNEL(bioc), 0, 0, NULL);
+
+    colo_send_message(mis->to_src_file, COLO_MESSAGE_VMSTATE_RECEIVED,
+                 &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    qemu_mutex_lock_iothread();
+    vmstate_loading = true;
+    ret = qemu_load_device_state(fb);
+    if (ret < 0) {
+        error_setg(errp, "COLO: load device state failed");
+        qemu_mutex_unlock_iothread();
+        return;
+    }
+
+#ifdef CONFIG_REPLICATION
+    replication_get_error_all(&local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        qemu_mutex_unlock_iothread();
+        return;
+    }
+
+    /* discard colo disk buffer */
+    replication_do_checkpoint_all(&local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        qemu_mutex_unlock_iothread();
+        return;
+    }
+#else
+    abort();
+#endif
+    /* Notify all filters of all NIC to do checkpoint */
+    colo_notify_filters_event(COLO_EVENT_CHECKPOINT, &local_err);
+
+    if (local_err) {
+        error_propagate(errp, local_err);
+        qemu_mutex_unlock_iothread();
+        return;
+    }
+
+    vmstate_loading = false;
+    vm_start();
+    trace_colo_vm_state_change("stop", "run");
+    qemu_mutex_unlock_iothread();
+
+    if (failover_get_state() == FAILOVER_STATUS_RELAUNCH) {
+        failover_set_state(FAILOVER_STATUS_RELAUNCH,
+                        FAILOVER_STATUS_NONE);
+        failover_request_active(NULL);
+        return;
+    }
+
+    colo_send_message(mis->to_src_file, COLO_MESSAGE_VMSTATE_LOADED,
+                 &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+    }
+}
+
+static void colo_wait_handle_message(MigrationIncomingState *mis,
+                QEMUFile *fb, QIOChannelBuffer *bioc, Error **errp)
 {
     COLOMessage msg;
     Error *local_err = NULL;
 
-    msg = colo_receive_message(f, &local_err);
+    msg = colo_receive_message(mis->from_src_file, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         return;
@@ -678,10 +803,9 @@ static void colo_wait_handle_message(QEMUFile *f, int *checkpoint_request,
 
     switch (msg) {
     case COLO_MESSAGE_CHECKPOINT_REQUEST:
-        *checkpoint_request = 1;
+        colo_incoming_process_checkpoint(mis, fb, bioc, errp);
         break;
     default:
-        *checkpoint_request = 0;
         error_setg(errp, "Got unknown COLO message: %d", msg);
         break;
     }
@@ -692,10 +816,7 @@ void *colo_process_incoming_thread(void *opaque)
     MigrationIncomingState *mis = opaque;
     QEMUFile *fb = NULL;
     QIOChannelBuffer *bioc = NULL; /* Cache incoming device state */
-    uint64_t total_size;
-    uint64_t value;
     Error *local_err = NULL;
-    int ret;
 
     rcu_register_thread();
     qemu_sem_init(&mis->colo_incoming_sem, 0);
@@ -749,134 +870,19 @@ void *colo_process_incoming_thread(void *opaque)
     }
 
     while (mis->state == MIGRATION_STATUS_COLO) {
-        int request = 0;
-
-        colo_wait_handle_message(mis->from_src_file, &request, &local_err);
+        colo_wait_handle_message(mis, fb, bioc, &local_err);
         if (local_err) {
-            goto out;
+            error_report_err(local_err);
+            break;
         }
-        assert(request);
         if (failover_get_state() != FAILOVER_STATUS_NONE) {
             error_report("failover request");
-            goto out;
-        }
-
-        qemu_mutex_lock_iothread();
-        vm_stop_force_state(RUN_STATE_COLO);
-        trace_colo_vm_state_change("run", "stop");
-        qemu_mutex_unlock_iothread();
-
-        /* FIXME: This is unnecessary for periodic checkpoint mode */
-        colo_send_message(mis->to_src_file, COLO_MESSAGE_CHECKPOINT_REPLY,
-                     &local_err);
-        if (local_err) {
-            goto out;
-        }
-
-        colo_receive_check_message(mis->from_src_file,
-                           COLO_MESSAGE_VMSTATE_SEND, &local_err);
-        if (local_err) {
-            goto out;
-        }
-
-        qemu_mutex_lock_iothread();
-        cpu_synchronize_all_pre_loadvm();
-        ret = qemu_loadvm_state_main(mis->from_src_file, mis);
-        qemu_mutex_unlock_iothread();
-
-        if (ret < 0) {
-            error_report("Load VM's live state (ram) error");
-            goto out;
-        }
-
-        value = colo_receive_message_value(mis->from_src_file,
-                                 COLO_MESSAGE_VMSTATE_SIZE, &local_err);
-        if (local_err) {
-            goto out;
-        }
-
-        /*
-         * Read VM device state data into channel buffer,
-         * It's better to re-use the memory allocated.
-         * Here we need to handle the channel buffer directly.
-         */
-        if (value > bioc->capacity) {
-            bioc->capacity = value;
-            bioc->data = g_realloc(bioc->data, bioc->capacity);
-        }
-        total_size = qemu_get_buffer(mis->from_src_file, bioc->data, value);
-        if (total_size != value) {
-            error_report("Got %" PRIu64 " VMState data, less than expected"
-                        " %" PRIu64, total_size, value);
-            goto out;
-        }
-        bioc->usage = total_size;
-        qio_channel_io_seek(QIO_CHANNEL(bioc), 0, 0, NULL);
-
-        colo_send_message(mis->to_src_file, COLO_MESSAGE_VMSTATE_RECEIVED,
-                     &local_err);
-        if (local_err) {
-            goto out;
-        }
-
-        qemu_mutex_lock_iothread();
-        vmstate_loading = true;
-        ret = qemu_load_device_state(fb);
-        if (ret < 0) {
-            error_report("COLO: load device state failed");
-            qemu_mutex_unlock_iothread();
-            goto out;
-        }
-
-#ifdef CONFIG_REPLICATION
-        replication_get_error_all(&local_err);
-        if (local_err) {
-            qemu_mutex_unlock_iothread();
-            goto out;
-        }
-
-        /* discard colo disk buffer */
-        replication_do_checkpoint_all(&local_err);
-        if (local_err) {
-            qemu_mutex_unlock_iothread();
-            goto out;
-        }
-#else
-        abort();
-#endif
-        /* Notify all filters of all NIC to do checkpoint */
-        colo_notify_filters_event(COLO_EVENT_CHECKPOINT, &local_err);
-
-        if (local_err) {
-            qemu_mutex_unlock_iothread();
-            goto out;
-        }
-
-        vmstate_loading = false;
-        vm_start();
-        trace_colo_vm_state_change("stop", "run");
-        qemu_mutex_unlock_iothread();
-
-        if (failover_get_state() == FAILOVER_STATUS_RELAUNCH) {
-            failover_set_state(FAILOVER_STATUS_RELAUNCH,
-                            FAILOVER_STATUS_NONE);
-            failover_request_active(NULL);
-            goto out;
-        }
-
-        colo_send_message(mis->to_src_file, COLO_MESSAGE_VMSTATE_LOADED,
-                     &local_err);
-        if (local_err) {
-            goto out;
+            break;
         }
     }
 
 out:
     vmstate_loading = false;
-    /* Throw the unreported error message after exited from loop */
-    if (local_err) {
-        error_report_err(local_err);
-    }
 
     /*
      * There are only two reasons we can get here, some error happened
-- 
2.21.0

[PATCH V2 3/8] savevm: Don't call colo_init_ram_cache twice

[zhanghailiang]

This helper has been called twice which is wrong.
Left the one where called while get COLO enable message
from source side.

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
---
 migration/migration.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 06d1ff9d56..e8c62c6e2e 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -484,11 +484,6 @@ static void process_incoming_migration_co(void *opaque)
             goto fail;
         }
 
-        if (colo_init_ram_cache() < 0) {
-            error_report("Init ram cache failed");
-            goto fail;
-        }
-
         qemu_thread_create(&mis->colo_incoming_thread, "COLO incoming",
              colo_process_incoming_thread, mis, QEMU_THREAD_JOINABLE);
         mis->have_colo_incoming_thread = true;
-- 
2.21.0

[PATCH V2 4/8] COLO: Optimize memory back-up process

[zhanghailiang]

This patch will reduce the downtime of VM for the initial process,
Privously, we copied all these memory in preparing stage of COLO
while we need to stop VM, which is a time-consuming process.
Here we optimize it by a trick, back-up every page while in migration
process while COLO is enabled, though it affects the speed of the
migration, but it obviously reduce the downtime of back-up all SVM'S
memory in COLO preparing stage.

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
---
 migration/colo.c |  3 +++
 migration/ram.c  | 68 +++++++++++++++++++++++++++++++++++-------------
 migration/ram.h  |  1 +
 3 files changed, 54 insertions(+), 18 deletions(-)

diff --git a/migration/colo.c b/migration/colo.c
index 93c5a452fb..44942c4e23 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -26,6 +26,7 @@
 #include "qemu/main-loop.h"
 #include "qemu/rcu.h"
 #include "migration/failover.h"
+#include "migration/ram.h"
 #ifdef CONFIG_REPLICATION
 #include "replication.h"
 #endif
@@ -845,6 +846,8 @@ void *colo_process_incoming_thread(void *opaque)
      */
     qemu_file_set_blocking(mis->from_src_file, true);
 
+    colo_incoming_start_dirty_log();
+
     bioc = qio_channel_buffer_new(COLO_BUFFER_BASE_SIZE);
     fb = qemu_fopen_channel_input(QIO_CHANNEL(bioc));
     object_unref(OBJECT(bioc));
diff --git a/migration/ram.c b/migration/ram.c
index ed23ed1c7c..ebf9e6ba51 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2277,6 +2277,7 @@ static void ram_list_init_bitmaps(void)
              * dirty_memory[DIRTY_MEMORY_MIGRATION] don't include the whole
              * guest memory.
              */
+
             block->bmap = bitmap_new(pages);
             bitmap_set(block->bmap, 0, pages);
             block->clear_bmap_shift = shift;
@@ -2986,7 +2987,6 @@ int colo_init_ram_cache(void)
                 }
                 return -errno;
             }
-            memcpy(block->colo_cache, block->host, block->used_length);
         }
     }
 
@@ -3000,19 +3000,36 @@ int colo_init_ram_cache(void)
 
         RAMBLOCK_FOREACH_NOT_IGNORED(block) {
             unsigned long pages = block->max_length >> TARGET_PAGE_BITS;
-
             block->bmap = bitmap_new(pages);
-            bitmap_set(block->bmap, 0, pages);
         }
     }
-    ram_state = g_new0(RAMState, 1);
-    ram_state->migration_dirty_pages = 0;
-    qemu_mutex_init(&ram_state->bitmap_mutex);
-    memory_global_dirty_log_start();
 
+    ram_state_init(&ram_state);
     return 0;
 }
 
+/* TODO: duplicated with ram_init_bitmaps */
+void colo_incoming_start_dirty_log(void)
+{
+    RAMBlock *block = NULL;
+    /* For memory_global_dirty_log_start below. */
+    qemu_mutex_lock_iothread();
+    qemu_mutex_lock_ramlist();
+
+    memory_global_dirty_log_sync();
+    WITH_RCU_READ_LOCK_GUARD() {
+        RAMBLOCK_FOREACH_NOT_IGNORED(block) {
+            ramblock_sync_dirty_bitmap(ram_state, block);
+            /* Discard this dirty bitmap record */
+            bitmap_zero(block->bmap, block->max_length >> TARGET_PAGE_BITS);
+        }
+        memory_global_dirty_log_start();
+    }
+    ram_state->migration_dirty_pages = 0;
+    qemu_mutex_unlock_ramlist();
+    qemu_mutex_unlock_iothread();
+}
+
 /* It is need to hold the global lock to call this helper */
 void colo_release_ram_cache(void)
 {
@@ -3032,9 +3049,7 @@ void colo_release_ram_cache(void)
             }
         }
     }
-    qemu_mutex_destroy(&ram_state->bitmap_mutex);
-    g_free(ram_state);
-    ram_state = NULL;
+    ram_state_cleanup(&ram_state);
 }
 
 /**
@@ -3302,7 +3317,6 @@ static void colo_flush_ram_cache(void)
             ramblock_sync_dirty_bitmap(ram_state, block);
         }
     }
-
     trace_colo_flush_ram_cache_begin(ram_state->migration_dirty_pages);
     WITH_RCU_READ_LOCK_GUARD() {
         block = QLIST_FIRST_RCU(&ram_list.blocks);
@@ -3348,7 +3362,7 @@ static int ram_load_precopy(QEMUFile *f)
 
     while (!ret && !(flags & RAM_SAVE_FLAG_EOS)) {
         ram_addr_t addr, total_ram_bytes;
-        void *host = NULL;
+        void *host = NULL, *host_bak = NULL;
         uint8_t ch;
 
         /*
@@ -3379,20 +3393,35 @@ static int ram_load_precopy(QEMUFile *f)
                      RAM_SAVE_FLAG_COMPRESS_PAGE | RAM_SAVE_FLAG_XBZRLE)) {
             RAMBlock *block = ram_block_from_stream(f, flags);
 
+            host = host_from_ram_block_offset(block, addr);
             /*
-             * After going into COLO, we should load the Page into colo_cache.
+             * After going into COLO stage, we should not load the page
+             * into SVM's memory diretly, we put them into colo_cache firstly.
+             * NOTE: We need to keep a copy of SVM's ram in colo_cache.
+             * Privously, we copied all these memory in preparing stage of COLO
+             * while we need to stop VM, which is a time-consuming process.
+             * Here we optimize it by a trick, back-up every page while in
+             * migration process while COLO is enabled, though it affects the
+             * speed of the migration, but it obviously reduce the downtime of
+             * back-up all SVM'S memory in COLO preparing stage.
              */
-            if (migration_incoming_in_colo_state()) {
-                host = colo_cache_from_block_offset(block, addr);
-            } else {
-                host = host_from_ram_block_offset(block, addr);
+            if (migration_incoming_colo_enabled()) {
+                if (migration_incoming_in_colo_state()) {
+                    /* In COLO stage, put all pages into cache temporarily */
+                    host = colo_cache_from_block_offset(block, addr);
+                } else {
+                   /*
+                    * In migration stage but before COLO stage,
+                    * Put all pages into both cache and SVM's memory.
+                    */
+                    host_bak = colo_cache_from_block_offset(block, addr);
+                }
             }
             if (!host) {
                 error_report("Illegal RAM offset " RAM_ADDR_FMT, addr);
                 ret = -EINVAL;
                 break;
             }
-
             if (!migration_incoming_in_colo_state()) {
                 ramblock_recv_bitmap_set(block, host);
             }
@@ -3506,6 +3535,9 @@ static int ram_load_precopy(QEMUFile *f)
         if (!ret) {
             ret = qemu_file_get_error(f);
         }
+        if (!ret && host_bak) {
+            memcpy(host_bak, host, TARGET_PAGE_SIZE);
+        }
     }
 
     ret |= wait_for_decompress_done();
diff --git a/migration/ram.h b/migration/ram.h
index a553d40751..5ceaff7cb4 100644
--- a/migration/ram.h
+++ b/migration/ram.h
@@ -66,5 +66,6 @@ int ram_dirty_bitmap_reload(MigrationState *s, RAMBlock *rb);
 /* ram cache */
 int colo_init_ram_cache(void);
 void colo_release_ram_cache(void);
+void colo_incoming_start_dirty_log(void);
 
 #endif
-- 
2.21.0

[PATCH V2 5/8] ram/colo: only record bitmap of dirty pages in COLO stage

[zhanghailiang]

It is only need to record bitmap of dirty pages while goes
into COLO stage.

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
---
 migration/ram.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index ebf9e6ba51..1b3f423351 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2735,7 +2735,7 @@ static inline void *host_from_ram_block_offset(RAMBlock *block,
 }
 
 static inline void *colo_cache_from_block_offset(RAMBlock *block,
-                                                 ram_addr_t offset)
+                             ram_addr_t offset, bool record_bitmap)
 {
     if (!offset_in_ramblock(block, offset)) {
         return NULL;
@@ -2751,7 +2751,8 @@ static inline void *colo_cache_from_block_offset(RAMBlock *block,
     * It help us to decide which pages in ram cache should be flushed
     * into VM's RAM later.
     */
-    if (!test_and_set_bit(offset >> TARGET_PAGE_BITS, block->bmap)) {
+    if (record_bitmap &&
+        !test_and_set_bit(offset >> TARGET_PAGE_BITS, block->bmap)) {
         ram_state->migration_dirty_pages++;
     }
     return block->colo_cache + offset;
@@ -3408,13 +3409,13 @@ static int ram_load_precopy(QEMUFile *f)
             if (migration_incoming_colo_enabled()) {
                 if (migration_incoming_in_colo_state()) {
                     /* In COLO stage, put all pages into cache temporarily */
-                    host = colo_cache_from_block_offset(block, addr);
+                    host = colo_cache_from_block_offset(block, addr, true);
                 } else {
                    /*
                     * In migration stage but before COLO stage,
                     * Put all pages into both cache and SVM's memory.
                     */
-                    host_bak = colo_cache_from_block_offset(block, addr);
+                    host_bak = colo_cache_from_block_offset(block, addr, false);
                 }
             }
             if (!host) {
-- 
2.21.0

[PATCH V2 6/8] migration: recognize COLO as part of activating process

[zhanghailiang]

We will migrate parts of dirty pages backgroud lively during the gap time
of two checkpoints, without this modification, it will not work
because ram_save_iterate() will check it before send RAM_SAVE_FLAG_EOS
at the end of it.

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
---
 migration/migration.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/migration/migration.c b/migration/migration.c
index e8c62c6e2e..f71c337600 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -840,6 +840,7 @@ bool migration_is_setup_or_active(int state)
     case MIGRATION_STATUS_PRE_SWITCHOVER:
     case MIGRATION_STATUS_DEVICE:
     case MIGRATION_STATUS_WAIT_UNPLUG:
+    case MIGRATION_STATUS_COLO:
         return true;
 
     default:
-- 
2.21.0

[PATCH V2 7/8] COLO: Migrate dirty pages during the gap of checkpointing

[zhanghailiang]

We can migrate some dirty pages during the gap of checkpointing,
by this way, we can reduce the amount of ram migrated during checkpointing.

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
---
 migration/colo.c       | 73 ++++++++++++++++++++++++++++++++++++++++--
 migration/migration.h  |  1 +
 migration/trace-events |  1 +
 qapi/migration.json    |  4 ++-
 4 files changed, 75 insertions(+), 4 deletions(-)

diff --git a/migration/colo.c b/migration/colo.c
index 44942c4e23..c36d94072f 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -47,6 +47,13 @@ static COLOMode last_colo_mode;
 
 #define COLO_BUFFER_BASE_SIZE (4 * 1024 * 1024)
 
+#define DEFAULT_RAM_PENDING_CHECK 1000
+
+/* should be calculated by bandwidth and max downtime ? */
+#define THRESHOLD_PENDING_SIZE (100 * 1024 * 1024UL)
+
+static int checkpoint_request;
+
 bool migration_in_colo_state(void)
 {
     MigrationState *s = migrate_get_current();
@@ -517,6 +524,20 @@ static void colo_compare_notify_checkpoint(Notifier *notifier, void *data)
     colo_checkpoint_notify(data);
 }
 
+static bool colo_need_migrate_ram_background(MigrationState *s)
+{
+    uint64_t pending_size, pend_pre, pend_compat, pend_post;
+    int64_t max_size = THRESHOLD_PENDING_SIZE;
+
+    qemu_savevm_state_pending(s->to_dst_file, max_size, &pend_pre,
+                              &pend_compat, &pend_post);
+    pending_size = pend_pre + pend_compat + pend_post;
+
+    trace_colo_need_migrate_ram_background(pending_size);
+    return (pending_size >= max_size);
+}
+
+
 static void colo_process_checkpoint(MigrationState *s)
 {
     QIOChannelBuffer *bioc;
@@ -572,6 +593,8 @@ static void colo_process_checkpoint(MigrationState *s)
 
     timer_mod(s->colo_delay_timer,
             current_time + s->parameters.x_checkpoint_delay);
+    timer_mod(s->pending_ram_check_timer,
+        current_time + DEFAULT_RAM_PENDING_CHECK);
 
     while (s->state == MIGRATION_STATUS_COLO) {
         if (failover_get_state() != FAILOVER_STATUS_NONE) {
@@ -584,9 +607,30 @@ static void colo_process_checkpoint(MigrationState *s)
         if (s->state != MIGRATION_STATUS_COLO) {
             goto out;
         }
-        ret = colo_do_checkpoint_transaction(s, bioc, fb);
-        if (ret < 0) {
-            goto out;
+        if (atomic_xchg(&checkpoint_request, 0)) {
+            /* start a colo checkpoint */
+            ret = colo_do_checkpoint_transaction(s, bioc, fb);
+            if (ret < 0) {
+                goto out;
+            }
+        } else {
+            if (colo_need_migrate_ram_background(s)) {
+                colo_send_message(s->to_dst_file,
+                                  COLO_MESSAGE_MIGRATE_RAM_BACKGROUND,
+                                  &local_err);
+                if (local_err) {
+                    goto out;
+                }
+
+                qemu_savevm_state_iterate(s->to_dst_file, false);
+                qemu_put_byte(s->to_dst_file, QEMU_VM_EOF);
+                ret = qemu_file_get_error(s->to_dst_file);
+                if (ret < 0) {
+                    error_setg_errno(&local_err, -ret,
+                        "Failed to send dirty pages backgroud");
+                    goto out;
+                }
+            }
         }
     }
 
@@ -627,6 +671,8 @@ out:
     colo_compare_unregister_notifier(&packets_compare_notifier);
     timer_del(s->colo_delay_timer);
     timer_free(s->colo_delay_timer);
+    timer_del(s->pending_ram_check_timer);
+    timer_free(s->pending_ram_check_timer);
     qemu_sem_destroy(&s->colo_checkpoint_sem);
 
     /*
@@ -644,6 +690,7 @@ void colo_checkpoint_notify(void *opaque)
     MigrationState *s = opaque;
     int64_t next_notify_time;
 
+    atomic_inc(&checkpoint_request);
     qemu_sem_post(&s->colo_checkpoint_sem);
     s->colo_checkpoint_time = qemu_clock_get_ms(QEMU_CLOCK_HOST);
     next_notify_time = s->colo_checkpoint_time +
@@ -651,6 +698,19 @@ void colo_checkpoint_notify(void *opaque)
     timer_mod(s->colo_delay_timer, next_notify_time);
 }
 
+static void colo_pending_ram_check_notify(void *opaque)
+{
+    int64_t next_notify_time;
+    MigrationState *s = opaque;
+
+    if (migration_in_colo_state()) {
+        next_notify_time = DEFAULT_RAM_PENDING_CHECK +
+                           qemu_clock_get_ms(QEMU_CLOCK_HOST);
+        timer_mod(s->pending_ram_check_timer, next_notify_time);
+        qemu_sem_post(&s->colo_checkpoint_sem);
+    }
+}
+
 void migrate_start_colo_process(MigrationState *s)
 {
     qemu_mutex_unlock_iothread();
@@ -658,6 +718,8 @@ void migrate_start_colo_process(MigrationState *s)
     s->colo_delay_timer =  timer_new_ms(QEMU_CLOCK_HOST,
                                 colo_checkpoint_notify, s);
 
+    s->pending_ram_check_timer = timer_new_ms(QEMU_CLOCK_HOST,
+                                colo_pending_ram_check_notify, s);
     qemu_sem_init(&s->colo_exit_sem, 0);
     migrate_set_state(&s->state, MIGRATION_STATUS_ACTIVE,
                       MIGRATION_STATUS_COLO);
@@ -806,6 +868,11 @@ static void colo_wait_handle_message(MigrationIncomingState *mis,
     case COLO_MESSAGE_CHECKPOINT_REQUEST:
         colo_incoming_process_checkpoint(mis, fb, bioc, errp);
         break;
+    case COLO_MESSAGE_MIGRATE_RAM_BACKGROUND:
+        if (qemu_loadvm_state_main(mis->from_src_file, mis) < 0) {
+            error_setg(errp, "Load ram background failed");
+        }
+        break;
     default:
         error_setg(errp, "Got unknown COLO message: %d", msg);
         break;
diff --git a/migration/migration.h b/migration/migration.h
index 8473ddfc88..5355259789 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -219,6 +219,7 @@ struct MigrationState
     QemuSemaphore colo_checkpoint_sem;
     int64_t colo_checkpoint_time;
     QEMUTimer *colo_delay_timer;
+    QEMUTimer *pending_ram_check_timer;
 
     /* The first error that has occurred.
        We used the mutex to be able to return the 1st error message */
diff --git a/migration/trace-events b/migration/trace-events
index 4ab0a503d2..f2ed0c8645 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -295,6 +295,7 @@ migration_tls_incoming_handshake_complete(void) ""
 colo_vm_state_change(const char *old, const char *new) "Change '%s' => '%s'"
 colo_send_message(const char *msg) "Send '%s' message"
 colo_receive_message(const char *msg) "Receive '%s' message"
+colo_need_migrate_ram_background(uint64_t pending_size) "Pending 0x%" PRIx64 " dirty ram"
 
 # colo-failover.c
 colo_failover_set_state(const char *new_state) "new state %s"
diff --git a/qapi/migration.json b/qapi/migration.json
index 52f3429969..73445f1978 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -977,12 +977,14 @@
 #
 # @vmstate-loaded: VM's state has been loaded by SVM.
 #
+# @migrate-ram-background: Send some dirty pages during the gap of COLO checkpoint
+#
 # Since: 2.8
 ##
 { 'enum': 'COLOMessage',
   'data': [ 'checkpoint-ready', 'checkpoint-request', 'checkpoint-reply',
             'vmstate-send', 'vmstate-size', 'vmstate-received',
-            'vmstate-loaded' ] }
+            'vmstate-loaded', 'migrate-ram-background' ] }
 
 ##
 # @COLOMode:
-- 
2.21.0

[PATCH V2 8/8] migration/colo: Only flush ram cache while do checkpoint

[zhanghailiang]

After add migrating ram backgroud, we will call ram_load
for this process, but we should not flush ram cache during
this process. Move the flush action to the right place.

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
---
 migration/colo.c | 1 +
 migration/ram.c  | 5 +----
 migration/ram.h  | 1 +
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/migration/colo.c b/migration/colo.c
index c36d94072f..18df8289f8 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -799,6 +799,7 @@ static void colo_incoming_process_checkpoint(MigrationIncomingState *mis,
 
     qemu_mutex_lock_iothread();
     vmstate_loading = true;
+    colo_flush_ram_cache();
     ret = qemu_load_device_state(fb);
     if (ret < 0) {
         error_setg(errp, "COLO: load device state failed");
diff --git a/migration/ram.c b/migration/ram.c
index 1b3f423351..7bc841d14f 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3305,7 +3305,7 @@ static bool postcopy_is_running(void)
  * Flush content of RAM cache into SVM's memory.
  * Only flush the pages that be dirtied by PVM or SVM or both.
  */
-static void colo_flush_ram_cache(void)
+void colo_flush_ram_cache(void)
 {
     RAMBlock *block = NULL;
     void *dst_host;
@@ -3576,9 +3576,6 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
     }
     trace_ram_load_complete(ret, seq_iter);
 
-    if (!ret  && migration_incoming_in_colo_state()) {
-        colo_flush_ram_cache();
-    }
     return ret;
 }
 
diff --git a/migration/ram.h b/migration/ram.h
index 5ceaff7cb4..ae14341482 100644
--- a/migration/ram.h
+++ b/migration/ram.h
@@ -67,5 +67,6 @@ int ram_dirty_bitmap_reload(MigrationState *s, RAMBlock *rb);
 int colo_init_ram_cache(void);
 void colo_release_ram_cache(void);
 void colo_incoming_start_dirty_log(void);
+void colo_flush_ram_cache(void);
 
 #endif
-- 
2.21.0

Re: [PATCH V2 7/8] COLO: Migrate dirty pages during the gap of checkpointing

[Eric Blake]

On 2/24/20 12:54 AM, zhanghailiang wrote:
> We can migrate some dirty pages during the gap of checkpointing,
> by this way, we can reduce the amount of ram migrated during checkpointing.
> 
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> ---

> +++ b/qapi/migration.json
> @@ -977,12 +977,14 @@
>   #
>   # @vmstate-loaded: VM's state has been loaded by SVM.
>   #
> +# @migrate-ram-background: Send some dirty pages during the gap of COLO checkpoint

Missing a '(since 5.0)' tag.

> +#
>   # Since: 2.8
>   ##
>   { 'enum': 'COLOMessage',
>     'data': [ 'checkpoint-ready', 'checkpoint-request', 'checkpoint-reply',
>               'vmstate-send', 'vmstate-size', 'vmstate-received',
> -            'vmstate-loaded' ] }
> +            'vmstate-loaded', 'migrate-ram-background' ] }
>   
>   ##
>   # @COLOMode:
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

RE: [PATCH V2 7/8] COLO: Migrate dirty pages during the gap of checkpointing

[Zhanghailiang]

> -----Original Message-----
> From: Eric Blake [mailto:eblake@redhat.com]
> Sent: Monday, February 24, 2020 11:19 PM
> To: Zhanghailiang <zhang.zhanghailiang@huawei.com>;
> qemu-devel@nongnu.org
> Cc: danielcho@qnap.com; dgilbert@redhat.com; quintela@redhat.com
> Subject: Re: [PATCH V2 7/8] COLO: Migrate dirty pages during the gap of
> checkpointing
> 
> On 2/24/20 12:54 AM, zhanghailiang wrote:
> > We can migrate some dirty pages during the gap of checkpointing,
> > by this way, we can reduce the amount of ram migrated during
> checkpointing.
> >
> > Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> > ---
> 
> > +++ b/qapi/migration.json
> > @@ -977,12 +977,14 @@
> >   #
> >   # @vmstate-loaded: VM's state has been loaded by SVM.
> >   #
> > +# @migrate-ram-background: Send some dirty pages during the gap of
> COLO checkpoint
> 
> Missing a '(since 5.0)' tag.
> 

OK, will add this in next version, I forgot to modify it in this version which you reminded
In previous version. :(

> > +#
> >   # Since: 2.8
> >   ##
> >   { 'enum': 'COLOMessage',
> >     'data': [ 'checkpoint-ready', 'checkpoint-request', 'checkpoint-reply',
> >               'vmstate-send', 'vmstate-size', 'vmstate-received',
> > -            'vmstate-loaded' ] }
> > +            'vmstate-loaded', 'migrate-ram-background' ] }
> >
> >   ##
> >   # @COLOMode:
> >
> 
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org

Re: [PATCH V2 4/8] COLO: Optimize memory back-up process

[Daniel Cho]

Hi Hailiang,

With version 2, the code in migration/ram.c

+            if (migration_incoming_colo_enabled()) {
+                if (migration_incoming_in_colo_state()) {
+                    /* In COLO stage, put all pages into cache temporarily */
+                    host = colo_cache_from_block_offset(block, addr);
+                } else {
+                   /*
+                    * In migration stage but before COLO stage,
+                    * Put all pages into both cache and SVM's memory.
+                    */
+                    host_bak = colo_cache_from_block_offset(block, addr);
+                }
             }
             if (!host) {
                 error_report("Illegal RAM offset " RAM_ADDR_FMT, addr);
                 ret = -EINVAL;
                 break;
             }

host = colo_cache_from_block_offset(block, addr);
host_bak = colo_cache_from_block_offset(block, addr);
Does it cause the "if(!host)" will go break if the condition goes
"host_bak = colo_cache_from_block_offset(block, addr);" ?

Best regards,
Daniel Cho

zhanghailiang <zhang.zhanghailiang@huawei.com> 於 2020年2月24日 週一 下午2:55寫道：
>
> This patch will reduce the downtime of VM for the initial process,
> Privously, we copied all these memory in preparing stage of COLO
> while we need to stop VM, which is a time-consuming process.
> Here we optimize it by a trick, back-up every page while in migration
> process while COLO is enabled, though it affects the speed of the
> migration, but it obviously reduce the downtime of back-up all SVM'S
> memory in COLO preparing stage.
>
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> ---
>  migration/colo.c |  3 +++
>  migration/ram.c  | 68 +++++++++++++++++++++++++++++++++++-------------
>  migration/ram.h  |  1 +
>  3 files changed, 54 insertions(+), 18 deletions(-)
>
> diff --git a/migration/colo.c b/migration/colo.c
> index 93c5a452fb..44942c4e23 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -26,6 +26,7 @@
>  #include "qemu/main-loop.h"
>  #include "qemu/rcu.h"
>  #include "migration/failover.h"
> +#include "migration/ram.h"
>  #ifdef CONFIG_REPLICATION
>  #include "replication.h"
>  #endif
> @@ -845,6 +846,8 @@ void *colo_process_incoming_thread(void *opaque)
>       */
>      qemu_file_set_blocking(mis->from_src_file, true);
>
> +    colo_incoming_start_dirty_log();
> +
>      bioc = qio_channel_buffer_new(COLO_BUFFER_BASE_SIZE);
>      fb = qemu_fopen_channel_input(QIO_CHANNEL(bioc));
>      object_unref(OBJECT(bioc));
> diff --git a/migration/ram.c b/migration/ram.c
> index ed23ed1c7c..ebf9e6ba51 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -2277,6 +2277,7 @@ static void ram_list_init_bitmaps(void)
>               * dirty_memory[DIRTY_MEMORY_MIGRATION] don't include the whole
>               * guest memory.
>               */
> +
>              block->bmap = bitmap_new(pages);
>              bitmap_set(block->bmap, 0, pages);
>              block->clear_bmap_shift = shift;
> @@ -2986,7 +2987,6 @@ int colo_init_ram_cache(void)
>                  }
>                  return -errno;
>              }
> -            memcpy(block->colo_cache, block->host, block->used_length);
>          }
>      }
>
> @@ -3000,19 +3000,36 @@ int colo_init_ram_cache(void)
>
>          RAMBLOCK_FOREACH_NOT_IGNORED(block) {
>              unsigned long pages = block->max_length >> TARGET_PAGE_BITS;
> -
>              block->bmap = bitmap_new(pages);
> -            bitmap_set(block->bmap, 0, pages);
>          }
>      }
> -    ram_state = g_new0(RAMState, 1);
> -    ram_state->migration_dirty_pages = 0;
> -    qemu_mutex_init(&ram_state->bitmap_mutex);
> -    memory_global_dirty_log_start();
>
> +    ram_state_init(&ram_state);
>      return 0;
>  }
>
> +/* TODO: duplicated with ram_init_bitmaps */
> +void colo_incoming_start_dirty_log(void)
> +{
> +    RAMBlock *block = NULL;
> +    /* For memory_global_dirty_log_start below. */
> +    qemu_mutex_lock_iothread();
> +    qemu_mutex_lock_ramlist();
> +
> +    memory_global_dirty_log_sync();
> +    WITH_RCU_READ_LOCK_GUARD() {
> +        RAMBLOCK_FOREACH_NOT_IGNORED(block) {
> +            ramblock_sync_dirty_bitmap(ram_state, block);
> +            /* Discard this dirty bitmap record */
> +            bitmap_zero(block->bmap, block->max_length >> TARGET_PAGE_BITS);
> +        }
> +        memory_global_dirty_log_start();
> +    }
> +    ram_state->migration_dirty_pages = 0;
> +    qemu_mutex_unlock_ramlist();
> +    qemu_mutex_unlock_iothread();
> +}
> +
>  /* It is need to hold the global lock to call this helper */
>  void colo_release_ram_cache(void)
>  {
> @@ -3032,9 +3049,7 @@ void colo_release_ram_cache(void)
>              }
>          }
>      }
> -    qemu_mutex_destroy(&ram_state->bitmap_mutex);
> -    g_free(ram_state);
> -    ram_state = NULL;
> +    ram_state_cleanup(&ram_state);
>  }
>
>  /**
> @@ -3302,7 +3317,6 @@ static void colo_flush_ram_cache(void)
>              ramblock_sync_dirty_bitmap(ram_state, block);
>          }
>      }
> -
>      trace_colo_flush_ram_cache_begin(ram_state->migration_dirty_pages);
>      WITH_RCU_READ_LOCK_GUARD() {
>          block = QLIST_FIRST_RCU(&ram_list.blocks);
> @@ -3348,7 +3362,7 @@ static int ram_load_precopy(QEMUFile *f)
>
>      while (!ret && !(flags & RAM_SAVE_FLAG_EOS)) {
>          ram_addr_t addr, total_ram_bytes;
> -        void *host = NULL;
> +        void *host = NULL, *host_bak = NULL;
>          uint8_t ch;
>
>          /*
> @@ -3379,20 +3393,35 @@ static int ram_load_precopy(QEMUFile *f)
>                       RAM_SAVE_FLAG_COMPRESS_PAGE | RAM_SAVE_FLAG_XBZRLE)) {
>              RAMBlock *block = ram_block_from_stream(f, flags);
>
> +            host = host_from_ram_block_offset(block, addr);
>              /*
> -             * After going into COLO, we should load the Page into colo_cache.
> +             * After going into COLO stage, we should not load the page
> +             * into SVM's memory diretly, we put them into colo_cache firstly.
> +             * NOTE: We need to keep a copy of SVM's ram in colo_cache.
> +             * Privously, we copied all these memory in preparing stage of COLO
> +             * while we need to stop VM, which is a time-consuming process.
> +             * Here we optimize it by a trick, back-up every page while in
> +             * migration process while COLO is enabled, though it affects the
> +             * speed of the migration, but it obviously reduce the downtime of
> +             * back-up all SVM'S memory in COLO preparing stage.
>               */
> -            if (migration_incoming_in_colo_state()) {
> -                host = colo_cache_from_block_offset(block, addr);
> -            } else {
> -                host = host_from_ram_block_offset(block, addr);
> +            if (migration_incoming_colo_enabled()) {
> +                if (migration_incoming_in_colo_state()) {
> +                    /* In COLO stage, put all pages into cache temporarily */
> +                    host = colo_cache_from_block_offset(block, addr);
> +                } else {
> +                   /*
> +                    * In migration stage but before COLO stage,
> +                    * Put all pages into both cache and SVM's memory.
> +                    */
> +                    host_bak = colo_cache_from_block_offset(block, addr);
> +                }
>              }
>              if (!host) {
>                  error_report("Illegal RAM offset " RAM_ADDR_FMT, addr);
>                  ret = -EINVAL;
>                  break;
>              }
> -
>              if (!migration_incoming_in_colo_state()) {
>                  ramblock_recv_bitmap_set(block, host);
>              }
> @@ -3506,6 +3535,9 @@ static int ram_load_precopy(QEMUFile *f)
>          if (!ret) {
>              ret = qemu_file_get_error(f);
>          }
> +        if (!ret && host_bak) {
> +            memcpy(host_bak, host, TARGET_PAGE_SIZE);
> +        }
>      }
>
>      ret |= wait_for_decompress_done();
> diff --git a/migration/ram.h b/migration/ram.h
> index a553d40751..5ceaff7cb4 100644
> --- a/migration/ram.h
> +++ b/migration/ram.h
> @@ -66,5 +66,6 @@ int ram_dirty_bitmap_reload(MigrationState *s, RAMBlock *rb);
>  /* ram cache */
>  int colo_init_ram_cache(void);
>  void colo_release_ram_cache(void);
> +void colo_incoming_start_dirty_log(void);
>
>  #endif
> --
> 2.21.0
>
>

RE: [PATCH V2 4/8] COLO: Optimize memory back-up process

[Zhanghailiang]

Hi,


> -----Original Message-----
> From: Daniel Cho [mailto:danielcho@qnap.com]
> Sent: Tuesday, February 25, 2020 10:53 AM
> To: Zhanghailiang <zhang.zhanghailiang@huawei.com>
> Cc: qemu-devel@nongnu.org; quintela@redhat.com; Dr. David Alan Gilbert
> <dgilbert@redhat.com>
> Subject: Re: [PATCH V2 4/8] COLO: Optimize memory back-up process
> 
> Hi Hailiang,
> 
> With version 2, the code in migration/ram.c
> 
> +            if (migration_incoming_colo_enabled()) {
> +                if (migration_incoming_in_colo_state()) {
> +                    /* In COLO stage, put all pages into cache
> temporarily */
> +                    host = colo_cache_from_block_offset(block, addr);
> +                } else {
> +                   /*
> +                    * In migration stage but before COLO stage,
> +                    * Put all pages into both cache and SVM's memory.
> +                    */
> +                    host_bak = colo_cache_from_block_offset(block,
> addr);
> +                }
>              }
>              if (!host) {
>                  error_report("Illegal RAM offset " RAM_ADDR_FMT,
> addr);
>                  ret = -EINVAL;
>                  break;
>              }
> 
> host = colo_cache_from_block_offset(block, addr);
> host_bak = colo_cache_from_block_offset(block, addr);
> Does it cause the "if(!host)" will go break if the condition goes
> "host_bak = colo_cache_from_block_offset(block, addr);" ?
> 

That will not happen, you may have missed this parts.

@@ -3379,20 +3393,35 @@ static int ram_load_precopy(QEMUFile *f)
                      RAM_SAVE_FLAG_COMPRESS_PAGE | RAM_SAVE_FLAG_XBZRLE)) {
             RAMBlock *block = ram_block_from_stream(f, flags);
 
+            host = host_from_ram_block_offset(block, addr);
             /*

We have given host a value unconditionally.


> Best regards,
> Daniel Cho
> 
> zhanghailiang <zhang.zhanghailiang@huawei.com> 於 2020年2月24日 週
> 一 下午2:55寫道：
> >
> > This patch will reduce the downtime of VM for the initial process,
> > Privously, we copied all these memory in preparing stage of COLO
> > while we need to stop VM, which is a time-consuming process.
> > Here we optimize it by a trick, back-up every page while in migration
> > process while COLO is enabled, though it affects the speed of the
> > migration, but it obviously reduce the downtime of back-up all SVM'S
> > memory in COLO preparing stage.
> >
> > Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> > ---
> >  migration/colo.c |  3 +++
> >  migration/ram.c  | 68
> +++++++++++++++++++++++++++++++++++-------------
> >  migration/ram.h  |  1 +
> >  3 files changed, 54 insertions(+), 18 deletions(-)
> >
> > diff --git a/migration/colo.c b/migration/colo.c
> > index 93c5a452fb..44942c4e23 100644
> > --- a/migration/colo.c
> > +++ b/migration/colo.c
> > @@ -26,6 +26,7 @@
> >  #include "qemu/main-loop.h"
> >  #include "qemu/rcu.h"
> >  #include "migration/failover.h"
> > +#include "migration/ram.h"
> >  #ifdef CONFIG_REPLICATION
> >  #include "replication.h"
> >  #endif
> > @@ -845,6 +846,8 @@ void *colo_process_incoming_thread(void
> *opaque)
> >       */
> >      qemu_file_set_blocking(mis->from_src_file, true);
> >
> > +    colo_incoming_start_dirty_log();
> > +
> >      bioc = qio_channel_buffer_new(COLO_BUFFER_BASE_SIZE);
> >      fb = qemu_fopen_channel_input(QIO_CHANNEL(bioc));
> >      object_unref(OBJECT(bioc));
> > diff --git a/migration/ram.c b/migration/ram.c
> > index ed23ed1c7c..ebf9e6ba51 100644
> > --- a/migration/ram.c
> > +++ b/migration/ram.c
> > @@ -2277,6 +2277,7 @@ static void ram_list_init_bitmaps(void)
> >               * dirty_memory[DIRTY_MEMORY_MIGRATION] don't
> include the whole
> >               * guest memory.
> >               */
> > +
> >              block->bmap = bitmap_new(pages);
> >              bitmap_set(block->bmap, 0, pages);
> >              block->clear_bmap_shift = shift;
> > @@ -2986,7 +2987,6 @@ int colo_init_ram_cache(void)
> >                  }
> >                  return -errno;
> >              }
> > -            memcpy(block->colo_cache, block->host,
> block->used_length);
> >          }
> >      }
> >
> > @@ -3000,19 +3000,36 @@ int colo_init_ram_cache(void)
> >
> >          RAMBLOCK_FOREACH_NOT_IGNORED(block) {
> >              unsigned long pages = block->max_length >>
> TARGET_PAGE_BITS;
> > -
> >              block->bmap = bitmap_new(pages);
> > -            bitmap_set(block->bmap, 0, pages);
> >          }
> >      }
> > -    ram_state = g_new0(RAMState, 1);
> > -    ram_state->migration_dirty_pages = 0;
> > -    qemu_mutex_init(&ram_state->bitmap_mutex);
> > -    memory_global_dirty_log_start();
> >
> > +    ram_state_init(&ram_state);
> >      return 0;
> >  }
> >
> > +/* TODO: duplicated with ram_init_bitmaps */
> > +void colo_incoming_start_dirty_log(void)
> > +{
> > +    RAMBlock *block = NULL;
> > +    /* For memory_global_dirty_log_start below. */
> > +    qemu_mutex_lock_iothread();
> > +    qemu_mutex_lock_ramlist();
> > +
> > +    memory_global_dirty_log_sync();
> > +    WITH_RCU_READ_LOCK_GUARD() {
> > +        RAMBLOCK_FOREACH_NOT_IGNORED(block) {
> > +            ramblock_sync_dirty_bitmap(ram_state, block);
> > +            /* Discard this dirty bitmap record */
> > +            bitmap_zero(block->bmap, block->max_length >>
> TARGET_PAGE_BITS);
> > +        }
> > +        memory_global_dirty_log_start();
> > +    }
> > +    ram_state->migration_dirty_pages = 0;
> > +    qemu_mutex_unlock_ramlist();
> > +    qemu_mutex_unlock_iothread();
> > +}
> > +
> >  /* It is need to hold the global lock to call this helper */
> >  void colo_release_ram_cache(void)
> >  {
> > @@ -3032,9 +3049,7 @@ void colo_release_ram_cache(void)
> >              }
> >          }
> >      }
> > -    qemu_mutex_destroy(&ram_state->bitmap_mutex);
> > -    g_free(ram_state);
> > -    ram_state = NULL;
> > +    ram_state_cleanup(&ram_state);
> >  }
> >
> >  /**
> > @@ -3302,7 +3317,6 @@ static void colo_flush_ram_cache(void)
> >              ramblock_sync_dirty_bitmap(ram_state, block);
> >          }
> >      }
> > -
> >
> trace_colo_flush_ram_cache_begin(ram_state->migration_dirty_pages);
> >      WITH_RCU_READ_LOCK_GUARD() {
> >          block = QLIST_FIRST_RCU(&ram_list.blocks);
> > @@ -3348,7 +3362,7 @@ static int ram_load_precopy(QEMUFile *f)
> >
> >      while (!ret && !(flags & RAM_SAVE_FLAG_EOS)) {
> >          ram_addr_t addr, total_ram_bytes;
> > -        void *host = NULL;
> > +        void *host = NULL, *host_bak = NULL;
> >          uint8_t ch;
> >
> >          /*
> > @@ -3379,20 +3393,35 @@ static int ram_load_precopy(QEMUFile *f)
> >                       RAM_SAVE_FLAG_COMPRESS_PAGE |
> RAM_SAVE_FLAG_XBZRLE)) {
> >              RAMBlock *block = ram_block_from_stream(f, flags);
> >
> > +            host = host_from_ram_block_offset(block, addr);
> >              /*
> > -             * After going into COLO, we should load the Page into
> colo_cache.
> > +             * After going into COLO stage, we should not load the page
> > +             * into SVM's memory diretly, we put them into colo_cache
> firstly.
> > +             * NOTE: We need to keep a copy of SVM's ram in
> colo_cache.
> > +             * Privously, we copied all these memory in preparing stage
> of COLO
> > +             * while we need to stop VM, which is a time-consuming
> process.
> > +             * Here we optimize it by a trick, back-up every page while
> in
> > +             * migration process while COLO is enabled, though it
> affects the
> > +             * speed of the migration, but it obviously reduce the
> downtime of
> > +             * back-up all SVM'S memory in COLO preparing stage.
> >               */
> > -            if (migration_incoming_in_colo_state()) {
> > -                host = colo_cache_from_block_offset(block, addr);
> > -            } else {
> > -                host = host_from_ram_block_offset(block, addr);
> > +            if (migration_incoming_colo_enabled()) {
> > +                if (migration_incoming_in_colo_state()) {
> > +                    /* In COLO stage, put all pages into cache
> temporarily */
> > +                    host = colo_cache_from_block_offset(block, addr);
> > +                } else {
> > +                   /*
> > +                    * In migration stage but before COLO stage,
> > +                    * Put all pages into both cache and SVM's
> memory.
> > +                    */
> > +                    host_bak = colo_cache_from_block_offset(block,
> addr);
> > +                }
> >              }
> >              if (!host) {
> >                  error_report("Illegal RAM offset " RAM_ADDR_FMT,
> addr);
> >                  ret = -EINVAL;
> >                  break;
> >              }
> > -
> >              if (!migration_incoming_in_colo_state()) {
> >                  ramblock_recv_bitmap_set(block, host);
> >              }
> > @@ -3506,6 +3535,9 @@ static int ram_load_precopy(QEMUFile *f)
> >          if (!ret) {
> >              ret = qemu_file_get_error(f);
> >          }
> > +        if (!ret && host_bak) {
> > +            memcpy(host_bak, host, TARGET_PAGE_SIZE);
> > +        }
> >      }
> >
> >      ret |= wait_for_decompress_done();
> > diff --git a/migration/ram.h b/migration/ram.h
> > index a553d40751..5ceaff7cb4 100644
> > --- a/migration/ram.h
> > +++ b/migration/ram.h
> > @@ -66,5 +66,6 @@ int ram_dirty_bitmap_reload(MigrationState *s,
> RAMBlock *rb);
> >  /* ram cache */
> >  int colo_init_ram_cache(void);
> >  void colo_release_ram_cache(void);
> > +void colo_incoming_start_dirty_log(void);
> >
> >  #endif
> > --
> > 2.21.0
> >
> >

Re: [PATCH V2 1/8] migration: fix COLO broken caused by a previous commit

[Juan Quintela]

zhanghailiang <zhang.zhanghailiang@huawei.com> wrote:
> This commit "migration: Create migration_is_running()" broke
> COLO. Becuase there is a process broken by this commit.
>
> colo_process_checkpoint
>  ->colo_do_checkpoint_transaction
>    ->migrate_set_block_enabled
>      ->qmp_migrate_set_capabilities
>
> It can be fixed by make COLO process as an exception,
> Maybe we need a better way to fix it.
>
> Cc: Juan Quintela <quintela@redhat.com>
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>

oops sorry.

Reviewed-by: Juan Quintela <quintela@redhat.com>

queued.

Re: [PATCH V2 3/8] savevm: Don't call colo_init_ram_cache twice

[Juan Quintela]

zhanghailiang <zhang.zhanghailiang@huawei.com> wrote:
> This helper has been called twice which is wrong.
> Left the one where called while get COLO enable message
> from source side.
>
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

Re: [PATCH V2 4/8] COLO: Optimize memory back-up process

[Dr. David Alan Gilbert]

* zhanghailiang (zhang.zhanghailiang@huawei.com) wrote:
> This patch will reduce the downtime of VM for the initial process,
> Privously, we copied all these memory in preparing stage of COLO
> while we need to stop VM, which is a time-consuming process.
> Here we optimize it by a trick, back-up every page while in migration
> process while COLO is enabled, though it affects the speed of the
> migration, but it obviously reduce the downtime of back-up all SVM'S
> memory in COLO preparing stage.
> 
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

I'll queue this as well; I'm going to clean up some minor things:

> ---
>  migration/colo.c |  3 +++
>  migration/ram.c  | 68 +++++++++++++++++++++++++++++++++++-------------
>  migration/ram.h  |  1 +
>  3 files changed, 54 insertions(+), 18 deletions(-)
> 
> diff --git a/migration/colo.c b/migration/colo.c
> index 93c5a452fb..44942c4e23 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -26,6 +26,7 @@
>  #include "qemu/main-loop.h"
>  #include "qemu/rcu.h"
>  #include "migration/failover.h"
> +#include "migration/ram.h"
>  #ifdef CONFIG_REPLICATION
>  #include "replication.h"
>  #endif
> @@ -845,6 +846,8 @@ void *colo_process_incoming_thread(void *opaque)
>       */
>      qemu_file_set_blocking(mis->from_src_file, true);
>  
> +    colo_incoming_start_dirty_log();
> +
>      bioc = qio_channel_buffer_new(COLO_BUFFER_BASE_SIZE);
>      fb = qemu_fopen_channel_input(QIO_CHANNEL(bioc));
>      object_unref(OBJECT(bioc));
> diff --git a/migration/ram.c b/migration/ram.c
> index ed23ed1c7c..ebf9e6ba51 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -2277,6 +2277,7 @@ static void ram_list_init_bitmaps(void)
>               * dirty_memory[DIRTY_MEMORY_MIGRATION] don't include the whole
>               * guest memory.
>               */
> +

That change is nice, but shouldn't really be here.

>              block->bmap = bitmap_new(pages);
>              bitmap_set(block->bmap, 0, pages);
>              block->clear_bmap_shift = shift;
> @@ -2986,7 +2987,6 @@ int colo_init_ram_cache(void)
>                  }
>                  return -errno;
>              }
> -            memcpy(block->colo_cache, block->host, block->used_length);
>          }
>      }
>  
> @@ -3000,19 +3000,36 @@ int colo_init_ram_cache(void)
>  
>          RAMBLOCK_FOREACH_NOT_IGNORED(block) {
>              unsigned long pages = block->max_length >> TARGET_PAGE_BITS;
> -
>              block->bmap = bitmap_new(pages);
> -            bitmap_set(block->bmap, 0, pages);
>          }
>      }
> -    ram_state = g_new0(RAMState, 1);
> -    ram_state->migration_dirty_pages = 0;
> -    qemu_mutex_init(&ram_state->bitmap_mutex);
> -    memory_global_dirty_log_start();
>  
> +    ram_state_init(&ram_state);
>      return 0;
>  }
>  
> +/* TODO: duplicated with ram_init_bitmaps */
> +void colo_incoming_start_dirty_log(void)
> +{
> +    RAMBlock *block = NULL;
> +    /* For memory_global_dirty_log_start below. */
> +    qemu_mutex_lock_iothread();
> +    qemu_mutex_lock_ramlist();
> +
> +    memory_global_dirty_log_sync();
> +    WITH_RCU_READ_LOCK_GUARD() {
> +        RAMBLOCK_FOREACH_NOT_IGNORED(block) {
> +            ramblock_sync_dirty_bitmap(ram_state, block);
> +            /* Discard this dirty bitmap record */
> +            bitmap_zero(block->bmap, block->max_length >> TARGET_PAGE_BITS);
> +        }
> +        memory_global_dirty_log_start();
> +    }
> +    ram_state->migration_dirty_pages = 0;
> +    qemu_mutex_unlock_ramlist();
> +    qemu_mutex_unlock_iothread();
> +}
> +
>  /* It is need to hold the global lock to call this helper */
>  void colo_release_ram_cache(void)
>  {
> @@ -3032,9 +3049,7 @@ void colo_release_ram_cache(void)
>              }
>          }
>      }
> -    qemu_mutex_destroy(&ram_state->bitmap_mutex);
> -    g_free(ram_state);
> -    ram_state = NULL;
> +    ram_state_cleanup(&ram_state);
>  }
>  
>  /**
> @@ -3302,7 +3317,6 @@ static void colo_flush_ram_cache(void)
>              ramblock_sync_dirty_bitmap(ram_state, block);
>          }
>      }
> -

I'll remove that

>      trace_colo_flush_ram_cache_begin(ram_state->migration_dirty_pages);
>      WITH_RCU_READ_LOCK_GUARD() {
>          block = QLIST_FIRST_RCU(&ram_list.blocks);
> @@ -3348,7 +3362,7 @@ static int ram_load_precopy(QEMUFile *f)
>  
>      while (!ret && !(flags & RAM_SAVE_FLAG_EOS)) {
>          ram_addr_t addr, total_ram_bytes;
> -        void *host = NULL;
> +        void *host = NULL, *host_bak = NULL;
>          uint8_t ch;
>  
>          /*
> @@ -3379,20 +3393,35 @@ static int ram_load_precopy(QEMUFile *f)
>                       RAM_SAVE_FLAG_COMPRESS_PAGE | RAM_SAVE_FLAG_XBZRLE)) {
>              RAMBlock *block = ram_block_from_stream(f, flags);
>  
> +            host = host_from_ram_block_offset(block, addr);
>              /*
> -             * After going into COLO, we should load the Page into colo_cache.
> +             * After going into COLO stage, we should not load the page
> +             * into SVM's memory diretly, we put them into colo_cache firstly.
                                        ^ typo - c
> +             * NOTE: We need to keep a copy of SVM's ram in colo_cache.
> +             * Privously, we copied all these memory in preparing stage of COLO
                    ^ typo - e

> +             * while we need to stop VM, which is a time-consuming process.
> +             * Here we optimize it by a trick, back-up every page while in
> +             * migration process while COLO is enabled, though it affects the
> +             * speed of the migration, but it obviously reduce the downtime of
> +             * back-up all SVM'S memory in COLO preparing stage.
>               */
> -            if (migration_incoming_in_colo_state()) {
> -                host = colo_cache_from_block_offset(block, addr);
> -            } else {
> -                host = host_from_ram_block_offset(block, addr);
> +            if (migration_incoming_colo_enabled()) {
> +                if (migration_incoming_in_colo_state()) {
> +                    /* In COLO stage, put all pages into cache temporarily */
> +                    host = colo_cache_from_block_offset(block, addr);
> +                } else {
> +                   /*
> +                    * In migration stage but before COLO stage,
> +                    * Put all pages into both cache and SVM's memory.
> +                    */
> +                    host_bak = colo_cache_from_block_offset(block, addr);
> +                }
>              }
>              if (!host) {
>                  error_report("Illegal RAM offset " RAM_ADDR_FMT, addr);
>                  ret = -EINVAL;
>                  break;
>              }
> -
>              if (!migration_incoming_in_colo_state()) {
>                  ramblock_recv_bitmap_set(block, host);
>              }
> @@ -3506,6 +3535,9 @@ static int ram_load_precopy(QEMUFile *f)
>          if (!ret) {
>              ret = qemu_file_get_error(f);
>          }
> +        if (!ret && host_bak) {
> +            memcpy(host_bak, host, TARGET_PAGE_SIZE);
> +        }
>      }
>  
>      ret |= wait_for_decompress_done();
> diff --git a/migration/ram.h b/migration/ram.h
> index a553d40751..5ceaff7cb4 100644
> --- a/migration/ram.h
> +++ b/migration/ram.h
> @@ -66,5 +66,6 @@ int ram_dirty_bitmap_reload(MigrationState *s, RAMBlock *rb);
>  /* ram cache */
>  int colo_init_ram_cache(void);
>  void colo_release_ram_cache(void);
> +void colo_incoming_start_dirty_log(void);
>  
>  #endif
> -- 
> 2.21.0
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH V2 5/8] ram/colo: only record bitmap of dirty pages in COLO stage

[Dr. David Alan Gilbert]

* zhanghailiang (zhang.zhanghailiang@huawei.com) wrote:
> It is only need to record bitmap of dirty pages while goes
> into COLO stage.
> 
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

and queued

> ---
>  migration/ram.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index ebf9e6ba51..1b3f423351 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -2735,7 +2735,7 @@ static inline void *host_from_ram_block_offset(RAMBlock *block,
>  }
>  
>  static inline void *colo_cache_from_block_offset(RAMBlock *block,
> -                                                 ram_addr_t offset)
> +                             ram_addr_t offset, bool record_bitmap)
>  {
>      if (!offset_in_ramblock(block, offset)) {
>          return NULL;
> @@ -2751,7 +2751,8 @@ static inline void *colo_cache_from_block_offset(RAMBlock *block,
>      * It help us to decide which pages in ram cache should be flushed
>      * into VM's RAM later.
>      */
> -    if (!test_and_set_bit(offset >> TARGET_PAGE_BITS, block->bmap)) {
> +    if (record_bitmap &&
> +        !test_and_set_bit(offset >> TARGET_PAGE_BITS, block->bmap)) {
>          ram_state->migration_dirty_pages++;
>      }
>      return block->colo_cache + offset;
> @@ -3408,13 +3409,13 @@ static int ram_load_precopy(QEMUFile *f)
>              if (migration_incoming_colo_enabled()) {
>                  if (migration_incoming_in_colo_state()) {
>                      /* In COLO stage, put all pages into cache temporarily */
> -                    host = colo_cache_from_block_offset(block, addr);
> +                    host = colo_cache_from_block_offset(block, addr, true);
>                  } else {
>                     /*
>                      * In migration stage but before COLO stage,
>                      * Put all pages into both cache and SVM's memory.
>                      */
> -                    host_bak = colo_cache_from_block_offset(block, addr);
> +                    host_bak = colo_cache_from_block_offset(block, addr, false);
>                  }
>              }
>              if (!host) {
> -- 
> 2.21.0
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH V2 6/8] migration: recognize COLO as part of activating process

[Dr. David Alan Gilbert]

* zhanghailiang (zhang.zhanghailiang@huawei.com) wrote:
> We will migrate parts of dirty pages backgroud lively during the gap time
> of two checkpoints, without this modification, it will not work
> because ram_save_iterate() will check it before send RAM_SAVE_FLAG_EOS
> at the end of it.
> 
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

and queued.

> ---
>  migration/migration.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index e8c62c6e2e..f71c337600 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -840,6 +840,7 @@ bool migration_is_setup_or_active(int state)
>      case MIGRATION_STATUS_PRE_SWITCHOVER:
>      case MIGRATION_STATUS_DEVICE:
>      case MIGRATION_STATUS_WAIT_UNPLUG:
> +    case MIGRATION_STATUS_COLO:
>          return true;
>  
>      default:
> -- 
> 2.21.0
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH V2 7/8] COLO: Migrate dirty pages during the gap of checkpointing

[Dr. David Alan Gilbert]

* zhanghailiang (zhang.zhanghailiang@huawei.com) wrote:
> We can migrate some dirty pages during the gap of checkpointing,
> by this way, we can reduce the amount of ram migrated during checkpointing.
> 
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> ---
>  migration/colo.c       | 73 ++++++++++++++++++++++++++++++++++++++++--
>  migration/migration.h  |  1 +
>  migration/trace-events |  1 +
>  qapi/migration.json    |  4 ++-
>  4 files changed, 75 insertions(+), 4 deletions(-)
> 
> diff --git a/migration/colo.c b/migration/colo.c
> index 44942c4e23..c36d94072f 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -47,6 +47,13 @@ static COLOMode last_colo_mode;
>  
>  #define COLO_BUFFER_BASE_SIZE (4 * 1024 * 1024)
>  
> +#define DEFAULT_RAM_PENDING_CHECK 1000
> +
> +/* should be calculated by bandwidth and max downtime ? */
> +#define THRESHOLD_PENDING_SIZE (100 * 1024 * 1024UL)

In the last version I asked to change these two values to parameters.

Dave

> +static int checkpoint_request;
> +
>  bool migration_in_colo_state(void)
>  {
>      MigrationState *s = migrate_get_current();
> @@ -517,6 +524,20 @@ static void colo_compare_notify_checkpoint(Notifier *notifier, void *data)
>      colo_checkpoint_notify(data);
>  }
>  
> +static bool colo_need_migrate_ram_background(MigrationState *s)
> +{
> +    uint64_t pending_size, pend_pre, pend_compat, pend_post;
> +    int64_t max_size = THRESHOLD_PENDING_SIZE;
> +
> +    qemu_savevm_state_pending(s->to_dst_file, max_size, &pend_pre,
> +                              &pend_compat, &pend_post);
> +    pending_size = pend_pre + pend_compat + pend_post;
> +
> +    trace_colo_need_migrate_ram_background(pending_size);
> +    return (pending_size >= max_size);
> +}
> +
> +
>  static void colo_process_checkpoint(MigrationState *s)
>  {
>      QIOChannelBuffer *bioc;
> @@ -572,6 +593,8 @@ static void colo_process_checkpoint(MigrationState *s)
>  
>      timer_mod(s->colo_delay_timer,
>              current_time + s->parameters.x_checkpoint_delay);
> +    timer_mod(s->pending_ram_check_timer,
> +        current_time + DEFAULT_RAM_PENDING_CHECK);
>  
>      while (s->state == MIGRATION_STATUS_COLO) {
>          if (failover_get_state() != FAILOVER_STATUS_NONE) {
> @@ -584,9 +607,30 @@ static void colo_process_checkpoint(MigrationState *s)
>          if (s->state != MIGRATION_STATUS_COLO) {
>              goto out;
>          }
> -        ret = colo_do_checkpoint_transaction(s, bioc, fb);
> -        if (ret < 0) {
> -            goto out;
> +        if (atomic_xchg(&checkpoint_request, 0)) {
> +            /* start a colo checkpoint */
> +            ret = colo_do_checkpoint_transaction(s, bioc, fb);
> +            if (ret < 0) {
> +                goto out;
> +            }
> +        } else {
> +            if (colo_need_migrate_ram_background(s)) {
> +                colo_send_message(s->to_dst_file,
> +                                  COLO_MESSAGE_MIGRATE_RAM_BACKGROUND,
> +                                  &local_err);
> +                if (local_err) {
> +                    goto out;
> +                }
> +
> +                qemu_savevm_state_iterate(s->to_dst_file, false);
> +                qemu_put_byte(s->to_dst_file, QEMU_VM_EOF);
> +                ret = qemu_file_get_error(s->to_dst_file);
> +                if (ret < 0) {
> +                    error_setg_errno(&local_err, -ret,
> +                        "Failed to send dirty pages backgroud");
> +                    goto out;
> +                }
> +            }
>          }
>      }
>  
> @@ -627,6 +671,8 @@ out:
>      colo_compare_unregister_notifier(&packets_compare_notifier);
>      timer_del(s->colo_delay_timer);
>      timer_free(s->colo_delay_timer);
> +    timer_del(s->pending_ram_check_timer);
> +    timer_free(s->pending_ram_check_timer);
>      qemu_sem_destroy(&s->colo_checkpoint_sem);
>  
>      /*
> @@ -644,6 +690,7 @@ void colo_checkpoint_notify(void *opaque)
>      MigrationState *s = opaque;
>      int64_t next_notify_time;
>  
> +    atomic_inc(&checkpoint_request);
>      qemu_sem_post(&s->colo_checkpoint_sem);
>      s->colo_checkpoint_time = qemu_clock_get_ms(QEMU_CLOCK_HOST);
>      next_notify_time = s->colo_checkpoint_time +
> @@ -651,6 +698,19 @@ void colo_checkpoint_notify(void *opaque)
>      timer_mod(s->colo_delay_timer, next_notify_time);
>  }
>  
> +static void colo_pending_ram_check_notify(void *opaque)
> +{
> +    int64_t next_notify_time;
> +    MigrationState *s = opaque;
> +
> +    if (migration_in_colo_state()) {
> +        next_notify_time = DEFAULT_RAM_PENDING_CHECK +
> +                           qemu_clock_get_ms(QEMU_CLOCK_HOST);
> +        timer_mod(s->pending_ram_check_timer, next_notify_time);
> +        qemu_sem_post(&s->colo_checkpoint_sem);
> +    }
> +}
> +
>  void migrate_start_colo_process(MigrationState *s)
>  {
>      qemu_mutex_unlock_iothread();
> @@ -658,6 +718,8 @@ void migrate_start_colo_process(MigrationState *s)
>      s->colo_delay_timer =  timer_new_ms(QEMU_CLOCK_HOST,
>                                  colo_checkpoint_notify, s);
>  
> +    s->pending_ram_check_timer = timer_new_ms(QEMU_CLOCK_HOST,
> +                                colo_pending_ram_check_notify, s);
>      qemu_sem_init(&s->colo_exit_sem, 0);
>      migrate_set_state(&s->state, MIGRATION_STATUS_ACTIVE,
>                        MIGRATION_STATUS_COLO);
> @@ -806,6 +868,11 @@ static void colo_wait_handle_message(MigrationIncomingState *mis,
>      case COLO_MESSAGE_CHECKPOINT_REQUEST:
>          colo_incoming_process_checkpoint(mis, fb, bioc, errp);
>          break;
> +    case COLO_MESSAGE_MIGRATE_RAM_BACKGROUND:
> +        if (qemu_loadvm_state_main(mis->from_src_file, mis) < 0) {
> +            error_setg(errp, "Load ram background failed");
> +        }
> +        break;
>      default:
>          error_setg(errp, "Got unknown COLO message: %d", msg);
>          break;
> diff --git a/migration/migration.h b/migration/migration.h
> index 8473ddfc88..5355259789 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -219,6 +219,7 @@ struct MigrationState
>      QemuSemaphore colo_checkpoint_sem;
>      int64_t colo_checkpoint_time;
>      QEMUTimer *colo_delay_timer;
> +    QEMUTimer *pending_ram_check_timer;
>  
>      /* The first error that has occurred.
>         We used the mutex to be able to return the 1st error message */
> diff --git a/migration/trace-events b/migration/trace-events
> index 4ab0a503d2..f2ed0c8645 100644
> --- a/migration/trace-events
> +++ b/migration/trace-events
> @@ -295,6 +295,7 @@ migration_tls_incoming_handshake_complete(void) ""
>  colo_vm_state_change(const char *old, const char *new) "Change '%s' => '%s'"
>  colo_send_message(const char *msg) "Send '%s' message"
>  colo_receive_message(const char *msg) "Receive '%s' message"
> +colo_need_migrate_ram_background(uint64_t pending_size) "Pending 0x%" PRIx64 " dirty ram"
>  
>  # colo-failover.c
>  colo_failover_set_state(const char *new_state) "new state %s"
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 52f3429969..73445f1978 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -977,12 +977,14 @@
>  #
>  # @vmstate-loaded: VM's state has been loaded by SVM.
>  #
> +# @migrate-ram-background: Send some dirty pages during the gap of COLO checkpoint
> +#
>  # Since: 2.8
>  ##
>  { 'enum': 'COLOMessage',
>    'data': [ 'checkpoint-ready', 'checkpoint-request', 'checkpoint-reply',
>              'vmstate-send', 'vmstate-size', 'vmstate-received',
> -            'vmstate-loaded' ] }
> +            'vmstate-loaded', 'migrate-ram-background' ] }
>  
>  ##
>  # @COLOMode:
> -- 
> 2.21.0
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH V2 8/8] migration/colo: Only flush ram cache while do checkpoint

[Dr. David Alan Gilbert]

* zhanghailiang (zhang.zhanghailiang@huawei.com) wrote:
> After add migrating ram backgroud, we will call ram_load
> for this process, but we should not flush ram cache during
> this process. Move the flush action to the right place.
> 
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  migration/colo.c | 1 +
>  migration/ram.c  | 5 +----
>  migration/ram.h  | 1 +
>  3 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/migration/colo.c b/migration/colo.c
> index c36d94072f..18df8289f8 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -799,6 +799,7 @@ static void colo_incoming_process_checkpoint(MigrationIncomingState *mis,
>  
>      qemu_mutex_lock_iothread();
>      vmstate_loading = true;
> +    colo_flush_ram_cache();
>      ret = qemu_load_device_state(fb);
>      if (ret < 0) {
>          error_setg(errp, "COLO: load device state failed");
> diff --git a/migration/ram.c b/migration/ram.c
> index 1b3f423351..7bc841d14f 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -3305,7 +3305,7 @@ static bool postcopy_is_running(void)
>   * Flush content of RAM cache into SVM's memory.
>   * Only flush the pages that be dirtied by PVM or SVM or both.
>   */
> -static void colo_flush_ram_cache(void)
> +void colo_flush_ram_cache(void)
>  {
>      RAMBlock *block = NULL;
>      void *dst_host;
> @@ -3576,9 +3576,6 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
>      }
>      trace_ram_load_complete(ret, seq_iter);
>  
> -    if (!ret  && migration_incoming_in_colo_state()) {
> -        colo_flush_ram_cache();
> -    }
>      return ret;
>  }
>  
> diff --git a/migration/ram.h b/migration/ram.h
> index 5ceaff7cb4..ae14341482 100644
> --- a/migration/ram.h
> +++ b/migration/ram.h
> @@ -67,5 +67,6 @@ int ram_dirty_bitmap_reload(MigrationState *s, RAMBlock *rb);
>  int colo_init_ram_cache(void);
>  void colo_release_ram_cache(void);
>  void colo_incoming_start_dirty_log(void);
> +void colo_flush_ram_cache(void);
>  
>  #endif
> -- 
> 2.21.0
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK