Message ID | 1286529360-5715-6-git-send-email-Jes.Sorensen@redhat.com |
---|---|
State | New |
Headers | show |
[cc: Anthony, please review the proposed incompatible change of the human monitor] Jes.Sorensen@redhat.com writes: > From: Jes Sorensen <Jes.Sorensen@redhat.com> > > Signed-off-by: Jes Sorensen <Jes.Sorensen@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.
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
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
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;