On Fri, 11 May 2012 12:04:47 -0500 Michael Roth <mdr...@linux.vnet.ibm.com> wrote:
> On Fri, May 11, 2012 at 01:22:33PM -0300, Luiz Capitulino wrote: > > On Fri, 27 Apr 2012 15:21:18 -0500 > > Michael Roth <mdr...@linux.vnet.ibm.com> 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 <mdr...@linux.vnet.ibm.com> > > > --- > > > 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. > > 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 I think Andreas answered you here (in the sense that it's ok to respin this patch), but I also would like to add that patch 4/7 doesn't seem a bug fix to me (at least not worth it for 1.1).