Re: [Qemu-devel] [PATCH 5/7] Switch migrate_set_speed() to take an 'o' argument rather than a float.
[cc: Anthony, please review the proposed incompatible change of the
human monitor]
jes.soren...@redhat.com writes:
From: Jes Sorensen jes.soren...@redhat.com
Signed-off-by: Jes Sorensen jes.soren...@redhat.com
---
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.
Re: [Qemu-devel] [PATCH 5/7] Switch migrate_set_speed() to take an 'o' argument rather than a float.
On 10/11/2010 11:03 AM, Markus Armbruster wrote:
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.
Personally, I'd rather see the patch I sent incorporated, so that there
is no change in the monitor at all.
Not going to make a fuss of it as long as libvirt is okay, though.
Paolo
Re: [Qemu-devel] [PATCH 5/7] Switch migrate_set_speed() to take an 'o' argument rather than a float.
On 10/11/10 11:03, Markus Armbruster wrote:
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?
I agree it is incompatible, however it was a conscious decision to get
rid of the inconsistency we had around various places for these types of
arguments. If there is strong opposition to this change, then I can
mangle the interface to allow for the old, but IMHO bad, default of the
monitor.
Help should be updated in the same patch; please squash 6/7 into 5/7.
Ok will do.
Cheers,
Jes
