On (Wed) 26 Oct 2016 [21:49:10], Hailiang Zhang wrote:
> On 2016/10/26 12:50, Amit Shah wrote:
> >On (Tue) 18 Oct 2016 [20:09:59], zhanghailiang wrote:
> >>Add a new migration state: MIGRATION_STATUS_COLO. Migration source side
> >>enters this state after the first live migration successfully finished
> >>if COLO is enabled by command 'migrate_set_capability x-colo on'.
> >>
> >>We reuse migration thread, so the process of checkpointing will be handled
> >>in migration thread.
> >>
> >>Signed-off-by: zhanghailiang <zhang.zhanghaili...@huawei.com>
> >>Signed-off-by: Li Zhijian <lizhij...@cn.fujitsu.com>
> >>Signed-off-by: Gonglei <arei.gong...@huawei.com>
> >>Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
> >
> >(snip)
> >
> >>+static void colo_process_checkpoint(MigrationState *s)
> >>+{
> >>+ qemu_mutex_lock_iothread();
> >>+ vm_start();
> >>+ qemu_mutex_unlock_iothread();
> >>+ trace_colo_vm_state_change("stop", "run");
> >>+
> >>+ /* TODO: COLO checkpoint savevm loop */
> >>+
> >>+ migrate_set_state(&s->state, MIGRATION_STATUS_COLO,
> >>+ MIGRATION_STATUS_COMPLETED);
> >
> >Is this just a temporary thing that'll be removed in the next patches?
>
> Yes, you are right, we will move this codes into failover process in the next
> patch, because after failover, we should finish the original migration, there
> are still some cleanup work need to be done.
>
> >I guess so - because once you enter COLO state, you want to remain in
> >it, right?
> >
>
> Yes.
>
> >I think the commit message implies that. So the commit msg and the
> >code are not in sync.
> >
>
> Hmm, i'll remove it here in this patch, is it OK ?
Yes.
>
> >(snip)
> >
> >>diff --git a/migration/migration.c b/migration/migration.c
> >>index f7dd9c6..462007d 100644
> >>--- a/migration/migration.c
> >>+++ b/migration/migration.c
> >>@@ -695,6 +695,10 @@ MigrationInfo *qmp_query_migrate(Error **errp)
> >>
> >> get_xbzrle_cache_stats(info);
> >> break;
> >>+ case MIGRATION_STATUS_COLO:
> >>+ info->has_status = true;
> >>+ /* TODO: display COLO specific information (checkpoint info etc.)
> >>*/
> >>+ break;
> >
> >When do you plan to add this? I guess it's important for debugging
> >and also to get the state of the system while colo is active. What
> >info do you have planned to display here?
> >
>
> IIRC, we have such patch which implemented this specific information in the
> previous
> version long time ago. Yes, it is quit useful, for example, the average/max
> time of
> pause while do checkpoint, the average/max number of dirty pages transferred
> to SVM,
> the amount time of VM in COLO state, the total checkpoint times, the count of
> checkpointing because of inconsistency of network packages compare.
Yes, please get this in soon as well.
Amit
