qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-devel] [PATCH v9 16/17] qapi: Tweak QmpInputVisitor to optionally


From: Eric Blake
Subject: [Qemu-devel] [PATCH v9 16/17] qapi: Tweak QmpInputVisitor to optionally do string conversion
Date: Wed, 13 Jul 2016 21:50:27 -0600

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 <address@hidden>:
Message-Id: <address@hidden>

Signed-off-by: Eric Blake <address@hidden>

---
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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]