| Message ID | 1464885987-4039-3-git-send-email-berrange@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 02/06/2016 18:46, Daniel P. Berrange wrote: > Currently the QmpInputVisitor assumes that all scalar > values are directly represented as their final types. > ie it assumes an 'int' is using QInt, and a 'bool' is > using QBool. > > This extends it so that QString is optionally permitted > for any of the non-string scalar types. This behaviour > is turned on by requesting the 'autocast' flag in the > constructor. > > This makes it possible to use QmpInputVisitor with a > QDict produced from QemuOpts, where everything is in > string format. Perhaps this should instead be a separate QmpStringInputVisitor visitor that _only_ accepts strings? You can reuse most of the QmpInputVisitor by putting it in the same file, because the struct and list visitors are compatible. I wonder if OptsVisitor could also be replaced by qemu_opts_to_qdict, an optional crumple step and the QmpStringInputVisitor (self-answer: probably not because of the magic integer range parsing). Paolo > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> > --- > docs/qapi-code-gen.txt | 2 +- > include/qapi/qmp-input-visitor.h | 6 +- > qapi/opts-visitor.c | 1 + > qapi/qmp-input-visitor.c | 89 ++++++++++++++++++++++------ > qmp.c | 2 +- > qom/qom-qobject.c | 2 +- > replay/replay-input.c | 2 +- > scripts/qapi-commands.py | 2 +- > tests/check-qnull.c | 2 +- > tests/test-qmp-commands.c | 2 +- > tests/test-qmp-input-strict.c | 2 +- > tests/test-qmp-input-visitor.c | 115 ++++++++++++++++++++++++++++++++++++- > tests/test-visitor-serialization.c | 2 +- > util/qemu-sockets.c | 2 +- > 14 files changed, 201 insertions(+), 30 deletions(-) > > diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt > index d7d6987..e21773e 100644 > --- a/docs/qapi-code-gen.txt > +++ b/docs/qapi-code-gen.txt > @@ -1008,7 +1008,7 @@ Example: > { > Error *err = NULL; > UserDefOne *retval; > - QmpInputVisitor *qiv = qmp_input_visitor_new(QOBJECT(args), true); > + QmpInputVisitor *qiv = qmp_input_visitor_new(QOBJECT(args), true, false); > QapiDeallocVisitor *qdv; > Visitor *v; > UserDefOneList *arg1 = NULL; > diff --git a/include/qapi/qmp-input-visitor.h b/include/qapi/qmp-input-visitor.h > index b0624d8..d35a053 100644 > --- a/include/qapi/qmp-input-visitor.h > +++ b/include/qapi/qmp-input-visitor.h > @@ -24,8 +24,12 @@ typedef struct QmpInputVisitor QmpInputVisitor; > * > * Set @strict to reject a parse that doesn't consume all keys of a > * dictionary; otherwise excess input is ignored. > + * Set @autocast to automatically convert string values into more > + * specific types (numbers, bools, etc) > */ > -QmpInputVisitor *qmp_input_visitor_new(QObject *obj, bool strict); > +QmpInputVisitor *qmp_input_visitor_new(QObject *obj, > + bool strict, > + bool autocast); > > void qmp_input_visitor_cleanup(QmpInputVisitor *v); > > diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c > index 4cf1cf8..00e4b1a 100644 > --- a/qapi/opts-visitor.c > +++ b/qapi/opts-visitor.c > @@ -347,6 +347,7 @@ opts_type_bool(Visitor *v, const char *name, bool *obj, Error **errp) > } > > if (opt->str) { > + /* Keep these values in sync with same code in qmp-input-visitor.c */ > if (strcmp(opt->str, "on") == 0 || > strcmp(opt->str, "yes") == 0 || > strcmp(opt->str, "y") == 0) { > diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c > index aea90a1..92023b1 100644 > --- a/qapi/qmp-input-visitor.c > +++ b/qapi/qmp-input-visitor.c > @@ -20,6 +20,7 @@ > #include "qemu-common.h" > #include "qapi/qmp/types.h" > #include "qapi/qmp/qerror.h" > +#include "qemu/cutils.h" > > #define QIV_STACK_SIZE 1024 > > @@ -45,6 +46,7 @@ struct QmpInputVisitor > > /* True to reject parse in visit_end_struct() if unvisited keys remain. */ > bool strict; > + bool autocast; > }; > > static QmpInputVisitor *to_qiv(Visitor *v) > @@ -254,15 +256,25 @@ static void qmp_input_type_int64(Visitor *v, const char *name, int64_t *obj, > Error **errp) > { > QmpInputVisitor *qiv = to_qiv(v); > - QInt *qint = qobject_to_qint(qmp_input_get_object(qiv, name, true)); > + QObject *qobj = qmp_input_get_object(qiv, name, true); > + QInt *qint; > + QString *qstr; > > - if (!qint) { > - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", > - "integer"); > + qint = qobject_to_qint(qobj); > + if (qint) { > + *obj = qint_get_int(qint); > return; > } > > - *obj = qint_get_int(qint); > + qstr = qobject_to_qstring(qobj); > + if (qstr && qstr->string && qiv->autocast) { > + if (qemu_strtoll(qstr->string, NULL, 10, obj) == 0) { > + return; > + } > + } > + > + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", > + "integer"); > } > > static void qmp_input_type_uint64(Visitor *v, const char *name, uint64_t *obj, > @@ -270,30 +282,61 @@ static void qmp_input_type_uint64(Visitor *v, const char *name, uint64_t *obj, > { > /* FIXME: qobject_to_qint mishandles values over INT64_MAX */ > QmpInputVisitor *qiv = to_qiv(v); > - QInt *qint = qobject_to_qint(qmp_input_get_object(qiv, name, true)); > + QObject *qobj = qmp_input_get_object(qiv, name, true); > + QInt *qint; > + QString *qstr; > > - if (!qint) { > - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", > - "integer"); > + qint = qobject_to_qint(qobj); > + if (qint) { > + *obj = qint_get_int(qint); > return; > } > > - *obj = qint_get_int(qint); > + qstr = qobject_to_qstring(qobj); > + if (qstr && qstr->string && qiv->autocast) { > + if (qemu_strtoull(qstr->string, NULL, 10, obj) == 0) { > + return; > + } > + } > + > + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", > + "integer"); > } > > static void qmp_input_type_bool(Visitor *v, const char *name, bool *obj, > Error **errp) > { > QmpInputVisitor *qiv = to_qiv(v); > - QBool *qbool = qobject_to_qbool(qmp_input_get_object(qiv, name, true)); > + QObject *qobj = qmp_input_get_object(qiv, name, true); > + QBool *qbool; > + QString *qstr; > > - if (!qbool) { > - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", > - "boolean"); > + qbool = qobject_to_qbool(qobj); > + if (qbool) { > + *obj = qbool_get_bool(qbool); > return; > } > > - *obj = qbool_get_bool(qbool); > + > + qstr = qobject_to_qstring(qobj); > + if (qstr && qstr->string && qiv->autocast) { > + /* Keep these values in sync with same code in opts-visitor.c */ > + if (!strcasecmp(qstr->string, "on") || > + !strcasecmp(qstr->string, "yes") || > + !strcasecmp(qstr->string, "true")) { > + *obj = true; > + return; > + } > + if (!strcasecmp(qstr->string, "off") || > + !strcasecmp(qstr->string, "no") || > + !strcasecmp(qstr->string, "false")) { > + *obj = false; > + return; > + } > + } > + > + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", > + "boolean"); > } > > static void qmp_input_type_str(Visitor *v, const char *name, char **obj, > @@ -319,6 +362,8 @@ static void qmp_input_type_number(Visitor *v, const char *name, double *obj, > QObject *qobj = qmp_input_get_object(qiv, name, true); > QInt *qint; > QFloat *qfloat; > + QString *qstr; > + char *endp; > > qint = qobject_to_qint(qobj); > if (qint) { > @@ -332,6 +377,15 @@ static void qmp_input_type_number(Visitor *v, const char *name, double *obj, > return; > } > > + qstr = qobject_to_qstring(qobj); > + if (qstr && qstr->string && qiv->autocast) { > + errno = 0; > + *obj = strtod(qstr->string, &endp); > + if (errno == 0 && endp != qstr->string && *endp == '\0') { > + return; > + } > + } > + > error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", > "number"); > } > @@ -381,7 +435,9 @@ void qmp_input_visitor_cleanup(QmpInputVisitor *v) > g_free(v); > } > > -QmpInputVisitor *qmp_input_visitor_new(QObject *obj, bool strict) > +QmpInputVisitor *qmp_input_visitor_new(QObject *obj, > + bool strict, > + bool autocast) > { > QmpInputVisitor *v; > > @@ -404,6 +460,7 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj, bool strict) > v->visitor.type_null = qmp_input_type_null; > v->visitor.optional = qmp_input_optional; > v->strict = strict; > + v->autocast = autocast; > > v->root = obj; > qobject_incref(obj); > diff --git a/qmp.c b/qmp.c > index 3165f87..dcd0953 100644 > --- a/qmp.c > +++ b/qmp.c > @@ -665,7 +665,7 @@ void qmp_object_add(const char *type, const char *id, > } > } > > - qiv = qmp_input_visitor_new(props, true); > + qiv = qmp_input_visitor_new(props, true, false); > obj = user_creatable_add_type(type, id, pdict, > qmp_input_get_visitor(qiv), errp); > qmp_input_visitor_cleanup(qiv); > diff --git a/qom/qom-qobject.c b/qom/qom-qobject.c > index b66088d..99666ce 100644 > --- a/qom/qom-qobject.c > +++ b/qom/qom-qobject.c > @@ -23,7 +23,7 @@ void object_property_set_qobject(Object *obj, QObject *value, > { > QmpInputVisitor *qiv; > /* TODO: Should we reject, rather than ignore, excess input? */ > - qiv = qmp_input_visitor_new(value, false); > + qiv = qmp_input_visitor_new(value, false, false); > object_property_set(obj, qmp_input_get_visitor(qiv), name, errp); > > qmp_input_visitor_cleanup(qiv); > diff --git a/replay/replay-input.c b/replay/replay-input.c > index 03e99d5..de82a59 100644 > --- a/replay/replay-input.c > +++ b/replay/replay-input.c > @@ -37,7 +37,7 @@ static InputEvent *qapi_clone_InputEvent(InputEvent *src) > return NULL; > } > > - qiv = qmp_input_visitor_new(obj, true); > + qiv = qmp_input_visitor_new(obj, true, false); > iv = qmp_input_get_visitor(qiv); > visit_type_InputEvent(iv, NULL, &dst, &error_abort); > qmp_input_visitor_cleanup(qiv); > diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py > index 8c6acb3..e48995d 100644 > --- a/scripts/qapi-commands.py > +++ b/scripts/qapi-commands.py > @@ -115,7 +115,7 @@ def gen_marshal(name, arg_type, ret_type): > > if arg_type and arg_type.members: > ret += mcgen(''' > - QmpInputVisitor *qiv = qmp_input_visitor_new(QOBJECT(args), true); > + QmpInputVisitor *qiv = qmp_input_visitor_new(QOBJECT(args), true, false); > QapiDeallocVisitor *qdv; > Visitor *v; > %(c_name)s arg = {0}; > diff --git a/tests/check-qnull.c b/tests/check-qnull.c > index fd9c68f..4c11755 100644 > --- a/tests/check-qnull.c > +++ b/tests/check-qnull.c > @@ -49,7 +49,7 @@ static void qnull_visit_test(void) > > g_assert(qnull_.refcnt == 1); > obj = qnull(); > - qiv = qmp_input_visitor_new(obj, true); > + qiv = qmp_input_visitor_new(obj, true, false); > qobject_decref(obj); > visit_type_null(qmp_input_get_visitor(qiv), NULL, &error_abort); > qmp_input_visitor_cleanup(qiv); > diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c > index 5c3edd7..c86b282 100644 > --- a/tests/test-qmp-commands.c > +++ b/tests/test-qmp-commands.c > @@ -222,7 +222,7 @@ static void test_dealloc_partial(void) > ud2_dict = qdict_new(); > qdict_put_obj(ud2_dict, "string0", QOBJECT(qstring_from_str(text))); > > - qiv = qmp_input_visitor_new(QOBJECT(ud2_dict), true); > + qiv = qmp_input_visitor_new(QOBJECT(ud2_dict), true, false); > visit_type_UserDefTwo(qmp_input_get_visitor(qiv), NULL, &ud2, &err); > qmp_input_visitor_cleanup(qiv); > QDECREF(ud2_dict); > diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c > index 4602529..f7f1f00 100644 > --- a/tests/test-qmp-input-strict.c > +++ b/tests/test-qmp-input-strict.c > @@ -55,7 +55,7 @@ static Visitor *validate_test_init_internal(TestInputVisitorData *data, > data->obj = qobject_from_jsonv(json_string, ap); > g_assert(data->obj); > > - data->qiv = qmp_input_visitor_new(data->obj, true); > + data->qiv = qmp_input_visitor_new(data->obj, true, false); > g_assert(data->qiv); > > v = qmp_input_get_visitor(data->qiv); > diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c > index cee07ce..5691dc3 100644 > --- a/tests/test-qmp-input-visitor.c > +++ b/tests/test-qmp-input-visitor.c > @@ -41,6 +41,7 @@ static void visitor_input_teardown(TestInputVisitorData *data, > function so that the JSON string used by the tests are kept in the test > functions (and not in main()). */ > static Visitor *visitor_input_test_init_internal(TestInputVisitorData *data, > + bool strict, bool autocast, > const char *json_string, > va_list *ap) > { > @@ -51,7 +52,7 @@ static Visitor *visitor_input_test_init_internal(TestInputVisitorData *data, > data->obj = qobject_from_jsonv(json_string, ap); > g_assert(data->obj); > > - data->qiv = qmp_input_visitor_new(data->obj, false); > + data->qiv = qmp_input_visitor_new(data->obj, strict, autocast); > g_assert(data->qiv); > > v = qmp_input_get_visitor(data->qiv); > @@ -60,6 +61,21 @@ static Visitor *visitor_input_test_init_internal(TestInputVisitorData *data, > return v; > } > > +static GCC_FMT_ATTR(4, 5) > +Visitor *visitor_input_test_init_full(TestInputVisitorData *data, > + bool strict, bool autocast, > + const char *json_string, ...) > +{ > + Visitor *v; > + va_list ap; > + > + va_start(ap, json_string); > + v = visitor_input_test_init_internal(data, strict, autocast, > + json_string, &ap); > + va_end(ap); > + return v; > +} > + > static GCC_FMT_ATTR(2, 3) > Visitor *visitor_input_test_init(TestInputVisitorData *data, > const char *json_string, ...) > @@ -68,7 +84,8 @@ Visitor *visitor_input_test_init(TestInputVisitorData *data, > va_list ap; > > va_start(ap, json_string); > - v = visitor_input_test_init_internal(data, json_string, &ap); > + v = visitor_input_test_init_internal(data, false, false, > + json_string, &ap); > va_end(ap); > return v; > } > @@ -83,7 +100,8 @@ Visitor *visitor_input_test_init(TestInputVisitorData *data, > static Visitor *visitor_input_test_init_raw(TestInputVisitorData *data, > const char *json_string) > { > - return visitor_input_test_init_internal(data, json_string, NULL); > + return visitor_input_test_init_internal(data, false, false, > + json_string, NULL); > } > > static void test_visitor_in_int(TestInputVisitorData *data, > @@ -115,6 +133,33 @@ static void test_visitor_in_int_overflow(TestInputVisitorData *data, > error_free_or_abort(&err); > } > > +static void test_visitor_in_int_autocast(TestInputVisitorData *data, > + const void *unused) > +{ > + int64_t res = 0, value = -42; > + Visitor *v; > + > + v = visitor_input_test_init_full(data, false, true, > + "\"-42\""); > + > + visit_type_int(v, NULL, &res, &error_abort); > + g_assert_cmpint(res, ==, value); > +} > + > +static void test_visitor_in_int_noautocast(TestInputVisitorData *data, > + const void *unused) > +{ > + int64_t res = 0; > + Visitor *v; > + Error *err = NULL; > + > + v = visitor_input_test_init(data, "\"-42\""); > + > + visit_type_int(v, NULL, &res, &err); > + g_assert(err != NULL); > + error_free(err); > +} > + > static void test_visitor_in_bool(TestInputVisitorData *data, > const void *unused) > { > @@ -127,6 +172,32 @@ static void test_visitor_in_bool(TestInputVisitorData *data, > g_assert_cmpint(res, ==, true); > } > > +static void test_visitor_in_bool_autocast(TestInputVisitorData *data, > + const void *unused) > +{ > + bool res = false; > + Visitor *v; > + > + v = visitor_input_test_init_full(data, false, true, "\"true\""); > + > + visit_type_bool(v, NULL, &res, &error_abort); > + g_assert_cmpint(res, ==, true); > +} > + > +static void test_visitor_in_bool_noautocast(TestInputVisitorData *data, > + const void *unused) > +{ > + bool res = false; > + Visitor *v; > + Error *err = NULL; > + > + v = visitor_input_test_init(data, "\"true\""); > + > + visit_type_bool(v, NULL, &res, &err); > + g_assert(err != NULL); > + error_free(err); > +} > + > static void test_visitor_in_number(TestInputVisitorData *data, > const void *unused) > { > @@ -139,6 +210,32 @@ static void test_visitor_in_number(TestInputVisitorData *data, > g_assert_cmpfloat(res, ==, value); > } > > +static void test_visitor_in_number_autocast(TestInputVisitorData *data, > + const void *unused) > +{ > + double res = 0, value = 3.14; > + Visitor *v; > + > + v = visitor_input_test_init_full(data, false, true, "\"3.14\""); > + > + visit_type_number(v, NULL, &res, &error_abort); > + g_assert_cmpfloat(res, ==, value); > +} > + > +static void test_visitor_in_number_noautocast(TestInputVisitorData *data, > + const void *unused) > +{ > + double res = 0; > + Visitor *v; > + Error *err = NULL; > + > + v = visitor_input_test_init(data, "\"3.14\""); > + > + visit_type_number(v, NULL, &res, &err); > + g_assert(err != NULL); > + error_free(err); > +} > + > static void test_visitor_in_string(TestInputVisitorData *data, > const void *unused) > { > @@ -835,10 +932,22 @@ int main(int argc, char **argv) > &in_visitor_data, test_visitor_in_int); > input_visitor_test_add("/visitor/input/int_overflow", > &in_visitor_data, test_visitor_in_int_overflow); > + input_visitor_test_add("/visitor/input/int_autocast", > + &in_visitor_data, test_visitor_in_int_autocast); > + input_visitor_test_add("/visitor/input/int_noautocast", > + &in_visitor_data, test_visitor_in_int_noautocast); > input_visitor_test_add("/visitor/input/bool", > &in_visitor_data, test_visitor_in_bool); > + input_visitor_test_add("/visitor/input/bool_autocast", > + &in_visitor_data, test_visitor_in_bool_autocast); > + input_visitor_test_add("/visitor/input/bool_noautocast", > + &in_visitor_data, test_visitor_in_bool_noautocast); > input_visitor_test_add("/visitor/input/number", > &in_visitor_data, test_visitor_in_number); > + input_visitor_test_add("/visitor/input/number_autocast", > + &in_visitor_data, test_visitor_in_number_autocast); > + input_visitor_test_add("/visitor/input/number_noautocast", > + &in_visitor_data, test_visitor_in_number_noautocast); > input_visitor_test_add("/visitor/input/string", > &in_visitor_data, test_visitor_in_string); > input_visitor_test_add("/visitor/input/enum", > diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c > index 7b14b5a..db618d8 100644 > --- a/tests/test-visitor-serialization.c > +++ b/tests/test-visitor-serialization.c > @@ -1038,7 +1038,7 @@ static void qmp_deserialize(void **native_out, void *datap, > obj = qobject_from_json(qstring_get_str(output_json)); > > QDECREF(output_json); > - d->qiv = qmp_input_visitor_new(obj, true); > + d->qiv = qmp_input_visitor_new(obj, true, false); > qobject_decref(obj_orig); > qobject_decref(obj); > visit(qmp_input_get_visitor(d->qiv), native_out, errp); > diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c > index 0d6cd1f..51c6a8e 100644 > --- a/util/qemu-sockets.c > +++ b/util/qemu-sockets.c > @@ -1145,7 +1145,7 @@ void qapi_copy_SocketAddress(SocketAddress **p_dest, > return; > } > > - qiv = qmp_input_visitor_new(obj, true); > + qiv = qmp_input_visitor_new(obj, true, false); > iv = qmp_input_get_visitor(qiv); > visit_type_SocketAddress(iv, NULL, p_dest, &error_abort); > qmp_input_visitor_cleanup(qiv); >
"Daniel P. Berrange" <berrange@redhat.com> writes: > Currently the QmpInputVisitor assumes that all scalar > values are directly represented as their final types. > ie it assumes an 'int' is using QInt, and a 'bool' is > using QBool. > > This extends it so that QString is optionally permitted > for any of the non-string scalar types. This behaviour > is turned on by requesting the 'autocast' flag in the > constructor. > > This makes it possible to use QmpInputVisitor with a > QDict produced from QemuOpts, where everything is in > string format. We're still digging. > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> > --- > docs/qapi-code-gen.txt | 2 +- > include/qapi/qmp-input-visitor.h | 6 +- > qapi/opts-visitor.c | 1 + > qapi/qmp-input-visitor.c | 89 ++++++++++++++++++++++------ > qmp.c | 2 +- > qom/qom-qobject.c | 2 +- > replay/replay-input.c | 2 +- > scripts/qapi-commands.py | 2 +- > tests/check-qnull.c | 2 +- > tests/test-qmp-commands.c | 2 +- > tests/test-qmp-input-strict.c | 2 +- > tests/test-qmp-input-visitor.c | 115 ++++++++++++++++++++++++++++++++++++- > tests/test-visitor-serialization.c | 2 +- > util/qemu-sockets.c | 2 +- > 14 files changed, 201 insertions(+), 30 deletions(-) > > diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt > index d7d6987..e21773e 100644 > --- a/docs/qapi-code-gen.txt > +++ b/docs/qapi-code-gen.txt > @@ -1008,7 +1008,7 @@ Example: > { > Error *err = NULL; > UserDefOne *retval; > - QmpInputVisitor *qiv = qmp_input_visitor_new(QOBJECT(args), true); > + QmpInputVisitor *qiv = qmp_input_visitor_new(QOBJECT(args), true, false); > QapiDeallocVisitor *qdv; > Visitor *v; > UserDefOneList *arg1 = NULL; > diff --git a/include/qapi/qmp-input-visitor.h b/include/qapi/qmp-input-visitor.h > index b0624d8..d35a053 100644 > --- a/include/qapi/qmp-input-visitor.h > +++ b/include/qapi/qmp-input-visitor.h > @@ -24,8 +24,12 @@ typedef struct QmpInputVisitor QmpInputVisitor; > * > * Set @strict to reject a parse that doesn't consume all keys of a > * dictionary; otherwise excess input is ignored. > + * Set @autocast to automatically convert string values into more > + * specific types (numbers, bools, etc) > */ > -QmpInputVisitor *qmp_input_visitor_new(QObject *obj, bool strict); > +QmpInputVisitor *qmp_input_visitor_new(QObject *obj, > + bool strict, > + bool autocast); > > void qmp_input_visitor_cleanup(QmpInputVisitor *v); > > diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c > index 4cf1cf8..00e4b1a 100644 > --- a/qapi/opts-visitor.c > +++ b/qapi/opts-visitor.c > @@ -347,6 +347,7 @@ opts_type_bool(Visitor *v, const char *name, bool *obj, Error **errp) > } > > if (opt->str) { > + /* Keep these values in sync with same code in qmp-input-visitor.c */ Also with parse_option_bool(). That's the canonical place, in fact. > if (strcmp(opt->str, "on") == 0 || > strcmp(opt->str, "yes") == 0 || > strcmp(opt->str, "y") == 0) { > diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c > index aea90a1..92023b1 100644 > --- a/qapi/qmp-input-visitor.c > +++ b/qapi/qmp-input-visitor.c > @@ -20,6 +20,7 @@ > #include "qemu-common.h" > #include "qapi/qmp/types.h" > #include "qapi/qmp/qerror.h" > +#include "qemu/cutils.h" > > #define QIV_STACK_SIZE 1024 > > @@ -45,6 +46,7 @@ struct QmpInputVisitor > > /* True to reject parse in visit_end_struct() if unvisited keys remain. */ > bool strict; > + bool autocast; > }; > > static QmpInputVisitor *to_qiv(Visitor *v) > @@ -254,15 +256,25 @@ static void qmp_input_type_int64(Visitor *v, const char *name, int64_t *obj, > Error **errp) > { > QmpInputVisitor *qiv = to_qiv(v); > - QInt *qint = qobject_to_qint(qmp_input_get_object(qiv, name, true)); > + QObject *qobj = qmp_input_get_object(qiv, name, true); > + QInt *qint; > + QString *qstr; > > - if (!qint) { > - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", > - "integer"); > + qint = qobject_to_qint(qobj); > + if (qint) { > + *obj = qint_get_int(qint); > return; > } > > - *obj = qint_get_int(qint); > + qstr = qobject_to_qstring(qobj); > + if (qstr && qstr->string && qiv->autocast) { > + if (qemu_strtoll(qstr->string, NULL, 10, obj) == 0) { In the commit message, you mentioned intended use: "with a QDict produced from QemuOpts, where everything is in string format." I figure you mean opts with empty opts->list->desc[]. For those, we accept any key and parse all values as strings. The idea with such QemuOpts is to *delay* parsing until we know how to parse. The delay should be transparent to the user. In particular, values should be parsed the same whether the parsing is delayed or not. The canonical QemuOpts value parser is qemu_opt_parse(). It parses integers with parse_option_number(). That function parses like stroull(qstr->string, ..., 0). Two differences: * stroll() vs. strtoull(): they differ in ERANGE behavior. This might be tolerable, but I haven't thought it through. * Base 0 vs 10: different syntax and semantics. Oops. > + return; > + } > + } > + > + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", > + "integer"); > } > > static void qmp_input_type_uint64(Visitor *v, const char *name, uint64_t *obj, > @@ -270,30 +282,61 @@ static void qmp_input_type_uint64(Visitor *v, const char *name, uint64_t *obj, > { > /* FIXME: qobject_to_qint mishandles values over INT64_MAX */ > QmpInputVisitor *qiv = to_qiv(v); > - QInt *qint = qobject_to_qint(qmp_input_get_object(qiv, name, true)); > + QObject *qobj = qmp_input_get_object(qiv, name, true); > + QInt *qint; > + QString *qstr; > > - if (!qint) { > - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", > - "integer"); > + qint = qobject_to_qint(qobj); > + if (qint) { > + *obj = qint_get_int(qint); > return; > } > > - *obj = qint_get_int(qint); > + qstr = qobject_to_qstring(qobj); > + if (qstr && qstr->string && qiv->autocast) { > + if (qemu_strtoull(qstr->string, NULL, 10, obj) == 0) { Same issue with base 0 vs. 10. > + return; > + } > + } > + > + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", > + "integer"); > } > > static void qmp_input_type_bool(Visitor *v, const char *name, bool *obj, > Error **errp) > { > QmpInputVisitor *qiv = to_qiv(v); > - QBool *qbool = qobject_to_qbool(qmp_input_get_object(qiv, name, true)); > + QObject *qobj = qmp_input_get_object(qiv, name, true); > + QBool *qbool; > + QString *qstr; > > - if (!qbool) { > - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", > - "boolean"); > + qbool = qobject_to_qbool(qobj); > + if (qbool) { > + *obj = qbool_get_bool(qbool); > return; > } > > - *obj = qbool_get_bool(qbool); > + > + qstr = qobject_to_qstring(qobj); > + if (qstr && qstr->string && qiv->autocast) { > + /* Keep these values in sync with same code in opts-visitor.c */ Also with parse_option_bool(). > + if (!strcasecmp(qstr->string, "on") || > + !strcasecmp(qstr->string, "yes") || > + !strcasecmp(qstr->string, "true")) { > + *obj = true; > + return; > + } > + if (!strcasecmp(qstr->string, "off") || > + !strcasecmp(qstr->string, "no") || > + !strcasecmp(qstr->string, "false")) { > + *obj = false; > + return; > + } You follow opts-visitor.c. It accepts more than parse_option_bool(). Glorious. I guess the only way out is to add another onion dome to qemu-option.c and accept yes, no, true, false there as well. > + } > + > + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", > + "boolean"); > } > > static void qmp_input_type_str(Visitor *v, const char *name, char **obj, > @@ -319,6 +362,8 @@ static void qmp_input_type_number(Visitor *v, const char *name, double *obj, > QObject *qobj = qmp_input_get_object(qiv, name, true); > QInt *qint; > QFloat *qfloat; > + QString *qstr; > + char *endp; > > qint = qobject_to_qint(qobj); > if (qint) { > @@ -332,6 +377,15 @@ static void qmp_input_type_number(Visitor *v, const char *name, double *obj, > return; > } > > + qstr = qobject_to_qstring(qobj); > + if (qstr && qstr->string && qiv->autocast) { > + errno = 0; > + *obj = strtod(qstr->string, &endp); > + if (errno == 0 && endp != qstr->string && *endp == '\0') { > + return; > + } > + } > + > error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", > "number"); > } QemuOpts do not support floating-point numbers. Doesn't make accepting them here wrong. > @@ -381,7 +435,9 @@ void qmp_input_visitor_cleanup(QmpInputVisitor *v) > g_free(v); > } > > -QmpInputVisitor *qmp_input_visitor_new(QObject *obj, bool strict) > +QmpInputVisitor *qmp_input_visitor_new(QObject *obj, > + bool strict, > + bool autocast) > { > QmpInputVisitor *v; > > @@ -404,6 +460,7 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj, bool strict) > v->visitor.type_null = qmp_input_type_null; > v->visitor.optional = qmp_input_optional; > v->strict = strict; > + v->autocast = autocast; > > v->root = obj; > qobject_incref(obj); > diff --git a/qmp.c b/qmp.c > index 3165f87..dcd0953 100644 > --- a/qmp.c > +++ b/qmp.c > @@ -665,7 +665,7 @@ void qmp_object_add(const char *type, const char *id, > } > } > > - qiv = qmp_input_visitor_new(props, true); > + qiv = qmp_input_visitor_new(props, true, false); > obj = user_creatable_add_type(type, id, pdict, > qmp_input_get_visitor(qiv), errp); > qmp_input_visitor_cleanup(qiv); > diff --git a/qom/qom-qobject.c b/qom/qom-qobject.c > index b66088d..99666ce 100644 > --- a/qom/qom-qobject.c > +++ b/qom/qom-qobject.c > @@ -23,7 +23,7 @@ void object_property_set_qobject(Object *obj, QObject *value, > { > QmpInputVisitor *qiv; > /* TODO: Should we reject, rather than ignore, excess input? */ > - qiv = qmp_input_visitor_new(value, false); > + qiv = qmp_input_visitor_new(value, false, false); > object_property_set(obj, qmp_input_get_visitor(qiv), name, errp); > > qmp_input_visitor_cleanup(qiv); > diff --git a/replay/replay-input.c b/replay/replay-input.c > index 03e99d5..de82a59 100644 > --- a/replay/replay-input.c > +++ b/replay/replay-input.c > @@ -37,7 +37,7 @@ static InputEvent *qapi_clone_InputEvent(InputEvent *src) > return NULL; > } > > - qiv = qmp_input_visitor_new(obj, true); > + qiv = qmp_input_visitor_new(obj, true, false); > iv = qmp_input_get_visitor(qiv); > visit_type_InputEvent(iv, NULL, &dst, &error_abort); > qmp_input_visitor_cleanup(qiv); > diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py > index 8c6acb3..e48995d 100644 > --- a/scripts/qapi-commands.py > +++ b/scripts/qapi-commands.py > @@ -115,7 +115,7 @@ def gen_marshal(name, arg_type, ret_type): > > if arg_type and arg_type.members: > ret += mcgen(''' > - QmpInputVisitor *qiv = qmp_input_visitor_new(QOBJECT(args), true); > + QmpInputVisitor *qiv = qmp_input_visitor_new(QOBJECT(args), true, false); > QapiDeallocVisitor *qdv; > Visitor *v; > %(c_name)s arg = {0}; > diff --git a/tests/check-qnull.c b/tests/check-qnull.c > index fd9c68f..4c11755 100644 > --- a/tests/check-qnull.c > +++ b/tests/check-qnull.c > @@ -49,7 +49,7 @@ static void qnull_visit_test(void) > > g_assert(qnull_.refcnt == 1); > obj = qnull(); > - qiv = qmp_input_visitor_new(obj, true); > + qiv = qmp_input_visitor_new(obj, true, false); > qobject_decref(obj); > visit_type_null(qmp_input_get_visitor(qiv), NULL, &error_abort); > qmp_input_visitor_cleanup(qiv); > diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c > index 5c3edd7..c86b282 100644 > --- a/tests/test-qmp-commands.c > +++ b/tests/test-qmp-commands.c > @@ -222,7 +222,7 @@ static void test_dealloc_partial(void) > ud2_dict = qdict_new(); > qdict_put_obj(ud2_dict, "string0", QOBJECT(qstring_from_str(text))); > > - qiv = qmp_input_visitor_new(QOBJECT(ud2_dict), true); > + qiv = qmp_input_visitor_new(QOBJECT(ud2_dict), true, false); > visit_type_UserDefTwo(qmp_input_get_visitor(qiv), NULL, &ud2, &err); > qmp_input_visitor_cleanup(qiv); > QDECREF(ud2_dict); > diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c > index 4602529..f7f1f00 100644 > --- a/tests/test-qmp-input-strict.c > +++ b/tests/test-qmp-input-strict.c > @@ -55,7 +55,7 @@ static Visitor *validate_test_init_internal(TestInputVisitorData *data, > data->obj = qobject_from_jsonv(json_string, ap); > g_assert(data->obj); > > - data->qiv = qmp_input_visitor_new(data->obj, true); > + data->qiv = qmp_input_visitor_new(data->obj, true, false); > g_assert(data->qiv); > > v = qmp_input_get_visitor(data->qiv); > diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c > index cee07ce..5691dc3 100644 > --- a/tests/test-qmp-input-visitor.c > +++ b/tests/test-qmp-input-visitor.c > @@ -41,6 +41,7 @@ static void visitor_input_teardown(TestInputVisitorData *data, > function so that the JSON string used by the tests are kept in the test > functions (and not in main()). */ > static Visitor *visitor_input_test_init_internal(TestInputVisitorData *data, > + bool strict, bool autocast, > const char *json_string, > va_list *ap) > { > @@ -51,7 +52,7 @@ static Visitor *visitor_input_test_init_internal(TestInputVisitorData *data, > data->obj = qobject_from_jsonv(json_string, ap); > g_assert(data->obj); > > - data->qiv = qmp_input_visitor_new(data->obj, false); > + data->qiv = qmp_input_visitor_new(data->obj, strict, autocast); > g_assert(data->qiv); > > v = qmp_input_get_visitor(data->qiv); > @@ -60,6 +61,21 @@ static Visitor *visitor_input_test_init_internal(TestInputVisitorData *data, > return v; > } > > +static GCC_FMT_ATTR(4, 5) > +Visitor *visitor_input_test_init_full(TestInputVisitorData *data, > + bool strict, bool autocast, > + const char *json_string, ...) > +{ > + Visitor *v; > + va_list ap; > + > + va_start(ap, json_string); > + v = visitor_input_test_init_internal(data, strict, autocast, > + json_string, &ap); > + va_end(ap); > + return v; > +} > + > static GCC_FMT_ATTR(2, 3) > Visitor *visitor_input_test_init(TestInputVisitorData *data, > const char *json_string, ...) > @@ -68,7 +84,8 @@ Visitor *visitor_input_test_init(TestInputVisitorData *data, > va_list ap; > > va_start(ap, json_string); > - v = visitor_input_test_init_internal(data, json_string, &ap); > + v = visitor_input_test_init_internal(data, false, false, > + json_string, &ap); > va_end(ap); > return v; > } > @@ -83,7 +100,8 @@ Visitor *visitor_input_test_init(TestInputVisitorData *data, > static Visitor *visitor_input_test_init_raw(TestInputVisitorData *data, > const char *json_string) > { > - return visitor_input_test_init_internal(data, json_string, NULL); > + return visitor_input_test_init_internal(data, false, false, > + json_string, NULL); > } > > static void test_visitor_in_int(TestInputVisitorData *data, > @@ -115,6 +133,33 @@ static void test_visitor_in_int_overflow(TestInputVisitorData *data, > error_free_or_abort(&err); > } > > +static void test_visitor_in_int_autocast(TestInputVisitorData *data, > + const void *unused) > +{ > + int64_t res = 0, value = -42; > + Visitor *v; > + > + v = visitor_input_test_init_full(data, false, true, > + "\"-42\""); > + > + visit_type_int(v, NULL, &res, &error_abort); > + g_assert_cmpint(res, ==, value); > +} > + > +static void test_visitor_in_int_noautocast(TestInputVisitorData *data, > + const void *unused) > +{ > + int64_t res = 0; > + Visitor *v; > + Error *err = NULL; > + > + v = visitor_input_test_init(data, "\"-42\""); > + > + visit_type_int(v, NULL, &res, &err); > + g_assert(err != NULL); > + error_free(err); > +} > + > static void test_visitor_in_bool(TestInputVisitorData *data, > const void *unused) > { > @@ -127,6 +172,32 @@ static void test_visitor_in_bool(TestInputVisitorData *data, > g_assert_cmpint(res, ==, true); > } > > +static void test_visitor_in_bool_autocast(TestInputVisitorData *data, > + const void *unused) > +{ > + bool res = false; > + Visitor *v; > + > + v = visitor_input_test_init_full(data, false, true, "\"true\""); > + > + visit_type_bool(v, NULL, &res, &error_abort); > + g_assert_cmpint(res, ==, true); > +} > + > +static void test_visitor_in_bool_noautocast(TestInputVisitorData *data, > + const void *unused) > +{ > + bool res = false; > + Visitor *v; > + Error *err = NULL; > + > + v = visitor_input_test_init(data, "\"true\""); > + > + visit_type_bool(v, NULL, &res, &err); > + g_assert(err != NULL); > + error_free(err); > +} > + > static void test_visitor_in_number(TestInputVisitorData *data, > const void *unused) > { > @@ -139,6 +210,32 @@ static void test_visitor_in_number(TestInputVisitorData *data, > g_assert_cmpfloat(res, ==, value); > } > > +static void test_visitor_in_number_autocast(TestInputVisitorData *data, > + const void *unused) > +{ > + double res = 0, value = 3.14; > + Visitor *v; > + > + v = visitor_input_test_init_full(data, false, true, "\"3.14\""); > + > + visit_type_number(v, NULL, &res, &error_abort); > + g_assert_cmpfloat(res, ==, value); > +} > + > +static void test_visitor_in_number_noautocast(TestInputVisitorData *data, > + const void *unused) > +{ > + double res = 0; > + Visitor *v; > + Error *err = NULL; > + > + v = visitor_input_test_init(data, "\"3.14\""); > + > + visit_type_number(v, NULL, &res, &err); > + g_assert(err != NULL); > + error_free(err); > +} > + > static void test_visitor_in_string(TestInputVisitorData *data, > const void *unused) > { > @@ -835,10 +932,22 @@ int main(int argc, char **argv) > &in_visitor_data, test_visitor_in_int); > input_visitor_test_add("/visitor/input/int_overflow", > &in_visitor_data, test_visitor_in_int_overflow); > + input_visitor_test_add("/visitor/input/int_autocast", > + &in_visitor_data, test_visitor_in_int_autocast); > + input_visitor_test_add("/visitor/input/int_noautocast", > + &in_visitor_data, test_visitor_in_int_noautocast); > input_visitor_test_add("/visitor/input/bool", > &in_visitor_data, test_visitor_in_bool); > + input_visitor_test_add("/visitor/input/bool_autocast", > + &in_visitor_data, test_visitor_in_bool_autocast); > + input_visitor_test_add("/visitor/input/bool_noautocast", > + &in_visitor_data, test_visitor_in_bool_noautocast); > input_visitor_test_add("/visitor/input/number", > &in_visitor_data, test_visitor_in_number); > + input_visitor_test_add("/visitor/input/number_autocast", > + &in_visitor_data, test_visitor_in_number_autocast); > + input_visitor_test_add("/visitor/input/number_noautocast", > + &in_visitor_data, test_visitor_in_number_noautocast); > input_visitor_test_add("/visitor/input/string", > &in_visitor_data, test_visitor_in_string); > input_visitor_test_add("/visitor/input/enum", > diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c > index 7b14b5a..db618d8 100644 > --- a/tests/test-visitor-serialization.c > +++ b/tests/test-visitor-serialization.c > @@ -1038,7 +1038,7 @@ static void qmp_deserialize(void **native_out, void *datap, > obj = qobject_from_json(qstring_get_str(output_json)); > > QDECREF(output_json); > - d->qiv = qmp_input_visitor_new(obj, true); > + d->qiv = qmp_input_visitor_new(obj, true, false); > qobject_decref(obj_orig); > qobject_decref(obj); > visit(qmp_input_get_visitor(d->qiv), native_out, errp); > diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c > index 0d6cd1f..51c6a8e 100644 > --- a/util/qemu-sockets.c > +++ b/util/qemu-sockets.c > @@ -1145,7 +1145,7 @@ void qapi_copy_SocketAddress(SocketAddress **p_dest, > return; > } > > - qiv = qmp_input_visitor_new(obj, true); > + qiv = qmp_input_visitor_new(obj, true, false); > iv = qmp_input_get_visitor(qiv); > visit_type_SocketAddress(iv, NULL, p_dest, &error_abort); > qmp_input_visitor_cleanup(qiv);
On Thu, Jun 09, 2016 at 04:03:50PM +0200, Markus Armbruster wrote: > "Daniel P. Berrange" <berrange@redhat.com> writes: > > > Currently the QmpInputVisitor assumes that all scalar > > values are directly represented as their final types. > > ie it assumes an 'int' is using QInt, and a 'bool' is > > using QBool. > > > > This extends it so that QString is optionally permitted > > for any of the non-string scalar types. This behaviour > > is turned on by requesting the 'autocast' flag in the > > constructor. > > > > This makes it possible to use QmpInputVisitor with a > > QDict produced from QemuOpts, where everything is in > > string format. > > We're still digging. > > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> > > --- > > docs/qapi-code-gen.txt | 2 +- > > include/qapi/qmp-input-visitor.h | 6 +- > > qapi/opts-visitor.c | 1 + > > qapi/qmp-input-visitor.c | 89 ++++++++++++++++++++++------ > > qmp.c | 2 +- > > qom/qom-qobject.c | 2 +- > > replay/replay-input.c | 2 +- > > scripts/qapi-commands.py | 2 +- > > tests/check-qnull.c | 2 +- > > tests/test-qmp-commands.c | 2 +- > > tests/test-qmp-input-strict.c | 2 +- > > tests/test-qmp-input-visitor.c | 115 ++++++++++++++++++++++++++++++++++++- > > tests/test-visitor-serialization.c | 2 +- > > util/qemu-sockets.c | 2 +- > > 14 files changed, 201 insertions(+), 30 deletions(-) > > diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c > > index 4cf1cf8..00e4b1a 100644 > > --- a/qapi/opts-visitor.c > > +++ b/qapi/opts-visitor.c > > @@ -347,6 +347,7 @@ opts_type_bool(Visitor *v, const char *name, bool *obj, Error **errp) > > } > > > > if (opt->str) { > > + /* Keep these values in sync with same code in qmp-input-visitor.c */ > > Also with parse_option_bool(). That's the canonical place, in fact. ....except parse_option_bool() doesn't allow "yes", "no", "y", "n" as valid values - it only permits "on" and "off" :-( I guess I could make the parse_option_bool() method non-static, make it accept these missing values, and then call it from all places so we have guaranteed consistency. > > > if (strcmp(opt->str, "on") == 0 || > > strcmp(opt->str, "yes") == 0 || > > strcmp(opt->str, "y") == 0) { > > diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c > > index aea90a1..92023b1 100644 > > --- a/qapi/qmp-input-visitor.c > > +++ b/qapi/qmp-input-visitor.c > > @@ -20,6 +20,7 @@ > > #include "qemu-common.h" > > #include "qapi/qmp/types.h" > > #include "qapi/qmp/qerror.h" > > +#include "qemu/cutils.h" > > > > #define QIV_STACK_SIZE 1024 > > > > @@ -45,6 +46,7 @@ struct QmpInputVisitor > > > > /* True to reject parse in visit_end_struct() if unvisited keys remain. */ > > bool strict; > > + bool autocast; > > }; > > > > static QmpInputVisitor *to_qiv(Visitor *v) > > @@ -254,15 +256,25 @@ static void qmp_input_type_int64(Visitor *v, const char *name, int64_t *obj, > > Error **errp) > > { > > QmpInputVisitor *qiv = to_qiv(v); > > - QInt *qint = qobject_to_qint(qmp_input_get_object(qiv, name, true)); > > + QObject *qobj = qmp_input_get_object(qiv, name, true); > > + QInt *qint; > > + QString *qstr; > > > > - if (!qint) { > > - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", > > - "integer"); > > + qint = qobject_to_qint(qobj); > > + if (qint) { > > + *obj = qint_get_int(qint); > > return; > > } > > > > - *obj = qint_get_int(qint); > > + qstr = qobject_to_qstring(qobj); > > + if (qstr && qstr->string && qiv->autocast) { > > + if (qemu_strtoll(qstr->string, NULL, 10, obj) == 0) { > > In the commit message, you mentioned intended use: "with a QDict > produced from QemuOpts, where everything is in string format." I figure > you mean opts with empty opts->list->desc[]. For those, we accept any > key and parse all values as strings. > > The idea with such QemuOpts is to *delay* parsing until we know how to > parse. The delay should be transparent to the user. In particular, > values should be parsed the same whether the parsing is delayed or not. > > The canonical QemuOpts value parser is qemu_opt_parse(). It parses > integers with parse_option_number(). That function parses like > stroull(qstr->string, ..., 0). Two differences: > > * stroll() vs. strtoull(): they differ in ERANGE behavior. This might > be tolerable, but I haven't thought it through. > > * Base 0 vs 10: different syntax and semantics. Oops. Sigh, I originally had my code using strtoull() directly as the parse_option_number() method does, but had to change it to make checkpatch.pl stfu thus creating incosistency. This is a great example of why I hate the fact that we only validate our style rules against new patches and not our existing codebase :-( Anyway, it seems to be something where we should refactor code to use the same parsing method from both places. eg by making the parse_option_number method non-static and just calling it from here. Regards, Daniel
On Wed, Jun 08, 2016 at 02:01:23PM +0200, Paolo Bonzini wrote: > > > On 02/06/2016 18:46, Daniel P. Berrange wrote: > > Currently the QmpInputVisitor assumes that all scalar > > values are directly represented as their final types. > > ie it assumes an 'int' is using QInt, and a 'bool' is > > using QBool. > > > > This extends it so that QString is optionally permitted > > for any of the non-string scalar types. This behaviour > > is turned on by requesting the 'autocast' flag in the > > constructor. > > > > This makes it possible to use QmpInputVisitor with a > > QDict produced from QemuOpts, where everything is in > > string format. > > Perhaps this should instead be a separate QmpStringInputVisitor visitor > that _only_ accepts strings? You can reuse most of the QmpInputVisitor > by putting it in the same file, because the struct and list visitors are > compatible. Yes, that actually works out quite nicely indeed, and in fact showed up a bug in my unit tests too :-) Regards, Daniel
"Daniel P. Berrange" <berrange@redhat.com> writes: > On Thu, Jun 09, 2016 at 04:03:50PM +0200, Markus Armbruster wrote: >> "Daniel P. Berrange" <berrange@redhat.com> writes: >> >> > Currently the QmpInputVisitor assumes that all scalar >> > values are directly represented as their final types. >> > ie it assumes an 'int' is using QInt, and a 'bool' is >> > using QBool. >> > >> > This extends it so that QString is optionally permitted >> > for any of the non-string scalar types. This behaviour >> > is turned on by requesting the 'autocast' flag in the >> > constructor. >> > >> > This makes it possible to use QmpInputVisitor with a >> > QDict produced from QemuOpts, where everything is in >> > string format. >> >> We're still digging. >> >> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> >> > --- >> > docs/qapi-code-gen.txt | 2 +- >> > include/qapi/qmp-input-visitor.h | 6 +- >> > qapi/opts-visitor.c | 1 + >> > qapi/qmp-input-visitor.c | 89 ++++++++++++++++++++++------ >> > qmp.c | 2 +- >> > qom/qom-qobject.c | 2 +- >> > replay/replay-input.c | 2 +- >> > scripts/qapi-commands.py | 2 +- >> > tests/check-qnull.c | 2 +- >> > tests/test-qmp-commands.c | 2 +- >> > tests/test-qmp-input-strict.c | 2 +- >> > tests/test-qmp-input-visitor.c | 115 ++++++++++++++++++++++++++++++++++++- >> > tests/test-visitor-serialization.c | 2 +- >> > util/qemu-sockets.c | 2 +- >> > 14 files changed, 201 insertions(+), 30 deletions(-) > > >> > diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c >> > index 4cf1cf8..00e4b1a 100644 >> > --- a/qapi/opts-visitor.c >> > +++ b/qapi/opts-visitor.c >> > @@ -347,6 +347,7 @@ opts_type_bool(Visitor *v, const char *name, bool *obj, Error **errp) >> > } >> > >> > if (opt->str) { >> > + /* Keep these values in sync with same code in qmp-input-visitor.c */ >> >> Also with parse_option_bool(). That's the canonical place, in fact. > > ....except parse_option_bool() doesn't allow "yes", "no", "y", "n" as valid > values - it only permits "on" and "off" :-( I guess I could make the > parse_option_bool() method non-static, make it accept these missing values, > and then call it from all places so we have guaranteed consistency. Or factor out its parsing guts and put them into cutils.c. Similar to how qemu_strtol() & friends are (or should be) the common number parsers. But that's optional. All I'm asking for this patch is an updated comment. >> >> > if (strcmp(opt->str, "on") == 0 || >> > strcmp(opt->str, "yes") == 0 || >> > strcmp(opt->str, "y") == 0) { >> > diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c >> > index aea90a1..92023b1 100644 >> > --- a/qapi/qmp-input-visitor.c >> > +++ b/qapi/qmp-input-visitor.c >> > @@ -20,6 +20,7 @@ >> > #include "qemu-common.h" >> > #include "qapi/qmp/types.h" >> > #include "qapi/qmp/qerror.h" >> > +#include "qemu/cutils.h" >> > >> > #define QIV_STACK_SIZE 1024 >> > >> > @@ -45,6 +46,7 @@ struct QmpInputVisitor >> > >> > /* True to reject parse in visit_end_struct() if unvisited keys remain. */ >> > bool strict; >> > + bool autocast; >> > }; >> > >> > static QmpInputVisitor *to_qiv(Visitor *v) >> > @@ -254,15 +256,25 @@ static void qmp_input_type_int64(Visitor *v, const char *name, int64_t *obj, >> > Error **errp) >> > { >> > QmpInputVisitor *qiv = to_qiv(v); >> > - QInt *qint = qobject_to_qint(qmp_input_get_object(qiv, name, true)); >> > + QObject *qobj = qmp_input_get_object(qiv, name, true); >> > + QInt *qint; >> > + QString *qstr; >> > >> > - if (!qint) { >> > - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", >> > - "integer"); >> > + qint = qobject_to_qint(qobj); >> > + if (qint) { >> > + *obj = qint_get_int(qint); >> > return; >> > } >> > >> > - *obj = qint_get_int(qint); >> > + qstr = qobject_to_qstring(qobj); >> > + if (qstr && qstr->string && qiv->autocast) { >> > + if (qemu_strtoll(qstr->string, NULL, 10, obj) == 0) { >> >> In the commit message, you mentioned intended use: "with a QDict >> produced from QemuOpts, where everything is in string format." I figure >> you mean opts with empty opts->list->desc[]. For those, we accept any >> key and parse all values as strings. >> >> The idea with such QemuOpts is to *delay* parsing until we know how to >> parse. The delay should be transparent to the user. In particular, >> values should be parsed the same whether the parsing is delayed or not. >> >> The canonical QemuOpts value parser is qemu_opt_parse(). It parses >> integers with parse_option_number(). That function parses like >> stroull(qstr->string, ..., 0). Two differences: >> >> * stroll() vs. strtoull(): they differ in ERANGE behavior. This might >> be tolerable, but I haven't thought it through. >> >> * Base 0 vs 10: different syntax and semantics. Oops. > > Sigh, I originally had my code using strtoull() directly as the > parse_option_number() method does, but had to change it to make > checkpatch.pl stfu thus creating incosistency. This is a great > example of why I hate the fact that we only validate our style > rules against new patches and not our existing codebase :-( > > Anyway, it seems to be something where we should refactor code to > use the same parsing method from both places. eg by making the > parse_option_number method non-static and just calling it from > here. To get this patch accepted, you need to fix the inconsistency. Refactoring to improve our chances the inconsistency stays fixed is welcome, but optional.
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt index d7d6987..e21773e 100644 --- a/docs/qapi-code-gen.txt +++ b/docs/qapi-code-gen.txt @@ -1008,7 +1008,7 @@ Example: { Error *err = NULL; UserDefOne *retval; - QmpInputVisitor *qiv = qmp_input_visitor_new(QOBJECT(args), true); + QmpInputVisitor *qiv = qmp_input_visitor_new(QOBJECT(args), true, false); QapiDeallocVisitor *qdv; Visitor *v; UserDefOneList *arg1 = NULL; diff --git a/include/qapi/qmp-input-visitor.h b/include/qapi/qmp-input-visitor.h index b0624d8..d35a053 100644 --- a/include/qapi/qmp-input-visitor.h +++ b/include/qapi/qmp-input-visitor.h @@ -24,8 +24,12 @@ typedef struct QmpInputVisitor QmpInputVisitor; * * Set @strict to reject a parse that doesn't consume all keys of a * dictionary; otherwise excess input is ignored. + * Set @autocast to automatically convert string values into more + * specific types (numbers, bools, etc) */ -QmpInputVisitor *qmp_input_visitor_new(QObject *obj, bool strict); +QmpInputVisitor *qmp_input_visitor_new(QObject *obj, + bool strict, + bool autocast); void qmp_input_visitor_cleanup(QmpInputVisitor *v); diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c index 4cf1cf8..00e4b1a 100644 --- a/qapi/opts-visitor.c +++ b/qapi/opts-visitor.c @@ -347,6 +347,7 @@ opts_type_bool(Visitor *v, const char *name, bool *obj, Error **errp) } if (opt->str) { + /* Keep these values in sync with same code in qmp-input-visitor.c */ if (strcmp(opt->str, "on") == 0 || strcmp(opt->str, "yes") == 0 || strcmp(opt->str, "y") == 0) { diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c index aea90a1..92023b1 100644 --- a/qapi/qmp-input-visitor.c +++ b/qapi/qmp-input-visitor.c @@ -20,6 +20,7 @@ #include "qemu-common.h" #include "qapi/qmp/types.h" #include "qapi/qmp/qerror.h" +#include "qemu/cutils.h" #define QIV_STACK_SIZE 1024 @@ -45,6 +46,7 @@ struct QmpInputVisitor /* True to reject parse in visit_end_struct() if unvisited keys remain. */ bool strict; + bool autocast; }; static QmpInputVisitor *to_qiv(Visitor *v) @@ -254,15 +256,25 @@ static void qmp_input_type_int64(Visitor *v, const char *name, int64_t *obj, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); - QInt *qint = qobject_to_qint(qmp_input_get_object(qiv, name, true)); + QObject *qobj = qmp_input_get_object(qiv, name, true); + QInt *qint; + QString *qstr; - if (!qint) { - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", - "integer"); + qint = qobject_to_qint(qobj); + if (qint) { + *obj = qint_get_int(qint); return; } - *obj = qint_get_int(qint); + qstr = qobject_to_qstring(qobj); + if (qstr && qstr->string && qiv->autocast) { + if (qemu_strtoll(qstr->string, NULL, 10, obj) == 0) { + return; + } + } + + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", + "integer"); } static void qmp_input_type_uint64(Visitor *v, const char *name, uint64_t *obj, @@ -270,30 +282,61 @@ static void qmp_input_type_uint64(Visitor *v, const char *name, uint64_t *obj, { /* FIXME: qobject_to_qint mishandles values over INT64_MAX */ QmpInputVisitor *qiv = to_qiv(v); - QInt *qint = qobject_to_qint(qmp_input_get_object(qiv, name, true)); + QObject *qobj = qmp_input_get_object(qiv, name, true); + QInt *qint; + QString *qstr; - if (!qint) { - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", - "integer"); + qint = qobject_to_qint(qobj); + if (qint) { + *obj = qint_get_int(qint); return; } - *obj = qint_get_int(qint); + qstr = qobject_to_qstring(qobj); + if (qstr && qstr->string && qiv->autocast) { + if (qemu_strtoull(qstr->string, NULL, 10, obj) == 0) { + return; + } + } + + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", + "integer"); } static void qmp_input_type_bool(Visitor *v, const char *name, bool *obj, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); - QBool *qbool = qobject_to_qbool(qmp_input_get_object(qiv, name, true)); + QObject *qobj = qmp_input_get_object(qiv, name, true); + QBool *qbool; + QString *qstr; - if (!qbool) { - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", - "boolean"); + qbool = qobject_to_qbool(qobj); + if (qbool) { + *obj = qbool_get_bool(qbool); return; } - *obj = qbool_get_bool(qbool); + + qstr = qobject_to_qstring(qobj); + if (qstr && qstr->string && qiv->autocast) { + /* Keep these values in sync with same code in opts-visitor.c */ + if (!strcasecmp(qstr->string, "on") || + !strcasecmp(qstr->string, "yes") || + !strcasecmp(qstr->string, "true")) { + *obj = true; + return; + } + if (!strcasecmp(qstr->string, "off") || + !strcasecmp(qstr->string, "no") || + !strcasecmp(qstr->string, "false")) { + *obj = false; + return; + } + } + + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", + "boolean"); } static void qmp_input_type_str(Visitor *v, const char *name, char **obj, @@ -319,6 +362,8 @@ static void qmp_input_type_number(Visitor *v, const char *name, double *obj, QObject *qobj = qmp_input_get_object(qiv, name, true); QInt *qint; QFloat *qfloat; + QString *qstr; + char *endp; qint = qobject_to_qint(qobj); if (qint) { @@ -332,6 +377,15 @@ static void qmp_input_type_number(Visitor *v, const char *name, double *obj, return; } + qstr = qobject_to_qstring(qobj); + if (qstr && qstr->string && qiv->autocast) { + errno = 0; + *obj = strtod(qstr->string, &endp); + if (errno == 0 && endp != qstr->string && *endp == '\0') { + return; + } + } + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "number"); } @@ -381,7 +435,9 @@ void qmp_input_visitor_cleanup(QmpInputVisitor *v) g_free(v); } -QmpInputVisitor *qmp_input_visitor_new(QObject *obj, bool strict) +QmpInputVisitor *qmp_input_visitor_new(QObject *obj, + bool strict, + bool autocast) { QmpInputVisitor *v; @@ -404,6 +460,7 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj, bool strict) v->visitor.type_null = qmp_input_type_null; v->visitor.optional = qmp_input_optional; v->strict = strict; + v->autocast = autocast; v->root = obj; qobject_incref(obj); diff --git a/qmp.c b/qmp.c index 3165f87..dcd0953 100644 --- a/qmp.c +++ b/qmp.c @@ -665,7 +665,7 @@ void qmp_object_add(const char *type, const char *id, } } - qiv = qmp_input_visitor_new(props, true); + qiv = qmp_input_visitor_new(props, true, false); obj = user_creatable_add_type(type, id, pdict, qmp_input_get_visitor(qiv), errp); qmp_input_visitor_cleanup(qiv); diff --git a/qom/qom-qobject.c b/qom/qom-qobject.c index b66088d..99666ce 100644 --- a/qom/qom-qobject.c +++ b/qom/qom-qobject.c @@ -23,7 +23,7 @@ void object_property_set_qobject(Object *obj, QObject *value, { QmpInputVisitor *qiv; /* TODO: Should we reject, rather than ignore, excess input? */ - qiv = qmp_input_visitor_new(value, false); + qiv = qmp_input_visitor_new(value, false, false); object_property_set(obj, qmp_input_get_visitor(qiv), name, errp); qmp_input_visitor_cleanup(qiv); diff --git a/replay/replay-input.c b/replay/replay-input.c index 03e99d5..de82a59 100644 --- a/replay/replay-input.c +++ b/replay/replay-input.c @@ -37,7 +37,7 @@ static InputEvent *qapi_clone_InputEvent(InputEvent *src) return NULL; } - qiv = qmp_input_visitor_new(obj, true); + qiv = qmp_input_visitor_new(obj, true, false); iv = qmp_input_get_visitor(qiv); visit_type_InputEvent(iv, NULL, &dst, &error_abort); qmp_input_visitor_cleanup(qiv); diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index 8c6acb3..e48995d 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -115,7 +115,7 @@ def gen_marshal(name, arg_type, ret_type): if arg_type and arg_type.members: ret += mcgen(''' - QmpInputVisitor *qiv = qmp_input_visitor_new(QOBJECT(args), true); + QmpInputVisitor *qiv = qmp_input_visitor_new(QOBJECT(args), true, false); QapiDeallocVisitor *qdv; Visitor *v; %(c_name)s arg = {0}; diff --git a/tests/check-qnull.c b/tests/check-qnull.c index fd9c68f..4c11755 100644 --- a/tests/check-qnull.c +++ b/tests/check-qnull.c @@ -49,7 +49,7 @@ static void qnull_visit_test(void) g_assert(qnull_.refcnt == 1); obj = qnull(); - qiv = qmp_input_visitor_new(obj, true); + qiv = qmp_input_visitor_new(obj, true, false); qobject_decref(obj); visit_type_null(qmp_input_get_visitor(qiv), NULL, &error_abort); qmp_input_visitor_cleanup(qiv); diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c index 5c3edd7..c86b282 100644 --- a/tests/test-qmp-commands.c +++ b/tests/test-qmp-commands.c @@ -222,7 +222,7 @@ static void test_dealloc_partial(void) ud2_dict = qdict_new(); qdict_put_obj(ud2_dict, "string0", QOBJECT(qstring_from_str(text))); - qiv = qmp_input_visitor_new(QOBJECT(ud2_dict), true); + qiv = qmp_input_visitor_new(QOBJECT(ud2_dict), true, false); visit_type_UserDefTwo(qmp_input_get_visitor(qiv), NULL, &ud2, &err); qmp_input_visitor_cleanup(qiv); QDECREF(ud2_dict); diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c index 4602529..f7f1f00 100644 --- a/tests/test-qmp-input-strict.c +++ b/tests/test-qmp-input-strict.c @@ -55,7 +55,7 @@ static Visitor *validate_test_init_internal(TestInputVisitorData *data, data->obj = qobject_from_jsonv(json_string, ap); g_assert(data->obj); - data->qiv = qmp_input_visitor_new(data->obj, true); + data->qiv = qmp_input_visitor_new(data->obj, true, false); g_assert(data->qiv); v = qmp_input_get_visitor(data->qiv); diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c index cee07ce..5691dc3 100644 --- a/tests/test-qmp-input-visitor.c +++ b/tests/test-qmp-input-visitor.c @@ -41,6 +41,7 @@ static void visitor_input_teardown(TestInputVisitorData *data, function so that the JSON string used by the tests are kept in the test functions (and not in main()). */ static Visitor *visitor_input_test_init_internal(TestInputVisitorData *data, + bool strict, bool autocast, const char *json_string, va_list *ap) { @@ -51,7 +52,7 @@ static Visitor *visitor_input_test_init_internal(TestInputVisitorData *data, data->obj = qobject_from_jsonv(json_string, ap); g_assert(data->obj); - data->qiv = qmp_input_visitor_new(data->obj, false); + data->qiv = qmp_input_visitor_new(data->obj, strict, autocast); g_assert(data->qiv); v = qmp_input_get_visitor(data->qiv); @@ -60,6 +61,21 @@ static Visitor *visitor_input_test_init_internal(TestInputVisitorData *data, return v; } +static GCC_FMT_ATTR(4, 5) +Visitor *visitor_input_test_init_full(TestInputVisitorData *data, + bool strict, bool autocast, + const char *json_string, ...) +{ + Visitor *v; + va_list ap; + + va_start(ap, json_string); + v = visitor_input_test_init_internal(data, strict, autocast, + json_string, &ap); + va_end(ap); + return v; +} + static GCC_FMT_ATTR(2, 3) Visitor *visitor_input_test_init(TestInputVisitorData *data, const char *json_string, ...) @@ -68,7 +84,8 @@ Visitor *visitor_input_test_init(TestInputVisitorData *data, va_list ap; va_start(ap, json_string); - v = visitor_input_test_init_internal(data, json_string, &ap); + v = visitor_input_test_init_internal(data, false, false, + json_string, &ap); va_end(ap); return v; } @@ -83,7 +100,8 @@ Visitor *visitor_input_test_init(TestInputVisitorData *data, static Visitor *visitor_input_test_init_raw(TestInputVisitorData *data, const char *json_string) { - return visitor_input_test_init_internal(data, json_string, NULL); + return visitor_input_test_init_internal(data, false, false, + json_string, NULL); } static void test_visitor_in_int(TestInputVisitorData *data, @@ -115,6 +133,33 @@ static void test_visitor_in_int_overflow(TestInputVisitorData *data, error_free_or_abort(&err); } +static void test_visitor_in_int_autocast(TestInputVisitorData *data, + const void *unused) +{ + int64_t res = 0, value = -42; + Visitor *v; + + v = visitor_input_test_init_full(data, false, true, + "\"-42\""); + + visit_type_int(v, NULL, &res, &error_abort); + g_assert_cmpint(res, ==, value); +} + +static void test_visitor_in_int_noautocast(TestInputVisitorData *data, + const void *unused) +{ + int64_t res = 0; + Visitor *v; + Error *err = NULL; + + v = visitor_input_test_init(data, "\"-42\""); + + visit_type_int(v, NULL, &res, &err); + g_assert(err != NULL); + error_free(err); +} + static void test_visitor_in_bool(TestInputVisitorData *data, const void *unused) { @@ -127,6 +172,32 @@ static void test_visitor_in_bool(TestInputVisitorData *data, g_assert_cmpint(res, ==, true); } +static void test_visitor_in_bool_autocast(TestInputVisitorData *data, + const void *unused) +{ + bool res = false; + Visitor *v; + + v = visitor_input_test_init_full(data, false, true, "\"true\""); + + visit_type_bool(v, NULL, &res, &error_abort); + g_assert_cmpint(res, ==, true); +} + +static void test_visitor_in_bool_noautocast(TestInputVisitorData *data, + const void *unused) +{ + bool res = false; + Visitor *v; + Error *err = NULL; + + v = visitor_input_test_init(data, "\"true\""); + + visit_type_bool(v, NULL, &res, &err); + g_assert(err != NULL); + error_free(err); +} + static void test_visitor_in_number(TestInputVisitorData *data, const void *unused) { @@ -139,6 +210,32 @@ static void test_visitor_in_number(TestInputVisitorData *data, g_assert_cmpfloat(res, ==, value); } +static void test_visitor_in_number_autocast(TestInputVisitorData *data, + const void *unused) +{ + double res = 0, value = 3.14; + Visitor *v; + + v = visitor_input_test_init_full(data, false, true, "\"3.14\""); + + visit_type_number(v, NULL, &res, &error_abort); + g_assert_cmpfloat(res, ==, value); +} + +static void test_visitor_in_number_noautocast(TestInputVisitorData *data, + const void *unused) +{ + double res = 0; + Visitor *v; + Error *err = NULL; + + v = visitor_input_test_init(data, "\"3.14\""); + + visit_type_number(v, NULL, &res, &err); + g_assert(err != NULL); + error_free(err); +} + static void test_visitor_in_string(TestInputVisitorData *data, const void *unused) { @@ -835,10 +932,22 @@ int main(int argc, char **argv) &in_visitor_data, test_visitor_in_int); input_visitor_test_add("/visitor/input/int_overflow", &in_visitor_data, test_visitor_in_int_overflow); + input_visitor_test_add("/visitor/input/int_autocast", + &in_visitor_data, test_visitor_in_int_autocast); + input_visitor_test_add("/visitor/input/int_noautocast", + &in_visitor_data, test_visitor_in_int_noautocast); input_visitor_test_add("/visitor/input/bool", &in_visitor_data, test_visitor_in_bool); + input_visitor_test_add("/visitor/input/bool_autocast", + &in_visitor_data, test_visitor_in_bool_autocast); + input_visitor_test_add("/visitor/input/bool_noautocast", + &in_visitor_data, test_visitor_in_bool_noautocast); input_visitor_test_add("/visitor/input/number", &in_visitor_data, test_visitor_in_number); + input_visitor_test_add("/visitor/input/number_autocast", + &in_visitor_data, test_visitor_in_number_autocast); + input_visitor_test_add("/visitor/input/number_noautocast", + &in_visitor_data, test_visitor_in_number_noautocast); input_visitor_test_add("/visitor/input/string", &in_visitor_data, test_visitor_in_string); input_visitor_test_add("/visitor/input/enum", diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c index 7b14b5a..db618d8 100644 --- a/tests/test-visitor-serialization.c +++ b/tests/test-visitor-serialization.c @@ -1038,7 +1038,7 @@ static void qmp_deserialize(void **native_out, void *datap, obj = qobject_from_json(qstring_get_str(output_json)); QDECREF(output_json); - d->qiv = qmp_input_visitor_new(obj, true); + d->qiv = qmp_input_visitor_new(obj, true, false); qobject_decref(obj_orig); qobject_decref(obj); visit(qmp_input_get_visitor(d->qiv), native_out, errp); diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c index 0d6cd1f..51c6a8e 100644 --- a/util/qemu-sockets.c +++ b/util/qemu-sockets.c @@ -1145,7 +1145,7 @@ void qapi_copy_SocketAddress(SocketAddress **p_dest, return; } - qiv = qmp_input_visitor_new(obj, true); + qiv = qmp_input_visitor_new(obj, true, false); iv = qmp_input_get_visitor(qiv); visit_type_SocketAddress(iv, NULL, p_dest, &error_abort); qmp_input_visitor_cleanup(qiv);
Currently the QmpInputVisitor assumes that all scalar values are directly represented as their final types. ie it assumes an 'int' is using QInt, and a 'bool' is using QBool. This extends it so that QString is optionally permitted for any of the non-string scalar types. This behaviour is turned on by requesting the 'autocast' flag in the constructor. This makes it possible to use QmpInputVisitor with a QDict produced from QemuOpts, where everything is in string format. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- docs/qapi-code-gen.txt | 2 +- include/qapi/qmp-input-visitor.h | 6 +- qapi/opts-visitor.c | 1 + qapi/qmp-input-visitor.c | 89 ++++++++++++++++++++++------ qmp.c | 2 +- qom/qom-qobject.c | 2 +- replay/replay-input.c | 2 +- scripts/qapi-commands.py | 2 +- tests/check-qnull.c | 2 +- tests/test-qmp-commands.c | 2 +- tests/test-qmp-input-strict.c | 2 +- tests/test-qmp-input-visitor.c | 115 ++++++++++++++++++++++++++++++++++++- tests/test-visitor-serialization.c | 2 +- util/qemu-sockets.c | 2 +- 14 files changed, 201 insertions(+), 30 deletions(-)