[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 5/7] Switch migrate_set_speed() to take an 'o' a
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 5/7] Switch migrate_set_speed() to take an 'o' argument rather than a float. |
Date: |
Mon, 11 Oct 2010 11:03:41 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) |
[cc: Anthony, please review the proposed incompatible change of the
human monitor]
address@hidden writes:
> From: Jes Sorensen <address@hidden>
>
> Signed-off-by: Jes Sorensen <address@hidden>
> ---
> hmp-commands.hx | 2 +-
> migration.c | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 81999aa..95bdb91 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -754,7 +754,7 @@ ETEXI
>
> {
> .name = "migrate_set_speed",
> - .args_type = "value:f",
> + .args_type = "value:o",
> .params = "value",
> .help = "set maximum speed (in bytes) for migrations",
> .user_print = monitor_user_noop,
> diff --git a/migration.c b/migration.c
> index 468d517..9ee8b17 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -132,10 +132,10 @@ int do_migrate_cancel(Monitor *mon, const QDict *qdict,
> QObject **ret_data)
>
> int do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject
> **ret_data)
> {
> - double d;
> + int64_t d;
> FdMigrationState *s;
>
> - d = qdict_get_double(qdict, "value");
> + d = qdict_get_int(qdict, "value");
> d = MAX(0, MIN(UINT32_MAX, d));
> max_throttle = d;
As noted before, this is an incompatible change of the human monitor
command: unit now defaults to 'M'. This must be noted *prominently* in
the commit message. Best in the subject.
Incompatible changes can break tools. Quick grep of libvirt:
src/qemu/qemu_monitor_json.c: cmd =
qemuMonitorJSONMakeCommand("migrate_set_speed",
src/qemu/qemu_monitor_text.c: if (virAsprintf(&cmd, "migrate_set_speed
%lum", bandwidth) < 0) {
Looks like we're safe here.
Anthony, is this incompatibility okay?
Help should be updated in the same patch; please squash 6/7 into 5/7.
- [Qemu-devel] [PATCH 1/7] Introduce strtosz() library function to convert a string to a byte count., (continued)
- [Qemu-devel] [PATCH 2/7] Support human unit formats in strtosz, eg. 1.0G, Jes . Sorensen, 2010/10/08
- [Qemu-devel] [PATCH 3/7] Add more error handling to strtosz(), Jes . Sorensen, 2010/10/08
- [Qemu-devel] [PATCH 4/7] Add support for 'o' octet (bytes) format as monitor parameter., Jes . Sorensen, 2010/10/08
- [Qemu-devel] [PATCH 5/7] Switch migrate_set_speed() to take an 'o' argument rather than a float., Jes . Sorensen, 2010/10/08
- Re: [Qemu-devel] [PATCH 5/7] Switch migrate_set_speed() to take an 'o' argument rather than a float.,
Markus Armbruster <=
- [Qemu-devel] [PATCH 6/7] Clarify default values in migration speed argument in monitor, Jes . Sorensen, 2010/10/08
- [Qemu-devel] [PATCH 7/7] Remove obsolete 'f' double parameter type, Jes . Sorensen, 2010/10/08