| Message ID | 1476792613-11712-11-git-send-email-zhang.zhanghailiang@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On (Tue) 18 Oct 2016 [20:10:06], zhanghailiang 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 <lcapitulino@redhat.com> > Cc: Eric Blake <eblake@redhat.com> > Cc: Markus Armbruster <armbru@redhat.com> > Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> > Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com> > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > diff --git a/hmp.c b/hmp.c > index 80f7f1f..759f4f4 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -318,6 +318,9 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict) > monitor_printf(mon, " %s: %" PRId64 " milliseconds", > MigrationParameter_lookup[MIGRATION_PARAMETER_DOWNTIME_LIMIT], > params->downtime_limit); > + monitor_printf(mon, " %s: %" PRId64, > + MigrationParameter_lookup[MIGRATION_PARAMETER_X_CHECKPOINT_DELAY], > + params->x_checkpoint_delay); > monitor_printf(mon, "\n"); > } > > @@ -1363,7 +1366,6 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) > case MIGRATION_PARAMETER_CPU_THROTTLE_INCREMENT: > p.has_cpu_throttle_increment = true; > use_int_value = true; > - break; Hm? > case MIGRATION_PARAMETER_TLS_CREDS: > p.has_tls_creds = true; > p.tls_creds = (char *) valuestr; > @@ -1386,6 +1388,10 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) Amit
On 10/26/2016 12:45 AM, Amit Shah wrote: > On (Tue) 18 Oct 2016 [20:10:06], zhanghailiang 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 <lcapitulino@redhat.com> >> Cc: Eric Blake <eblake@redhat.com> >> Cc: Markus Armbruster <armbru@redhat.com> >> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> >> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com> >> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > > >> @@ -1363,7 +1366,6 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) >> case MIGRATION_PARAMETER_CPU_THROTTLE_INCREMENT: >> p.has_cpu_throttle_increment = true; >> use_int_value = true; >> - break; > > Hm? > Looks like an improper conflict resolution after commit bb2b777. Definitely a hunk that does not belong in this patch; but with it removed, the rest of the patch earns: Reviewed-by: Eric Blake <eblake@redhat.com>
On 2016/10/26 13:45, Amit Shah wrote: > On (Tue) 18 Oct 2016 [20:10:06], zhanghailiang 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 <lcapitulino@redhat.com> >> Cc: Eric Blake <eblake@redhat.com> >> Cc: Markus Armbruster <armbru@redhat.com> >> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> >> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com> >> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > > >> diff --git a/hmp.c b/hmp.c >> index 80f7f1f..759f4f4 100644 >> --- a/hmp.c >> +++ b/hmp.c >> @@ -318,6 +318,9 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict) >> monitor_printf(mon, " %s: %" PRId64 " milliseconds", >> MigrationParameter_lookup[MIGRATION_PARAMETER_DOWNTIME_LIMIT], >> params->downtime_limit); >> + monitor_printf(mon, " %s: %" PRId64, >> + MigrationParameter_lookup[MIGRATION_PARAMETER_X_CHECKPOINT_DELAY], >> + params->x_checkpoint_delay); >> monitor_printf(mon, "\n"); >> } >> >> @@ -1363,7 +1366,6 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) >> case MIGRATION_PARAMETER_CPU_THROTTLE_INCREMENT: >> p.has_cpu_throttle_increment = true; >> use_int_value = true; >> - break; > > Hm? Oops, It seems that i remove it accidently while rebase to upstream. I'll fix it in next version, thanks > >> case MIGRATION_PARAMETER_TLS_CREDS: >> p.has_tls_creds = true; >> p.tls_creds = (char *) valuestr; >> @@ -1386,6 +1388,10 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) > > > Amit > > . >
Hi Eric, On 2016/10/26 21:39, Eric Blake wrote: > On 10/26/2016 12:45 AM, Amit Shah wrote: >> On (Tue) 18 Oct 2016 [20:10:06], zhanghailiang 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 <lcapitulino@redhat.com> >>> Cc: Eric Blake <eblake@redhat.com> >>> Cc: Markus Armbruster <armbru@redhat.com> >>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> >>> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com> >>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> >> >> > >>> @@ -1363,7 +1366,6 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) >>> case MIGRATION_PARAMETER_CPU_THROTTLE_INCREMENT: >>> p.has_cpu_throttle_increment = true; >>> use_int_value = true; >>> - break; >> >> Hm? >> > > Looks like an improper conflict resolution after commit bb2b777. > Definitely a hunk that does not belong in this patch; but with it > removed, the rest of the patch earns: > Thanks very much, yes, I accidentally remove it while fixed the conflict with the upstream. I'll fix it in next version :) > Reviewed-by: Eric Blake <eblake@redhat.com> >
diff --git a/docs/qmp-commands.txt b/docs/qmp-commands.txt index bf2b77d..13ab523 100644 --- a/docs/qmp-commands.txt +++ b/docs/qmp-commands.txt @@ -2916,6 +2916,8 @@ Set migration parameters - "max-bandwidth": set maximum speed for migrations (in bytes/sec) (json-int) - "downtime-limit": set maximum tolerated downtime (in milliseconds) for migrations (json-int) +- "x-checkpoint-delay": set the delay time for periodic checkpoint (json-int) + Arguments: Example: diff --git a/hmp.c b/hmp.c index 80f7f1f..759f4f4 100644 --- a/hmp.c +++ b/hmp.c @@ -318,6 +318,9 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict) monitor_printf(mon, " %s: %" PRId64 " milliseconds", MigrationParameter_lookup[MIGRATION_PARAMETER_DOWNTIME_LIMIT], params->downtime_limit); + monitor_printf(mon, " %s: %" PRId64, + MigrationParameter_lookup[MIGRATION_PARAMETER_X_CHECKPOINT_DELAY], + params->x_checkpoint_delay); monitor_printf(mon, "\n"); } @@ -1363,7 +1366,6 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) case MIGRATION_PARAMETER_CPU_THROTTLE_INCREMENT: p.has_cpu_throttle_increment = true; use_int_value = true; - break; case MIGRATION_PARAMETER_TLS_CREDS: p.has_tls_creds = true; p.tls_creds = (char *) valuestr; @@ -1386,6 +1388,10 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) p.has_downtime_limit = true; use_int_value = true; break; + case MIGRATION_PARAMETER_X_CHECKPOINT_DELAY: + p.has_x_checkpoint_delay = true; + use_int_value = true; + break; } if (use_int_value) { @@ -1402,6 +1408,7 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) p.cpu_throttle_initial = valueint; p.cpu_throttle_increment = valueint; p.downtime_limit = valueint; + p.x_checkpoint_delay = valueint; } qmp_migrate_set_parameters(&p, &err); diff --git a/migration/migration.c b/migration/migration.c index 13ea7e9..c44e129 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -63,6 +63,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_X_CHECKPOINT_DELAY 200 + static NotifierList migration_state_notifiers = NOTIFIER_LIST_INITIALIZER(migration_state_notifiers); @@ -95,6 +100,7 @@ MigrationState *migrate_get_current(void) .cpu_throttle_increment = DEFAULT_MIGRATE_CPU_THROTTLE_INCREMENT, .max_bandwidth = MAX_THROTTLE, .downtime_limit = DEFAULT_MIGRATE_SET_DOWNTIME, + .x_checkpoint_delay = DEFAULT_MIGRATE_X_CHECKPOINT_DELAY, }, }; @@ -587,6 +593,7 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp) params->max_bandwidth = s->parameters.max_bandwidth; params->has_downtime_limit = true; params->downtime_limit = s->parameters.downtime_limit; + params->x_checkpoint_delay = s->parameters.x_checkpoint_delay; return params; } @@ -845,6 +852,11 @@ void qmp_migrate_set_parameters(MigrationParameters *params, Error **errp) "an integer in the range of 0 to 2000000 milliseconds"); return; } + if (params->has_x_checkpoint_delay && (params->x_checkpoint_delay < 0)) { + error_setg(errp, QERR_INVALID_PARAMETER_VALUE, + "x_checkpoint_delay", + "is invalid, it should be positive"); + } if (params->has_compress_level) { s->parameters.compress_level = params->compress_level; @@ -879,6 +891,10 @@ void qmp_migrate_set_parameters(MigrationParameters *params, Error **errp) if (params->has_downtime_limit) { s->parameters.downtime_limit = params->downtime_limit; } + + if (params->has_x_checkpoint_delay) { + s->parameters.x_checkpoint_delay = params->x_checkpoint_delay; + } } diff --git a/qapi-schema.json b/qapi-schema.json index 85c571b..b6950be 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -673,19 +673,24 @@ # @downtime-limit: set maximum tolerated downtime for migration. maximum # downtime in milliseconds (Since 2.8) # +# @x-checkpoint-delay: The delay time (in ms) between two COLO checkpoints in +# periodic mode. (Since 2.8) +# # Since: 2.4 ## { 'enum': 'MigrationParameter', 'data': ['compress-level', 'compress-threads', 'decompress-threads', 'cpu-throttle-initial', 'cpu-throttle-increment', 'tls-creds', 'tls-hostname', 'max-bandwidth', - 'downtime-limit'] } + 'downtime-limit', 'x-checkpoint-delay' ] } # # @migrate-set-parameters # # Set various migration parameters. See MigrationParameters for details. # +# @x-checkpoint-delay: the delay time between two checkpoints. (Since 2.8) +# # Since: 2.4 ## { 'command': 'migrate-set-parameters', 'boxed': true, @@ -734,6 +739,8 @@ # @downtime-limit: set maximum tolerated downtime for migration. maximum # downtime in milliseconds (Since 2.8) # +# @x-checkpoint-delay: the delay time between two COLO checkpoints. (Since 2.8) +# # Since: 2.4 ## { 'struct': 'MigrationParameters', @@ -745,7 +752,9 @@ '*tls-creds': 'str', '*tls-hostname': 'str', '*max-bandwidth': 'int', - '*downtime-limit': 'int'} } + '*downtime-limit': 'int', + '*x-checkpoint-delay': 'int'} } + ## # @query-migrate-parameters #