From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48202) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZydAV-0002W3-FF for qemu-devel@nongnu.org; Tue, 17 Nov 2015 05:07:23 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZydAR-0002QO-6Z for qemu-devel@nongnu.org; Tue, 17 Nov 2015 05:07:19 -0500 Received: from szxga03-in.huawei.com ([119.145.14.66]:24783) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZydAQ-0002Pe-8A for qemu-devel@nongnu.org; Tue, 17 Nov 2015 05:07:15 -0500 References: <1446551816-15768-1-git-send-email-zhang.zhanghailiang@huawei.com> <1446551816-15768-18-git-send-email-zhang.zhanghailiang@huawei.com> <20151113183440.GO2456@work-vm> From: zhanghailiang Message-ID: <564AEF37.90606@huawei.com> Date: Tue, 17 Nov 2015 17:11:19 +0800 MIME-Version: 1.0 In-Reply-To: <20151113183440.GO2456@work-vm> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH COLO-Frame v10 17/38] COLO: synchronize PVM's state to SVM periodically List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Dr. David Alan Gilbert" Cc: lizhijian@cn.fujitsu.com, quintela@redhat.com, yunhong.jiang@intel.com, eddie.dong@intel.com, peter.huangpeng@huawei.com, qemu-devel@nongnu.org, arei.gonglei@huawei.com, stefanha@redhat.com, amit.shah@redhat.com On 2015/11/14 2:34, Dr. David Alan Gilbert wrote: > * zhanghailiang (zhang.zhanghailiang@huawei.com) wrote: >> Do checkpoint periodically, the default interval is 200ms. >> >> Signed-off-by: zhanghailiang >> Signed-off-by: Li Zhijian >> --- >> migration/colo.c | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) >> >> diff --git a/migration/colo.c b/migration/colo.c >> index 0efab21..a6791f4 100644 >> --- a/migration/colo.c >> +++ b/migration/colo.c >> @@ -11,12 +11,19 @@ >> */ >> >> #include >> +#include "qemu/timer.h" >> #include "sysemu/sysemu.h" >> #include "migration/colo.h" >> #include "trace.h" >> #include "qemu/error-report.h" >> #include "qemu/sockets.h" >> >> +/* >> + * checkpoint interval: unit ms >> + * Note: Please change this default value to 10000 when we support hybrid mode. >> + */ >> +#define CHECKPOINT_MAX_PEROID 200 > > Why not put the patch that makes this a configurable parameter before this, > and then we can use it straight away? > Do you mean setting this value by command "migrate_set_parameter" ? I have realized it in patch 26. >> /* colo buffer */ >> #define COLO_BUFFER_BASE_SIZE (4 * 1024 * 1024) >> >> @@ -183,6 +190,7 @@ out: >> static void colo_process_checkpoint(MigrationState *s) >> { >> QEMUSizedBuffer *buffer = NULL; >> + int64_t current_time, checkpoint_time = qemu_clock_get_ms(QEMU_CLOCK_HOST); >> int fd, ret = 0; >> >> /* Dup the fd of to_dst_file */ >> @@ -220,11 +228,17 @@ static void colo_process_checkpoint(MigrationState *s) >> trace_colo_vm_state_change("stop", "run"); >> >> while (s->state == MIGRATION_STATUS_COLO) { >> + current_time = qemu_clock_get_ms(QEMU_CLOCK_HOST); >> + if (current_time - checkpoint_time < CHECKPOINT_MAX_PEROID) { >> + g_usleep(100000); >> + continue; >> + } > > I'm a bit concerned at the 100ms wait, when the period is 200ms; > depending how the times work out, couldn't we end up waiting for just > under 300ms? - that's a big error - and it's even more weird when > we make it configurable later. > Agreed. > I don't think we've got a sleep-until, which is a shame; but how > about something like: > > if (current_time - checkpoint_time < CHECKPOINT_MAX_PEROID) { > int64_t delay_ms; > delay_ms = CHECKPOINT_MAX_PERIOD - (current_time - checkpoint_time); > g_usleep (delay_ms * 1000); > } > That's a reasonable modification. I will fix it like that in next version. Thanks， zhanghailiang > Dave > >> /* start a colo checkpoint */ >> ret = colo_do_checkpoint_transaction(s, buffer); >> if (ret < 0) { >> goto out; >> } >> + checkpoint_time = qemu_clock_get_ms(QEMU_CLOCK_HOST); >> } >> >> out: >> -- >> 1.8.3.1 >> >> > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK > > . >