| Message ID | 1438159544-6224-32-git-send-email-zhang.zhanghailiang@huawei.com |
|---|---|
| State | New |
| Headers | show |
On 07/29/2015 02:45 AM, zhanghailiang wrote: > With this command, we can control the period of checkpoint, if > there is no comparison of net packets. > > 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> > --- > hmp-commands.hx | 15 +++++++++++++++ > hmp.c | 7 +++++++ > hmp.h | 1 + > migration/colo.c | 11 ++++++++++- > qapi-schema.json | 13 +++++++++++++ > qmp-commands.hx | 22 ++++++++++++++++++++++ > stubs/migration-colo.c | 4 ++++ > 7 files changed, 72 insertions(+), 1 deletion(-) Interface review: > +++ b/qapi-schema.json > @@ -691,6 +691,19 @@ > { 'command': 'colo-lost-heartbeat' } > > ## > +# @colo-set-checkpoint-period > +# > +# Set colo checkpoint period > +# > +# @value: period of colo checkpoint in ms > +# > +# Returns: nothing on success Redundant line; you could omit it. > +# > +# Since: 2.4 2.5 > +## > +{ 'command': 'colo-set-checkpoint-period', 'data': {'value': 'int'} } I hate write-only interfaces; where can I query the current period? > +++ b/stubs/migration-colo.c > @@ -52,3 +52,7 @@ void qmp_colo_lost_heartbeat(Error **errp) > " with --enable-colo option in order to support" > " COLO feature"); > } > + > +void qmp_colo_set_checkpoint_period(int64_t value, Error **errp) > +{ > +} Shouldn't the stub function set an error, rather than being a no-op?
On 2015/8/29 6:26, Eric Blake wrote: > On 07/29/2015 02:45 AM, zhanghailiang wrote: >> With this command, we can control the period of checkpoint, if >> there is no comparison of net packets. >> >> 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> >> --- >> hmp-commands.hx | 15 +++++++++++++++ >> hmp.c | 7 +++++++ >> hmp.h | 1 + >> migration/colo.c | 11 ++++++++++- >> qapi-schema.json | 13 +++++++++++++ >> qmp-commands.hx | 22 ++++++++++++++++++++++ >> stubs/migration-colo.c | 4 ++++ >> 7 files changed, 72 insertions(+), 1 deletion(-) > > Interface review: > >> +++ b/qapi-schema.json >> @@ -691,6 +691,19 @@ >> { 'command': 'colo-lost-heartbeat' } >> >> ## >> +# @colo-set-checkpoint-period >> +# >> +# Set colo checkpoint period >> +# >> +# @value: period of colo checkpoint in ms >> +# >> +# Returns: nothing on success > > Redundant line; you could omit it. > >> +# >> +# Since: 2.4 > > 2.5 > >> +## >> +{ 'command': 'colo-set-checkpoint-period', 'data': {'value': 'int'} } > > I hate write-only interfaces; where can I query the current period? > Yes, it is not graceful, actually, this should be convert to use migrate-set-parameters/query-migrate-parameters commands to set/get the value, just as Dave's "[RFC/COLO: 1/3] COLO: Hybrid mode" patch does. But these two commands should be reconstruct. Markus has promised to do this. Hi Markus, is it still in your schedule ? I will convert command to use migrate-set-parameters/query-migrate-parameters temporarily for next version. thanks. >> +++ b/stubs/migration-colo.c >> @@ -52,3 +52,7 @@ void qmp_colo_lost_heartbeat(Error **errp) >> " with --enable-colo option in order to support" >> " COLO feature"); >> } >> + >> +void qmp_colo_set_checkpoint_period(int64_t value, Error **errp) >> +{ >> +} > > Shouldn't the stub function set an error, rather than being a no-op? > Yes, we should set an error, will fix in next version, thanks.
diff --git a/hmp-commands.hx b/hmp-commands.hx index 410637f..9164961 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -1049,6 +1049,21 @@ Tell COLO that heartbeat is lost, a failover or takeover is needed. ETEXI { + .name = "colo_set_checkpoint_period", + .args_type = "value:i", + .params = "value", + .help = "set checkpoint period (in ms) for colo. " + "Defaults to 100ms", + .mhandler.cmd = hmp_colo_set_checkpoint_period, + }, + +STEXI +@item migrate_set_checkpoint_period @var{value} +@findex migrate_set_checkpoint_period +Set checkpoint period to @var{value} (in ms) for colo. +ETEXI + + { .name = "client_migrate_info", .args_type = "protocol:s,hostname:s,port:i?,tls-port:i?,cert-subject:s?", .params = "protocol hostname port tls-port cert-subject", diff --git a/hmp.c b/hmp.c index 7f3a8a9..c464fc9 100644 --- a/hmp.c +++ b/hmp.c @@ -1280,6 +1280,13 @@ void hmp_colo_lost_heartbeat(Monitor *mon, const QDict *qdict) hmp_handle_error(mon, &err); } +void hmp_colo_set_checkpoint_period(Monitor *mon, const QDict *qdict) +{ + int64_t value = qdict_get_int(qdict, "value"); + + qmp_colo_set_checkpoint_period(value, NULL); +} + void hmp_set_password(Monitor *mon, const QDict *qdict) { const char *protocol = qdict_get_str(qdict, "protocol"); diff --git a/hmp.h b/hmp.h index c36c99c..d66dc76 100644 --- a/hmp.h +++ b/hmp.h @@ -69,6 +69,7 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict); void hmp_migrate_set_cache_size(Monitor *mon, const QDict *qdict); void hmp_client_migrate_info(Monitor *mon, const QDict *qdict); void hmp_colo_lost_heartbeat(Monitor *mon, const QDict *qdict); +void hmp_colo_set_checkpoint_period(Monitor *mon, const QDict *qdict); void hmp_set_password(Monitor *mon, const QDict *qdict); void hmp_expire_password(Monitor *mon, const QDict *qdict); void hmp_eject(Monitor *mon, const QDict *qdict); diff --git a/migration/colo.c b/migration/colo.c index f5bb668..b466959 100644 --- a/migration/colo.c +++ b/migration/colo.c @@ -19,6 +19,7 @@ #include "migration/failover.h" #include "qapi-event.h" #include "net/colo-nic.h" +#include "qmp-commands.h" /* * We should not do checkpoint one after another without any time interval, @@ -84,6 +85,9 @@ const char * const COLOCommand_lookup[] = { static QEMUBH *colo_bh; static bool vmstate_loading; + +int64_t colo_checkpoint_period = CHECKPOINT_MAX_PEROID; + /* colo buffer */ #define COLO_BUFFER_BASE_SIZE (4 * 1024 * 1024) @@ -106,6 +110,11 @@ bool migration_incoming_in_colo_state(void) return (mis && (mis->state == MIGRATION_STATUS_COLO)); } +void qmp_colo_set_checkpoint_period(int64_t value, Error **errp) +{ + colo_checkpoint_period = value; +} + static bool colo_runstate_is_stopped(void) { return runstate_check(RUN_STATE_COLO) || !runstate_is_running(); @@ -419,7 +428,7 @@ static void *colo_thread(void *opaque) * and then check if we need checkpoint again. */ current_time = qemu_clock_get_ms(QEMU_CLOCK_HOST); - if (current_time - checkpoint_time < CHECKPOINT_MAX_PEROID) { + if (current_time - checkpoint_time < colo_checkpoint_period) { g_usleep(100000); continue; } diff --git a/qapi-schema.json b/qapi-schema.json index 0460dad..9bd282d 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -691,6 +691,19 @@ { 'command': 'colo-lost-heartbeat' } ## +# @colo-set-checkpoint-period +# +# Set colo checkpoint period +# +# @value: period of colo checkpoint in ms +# +# Returns: nothing on success +# +# Since: 2.4 +## +{ 'command': 'colo-set-checkpoint-period', 'data': {'value': 'int'} } + +## # @MouseInfo: # # Information about a mouse device. diff --git a/qmp-commands.hx b/qmp-commands.hx index 28a7962..4fd01a7 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -800,6 +800,28 @@ Example: EQMP { + .name = "colo-set-checkpoint-period", + .args_type = "value:i", + .mhandler.cmd_new = qmp_marshal_input_colo_set_checkpoint_period, + }, + +SQMP +colo-set-checkpoint-period +-------------------------- + +set checkpoint period + +Arguments: +- "value": checkpoint period + +Example: + +-> { "execute": "colo-set-checkpoint-period", "arguments": { "value": "1000" } } +<- { "return": {} } + +EQMP + + { .name = "client_migrate_info", .args_type = "protocol:s,hostname:s,port:i?,tls-port:i?,cert-subject:s?", .params = "protocol hostname port tls-port cert-subject", diff --git a/stubs/migration-colo.c b/stubs/migration-colo.c index 0edc59c..9d3b9e7 100644 --- a/stubs/migration-colo.c +++ b/stubs/migration-colo.c @@ -52,3 +52,7 @@ void qmp_colo_lost_heartbeat(Error **errp) " with --enable-colo option in order to support" " COLO feature"); } + +void qmp_colo_set_checkpoint_period(int64_t value, Error **errp) +{ +}