[Qemu-devel] [PATCH v5 02/11] qapi: allow QmpInputVisitor to auto-cast types

["Daniel P. Berrange" <berrange@redhat.com>]

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

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);
-- 
2.5.5