Re: [Qemu-devel] [PATCH 2/7] qapi: QMP input visitor, handle floats parsed as ints

[Andreas Färber]

Am 11.05.2012 19:04, schrieb Michael Roth:
> On Fri, May 11, 2012 at 01:22:33PM -0300, Luiz Capitulino wrote:
>> On Fri, 27 Apr 2012 15:21:18 -0500
>> Michael Roth <address@hidden> wrote:
>>
>>> JSON numbers can be interpreted as either integers or floating point
>>> values depending on their representation. As a result, QMP input visitor
>>> might visit a QInt when it was expecting a QFloat, so add handling to
>>> account for this.
>>>
>>> Signed-off-by: Michael Roth <address@hidden>
>>> ---
>>>  qapi/qmp-input-visitor.c |    9 +++++++--
>>>  1 files changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
>>> index 4cdc47d..bc91134 100644
>>> --- a/qapi/qmp-input-visitor.c
>>> +++ b/qapi/qmp-input-visitor.c
>>> @@ -246,13 +246,18 @@ static void qmp_input_type_number(Visitor *v, double 
>>> *obj, const char *name,
>>>      QmpInputVisitor *qiv = to_qiv(v);
>>>      QObject *qobj = qmp_input_get_object(qiv, name);
>>>  
>>> -    if (!qobj || qobject_type(qobj) != QTYPE_QFLOAT) {
>>> +    if (!qobj || (qobject_type(qobj) != QTYPE_QFLOAT &&
>>> +        qobject_type(qobj) != QTYPE_QINT)) {
>>>          error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
>>>                    "double");
>>
>> s/double/number
>>
>> It's also important to note that migrate_set_downtime is (positively) 
>> affected
>> by this change. Today, it only accepts a double, eg.:
>>
>> { "execute": "migrate_set_downtime", "arguments": { "value": 1 } }
>> {
>>     "error": {
>>         "class": "InvalidParameterType", 
>>         "desc": "Invalid parameter type for 'value', expected: double", 
>>         "data": {
>>             "name": "value", 
>>             "expected": "double"
>>         }
>>     }
>> }
>>
>> That's a bug (as it's documented to accept a number) and this patch fixes it.
>> There's an interface change, but I think it won't cause problems in practice.
>>
>> Please, fix the error message above and would be nice to get this (and patch
>> 02/07) as a separate series for 1.1.
> 
> Hmm, this is kinda awkward because I don't think we can change it without
> breaking any trees based off qom-next.

That's no problem at all, it is announced as rebasing and I have scripts
in place to pull and recursively rebase all my QOM branches.

For such a trivial textual change I can even fix it up manually. :)

Since Luiz is now taking care of this one I will simply drop it from my
qom-1.1 branch to avoid any confusion.

Andreas

> Since the error msg predates the bug fix (it just becomes more obviously
> wrong as a result of the fix), can you pull these commits as is into your
> QMP tree?
> 
> In the meantime I can send another patch, based on qom-next or qmp, that
> fixes the error msg. We can then get all 3 into 1.1/master via QMP tree, and I
> think qom-next should still merge cleanly back into master after 1.1
> 
>>
>>
>>>          return;
>>>      }
>>>  
>>> -    *obj = qfloat_get_double(qobject_to_qfloat(qobj));
>>> +    if (qobject_type(qobj) == QTYPE_QINT) {
>>> +        *obj = qint_get_int(qobject_to_qint(qobj));
>>> +    } else {
>>> +        *obj = qfloat_get_double(qobject_to_qfloat(qobj));
>>> +    }
>>>  }
>>>  
>>>  static void qmp_input_start_optional(Visitor *v, bool *present,

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg