[Qemu-devel] [PATCH] correct migrate_set_speed's args_type

[Qemu-devel] [PATCH] correct migrate_set_speed's args_type

[Wen Congyang]

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(-)

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,
-- 
1.7.1

[Qemu-devel] Re: [PATCH] correct migrate_set_speed's args_type

[Luiz Capitulino]

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,

[Qemu-devel] Re: [PATCH] correct migrate_set_speed's args_type

[Luiz Capitulino]

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,
> 

[Qemu-devel] Re: [PATCH] correct migrate_set_speed's args_type

[Wen Congyang]

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,
>>
> 
> 

[Qemu-devel] Re: [PATCH] correct migrate_set_speed's args_type

[Luiz Capitulino]

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,
> >>
> > 
> > 
> 

[Qemu-devel] [PATCH v2] correct migrate_set_speed's args_type

[Wen Congyang]

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.

This bug was caused by 07de3e60b05 and hence affects master only.

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>

---
 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,
-- 1.7.1 

[Qemu-devel] Re: [PATCH v2] correct migrate_set_speed's args_type

[Luiz Capitulino]

On Thu, 25 Nov 2010 09:06:05 +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.
> 
> This bug was caused by 07de3e60b05 and hence affects master only.
> 
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>

Applied, thanks.

> 
> ---
>  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,
> -- 1.7.1 
> 

[Qemu-devel] Re: [PATCH] correct migrate_set_speed's args_type

[Markus Armbruster]

[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

[Qemu-devel] Re: [PATCH] correct migrate_set_speed's args_type

[Luiz Capitulino]

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
>