* [Qemu-devel] [PATCH] Move migrate_set_speed and migrate_set_downtime into migrate_set_parameter
@ 2016-09-05 7:45 Ashijeet Acharya
2016-09-05 8:01 ` Paolo Bonzini
0 siblings, 1 reply; 22+ messages in thread
From: Ashijeet Acharya @ 2016-09-05 7:45 UTC (permalink / raw)
To: lcapitulino
Cc: quintela, amit.shah, eblake, armbru, dgilbert, qemu-devel,
Ashijeet Acharya
Include migrate_set_speed and migrate_set_downtime inside migrate_set_parameters respectively for setting maximum migration speed and expected downtime parameters. Also add the query part for both in qmp and hmp qemu control interfaces.
Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
---
hmp-commands.hx | 35 +++-------------------------
hmp.c | 40 +++++++++++++++++++++-----------
include/migration/migration.h | 5 +++-
migration/migration.c | 34 ++++++++++++++++++++++-----
qapi-schema.json | 53 ++++++++++++++++++-------------------------
qmp-commands.hx | 51 ++++-------------------------------------
6 files changed, 88 insertions(+), 130 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 848efee..656fbe8 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -964,35 +964,6 @@ Set cache size to @var{value} (in bytes) for xbzrle migrations.
ETEXI
{
- .name = "migrate_set_speed",
- .args_type = "value:o",
- .params = "value",
- .help = "set maximum speed (in bytes) for migrations. "
- "Defaults to MB if no size suffix is specified, ie. B/K/M/G/T",
- .mhandler.cmd = hmp_migrate_set_speed,
- },
-
-STEXI
-@item migrate_set_speed @var{value}
-@findex migrate_set_speed
-Set maximum speed to @var{value} (in bytes) for migrations.
-ETEXI
-
- {
- .name = "migrate_set_downtime",
- .args_type = "value:T",
- .params = "value",
- .help = "set maximum tolerated downtime (in seconds) for migrations",
- .mhandler.cmd = hmp_migrate_set_downtime,
- },
-
-STEXI
-@item migrate_set_downtime @var{second}
-@findex migrate_set_downtime
-Set maximum tolerated downtime (in seconds) for migration.
-ETEXI
-
- {
.name = "migrate_set_capability",
.args_type = "capability:s,state:b",
.params = "capability state",
@@ -1009,15 +980,15 @@ ETEXI
{
.name = "migrate_set_parameter",
- .args_type = "parameter:s,value:s",
- .params = "parameter value",
+ .args_type = "parameter:s,value:s?,speed:o?",
+ .params = "parameter value speed",
.help = "Set the parameter for migration",
.mhandler.cmd = hmp_migrate_set_parameter,
.command_completion = migrate_set_parameter_completion,
},
STEXI
-@item migrate_set_parameter @var{parameter} @var{value}
+@item migrate_set_parameter @var{parameter} @var{value} @var{speed}
@findex migrate_set_parameter
Set the parameter @var{parameter} for migration.
ETEXI
diff --git a/hmp.c b/hmp.c
index cc2056e..f61140b 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_MIGRATE_SET_SPEED],
+ params->migrate_set_speed);
+ monitor_printf(mon, " %s: %" PRId64,
+ MigrationParameter_lookup[MIGRATION_PARAMETER_MIGRATE_SET_DOWNTIME],
+ params->migrate_set_downtime);
monitor_printf(mon, "\n");
}
@@ -1193,12 +1199,6 @@ void hmp_migrate_incoming(Monitor *mon, const QDict *qdict)
hmp_handle_error(mon, &err);
}
-void hmp_migrate_set_downtime(Monitor *mon, const QDict *qdict)
-{
- double value = qdict_get_double(qdict, "value");
- qmp_migrate_set_downtime(value, NULL);
-}
-
void hmp_migrate_set_cache_size(Monitor *mon, const QDict *qdict)
{
int64_t value = qdict_get_int(qdict, "value");
@@ -1211,12 +1211,6 @@ void hmp_migrate_set_cache_size(Monitor *mon, const QDict *qdict)
}
}
-void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict)
-{
- int64_t value = qdict_get_int(qdict, "value");
- qmp_migrate_set_speed(value, NULL);
-}
-
void hmp_migrate_set_capability(Monitor *mon, const QDict *qdict)
{
const char *cap = qdict_get_str(qdict, "capability");
@@ -1250,8 +1244,11 @@ void hmp_migrate_set_capability(Monitor *mon, const QDict *qdict)
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");
+ const char *valuestr = qdict_get_try_str(qdict, "value");
+ int64_t valuespeed = 0;
+ double valuedowntime = 0;
long valueint = 0;
+ char *endp;
Error *err = NULL;
bool has_compress_level = false;
bool has_compress_threads = false;
@@ -1260,6 +1257,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_migrate_set_speed = false;
+ bool has_migrate_set_downtime = false;
bool use_int_value = false;
int i;
@@ -1291,6 +1290,19 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
case MIGRATION_PARAMETER_TLS_HOSTNAME:
has_tls_hostname = true;
break;
+ case MIGRATION_PARAMETER_MIGRATE_SET_SPEED:
+ has_migrate_set_speed = true;
+ valuespeed = qdict_get_int(qdict, "speed");
+ break;
+ case MIGRATION_PARAMETER_MIGRATE_SET_DOWNTIME:
+ has_migrate_set_downtime = 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 +1320,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_migrate_set_speed, valuespeed,
+ has_migrate_set_downtime, valuedowntime,
&err);
break;
}
diff --git a/include/migration/migration.h b/include/migration/migration.h
index 3c96623..65cd2d7 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;
@@ -205,6 +204,10 @@ void migration_tls_channel_connect(MigrationState *s,
uint64_t migrate_max_downtime(void);
+void migrate_set_preferred_speed(int64_t value, Error **errp);
+
+void migrate_set_expected_downtime(double value, Error **errp);
+
void exec_start_incoming_migration(const char *host_port, Error **errp);
void exec_start_outgoing_migration(MigrationState *s, const char *host_port, Error **errp);
diff --git a/migration/migration.c b/migration/migration.c
index 955d5ee..a0385ce 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,
+ .migrate_set_speed = MAX_THROTTLE,
+ .migrate_set_downtime = 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->migrate_set_speed = s->parameters.migrate_set_speed;
+ params->migrate_set_downtime = s->parameters.migrate_set_downtime;
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_migrate_set_speed,
+ int64_t migrate_set_speed,
+ bool has_migrate_set_downtime,
+ double migrate_set_downtime,
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_migrate_set_speed) {
+ migrate_set_preferred_speed(migrate_set_speed, NULL);
+ }
+ if (has_migrate_set_downtime) {
+ migrate_set_expected_downtime(migrate_set_downtime, NULL);
+ }
}
@@ -1163,7 +1179,7 @@ int64_t qmp_query_migrate_cache_size(Error **errp)
return migrate_xbzrle_cache_size();
}
-void qmp_migrate_set_speed(int64_t value, Error **errp)
+void migrate_set_preferred_speed(int64_t value, Error **errp)
{
MigrationState *s;
@@ -1175,18 +1191,24 @@ void qmp_migrate_set_speed(int64_t value, Error **errp)
}
s = migrate_get_current();
- s->bandwidth_limit = value;
+ s->parameters.migrate_set_speed = value;
if (s->to_dst_file) {
qemu_file_set_rate_limit(s->to_dst_file,
- s->bandwidth_limit / XFER_LIMIT_RATIO);
+ s->parameters.migrate_set_speed / XFER_LIMIT_RATIO);
}
}
-void qmp_migrate_set_downtime(double value, Error **errp)
+void migrate_set_expected_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.migrate_set_downtime = 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.migrate_set_speed / 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..36b89d9 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -637,12 +637,17 @@
# hostname must be provided so that the server's x509
# certificate identity can be validated. (Since 2.7)
#
+# @migrate-set-speed: to set maximum speed for migration. A value lesser than
+# zero will be automatically round upto zero.
+#
+# @migrate-set-downtime: set maximum tolerated downtime for migration.
# 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', 'migrate-set-speed',
+ 'migrate-set-downtime'] }
#
# @migrate-set-parameters
@@ -678,6 +683,11 @@
# hostname must be provided so that the server's x509
# certificate identity can be validated. (Since 2.7)
#
+# @migrate-set-speed: to set maximum speed for migration. A value lesser than
+# zero will be automatically round upto zero.
+#
+# @migrate-set-downtime: set maximum tolerated downtime for migration.
+#
# Since: 2.4
##
{ 'command': 'migrate-set-parameters',
@@ -687,7 +697,9 @@
'*cpu-throttle-initial': 'int',
'*cpu-throttle-increment': 'int',
'*tls-creds': 'str',
- '*tls-hostname': 'str'} }
+ '*tls-hostname': 'str',
+ '*migrate-set-speed': 'int',
+ '*migrate-set-downtime': 'number'} }
#
# @MigrationParameters
@@ -721,6 +733,11 @@
# hostname must be provided so that the server's x509
# certificate identity can be validated. (Since 2.7)
#
+# @migrate-set-speed: to set maximum speed for migration. A value lesser than
+# zero will be automatically round upto zero.
+#
+# @migrate-set-downtime: set maximum tolerated downtime for migration.
+#
# Since: 2.4
##
{ 'struct': 'MigrationParameters',
@@ -730,7 +747,9 @@
'cpu-throttle-initial': 'int',
'cpu-throttle-increment': 'int',
'tls-creds': 'str',
- 'tls-hostname': 'str'} }
+ 'tls-hostname': 'str',
+ 'migrate-set-speed': 'int',
+ 'migrate-set-downtime': 'int'} }
##
# @query-migrate-parameters
#
@@ -1804,34 +1823,6 @@
{ 'command': 'migrate_cancel' }
##
-# @migrate_set_downtime
-#
-# Set maximum tolerated downtime for migration.
-#
-# @value: maximum downtime in seconds
-#
-# Returns: nothing on success
-#
-# Since: 0.14.0
-##
-{ 'command': 'migrate_set_downtime', 'data': {'value': 'number'} }
-
-##
-# @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
-##
-{ 'command': 'migrate_set_speed', 'data': {'value': 'int'} }
-
-##
# @migrate-set-cache-size
#
# Set XBZRLE cache size
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 6866264..ce740e4 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -784,52 +784,6 @@ Example:
EQMP
{
- .name = "migrate_set_speed",
- .args_type = "value:o",
- .mhandler.cmd_new = qmp_marshal_migrate_set_speed,
- },
-
-SQMP
-migrate_set_speed
------------------
-
-Set maximum speed for migrations.
-
-Arguments:
-
-- "value": maximum speed, in bytes per second (json-int)
-
-Example:
-
--> { "execute": "migrate_set_speed", "arguments": { "value": 1024 } }
-<- { "return": {} }
-
-EQMP
-
- {
- .name = "migrate_set_downtime",
- .args_type = "value:T",
- .mhandler.cmd_new = qmp_marshal_migrate_set_downtime,
- },
-
-SQMP
-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": {} }
-
-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",
@@ -3790,6 +3744,9 @@ Set migration parameters
throttled for auto-converge (json-int)
- "cpu-throttle-increment": set throttle increasing percentage for
auto-converge (json-int)
+- "migrate-set-speed": set maximum speed for migrations (json-int)
+- "migrate-set-downtime": set maximum tolerated downtime (in seconds) for
+ migrations (json-number)
Arguments:
@@ -3803,7 +3760,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?,migrate-set-speed:i?,migrate-set-downtime:T?",
.mhandler.cmd_new = qmp_marshal_migrate_set_parameters,
},
SQMP
--
2.6.2
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] Move migrate_set_speed and migrate_set_downtime into migrate_set_parameter
2016-09-05 7:45 [Qemu-devel] [PATCH] Move migrate_set_speed and migrate_set_downtime into migrate_set_parameter Ashijeet Acharya
@ 2016-09-05 8:01 ` Paolo Bonzini
2016-09-05 8:11 ` Ashijeet Acharya
0 siblings, 1 reply; 22+ messages in thread
From: Paolo Bonzini @ 2016-09-05 8:01 UTC (permalink / raw)
To: Ashijeet Acharya, lcapitulino
Cc: quintela, qemu-devel, armbru, amit.shah, dgilbert
On 05/09/2016 09:45, Ashijeet Acharya wrote:
> Include migrate_set_speed and migrate_set_downtime inside
> migrate_set_parameters respectively for setting maximum migration
> speed and expected downtime parameters. Also add the query part for
> both in qmp and hmp qemu control interfaces.
>
> Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
You cannot break backwards compatibility for everyone that is using
those commands, sorry.
Paolo
> ---
> hmp-commands.hx | 35 +++-------------------------
> hmp.c | 40 +++++++++++++++++++++-----------
> include/migration/migration.h | 5 +++-
> migration/migration.c | 34 ++++++++++++++++++++++-----
> qapi-schema.json | 53 ++++++++++++++++++-------------------------
> qmp-commands.hx | 51 ++++-------------------------------------
> 6 files changed, 88 insertions(+), 130 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 848efee..656fbe8 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -964,35 +964,6 @@ Set cache size to @var{value} (in bytes) for xbzrle migrations.
> ETEXI
>
> {
> - .name = "migrate_set_speed",
> - .args_type = "value:o",
> - .params = "value",
> - .help = "set maximum speed (in bytes) for migrations. "
> - "Defaults to MB if no size suffix is specified, ie. B/K/M/G/T",
> - .mhandler.cmd = hmp_migrate_set_speed,
> - },
> -
> -STEXI
> -@item migrate_set_speed @var{value}
> -@findex migrate_set_speed
> -Set maximum speed to @var{value} (in bytes) for migrations.
> -ETEXI
> -
> - {
> - .name = "migrate_set_downtime",
> - .args_type = "value:T",
> - .params = "value",
> - .help = "set maximum tolerated downtime (in seconds) for migrations",
> - .mhandler.cmd = hmp_migrate_set_downtime,
> - },
> -
> -STEXI
> -@item migrate_set_downtime @var{second}
> -@findex migrate_set_downtime
> -Set maximum tolerated downtime (in seconds) for migration.
> -ETEXI
> -
> - {
> .name = "migrate_set_capability",
> .args_type = "capability:s,state:b",
> .params = "capability state",
> @@ -1009,15 +980,15 @@ ETEXI
>
> {
> .name = "migrate_set_parameter",
> - .args_type = "parameter:s,value:s",
> - .params = "parameter value",
> + .args_type = "parameter:s,value:s?,speed:o?",
> + .params = "parameter value speed",
> .help = "Set the parameter for migration",
> .mhandler.cmd = hmp_migrate_set_parameter,
> .command_completion = migrate_set_parameter_completion,
> },
>
> STEXI
> -@item migrate_set_parameter @var{parameter} @var{value}
> +@item migrate_set_parameter @var{parameter} @var{value} @var{speed}
> @findex migrate_set_parameter
> Set the parameter @var{parameter} for migration.
> ETEXI
> diff --git a/hmp.c b/hmp.c
> index cc2056e..f61140b 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_MIGRATE_SET_SPEED],
> + params->migrate_set_speed);
> + monitor_printf(mon, " %s: %" PRId64,
> + MigrationParameter_lookup[MIGRATION_PARAMETER_MIGRATE_SET_DOWNTIME],
> + params->migrate_set_downtime);
> monitor_printf(mon, "\n");
> }
>
> @@ -1193,12 +1199,6 @@ void hmp_migrate_incoming(Monitor *mon, const QDict *qdict)
> hmp_handle_error(mon, &err);
> }
>
> -void hmp_migrate_set_downtime(Monitor *mon, const QDict *qdict)
> -{
> - double value = qdict_get_double(qdict, "value");
> - qmp_migrate_set_downtime(value, NULL);
> -}
> -
> void hmp_migrate_set_cache_size(Monitor *mon, const QDict *qdict)
> {
> int64_t value = qdict_get_int(qdict, "value");
> @@ -1211,12 +1211,6 @@ void hmp_migrate_set_cache_size(Monitor *mon, const QDict *qdict)
> }
> }
>
> -void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict)
> -{
> - int64_t value = qdict_get_int(qdict, "value");
> - qmp_migrate_set_speed(value, NULL);
> -}
> -
> void hmp_migrate_set_capability(Monitor *mon, const QDict *qdict)
> {
> const char *cap = qdict_get_str(qdict, "capability");
> @@ -1250,8 +1244,11 @@ void hmp_migrate_set_capability(Monitor *mon, const QDict *qdict)
> 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");
> + const char *valuestr = qdict_get_try_str(qdict, "value");
> + int64_t valuespeed = 0;
> + double valuedowntime = 0;
> long valueint = 0;
> + char *endp;
> Error *err = NULL;
> bool has_compress_level = false;
> bool has_compress_threads = false;
> @@ -1260,6 +1257,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_migrate_set_speed = false;
> + bool has_migrate_set_downtime = false;
> bool use_int_value = false;
> int i;
>
> @@ -1291,6 +1290,19 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
> case MIGRATION_PARAMETER_TLS_HOSTNAME:
> has_tls_hostname = true;
> break;
> + case MIGRATION_PARAMETER_MIGRATE_SET_SPEED:
> + has_migrate_set_speed = true;
> + valuespeed = qdict_get_int(qdict, "speed");
> + break;
> + case MIGRATION_PARAMETER_MIGRATE_SET_DOWNTIME:
> + has_migrate_set_downtime = 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 +1320,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_migrate_set_speed, valuespeed,
> + has_migrate_set_downtime, valuedowntime,
> &err);
> break;
> }
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index 3c96623..65cd2d7 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;
> @@ -205,6 +204,10 @@ void migration_tls_channel_connect(MigrationState *s,
>
> uint64_t migrate_max_downtime(void);
>
> +void migrate_set_preferred_speed(int64_t value, Error **errp);
> +
> +void migrate_set_expected_downtime(double value, Error **errp);
> +
> void exec_start_incoming_migration(const char *host_port, Error **errp);
>
> void exec_start_outgoing_migration(MigrationState *s, const char *host_port, Error **errp);
> diff --git a/migration/migration.c b/migration/migration.c
> index 955d5ee..a0385ce 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,
> + .migrate_set_speed = MAX_THROTTLE,
> + .migrate_set_downtime = 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->migrate_set_speed = s->parameters.migrate_set_speed;
> + params->migrate_set_downtime = s->parameters.migrate_set_downtime;
>
> 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_migrate_set_speed,
> + int64_t migrate_set_speed,
> + bool has_migrate_set_downtime,
> + double migrate_set_downtime,
> 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_migrate_set_speed) {
> + migrate_set_preferred_speed(migrate_set_speed, NULL);
> + }
> + if (has_migrate_set_downtime) {
> + migrate_set_expected_downtime(migrate_set_downtime, NULL);
> + }
> }
>
>
> @@ -1163,7 +1179,7 @@ int64_t qmp_query_migrate_cache_size(Error **errp)
> return migrate_xbzrle_cache_size();
> }
>
> -void qmp_migrate_set_speed(int64_t value, Error **errp)
> +void migrate_set_preferred_speed(int64_t value, Error **errp)
> {
> MigrationState *s;
>
> @@ -1175,18 +1191,24 @@ void qmp_migrate_set_speed(int64_t value, Error **errp)
> }
>
> s = migrate_get_current();
> - s->bandwidth_limit = value;
> + s->parameters.migrate_set_speed = value;
> if (s->to_dst_file) {
> qemu_file_set_rate_limit(s->to_dst_file,
> - s->bandwidth_limit / XFER_LIMIT_RATIO);
> + s->parameters.migrate_set_speed / XFER_LIMIT_RATIO);
> }
> }
>
> -void qmp_migrate_set_downtime(double value, Error **errp)
> +void migrate_set_expected_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.migrate_set_downtime = 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.migrate_set_speed / 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..36b89d9 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -637,12 +637,17 @@
> # hostname must be provided so that the server's x509
> # certificate identity can be validated. (Since 2.7)
> #
> +# @migrate-set-speed: to set maximum speed for migration. A value lesser than
> +# zero will be automatically round upto zero.
> +#
> +# @migrate-set-downtime: set maximum tolerated downtime for migration.
> # 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', 'migrate-set-speed',
> + 'migrate-set-downtime'] }
>
> #
> # @migrate-set-parameters
> @@ -678,6 +683,11 @@
> # hostname must be provided so that the server's x509
> # certificate identity can be validated. (Since 2.7)
> #
> +# @migrate-set-speed: to set maximum speed for migration. A value lesser than
> +# zero will be automatically round upto zero.
> +#
> +# @migrate-set-downtime: set maximum tolerated downtime for migration.
> +#
> # Since: 2.4
> ##
> { 'command': 'migrate-set-parameters',
> @@ -687,7 +697,9 @@
> '*cpu-throttle-initial': 'int',
> '*cpu-throttle-increment': 'int',
> '*tls-creds': 'str',
> - '*tls-hostname': 'str'} }
> + '*tls-hostname': 'str',
> + '*migrate-set-speed': 'int',
> + '*migrate-set-downtime': 'number'} }
>
> #
> # @MigrationParameters
> @@ -721,6 +733,11 @@
> # hostname must be provided so that the server's x509
> # certificate identity can be validated. (Since 2.7)
> #
> +# @migrate-set-speed: to set maximum speed for migration. A value lesser than
> +# zero will be automatically round upto zero.
> +#
> +# @migrate-set-downtime: set maximum tolerated downtime for migration.
> +#
> # Since: 2.4
> ##
> { 'struct': 'MigrationParameters',
> @@ -730,7 +747,9 @@
> 'cpu-throttle-initial': 'int',
> 'cpu-throttle-increment': 'int',
> 'tls-creds': 'str',
> - 'tls-hostname': 'str'} }
> + 'tls-hostname': 'str',
> + 'migrate-set-speed': 'int',
> + 'migrate-set-downtime': 'int'} }
> ##
> # @query-migrate-parameters
> #
> @@ -1804,34 +1823,6 @@
> { 'command': 'migrate_cancel' }
>
> ##
> -# @migrate_set_downtime
> -#
> -# Set maximum tolerated downtime for migration.
> -#
> -# @value: maximum downtime in seconds
> -#
> -# Returns: nothing on success
> -#
> -# Since: 0.14.0
> -##
> -{ 'command': 'migrate_set_downtime', 'data': {'value': 'number'} }
> -
> -##
> -# @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
> -##
> -{ 'command': 'migrate_set_speed', 'data': {'value': 'int'} }
> -
> -##
> # @migrate-set-cache-size
> #
> # Set XBZRLE cache size
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 6866264..ce740e4 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -784,52 +784,6 @@ Example:
> EQMP
>
> {
> - .name = "migrate_set_speed",
> - .args_type = "value:o",
> - .mhandler.cmd_new = qmp_marshal_migrate_set_speed,
> - },
> -
> -SQMP
> -migrate_set_speed
> ------------------
> -
> -Set maximum speed for migrations.
> -
> -Arguments:
> -
> -- "value": maximum speed, in bytes per second (json-int)
> -
> -Example:
> -
> --> { "execute": "migrate_set_speed", "arguments": { "value": 1024 } }
> -<- { "return": {} }
> -
> -EQMP
> -
> - {
> - .name = "migrate_set_downtime",
> - .args_type = "value:T",
> - .mhandler.cmd_new = qmp_marshal_migrate_set_downtime,
> - },
> -
> -SQMP
> -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": {} }
> -
> -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",
> @@ -3790,6 +3744,9 @@ Set migration parameters
> throttled for auto-converge (json-int)
> - "cpu-throttle-increment": set throttle increasing percentage for
> auto-converge (json-int)
> +- "migrate-set-speed": set maximum speed for migrations (json-int)
> +- "migrate-set-downtime": set maximum tolerated downtime (in seconds) for
> + migrations (json-number)
>
> Arguments:
>
> @@ -3803,7 +3760,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?,migrate-set-speed:i?,migrate-set-downtime:T?",
> .mhandler.cmd_new = qmp_marshal_migrate_set_parameters,
> },
> SQMP
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] Move migrate_set_speed and migrate_set_downtime into migrate_set_parameter
2016-09-05 8:01 ` Paolo Bonzini
@ 2016-09-05 8:11 ` Ashijeet Acharya
2016-09-05 8:16 ` Paolo Bonzini
0 siblings, 1 reply; 22+ messages in thread
From: Ashijeet Acharya @ 2016-09-05 8:11 UTC (permalink / raw)
To: Paolo Bonzini
Cc: lcapitulino, quintela, QEMU Developers, armbru, amit.shah, dgilbert
On Mon, Sep 5, 2016 at 1:31 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 05/09/2016 09:45, Ashijeet Acharya wrote:
>> Include migrate_set_speed and migrate_set_downtime inside
>> migrate_set_parameters respectively for setting maximum migration
>> speed and expected downtime parameters. Also add the query part for
>> both in qmp and hmp qemu control interfaces.
>>
>> Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
>
> You cannot break backwards compatibility for everyone that is using
> those commands, sorry.
>
> Paolo
So should I keep the old commands too and add the new ones anyway for
the query part?
The old ones will also be modified for query. So we will have
compatibility as well as the
query.
Ashijeet
>
>> ---
>> hmp-commands.hx | 35 +++-------------------------
>> hmp.c | 40 +++++++++++++++++++++-----------
>> include/migration/migration.h | 5 +++-
>> migration/migration.c | 34 ++++++++++++++++++++++-----
>> qapi-schema.json | 53 ++++++++++++++++++-------------------------
>> qmp-commands.hx | 51 ++++-------------------------------------
>> 6 files changed, 88 insertions(+), 130 deletions(-)
>>
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index 848efee..656fbe8 100644
>> --- a/hmp-commands.hx
>> +++ b/hmp-commands.hx
>> @@ -964,35 +964,6 @@ Set cache size to @var{value} (in bytes) for xbzrle migrations.
>> ETEXI
>>
>> {
>> - .name = "migrate_set_speed",
>> - .args_type = "value:o",
>> - .params = "value",
>> - .help = "set maximum speed (in bytes) for migrations. "
>> - "Defaults to MB if no size suffix is specified, ie. B/K/M/G/T",
>> - .mhandler.cmd = hmp_migrate_set_speed,
>> - },
>> -
>> -STEXI
>> -@item migrate_set_speed @var{value}
>> -@findex migrate_set_speed
>> -Set maximum speed to @var{value} (in bytes) for migrations.
>> -ETEXI
>> -
>> - {
>> - .name = "migrate_set_downtime",
>> - .args_type = "value:T",
>> - .params = "value",
>> - .help = "set maximum tolerated downtime (in seconds) for migrations",
>> - .mhandler.cmd = hmp_migrate_set_downtime,
>> - },
>> -
>> -STEXI
>> -@item migrate_set_downtime @var{second}
>> -@findex migrate_set_downtime
>> -Set maximum tolerated downtime (in seconds) for migration.
>> -ETEXI
>> -
>> - {
>> .name = "migrate_set_capability",
>> .args_type = "capability:s,state:b",
>> .params = "capability state",
>> @@ -1009,15 +980,15 @@ ETEXI
>>
>> {
>> .name = "migrate_set_parameter",
>> - .args_type = "parameter:s,value:s",
>> - .params = "parameter value",
>> + .args_type = "parameter:s,value:s?,speed:o?",
>> + .params = "parameter value speed",
>> .help = "Set the parameter for migration",
>> .mhandler.cmd = hmp_migrate_set_parameter,
>> .command_completion = migrate_set_parameter_completion,
>> },
>>
>> STEXI
>> -@item migrate_set_parameter @var{parameter} @var{value}
>> +@item migrate_set_parameter @var{parameter} @var{value} @var{speed}
>> @findex migrate_set_parameter
>> Set the parameter @var{parameter} for migration.
>> ETEXI
>> diff --git a/hmp.c b/hmp.c
>> index cc2056e..f61140b 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_MIGRATE_SET_SPEED],
>> + params->migrate_set_speed);
>> + monitor_printf(mon, " %s: %" PRId64,
>> + MigrationParameter_lookup[MIGRATION_PARAMETER_MIGRATE_SET_DOWNTIME],
>> + params->migrate_set_downtime);
>> monitor_printf(mon, "\n");
>> }
>>
>> @@ -1193,12 +1199,6 @@ void hmp_migrate_incoming(Monitor *mon, const QDict *qdict)
>> hmp_handle_error(mon, &err);
>> }
>>
>> -void hmp_migrate_set_downtime(Monitor *mon, const QDict *qdict)
>> -{
>> - double value = qdict_get_double(qdict, "value");
>> - qmp_migrate_set_downtime(value, NULL);
>> -}
>> -
>> void hmp_migrate_set_cache_size(Monitor *mon, const QDict *qdict)
>> {
>> int64_t value = qdict_get_int(qdict, "value");
>> @@ -1211,12 +1211,6 @@ void hmp_migrate_set_cache_size(Monitor *mon, const QDict *qdict)
>> }
>> }
>>
>> -void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict)
>> -{
>> - int64_t value = qdict_get_int(qdict, "value");
>> - qmp_migrate_set_speed(value, NULL);
>> -}
>> -
>> void hmp_migrate_set_capability(Monitor *mon, const QDict *qdict)
>> {
>> const char *cap = qdict_get_str(qdict, "capability");
>> @@ -1250,8 +1244,11 @@ void hmp_migrate_set_capability(Monitor *mon, const QDict *qdict)
>> 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");
>> + const char *valuestr = qdict_get_try_str(qdict, "value");
>> + int64_t valuespeed = 0;
>> + double valuedowntime = 0;
>> long valueint = 0;
>> + char *endp;
>> Error *err = NULL;
>> bool has_compress_level = false;
>> bool has_compress_threads = false;
>> @@ -1260,6 +1257,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_migrate_set_speed = false;
>> + bool has_migrate_set_downtime = false;
>> bool use_int_value = false;
>> int i;
>>
>> @@ -1291,6 +1290,19 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
>> case MIGRATION_PARAMETER_TLS_HOSTNAME:
>> has_tls_hostname = true;
>> break;
>> + case MIGRATION_PARAMETER_MIGRATE_SET_SPEED:
>> + has_migrate_set_speed = true;
>> + valuespeed = qdict_get_int(qdict, "speed");
>> + break;
>> + case MIGRATION_PARAMETER_MIGRATE_SET_DOWNTIME:
>> + has_migrate_set_downtime = 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 +1320,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_migrate_set_speed, valuespeed,
>> + has_migrate_set_downtime, valuedowntime,
>> &err);
>> break;
>> }
>> diff --git a/include/migration/migration.h b/include/migration/migration.h
>> index 3c96623..65cd2d7 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;
>> @@ -205,6 +204,10 @@ void migration_tls_channel_connect(MigrationState *s,
>>
>> uint64_t migrate_max_downtime(void);
>>
>> +void migrate_set_preferred_speed(int64_t value, Error **errp);
>> +
>> +void migrate_set_expected_downtime(double value, Error **errp);
>> +
>> void exec_start_incoming_migration(const char *host_port, Error **errp);
>>
>> void exec_start_outgoing_migration(MigrationState *s, const char *host_port, Error **errp);
>> diff --git a/migration/migration.c b/migration/migration.c
>> index 955d5ee..a0385ce 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,
>> + .migrate_set_speed = MAX_THROTTLE,
>> + .migrate_set_downtime = 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->migrate_set_speed = s->parameters.migrate_set_speed;
>> + params->migrate_set_downtime = s->parameters.migrate_set_downtime;
>>
>> 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_migrate_set_speed,
>> + int64_t migrate_set_speed,
>> + bool has_migrate_set_downtime,
>> + double migrate_set_downtime,
>> 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_migrate_set_speed) {
>> + migrate_set_preferred_speed(migrate_set_speed, NULL);
>> + }
>> + if (has_migrate_set_downtime) {
>> + migrate_set_expected_downtime(migrate_set_downtime, NULL);
>> + }
>> }
>>
>>
>> @@ -1163,7 +1179,7 @@ int64_t qmp_query_migrate_cache_size(Error **errp)
>> return migrate_xbzrle_cache_size();
>> }
>>
>> -void qmp_migrate_set_speed(int64_t value, Error **errp)
>> +void migrate_set_preferred_speed(int64_t value, Error **errp)
>> {
>> MigrationState *s;
>>
>> @@ -1175,18 +1191,24 @@ void qmp_migrate_set_speed(int64_t value, Error **errp)
>> }
>>
>> s = migrate_get_current();
>> - s->bandwidth_limit = value;
>> + s->parameters.migrate_set_speed = value;
>> if (s->to_dst_file) {
>> qemu_file_set_rate_limit(s->to_dst_file,
>> - s->bandwidth_limit / XFER_LIMIT_RATIO);
>> + s->parameters.migrate_set_speed / XFER_LIMIT_RATIO);
>> }
>> }
>>
>> -void qmp_migrate_set_downtime(double value, Error **errp)
>> +void migrate_set_expected_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.migrate_set_downtime = 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.migrate_set_speed / 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..36b89d9 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -637,12 +637,17 @@
>> # hostname must be provided so that the server's x509
>> # certificate identity can be validated. (Since 2.7)
>> #
>> +# @migrate-set-speed: to set maximum speed for migration. A value lesser than
>> +# zero will be automatically round upto zero.
>> +#
>> +# @migrate-set-downtime: set maximum tolerated downtime for migration.
>> # 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', 'migrate-set-speed',
>> + 'migrate-set-downtime'] }
>>
>> #
>> # @migrate-set-parameters
>> @@ -678,6 +683,11 @@
>> # hostname must be provided so that the server's x509
>> # certificate identity can be validated. (Since 2.7)
>> #
>> +# @migrate-set-speed: to set maximum speed for migration. A value lesser than
>> +# zero will be automatically round upto zero.
>> +#
>> +# @migrate-set-downtime: set maximum tolerated downtime for migration.
>> +#
>> # Since: 2.4
>> ##
>> { 'command': 'migrate-set-parameters',
>> @@ -687,7 +697,9 @@
>> '*cpu-throttle-initial': 'int',
>> '*cpu-throttle-increment': 'int',
>> '*tls-creds': 'str',
>> - '*tls-hostname': 'str'} }
>> + '*tls-hostname': 'str',
>> + '*migrate-set-speed': 'int',
>> + '*migrate-set-downtime': 'number'} }
>>
>> #
>> # @MigrationParameters
>> @@ -721,6 +733,11 @@
>> # hostname must be provided so that the server's x509
>> # certificate identity can be validated. (Since 2.7)
>> #
>> +# @migrate-set-speed: to set maximum speed for migration. A value lesser than
>> +# zero will be automatically round upto zero.
>> +#
>> +# @migrate-set-downtime: set maximum tolerated downtime for migration.
>> +#
>> # Since: 2.4
>> ##
>> { 'struct': 'MigrationParameters',
>> @@ -730,7 +747,9 @@
>> 'cpu-throttle-initial': 'int',
>> 'cpu-throttle-increment': 'int',
>> 'tls-creds': 'str',
>> - 'tls-hostname': 'str'} }
>> + 'tls-hostname': 'str',
>> + 'migrate-set-speed': 'int',
>> + 'migrate-set-downtime': 'int'} }
>> ##
>> # @query-migrate-parameters
>> #
>> @@ -1804,34 +1823,6 @@
>> { 'command': 'migrate_cancel' }
>>
>> ##
>> -# @migrate_set_downtime
>> -#
>> -# Set maximum tolerated downtime for migration.
>> -#
>> -# @value: maximum downtime in seconds
>> -#
>> -# Returns: nothing on success
>> -#
>> -# Since: 0.14.0
>> -##
>> -{ 'command': 'migrate_set_downtime', 'data': {'value': 'number'} }
>> -
>> -##
>> -# @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
>> -##
>> -{ 'command': 'migrate_set_speed', 'data': {'value': 'int'} }
>> -
>> -##
>> # @migrate-set-cache-size
>> #
>> # Set XBZRLE cache size
>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>> index 6866264..ce740e4 100644
>> --- a/qmp-commands.hx
>> +++ b/qmp-commands.hx
>> @@ -784,52 +784,6 @@ Example:
>> EQMP
>>
>> {
>> - .name = "migrate_set_speed",
>> - .args_type = "value:o",
>> - .mhandler.cmd_new = qmp_marshal_migrate_set_speed,
>> - },
>> -
>> -SQMP
>> -migrate_set_speed
>> ------------------
>> -
>> -Set maximum speed for migrations.
>> -
>> -Arguments:
>> -
>> -- "value": maximum speed, in bytes per second (json-int)
>> -
>> -Example:
>> -
>> --> { "execute": "migrate_set_speed", "arguments": { "value": 1024 } }
>> -<- { "return": {} }
>> -
>> -EQMP
>> -
>> - {
>> - .name = "migrate_set_downtime",
>> - .args_type = "value:T",
>> - .mhandler.cmd_new = qmp_marshal_migrate_set_downtime,
>> - },
>> -
>> -SQMP
>> -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": {} }
>> -
>> -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",
>> @@ -3790,6 +3744,9 @@ Set migration parameters
>> throttled for auto-converge (json-int)
>> - "cpu-throttle-increment": set throttle increasing percentage for
>> auto-converge (json-int)
>> +- "migrate-set-speed": set maximum speed for migrations (json-int)
>> +- "migrate-set-downtime": set maximum tolerated downtime (in seconds) for
>> + migrations (json-number)
>>
>> Arguments:
>>
>> @@ -3803,7 +3760,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?,migrate-set-speed:i?,migrate-set-downtime:T?",
>> .mhandler.cmd_new = qmp_marshal_migrate_set_parameters,
>> },
>> SQMP
>>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] Move migrate_set_speed and migrate_set_downtime into migrate_set_parameter
2016-09-05 8:11 ` Ashijeet Acharya
@ 2016-09-05 8:16 ` Paolo Bonzini
2016-09-05 8:25 ` Ashijeet Acharya
0 siblings, 1 reply; 22+ messages in thread
From: Paolo Bonzini @ 2016-09-05 8:16 UTC (permalink / raw)
To: Ashijeet Acharya
Cc: quintela, armbru, QEMU Developers, amit.shah, lcapitulino, dgilbert
On 05/09/2016 10:11, Ashijeet Acharya wrote:
> > > Include migrate_set_speed and migrate_set_downtime inside
> > > migrate_set_parameters respectively for setting maximum migration
> > > speed and expected downtime parameters. Also add the query part for
> > > both in qmp and hmp qemu control interfaces.
> > >
> > > Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
> >
> > You cannot break backwards compatibility for everyone that is using
> > those commands, sorry.
>
> So should I keep the old commands too and add the new ones anyway for
> the query part?
You do not need query for the old ones, but you can indeed add support
for speed and downtime in migrate-set-parameters and MigrationParameters.
(Note: I'm not a migration maintainer, so they might say something else).
Paolo
> The old ones will also be modified for query. So we will have
> compatibility as well as the
> query.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] Move migrate_set_speed and migrate_set_downtime into migrate_set_parameter
2016-09-05 8:16 ` Paolo Bonzini
@ 2016-09-05 8:25 ` Ashijeet Acharya
2016-09-05 11:16 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 22+ messages in thread
From: Ashijeet Acharya @ 2016-09-05 8:25 UTC (permalink / raw)
To: Paolo Bonzini
Cc: quintela, armbru, QEMU Developers, amit.shah, lcapitulino, dgilbert
On Mon, Sep 5, 2016 at 1:46 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 05/09/2016 10:11, Ashijeet Acharya wrote:
>> > > Include migrate_set_speed and migrate_set_downtime inside
>> > > migrate_set_parameters respectively for setting maximum migration
>> > > speed and expected downtime parameters. Also add the query part for
>> > > both in qmp and hmp qemu control interfaces.
>> > >
>> > > Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
>> >
>> > You cannot break backwards compatibility for everyone that is using
>> > those commands, sorry.
>>
>> So should I keep the old commands too and add the new ones anyway for
>> the query part?
>
> You do not need query for the old ones, but you can indeed add support
> for speed and downtime in migrate-set-parameters and MigrationParameters.
Right. So I will add the old-commands back and just add support for the new ones
under migration-set-parameters along with the query.
> (Note: I'm not a migration maintainer, so they might say something else).
Okay. I will wait for their views too.
Ashijeet
>
> Paolo
>
>> The old ones will also be modified for query. So we will have
>> compatibility as well as the
>> query.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] Move migrate_set_speed and migrate_set_downtime into migrate_set_parameter
2016-09-05 8:25 ` Ashijeet Acharya
@ 2016-09-05 11:16 ` Dr. David Alan Gilbert
2016-09-05 11:37 ` [Qemu-devel] [PATCH v2] " Ashijeet Acharya
2016-09-06 13:16 ` [Qemu-devel] [PATCH] " Juan Quintela
0 siblings, 2 replies; 22+ messages in thread
From: Dr. David Alan Gilbert @ 2016-09-05 11:16 UTC (permalink / raw)
To: Ashijeet Acharya
Cc: Paolo Bonzini, quintela, armbru, QEMU Developers, amit.shah, lcapitulino
* Ashijeet Acharya (ashijeetacharya@gmail.com) wrote:
> On Mon, Sep 5, 2016 at 1:46 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> >
> >
> > On 05/09/2016 10:11, Ashijeet Acharya wrote:
> >> > > Include migrate_set_speed and migrate_set_downtime inside
> >> > > migrate_set_parameters respectively for setting maximum migration
> >> > > speed and expected downtime parameters. Also add the query part for
> >> > > both in qmp and hmp qemu control interfaces.
> >> > >
> >> > > Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
> >> >
> >> > You cannot break backwards compatibility for everyone that is using
> >> > those commands, sorry.
> >>
> >> So should I keep the old commands too and add the new ones anyway for
> >> the query part?
> >
> > You do not need query for the old ones, but you can indeed add support
> > for speed and downtime in migrate-set-parameters and MigrationParameters.
>
> Right. So I will add the old-commands back and just add support for the new ones
> under migration-set-parameters along with the query.
Yes add them to both migrate_set_parameters and query_migrate_parameters
and the hmp equivalents; but don't remove the old functions; just make
them simple wrappers to call the migrate_set_parameters etc.
Add a comment on them like 'kept for compatibility'.
Dave
> > (Note: I'm not a migration maintainer, so they might say something else).
>
> Okay. I will wait for their views too.
>
> Ashijeet
> >
> > Paolo
> >
> >> The old ones will also be modified for query. So we will have
> >> compatibility as well as the
> >> query.
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v2] Move migrate_set_speed and migrate_set_downtime into migrate_set_parameter
2016-09-05 11:16 ` Dr. David Alan Gilbert
@ 2016-09-05 11:37 ` Ashijeet Acharya
2016-09-05 11:45 ` Paolo Bonzini
2016-09-06 13:16 ` [Qemu-devel] [PATCH] " Juan Quintela
1 sibling, 1 reply; 22+ messages in thread
From: Ashijeet Acharya @ 2016-09-05 11:37 UTC (permalink / raw)
To: lcapitulino
Cc: quintela, amit.shah, eblake, armbru, dgilbert, pbonzini,
qemu-devel, Ashijeet Acharya
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-commands.hx | 6 +++---
hmp.c | 30 +++++++++++++++++++++++++++++-
include/migration/migration.h | 1 -
migration/migration.c | 30 ++++++++++++++++++++++++++----
qapi-schema.json | 26 +++++++++++++++++++++++---
qmp-commands.hx | 13 ++++++++++---
6 files changed, 91 insertions(+), 15 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 848efee..4dc7899 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1009,15 +1009,15 @@ ETEXI
{
.name = "migrate_set_parameter",
- .args_type = "parameter:s,value:s",
- .params = "parameter value",
+ .args_type = "parameter:s,value:s?,speed:o?",
+ .params = "parameter value speed",
.help = "Set the parameter for migration",
.mhandler.cmd = hmp_migrate_set_parameter,
.command_completion = migrate_set_parameter_completion,
},
STEXI
-@item migrate_set_parameter @var{parameter} @var{value}
+@item migrate_set_parameter @var{parameter} @var{value} @var{speed}
@findex migrate_set_parameter
Set the parameter @var{parameter} for migration.
ETEXI
diff --git a/hmp.c b/hmp.c
index cc2056e..fd50e83 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_MIGRATE_SET_SPEED],
+ params->migrate_set_speed);
+ monitor_printf(mon, " %s: %" PRId64,
+ MigrationParameter_lookup[MIGRATION_PARAMETER_MIGRATE_SET_DOWNTIME],
+ params->migrate_set_downtime);
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");
@@ -1250,8 +1258,11 @@ void hmp_migrate_set_capability(Monitor *mon, const QDict *qdict)
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");
+ const char *valuestr = qdict_get_try_str(qdict, "value");
+ int64_t valuespeed = 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_migrate_set_speed = false;
+ bool has_migrate_set_downtime = false;
bool use_int_value = false;
int i;
@@ -1291,6 +1304,19 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
case MIGRATION_PARAMETER_TLS_HOSTNAME:
has_tls_hostname = true;
break;
+ case MIGRATION_PARAMETER_MIGRATE_SET_SPEED:
+ has_migrate_set_speed = true;
+ valuespeed = qdict_get_int(qdict, "speed");
+ break;
+ case MIGRATION_PARAMETER_MIGRATE_SET_DOWNTIME:
+ has_migrate_set_downtime = 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 +1334,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_migrate_set_speed, valuespeed,
+ has_migrate_set_downtime, 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..3768911 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,
+ .migrate_set_speed = MAX_THROTTLE,
+ .migrate_set_downtime = 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->migrate_set_speed = s->parameters.migrate_set_speed;
+ params->migrate_set_downtime = s->parameters.migrate_set_downtime;
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_migrate_set_speed,
+ int64_t migrate_set_speed,
+ bool has_migrate_set_downtime,
+ double migrate_set_downtime,
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_migrate_set_speed) {
+ qmp_migrate_set_speed(migrate_set_speed, NULL);
+ }
+ if (has_migrate_set_downtime) {
+ qmp_migrate_set_downtime(migrate_set_downtime, 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.migrate_set_speed = value;
if (s->to_dst_file) {
qemu_file_set_rate_limit(s->to_dst_file,
- s->bandwidth_limit / XFER_LIMIT_RATIO);
+ s->parameters.migrate_set_speed / 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.migrate_set_downtime = 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.migrate_set_speed / 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..b98be44 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)
#
+# @migrate-set-speed: to set maximum speed for migration. A value lesser than
+# zero will be automatically round upto zero.
+#
+# @migrate-set-downtime: set maximum tolerated downtime for migration.
+#
# 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', 'migrate-set-speed',
+ 'migrate-set-downtime'] }
#
# @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)
#
+# @migrate-set-speed: to set maximum speed for migration. A value lesser than
+# zero will be automatically round upto zero.
+#
+# @migrate-set-downtime: set maximum tolerated downtime for migration.
+#
# 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',
+ '*migrate-set-speed': 'int',
+ '*migrate-set-downtime': 'number'} }
#
# @MigrationParameters
@@ -721,6 +734,11 @@
# hostname must be provided so that the server's x509
# certificate identity can be validated. (Since 2.7)
#
+# @migrate-set-speed: to set maximum speed for migration. A value lesser than
+# zero will be automatically round upto zero.
+#
+# @migrate-set-downtime: set maximum tolerated downtime for migration.
+#
# 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',
+ 'migrate-set-speed': 'int',
+ 'migrate-set-downtime': 'int'} }
##
# @query-migrate-parameters
#
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 6866264..c4d3809 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)
-
+- "migrate-set-speed": set maximum speed for migrations (json-octets)
+- "migrate-set-downtime": 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?,migrate-set-speed:o?,migrate-set-downtime: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)
+ - "migrate-set-speed" : maximium migration speed (json-octets)
+ - "migration-set-downtime" : maximum tolerated downtime of migration
+ (json-number)
Arguments:
@@ -3832,7 +3837,9 @@ Example:
"cpu-throttle-increment": 10,
"compress-threads": 8,
"compress-level": 1,
- "cpu-throttle-initial": 20
+ "cpu-throttle-initial": 20,
+ "migration-set-speed": 33554432,
+ "migration-set-downtime": 300000000
}
}
--
2.6.2
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2] Move migrate_set_speed and migrate_set_downtime into migrate_set_parameter
2016-09-05 11:37 ` [Qemu-devel] [PATCH v2] " Ashijeet Acharya
@ 2016-09-05 11:45 ` Paolo Bonzini
2016-09-05 11:59 ` Ashijeet Acharya
2016-09-05 13:09 ` Ashijeet Acharya
0 siblings, 2 replies; 22+ messages in thread
From: Paolo Bonzini @ 2016-09-05 11:45 UTC (permalink / raw)
To: Ashijeet Acharya, lcapitulino
Cc: quintela, amit.shah, eblake, armbru, dgilbert, qemu-devel
On 05/09/2016 13:37, 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.
>
> Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
> ---
> hmp-commands.hx | 6 +++---
> hmp.c | 30 +++++++++++++++++++++++++++++-
> include/migration/migration.h | 1 -
> migration/migration.c | 30 ++++++++++++++++++++++++++----
> qapi-schema.json | 26 +++++++++++++++++++++++---
> qmp-commands.hx | 13 ++++++++++---
> 6 files changed, 91 insertions(+), 15 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 848efee..4dc7899 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1009,15 +1009,15 @@ ETEXI
>
> {
> .name = "migrate_set_parameter",
> - .args_type = "parameter:s,value:s",
> - .params = "parameter value",
> + .args_type = "parameter:s,value:s?,speed:o?",
> + .params = "parameter value speed",
> .help = "Set the parameter for migration",
> .mhandler.cmd = hmp_migrate_set_parameter,
> .command_completion = migrate_set_parameter_completion,
> },
>
> STEXI
> -@item migrate_set_parameter @var{parameter} @var{value}
> +@item migrate_set_parameter @var{parameter} @var{value} @var{speed}
This is wrong, it should not use a third argument.
migrate_set_parameter is just receiving a key/value pair.
Since value is a string, you can use qemu_strtosz in
hmp_migrate_set_parameter to convert it to bytes and print an "invalid
size" error if invalid.
> @findex migrate_set_parameter
> Set the parameter @var{parameter} for migration.
> ETEXI
> diff --git a/hmp.c b/hmp.c
> index cc2056e..fd50e83 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_MIGRATE_SET_SPEED],
> + params->migrate_set_speed);
> + monitor_printf(mon, " %s: %" PRId64,
> + MigrationParameter_lookup[MIGRATION_PARAMETER_MIGRATE_SET_DOWNTIME],
> + params->migrate_set_downtime);
> 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");
> @@ -1250,8 +1258,11 @@ void hmp_migrate_set_capability(Monitor *mon, const QDict *qdict)
> 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");
> + const char *valuestr = qdict_get_try_str(qdict, "value");
> + int64_t valuespeed = 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_migrate_set_speed = false;
> + bool has_migrate_set_downtime = false;
> bool use_int_value = false;
> int i;
>
> @@ -1291,6 +1304,19 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
> case MIGRATION_PARAMETER_TLS_HOSTNAME:
> has_tls_hostname = true;
> break;
> + case MIGRATION_PARAMETER_MIGRATE_SET_SPEED:
> + has_migrate_set_speed = true;
> + valuespeed = qdict_get_int(qdict, "speed");
> + break;
> + case MIGRATION_PARAMETER_MIGRATE_SET_DOWNTIME:
> + has_migrate_set_downtime = 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) {
> @@ -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,
> + .migrate_set_speed = MAX_THROTTLE,
> + .migrate_set_downtime = 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->migrate_set_speed = s->parameters.migrate_set_speed;
> + params->migrate_set_downtime = s->parameters.migrate_set_downtime;
>
> 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_migrate_set_speed,
> + int64_t migrate_set_speed,
> + bool has_migrate_set_downtime,
> + double migrate_set_downtime,
> 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_migrate_set_speed) {
> + qmp_migrate_set_speed(migrate_set_speed, NULL);
> + }
> + if (has_migrate_set_downtime) {
> + qmp_migrate_set_downtime(migrate_set_downtime, NULL);
> + }
This is backwards; qmp_migrate_set_speed and qmp_migrate_set_downtime
should be implemented in terms of qmp_migrate_set_parameters, not the
other way round.
> }
>
>
> @@ -1175,18 +1191,24 @@ void qmp_migrate_set_speed(int64_t value, Error **errp)
> }
>
> s = migrate_get_current();
> - s->bandwidth_limit = value;
> + s->parameters.migrate_set_speed = value;
> if (s->to_dst_file) {
> qemu_file_set_rate_limit(s->to_dst_file,
> - s->bandwidth_limit / XFER_LIMIT_RATIO);
> + s->parameters.migrate_set_speed / 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.migrate_set_downtime = 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.migrate_set_speed / 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..b98be44 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)
> #
> +# @migrate-set-speed: to set maximum speed for migration. A value lesser than
> +# zero will be automatically round upto zero.
> +#
> +# @migrate-set-downtime: set maximum tolerated downtime for migration.
Please add "Since 2.8" to the documentation for the new members.
> +#
> # 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', 'migrate-set-speed',
> + 'migrate-set-downtime'] }
MigrationParameter names are not verbs. It should be "downtime-limit"
and "max-bandwidth", or something similar.
>
> #
> # @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)
> #
> +# @migrate-set-speed: to set maximum speed for migration. A value lesser than
> +# zero will be automatically round upto zero.
> +#
> +# @migrate-set-downtime: set maximum tolerated downtime for migration.
> +#
> # 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',
> + '*migrate-set-speed': 'int',
> + '*migrate-set-downtime': 'number'} }
Same here.
>
> #
> # @MigrationParameters
> @@ -721,6 +734,11 @@
> # hostname must be provided so that the server's x509
> # certificate identity can be validated. (Since 2.7)
> #
> +# @migrate-set-speed: to set maximum speed for migration. A value lesser than
> +# zero will be automatically round upto zero.
> +#
> +# @migrate-set-downtime: set maximum tolerated downtime for migration.
Same here, note that these are "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',
> + 'migrate-set-speed': 'int',
> + 'migrate-set-downtime': 'int'} }
Same here about the names.
> ##
> # @query-migrate-parameters
> #
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 6866264..c4d3809 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)
> -
> +- "migrate-set-speed": set maximum speed for migrations (json-octets)
> +- "migrate-set-downtime": set maximum tolerated downtime (in seconds) for
> + migrations (json-number)
Same here about the names.
> 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?,migrate-set-speed:o?,migrate-set-downtime:T?",
Same here about the names. Also use "i" for QMP commands.
> .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)
> + - "migrate-set-speed" : maximium migration speed (json-octets)
> + - "migration-set-downtime" : maximum tolerated downtime of migration
> + (json-number)
Same here about the names.
> Arguments:
>
> @@ -3832,7 +3837,9 @@ Example:
> "cpu-throttle-increment": 10,
> "compress-threads": 8,
> "compress-level": 1,
> - "cpu-throttle-initial": 20
> + "cpu-throttle-initial": 20,
> + "migration-set-speed": 33554432,
> + "migration-set-downtime": 300000000
Same here about the names.
> }
> }
>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2] Move migrate_set_speed and migrate_set_downtime into migrate_set_parameter
2016-09-05 11:45 ` Paolo Bonzini
@ 2016-09-05 11:59 ` Ashijeet Acharya
2016-09-05 12:01 ` Paolo Bonzini
2016-09-05 13:09 ` Ashijeet Acharya
1 sibling, 1 reply; 22+ messages in thread
From: Ashijeet Acharya @ 2016-09-05 11:59 UTC (permalink / raw)
To: Paolo Bonzini
Cc: lcapitulino, quintela, amit.shah, eblake, armbru, dgilbert,
QEMU Developers
On Mon, Sep 5, 2016 at 5:15 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 05/09/2016 13:37, 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.
>>
>> Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
>> ---
>> hmp-commands.hx | 6 +++---
>> hmp.c | 30 +++++++++++++++++++++++++++++-
>> include/migration/migration.h | 1 -
>> migration/migration.c | 30 ++++++++++++++++++++++++++----
>> qapi-schema.json | 26 +++++++++++++++++++++++---
>> qmp-commands.hx | 13 ++++++++++---
>> 6 files changed, 91 insertions(+), 15 deletions(-)
>>
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index 848efee..4dc7899 100644
>> --- a/hmp-commands.hx
>> +++ b/hmp-commands.hx
>> @@ -1009,15 +1009,15 @@ ETEXI
>>
>> {
>> .name = "migrate_set_parameter",
>> - .args_type = "parameter:s,value:s",
>> - .params = "parameter value",
>> + .args_type = "parameter:s,value:s?,speed:o?",
>> + .params = "parameter value speed",
>> .help = "Set the parameter for migration",
>> .mhandler.cmd = hmp_migrate_set_parameter,
>> .command_completion = migrate_set_parameter_completion,
>> },
>>
>> STEXI
>> -@item migrate_set_parameter @var{parameter} @var{value}
>> +@item migrate_set_parameter @var{parameter} @var{value} @var{speed}
>
> This is wrong, it should not use a third argument.
> migrate_set_parameter is just receiving a key/value pair.
>
> Since value is a string, you can use qemu_strtosz in
> hmp_migrate_set_parameter to convert it to bytes and print an "invalid
> size" error if invalid.
Yeah. This one was bugging me but wasn't sure what the right logic was.
Will make these changes asap.
>
>> @findex migrate_set_parameter
>> Set the parameter @var{parameter} for migration.
>> ETEXI
>> diff --git a/hmp.c b/hmp.c
>> index cc2056e..fd50e83 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_MIGRATE_SET_SPEED],
>> + params->migrate_set_speed);
>> + monitor_printf(mon, " %s: %" PRId64,
>> + MigrationParameter_lookup[MIGRATION_PARAMETER_MIGRATE_SET_DOWNTIME],
>> + params->migrate_set_downtime);
>> 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");
>> @@ -1250,8 +1258,11 @@ void hmp_migrate_set_capability(Monitor *mon, const QDict *qdict)
>> 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");
>> + const char *valuestr = qdict_get_try_str(qdict, "value");
>> + int64_t valuespeed = 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_migrate_set_speed = false;
>> + bool has_migrate_set_downtime = false;
>> bool use_int_value = false;
>> int i;
>>
>> @@ -1291,6 +1304,19 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
>> case MIGRATION_PARAMETER_TLS_HOSTNAME:
>> has_tls_hostname = true;
>> break;
>> + case MIGRATION_PARAMETER_MIGRATE_SET_SPEED:
>> + has_migrate_set_speed = true;
>> + valuespeed = qdict_get_int(qdict, "speed");
>> + break;
>> + case MIGRATION_PARAMETER_MIGRATE_SET_DOWNTIME:
>> + has_migrate_set_downtime = 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) {
>> @@ -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,
>> + .migrate_set_speed = MAX_THROTTLE,
>> + .migrate_set_downtime = 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->migrate_set_speed = s->parameters.migrate_set_speed;
>> + params->migrate_set_downtime = s->parameters.migrate_set_downtime;
>>
>> 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_migrate_set_speed,
>> + int64_t migrate_set_speed,
>> + bool has_migrate_set_downtime,
>> + double migrate_set_downtime,
>> 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_migrate_set_speed) {
>> + qmp_migrate_set_speed(migrate_set_speed, NULL);
>> + }
>> + if (has_migrate_set_downtime) {
>> + qmp_migrate_set_downtime(migrate_set_downtime, NULL);
>> + }
>
> This is backwards; qmp_migrate_set_speed and qmp_migrate_set_downtime
> should be implemented in terms of qmp_migrate_set_parameters, not the
> other way round.
Okay. Got it!
>
>> }
>>
>>
>> @@ -1175,18 +1191,24 @@ void qmp_migrate_set_speed(int64_t value, Error **errp)
>> }
>>
>> s = migrate_get_current();
>> - s->bandwidth_limit = value;
>> + s->parameters.migrate_set_speed = value;
>> if (s->to_dst_file) {
>> qemu_file_set_rate_limit(s->to_dst_file,
>> - s->bandwidth_limit / XFER_LIMIT_RATIO);
>> + s->parameters.migrate_set_speed / 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.migrate_set_downtime = 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.migrate_set_speed / 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..b98be44 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)
>> #
>> +# @migrate-set-speed: to set maximum speed for migration. A value lesser than
>> +# zero will be automatically round upto zero.
>> +#
>> +# @migrate-set-downtime: set maximum tolerated downtime for migration.
>
> Please add "Since 2.8" to the documentation for the new members.
Alright.
>
>> +#
>> # 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', 'migrate-set-speed',
>> + 'migrate-set-downtime'] }
>
> MigrationParameter names are not verbs. It should be "downtime-limit"
> and "max-bandwidth", or something similar.
>
I will change the names then.
>>
>> #
>> # @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)
>> #
>> +# @migrate-set-speed: to set maximum speed for migration. A value lesser than
>> +# zero will be automatically round upto zero.
>> +#
>> +# @migrate-set-downtime: set maximum tolerated downtime for migration.
>> +#
>> # 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',
>> + '*migrate-set-speed': 'int',
>> + '*migrate-set-downtime': 'number'} }
>
> Same here.
>
>>
>> #
>> # @MigrationParameters
>> @@ -721,6 +734,11 @@
>> # hostname must be provided so that the server's x509
>> # certificate identity can be validated. (Since 2.7)
>> #
>> +# @migrate-set-speed: to set maximum speed for migration. A value lesser than
>> +# zero will be automatically round upto zero.
>> +#
>> +# @migrate-set-downtime: set maximum tolerated downtime for migration.
>
> Same here, note that these are "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',
>> + 'migrate-set-speed': 'int',
>> + 'migrate-set-downtime': 'int'} }
>
> Same here about the names.
>
>> ##
>> # @query-migrate-parameters
>> #
>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>> index 6866264..c4d3809 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)
>> -
>> +- "migrate-set-speed": set maximum speed for migrations (json-octets)
>> +- "migrate-set-downtime": set maximum tolerated downtime (in seconds) for
>> + migrations (json-number)
>
> Same here about the names.
>
>> 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?,migrate-set-speed:o?,migrate-set-downtime:T?",
>
> Same here about the names. Also use "i" for QMP commands.
>
>> .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)
>> + - "migrate-set-speed" : maximium migration speed (json-octets)
>> + - "migration-set-downtime" : maximum tolerated downtime of migration
>> + (json-number)
>
> Same here about the names.
>
>> Arguments:
>>
>> @@ -3832,7 +3837,9 @@ Example:
>> "cpu-throttle-increment": 10,
>> "compress-threads": 8,
>> "compress-level": 1,
>> - "cpu-throttle-initial": 20
>> + "cpu-throttle-initial": 20,
>> + "migration-set-speed": 33554432,
>> + "migration-set-downtime": 300000000
>
> Same here about the names.
>
>> }
>> }
>>
>>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2] Move migrate_set_speed and migrate_set_downtime into migrate_set_parameter
2016-09-05 11:59 ` Ashijeet Acharya
@ 2016-09-05 12:01 ` Paolo Bonzini
2016-09-05 12:08 ` Ashijeet Acharya
0 siblings, 1 reply; 22+ messages in thread
From: Paolo Bonzini @ 2016-09-05 12:01 UTC (permalink / raw)
To: Ashijeet Acharya
Cc: lcapitulino, quintela, amit.shah, eblake, armbru, dgilbert,
QEMU Developers
On 05/09/2016 13:59, Ashijeet Acharya wrote:
>>> >> + if (has_migrate_set_speed) {
>>> >> + qmp_migrate_set_speed(migrate_set_speed, NULL);
>>> >> + }
>>> >> + if (has_migrate_set_downtime) {
>>> >> + qmp_migrate_set_downtime(migrate_set_downtime, NULL);
>>> >> + }
>> >
>> > This is backwards; qmp_migrate_set_speed and qmp_migrate_set_downtime
>> > should be implemented in terms of qmp_migrate_set_parameters, not the
>> > other way round.
>
> Okay. Got it!
Thinking more about it, it's even better to have two patches. One doing
it this way (qmp_migrate_set_parameters calls the old functions), the
second that reverses the direction.
Thanks!
Paolo
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2] Move migrate_set_speed and migrate_set_downtime into migrate_set_parameter
2016-09-05 12:01 ` Paolo Bonzini
@ 2016-09-05 12:08 ` Ashijeet Acharya
0 siblings, 0 replies; 22+ messages in thread
From: Ashijeet Acharya @ 2016-09-05 12:08 UTC (permalink / raw)
To: Paolo Bonzini
Cc: lcapitulino, quintela, amit.shah, eblake, armbru, dgilbert,
QEMU Developers
On Mon, Sep 5, 2016 at 5:31 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 05/09/2016 13:59, Ashijeet Acharya wrote:
>>>> >> + if (has_migrate_set_speed) {
>>>> >> + qmp_migrate_set_speed(migrate_set_speed, NULL);
>>>> >> + }
>>>> >> + if (has_migrate_set_downtime) {
>>>> >> + qmp_migrate_set_downtime(migrate_set_downtime, NULL);
>>>> >> + }
>>> >
>>> > This is backwards; qmp_migrate_set_speed and qmp_migrate_set_downtime
>>> > should be implemented in terms of qmp_migrate_set_parameters, not the
>>> > other way round.
>>
>> Okay. Got it!
>
> Thinking more about it, it's even better to have two patches. One doing
> it this way (qmp_migrate_set_parameters calls the old functions), the
> second that reverses the direction.
>
Fine, I will send both the versions soon.
Ashijeet
> Thanks!
>
> Paolo
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2] Move migrate_set_speed and migrate_set_downtime into migrate_set_parameter
2016-09-05 11:45 ` Paolo Bonzini
2016-09-05 11:59 ` Ashijeet Acharya
@ 2016-09-05 13:09 ` Ashijeet Acharya
2016-09-05 13:16 ` Paolo Bonzini
1 sibling, 1 reply; 22+ messages in thread
From: Ashijeet Acharya @ 2016-09-05 13:09 UTC (permalink / raw)
To: Paolo Bonzini
Cc: lcapitulino, quintela, amit.shah, eblake, armbru, dgilbert,
QEMU Developers
On Mon, Sep 5, 2016 at 5:15 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 05/09/2016 13:37, 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.
>>
>> Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
>> ---
>> hmp-commands.hx | 6 +++---
>> hmp.c | 30 +++++++++++++++++++++++++++++-
>> include/migration/migration.h | 1 -
>> migration/migration.c | 30 ++++++++++++++++++++++++++----
>> qapi-schema.json | 26 +++++++++++++++++++++++---
>> qmp-commands.hx | 13 ++++++++++---
>> 6 files changed, 91 insertions(+), 15 deletions(-)
>> 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?,migrate-set-speed:o?,migrate-set-downtime:T?",
>
> Same here about the names. Also use "i" for QMP commands.
I think we will have to use "T" for downtime at-least otherwise you
cant use float values for setting seconds like "0.2" for example.
No issues using "i" with bandwidth though.
Ashijeet
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2] Move migrate_set_speed and migrate_set_downtime into migrate_set_parameter
2016-09-05 13:09 ` Ashijeet Acharya
@ 2016-09-05 13:16 ` Paolo Bonzini
2016-09-05 14:26 ` [Qemu-devel] [PATCH v3] " Ashijeet Acharya
` (2 more replies)
0 siblings, 3 replies; 22+ messages in thread
From: Paolo Bonzini @ 2016-09-05 13:16 UTC (permalink / raw)
To: Ashijeet Acharya
Cc: lcapitulino, quintela, amit.shah, eblake, armbru, dgilbert,
QEMU Developers
On 05/09/2016 15:09, Ashijeet Acharya wrote:
>>> >> + "compress-level:i?,compress-threads:i?,decompress-threads:i?,cpu-throttle-initial:i?,cpu-throttle-increment:i?,migrate-set-speed:o?,migrate-set-downtime:T?",
>> >
>> > Same here about the names. Also use "i" for QMP commands.
> I think we will have to use "T" for downtime at-least otherwise you
> cant use float values for setting seconds like "0.2" for example.
> No issues using "i" with bandwidth though.
Right, I should have mentioned that my remark was about the "o" (it
caught my eye because of your change to hmp-commands.hx). Sorry!
Paolo
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v3] Move migrate_set_speed and migrate_set_downtime into migrate_set_parameter
2016-09-05 13:16 ` Paolo Bonzini
@ 2016-09-05 14:26 ` Ashijeet Acharya
2016-09-05 14:36 ` Daniel P. Berrange
2016-09-05 14:28 ` [Qemu-devel] [PATCH v3] Move migrate_set_speed and migrate_set_downtime into migrate_set_parameter (reversed logic) Ashijeet Acharya
2016-09-05 15:56 ` [Qemu-devel] [PATCH v2] Move migrate_set_speed and migrate_set_downtime into migrate_set_parameter Daniel P. Berrange
2 siblings, 1 reply; 22+ messages in thread
From: Ashijeet Acharya @ 2016-09-05 14:26 UTC (permalink / raw)
To: lcapitulino
Cc: quintela, amit.shah, eblake, armbru, dgilbert, pbonzini,
qemu-devel, Ashijeet Acharya
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(-)
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
}
}
--
2.6.2
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v3] Move migrate_set_speed and migrate_set_downtime into migrate_set_parameter (reversed logic)
2016-09-05 13:16 ` Paolo Bonzini
2016-09-05 14:26 ` [Qemu-devel] [PATCH v3] " Ashijeet Acharya
@ 2016-09-05 14:28 ` Ashijeet Acharya
2016-09-05 15:56 ` [Qemu-devel] [PATCH v2] Move migrate_set_speed and migrate_set_downtime into migrate_set_parameter Daniel P. Berrange
2 siblings, 0 replies; 22+ messages in thread
From: Ashijeet Acharya @ 2016-09-05 14:28 UTC (permalink / raw)
To: lcapitulino
Cc: quintela, amit.shah, eblake, armbru, dgilbert, pbonzini,
qemu-devel, Ashijeet Acharya
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. Make qmp_migrate_set_speed() and qmp_migrate_set_downtime() as wrappers on qmp_migrate_set_parameters().
Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
---
hmp.c | 33 ++++++++++++
include/migration/migration.h | 1 -
migration/migration.c | 116 ++++++++++++++++++++++++++++++++----------
qapi-schema.json | 26 ++++++++--
qmp-commands.hx | 13 +++--
5 files changed, 156 insertions(+), 33 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 */
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..df08867 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,29 @@ 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) {
+ if (max_bandwidth < 0) {
+ max_bandwidth = 0;
+ }
+ if (max_bandwidth > SIZE_MAX) {
+ max_bandwidth = SIZE_MAX;
+ }
+
+ s->parameters.max_bandwidth = max_bandwidth;
+ if (s->to_dst_file) {
+ qemu_file_set_rate_limit(s->to_dst_file,
+ s->parameters.max_bandwidth / XFER_LIMIT_RATIO);
+ }
+ }
+ if (has_downtime_limit) {
+ downtime_limit *= 1e9;
+ downtime_limit = MAX(0, MIN(UINT64_MAX, downtime_limit));
+
+ s = migrate_get_current();
+
+ max_downtime = (uint64_t)downtime_limit;
+ s->parameters.downtime_limit = max_downtime;
+ }
}
@@ -1163,30 +1196,61 @@ int64_t qmp_query_migrate_cache_size(Error **errp)
return migrate_xbzrle_cache_size();
}
-void qmp_migrate_set_speed(int64_t value, Error **errp)
-{
- MigrationState *s;
-
- if (value < 0) {
- value = 0;
- }
- if (value > SIZE_MAX) {
- value = SIZE_MAX;
- }
-
- s = migrate_get_current();
- s->bandwidth_limit = value;
- if (s->to_dst_file) {
- qemu_file_set_rate_limit(s->to_dst_file,
- s->bandwidth_limit / XFER_LIMIT_RATIO);
- }
-}
-
-void qmp_migrate_set_downtime(double value, Error **errp)
-{
- value *= 1e9;
- value = MAX(0, MIN(UINT64_MAX, value));
- max_downtime = (uint64_t)value;
+void qmp_migrate_set_speed(int64_t valuebw, Error **errp)
+{
+ bool has_compress_level = false;
+ bool has_compress_threads = false;
+ bool has_decompress_threads = false;
+ bool has_cpu_throttle_initial = false;
+ bool has_cpu_throttle_increment = false;
+ bool has_tls_creds = false;
+ bool has_tls_hostname = false;
+ bool has_max_bandwidth = true;
+ bool has_downtime_limit = false;
+ const char *valuestr = NULL;
+ long valueint = 0;
+ double valuedowntime = 0;
+ Error *err = NULL;
+
+ qmp_migrate_set_parameters(has_compress_level, valueint,
+ has_compress_threads, valueint,
+ has_decompress_threads, valueint,
+ has_cpu_throttle_initial, valueint,
+ has_cpu_throttle_increment, valueint,
+ has_tls_creds, valuestr,
+ has_tls_hostname, valuestr,
+ has_max_bandwidth, valuebw,
+ has_downtime_limit, valuedowntime,
+ &err);
+
+}
+
+void qmp_migrate_set_downtime(double valuedowntime, Error **errp)
+{
+ bool has_compress_level = false;
+ bool has_compress_threads = false;
+ bool has_decompress_threads = false;
+ bool has_cpu_throttle_initial = false;
+ 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 = true;
+ const char *valuestr = NULL;
+ long valueint = 0;
+ int64_t valuebw = 0;
+ Error *err = NULL;
+
+ qmp_migrate_set_parameters(has_compress_level, valueint,
+ has_compress_threads, valueint,
+ has_decompress_threads, valueint,
+ has_cpu_throttle_initial, valueint,
+ has_cpu_throttle_increment, valueint,
+ has_tls_creds, valuestr,
+ has_tls_hostname, valuestr,
+ has_max_bandwidth, valuebw,
+ has_downtime_limit, valuedowntime,
+ &err);
}
bool migrate_postcopy_ram(void)
@@ -1858,7 +1922,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
}
}
--
2.6.2
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v3] Move migrate_set_speed and migrate_set_downtime into migrate_set_parameter
2016-09-05 14:26 ` [Qemu-devel] [PATCH v3] " Ashijeet Acharya
@ 2016-09-05 14:36 ` Daniel P. Berrange
2016-09-05 15:14 ` Ashijeet Acharya
0 siblings, 1 reply; 22+ messages in thread
From: Daniel P. Berrange @ 2016-09-05 14:36 UTC (permalink / raw)
To: Ashijeet Acharya
Cc: lcapitulino, quintela, qemu-devel, armbru, dgilbert, amit.shah, pbonzini
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
--
|: 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 :|
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v3] Move migrate_set_speed and migrate_set_downtime into migrate_set_parameter
2016-09-05 14:36 ` Daniel P. Berrange
@ 2016-09-05 15:14 ` Ashijeet Acharya
2016-09-05 15:39 ` Daniel P. Berrange
0 siblings, 1 reply; 22+ messages in thread
From: Ashijeet Acharya @ 2016-09-05 15:14 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: lcapitulino, quintela, QEMU Developers, armbru, dgilbert,
amit.shah, Paolo Bonzini
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 :|
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v3] Move migrate_set_speed and migrate_set_downtime into migrate_set_parameter
2016-09-05 15:14 ` Ashijeet Acharya
@ 2016-09-05 15:39 ` Daniel P. Berrange
2016-09-05 15:46 ` Ashijeet Acharya
0 siblings, 1 reply; 22+ messages in thread
From: Daniel P. Berrange @ 2016-09-05 15:39 UTC (permalink / raw)
To: Ashijeet Acharya
Cc: lcapitulino, quintela, QEMU Developers, armbru, dgilbert,
amit.shah, Paolo Bonzini
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
--
|: 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 :|
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v3] Move migrate_set_speed and migrate_set_downtime into migrate_set_parameter
2016-09-05 15:39 ` Daniel P. Berrange
@ 2016-09-05 15:46 ` Ashijeet Acharya
0 siblings, 0 replies; 22+ messages in thread
From: Ashijeet Acharya @ 2016-09-05 15:46 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: lcapitulino, quintela, QEMU Developers, armbru, dgilbert,
amit.shah, Paolo Bonzini
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 :|
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2] Move migrate_set_speed and migrate_set_downtime into migrate_set_parameter
2016-09-05 13:16 ` Paolo Bonzini
2016-09-05 14:26 ` [Qemu-devel] [PATCH v3] " Ashijeet Acharya
2016-09-05 14:28 ` [Qemu-devel] [PATCH v3] Move migrate_set_speed and migrate_set_downtime into migrate_set_parameter (reversed logic) Ashijeet Acharya
@ 2016-09-05 15:56 ` Daniel P. Berrange
2016-09-06 8:16 ` Markus Armbruster
2 siblings, 1 reply; 22+ messages in thread
From: Daniel P. Berrange @ 2016-09-05 15:56 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Ashijeet Acharya, quintela, QEMU Developers, armbru, dgilbert,
amit.shah, lcapitulino
On Mon, Sep 05, 2016 at 03:16:04PM +0200, Paolo Bonzini wrote:
>
>
> On 05/09/2016 15:09, Ashijeet Acharya wrote:
> >>> >> + "compress-level:i?,compress-threads:i?,decompress-threads:i?,cpu-throttle-initial:i?,cpu-throttle-increment:i?,migrate-set-speed:o?,migrate-set-downtime:T?",
> >> >
> >> > Same here about the names. Also use "i" for QMP commands.
> > I think we will have to use "T" for downtime at-least otherwise you
> > cant use float values for setting seconds like "0.2" for example.
> > No issues using "i" with bandwidth though.
>
> Right, I should have mentioned that my remark was about the "o" (it
> caught my eye because of your change to hmp-commands.hx). Sorry!
I've always thought it a rather wierd to use fractional seconds for
downtime. It sort of made sense for HMP, but should really never have
been carried over into QMP. IMHO, we should make it just take integer
milliseconds for downtime with the new API.
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 :|
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2] Move migrate_set_speed and migrate_set_downtime into migrate_set_parameter
2016-09-05 15:56 ` [Qemu-devel] [PATCH v2] Move migrate_set_speed and migrate_set_downtime into migrate_set_parameter Daniel P. Berrange
@ 2016-09-06 8:16 ` Markus Armbruster
0 siblings, 0 replies; 22+ messages in thread
From: Markus Armbruster @ 2016-09-06 8:16 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: Paolo Bonzini, quintela, QEMU Developers, dgilbert,
Ashijeet Acharya, amit.shah, lcapitulino
"Daniel P. Berrange" <berrange@redhat.com> writes:
> On Mon, Sep 05, 2016 at 03:16:04PM +0200, Paolo Bonzini wrote:
>>
>>
>> On 05/09/2016 15:09, Ashijeet Acharya wrote:
>> >>> >> + "compress-level:i?,compress-threads:i?,decompress-threads:i?,cpu-throttle-initial:i?,cpu-throttle-increment:i?,migrate-set-speed:o?,migrate-set-downtime:T?",
>> >> >
>> >> > Same here about the names. Also use "i" for QMP commands.
>> > I think we will have to use "T" for downtime at-least otherwise you
>> > cant use float values for setting seconds like "0.2" for example.
>> > No issues using "i" with bandwidth though.
>>
>> Right, I should have mentioned that my remark was about the "o" (it
>> caught my eye because of your change to hmp-commands.hx). Sorry!
>
> I've always thought it a rather wierd to use fractional seconds for
> downtime. It sort of made sense for HMP, but should really never have
> been carried over into QMP. IMHO, we should make it just take integer
> milliseconds for downtime with the new API.
Seconds are a fine unit, and using floating-point numbers isn't weird,
especially not in JSON.
What's actually weird is our zoo of time units: seconds
(migrate_set_downtime), milliseconds (MigrationInfo), microseconds
(NetdevTapOptions), nanoseconds (BlockDeviceTimedStats), seconds +
fractional nanoseconds (SnapshotInfo), ... No adult supervision.
We should've picked *one* time unit for QMP. The obvious time unit is
seconds. Nanoseconds would've worked, too.
Time units are probably beyond repair now. We can still try to make
them at least locally consistent, i.e. stick to what related interfaces
use.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] Move migrate_set_speed and migrate_set_downtime into migrate_set_parameter
2016-09-05 11:16 ` Dr. David Alan Gilbert
2016-09-05 11:37 ` [Qemu-devel] [PATCH v2] " Ashijeet Acharya
@ 2016-09-06 13:16 ` Juan Quintela
1 sibling, 0 replies; 22+ messages in thread
From: Juan Quintela @ 2016-09-06 13:16 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: Ashijeet Acharya, Paolo Bonzini, armbru, QEMU Developers,
amit.shah, lcapitulino
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Ashijeet Acharya (ashijeetacharya@gmail.com) wrote:
>> On Mon, Sep 5, 2016 at 1:46 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> >
>> >
>> > On 05/09/2016 10:11, Ashijeet Acharya wrote:
>> >> > > Include migrate_set_speed and migrate_set_downtime inside
>> >> > > migrate_set_parameters respectively for setting maximum migration
>> >> > > speed and expected downtime parameters. Also add the query part for
>> >> > > both in qmp and hmp qemu control interfaces.
>> >> > >
>> >> > > Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
>> >> >
>> >> > You cannot break backwards compatibility for everyone that is using
>> >> > those commands, sorry.
>> >>
>> >> So should I keep the old commands too and add the new ones anyway for
>> >> the query part?
>> >
>> > You do not need query for the old ones, but you can indeed add support
>> > for speed and downtime in migrate-set-parameters and MigrationParameters.
>>
>> Right. So I will add the old-commands back and just add support for the new ones
>> under migration-set-parameters along with the query.
>
> Yes add them to both migrate_set_parameters and query_migrate_parameters
> and the hmp equivalents; but don't remove the old functions; just make
> them simple wrappers to call the migrate_set_parameters etc.
> Add a comment on them like 'kept for compatibility'.
I agree with that approach.
Thanks, Juan.
>
> Dave
>
>> > (Note: I'm not a migration maintainer, so they might say something else).
>>
>> Okay. I will wait for their views too.
>>
>> Ashijeet
>> >
>> > Paolo
>> >
>> >> The old ones will also be modified for query. So we will have
>> >> compatibility as well as the
>> >> query.
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2016-09-06 13:17 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-05 7:45 [Qemu-devel] [PATCH] Move migrate_set_speed and migrate_set_downtime into migrate_set_parameter Ashijeet Acharya
2016-09-05 8:01 ` Paolo Bonzini
2016-09-05 8:11 ` Ashijeet Acharya
2016-09-05 8:16 ` Paolo Bonzini
2016-09-05 8:25 ` Ashijeet Acharya
2016-09-05 11:16 ` Dr. David Alan Gilbert
2016-09-05 11:37 ` [Qemu-devel] [PATCH v2] " Ashijeet Acharya
2016-09-05 11:45 ` Paolo Bonzini
2016-09-05 11:59 ` Ashijeet Acharya
2016-09-05 12:01 ` Paolo Bonzini
2016-09-05 12:08 ` Ashijeet Acharya
2016-09-05 13:09 ` Ashijeet Acharya
2016-09-05 13:16 ` Paolo Bonzini
2016-09-05 14:26 ` [Qemu-devel] [PATCH v3] " Ashijeet Acharya
2016-09-05 14:36 ` Daniel P. Berrange
2016-09-05 15:14 ` Ashijeet Acharya
2016-09-05 15:39 ` Daniel P. Berrange
2016-09-05 15:46 ` Ashijeet Acharya
2016-09-05 14:28 ` [Qemu-devel] [PATCH v3] Move migrate_set_speed and migrate_set_downtime into migrate_set_parameter (reversed logic) Ashijeet Acharya
2016-09-05 15:56 ` [Qemu-devel] [PATCH v2] Move migrate_set_speed and migrate_set_downtime into migrate_set_parameter Daniel P. Berrange
2016-09-06 8:16 ` Markus Armbruster
2016-09-06 13:16 ` [Qemu-devel] [PATCH] " Juan Quintela
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.