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 adds an alternative mode where a QString can also be parsed in place of the native type, by adding a parameter and updating all callers. For now, only the testsuite sets the new autocast parameter. This makes it possible to use QmpInputVisitor with a QDict produced from QemuOpts, where everything is in string format. It also makes it possible for the next patch to support back-compat in legacy commands like 'netdev_add' that no longer use QemuOpts, but must parse the same set of QMP constructs that QemuOpts would historically allow. Based heavily on a patch by Daniel P. Berrange <berra...@redhat.com>: Message-Id: <1467724312-9378-4-git-send-email-berra...@redhat.com> Signed-off-by: Eric Blake <ebl...@redhat.com> --- v9: new patch (but heavily draws from 3/7 in Dan's v7 series, Provide a QOM-based authorization API) --- scripts/qapi-commands.py | 2 +- include/qapi/qmp-input-visitor.h | 24 ++++-- qapi/qmp-input-visitor.c | 100 ++++++++++++++++++++++--- qmp.c | 2 +- qom/qom-qobject.c | 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 | 150 ++++++++++++++++++++++++++++++++++++- tests/test-visitor-serialization.c | 2 +- docs/qapi-code-gen.txt | 2 +- 11 files changed, 264 insertions(+), 26 deletions(-) diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index a06a2c4..f526c13 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -117,7 +117,7 @@ def gen_marshal(name, arg_type, boxed, ret_type): Visitor *v; %(c_name)s arg = {0}; - v = qmp_input_visitor_new(QOBJECT(args), true); + v = qmp_input_visitor_new(QOBJECT(args), true, false); visit_start_struct(v, NULL, NULL, 0, &err); if (err) { goto out; diff --git a/include/qapi/qmp-input-visitor.h b/include/qapi/qmp-input-visitor.h index f3ff5f3..275a167 100644 --- a/include/qapi/qmp-input-visitor.h +++ b/include/qapi/qmp-input-visitor.h @@ -19,12 +19,26 @@ typedef struct QmpInputVisitor QmpInputVisitor; -/* - * Return a new input visitor that converts QMP to QAPI. +/** + * qmp_input_visitor_new: + * @obj: the input object to visit + * @strict: whether to require that all input keys are consumed + * @autocast: whether to allow strings in place of other scalars * - * Set @strict to reject a parse that doesn't consume all keys of a - * dictionary; otherwise excess input is ignored. + * Create a new input visitor that converts QMP to QAPI. + * + * If @strict is set to true, then an error will be reported if any + * dict keys are not consumed during visitation. + * + * When @autocast is false, any scalar values in the @obj input data + * structure should be in the required type already. ie if visiting a + * bool, the value should already be a QBool instance. Otherwise, a + * string value that can be parsed as the scalar is acceptable; + * however, it is not possible to parse QAPI alternates in autocast + * mode. + * + * Returns: a new input visitor */ -Visitor *qmp_input_visitor_new(QObject *obj, bool strict); +Visitor *qmp_input_visitor_new(QObject *obj, bool strict, bool autocast); #endif diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c index 64dd392..7a1b93a 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 @@ -47,6 +48,9 @@ struct QmpInputVisitor /* True to reject parse in visit_end_struct() if unvisited keys remain. */ bool strict; + + /* True to allow strings in place of native scalars. */ + bool autocast; }; static QmpInputVisitor *to_qiv(Visitor *v) @@ -234,6 +238,8 @@ static void qmp_input_start_alternate(Visitor *v, const char *name, QmpInputVisitor *qiv = to_qiv(v); QObject *qobj = qmp_input_get_object(qiv, name, false); + assert(!qiv->autocast); + if (!qobj) { *obj = NULL; error_setg(errp, QERR_MISSING_PARAMETER, name ? name : "null"); @@ -250,11 +256,21 @@ 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 = qobject_to_qint(qobj); if (!qint) { - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", - "integer"); + QString *qstr = qobject_to_qstring(qobj); + + if (qiv->autocast && qstr) { + uint64_t ret = *obj; + + parse_option_number(name, qstr->string, &ret, errp); + *obj = ret; + } else { + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", + "integer"); + } return; } @@ -266,11 +282,18 @@ 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 = qobject_to_qint(qobj); if (!qint) { - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", - "integer"); + QString *qstr = qobject_to_qstring(qobj); + + if (qiv->autocast && qstr) { + parse_option_number(name, qstr->string, obj, errp); + } else { + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", + "integer"); + } return; } @@ -281,11 +304,18 @@ 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 = qobject_to_qbool(qobj); if (!qbool) { - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", - "boolean"); + QString *qstr = qobject_to_qstring(qobj); + + if (qiv->autocast && qstr) { + parse_option_bool(name, qstr->string, obj, errp); + } else { + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", + "boolean"); + } return; } @@ -315,6 +345,7 @@ 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; qint = qobject_to_qint(qobj); if (qint) { @@ -328,6 +359,19 @@ static void qmp_input_type_number(Visitor *v, const char *name, double *obj, return; } + qstr = qobject_to_qstring(qobj); + if (qiv->autocast && qstr) { + char *endp; + double value; + + errno = 0; + value = strtod(qstr->string, &endp); + if (errno == 0 && endp != qstr->string && *endp == '\0') { + *obj = value; + return; + } + } + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "number"); } @@ -353,6 +397,40 @@ static void qmp_input_type_null(Visitor *v, const char *name, Error **errp) } } +static void qmp_input_type_size(Visitor *v, const char *name, uint64_t *obj, + Error **errp) +{ + /* FIXME: qobject_to_qint mishandles values over INT64_MAX */ + QmpInputVisitor *qiv = to_qiv(v); + QObject *qobj = qmp_input_get_object(qiv, name, true); + QInt *qint = qobject_to_qint(qobj); + + if (!qint) { + QString *qstr = qobject_to_qstring(qobj); + + if (qiv->autocast && qstr) { + int64_t val; + char *endptr; + + val = qemu_strtosz_suffix(qstr->string, &endptr, + QEMU_STRTOSZ_DEFSUFFIX_B); + if (val < 0 || *endptr) { + error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name, + "a size value representible as a non-negative" + " int64"); + } else { + *obj = val; + } + } else { + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", + "integer"); + } + return; + } + + *obj = qint_get_int(qint); +} + static void qmp_input_optional(Visitor *v, const char *name, bool *present) { QmpInputVisitor *qiv = to_qiv(v); @@ -380,7 +458,7 @@ static void qmp_input_free(Visitor *v) g_free(qiv); } -Visitor *qmp_input_visitor_new(QObject *obj, bool strict) +Visitor *qmp_input_visitor_new(QObject *obj, bool strict, bool autocast) { QmpInputVisitor *v; @@ -401,9 +479,11 @@ Visitor *qmp_input_visitor_new(QObject *obj, bool strict) v->visitor.type_number = qmp_input_type_number; v->visitor.type_any = qmp_input_type_any; v->visitor.type_null = qmp_input_type_null; + v->visitor.type_size = qmp_input_type_size; v->visitor.optional = qmp_input_optional; v->visitor.free = qmp_input_free; v->strict = strict; + v->autocast = autocast; v->root = obj; qobject_incref(obj); diff --git a/qmp.c b/qmp.c index b6d531e..ff93890 100644 --- a/qmp.c +++ b/qmp.c @@ -666,7 +666,7 @@ void qmp_object_add(const char *type, const char *id, } } - v = qmp_input_visitor_new(props, true); + v = qmp_input_visitor_new(props, true, false); obj = user_creatable_add_type(type, id, pdict, v, errp); visit_free(v); if (obj) { diff --git a/qom/qom-qobject.c b/qom/qom-qobject.c index c225abc..974c7ea 100644 --- a/qom/qom-qobject.c +++ b/qom/qom-qobject.c @@ -23,7 +23,7 @@ void object_property_set_qobject(Object *obj, QObject *value, { Visitor *v; /* TODO: Should we reject, rather than ignore, excess input? */ - v = qmp_input_visitor_new(value, false); + v = qmp_input_visitor_new(value, false, false); object_property_set(obj, v, name, errp); visit_free(v); } diff --git a/tests/check-qnull.c b/tests/check-qnull.c index dc906b1..483ed6d 100644 --- a/tests/check-qnull.c +++ b/tests/check-qnull.c @@ -47,7 +47,7 @@ static void qnull_visit_test(void) g_assert(qnull_.refcnt == 1); obj = qnull(); - v = qmp_input_visitor_new(obj, true); + v = qmp_input_visitor_new(obj, true, false); qobject_decref(obj); visit_type_null(v, NULL, &error_abort); visit_free(v); diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c index 5af1a46..974841c 100644 --- a/tests/test-qmp-commands.c +++ b/tests/test-qmp-commands.c @@ -229,7 +229,7 @@ static void test_dealloc_partial(void) ud2_dict = qdict_new(); qdict_put_obj(ud2_dict, "string0", QOBJECT(qstring_from_str(text))); - v = qmp_input_visitor_new(QOBJECT(ud2_dict), true); + v = qmp_input_visitor_new(QOBJECT(ud2_dict), true, false); visit_type_UserDefTwo(v, NULL, &ud2, &err); visit_free(v); QDECREF(ud2_dict); diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c index 814550a..93ee137 100644 --- a/tests/test-qmp-input-strict.c +++ b/tests/test-qmp-input-strict.c @@ -53,7 +53,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); return data->qiv; } diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c index f583dce..29c4562 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) { @@ -49,11 +50,26 @@ 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); return data->qiv; } +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, ...) @@ -62,7 +78,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; } @@ -77,7 +94,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, @@ -109,6 +127,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) { @@ -121,6 +166,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, "\"yes\""); + + 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) { @@ -133,6 +204,63 @@ 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_size_autocast(TestInputVisitorData *data, + const void *unused) +{ + uint64_t res, value = 500 * 1024 * 1024; + Visitor *v; + + v = visitor_input_test_init_full(data, false, true, "\"500M\""); + + visit_type_size(v, NULL, &res, &error_abort); + g_assert_cmpfloat(res, ==, value); + + v = visitor_input_test_init_full(data, false, true, "524288000"); + + visit_type_size(v, NULL, &res, &error_abort); + g_assert_cmpfloat(res, ==, value); +} + +static void test_visitor_in_size_noautocast(TestInputVisitorData *data, + const void *unused) +{ + uint64_t res = 0; + Visitor *v; + Error *err = NULL; + + v = visitor_input_test_init(data, "\"500M\""); + + visit_type_size(v, NULL, &res, &err); + g_assert(err != NULL); + error_free(err); +} + static void test_visitor_in_string(TestInputVisitorData *data, const void *unused) { @@ -841,10 +969,26 @@ 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/size_autocast", + &in_visitor_data, test_visitor_in_size_autocast); + input_visitor_test_add("/visitor/input/size_noautocast", + &in_visitor_data, test_visitor_in_size_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 dba4670..ddd5837 100644 --- a/tests/test-visitor-serialization.c +++ b/tests/test-visitor-serialization.c @@ -1040,7 +1040,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(d->qiv, native_out, errp); diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt index de298dc..961015c 100644 --- a/docs/qapi-code-gen.txt +++ b/docs/qapi-code-gen.txt @@ -1024,7 +1024,7 @@ Example: Visitor *v; UserDefOneList *arg1 = NULL; - v = qmp_input_visitor_new(QOBJECT(args), true); + v = qmp_input_visitor_new(QOBJECT(args), true, false); visit_start_struct(v, NULL, NULL, 0, &err); if (err) { goto out; -- 2.5.5