From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37355) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a7EGb-0006c9-AL for qemu-devel@nongnu.org; Thu, 10 Dec 2015 22:21:10 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a7EGX-0007nY-7h for qemu-devel@nongnu.org; Thu, 10 Dec 2015 22:21:09 -0500 Received: from szxga03-in.huawei.com ([119.145.14.66]:26689) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a7EGW-0007mq-7s for qemu-devel@nongnu.org; Thu, 10 Dec 2015 22:21:05 -0500 References: <1448357149-17572-1-git-send-email-zhang.zhanghailiang@huawei.com> <1448357149-17572-20-git-send-email-zhang.zhanghailiang@huawei.com> <20151209185024.GA12745@work-vm> From: Hailiang Zhang Message-ID: <566A40FD.7000104@huawei.com> Date: Fri, 11 Dec 2015 11:20:29 +0800 MIME-Version: 1.0 In-Reply-To: <20151209185024.GA12745@work-vm> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH COLO-Frame v11 19/39] COLO: Add checkpoint-delay parameter for migrate-set-parameters 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, Markus Armbruster , 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, Luiz Capitulino , hongyang.yang@easystack.cn On 2015/12/10 2:50, Dr. David Alan Gilbert wrote: > * zhanghailiang (zhang.zhanghailiang@huawei.com) wrote: >> Add checkpoint-delay parameter for migrate-set-parameters, so that >> we can control the checkpoint frequency when COLO is in periodic mode. >> >> Cc: Luiz Capitulino >> Cc: Eric Blake >> Cc: Markus Armbruster >> Signed-off-by: zhanghailiang >> Signed-off-by: Li Zhijian > > It's probably a good idea to make this x- but other than that: > OK, i will fix it, thanks. > Reviewed-by: Dr. David Alan Gilbert > > >> --- >> v11: >> - Move this patch ahead of the patch where uses 'checkpoint_delay' >> (Dave's suggestion) >> v10: >> - Fix related qmp command >> --- >> hmp.c | 7 +++++++ >> migration/migration.c | 23 ++++++++++++++++++++++- >> qapi-schema.json | 19 ++++++++++++++++--- >> qmp-commands.hx | 2 +- >> 4 files changed, 46 insertions(+), 5 deletions(-) >> >> diff --git a/hmp.c b/hmp.c >> index 2140605..ec038f1 100644 >> --- a/hmp.c >> +++ b/hmp.c >> @@ -284,6 +284,9 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict) >> monitor_printf(mon, " %s: %" PRId64, >> MigrationParameter_lookup[MIGRATION_PARAMETER_X_CPU_THROTTLE_INCREMENT], >> params->x_cpu_throttle_increment); >> + monitor_printf(mon, " %s: %" PRId64, >> + MigrationParameter_lookup[MIGRATION_PARAMETER_CHECKPOINT_DELAY], >> + params->checkpoint_delay); >> monitor_printf(mon, "\n"); >> } >> >> @@ -1237,6 +1240,7 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) >> bool has_decompress_threads = false; >> bool has_x_cpu_throttle_initial = false; >> bool has_x_cpu_throttle_increment = false; >> + bool has_checkpoint_delay = false; >> int i; >> >> for (i = 0; i < MIGRATION_PARAMETER_MAX; i++) { >> @@ -1256,6 +1260,8 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) >> break; >> case MIGRATION_PARAMETER_X_CPU_THROTTLE_INCREMENT: >> has_x_cpu_throttle_increment = true; >> + case MIGRATION_PARAMETER_CHECKPOINT_DELAY: >> + has_checkpoint_delay = true; >> break; >> } >> qmp_migrate_set_parameters(has_compress_level, value, >> @@ -1263,6 +1269,7 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) >> has_decompress_threads, value, >> has_x_cpu_throttle_initial, value, >> has_x_cpu_throttle_increment, value, >> + has_checkpoint_delay, value, >> &err); >> break; >> } >> diff --git a/migration/migration.c b/migration/migration.c >> index a4c690d..48545f3 100644 >> --- a/migration/migration.c >> +++ b/migration/migration.c >> @@ -56,6 +56,11 @@ >> /* Migration XBZRLE default cache size */ >> #define DEFAULT_MIGRATE_CACHE_SIZE (64 * 1024 * 1024) >> >> +/* The delay time (in ms) between two COLO checkpoints >> + * Note: Please change this default value to 10000 when we support hybrid mode. >> + */ >> +#define DEFAULT_MIGRATE_CHECKPOINT_DELAY 200 >> + >> static NotifierList migration_state_notifiers = >> NOTIFIER_LIST_INITIALIZER(migration_state_notifiers); >> >> @@ -91,6 +96,8 @@ MigrationState *migrate_get_current(void) >> DEFAULT_MIGRATE_X_CPU_THROTTLE_INITIAL, >> .parameters[MIGRATION_PARAMETER_X_CPU_THROTTLE_INCREMENT] = >> DEFAULT_MIGRATE_X_CPU_THROTTLE_INCREMENT, >> + .parameters[MIGRATION_PARAMETER_CHECKPOINT_DELAY] = >> + DEFAULT_MIGRATE_CHECKPOINT_DELAY, >> }; >> >> if (!once) { >> @@ -530,6 +537,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp) >> s->parameters[MIGRATION_PARAMETER_X_CPU_THROTTLE_INITIAL]; >> params->x_cpu_throttle_increment = >> s->parameters[MIGRATION_PARAMETER_X_CPU_THROTTLE_INCREMENT]; >> + params->checkpoint_delay = >> + s->parameters[MIGRATION_PARAMETER_CHECKPOINT_DELAY]; >> >> return params; >> } >> @@ -736,7 +745,10 @@ void qmp_migrate_set_parameters(bool has_compress_level, >> bool has_x_cpu_throttle_initial, >> int64_t x_cpu_throttle_initial, >> bool has_x_cpu_throttle_increment, >> - int64_t x_cpu_throttle_increment, Error **errp) >> + int64_t x_cpu_throttle_increment, >> + bool has_checkpoint_delay, >> + int64_t checkpoint_delay, >> + Error **errp) >> { >> MigrationState *s = migrate_get_current(); >> >> @@ -771,6 +783,11 @@ void qmp_migrate_set_parameters(bool has_compress_level, >> "x_cpu_throttle_increment", >> "an integer in the range of 1 to 99"); >> } >> + if (has_checkpoint_delay && (checkpoint_delay < 0)) { >> + error_setg(errp, QERR_INVALID_PARAMETER_VALUE, >> + "checkpoint_delay", >> + "is invalid, it should be positive"); >> + } >> >> if (has_compress_level) { >> s->parameters[MIGRATION_PARAMETER_COMPRESS_LEVEL] = compress_level; >> @@ -791,6 +808,10 @@ void qmp_migrate_set_parameters(bool has_compress_level, >> s->parameters[MIGRATION_PARAMETER_X_CPU_THROTTLE_INCREMENT] = >> x_cpu_throttle_increment; >> } >> + >> + if (has_checkpoint_delay) { >> + s->parameters[MIGRATION_PARAMETER_CHECKPOINT_DELAY] = checkpoint_delay; >> + } >> } >> >> void qmp_migrate_start_postcopy(Error **errp) >> diff --git a/qapi-schema.json b/qapi-schema.json >> index 5ee089d..b4b47ed 100644 >> --- a/qapi-schema.json >> +++ b/qapi-schema.json >> @@ -625,11 +625,16 @@ >> # @x-cpu-throttle-increment: throttle percentage increase each time >> # auto-converge detects that migration is not making >> # progress. The default value is 10. (Since 2.5) >> +# >> +# @checkpoint-delay: The delay time (in ms) between two COLO checkpoints in >> +# periodic mode. (Since 2.6) >> +# >> # Since: 2.4 >> ## >> { 'enum': 'MigrationParameter', >> 'data': ['compress-level', 'compress-threads', 'decompress-threads', >> - 'x-cpu-throttle-initial', 'x-cpu-throttle-increment'] } >> + 'x-cpu-throttle-initial', 'x-cpu-throttle-increment', >> + 'checkpoint-delay' ] } >> >> # >> # @migrate-set-parameters >> @@ -649,6 +654,9 @@ >> # @x-cpu-throttle-increment: throttle percentage increase each time >> # auto-converge detects that migration is not making >> # progress. The default value is 10. (Since 2.5) >> +# >> +# @checkpoint-delay: the delay time between two checkpoints. (Since 2.6) >> +# >> # Since: 2.4 >> ## >> { 'command': 'migrate-set-parameters', >> @@ -656,7 +664,8 @@ >> '*compress-threads': 'int', >> '*decompress-threads': 'int', >> '*x-cpu-throttle-initial': 'int', >> - '*x-cpu-throttle-increment': 'int'} } >> + '*x-cpu-throttle-increment': 'int', >> + '*checkpoint-delay': 'int' } } >> >> # >> # @MigrationParameters >> @@ -675,6 +684,8 @@ >> # auto-converge detects that migration is not making >> # progress. The default value is 10. (Since 2.5) >> # >> +# @checkpoint-delay: the delay time between two COLO checkpoints >> +# >> # Since: 2.4 >> ## >> { 'struct': 'MigrationParameters', >> @@ -682,7 +693,9 @@ >> 'compress-threads': 'int', >> 'decompress-threads': 'int', >> 'x-cpu-throttle-initial': 'int', >> - 'x-cpu-throttle-increment': 'int'} } >> + 'x-cpu-throttle-increment': 'int', >> + 'checkpoint-delay': 'int'} } >> + >> ## >> # @query-migrate-parameters >> # >> diff --git a/qmp-commands.hx b/qmp-commands.hx >> index 92042d8..c39f2f0 100644 >> --- a/qmp-commands.hx >> +++ b/qmp-commands.hx >> @@ -3664,7 +3664,7 @@ EQMP >> { >> .name = "migrate-set-parameters", >> .args_type = >> - "compress-level:i?,compress-threads:i?,decompress-threads:i?", >> + "compress-level:i?,compress-threads:i?,decompress-threads:i?,checkpoint-delay:i?", >> .mhandler.cmd_new = qmp_marshal_migrate_set_parameters, >> }, >> SQMP >> -- >> 1.8.3.1 >> >> > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK > > . >