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 <berra...@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); >