On 09/08/2016 10:59 AM, Ashijeet Acharya wrote: Long subject line; we strive to stay below 70 bytes so that you can prepend commit ids and still stay in an 80-column window. Also, you can look at 'git shortlog -30' to get a feel for average length. Better might be:
migrate: move max-bandwidth and downtime-limit to migrate_set_parameter
> Mark old-commands for speed and downtime as deprecated.
> Move max-bandwidth and downtime-limit into migrate-set-parameters for
> setting maximum migration speed and expected downtime limit parameters
> respectively.
> Change downtime units to milliseconds (only for new-command) and update
> the query part in both hmp and qmp qemu control interfaces.
>
> Signed-off-by: Ashijeet Acharya <ashijeetacha...@gmail.com>
> ---
> hmp.c | 26 ++++++++++
Between the --- and diffstat is a nice place to tell us how v4 differs
from v3, to focus the attention of reviewers that have already seen this
once.
> +++ 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);
Worth including a unit (bytes/second, right?) in the output here?
> + monitor_printf(mon, " %s: %" PRId64,
> + MigrationParameter_lookup[MIGRATION_PARAMETER_DOWNTIME_LIMIT],
> + params->downtime_limit);
and here (milliseconds, right)? Particularly since the two are on
different scales.
> 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)
s/old-commands/backwards/
> {
> 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)
and again
> @@ -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,
> + int64_t downtime_limit,
> Error **errp)
> {
> + if (has_downtime_limit) {
> + downtime_limit *= 1000000; /* convert to nanoseconds */
> +
Still risks overflow on user-provided input. You should validate that
the user's input is reasonable, rather than accidentally setting a
too-small limit when the user meant for something so long as to appear
infinite.
> +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;
> + 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, valueint,
> + &err);
> +
And you may want to wait for v5 until after I post my patch later today,
so you can see what I meant in the v4 discussion about using the qapi
features for shorter code.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
