From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47335) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bzdu1-0003VD-Df for qemu-devel@nongnu.org; Thu, 27 Oct 2016 02:11:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bzdty-0001eU-5x for qemu-devel@nongnu.org; Thu, 27 Oct 2016 02:11:01 -0400 Received: from szxga02-in.huawei.com ([119.145.14.65]:33878) by eggs.gnu.org with esmtps (TLS1.0:RSA_ARCFOUR_SHA1:16) (Exim 4.71) (envelope-from ) id 1bzdtx-0001d6-J6 for qemu-devel@nongnu.org; Thu, 27 Oct 2016 02:10:58 -0400 References: <1476792613-11712-1-git-send-email-zhang.zhanghailiang@huawei.com> <1476792613-11712-4-git-send-email-zhang.zhanghailiang@huawei.com> <20161026045032.GC1679@amit-lp.rh> <5810B456.4000300@huawei.com> <20161027035818.GD1476@amit-lp.rh> From: Hailiang Zhang Message-ID: <58119A59.5090503@huawei.com> Date: Thu, 27 Oct 2016 14:10:33 +0800 MIME-Version: 1.0 In-Reply-To: <20161027035818.GD1476@amit-lp.rh> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH COLO-Frame (Base) v21 03/17] migration: Enter into COLO mode after migration if COLO is enabled List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Amit Shah Cc: quintela@redhat.com, qemu-devel@nongnu.org, dgilbert@redhat.com, wency@cn.fujitsu.com, lizhijian@cn.fujitsu.com, xiecl.fnst@cn.fujitsu.com, Gonglei On 2016/10/27 11:58, Amit Shah wrote: > 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 >>>> Signed-off-by: Li Zhijian >>>> Signed-off-by: Gonglei >>>> Reviewed-by: Dr. David Alan Gilbert >>> >>> (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. > > OK, we will do it, thanks. > Amit > > . >