* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote:
> On 2015/11/7 2:59, Dr. David Alan Gilbert wrote:
> >* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote:
> >>The main process of checkpoint is to synchronize SVM with PVM.
> >>VM's state includes ram and device state. So we will migrate PVM's
> >>state to SVM when do checkpoint, just like migration does.
> >>
> >>We will cache PVM's state in slave, we use QEMUSizedBuffer
> >>to store the data, we need to know the size of VM state, so in master,
> >>we use qsb to store VM state temporarily, get the data size by call
> >>qsb_get_length()
> >>and then migrate the data to the qsb in the secondary side.
> >>
> >>Signed-off-by: zhanghailiang <zhang.zhanghaili...@huawei.com>
> >>Signed-off-by: Gonglei <arei.gong...@huawei.com>
> >>Signed-off-by: Li Zhijian <lizhij...@cn.fujitsu.com>
> >>---
> >> migration/colo.c | 68
> >> ++++++++++++++++++++++++++++++++++++++++++++++++++----
> >> migration/ram.c | 47 +++++++++++++++++++++++++++++--------
> >> migration/savevm.c | 2 +-
> >> 3 files changed, 101 insertions(+), 16 deletions(-)
> >>
> >>diff --git a/migration/colo.c b/migration/colo.c
> >>index 2510762..b865513 100644
> >>--- a/migration/colo.c
> >>+++ b/migration/colo.c
> >>@@ -17,6 +17,9 @@
> >> #include "qemu/error-report.h"
> >> #include "qemu/sockets.h"
> >>
> >>+/* colo buffer */
> >>+#define COLO_BUFFER_BASE_SIZE (4 * 1024 * 1024)
> >>+
> >> bool colo_supported(void)
> >> {
> >> return true;
> >>@@ -94,9 +97,12 @@ static int colo_ctl_get(QEMUFile *f, uint32_t require)
> >> return value;
> >> }
> >>
> >>-static int colo_do_checkpoint_transaction(MigrationState *s)
> >>+static int colo_do_checkpoint_transaction(MigrationState *s,
> >>+ QEMUSizedBuffer *buffer)
> >> {
> >> int ret;
> >>+ size_t size;
> >>+ QEMUFile *trans = NULL;
> >>
> >> ret = colo_ctl_put(s->to_dst_file, COLO_COMMAND_CHECKPOINT_REQUEST,
> >> 0);
> >> if (ret < 0) {
> >>@@ -107,15 +113,47 @@ static int
> >>colo_do_checkpoint_transaction(MigrationState *s)
> >> if (ret < 0) {
> >> goto out;
> >> }
> >>+ /* Reset colo buffer and open it for write */
> >>+ qsb_set_length(buffer, 0);
> >>+ trans = qemu_bufopen("w", buffer);
> >>+ if (!trans) {
> >>+ error_report("Open colo buffer for write failed");
> >>+ goto out;
> >>+ }
> >>
> >>- /* TODO: suspend and save vm state to colo buffer */
> >>+ qemu_mutex_lock_iothread();
> >>+ vm_stop_force_state(RUN_STATE_COLO);
> >>+ qemu_mutex_unlock_iothread();
> >>+ trace_colo_vm_state_change("run", "stop");
> >>+
> >>+ /* Disable block migration */
> >>+ s->params.blk = 0;
> >>+ s->params.shared = 0;
> >>+ qemu_savevm_state_header(trans);
> >>+ qemu_savevm_state_begin(trans, &s->params);
> >>+ qemu_mutex_lock_iothread();
> >>+ qemu_savevm_state_complete(trans);
> >>+ qemu_mutex_unlock_iothread();
> >>+
> >>+ qemu_fflush(trans);
> >>
> >> ret = colo_ctl_put(s->to_dst_file, COLO_COMMAND_VMSTATE_SEND, 0);
> >> if (ret < 0) {
> >> goto out;
> >> }
> >>+ /* we send the total size of the vmstate first */
> >>+ size = qsb_get_length(buffer);
> >>+ ret = colo_ctl_put(s->to_dst_file, COLO_COMMAND_VMSTATE_SIZE, size);
> >>+ if (ret < 0) {
> >>+ goto out;
> >>+ }
> >>
> >>- /* TODO: send vmstate to Secondary */
> >>+ qsb_put_buffer(s->to_dst_file, buffer, size);
> >>+ qemu_fflush(s->to_dst_file);
> >>+ ret = qemu_file_get_error(s->to_dst_file);
> >>+ if (ret < 0) {
> >>+ goto out;
> >>+ }
> >>
> >> ret = colo_ctl_get(s->from_dst_file, COLO_COMMAND_VMSTATE_RECEIVED);
> >> if (ret < 0) {
> >>@@ -127,14 +165,24 @@ static int
> >>colo_do_checkpoint_transaction(MigrationState *s)
> >> goto out;
> >> }
> >>
> >>- /* TODO: resume Primary */
> >>+ ret = 0;
> >>+ /* resume master */
> >>+ qemu_mutex_lock_iothread();
> >>+ vm_start();
> >>+ qemu_mutex_unlock_iothread();
> >>+ trace_colo_vm_state_change("stop", "run");
> >>
> >> out:
> >>+ if (trans) {
> >>+ qemu_fclose(trans);
> >>+ }
> >>+
> >> return ret;
> >> }
> >>
> >> static void colo_process_checkpoint(MigrationState *s)
> >> {
> >>+ QEMUSizedBuffer *buffer = NULL;
> >> int fd, ret = 0;
> >>
> >> /* Dup the fd of to_dst_file */
> >>@@ -159,6 +207,13 @@ static void colo_process_checkpoint(MigrationState *s)
> >> goto out;
> >> }
> >>
> >>+ buffer = qsb_create(NULL, COLO_BUFFER_BASE_SIZE);
> >>+ if (buffer == NULL) {
> >>+ ret = -ENOMEM;
> >>+ error_report("Failed to allocate buffer!");
> >
> >Please say 'Failed to allocate colo buffer'; QEMU has lots and lots of
> >buffers.
> >
>
> OK， will fix it in next version.
>
> >>+ goto out;
> >>+ }
> >>+
> >> qemu_mutex_lock_iothread();
> >> vm_start();
> >> qemu_mutex_unlock_iothread();
> >>@@ -166,7 +221,7 @@ static void colo_process_checkpoint(MigrationState *s)
> >>
> >> while (s->state == MIGRATION_STATUS_COLO) {
> >> /* start a colo checkpoint */
> >>- ret = colo_do_checkpoint_transaction(s);
> >>+ ret = colo_do_checkpoint_transaction(s, buffer);
> >> if (ret < 0) {
> >> goto out;
> >> }
> >>@@ -179,6 +234,9 @@ out:
> >> migrate_set_state(&s->state, MIGRATION_STATUS_COLO,
> >> MIGRATION_STATUS_COMPLETED);
> >>
> >>+ qsb_free(buffer);
> >>+ buffer = NULL;
> >>+
> >> if (s->from_dst_file) {
> >> qemu_fclose(s->from_dst_file);
> >> }
> >>diff --git a/migration/ram.c b/migration/ram.c
> >>index a25bcc7..5784c15 100644
> >>--- a/migration/ram.c
> >>+++ b/migration/ram.c
> >>@@ -38,6 +38,7 @@
> >> #include "trace.h"
> >> #include "exec/ram_addr.h"
> >> #include "qemu/rcu_queue.h"
> >>+#include "migration/colo.h"
> >>
> >> #ifdef DEBUG_MIGRATION_RAM
> >> #define DPRINTF(fmt, ...) \
> >>@@ -1165,15 +1166,8 @@ void migration_bitmap_extend(ram_addr_t old,
> >>ram_addr_t new)
> >> }
> >> }
> >>
> >>-/* Each of ram_save_setup, ram_save_iterate and ram_save_complete has
> >>- * long-running RCU critical section. When rcu-reclaims in the code
> >>- * start to become numerous it will be necessary to reduce the
> >>- * granularity of these critical sections.
> >>- */
> >>-
> >>-static int ram_save_setup(QEMUFile *f, void *opaque)
> >>+static int ram_save_init_globals(void)
> >> {
> >>- RAMBlock *block;
> >> int64_t ram_bitmap_pages; /* Size of bitmap in pages, including gaps
> >> */
> >>
> >> dirty_rate_high_cnt = 0;
> >>@@ -1233,6 +1227,31 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
> >> migration_bitmap_sync();
> >> qemu_mutex_unlock_ramlist();
> >> qemu_mutex_unlock_iothread();
> >>+ rcu_read_unlock();
> >>+
> >>+ return 0;
> >>+}
> >
> >It surprises me you want migration_bitmap_sync in ram_save_init_globals(),
> >but I guess you want the first sync at the start.
> >
>
> Er, sorry，i don't quite understand.
> Here. I just split part codes of ram_save_setup()
> into a helper function ram_save_init_global(), to make it more clear.
> We can't do initial work for twice. Is there any thing wrong ?
No, that's OK - it just seemed odd for a function like 'init_globals'
to do such a big side effect of doing the sync; but yes, it makes sense
since it's just a split.
> >>+/* Each of ram_save_setup, ram_save_iterate and ram_save_complete has
> >>+ * long-running RCU critical section. When rcu-reclaims in the code
> >>+ * start to become numerous it will be necessary to reduce the
> >>+ * granularity of these critical sections.
> >>+ */
> >>+
> >>+static int ram_save_setup(QEMUFile *f, void *opaque)
> >>+{
> >>+ RAMBlock *block;
> >>+
> >>+ /*
> >>+ * migration has already setup the bitmap, reuse it.
> >>+ */
> >>+ if (!migration_in_colo_state()) {
> >>+ if (ram_save_init_globals() < 0) {
> >>+ return -1;
> >>+ }
> >>+ }
> >>+
> >>+ rcu_read_lock();
> >>
> >> qemu_put_be64(f, ram_bytes_total() | RAM_SAVE_FLAG_MEM_SIZE);
> >>
> >>@@ -1332,7 +1351,8 @@ static int ram_save_complete(QEMUFile *f, void
> >>*opaque)
> >> while (true) {
> >> int pages;
> >>
> >>- pages = ram_find_and_save_block(f, true, &bytes_transferred);
> >>+ pages = ram_find_and_save_block(f, !migration_in_colo_state(),
> >>+ &bytes_transferred);
> >> /* no more blocks to sent */
> >> if (pages == 0) {
> >> break;
> >>@@ -1343,8 +1363,15 @@ static int ram_save_complete(QEMUFile *f, void
> >>*opaque)
> >> ram_control_after_iterate(f, RAM_CONTROL_FINISH);
> >>
> >> rcu_read_unlock();
> >>+ /*
> >>+ * Since we need to reuse dirty bitmap in colo,
> >>+ * don't cleanup the bitmap.
> >>+ */
> >>+ if (!migrate_colo_enabled() ||
> >>+ migration_has_failed(migrate_get_current())) {
> >>+ migration_end();
> >>+ }
> >>
> >>- migration_end();
> >> qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
> >>
> >> return 0;
> >>diff --git a/migration/savevm.c b/migration/savevm.c
> >>index dbcc39a..0faf12b 100644
> >>--- a/migration/savevm.c
> >>+++ b/migration/savevm.c
> >>@@ -48,7 +48,7 @@
> >> #include "qemu/iov.h"
> >> #include "block/snapshot.h"
> >> #include "block/qapi.h"
> >>-
> >>+#include "migration/colo.h"
> >>
> >> #ifndef ETH_P_RARP
> >> #define ETH_P_RARP 0x8035
> >
> >Wrong patch?
> >
>
> No, we have call migration_in_colo_state() in qemu_savevm_state_begin().
> So we have to include "migration/colo.h"
I don't think you use it in savevm.c until patch 30, so you can add
the #include in patch 30 (or whichever is the patch that first needs it).
Dave
>
> >
> >So other than those minor things:
> >
> >Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
> >
> >but watch out for the recent changes to migrate_end that went in
> >a few days ago.
> >
>
> Thanks for reminding me, i have rebased that. ;)
>
> zhanghailiang
>
> >Dave
> >
> >>--
> >>1.8.3.1
> >>
> >>
> >--
> >Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
> >
> >.
> >
>
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
