| Message ID | 4CEB5406.3060300@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
On Tue, 23 Nov 2010 13:41:26 +0800 Wen Congyang <wency@cn.fujitsu.com> wrote: > The args_type of migrate_set_speed in qmp-commands.hx is wrong. > When we set migrate speed by json, qemu will be core dumped. > > Signed-off-by: Wen Congyang Nice catch. Was caused by 07de3e60b05 and hence affects master only. Could you please mention that in the commit log? Also, your email address is missing in the signed-off-by line. > > --- > qmp-commands.hx | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/qmp-commands.hx b/qmp-commands.hx > index 793cf1c..16bdb08 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -495,7 +495,7 @@ EQMP > > { > .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,
On Tue, 23 Nov 2010 10:43:48 -0200 Luiz Capitulino <lcapitulino@redhat.com> wrote: > On Tue, 23 Nov 2010 13:41:26 +0800 > Wen Congyang <wency@cn.fujitsu.com> wrote: > > > The args_type of migrate_set_speed in qmp-commands.hx is wrong. > > When we set migrate speed by json, qemu will be core dumped. > > > > Signed-off-by: Wen Congyang > > Nice catch. > > Was caused by 07de3e60b05 and hence affects master only. Could you please > mention that in the commit log? Also, your email address is missing > in the signed-off-by line. There's another problem there: we used to accept a json number but now we accept only a json integer. Markus, are you aware of this change? > > > > > --- > > qmp-commands.hx | 2 +- > > 1 files changed, 1 insertions(+), 1 deletions(-) > > > > diff --git a/qmp-commands.hx b/qmp-commands.hx > > index 793cf1c..16bdb08 100644 > > --- a/qmp-commands.hx > > +++ b/qmp-commands.hx > > @@ -495,7 +495,7 @@ EQMP > > > > { > > .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, >
At 2010年11月23日 21:31, Luiz Capitulino Write: > On Tue, 23 Nov 2010 10:43:48 -0200 > Luiz Capitulino <lcapitulino@redhat.com> wrote: > >> On Tue, 23 Nov 2010 13:41:26 +0800 >> Wen Congyang <wency@cn.fujitsu.com> wrote: >> >>> The args_type of migrate_set_speed in qmp-commands.hx is wrong. >>> When we set migrate speed by json, qemu will be core dumped. >>> >>> Signed-off-by: Wen Congyang >> >> Nice catch. >> >> Was caused by 07de3e60b05 and hence affects master only. Could you please >> mention that in the commit log? Also, your email address is missing >> in the signed-off-by line. > > There's another problem there: we used to accept a json number but now we > accept only a json integer. Do you mean the parameter passed to migrate_set_speed by json should be float or integer? But in the function do_migrate_set_speed(), we only accept a integer. > > Markus, are you aware of this change? > >> >>> >>> --- >>> qmp-commands.hx | 2 +- >>> 1 files changed, 1 insertions(+), 1 deletions(-) >>> >>> diff --git a/qmp-commands.hx b/qmp-commands.hx >>> index 793cf1c..16bdb08 100644 >>> --- a/qmp-commands.hx >>> +++ b/qmp-commands.hx >>> @@ -495,7 +495,7 @@ EQMP >>> >>> { >>> .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, >> > >
On Wed, 24 Nov 2010 09:28:54 +0800 Wen Congyang <wency@cn.fujitsu.com> wrote: > At 2010年11月23日 21:31, Luiz Capitulino Write: > > On Tue, 23 Nov 2010 10:43:48 -0200 > > Luiz Capitulino <lcapitulino@redhat.com> wrote: > > > >> On Tue, 23 Nov 2010 13:41:26 +0800 > >> Wen Congyang <wency@cn.fujitsu.com> wrote: > >> > >>> The args_type of migrate_set_speed in qmp-commands.hx is wrong. > >>> When we set migrate speed by json, qemu will be core dumped. > >>> > >>> Signed-off-by: Wen Congyang > >> > >> Nice catch. > >> > >> Was caused by 07de3e60b05 and hence affects master only. Could you please > >> mention that in the commit log? Also, your email address is missing > >> in the signed-off-by line. > > > > There's another problem there: we used to accept a json number but now we > > accept only a json integer. > > Do you mean the parameter passed to migrate_set_speed by json should be > float or integer? We used to accept both until the aforementioned commit. > But in the function do_migrate_set_speed(), we only accept a integer. Yes, that's a behavior change and I'd like to check with Markus that he's aware of it. In any case, I'll apply your fix as soon as you send me a new version with the changes I asked. > > > > > Markus, are you aware of this change? > > > >> > >>> > >>> --- > >>> qmp-commands.hx | 2 +- > >>> 1 files changed, 1 insertions(+), 1 deletions(-) > >>> > >>> diff --git a/qmp-commands.hx b/qmp-commands.hx > >>> index 793cf1c..16bdb08 100644 > >>> --- a/qmp-commands.hx > >>> +++ b/qmp-commands.hx > >>> @@ -495,7 +495,7 @@ EQMP > >>> > >>> { > >>> .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, > >> > > > > >
[Note cc: Dan, Avi] Luiz Capitulino <lcapitulino@redhat.com> writes: > On Tue, 23 Nov 2010 10:43:48 -0200 > Luiz Capitulino <lcapitulino@redhat.com> wrote: > >> On Tue, 23 Nov 2010 13:41:26 +0800 >> Wen Congyang <wency@cn.fujitsu.com> wrote: >> >> > The args_type of migrate_set_speed in qmp-commands.hx is wrong. >> > When we set migrate speed by json, qemu will be core dumped. >> > >> > Signed-off-by: Wen Congyang >> >> Nice catch. >> >> Was caused by 07de3e60b05 and hence affects master only. Could you please >> mention that in the commit log? Also, your email address is missing >> in the signed-off-by line. > > There's another problem there: we used to accept a json number but now we > accept only a json integer. > > Markus, are you aware of this change? I pointed out the incompatible change in my review[*] of the offending patch: This isn't backwards bug-compatible. Before, a client could send any number. Any fractional part was ignored. Now, the number must be an integer. Other numbers are rejected. I don't care myself, but others have argued most forcefully for keeping QMP fully backward compatible from 0.13 on, so they might object. "Others" did not object, so this went in. Not released yet, so it's not too late to object. [*] http://lists.gnu.org/archive/html/qemu-devel/2010-09/msg01905.html
On Fri, 10 Dec 2010 16:20:34 +0100 Markus Armbruster <armbru@redhat.com> wrote: > [Note cc: Dan, Avi] > > Luiz Capitulino <lcapitulino@redhat.com> writes: > > > On Tue, 23 Nov 2010 10:43:48 -0200 > > Luiz Capitulino <lcapitulino@redhat.com> wrote: > > > >> On Tue, 23 Nov 2010 13:41:26 +0800 > >> Wen Congyang <wency@cn.fujitsu.com> wrote: > >> > >> > The args_type of migrate_set_speed in qmp-commands.hx is wrong. > >> > When we set migrate speed by json, qemu will be core dumped. > >> > > >> > Signed-off-by: Wen Congyang > >> > >> Nice catch. > >> > >> Was caused by 07de3e60b05 and hence affects master only. Could you please > >> mention that in the commit log? Also, your email address is missing > >> in the signed-off-by line. > > > > There's another problem there: we used to accept a json number but now we > > accept only a json integer. > > > > Markus, are you aware of this change? > > I pointed out the incompatible change in my review[*] of the offending > patch: Ok, I can remember now. I really think this is a small improvement and I doubt there's any client out there depending on this. > This isn't backwards bug-compatible. > > Before, a client could send any number. Any fractional part was > ignored. > > Now, the number must be an integer. Other numbers are rejected. > > I don't care myself, but others have argued most forcefully for keeping > QMP fully backward compatible from 0.13 on, so they might object. > > "Others" did not object, so this went in. Not released yet, so it's not > too late to object. > > > [*] http://lists.gnu.org/archive/html/qemu-devel/2010-09/msg01905.html >
diff --git a/qmp-commands.hx b/qmp-commands.hx index 793cf1c..16bdb08 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -495,7 +495,7 @@ EQMP { .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,
The args_type of migrate_set_speed in qmp-commands.hx is wrong. When we set migrate speed by json, qemu will be core dumped. Signed-off-by: Wen Congyang --- qmp-commands.hx | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)