| Message ID | 1473085586-6834-1-git-send-email-ashijeetacharya@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Sep 05, 2016 at 07:56:26PM +0530, Ashijeet Acharya wrote: > Include migrate_set_speed and migrate_set_downtime inside migrate_set_parameters for setting maximum migration speed and expected downtime parameters respectively. Also update the query part for both in qmp and hmp qemu control interfaces. Please put line breaks in your commit message at 72 characters Also, when sending v2, v3, etc it is preferred to send it as a new top-level thread. ie, don't set headers to be a reply to your v1. > Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com> > --- > hmp.c | 33 +++++++++++++++++++++++++++++++++ > include/migration/migration.h | 1 - > migration/migration.c | 30 ++++++++++++++++++++++++++---- > qapi-schema.json | 26 +++++++++++++++++++++++--- > qmp-commands.hx | 13 ++++++++++--- > 5 files changed, 92 insertions(+), 11 deletions(-) > > diff --git a/hmp.c b/hmp.c > index cc2056e..c92769b 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -304,6 +304,12 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict) > monitor_printf(mon, " %s: '%s'", > MigrationParameter_lookup[MIGRATION_PARAMETER_TLS_HOSTNAME], > params->tls_hostname ? : ""); > + monitor_printf(mon, " %s: %" PRId64, > + MigrationParameter_lookup[MIGRATION_PARAMETER_MAX_BANDWIDTH], > + params->max_bandwidth); > + monitor_printf(mon, " %s: %" PRId64, > + MigrationParameter_lookup[MIGRATION_PARAMETER_DOWNTIME_LIMIT], > + params->downtime_limit); > monitor_printf(mon, "\n"); > } > > @@ -1193,6 +1199,7 @@ void hmp_migrate_incoming(Monitor *mon, const QDict *qdict) > hmp_handle_error(mon, &err); > } > > +/* Kept for old-commands compatibility */ I don't think this comment really adds any value. Instead, in the qapi schema, you should mark the legacy methods as deprecated though. > void hmp_migrate_set_downtime(Monitor *mon, const QDict *qdict) > { > double value = qdict_get_double(qdict, "value"); > @@ -1211,6 +1218,7 @@ void hmp_migrate_set_cache_size(Monitor *mon, const QDict *qdict) > } > } > > +/* Kept for old-commands compatibility */ > void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict) > { > int64_t value = qdict_get_int(qdict, "value"); > @@ -1251,7 +1259,10 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) > { > const char *param = qdict_get_str(qdict, "parameter"); > const char *valuestr = qdict_get_str(qdict, "value"); > + int64_t valuebw = 0; > + double valuedowntime = 0; > long valueint = 0; > + char *endp; > Error *err = NULL; > bool has_compress_level = false; > bool has_compress_threads = false; > @@ -1260,6 +1271,8 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) > bool has_cpu_throttle_increment = false; > bool has_tls_creds = false; > bool has_tls_hostname = false; > + bool has_max_bandwidth = false; > + bool has_downtime_limit = false; > bool use_int_value = false; > int i; > > @@ -1291,6 +1304,24 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) > case MIGRATION_PARAMETER_TLS_HOSTNAME: > has_tls_hostname = true; > break; > + case MIGRATION_PARAMETER_MAX_BANDWIDTH: > + has_max_bandwidth = true; > + valuebw = qemu_strtosz(valuestr, &endp); > + if (valuebw < 0 || (size_t)valuebw != valuebw || *endp != '\0' > + || !is_power_of_2(valuebw)) { > + error_setg(&err, "Invalid size %s", valuestr); > + goto cleanup; > + } > + break; > + case MIGRATION_PARAMETER_DOWNTIME_LIMIT: > + has_downtime_limit = true; > + valuedowntime = strtod(valuestr, &endp); > + if (valuestr == endp) { > + error_setg(&err, "Unable to parse '%s' as a number", > + valuestr); > + goto cleanup; > + } > + break; > } > > if (use_int_value) { > @@ -1308,6 +1339,8 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) > has_cpu_throttle_increment, valueint, > has_tls_creds, valuestr, > has_tls_hostname, valuestr, > + has_max_bandwidth, valuebw, > + has_downtime_limit, valuedowntime, > &err); > break; > } > diff --git a/include/migration/migration.h b/include/migration/migration.h > index 3c96623..a5429ee 100644 > --- a/include/migration/migration.h > +++ b/include/migration/migration.h > @@ -129,7 +129,6 @@ struct MigrationSrcPageRequest { > > struct MigrationState > { > - int64_t bandwidth_limit; > size_t bytes_xfer; > size_t xfer_limit; > QemuThread thread; > diff --git a/migration/migration.c b/migration/migration.c > index 955d5ee..4b54b58 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -44,6 +44,9 @@ > #define BUFFER_DELAY 100 > #define XFER_LIMIT_RATIO (1000 / BUFFER_DELAY) > > +/* Amount of nanoseconds we are willing to wait for migration to be down. */ > +#define DEFAULT_MIGRATE_SET_DOWNTIME 300000000 > + > /* Default compression thread count */ > #define DEFAULT_MIGRATE_COMPRESS_THREAD_COUNT 8 > /* Default decompression thread count, usually decompression is at > @@ -80,7 +83,6 @@ MigrationState *migrate_get_current(void) > static bool once; > static MigrationState current_migration = { > .state = MIGRATION_STATUS_NONE, > - .bandwidth_limit = MAX_THROTTLE, > .xbzrle_cache_size = DEFAULT_MIGRATE_CACHE_SIZE, > .mbps = -1, > .parameters = { > @@ -89,6 +91,8 @@ MigrationState *migrate_get_current(void) > .decompress_threads = DEFAULT_MIGRATE_DECOMPRESS_THREAD_COUNT, > .cpu_throttle_initial = DEFAULT_MIGRATE_CPU_THROTTLE_INITIAL, > .cpu_throttle_increment = DEFAULT_MIGRATE_CPU_THROTTLE_INCREMENT, > + .max_bandwidth = MAX_THROTTLE, > + .downtime_limit = DEFAULT_MIGRATE_SET_DOWNTIME, > }, > }; > > @@ -566,6 +570,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp) > params->cpu_throttle_increment = s->parameters.cpu_throttle_increment; > params->tls_creds = g_strdup(s->parameters.tls_creds); > params->tls_hostname = g_strdup(s->parameters.tls_hostname); > + params->max_bandwidth = s->parameters.max_bandwidth; > + params->downtime_limit = s->parameters.downtime_limit; > > return params; > } > @@ -773,6 +779,10 @@ void qmp_migrate_set_parameters(bool has_compress_level, > const char *tls_creds, > bool has_tls_hostname, > const char *tls_hostname, > + bool has_max_bandwidth, > + int64_t max_bandwidth, > + bool has_downtime_limit, > + double downtime_limit, > Error **errp) > { > MigrationState *s = migrate_get_current(); > @@ -832,6 +842,12 @@ void qmp_migrate_set_parameters(bool has_compress_level, > g_free(s->parameters.tls_hostname); > s->parameters.tls_hostname = g_strdup(tls_hostname); > } > + if (has_max_bandwidth) { > + qmp_migrate_set_speed(max_bandwidth, NULL); > + } > + if (has_downtime_limit) { > + qmp_migrate_set_downtime(downtime_limit, NULL); > + } > } > > > @@ -1175,18 +1191,24 @@ void qmp_migrate_set_speed(int64_t value, Error **errp) > } > > s = migrate_get_current(); > - s->bandwidth_limit = value; > + s->parameters.max_bandwidth = value; > if (s->to_dst_file) { > qemu_file_set_rate_limit(s->to_dst_file, > - s->bandwidth_limit / XFER_LIMIT_RATIO); > + s->parameters.max_bandwidth / XFER_LIMIT_RATIO); > } > } > > void qmp_migrate_set_downtime(double value, Error **errp) > { > + MigrationState *s; > + > value *= 1e9; > value = MAX(0, MIN(UINT64_MAX, value)); > + > + s = migrate_get_current(); > + > max_downtime = (uint64_t)value; > + s->parameters.downtime_limit = max_downtime; > } > > bool migrate_postcopy_ram(void) > @@ -1858,7 +1880,7 @@ void migrate_fd_connect(MigrationState *s) > > qemu_file_set_blocking(s->to_dst_file, true); > qemu_file_set_rate_limit(s->to_dst_file, > - s->bandwidth_limit / XFER_LIMIT_RATIO); > + s->parameters.max_bandwidth / XFER_LIMIT_RATIO); > > /* Notify before starting migration thread */ > notifier_list_notify(&migration_state_notifiers, s); > diff --git a/qapi-schema.json b/qapi-schema.json > index 5658723..250eac5 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -637,12 +637,18 @@ > # hostname must be provided so that the server's x509 > # certificate identity can be validated. (Since 2.7) > # > +# @max-bandwidth: to set maximum speed for migration. A value lesser than > +# zero will be automatically round upto zero. Since 2.8) Document the units for this ? eg is it bits-per-second, kb-per-second, mb-per-second, etc > +# > +# @downtime-limit: set maximum tolerated downtime for migration. Since 2.8) Again, units ? nanoseconds ? milliseconds ? > +# > # Since: 2.4 > ## > { 'enum': 'MigrationParameter', > 'data': ['compress-level', 'compress-threads', 'decompress-threads', > 'cpu-throttle-initial', 'cpu-throttle-increment', > - 'tls-creds', 'tls-hostname'] } > + 'tls-creds', 'tls-hostname', 'max-bandwidth', > + 'downtime-limit'] } > > # > # @migrate-set-parameters > @@ -678,6 +684,11 @@ > # hostname must be provided so that the server's x509 > # certificate identity can be validated. (Since 2.7) > # > +# @max-bandwidth: to set maximum speed for migration. A value lesser than > +# zero will be automatically round upto zero. Since 2.8) Units > +# > +# @downtime-limit: set maximum tolerated downtime for migration. Since 2.8) Units > +# > # Since: 2.4 > ## > { 'command': 'migrate-set-parameters', > @@ -687,7 +698,9 @@ > '*cpu-throttle-initial': 'int', > '*cpu-throttle-increment': 'int', > '*tls-creds': 'str', > - '*tls-hostname': 'str'} } > + '*tls-hostname': 'str', > + '*max-bandwidth': 'int', > + '*downtime-limit': 'number'} } > > # > # @MigrationParameters > @@ -721,6 +734,11 @@ > # hostname must be provided so that the server's x509 > # certificate identity can be validated. (Since 2.7) > # > +# @max-bandwidth: to set maximum speed for migration. A value lesser than > +# zero will be automatically round upto zero. (Since 2.8) > +# > +# @downtime-limit: set maximum tolerated downtime for migration. Since 2.8) > +# > # Since: 2.4 > ## > { 'struct': 'MigrationParameters', > @@ -730,7 +748,9 @@ > 'cpu-throttle-initial': 'int', > 'cpu-throttle-increment': 'int', > 'tls-creds': 'str', > - 'tls-hostname': 'str'} } > + 'tls-hostname': 'str', > + 'max-bandwidth': 'int', > + 'downtime-limit': 'int'} } > ## > # @query-migrate-parameters > # > diff --git a/qmp-commands.hx b/qmp-commands.hx > index 6866264..0418cab 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -3790,7 +3790,9 @@ Set migration parameters > throttled for auto-converge (json-int) > - "cpu-throttle-increment": set throttle increasing percentage for > auto-converge (json-int) > - > +- "max-bandwidth": set maximum speed for migrations (json-int) Document units > +- "downtime-limit": set maximum tolerated downtime (in seconds) for > + migrations (json-number) I'm sure units for this are not "seconds" as that is way too coarse. > Arguments: > > Example: > @@ -3803,7 +3805,7 @@ EQMP > { > .name = "migrate-set-parameters", > .args_type = > - "compress-level:i?,compress-threads:i?,decompress-threads:i?,cpu-throttle-initial:i?,cpu-throttle-increment:i?", > + "compress-level:i?,compress-threads:i?,decompress-threads:i?,cpu-throttle-initial:i?,cpu-throttle-increment:i?,max-bandwidth:i?,downtime-limit:T?", > .mhandler.cmd_new = qmp_marshal_migrate_set_parameters, > }, > SQMP > @@ -3820,6 +3822,9 @@ Query current migration parameters > throttled (json-int) > - "cpu-throttle-increment" : throttle increasing percentage for > auto-converge (json-int) > + - "max-bandwidth" : maximium migration speed (json-int) > + - "downtime-limit" : maximum tolerated downtime of migration > + (json-int) Document units Regards, Daniel
On Mon, Sep 5, 2016 at 8:06 PM, Daniel P. Berrange <berrange@redhat.com> wrote: > On Mon, Sep 05, 2016 at 07:56:26PM +0530, Ashijeet Acharya wrote: >> Include migrate_set_speed and migrate_set_downtime inside migrate_set_parameters for setting maximum migration speed and expected downtime parameters respectively. Also update the query part for both in qmp and hmp qemu control interfaces. > > Please put line breaks in your commit message at 72 characters > > > Also, when sending v2, v3, etc it is preferred to send it as a new > top-level thread. ie, don't set headers to be a reply to your v1. Alright i will keep that in mind from now onwards. > >> Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com> >> --- >> hmp.c | 33 +++++++++++++++++++++++++++++++++ >> include/migration/migration.h | 1 - >> migration/migration.c | 30 ++++++++++++++++++++++++++---- >> qapi-schema.json | 26 +++++++++++++++++++++++--- >> qmp-commands.hx | 13 ++++++++++--- >> 5 files changed, 92 insertions(+), 11 deletions(-) >> >> diff --git a/hmp.c b/hmp.c >> index cc2056e..c92769b 100644 >> --- a/hmp.c >> +++ b/hmp.c >> @@ -304,6 +304,12 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict) >> monitor_printf(mon, " %s: '%s'", >> MigrationParameter_lookup[MIGRATION_PARAMETER_TLS_HOSTNAME], >> params->tls_hostname ? : ""); >> + monitor_printf(mon, " %s: %" PRId64, >> + MigrationParameter_lookup[MIGRATION_PARAMETER_MAX_BANDWIDTH], >> + params->max_bandwidth); >> + monitor_printf(mon, " %s: %" PRId64, >> + MigrationParameter_lookup[MIGRATION_PARAMETER_DOWNTIME_LIMIT], >> + params->downtime_limit); >> monitor_printf(mon, "\n"); >> } >> >> @@ -1193,6 +1199,7 @@ void hmp_migrate_incoming(Monitor *mon, const QDict *qdict) >> hmp_handle_error(mon, &err); >> } >> >> +/* Kept for old-commands compatibility */ > > I don't think this comment really adds any value. Instead, in the qapi > schema, you should mark the legacy methods as deprecated though. Hmm, that would make more sense. > >> void hmp_migrate_set_downtime(Monitor *mon, const QDict *qdict) >> { >> double value = qdict_get_double(qdict, "value"); >> @@ -1211,6 +1218,7 @@ void hmp_migrate_set_cache_size(Monitor *mon, const QDict *qdict) >> } >> } >> >> +/* Kept for old-commands compatibility */ >> void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict) >> { >> int64_t value = qdict_get_int(qdict, "value"); >> @@ -1251,7 +1259,10 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) >> { >> const char *param = qdict_get_str(qdict, "parameter"); >> const char *valuestr = qdict_get_str(qdict, "value"); >> + int64_t valuebw = 0; >> + double valuedowntime = 0; >> long valueint = 0; >> + char *endp; >> Error *err = NULL; >> bool has_compress_level = false; >> bool has_compress_threads = false; >> @@ -1260,6 +1271,8 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) >> bool has_cpu_throttle_increment = false; >> bool has_tls_creds = false; >> bool has_tls_hostname = false; >> + bool has_max_bandwidth = false; >> + bool has_downtime_limit = false; >> bool use_int_value = false; >> int i; >> >> @@ -1291,6 +1304,24 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) >> case MIGRATION_PARAMETER_TLS_HOSTNAME: >> has_tls_hostname = true; >> break; >> + case MIGRATION_PARAMETER_MAX_BANDWIDTH: >> + has_max_bandwidth = true; >> + valuebw = qemu_strtosz(valuestr, &endp); >> + if (valuebw < 0 || (size_t)valuebw != valuebw || *endp != '\0' >> + || !is_power_of_2(valuebw)) { >> + error_setg(&err, "Invalid size %s", valuestr); >> + goto cleanup; >> + } >> + break; >> + case MIGRATION_PARAMETER_DOWNTIME_LIMIT: >> + has_downtime_limit = true; >> + valuedowntime = strtod(valuestr, &endp); >> + if (valuestr == endp) { >> + error_setg(&err, "Unable to parse '%s' as a number", >> + valuestr); >> + goto cleanup; >> + } >> + break; >> } >> >> if (use_int_value) { >> @@ -1308,6 +1339,8 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) >> has_cpu_throttle_increment, valueint, >> has_tls_creds, valuestr, >> has_tls_hostname, valuestr, >> + has_max_bandwidth, valuebw, >> + has_downtime_limit, valuedowntime, >> &err); >> break; >> } >> diff --git a/include/migration/migration.h b/include/migration/migration.h >> index 3c96623..a5429ee 100644 >> --- a/include/migration/migration.h >> +++ b/include/migration/migration.h >> @@ -129,7 +129,6 @@ struct MigrationSrcPageRequest { >> >> struct MigrationState >> { >> - int64_t bandwidth_limit; >> size_t bytes_xfer; >> size_t xfer_limit; >> QemuThread thread; >> diff --git a/migration/migration.c b/migration/migration.c >> index 955d5ee..4b54b58 100644 >> --- a/migration/migration.c >> +++ b/migration/migration.c >> @@ -44,6 +44,9 @@ >> #define BUFFER_DELAY 100 >> #define XFER_LIMIT_RATIO (1000 / BUFFER_DELAY) >> >> +/* Amount of nanoseconds we are willing to wait for migration to be down. */ >> +#define DEFAULT_MIGRATE_SET_DOWNTIME 300000000 >> + >> /* Default compression thread count */ >> #define DEFAULT_MIGRATE_COMPRESS_THREAD_COUNT 8 >> /* Default decompression thread count, usually decompression is at >> @@ -80,7 +83,6 @@ MigrationState *migrate_get_current(void) >> static bool once; >> static MigrationState current_migration = { >> .state = MIGRATION_STATUS_NONE, >> - .bandwidth_limit = MAX_THROTTLE, >> .xbzrle_cache_size = DEFAULT_MIGRATE_CACHE_SIZE, >> .mbps = -1, >> .parameters = { >> @@ -89,6 +91,8 @@ MigrationState *migrate_get_current(void) >> .decompress_threads = DEFAULT_MIGRATE_DECOMPRESS_THREAD_COUNT, >> .cpu_throttle_initial = DEFAULT_MIGRATE_CPU_THROTTLE_INITIAL, >> .cpu_throttle_increment = DEFAULT_MIGRATE_CPU_THROTTLE_INCREMENT, >> + .max_bandwidth = MAX_THROTTLE, >> + .downtime_limit = DEFAULT_MIGRATE_SET_DOWNTIME, >> }, >> }; >> >> @@ -566,6 +570,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp) >> params->cpu_throttle_increment = s->parameters.cpu_throttle_increment; >> params->tls_creds = g_strdup(s->parameters.tls_creds); >> params->tls_hostname = g_strdup(s->parameters.tls_hostname); >> + params->max_bandwidth = s->parameters.max_bandwidth; >> + params->downtime_limit = s->parameters.downtime_limit; >> >> return params; >> } >> @@ -773,6 +779,10 @@ void qmp_migrate_set_parameters(bool has_compress_level, >> const char *tls_creds, >> bool has_tls_hostname, >> const char *tls_hostname, >> + bool has_max_bandwidth, >> + int64_t max_bandwidth, >> + bool has_downtime_limit, >> + double downtime_limit, >> Error **errp) >> { >> MigrationState *s = migrate_get_current(); >> @@ -832,6 +842,12 @@ void qmp_migrate_set_parameters(bool has_compress_level, >> g_free(s->parameters.tls_hostname); >> s->parameters.tls_hostname = g_strdup(tls_hostname); >> } >> + if (has_max_bandwidth) { >> + qmp_migrate_set_speed(max_bandwidth, NULL); >> + } >> + if (has_downtime_limit) { >> + qmp_migrate_set_downtime(downtime_limit, NULL); >> + } >> } >> >> >> @@ -1175,18 +1191,24 @@ void qmp_migrate_set_speed(int64_t value, Error **errp) >> } >> >> s = migrate_get_current(); >> - s->bandwidth_limit = value; >> + s->parameters.max_bandwidth = value; >> if (s->to_dst_file) { >> qemu_file_set_rate_limit(s->to_dst_file, >> - s->bandwidth_limit / XFER_LIMIT_RATIO); >> + s->parameters.max_bandwidth / XFER_LIMIT_RATIO); >> } >> } >> >> void qmp_migrate_set_downtime(double value, Error **errp) >> { >> + MigrationState *s; >> + >> value *= 1e9; >> value = MAX(0, MIN(UINT64_MAX, value)); >> + >> + s = migrate_get_current(); >> + >> max_downtime = (uint64_t)value; >> + s->parameters.downtime_limit = max_downtime; >> } >> >> bool migrate_postcopy_ram(void) >> @@ -1858,7 +1880,7 @@ void migrate_fd_connect(MigrationState *s) >> >> qemu_file_set_blocking(s->to_dst_file, true); >> qemu_file_set_rate_limit(s->to_dst_file, >> - s->bandwidth_limit / XFER_LIMIT_RATIO); >> + s->parameters.max_bandwidth / XFER_LIMIT_RATIO); >> >> /* Notify before starting migration thread */ >> notifier_list_notify(&migration_state_notifiers, s); >> diff --git a/qapi-schema.json b/qapi-schema.json >> index 5658723..250eac5 100644 >> --- a/qapi-schema.json >> +++ b/qapi-schema.json >> @@ -637,12 +637,18 @@ >> # hostname must be provided so that the server's x509 >> # certificate identity can be validated. (Since 2.7) >> # >> +# @max-bandwidth: to set maximum speed for migration. A value lesser than >> +# zero will be automatically round upto zero. Since 2.8) > > Document the units for this ? eg is it bits-per-second, kb-per-second, > mb-per-second, etc > Should I document it the way it is for old-commands? Like this; # @migrate_set_speed # # Set maximum speed for migration. # # @value: maximum speed in bytes. # # Returns: nothing on success # # Notes: A value lesser than zero will be automatically round up to zero. # # Since: 0.14.0 ## >> +# >> +# @downtime-limit: set maximum tolerated downtime for migration. Since 2.8) > > Again, units ? nanoseconds ? milliseconds ? Like this; # @migrate_set_downtime # # Set maximum tolerated downtime for migration. # # @value: maximum downtime in seconds # # Returns: nothing on success # # Since: 0.14.0 > >> +# >> # Since: 2.4 >> ## >> { 'enum': 'MigrationParameter', >> 'data': ['compress-level', 'compress-threads', 'decompress-threads', >> 'cpu-throttle-initial', 'cpu-throttle-increment', >> - 'tls-creds', 'tls-hostname'] } >> + 'tls-creds', 'tls-hostname', 'max-bandwidth', >> + 'downtime-limit'] } >> >> # >> # @migrate-set-parameters >> @@ -678,6 +684,11 @@ >> # hostname must be provided so that the server's x509 >> # certificate identity can be validated. (Since 2.7) >> # >> +# @max-bandwidth: to set maximum speed for migration. A value lesser than >> +# zero will be automatically round upto zero. Since 2.8) > > Units > >> +# >> +# @downtime-limit: set maximum tolerated downtime for migration. Since 2.8) > > Units > >> +# >> # Since: 2.4 >> ## >> { 'command': 'migrate-set-parameters', >> @@ -687,7 +698,9 @@ >> '*cpu-throttle-initial': 'int', >> '*cpu-throttle-increment': 'int', >> '*tls-creds': 'str', >> - '*tls-hostname': 'str'} } >> + '*tls-hostname': 'str', >> + '*max-bandwidth': 'int', >> + '*downtime-limit': 'number'} } >> >> # >> # @MigrationParameters >> @@ -721,6 +734,11 @@ >> # hostname must be provided so that the server's x509 >> # certificate identity can be validated. (Since 2.7) >> # >> +# @max-bandwidth: to set maximum speed for migration. A value lesser than >> +# zero will be automatically round upto zero. (Since 2.8) >> +# >> +# @downtime-limit: set maximum tolerated downtime for migration. Since 2.8) >> +# >> # Since: 2.4 >> ## >> { 'struct': 'MigrationParameters', >> @@ -730,7 +748,9 @@ >> 'cpu-throttle-initial': 'int', >> 'cpu-throttle-increment': 'int', >> 'tls-creds': 'str', >> - 'tls-hostname': 'str'} } >> + 'tls-hostname': 'str', >> + 'max-bandwidth': 'int', >> + 'downtime-limit': 'int'} } >> ## >> # @query-migrate-parameters >> # >> diff --git a/qmp-commands.hx b/qmp-commands.hx >> index 6866264..0418cab 100644 >> --- a/qmp-commands.hx >> +++ b/qmp-commands.hx >> @@ -3790,7 +3790,9 @@ Set migration parameters >> throttled for auto-converge (json-int) >> - "cpu-throttle-increment": set throttle increasing percentage for >> auto-converge (json-int) >> - >> +- "max-bandwidth": set maximum speed for migrations (json-int) > > Document units > >> +- "downtime-limit": set maximum tolerated downtime (in seconds) for >> + migrations (json-number) > > I'm sure units for this are not "seconds" as that is way too coarse. I dont know, that is exactly what was documented for the old-command migrate_set_downtime. This is what i found; migrate_set_downtime -------------------- Set maximum tolerated downtime (in seconds) for migrations. Arguments: - "value": maximum downtime (json-number) Example: -> { "execute": "migrate_set_downtime", "arguments": { "value": 0.1 } } <- { "return": {} } > >> Arguments: >> >> Example: >> @@ -3803,7 +3805,7 @@ EQMP >> { >> .name = "migrate-set-parameters", >> .args_type = >> - "compress-level:i?,compress-threads:i?,decompress-threads:i?,cpu-throttle-initial:i?,cpu-throttle-increment:i?", >> + "compress-level:i?,compress-threads:i?,decompress-threads:i?,cpu-throttle-initial:i?,cpu-throttle-increment:i?,max-bandwidth:i?,downtime-limit:T?", >> .mhandler.cmd_new = qmp_marshal_migrate_set_parameters, >> }, >> SQMP >> @@ -3820,6 +3822,9 @@ Query current migration parameters >> throttled (json-int) >> - "cpu-throttle-increment" : throttle increasing percentage for >> auto-converge (json-int) >> + - "max-bandwidth" : maximium migration speed (json-int) >> + - "downtime-limit" : maximum tolerated downtime of migration >> + (json-int) > > Document units > > > Regards, > Daniel > -- > |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| > |: http://libvirt.org -o- http://virt-manager.org :| > |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| > |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Mon, Sep 05, 2016 at 08:44:26PM +0530, Ashijeet Acharya wrote: > On Mon, Sep 5, 2016 at 8:06 PM, Daniel P. Berrange <berrange@redhat.com> wrote: > > On Mon, Sep 05, 2016 at 07:56:26PM +0530, Ashijeet Acharya wrote: > >> diff --git a/qapi-schema.json b/qapi-schema.json > >> index 5658723..250eac5 100644 > >> --- a/qapi-schema.json > >> +++ b/qapi-schema.json > >> @@ -637,12 +637,18 @@ > >> # hostname must be provided so that the server's x509 > >> # certificate identity can be validated. (Since 2.7) > >> # > >> +# @max-bandwidth: to set maximum speed for migration. A value lesser than > >> +# zero will be automatically round upto zero. Since 2.8) > > > > Document the units for this ? eg is it bits-per-second, kb-per-second, > > mb-per-second, etc > > > > Should I document it the way it is for old-commands? Like this; > > # @migrate_set_speed > # > # Set maximum speed for migration. > # > # @value: maximum speed in bytes. > # > # Returns: nothing on success > # > # Notes: A value lesser than zero will be automatically round up to zero. > # > # Since: 0.14.0 > ## No, that syntax isn't appropriate for documenting parameters. You hjust have to put it all together. @max-bandwidth: set maximum speed for migration in bytes-per-second. A value lesser than zero will be automatically round upto zero. Since 2.8) Oh and IMHO we should reject values less than zero as invalid, with an error message, not silently interpret them as meaning zero. Regards, Daniel
On Mon, Sep 5, 2016 at 9:09 PM, Daniel P. Berrange <berrange@redhat.com> wrote: > On Mon, Sep 05, 2016 at 08:44:26PM +0530, Ashijeet Acharya wrote: >> On Mon, Sep 5, 2016 at 8:06 PM, Daniel P. Berrange <berrange@redhat.com> wrote: >> > On Mon, Sep 05, 2016 at 07:56:26PM +0530, Ashijeet Acharya wrote: > >> >> diff --git a/qapi-schema.json b/qapi-schema.json >> >> index 5658723..250eac5 100644 >> >> --- a/qapi-schema.json >> >> +++ b/qapi-schema.json >> >> @@ -637,12 +637,18 @@ >> >> # hostname must be provided so that the server's x509 >> >> # certificate identity can be validated. (Since 2.7) >> >> # >> >> +# @max-bandwidth: to set maximum speed for migration. A value lesser than >> >> +# zero will be automatically round upto zero. Since 2.8) >> > >> > Document the units for this ? eg is it bits-per-second, kb-per-second, >> > mb-per-second, etc >> > >> >> Should I document it the way it is for old-commands? Like this; >> >> # @migrate_set_speed >> # >> # Set maximum speed for migration. >> # >> # @value: maximum speed in bytes. >> # >> # Returns: nothing on success >> # >> # Notes: A value lesser than zero will be automatically round up to zero. >> # >> # Since: 0.14.0 >> ## > > No, that syntax isn't appropriate for documenting parameters. You hjust > have to put it all together. Yeah, i was not talking about the syntax, only the comments. > > @max-bandwidth: set maximum speed for migration in bytes-per-second. A > value lesser than zero will be automatically round upto > zero. Since 2.8) > > Oh and IMHO we should reject values less than zero as invalid, with an > error message, not silently interpret them as meaning zero. Alright, so that will take "A value lesser than zero will be automatically round upto zero" out of the comment. Also as I mentioned in the previous thread; the units for downtime are mentioned to be seconds in the old-commands too, so I am keeping that as seconds. Please tell me if you have something else in mind. Ashijeet > > Regards, > Daniel > -- > |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| > |: http://libvirt.org -o- http://virt-manager.org :| > |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| > |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
diff --git a/hmp.c b/hmp.c index cc2056e..c92769b 100644 --- a/hmp.c +++ b/hmp.c @@ -304,6 +304,12 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict) monitor_printf(mon, " %s: '%s'", MigrationParameter_lookup[MIGRATION_PARAMETER_TLS_HOSTNAME], params->tls_hostname ? : ""); + monitor_printf(mon, " %s: %" PRId64, + MigrationParameter_lookup[MIGRATION_PARAMETER_MAX_BANDWIDTH], + params->max_bandwidth); + monitor_printf(mon, " %s: %" PRId64, + MigrationParameter_lookup[MIGRATION_PARAMETER_DOWNTIME_LIMIT], + params->downtime_limit); monitor_printf(mon, "\n"); } @@ -1193,6 +1199,7 @@ void hmp_migrate_incoming(Monitor *mon, const QDict *qdict) hmp_handle_error(mon, &err); } +/* Kept for old-commands compatibility */ void hmp_migrate_set_downtime(Monitor *mon, const QDict *qdict) { double value = qdict_get_double(qdict, "value"); @@ -1211,6 +1218,7 @@ void hmp_migrate_set_cache_size(Monitor *mon, const QDict *qdict) } } +/* Kept for old-commands compatibility */ void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict) { int64_t value = qdict_get_int(qdict, "value"); @@ -1251,7 +1259,10 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) { const char *param = qdict_get_str(qdict, "parameter"); const char *valuestr = qdict_get_str(qdict, "value"); + int64_t valuebw = 0; + double valuedowntime = 0; long valueint = 0; + char *endp; Error *err = NULL; bool has_compress_level = false; bool has_compress_threads = false; @@ -1260,6 +1271,8 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) bool has_cpu_throttle_increment = false; bool has_tls_creds = false; bool has_tls_hostname = false; + bool has_max_bandwidth = false; + bool has_downtime_limit = false; bool use_int_value = false; int i; @@ -1291,6 +1304,24 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) case MIGRATION_PARAMETER_TLS_HOSTNAME: has_tls_hostname = true; break; + case MIGRATION_PARAMETER_MAX_BANDWIDTH: + has_max_bandwidth = true; + valuebw = qemu_strtosz(valuestr, &endp); + if (valuebw < 0 || (size_t)valuebw != valuebw || *endp != '\0' + || !is_power_of_2(valuebw)) { + error_setg(&err, "Invalid size %s", valuestr); + goto cleanup; + } + break; + case MIGRATION_PARAMETER_DOWNTIME_LIMIT: + has_downtime_limit = true; + valuedowntime = strtod(valuestr, &endp); + if (valuestr == endp) { + error_setg(&err, "Unable to parse '%s' as a number", + valuestr); + goto cleanup; + } + break; } if (use_int_value) { @@ -1308,6 +1339,8 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) has_cpu_throttle_increment, valueint, has_tls_creds, valuestr, has_tls_hostname, valuestr, + has_max_bandwidth, valuebw, + has_downtime_limit, valuedowntime, &err); break; } diff --git a/include/migration/migration.h b/include/migration/migration.h index 3c96623..a5429ee 100644 --- a/include/migration/migration.h +++ b/include/migration/migration.h @@ -129,7 +129,6 @@ struct MigrationSrcPageRequest { struct MigrationState { - int64_t bandwidth_limit; size_t bytes_xfer; size_t xfer_limit; QemuThread thread; diff --git a/migration/migration.c b/migration/migration.c index 955d5ee..4b54b58 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -44,6 +44,9 @@ #define BUFFER_DELAY 100 #define XFER_LIMIT_RATIO (1000 / BUFFER_DELAY) +/* Amount of nanoseconds we are willing to wait for migration to be down. */ +#define DEFAULT_MIGRATE_SET_DOWNTIME 300000000 + /* Default compression thread count */ #define DEFAULT_MIGRATE_COMPRESS_THREAD_COUNT 8 /* Default decompression thread count, usually decompression is at @@ -80,7 +83,6 @@ MigrationState *migrate_get_current(void) static bool once; static MigrationState current_migration = { .state = MIGRATION_STATUS_NONE, - .bandwidth_limit = MAX_THROTTLE, .xbzrle_cache_size = DEFAULT_MIGRATE_CACHE_SIZE, .mbps = -1, .parameters = { @@ -89,6 +91,8 @@ MigrationState *migrate_get_current(void) .decompress_threads = DEFAULT_MIGRATE_DECOMPRESS_THREAD_COUNT, .cpu_throttle_initial = DEFAULT_MIGRATE_CPU_THROTTLE_INITIAL, .cpu_throttle_increment = DEFAULT_MIGRATE_CPU_THROTTLE_INCREMENT, + .max_bandwidth = MAX_THROTTLE, + .downtime_limit = DEFAULT_MIGRATE_SET_DOWNTIME, }, }; @@ -566,6 +570,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp) params->cpu_throttle_increment = s->parameters.cpu_throttle_increment; params->tls_creds = g_strdup(s->parameters.tls_creds); params->tls_hostname = g_strdup(s->parameters.tls_hostname); + params->max_bandwidth = s->parameters.max_bandwidth; + params->downtime_limit = s->parameters.downtime_limit; return params; } @@ -773,6 +779,10 @@ void qmp_migrate_set_parameters(bool has_compress_level, const char *tls_creds, bool has_tls_hostname, const char *tls_hostname, + bool has_max_bandwidth, + int64_t max_bandwidth, + bool has_downtime_limit, + double downtime_limit, Error **errp) { MigrationState *s = migrate_get_current(); @@ -832,6 +842,12 @@ void qmp_migrate_set_parameters(bool has_compress_level, g_free(s->parameters.tls_hostname); s->parameters.tls_hostname = g_strdup(tls_hostname); } + if (has_max_bandwidth) { + qmp_migrate_set_speed(max_bandwidth, NULL); + } + if (has_downtime_limit) { + qmp_migrate_set_downtime(downtime_limit, NULL); + } } @@ -1175,18 +1191,24 @@ void qmp_migrate_set_speed(int64_t value, Error **errp) } s = migrate_get_current(); - s->bandwidth_limit = value; + s->parameters.max_bandwidth = value; if (s->to_dst_file) { qemu_file_set_rate_limit(s->to_dst_file, - s->bandwidth_limit / XFER_LIMIT_RATIO); + s->parameters.max_bandwidth / XFER_LIMIT_RATIO); } } void qmp_migrate_set_downtime(double value, Error **errp) { + MigrationState *s; + value *= 1e9; value = MAX(0, MIN(UINT64_MAX, value)); + + s = migrate_get_current(); + max_downtime = (uint64_t)value; + s->parameters.downtime_limit = max_downtime; } bool migrate_postcopy_ram(void) @@ -1858,7 +1880,7 @@ void migrate_fd_connect(MigrationState *s) qemu_file_set_blocking(s->to_dst_file, true); qemu_file_set_rate_limit(s->to_dst_file, - s->bandwidth_limit / XFER_LIMIT_RATIO); + s->parameters.max_bandwidth / XFER_LIMIT_RATIO); /* Notify before starting migration thread */ notifier_list_notify(&migration_state_notifiers, s); diff --git a/qapi-schema.json b/qapi-schema.json index 5658723..250eac5 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -637,12 +637,18 @@ # hostname must be provided so that the server's x509 # certificate identity can be validated. (Since 2.7) # +# @max-bandwidth: to set maximum speed for migration. A value lesser than +# zero will be automatically round upto zero. Since 2.8) +# +# @downtime-limit: set maximum tolerated downtime for migration. 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'] } + 'tls-creds', 'tls-hostname', 'max-bandwidth', + 'downtime-limit'] } # # @migrate-set-parameters @@ -678,6 +684,11 @@ # hostname must be provided so that the server's x509 # certificate identity can be validated. (Since 2.7) # +# @max-bandwidth: to set maximum speed for migration. A value lesser than +# zero will be automatically round upto zero. Since 2.8) +# +# @downtime-limit: set maximum tolerated downtime for migration. Since 2.8) +# # Since: 2.4 ## { 'command': 'migrate-set-parameters', @@ -687,7 +698,9 @@ '*cpu-throttle-initial': 'int', '*cpu-throttle-increment': 'int', '*tls-creds': 'str', - '*tls-hostname': 'str'} } + '*tls-hostname': 'str', + '*max-bandwidth': 'int', + '*downtime-limit': 'number'} } # # @MigrationParameters @@ -721,6 +734,11 @@ # hostname must be provided so that the server's x509 # certificate identity can be validated. (Since 2.7) # +# @max-bandwidth: to set maximum speed for migration. A value lesser than +# zero will be automatically round upto zero. (Since 2.8) +# +# @downtime-limit: set maximum tolerated downtime for migration. Since 2.8) +# # Since: 2.4 ## { 'struct': 'MigrationParameters', @@ -730,7 +748,9 @@ 'cpu-throttle-initial': 'int', 'cpu-throttle-increment': 'int', 'tls-creds': 'str', - 'tls-hostname': 'str'} } + 'tls-hostname': 'str', + 'max-bandwidth': 'int', + 'downtime-limit': 'int'} } ## # @query-migrate-parameters # diff --git a/qmp-commands.hx b/qmp-commands.hx index 6866264..0418cab 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -3790,7 +3790,9 @@ Set migration parameters throttled for auto-converge (json-int) - "cpu-throttle-increment": set throttle increasing percentage for auto-converge (json-int) - +- "max-bandwidth": set maximum speed for migrations (json-int) +- "downtime-limit": set maximum tolerated downtime (in seconds) for + migrations (json-number) Arguments: Example: @@ -3803,7 +3805,7 @@ EQMP { .name = "migrate-set-parameters", .args_type = - "compress-level:i?,compress-threads:i?,decompress-threads:i?,cpu-throttle-initial:i?,cpu-throttle-increment:i?", + "compress-level:i?,compress-threads:i?,decompress-threads:i?,cpu-throttle-initial:i?,cpu-throttle-increment:i?,max-bandwidth:i?,downtime-limit:T?", .mhandler.cmd_new = qmp_marshal_migrate_set_parameters, }, SQMP @@ -3820,6 +3822,9 @@ Query current migration parameters throttled (json-int) - "cpu-throttle-increment" : throttle increasing percentage for auto-converge (json-int) + - "max-bandwidth" : maximium migration speed (json-int) + - "downtime-limit" : maximum tolerated downtime of migration + (json-int) Arguments: @@ -3832,7 +3837,9 @@ Example: "cpu-throttle-increment": 10, "compress-threads": 8, "compress-level": 1, - "cpu-throttle-initial": 20 + "cpu-throttle-initial": 20, + "max-downtime": 33554432, + "downtime-limit": 300000000 } }
Include migrate_set_speed and migrate_set_downtime inside migrate_set_parameters for setting maximum migration speed and expected downtime parameters respectively. Also update the query part for both in qmp and hmp qemu control interfaces. Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com> --- hmp.c | 33 +++++++++++++++++++++++++++++++++ include/migration/migration.h | 1 - migration/migration.c | 30 ++++++++++++++++++++++++++---- qapi-schema.json | 26 +++++++++++++++++++++++--- qmp-commands.hx | 13 ++++++++++--- 5 files changed, 92 insertions(+), 11 deletions(-)