All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v13 0/6] QAPI/QOM work for non-scalar object properties
@ 2016-09-19 11:58 Daniel P. Berrange
  2016-09-19 11:58 ` [Qemu-devel] [PATCH v13 1/6] qdict: implement a qdict_crumple method for un-flattening a dict Daniel P. Berrange
                   ` (5 more replies)
  0 siblings, 6 replies; 18+ messages in thread
From: Daniel P. Berrange @ 2016-09-19 11:58 UTC (permalink / raw)
  To: qemu-devel
  Cc: Markus Armbruster, Max Reitz, Marc-André Lureau,
	Paolo Bonzini, Andreas Färber, Kevin Wolf,
	Daniel P. Berrange

This patch series contains only the QAPI/QOM bits of my previous
access control patch series:

 v1: https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg04618.html
 v2: https://lists.gnu.org/archive/html/qemu-devel/2016-03/msg01454.html
 v3: https://lists.gnu.org/archive/html/qemu-devel/2016-03/msg02498.html
 v4: https://lists.gnu.org/archive/html/qemu-devel/2016-05/msg01661.html
 v5: https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg00485.html
 v6: https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg03876.html
 v7: https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg00919.html
 v8: https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg03115.html
 v9: https://lists.gnu.org/archive/html/qemu-devel/2016-08/msg02653.html
 v10: https://lists.gnu.org/archive/html/qemu-devel/2016-08/msg02694.html
 v11: https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg00652.html
 v12: https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg03559.html

This series provides the infrastructure to allow use of non-scalar
properties with the -object CLI arg, and object_add monitor commands.
eg a property which is a list of structs. The syntax used for this is
intentionally compatible with the syntax used by the block layer. This
will allow the qdict_crumple method to be used by the block layer to
convert from QemuOpts into structured QAPI block layer structs at a
future date. It is already used by one of Max's patch series, and
recent patches for glusterfs multiple-host support could have made
use of it to simplify code.

Changed in v13:

 - Fix typos (Kevin)
 - Remove unneeded line breaks (Kevin)

Changed in v12:

 - More user friendly error message for mixing dict/list
   keys (Kevin)
 - Report error instead of assert for non-contiguous list
   keys (Kevin)
 - Fix tests for non-contiguous list keys (Kevin)
 - Add tests for escaping of '.' when crumpling (Kevin)
 - Fix remaining references to Qmp(In|Out)putVisitor (Markus)
 - Misc typos / whitespace fixes (Eric, Kevin)
 - Avoid touching 'ret' when parsing int64 fails (Eric)
 - Testing of more edge cases in QObjectInputVisitor (Eric)
 - Simplify API doc format (Markus)
 - Use parse_option_size instead of qemu_strtosz_suffix
   for consistency (Kevin)
 - Use safer qobject_to_qdict casts (Eric)

Changed in v11:

 - Split QAPI/QOM patches off from the access control patches

Changed in v10:

 - Fixed stupid build mistake

Changed in v9:

 - Rename QmpInputVisitor -> QObjectInputVisitor (Markus/Eric)
 - Rename QmpOutputVisitor -> QObjectOutputVisitor (Markus/Eric)
 - Drop "strict" param from qobject_string_visitor_new() (Marus)
 - Misc docs typos
 - Add a visitor able to use strict or string types (for Eric's
   netdev series)
 - Add a authorization API implementation that uses PAM

Changed in v8:

 - Rebase due to merge of Visitor API changes (Eric)

Changed in v7:

 - Misc typos in API docs (Marc-André)
 - Fix parsing of properties using type_size visitor (Marc-André)
 - Mark based auth class as abstract (Marc-André)
 - Fix QAPI version annotations to say 2.7 (Marc-André)

Changed in v6:

 - Switch from while() to for() loop for iterating over
   dicts (Markus)
 - Avoid redundant strdup (Markus)
 - Rewrap comments at 70 chars (Markus)
 - Change qdict_list_size() to qdict_is_list() (Markus)
 - Misc docs changes (Markus)
 - Change QmpInputVisitor so the code for handling the
   string types is separate from code using native
   scalar types (Paolo)
 - Centralize code parsing bool strings (Markus)
 - Centralize code parsing int strings (Markus)

Changed in v5:

 - Resolved conflicts with Eric's visitor refactoring which
   made it stricter about struct begin/end calls
 - Added support for ACLs to migration code now its TLS
   support is merged.
 - Fixed typos in example in commit message

Changed in v4:
 - Ensure examples use shell escaping for '*' (Eric)
 - Add more tests for crumple impl (Eric)
 - Raise error if sasl-acl/tls-acl are requested but
   sasl/tls auth are not enabled (Eric)
 - Document return codes for auth check more clearly (Eric)
 - Don't silently turn a glob match into a strcmp
 - Other misc small typos/fixes (Eric)

Changed in v3:

 - Created separate qdict_list_size method (Max)
 - Added unit tests for case of empty dict (Max)
 - Fix variable names to use underscore separator (Max)
 - Fix potential free of uninitialized variables (Max)
 - Use QObject APIs for casts, instead of C type casts (Max)

Changed in v2:

 - Adapt to changes in qapi visitor APIs
 - Add a 'bool recursive' flag to qdict_crumple (Max)
 - Fix memory leaks in qdict_crumple (Max)
 - Split out key splitting code from qdict_crumple (Max)
 - Use saner variable names in qdict_crumple (Max)
 - Added some tests for bad inputs to qdict_crumple


Daniel P. Berrange (6):
  qdict: implement a qdict_crumple method for un-flattening a dict
  option: make parse_option_bool/number non-static
  qapi: rename QmpInputVisitor to QObjectInputVisitor
  qapi: rename QmpOutputVisitor to QObjectOutputVisitor
  qapi: add a QObjectInputVisitor that does string conversion
  qom: support arbitrary non-scalar properties with -object

 block/qapi.c                                       |   4 +-
 blockdev.c                                         |   4 +-
 docs/qapi-code-gen.txt                             |   4 +-
 hmp.c                                              |  16 +-
 include/qapi/qmp-input-visitor.h                   |  30 --
 include/qapi/qmp/qdict.h                           |   1 +
 include/qapi/qobject-input-visitor.h               |  54 ++
 ...p-output-visitor.h => qobject-output-visitor.h} |  10 +-
 include/qapi/visitor.h                             |   6 +-
 include/qemu/option.h                              |   4 +
 include/qom/object_interfaces.h                    |  10 +-
 monitor.c                                          |   2 +-
 qapi/Makefile.objs                                 |   4 +-
 qapi/opts-visitor.c                                |  19 +-
 qapi/qapi-clone-visitor.c                          |   2 +-
 qapi/qmp-input-visitor.c                           | 412 ----------------
 qapi/qmp-output-visitor.c                          | 256 ----------
 qapi/qobject-input-visitor.c                       | 543 +++++++++++++++++++++
 qapi/qobject-output-visitor.c                      | 254 ++++++++++
 qemu-img.c                                         |   8 +-
 qmp.c                                              |   6 +-
 qobject/qdict.c                                    | 291 +++++++++++
 qom/object_interfaces.c                            |  47 +-
 qom/qom-qobject.c                                  |   8 +-
 scripts/qapi-commands.py                           |   8 +-
 scripts/qapi-event.py                              |   4 +-
 target-s390x/cpu_models.c                          |   4 +-
 tests/.gitignore                                   |   6 +-
 tests/Makefile.include                             |  20 +-
 tests/check-qdict.c                                | 260 ++++++++++
 tests/check-qnull.c                                |   8 +-
 tests/check-qom-proplist.c                         | 334 ++++++++++++-
 tests/test-qmp-commands.c                          |   4 +-
 ...-input-strict.c => test-qobject-input-strict.c} |   6 +-
 ...nput-visitor.c => test-qobject-input-visitor.c} | 207 +++++++-
 ...put-visitor.c => test-qobject-output-visitor.c} |   6 +-
 tests/test-string-input-visitor.c                  |   2 +-
 tests/test-string-output-visitor.c                 |   2 +-
 tests/test-visitor-serialization.c                 |   8 +-
 util/qemu-option.c                                 |  27 +-
 util/qemu-sockets.c                                |   4 +-
 41 files changed, 2073 insertions(+), 832 deletions(-)
 delete mode 100644 include/qapi/qmp-input-visitor.h
 create mode 100644 include/qapi/qobject-input-visitor.h
 rename include/qapi/{qmp-output-visitor.h => qobject-output-visitor.h} (66%)
 delete mode 100644 qapi/qmp-input-visitor.c
 delete mode 100644 qapi/qmp-output-visitor.c
 create mode 100644 qapi/qobject-input-visitor.c
 create mode 100644 qapi/qobject-output-visitor.c
 rename tests/{test-qmp-input-strict.c => test-qobject-input-strict.c} (98%)
 rename tests/{test-qmp-input-visitor.c => test-qobject-input-visitor.c} (81%)
 rename tests/{test-qmp-output-visitor.c => test-qobject-output-visitor.c} (99%)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH v13 1/6] qdict: implement a qdict_crumple method for un-flattening a dict
  2016-09-19 11:58 [Qemu-devel] [PATCH v13 0/6] QAPI/QOM work for non-scalar object properties Daniel P. Berrange
@ 2016-09-19 11:58 ` Daniel P. Berrange
  2016-09-19 11:58 ` [Qemu-devel] [PATCH v13 2/6] option: make parse_option_bool/number non-static Daniel P. Berrange
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 18+ messages in thread
From: Daniel P. Berrange @ 2016-09-19 11:58 UTC (permalink / raw)
  To: qemu-devel
  Cc: Markus Armbruster, Max Reitz, Marc-André Lureau,
	Paolo Bonzini, Andreas Färber, Kevin Wolf,
	Daniel P. Berrange

The qdict_flatten() method will take a dict whose elements are
further nested dicts/lists and flatten them by concatenating
keys.

The qdict_crumple() method aims to do the reverse, taking a flat
qdict, and turning it into a set of nested dicts/lists. It will
apply nesting based on the key name, with a '.' indicating a
new level in the hierarchy. If the keys in the nested structure
are all numeric, it will create a list, otherwise it will create
a dict.

If the keys are a mixture of numeric and non-numeric, or the
numeric keys are not in strictly ascending order, an error will
be reported.

As an example, a flat dict containing

 {
   'foo.0.bar': 'one',
   'foo.0.wizz': '1',
   'foo.1.bar': 'two',
   'foo.1.wizz': '2'
 }

will get turned into a dict with one element 'foo' whose
value is a list. The list elements will each in turn be
dicts.

 {
   'foo': [
     { 'bar': 'one', 'wizz': '1' },
     { 'bar': 'two', 'wizz': '2' }
   ],
 }

If the key is intended to contain a literal '.', then it must
be escaped as '..'. ie a flat dict

  {
     'foo..bar': 'wizz',
     'bar.foo..bar': 'eek',
     'bar.hello': 'world'
  }

Will end up as

  {
     'foo.bar': 'wizz',
     'bar': {
        'foo.bar': 'eek',
        'hello': 'world'
     }
  }

The intent of this function is that it allows a set of QemuOpts
to be turned into a nested data structure that mirrors the nesting
used when the same object is defined over QMP.

Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 include/qapi/qmp/qdict.h |   1 +
 qobject/qdict.c          | 291 +++++++++++++++++++++++++++++++++++++++++++++++
 tests/check-qdict.c      | 260 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 552 insertions(+)

diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
index 71b8eb0..8a3ac13 100644
--- a/include/qapi/qmp/qdict.h
+++ b/include/qapi/qmp/qdict.h
@@ -73,6 +73,7 @@ void qdict_flatten(QDict *qdict);
 void qdict_extract_subqdict(QDict *src, QDict **dst, const char *start);
 void qdict_array_split(QDict *src, QList **dst);
 int qdict_array_entries(QDict *src, const char *subqdict);
+QObject *qdict_crumple(QDict *src, bool recursive, Error **errp);
 
 void qdict_join(QDict *dest, QDict *src, bool overwrite);
 
diff --git a/qobject/qdict.c b/qobject/qdict.c
index 60f158c..476458a 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -17,6 +17,7 @@
 #include "qapi/qmp/qbool.h"
 #include "qapi/qmp/qstring.h"
 #include "qapi/qmp/qobject.h"
+#include "qapi/error.h"
 #include "qemu/queue.h"
 #include "qemu-common.h"
 #include "qemu/cutils.h"
@@ -683,6 +684,296 @@ void qdict_array_split(QDict *src, QList **dst)
     }
 }
 
+
+/**
+ * qdict_split_flat_key:
+ * @key: the key string to split
+ * @prefix: non-NULL pointer to hold extracted prefix
+ * @suffix: non-NULL pointer to remaining suffix
+ *
+ * Given a flattened key such as 'foo.0.bar', split it into two parts
+ * at the first '.' separator. Allows double dot ('..') to escape the
+ * normal separator.
+ *
+ * eg
+ *    'foo.0.bar' -> prefix='foo' and suffix='0.bar'
+ *    'foo..0.bar' -> prefix='foo.0' and suffix='bar'
+ *
+ * The '..' sequence will be unescaped in the returned 'prefix'
+ * string. The 'suffix' string will be left in escaped format, so it
+ * can be fed back into the qdict_split_flat_key() key as the input
+ * later.
+ *
+ * The caller is responsible for freeing the string returned in @prefix
+ * using g_free().
+ */
+static void qdict_split_flat_key(const char *key, char **prefix,
+                                 const char **suffix)
+{
+    const char *separator;
+    size_t i, j;
+
+    /* Find first '.' separator, but if there is a pair '..'
+     * that acts as an escape, so skip over '..' */
+    separator = NULL;
+    do {
+        if (separator) {
+            separator += 2;
+        } else {
+            separator = key;
+        }
+        separator = strchr(separator, '.');
+    } while (separator && separator[1] == '.');
+
+    if (separator) {
+        *prefix = g_strndup(key, separator - key);
+        *suffix = separator + 1;
+    } else {
+        *prefix = g_strdup(key);
+        *suffix = NULL;
+    }
+
+    /* Unescape the '..' sequence into '.' */
+    for (i = 0, j = 0; (*prefix)[i] != '\0'; i++, j++) {
+        if ((*prefix)[i] == '.') {
+            assert((*prefix)[i + 1] == '.');
+            i++;
+        }
+        (*prefix)[j] = (*prefix)[i];
+    }
+    (*prefix)[j] = '\0';
+}
+
+
+/**
+ * qdict_is_list:
+ * @maybe_list: dict to check if keys represent list elements.
+ *
+ * Determine whether all keys in @maybe_list are valid list elements.
+ * If @maybe_list is non-zero in length and all the keys look like
+ * valid list indexes, this will return 1. If @maybe_list is zero
+ * length or all keys are non-numeric then it will return 0 to indicate
+ * it is a normal qdict. If there is a mix of numeric and non-numeric
+ * keys, or the list indexes are non-contiguous, an error is reported.
+ *
+ * Returns: 1 if a valid list, 0 if a dict, -1 on error
+ */
+static int qdict_is_list(QDict *maybe_list, Error **errp)
+{
+    const QDictEntry *ent;
+    ssize_t len = 0;
+    ssize_t max = -1;
+    int is_list = -1;
+    int64_t val;
+
+    for (ent = qdict_first(maybe_list); ent != NULL;
+         ent = qdict_next(maybe_list, ent)) {
+
+        if (qemu_strtoll(ent->key, NULL, 10, &val) == 0) {
+            if (is_list == -1) {
+                is_list = 1;
+            } else if (!is_list) {
+                error_setg(errp,
+                           "Cannot mix list and non-list keys");
+                return -1;
+            }
+            len++;
+            if (val > max) {
+                max = val;
+            }
+        } else {
+            if (is_list == -1) {
+                is_list = 0;
+            } else if (is_list) {
+                error_setg(errp,
+                           "Cannot mix list and non-list keys");
+                return -1;
+            }
+        }
+    }
+
+    if (is_list == -1) {
+        assert(!qdict_size(maybe_list));
+        is_list = 0;
+    }
+
+    /* NB this isn't a perfect check - eg it won't catch
+     * a list containing '1', '+1', '01', '3', but that
+     * does not matter - we've still proved that the
+     * input is a list. It is up the caller to do a
+     * stricter check if desired */
+    if (len != (max + 1)) {
+        error_setg(errp, "List indexes are not contiguous, "
+                   "saw %zd elements but %zd largest index",
+                   len, max);
+        return -1;
+    }
+
+    return is_list;
+}
+
+/**
+ * qdict_crumple:
+ * @src: the original flat dictionary (only scalar values) to crumple
+ * @recursive: true to recursively crumple nested dictionaries
+ *
+ * Takes a flat dictionary whose keys use '.' separator to indicate
+ * nesting, and values are scalars, and crumples it into a nested
+ * structure. If the @recursive parameter is false, then only the
+ * first level of structure implied by the keys will be crumpled. If
+ * @recursive is true, then the input will be recursively crumpled to
+ * expand all levels of structure in the keys.
+ *
+ * To include a literal '.' in a key name, it must be escaped as '..'
+ *
+ * For example, an input of:
+ *
+ * { 'foo.0.bar': 'one', 'foo.0.wizz': '1',
+ *   'foo.1.bar': 'two', 'foo.1.wizz': '2' }
+ *
+ * will result in any output of:
+ *
+ * {
+ *   'foo': [
+ *      { 'bar': 'one', 'wizz': '1' },
+ *      { 'bar': 'two', 'wizz': '2' }
+ *   ],
+ * }
+ *
+ * The following scenarios in the input dict will result in an
+ * error being returned:
+ *
+ *  - Any values in @src are non-scalar types
+ *  - If keys in @src imply that a particular level is both a
+ *    list and a dict. eg, "foo.0.bar" and "foo.eek.bar".
+ *  - If keys in @src imply that a particular level is a list,
+ *    but the indexes are non-contigous. eg "foo.0.bar" and
+ *    "foo.2.bar" without any "foo.1.bar" present.
+ *  - If keys in @src represent list indexes, but are not in
+ *    the "%zu" format. eg "foo.+0.bar"
+ *
+ * Returns: either a QDict or QList for the nested data structure, or NULL
+ * on error
+ */
+QObject *qdict_crumple(QDict *src, bool recursive, Error **errp)
+{
+    const QDictEntry *ent;
+    QDict *two_level, *multi_level = NULL;
+    QObject *dst = NULL, *child;
+    size_t i;
+    char *prefix = NULL;
+    const char *suffix = NULL;
+    int is_list;
+
+    two_level = qdict_new();
+
+    /* Step 1: split our totally flat dict into a two level dict */
+    for (ent = qdict_first(src); ent != NULL; ent = qdict_next(src, ent)) {
+        /* Input should be totally flat, so if we see a dict/list as a
+         * value that's invalid usage of the API */
+        if (qobject_type(ent->value) == QTYPE_QDICT ||
+            qobject_type(ent->value) == QTYPE_QLIST) {
+            error_setg(errp, "Value %s is not a scalar",
+                       ent->key);
+            goto error;
+        }
+
+        qdict_split_flat_key(ent->key, &prefix, &suffix);
+
+        child = qdict_get(two_level, prefix);
+        if (suffix) {
+            if (child) {
+                if (qobject_type(child) != QTYPE_QDICT) {
+                    error_setg(errp, "Key %s prefix is already set as a scalar",
+                               prefix);
+                    goto error;
+                }
+            } else {
+                child = QOBJECT(qdict_new());
+                qdict_put_obj(two_level, prefix, child);
+            }
+            qobject_incref(ent->value);
+            qdict_put_obj(qobject_to_qdict(child), suffix, ent->value);
+        } else {
+            if (child) {
+                error_setg(errp, "Key %s prefix is already set as a dict",
+                           prefix);
+                goto error;
+            }
+            qobject_incref(ent->value);
+            qdict_put_obj(two_level, prefix, ent->value);
+        }
+
+        g_free(prefix);
+        prefix = NULL;
+    }
+
+    /* Step 2: optionally process the two level dict recursively
+     * into a multi-level dict */
+    if (recursive) {
+        multi_level = qdict_new();
+        for (ent = qdict_first(two_level); ent != NULL;
+             ent = qdict_next(two_level, ent)) {
+
+            if (qobject_type(ent->value) == QTYPE_QDICT) {
+                child = qdict_crumple(qobject_to_qdict(ent->value),
+                                      recursive, errp);
+                if (!child) {
+                    goto error;
+                }
+
+                qdict_put_obj(multi_level, ent->key, child);
+            } else {
+                qobject_incref(ent->value);
+                qdict_put_obj(multi_level, ent->key, ent->value);
+            }
+        }
+        QDECREF(two_level);
+    } else {
+        multi_level = two_level;
+    }
+    two_level = NULL;
+
+    /* Step 3: detect if we need to turn our dict into list */
+    is_list = qdict_is_list(multi_level, errp);
+    if (is_list < 0) {
+        goto error;
+    }
+
+    if (is_list) {
+        dst = QOBJECT(qlist_new());
+
+        for (i = 0; i < qdict_size(multi_level); i++) {
+            char *key = g_strdup_printf("%zu", i);
+
+            child = qdict_get(multi_level, key);
+            g_free(key);
+
+            if (!child) {
+                error_setg(errp, "Missing list index %zu", i);
+                goto error;
+            }
+
+            qobject_incref(child);
+            qlist_append_obj(qobject_to_qlist(dst), child);
+        }
+        QDECREF(multi_level);
+        multi_level = NULL;
+    } else {
+        dst = QOBJECT(multi_level);
+    }
+
+    return dst;
+
+ error:
+    g_free(prefix);
+    QDECREF(multi_level);
+    QDECREF(two_level);
+    qobject_decref(dst);
+    return NULL;
+}
+
+
 /**
  * qdict_array_entries(): Returns the number of direct array entries if the
  * sub-QDict of src specified by the prefix in subqdict (or src itself for
diff --git a/tests/check-qdict.c b/tests/check-qdict.c
index 42da1e6..bb0bf75 100644
--- a/tests/check-qdict.c
+++ b/tests/check-qdict.c
@@ -14,6 +14,7 @@
 #include "qapi/qmp/qint.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qstring.h"
+#include "qapi/error.h"
 #include "qemu-common.h"
 
 /*
@@ -595,6 +596,254 @@ static void qdict_join_test(void)
     QDECREF(dict2);
 }
 
+
+static void qdict_crumple_test_nonrecursive(void)
+{
+    QDict *src, *dst, *rules, *vnc, *acl;
+    QObject *child, *res;
+
+    src = qdict_new();
+    qdict_put(src, "vnc.listen.addr", qstring_from_str("127.0.0.1"));
+    qdict_put(src, "vnc.listen.port", qstring_from_str("5901"));
+    qdict_put(src, "rule.0.match", qstring_from_str("fred"));
+    qdict_put(src, "rule.0.policy", qstring_from_str("allow"));
+    qdict_put(src, "rule.1.match", qstring_from_str("bob"));
+    qdict_put(src, "rule.1.policy", qstring_from_str("deny"));
+    qdict_put(src, "acl..name", qstring_from_str("acl0"));
+    qdict_put(src, "acl.rule..name", qstring_from_str("acl0"));
+
+    res = qdict_crumple(src, false, &error_abort);
+
+    g_assert_cmpint(qobject_type(res), ==, QTYPE_QDICT);
+
+    dst = qobject_to_qdict(res);
+
+    g_assert_cmpint(qdict_size(dst), ==, 4);
+
+    child = qdict_get(dst, "vnc");
+    g_assert_cmpint(qobject_type(child), ==, QTYPE_QDICT);
+    vnc = qdict_get_qdict(dst, "vnc");
+
+    g_assert_cmpint(qdict_size(vnc), ==, 2);
+
+    g_assert_cmpstr("127.0.0.1", ==, qdict_get_str(vnc, "listen.addr"));
+    g_assert_cmpstr("5901", ==, qdict_get_str(vnc, "listen.port"));
+
+    child = qdict_get(dst, "rule");
+    g_assert_cmpint(qobject_type(child), ==, QTYPE_QDICT);
+    rules = qdict_get_qdict(dst, "rule");
+
+    g_assert_cmpint(qdict_size(rules), ==, 4);
+
+    g_assert_cmpstr("fred", ==, qdict_get_str(rules, "0.match"));
+    g_assert_cmpstr("allow", ==, qdict_get_str(rules, "0.policy"));
+    g_assert_cmpstr("bob", ==, qdict_get_str(rules, "1.match"));
+    g_assert_cmpstr("deny", ==, qdict_get_str(rules, "1.policy"));
+
+    /* With non-recursive crumpling, we should only see the first
+     * level names unescaped */
+    g_assert_cmpstr("acl0", ==, qdict_get_str(dst, "acl.name"));
+    child = qdict_get(dst, "acl");
+    g_assert_cmpint(qobject_type(child), ==, QTYPE_QDICT);
+    acl = qdict_get_qdict(dst, "acl");
+    g_assert_cmpstr("acl0", ==, qdict_get_str(acl, "rule..name"));
+
+    QDECREF(src);
+    QDECREF(dst);
+}
+
+
+static void qdict_crumple_test_listtop(void)
+{
+    QDict *src, *rule;
+    QList *rules;
+    QObject *res;
+
+    src = qdict_new();
+    qdict_put(src, "0.match.name", qstring_from_str("Fred"));
+    qdict_put(src, "0.match.email", qstring_from_str("fred@example.com"));
+    qdict_put(src, "0.policy", qstring_from_str("allow"));
+    qdict_put(src, "1.match.name", qstring_from_str("Bob"));
+    qdict_put(src, "1.match.email", qstring_from_str("bob@example.com"));
+    qdict_put(src, "1.policy", qstring_from_str("deny"));
+
+    res = qdict_crumple(src, false, &error_abort);
+
+    g_assert_cmpint(qobject_type(res), ==, QTYPE_QLIST);
+
+    rules = qobject_to_qlist(res);
+
+    g_assert_cmpint(qlist_size(rules), ==, 2);
+
+    g_assert_cmpint(qobject_type(qlist_peek(rules)), ==, QTYPE_QDICT);
+    rule = qobject_to_qdict(qlist_pop(rules));
+    g_assert_cmpint(qdict_size(rule), ==, 3);
+
+    g_assert_cmpstr("Fred", ==, qdict_get_str(rule, "match.name"));
+    g_assert_cmpstr("fred@example.com", ==, qdict_get_str(rule, "match.email"));
+    g_assert_cmpstr("allow", ==, qdict_get_str(rule, "policy"));
+    QDECREF(rule);
+
+    g_assert_cmpint(qobject_type(qlist_peek(rules)), ==, QTYPE_QDICT);
+    rule = qobject_to_qdict(qlist_pop(rules));
+    g_assert_cmpint(qdict_size(rule), ==, 3);
+
+    g_assert_cmpstr("Bob", ==, qdict_get_str(rule, "match.name"));
+    g_assert_cmpstr("bob@example.com", ==, qdict_get_str(rule, "match.email"));
+    g_assert_cmpstr("deny", ==, qdict_get_str(rule, "policy"));
+    QDECREF(rule);
+
+    QDECREF(src);
+    qobject_decref(res);
+}
+
+
+static void qdict_crumple_test_recursive(void)
+{
+    QDict *src, *dst, *rule, *vnc, *acl, *listen;
+    QObject *child, *res;
+    QList *rules;
+
+    src = qdict_new();
+    qdict_put(src, "vnc.listen.addr", qstring_from_str("127.0.0.1"));
+    qdict_put(src, "vnc.listen.port", qstring_from_str("5901"));
+    qdict_put(src, "vnc.acl.rules.0.match", qstring_from_str("fred"));
+    qdict_put(src, "vnc.acl.rules.0.policy", qstring_from_str("allow"));
+    qdict_put(src, "vnc.acl.rules.1.match", qstring_from_str("bob"));
+    qdict_put(src, "vnc.acl.rules.1.policy", qstring_from_str("deny"));
+    qdict_put(src, "vnc.acl.default", qstring_from_str("deny"));
+    qdict_put(src, "vnc.acl..name", qstring_from_str("acl0"));
+    qdict_put(src, "vnc.acl.rule..name", qstring_from_str("acl0"));
+
+    res = qdict_crumple(src, true, &error_abort);
+
+    g_assert_cmpint(qobject_type(res), ==, QTYPE_QDICT);
+
+    dst = qobject_to_qdict(res);
+
+    g_assert_cmpint(qdict_size(dst), ==, 1);
+
+    child = qdict_get(dst, "vnc");
+    g_assert_cmpint(qobject_type(child), ==, QTYPE_QDICT);
+    vnc = qobject_to_qdict(child);
+
+    child = qdict_get(vnc, "listen");
+    g_assert_cmpint(qobject_type(child), ==, QTYPE_QDICT);
+    listen = qobject_to_qdict(child);
+    g_assert_cmpstr("127.0.0.1", ==, qdict_get_str(listen, "addr"));
+    g_assert_cmpstr("5901", ==, qdict_get_str(listen, "port"));
+
+    child = qdict_get(vnc, "acl");
+    g_assert_cmpint(qobject_type(child), ==, QTYPE_QDICT);
+    acl = qobject_to_qdict(child);
+
+    child = qdict_get(acl, "rules");
+    g_assert_cmpint(qobject_type(child), ==, QTYPE_QLIST);
+    rules = qobject_to_qlist(child);
+    g_assert_cmpint(qlist_size(rules), ==, 2);
+
+    rule = qobject_to_qdict(qlist_pop(rules));
+    g_assert_cmpint(qdict_size(rule), ==, 2);
+    g_assert_cmpstr("fred", ==, qdict_get_str(rule, "match"));
+    g_assert_cmpstr("allow", ==, qdict_get_str(rule, "policy"));
+    QDECREF(rule);
+
+    rule = qobject_to_qdict(qlist_pop(rules));
+    g_assert_cmpint(qdict_size(rule), ==, 2);
+    g_assert_cmpstr("bob", ==, qdict_get_str(rule, "match"));
+    g_assert_cmpstr("deny", ==, qdict_get_str(rule, "policy"));
+    QDECREF(rule);
+
+    /* With recursive crumpling, we should see all names unescaped */
+    g_assert_cmpstr("acl0", ==, qdict_get_str(vnc, "acl.name"));
+    child = qdict_get(vnc, "acl");
+    g_assert_cmpint(qobject_type(child), ==, QTYPE_QDICT);
+    acl = qdict_get_qdict(vnc, "acl");
+    g_assert_cmpstr("acl0", ==, qdict_get_str(acl, "rule.name"));
+
+    QDECREF(src);
+    QDECREF(dst);
+}
+
+
+static void qdict_crumple_test_empty(void)
+{
+    QDict *src, *dst;
+
+    src = qdict_new();
+
+    dst = (QDict *)qdict_crumple(src, true, &error_abort);
+
+    g_assert_cmpint(qdict_size(dst), ==, 0);
+
+    QDECREF(src);
+    QDECREF(dst);
+}
+
+
+static void qdict_crumple_test_bad_inputs(void)
+{
+    QDict *src;
+    Error *error = NULL;
+
+    src = qdict_new();
+    /* rule.0 can't be both a string and a dict */
+    qdict_put(src, "rule.0", qstring_from_str("fred"));
+    qdict_put(src, "rule.0.policy", qstring_from_str("allow"));
+
+    g_assert(qdict_crumple(src, true, &error) == NULL);
+    g_assert(error != NULL);
+    error_free(error);
+    error = NULL;
+    QDECREF(src);
+
+    src = qdict_new();
+    /* rule can't be both a list and a dict */
+    qdict_put(src, "rule.0", qstring_from_str("fred"));
+    qdict_put(src, "rule.a", qstring_from_str("allow"));
+
+    g_assert(qdict_crumple(src, true, &error) == NULL);
+    g_assert(error != NULL);
+    error_free(error);
+    error = NULL;
+    QDECREF(src);
+
+    src = qdict_new();
+    /* The input should be flat, ie no dicts or lists */
+    qdict_put(src, "rule.a", qdict_new());
+    qdict_put(src, "rule.b", qstring_from_str("allow"));
+
+    g_assert(qdict_crumple(src, true, &error) == NULL);
+    g_assert(error != NULL);
+    error_free(error);
+    error = NULL;
+    QDECREF(src);
+
+
+    src = qdict_new();
+    /* List indexes must not have gaps */
+    qdict_put(src, "rule.0", qstring_from_str("deny"));
+    qdict_put(src, "rule.3", qstring_from_str("allow"));
+
+    g_assert(qdict_crumple(src, true, &error) == NULL);
+    g_assert(error != NULL);
+    error_free(error);
+    error = NULL;
+    QDECREF(src);
+
+
+    src = qdict_new();
+    /* List indexes must be in %zu format */
+    qdict_put(src, "rule.0", qstring_from_str("deny"));
+    qdict_put(src, "rule.+1", qstring_from_str("allow"));
+
+    g_assert(qdict_crumple(src, true, &error) == NULL);
+    g_assert(error != NULL);
+    error_free(error);
+    error = NULL;
+    QDECREF(src);
+}
+
 /*
  * Errors test-cases
  */
@@ -742,6 +991,17 @@ int main(int argc, char **argv)
     g_test_add_func("/errors/put_exists", qdict_put_exists_test);
     g_test_add_func("/errors/get_not_exists", qdict_get_not_exists_test);
 
+    g_test_add_func("/public/crumple/nonrecursive",
+                    qdict_crumple_test_nonrecursive);
+    g_test_add_func("/public/crumple/recursive",
+                    qdict_crumple_test_recursive);
+    g_test_add_func("/public/crumple/listtop",
+                    qdict_crumple_test_listtop);
+    g_test_add_func("/public/crumple/empty",
+                    qdict_crumple_test_empty);
+    g_test_add_func("/public/crumple/bad_inputs",
+                    qdict_crumple_test_bad_inputs);
+
     /* The Big one */
     if (g_test_slow()) {
         g_test_add_func("/stress/test", qdict_stress_test);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH v13 2/6] option: make parse_option_bool/number non-static
  2016-09-19 11:58 [Qemu-devel] [PATCH v13 0/6] QAPI/QOM work for non-scalar object properties Daniel P. Berrange
  2016-09-19 11:58 ` [Qemu-devel] [PATCH v13 1/6] qdict: implement a qdict_crumple method for un-flattening a dict Daniel P. Berrange
@ 2016-09-19 11:58 ` Daniel P. Berrange
  2016-09-23 10:24   ` Kevin Wolf
  2016-09-19 11:58 ` [Qemu-devel] [PATCH v13 3/6] qapi: rename QmpInputVisitor to QObjectInputVisitor Daniel P. Berrange
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Daniel P. Berrange @ 2016-09-19 11:58 UTC (permalink / raw)
  To: qemu-devel
  Cc: Markus Armbruster, Max Reitz, Marc-André Lureau,
	Paolo Bonzini, Andreas Färber, Kevin Wolf,
	Daniel P. Berrange

The opts-visitor.c opts_type_bool() method has code for
parsing a string to set a bool value, as does the
qemu-option.c parse_option_bool() method, except it
handles fewer cases.

To enable consistency across the codebase, extend
parse_option_bool() to handle "yes", "no", "y" and
"n", and make it non-static. Convert the opts
visitor to call this method directly.

Also make parse_option_number() non-static to allow
for similar reuse later.

Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 include/qemu/option.h |  4 ++++
 qapi/opts-visitor.c   | 19 +------------------
 util/qemu-option.c    | 27 ++++++++++++++++-----------
 3 files changed, 21 insertions(+), 29 deletions(-)

diff --git a/include/qemu/option.h b/include/qemu/option.h
index 1f9e3f9..2a5266f 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -37,6 +37,10 @@ int get_param_value(char *buf, int buf_size,
                     const char *tag, const char *str);
 
 
+void parse_option_bool(const char *name, const char *value, bool *ret,
+                       Error **errp);
+void parse_option_number(const char *name, const char *value,
+                         uint64_t *ret, Error **errp);
 void parse_option_size(const char *name, const char *value,
                        uint64_t *ret, Error **errp);
 bool has_help_option(const char *param);
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 1048bbc..084f7cc 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -334,7 +334,6 @@ opts_type_str(Visitor *v, const char *name, char **obj, Error **errp)
 }
 
 
-/* mimics qemu-option.c::parse_option_bool() */
 static void
 opts_type_bool(Visitor *v, const char *name, bool *obj, Error **errp)
 {
@@ -346,23 +345,7 @@ opts_type_bool(Visitor *v, const char *name, bool *obj, Error **errp)
         return;
     }
 
-    if (opt->str) {
-        if (strcmp(opt->str, "on") == 0 ||
-            strcmp(opt->str, "yes") == 0 ||
-            strcmp(opt->str, "y") == 0) {
-            *obj = true;
-        } else if (strcmp(opt->str, "off") == 0 ||
-            strcmp(opt->str, "no") == 0 ||
-            strcmp(opt->str, "n") == 0) {
-            *obj = false;
-        } else {
-            error_setg(errp, QERR_INVALID_PARAMETER_VALUE, opt->name,
-                       "on|yes|y|off|no|n");
-            return;
-        }
-    } else {
-        *obj = true;
-    }
+    parse_option_bool(opt->name, opt->str, obj, errp);
 
     processed(ov, name);
 }
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 3467dc2..6fc9ccb 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -125,25 +125,30 @@ int get_param_value(char *buf, int buf_size,
     return get_next_param_value(buf, buf_size, tag, &str);
 }
 
-static void parse_option_bool(const char *name, const char *value, bool *ret,
-                              Error **errp)
+void parse_option_bool(const char *name, const char *value, bool *ret,
+                       Error **errp)
 {
     if (value != NULL) {
-        if (!strcmp(value, "on")) {
-            *ret = 1;
-        } else if (!strcmp(value, "off")) {
-            *ret = 0;
+        if (strcmp(value, "on") == 0 ||
+            strcmp(value, "yes") == 0 ||
+            strcmp(value, "y") == 0) {
+            *ret = true;
+        } else if (strcmp(value, "off") == 0 ||
+            strcmp(value, "no") == 0 ||
+            strcmp(value, "n") == 0) {
+            *ret = false;
         } else {
-            error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
-                       name, "'on' or 'off'");
+            error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name,
+                       "on|yes|y|off|no|n");
+            return;
         }
     } else {
-        *ret = 1;
+        *ret = true;
     }
 }
 
-static void parse_option_number(const char *name, const char *value,
-                                uint64_t *ret, Error **errp)
+void parse_option_number(const char *name, const char *value,
+                         uint64_t *ret, Error **errp)
 {
     char *postfix;
     uint64_t number;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH v13 3/6] qapi: rename QmpInputVisitor to QObjectInputVisitor
  2016-09-19 11:58 [Qemu-devel] [PATCH v13 0/6] QAPI/QOM work for non-scalar object properties Daniel P. Berrange
  2016-09-19 11:58 ` [Qemu-devel] [PATCH v13 1/6] qdict: implement a qdict_crumple method for un-flattening a dict Daniel P. Berrange
  2016-09-19 11:58 ` [Qemu-devel] [PATCH v13 2/6] option: make parse_option_bool/number non-static Daniel P. Berrange
@ 2016-09-19 11:58 ` Daniel P. Berrange
  2016-09-19 11:58 ` [Qemu-devel] [PATCH v13 4/6] qapi: rename QmpOutputVisitor to QObjectOutputVisitor Daniel P. Berrange
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 18+ messages in thread
From: Daniel P. Berrange @ 2016-09-19 11:58 UTC (permalink / raw)
  To: qemu-devel
  Cc: Markus Armbruster, Max Reitz, Marc-André Lureau,
	Paolo Bonzini, Andreas Färber, Kevin Wolf,
	Daniel P. Berrange

The QmpInputVisitor has no direct dependency on QMP. It is
valid to use it anywhere that one has a QObject. Rename it
to better reflect its functionality as a generic QObject
to QAPI converter.

Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 docs/qapi-code-gen.txt                             |   2 +-
 ...qmp-input-visitor.h => qobject-input-visitor.h} |  10 +-
 include/qapi/visitor.h                             |   6 +-
 monitor.c                                          |   2 +-
 qapi/Makefile.objs                                 |   2 +-
 ...qmp-input-visitor.c => qobject-input-visitor.c} | 171 +++++++++++----------
 qmp.c                                              |   4 +-
 qom/qom-qobject.c                                  |   4 +-
 scripts/qapi-commands.py                           |   4 +-
 target-s390x/cpu_models.c                          |   4 +-
 tests/.gitignore                                   |   4 +-
 tests/Makefile.include                             |  12 +-
 tests/check-qnull.c                                |   4 +-
 tests/test-qmp-commands.c                          |   4 +-
 ...-input-strict.c => test-qobject-input-strict.c} |   6 +-
 ...nput-visitor.c => test-qobject-input-visitor.c} |   6 +-
 tests/test-string-input-visitor.c                  |   2 +-
 tests/test-visitor-serialization.c                 |   4 +-
 util/qemu-sockets.c                                |   2 +-
 19 files changed, 128 insertions(+), 125 deletions(-)
 rename include/qapi/{qmp-input-visitor.h => qobject-input-visitor.h} (63%)
 rename qapi/{qmp-input-visitor.c => qobject-input-visitor.c} (56%)
 rename tests/{test-qmp-input-strict.c => test-qobject-input-strict.c} (98%)
 rename tests/{test-qmp-input-visitor.c => test-qobject-input-visitor.c} (99%)

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index de298dc..a011872 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 = qobject_input_visitor_new(QOBJECT(args), true);
         visit_start_struct(v, NULL, NULL, 0, &err);
         if (err) {
             goto out;
diff --git a/include/qapi/qmp-input-visitor.h b/include/qapi/qobject-input-visitor.h
similarity index 63%
rename from include/qapi/qmp-input-visitor.h
rename to include/qapi/qobject-input-visitor.h
index f3ff5f3..cde328d 100644
--- a/include/qapi/qmp-input-visitor.h
+++ b/include/qapi/qobject-input-visitor.h
@@ -11,20 +11,20 @@
  *
  */
 
-#ifndef QMP_INPUT_VISITOR_H
-#define QMP_INPUT_VISITOR_H
+#ifndef QOBJECT_INPUT_VISITOR_H
+#define QOBJECT_INPUT_VISITOR_H
 
 #include "qapi/visitor.h"
 #include "qapi/qmp/qobject.h"
 
-typedef struct QmpInputVisitor QmpInputVisitor;
+typedef struct QObjectInputVisitor QObjectInputVisitor;
 
 /*
- * Return a new input visitor that converts QMP to QAPI.
+ * Return a new input visitor that converts a QObject to a QAPI object.
  *
  * Set @strict to reject a parse that doesn't consume all keys of a
  * dictionary; otherwise excess input is ignored.
  */
-Visitor *qmp_input_visitor_new(QObject *obj, bool strict);
+Visitor *qobject_input_visitor_new(QObject *obj, bool strict);
 
 #endif
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 6c77a91..9bb6cba 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -25,14 +25,14 @@
  * for doing work at each node of a QAPI graph; it can also be used
  * for a virtual walk, where there is no actual QAPI C struct.
  *
- * There are four kinds of visitor classes: input visitors (QMP,
+ * There are four kinds of visitor classes: input visitors (QObject,
  * string, and QemuOpts) parse an external representation and build
- * the corresponding QAPI graph, output visitors (QMP and string) take
+ * the corresponding QAPI graph, output visitors (QObject and string) take
  * a completed QAPI graph and generate an external representation, the
  * dealloc visitor can take a QAPI graph (possibly partially
  * constructed) and recursively free its resources, and the clone
  * visitor performs a deep clone of one QAPI object to another.  While
- * the dealloc and QMP input/output visitors are general, the string,
+ * the dealloc and QObject input/output visitors are general, the string,
  * QemuOpts, and clone visitors have some implementation limitations;
  * see the documentation for each visitor for more details on what it
  * supports.  Also, see visitor-impl.h for the callback contracts
diff --git a/monitor.c b/monitor.c
index 5c00373..b151934 100644
--- a/monitor.c
+++ b/monitor.c
@@ -998,7 +998,7 @@ EventInfoList *qmp_query_events(Error **errp)
  * directly into QObject instead of first parsing it with
  * visit_type_SchemaInfoList() into a SchemaInfoList, then marshal it
  * to QObject with generated output marshallers, every time.  Instead,
- * we do it in test-qmp-input-visitor.c, just to make sure
+ * we do it in test-qobject-input-visitor.c, just to make sure
  * qapi-introspect.py's output actually conforms to the schema.
  */
 static void qmp_query_qmp_schema(QDict *qdict, QObject **ret_data,
diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs
index 7ea4aeb..6ec7bdc 100644
--- a/qapi/Makefile.objs
+++ b/qapi/Makefile.objs
@@ -1,4 +1,4 @@
-util-obj-y = qapi-visit-core.o qapi-dealloc-visitor.o qmp-input-visitor.o
+util-obj-y = qapi-visit-core.o qapi-dealloc-visitor.o qobject-input-visitor.o
 util-obj-y += qmp-output-visitor.o qmp-registry.o qmp-dispatch.o
 util-obj-y += string-input-visitor.o string-output-visitor.o
 util-obj-y += opts-visitor.o qapi-clone-visitor.o
diff --git a/qapi/qmp-input-visitor.c b/qapi/qobject-input-visitor.c
similarity index 56%
rename from qapi/qmp-input-visitor.c
rename to qapi/qobject-input-visitor.c
index 64dd392..5ff3db3 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -14,7 +14,7 @@
 
 #include "qemu/osdep.h"
 #include "qapi/error.h"
-#include "qapi/qmp-input-visitor.h"
+#include "qapi/qobject-input-visitor.h"
 #include "qapi/visitor-impl.h"
 #include "qemu/queue.h"
 #include "qemu-common.h"
@@ -34,7 +34,7 @@ typedef struct StackObject
     QSLIST_ENTRY(StackObject) node;
 } StackObject;
 
-struct QmpInputVisitor
+struct QObjectInputVisitor
 {
     Visitor visitor;
 
@@ -49,14 +49,14 @@ struct QmpInputVisitor
     bool strict;
 };
 
-static QmpInputVisitor *to_qiv(Visitor *v)
+static QObjectInputVisitor *to_qiv(Visitor *v)
 {
-    return container_of(v, QmpInputVisitor, visitor);
+    return container_of(v, QObjectInputVisitor, visitor);
 }
 
-static QObject *qmp_input_get_object(QmpInputVisitor *qiv,
-                                     const char *name,
-                                     bool consume)
+static QObject *qobject_input_get_object(QObjectInputVisitor *qiv,
+                                         const char *name,
+                                         bool consume)
 {
     StackObject *tos;
     QObject *qobj;
@@ -97,8 +97,9 @@ static void qdict_add_key(const char *key, QObject *obj, void *opaque)
     g_hash_table_insert(h, (gpointer) key, NULL);
 }
 
-static const QListEntry *qmp_input_push(QmpInputVisitor *qiv, QObject *obj,
-                                        void *qapi, Error **errp)
+static const QListEntry *qobject_input_push(QObjectInputVisitor *qiv,
+                                            QObject *obj, void *qapi,
+                                            Error **errp)
 {
     GHashTable *h;
     StackObject *tos = g_new0(StackObject, 1);
@@ -120,9 +121,9 @@ static const QListEntry *qmp_input_push(QmpInputVisitor *qiv, QObject *obj,
 }
 
 
-static void qmp_input_check_struct(Visitor *v, Error **errp)
+static void qobject_input_check_struct(Visitor *v, Error **errp)
 {
-    QmpInputVisitor *qiv = to_qiv(v);
+    QObjectInputVisitor *qiv = to_qiv(v);
     StackObject *tos = QSLIST_FIRST(&qiv->stack);
 
     assert(tos && !tos->entry);
@@ -140,7 +141,7 @@ static void qmp_input_check_struct(Visitor *v, Error **errp)
     }
 }
 
-static void qmp_input_stack_object_free(StackObject *tos)
+static void qobject_input_stack_object_free(StackObject *tos)
 {
     if (tos->h) {
         g_hash_table_unref(tos->h);
@@ -149,21 +150,21 @@ static void qmp_input_stack_object_free(StackObject *tos)
     g_free(tos);
 }
 
-static void qmp_input_pop(Visitor *v, void **obj)
+static void qobject_input_pop(Visitor *v, void **obj)
 {
-    QmpInputVisitor *qiv = to_qiv(v);
+    QObjectInputVisitor *qiv = to_qiv(v);
     StackObject *tos = QSLIST_FIRST(&qiv->stack);
 
     assert(tos && tos->qapi == obj);
     QSLIST_REMOVE_HEAD(&qiv->stack, node);
-    qmp_input_stack_object_free(tos);
+    qobject_input_stack_object_free(tos);
 }
 
-static void qmp_input_start_struct(Visitor *v, const char *name, void **obj,
-                                   size_t size, Error **errp)
+static void qobject_input_start_struct(Visitor *v, const char *name, void **obj,
+                                       size_t size, Error **errp)
 {
-    QmpInputVisitor *qiv = to_qiv(v);
-    QObject *qobj = qmp_input_get_object(qiv, name, true);
+    QObjectInputVisitor *qiv = to_qiv(v);
+    QObject *qobj = qobject_input_get_object(qiv, name, true);
     Error *err = NULL;
 
     if (obj) {
@@ -175,7 +176,7 @@ static void qmp_input_start_struct(Visitor *v, const char *name, void **obj,
         return;
     }
 
-    qmp_input_push(qiv, qobj, obj, &err);
+    qobject_input_push(qiv, qobj, obj, &err);
     if (err) {
         error_propagate(errp, err);
         return;
@@ -187,11 +188,12 @@ static void qmp_input_start_struct(Visitor *v, const char *name, void **obj,
 }
 
 
-static void qmp_input_start_list(Visitor *v, const char *name,
-                                 GenericList **list, size_t size, Error **errp)
+static void qobject_input_start_list(Visitor *v, const char *name,
+                                     GenericList **list, size_t size,
+                                     Error **errp)
 {
-    QmpInputVisitor *qiv = to_qiv(v);
-    QObject *qobj = qmp_input_get_object(qiv, name, true);
+    QObjectInputVisitor *qiv = to_qiv(v);
+    QObject *qobj = qobject_input_get_object(qiv, name, true);
     const QListEntry *entry;
 
     if (!qobj || qobject_type(qobj) != QTYPE_QLIST) {
@@ -203,7 +205,7 @@ static void qmp_input_start_list(Visitor *v, const char *name,
         return;
     }
 
-    entry = qmp_input_push(qiv, qobj, list, errp);
+    entry = qobject_input_push(qiv, qobj, list, errp);
     if (list) {
         if (entry) {
             *list = g_malloc0(size);
@@ -213,10 +215,10 @@ static void qmp_input_start_list(Visitor *v, const char *name,
     }
 }
 
-static GenericList *qmp_input_next_list(Visitor *v, GenericList *tail,
-                                        size_t size)
+static GenericList *qobject_input_next_list(Visitor *v, GenericList *tail,
+                                            size_t size)
 {
-    QmpInputVisitor *qiv = to_qiv(v);
+    QObjectInputVisitor *qiv = to_qiv(v);
     StackObject *so = QSLIST_FIRST(&qiv->stack);
 
     if (!so->entry) {
@@ -227,12 +229,12 @@ static GenericList *qmp_input_next_list(Visitor *v, GenericList *tail,
 }
 
 
-static void qmp_input_start_alternate(Visitor *v, const char *name,
-                                      GenericAlternate **obj, size_t size,
-                                      bool promote_int, Error **errp)
+static void qobject_input_start_alternate(Visitor *v, const char *name,
+                                          GenericAlternate **obj, size_t size,
+                                          bool promote_int, Error **errp)
 {
-    QmpInputVisitor *qiv = to_qiv(v);
-    QObject *qobj = qmp_input_get_object(qiv, name, false);
+    QObjectInputVisitor *qiv = to_qiv(v);
+    QObject *qobj = qobject_input_get_object(qiv, name, false);
 
     if (!qobj) {
         *obj = NULL;
@@ -246,11 +248,11 @@ static void qmp_input_start_alternate(Visitor *v, const char *name,
     }
 }
 
-static void qmp_input_type_int64(Visitor *v, const char *name, int64_t *obj,
-                                 Error **errp)
+static void qobject_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));
+    QObjectInputVisitor *qiv = to_qiv(v);
+    QInt *qint = qobject_to_qint(qobject_input_get_object(qiv, name, true));
 
     if (!qint) {
         error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
@@ -261,12 +263,12 @@ static void qmp_input_type_int64(Visitor *v, const char *name, int64_t *obj,
     *obj = qint_get_int(qint);
 }
 
-static void qmp_input_type_uint64(Visitor *v, const char *name, uint64_t *obj,
-                                  Error **errp)
+static void qobject_input_type_uint64(Visitor *v, const char *name,
+                                      uint64_t *obj, Error **errp)
 {
     /* 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));
+    QObjectInputVisitor *qiv = to_qiv(v);
+    QInt *qint = qobject_to_qint(qobject_input_get_object(qiv, name, true));
 
     if (!qint) {
         error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
@@ -277,11 +279,11 @@ static void qmp_input_type_uint64(Visitor *v, const char *name, uint64_t *obj,
     *obj = qint_get_int(qint);
 }
 
-static void qmp_input_type_bool(Visitor *v, const char *name, bool *obj,
-                                Error **errp)
+static void qobject_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));
+    QObjectInputVisitor *qiv = to_qiv(v);
+    QBool *qbool = qobject_to_qbool(qobject_input_get_object(qiv, name, true));
 
     if (!qbool) {
         error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
@@ -292,11 +294,12 @@ static void qmp_input_type_bool(Visitor *v, const char *name, bool *obj,
     *obj = qbool_get_bool(qbool);
 }
 
-static void qmp_input_type_str(Visitor *v, const char *name, char **obj,
-                               Error **errp)
+static void qobject_input_type_str(Visitor *v, const char *name, char **obj,
+                                   Error **errp)
 {
-    QmpInputVisitor *qiv = to_qiv(v);
-    QString *qstr = qobject_to_qstring(qmp_input_get_object(qiv, name, true));
+    QObjectInputVisitor *qiv = to_qiv(v);
+    QString *qstr = qobject_to_qstring(qobject_input_get_object(qiv, name,
+                                                                true));
 
     if (!qstr) {
         *obj = NULL;
@@ -308,11 +311,11 @@ static void qmp_input_type_str(Visitor *v, const char *name, char **obj,
     *obj = g_strdup(qstring_get_str(qstr));
 }
 
-static void qmp_input_type_number(Visitor *v, const char *name, double *obj,
-                                  Error **errp)
+static void qobject_input_type_number(Visitor *v, const char *name, double *obj,
+                                      Error **errp)
 {
-    QmpInputVisitor *qiv = to_qiv(v);
-    QObject *qobj = qmp_input_get_object(qiv, name, true);
+    QObjectInputVisitor *qiv = to_qiv(v);
+    QObject *qobj = qobject_input_get_object(qiv, name, true);
     QInt *qint;
     QFloat *qfloat;
 
@@ -332,20 +335,20 @@ static void qmp_input_type_number(Visitor *v, const char *name, double *obj,
                "number");
 }
 
-static void qmp_input_type_any(Visitor *v, const char *name, QObject **obj,
-                               Error **errp)
+static void qobject_input_type_any(Visitor *v, const char *name, QObject **obj,
+                                   Error **errp)
 {
-    QmpInputVisitor *qiv = to_qiv(v);
-    QObject *qobj = qmp_input_get_object(qiv, name, true);
+    QObjectInputVisitor *qiv = to_qiv(v);
+    QObject *qobj = qobject_input_get_object(qiv, name, true);
 
     qobject_incref(qobj);
     *obj = qobj;
 }
 
-static void qmp_input_type_null(Visitor *v, const char *name, Error **errp)
+static void qobject_input_type_null(Visitor *v, const char *name, Error **errp)
 {
-    QmpInputVisitor *qiv = to_qiv(v);
-    QObject *qobj = qmp_input_get_object(qiv, name, true);
+    QObjectInputVisitor *qiv = to_qiv(v);
+    QObject *qobj = qobject_input_get_object(qiv, name, true);
 
     if (qobject_type(qobj) != QTYPE_QNULL) {
         error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
@@ -353,10 +356,10 @@ static void qmp_input_type_null(Visitor *v, const char *name, Error **errp)
     }
 }
 
-static void qmp_input_optional(Visitor *v, const char *name, bool *present)
+static void qobject_input_optional(Visitor *v, const char *name, bool *present)
 {
-    QmpInputVisitor *qiv = to_qiv(v);
-    QObject *qobj = qmp_input_get_object(qiv, name, false);
+    QObjectInputVisitor *qiv = to_qiv(v);
+    QObject *qobj = qobject_input_get_object(qiv, name, false);
 
     if (!qobj) {
         *present = false;
@@ -366,43 +369,43 @@ static void qmp_input_optional(Visitor *v, const char *name, bool *present)
     *present = true;
 }
 
-static void qmp_input_free(Visitor *v)
+static void qobject_input_free(Visitor *v)
 {
-    QmpInputVisitor *qiv = to_qiv(v);
+    QObjectInputVisitor *qiv = to_qiv(v);
     while (!QSLIST_EMPTY(&qiv->stack)) {
         StackObject *tos = QSLIST_FIRST(&qiv->stack);
 
         QSLIST_REMOVE_HEAD(&qiv->stack, node);
-        qmp_input_stack_object_free(tos);
+        qobject_input_stack_object_free(tos);
     }
 
     qobject_decref(qiv->root);
     g_free(qiv);
 }
 
-Visitor *qmp_input_visitor_new(QObject *obj, bool strict)
+Visitor *qobject_input_visitor_new(QObject *obj, bool strict)
 {
-    QmpInputVisitor *v;
+    QObjectInputVisitor *v;
 
     v = g_malloc0(sizeof(*v));
 
     v->visitor.type = VISITOR_INPUT;
-    v->visitor.start_struct = qmp_input_start_struct;
-    v->visitor.check_struct = qmp_input_check_struct;
-    v->visitor.end_struct = qmp_input_pop;
-    v->visitor.start_list = qmp_input_start_list;
-    v->visitor.next_list = qmp_input_next_list;
-    v->visitor.end_list = qmp_input_pop;
-    v->visitor.start_alternate = qmp_input_start_alternate;
-    v->visitor.type_int64 = qmp_input_type_int64;
-    v->visitor.type_uint64 = qmp_input_type_uint64;
-    v->visitor.type_bool = qmp_input_type_bool;
-    v->visitor.type_str = qmp_input_type_str;
-    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.optional = qmp_input_optional;
-    v->visitor.free = qmp_input_free;
+    v->visitor.start_struct = qobject_input_start_struct;
+    v->visitor.check_struct = qobject_input_check_struct;
+    v->visitor.end_struct = qobject_input_pop;
+    v->visitor.start_list = qobject_input_start_list;
+    v->visitor.next_list = qobject_input_next_list;
+    v->visitor.end_list = qobject_input_pop;
+    v->visitor.start_alternate = qobject_input_start_alternate;
+    v->visitor.type_int64 = qobject_input_type_int64;
+    v->visitor.type_uint64 = qobject_input_type_uint64;
+    v->visitor.type_bool = qobject_input_type_bool;
+    v->visitor.type_str = qobject_input_type_str;
+    v->visitor.type_number = qobject_input_type_number;
+    v->visitor.type_any = qobject_input_type_any;
+    v->visitor.type_null = qobject_input_type_null;
+    v->visitor.optional = qobject_input_optional;
+    v->visitor.free = qobject_input_free;
     v->strict = strict;
 
     v->root = obj;
diff --git a/qmp.c b/qmp.c
index dea8f81..8786c1b 100644
--- a/qmp.c
+++ b/qmp.c
@@ -30,7 +30,7 @@
 #include "qom/qom-qobject.h"
 #include "qapi/qmp/qerror.h"
 #include "qapi/qmp/qobject.h"
-#include "qapi/qmp-input-visitor.h"
+#include "qapi/qobject-input-visitor.h"
 #include "hw/boards.h"
 #include "qom/object_interfaces.h"
 #include "hw/mem/pc-dimm.h"
@@ -687,7 +687,7 @@ void qmp_object_add(const char *type, const char *id,
         }
     }
 
-    v = qmp_input_visitor_new(props, true);
+    v = qobject_input_visitor_new(props, true);
     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..81959e0 100644
--- a/qom/qom-qobject.c
+++ b/qom/qom-qobject.c
@@ -15,7 +15,7 @@
 #include "qom/object.h"
 #include "qom/qom-qobject.h"
 #include "qapi/visitor.h"
-#include "qapi/qmp-input-visitor.h"
+#include "qapi/qobject-input-visitor.h"
 #include "qapi/qmp-output-visitor.h"
 
 void object_property_set_qobject(Object *obj, QObject *value,
@@ -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 = qobject_input_visitor_new(value, false);
     object_property_set(obj, v, name, errp);
     visit_free(v);
 }
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index a06a2c4..3010163 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 = qobject_input_visitor_new(QOBJECT(args), true);
     visit_start_struct(v, NULL, NULL, 0, &err);
     if (err) {
         goto out;
@@ -279,7 +279,7 @@ fdef.write(mcgen('''
 #include "qapi/qmp/dispatch.h"
 #include "qapi/visitor.h"
 #include "qapi/qmp-output-visitor.h"
-#include "qapi/qmp-input-visitor.h"
+#include "qapi/qobject-input-visitor.h"
 #include "qapi/dealloc-visitor.h"
 #include "%(prefix)sqapi-types.h"
 #include "%(prefix)sqapi-visit.h"
diff --git a/target-s390x/cpu_models.c b/target-s390x/cpu_models.c
index 3ff6a70..c1e729d 100644
--- a/target-s390x/cpu_models.c
+++ b/target-s390x/cpu_models.c
@@ -17,7 +17,7 @@
 #include "qapi/visitor.h"
 #include "qemu/error-report.h"
 #include "qapi/qmp/qerror.h"
-#include "qapi/qmp-input-visitor.h"
+#include "qapi/qobject-input-visitor.h"
 #include "qapi/qmp/qbool.h"
 #ifndef CONFIG_USER_ONLY
 #include "sysemu/arch_init.h"
@@ -345,7 +345,7 @@ static void cpu_model_from_info(S390CPUModel *model, const CpuModelInfo *info,
     }
 
     if (qdict) {
-        visitor = qmp_input_visitor_new(info->props, true);
+        visitor = qobject_input_visitor_new(info->props, true);
         visit_start_struct(visitor, NULL, NULL, 0, errp);
         if (*errp) {
             object_unref(obj);
diff --git a/tests/.gitignore b/tests/.gitignore
index b4a9cfc..f65dac0 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -57,8 +57,8 @@ test-qht-par
 test-qmp-commands
 test-qmp-commands.h
 test-qmp-event
-test-qmp-input-strict
-test-qmp-input-visitor
+test-qobject-input-strict
+test-qobject-input-visitor
 test-qmp-introspect.[ch]
 test-qmp-marshal.c
 test-qmp-output-visitor
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 2f11064..7306196 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -24,9 +24,9 @@ check-unit-y += tests/test-qmp-output-visitor$(EXESUF)
 gcov-files-test-qmp-output-visitor-y = qapi/qmp-output-visitor.c
 check-unit-y += tests/test-clone-visitor$(EXESUF)
 gcov-files-test-clone-visitor-y = qapi/qapi-clone-visitor.c
-check-unit-y += tests/test-qmp-input-visitor$(EXESUF)
-gcov-files-test-qmp-input-visitor-y = qapi/qmp-input-visitor.c
-check-unit-y += tests/test-qmp-input-strict$(EXESUF)
+check-unit-y += tests/test-qobject-input-visitor$(EXESUF)
+gcov-files-test-qobject-input-visitor-y = qapi/qobject-input-visitor.c
+check-unit-y += tests/test-qobject-input-strict$(EXESUF)
 check-unit-y += tests/test-qmp-commands$(EXESUF)
 gcov-files-test-qmp-commands-y = qapi/qmp-dispatch.c
 check-unit-y += tests/test-string-input-visitor$(EXESUF)
@@ -437,7 +437,7 @@ test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.o \
 	tests/test-coroutine.o tests/test-string-output-visitor.o \
 	tests/test-string-input-visitor.o tests/test-qmp-output-visitor.o \
 	tests/test-clone-visitor.o \
-	tests/test-qmp-input-visitor.o tests/test-qmp-input-strict.o \
+	tests/test-qobject-input-visitor.o tests/test-qobject-input-strict.o \
 	tests/test-qmp-commands.o tests/test-visitor-serialization.o \
 	tests/test-x86-cpuid.o tests/test-mul64.o tests/test-int128.o \
 	tests/test-opts-visitor.o tests/test-qmp-event.o \
@@ -540,8 +540,8 @@ tests/test-string-input-visitor$(EXESUF): tests/test-string-input-visitor.o $(te
 tests/test-qmp-event$(EXESUF): tests/test-qmp-event.o $(test-qapi-obj-y)
 tests/test-qmp-output-visitor$(EXESUF): tests/test-qmp-output-visitor.o $(test-qapi-obj-y)
 tests/test-clone-visitor$(EXESUF): tests/test-clone-visitor.o $(test-qapi-obj-y)
-tests/test-qmp-input-visitor$(EXESUF): tests/test-qmp-input-visitor.o $(test-qapi-obj-y)
-tests/test-qmp-input-strict$(EXESUF): tests/test-qmp-input-strict.o $(test-qapi-obj-y)
+tests/test-qobject-input-visitor$(EXESUF): tests/test-qobject-input-visitor.o $(test-qapi-obj-y)
+tests/test-qobject-input-strict$(EXESUF): tests/test-qobject-input-strict.o $(test-qapi-obj-y)
 tests/test-qmp-commands$(EXESUF): tests/test-qmp-commands.o tests/test-qmp-marshal.o $(test-qapi-obj-y)
 tests/test-visitor-serialization$(EXESUF): tests/test-visitor-serialization.o $(test-qapi-obj-y)
 tests/test-opts-visitor$(EXESUF): tests/test-opts-visitor.o $(test-qapi-obj-y)
diff --git a/tests/check-qnull.c b/tests/check-qnull.c
index dc906b1..eeb803a 100644
--- a/tests/check-qnull.c
+++ b/tests/check-qnull.c
@@ -10,7 +10,7 @@
 
 #include "qapi/qmp/qobject.h"
 #include "qemu-common.h"
-#include "qapi/qmp-input-visitor.h"
+#include "qapi/qobject-input-visitor.h"
 #include "qapi/qmp-output-visitor.h"
 #include "qapi/error.h"
 
@@ -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 = qobject_input_visitor_new(obj, true);
     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 261fd9e..ffb145f 100644
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -4,7 +4,7 @@
 #include "test-qmp-commands.h"
 #include "qapi/qmp/dispatch.h"
 #include "qemu/module.h"
-#include "qapi/qmp-input-visitor.h"
+#include "qapi/qobject-input-visitor.h"
 #include "tests/test-qapi-types.h"
 #include "tests/test-qapi-visit.h"
 
@@ -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 = qobject_input_visitor_new(QOBJECT(ud2_dict), true);
         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-qobject-input-strict.c
similarity index 98%
rename from tests/test-qmp-input-strict.c
rename to tests/test-qobject-input-strict.c
index 814550a..8250365 100644
--- a/tests/test-qmp-input-strict.c
+++ b/tests/test-qobject-input-strict.c
@@ -1,5 +1,5 @@
 /*
- * QMP Input Visitor unit-tests (strict mode).
+ * QObject Input Visitor unit-tests (strict mode).
  *
  * Copyright (C) 2011-2012, 2015 Red Hat Inc.
  *
@@ -15,7 +15,7 @@
 
 #include "qemu-common.h"
 #include "qapi/error.h"
-#include "qapi/qmp-input-visitor.h"
+#include "qapi/qobject-input-visitor.h"
 #include "test-qapi-types.h"
 #include "test-qapi-visit.h"
 #include "qapi/qmp/types.h"
@@ -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 = qobject_input_visitor_new(data->obj, true);
     g_assert(data->qiv);
     return data->qiv;
 }
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qobject-input-visitor.c
similarity index 99%
rename from tests/test-qmp-input-visitor.c
rename to tests/test-qobject-input-visitor.c
index f583dce..0e65e63 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qobject-input-visitor.c
@@ -1,5 +1,5 @@
 /*
- * QMP Input Visitor unit-tests.
+ * QObject Input Visitor unit-tests.
  *
  * Copyright (C) 2011-2016 Red Hat Inc.
  *
@@ -14,7 +14,7 @@
 
 #include "qemu-common.h"
 #include "qapi/error.h"
-#include "qapi/qmp-input-visitor.h"
+#include "qapi/qobject-input-visitor.h"
 #include "test-qapi-types.h"
 #include "test-qapi-visit.h"
 #include "qapi/qmp/types.h"
@@ -49,7 +49,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 = qobject_input_visitor_new(data->obj, false);
     g_assert(data->qiv);
     return data->qiv;
 }
diff --git a/tests/test-string-input-visitor.c b/tests/test-string-input-visitor.c
index a679fbc..7f10e25 100644
--- a/tests/test-string-input-visitor.c
+++ b/tests/test-string-input-visitor.c
@@ -4,7 +4,7 @@
  * Copyright (C) 2012 Red Hat Inc.
  *
  * Authors:
- *  Paolo Bonzini <pbonzini@redhat.com> (based on test-qmp-input-visitor)
+ *  Paolo Bonzini <pbonzini@redhat.com> (based on test-qobject-input-visitor)
  *
  * This work is licensed under the terms of the GNU GPL, version 2 or later.
  * See the COPYING file in the top-level directory.
diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c
index dba4670..51df428 100644
--- a/tests/test-visitor-serialization.c
+++ b/tests/test-visitor-serialization.c
@@ -20,7 +20,7 @@
 #include "qapi/error.h"
 #include "qapi/qmp/types.h"
 #include "qapi/qmp/qjson.h"
-#include "qapi/qmp-input-visitor.h"
+#include "qapi/qobject-input-visitor.h"
 #include "qapi/qmp-output-visitor.h"
 #include "qapi/string-input-visitor.h"
 #include "qapi/string-output-visitor.h"
@@ -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 = qobject_input_visitor_new(obj, true);
     qobject_decref(obj_orig);
     qobject_decref(obj);
     visit(d->qiv, native_out, errp);
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 6db48b3..9e30a21 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -21,7 +21,7 @@
 #include "qapi/error.h"
 #include "qemu/sockets.h"
 #include "qemu/main-loop.h"
-#include "qapi/qmp-input-visitor.h"
+#include "qapi/qobject-input-visitor.h"
 #include "qapi/qmp-output-visitor.h"
 #include "qapi-visit.h"
 #include "qemu/cutils.h"
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH v13 4/6] qapi: rename QmpOutputVisitor to QObjectOutputVisitor
  2016-09-19 11:58 [Qemu-devel] [PATCH v13 0/6] QAPI/QOM work for non-scalar object properties Daniel P. Berrange
                   ` (2 preceding siblings ...)
  2016-09-19 11:58 ` [Qemu-devel] [PATCH v13 3/6] qapi: rename QmpInputVisitor to QObjectInputVisitor Daniel P. Berrange
@ 2016-09-19 11:58 ` Daniel P. Berrange
  2016-09-19 11:58 ` [Qemu-devel] [PATCH v13 5/6] qapi: add a QObjectInputVisitor that does string conversion Daniel P. Berrange
  2016-09-19 11:58 ` [Qemu-devel] [PATCH v13 6/6] qom: support arbitrary non-scalar properties with -object Daniel P. Berrange
  5 siblings, 0 replies; 18+ messages in thread
From: Daniel P. Berrange @ 2016-09-19 11:58 UTC (permalink / raw)
  To: qemu-devel
  Cc: Markus Armbruster, Max Reitz, Marc-André Lureau,
	Paolo Bonzini, Andreas Färber, Kevin Wolf,
	Daniel P. Berrange

The QmpOutputVisitor has no direct dependency on QMP. It is
valid to use it anywhere that one wants a QObject. Rename it
to better reflect its functionality as a generic QAPI
to QObject converter.

Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 block/qapi.c                                       |   4 +-
 blockdev.c                                         |   4 +-
 docs/qapi-code-gen.txt                             |   2 +-
 ...p-output-visitor.h => qobject-output-visitor.h} |  10 +-
 qapi/Makefile.objs                                 |   2 +-
 qapi/qapi-clone-visitor.c                          |   2 +-
 qapi/qmp-output-visitor.c                          | 256 ---------------------
 qapi/qobject-output-visitor.c                      | 254 ++++++++++++++++++++
 qemu-img.c                                         |   8 +-
 qom/object_interfaces.c                            |   2 +-
 qom/qom-qobject.c                                  |   4 +-
 scripts/qapi-commands.py                           |   4 +-
 scripts/qapi-event.py                              |   4 +-
 tests/.gitignore                                   |   2 +-
 tests/Makefile.include                             |   8 +-
 tests/check-qnull.c                                |   4 +-
 ...put-visitor.c => test-qobject-output-visitor.c} |   6 +-
 tests/test-string-output-visitor.c                 |   2 +-
 tests/test-visitor-serialization.c                 |   4 +-
 util/qemu-sockets.c                                |   2 +-
 20 files changed, 291 insertions(+), 293 deletions(-)
 rename include/qapi/{qmp-output-visitor.h => qobject-output-visitor.h} (66%)
 delete mode 100644 qapi/qmp-output-visitor.c
 create mode 100644 qapi/qobject-output-visitor.c
 rename tests/{test-qmp-output-visitor.c => test-qobject-output-visitor.c} (99%)

diff --git a/block/qapi.c b/block/qapi.c
index 6f947e3..f35c89f 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -29,7 +29,7 @@
 #include "block/write-threshold.h"
 #include "qmp-commands.h"
 #include "qapi-visit.h"
-#include "qapi/qmp-output-visitor.h"
+#include "qapi/qobject-output-visitor.h"
 #include "qapi/qmp/types.h"
 #include "sysemu/block-backend.h"
 #include "qemu/cutils.h"
@@ -691,7 +691,7 @@ void bdrv_image_info_specific_dump(fprintf_function func_fprintf, void *f,
                                    ImageInfoSpecific *info_spec)
 {
     QObject *obj, *data;
-    Visitor *v = qmp_output_visitor_new(&obj);
+    Visitor *v = qobject_output_visitor_new(&obj);
 
     visit_type_ImageInfoSpecific(v, NULL, &info_spec, &error_abort);
     visit_complete(v, &obj);
diff --git a/blockdev.c b/blockdev.c
index 3010393..c69dcc1 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -43,7 +43,7 @@
 #include "qapi/qmp/types.h"
 #include "qapi-visit.h"
 #include "qapi/qmp/qerror.h"
-#include "qapi/qmp-output-visitor.h"
+#include "qapi/qobject-output-visitor.h"
 #include "qapi/util.h"
 #include "sysemu/sysemu.h"
 #include "block/block_int.h"
@@ -3783,7 +3783,7 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
     BlockDriverState *bs;
     BlockBackend *blk = NULL;
     QObject *obj;
-    Visitor *v = qmp_output_visitor_new(&obj);
+    Visitor *v = qobject_output_visitor_new(&obj);
     QDict *qdict;
     Error *local_err = NULL;
 
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index a011872..dd19172 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -1005,7 +1005,7 @@ Example:
         Error *err = NULL;
         Visitor *v;
 
-        v = qmp_output_visitor_new(ret_out);
+        v = qobject_output_visitor_new(ret_out);
         visit_type_UserDefOne(v, "unused", &ret_in, &err);
         if (!err) {
             visit_complete(v, ret_out);
diff --git a/include/qapi/qmp-output-visitor.h b/include/qapi/qobject-output-visitor.h
similarity index 66%
rename from include/qapi/qmp-output-visitor.h
rename to include/qapi/qobject-output-visitor.h
index 040fdda..358c959 100644
--- a/include/qapi/qmp-output-visitor.h
+++ b/include/qapi/qobject-output-visitor.h
@@ -11,20 +11,20 @@
  *
  */
 
-#ifndef QMP_OUTPUT_VISITOR_H
-#define QMP_OUTPUT_VISITOR_H
+#ifndef QOBJECT_OUTPUT_VISITOR_H
+#define QOBJECT_OUTPUT_VISITOR_H
 
 #include "qapi/visitor.h"
 #include "qapi/qmp/qobject.h"
 
-typedef struct QmpOutputVisitor QmpOutputVisitor;
+typedef struct QObjectOutputVisitor QObjectOutputVisitor;
 
 /*
- * Create a new QMP output visitor.
+ * Create a new QOBJECT output visitor.
  *
  * If everything else succeeds, pass @result to visit_complete() to
  * collect the result of the visit.
  */
-Visitor *qmp_output_visitor_new(QObject **result);
+Visitor *qobject_output_visitor_new(QObject **result);
 
 #endif
diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs
index 6ec7bdc..33906ff 100644
--- a/qapi/Makefile.objs
+++ b/qapi/Makefile.objs
@@ -1,5 +1,5 @@
 util-obj-y = qapi-visit-core.o qapi-dealloc-visitor.o qobject-input-visitor.o
-util-obj-y += qmp-output-visitor.o qmp-registry.o qmp-dispatch.o
+util-obj-y += qobject-output-visitor.o qmp-registry.o qmp-dispatch.o
 util-obj-y += string-input-visitor.o string-output-visitor.o
 util-obj-y += opts-visitor.o qapi-clone-visitor.o
 util-obj-y += qmp-event.o
diff --git a/qapi/qapi-clone-visitor.c b/qapi/qapi-clone-visitor.c
index 0bb8216..34086cb 100644
--- a/qapi/qapi-clone-visitor.c
+++ b/qapi/qapi-clone-visitor.c
@@ -110,7 +110,7 @@ static void qapi_clone_type_str(Visitor *v, const char *name, char **obj,
     assert(qcv->depth);
     /*
      * Pointer was already cloned by g_memdup; create fresh copy.
-     * Note that as long as qmp-output-visitor accepts NULL instead of
+     * Note that as long as qobject-output-visitor accepts NULL instead of
      * "", then we must do likewise. However, we want to obey the
      * input visitor semantics of never producing NULL when the empty
      * string is intended.
diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
deleted file mode 100644
index 9e3b67c..0000000
--- a/qapi/qmp-output-visitor.c
+++ /dev/null
@@ -1,256 +0,0 @@
-/*
- * Core Definitions for QAPI/QMP Command Registry
- *
- * Copyright (C) 2012-2016 Red Hat, Inc.
- * Copyright IBM, Corp. 2011
- *
- * Authors:
- *  Anthony Liguori   <aliguori@us.ibm.com>
- *
- * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
- * See the COPYING.LIB file in the top-level directory.
- *
- */
-
-#include "qemu/osdep.h"
-#include "qapi/qmp-output-visitor.h"
-#include "qapi/visitor-impl.h"
-#include "qemu/queue.h"
-#include "qemu-common.h"
-#include "qapi/qmp/types.h"
-
-typedef struct QStackEntry
-{
-    QObject *value;
-    void *qapi; /* sanity check that caller uses same pointer */
-    QSLIST_ENTRY(QStackEntry) node;
-} QStackEntry;
-
-struct QmpOutputVisitor
-{
-    Visitor visitor;
-    QSLIST_HEAD(, QStackEntry) stack; /* Stack of unfinished containers */
-    QObject *root; /* Root of the output visit */
-    QObject **result; /* User's storage location for result */
-};
-
-#define qmp_output_add(qov, name, value) \
-    qmp_output_add_obj(qov, name, QOBJECT(value))
-#define qmp_output_push(qov, value, qapi) \
-    qmp_output_push_obj(qov, QOBJECT(value), qapi)
-
-static QmpOutputVisitor *to_qov(Visitor *v)
-{
-    return container_of(v, QmpOutputVisitor, visitor);
-}
-
-/* Push @value onto the stack of current QObjects being built */
-static void qmp_output_push_obj(QmpOutputVisitor *qov, QObject *value,
-                                void *qapi)
-{
-    QStackEntry *e = g_malloc0(sizeof(*e));
-
-    assert(qov->root);
-    assert(value);
-    e->value = value;
-    e->qapi = qapi;
-    QSLIST_INSERT_HEAD(&qov->stack, e, node);
-}
-
-/* Pop a value off the stack of QObjects being built, and return it. */
-static QObject *qmp_output_pop(QmpOutputVisitor *qov, void *qapi)
-{
-    QStackEntry *e = QSLIST_FIRST(&qov->stack);
-    QObject *value;
-
-    assert(e);
-    assert(e->qapi == qapi);
-    QSLIST_REMOVE_HEAD(&qov->stack, node);
-    value = e->value;
-    assert(value);
-    g_free(e);
-    return value;
-}
-
-/* Add @value to the current QObject being built.
- * If the stack is visiting a dictionary or list, @value is now owned
- * by that container. Otherwise, @value is now the root.  */
-static void qmp_output_add_obj(QmpOutputVisitor *qov, const char *name,
-                               QObject *value)
-{
-    QStackEntry *e = QSLIST_FIRST(&qov->stack);
-    QObject *cur = e ? e->value : NULL;
-
-    if (!cur) {
-        /* Don't allow reuse of visitor on more than one root */
-        assert(!qov->root);
-        qov->root = value;
-    } else {
-        switch (qobject_type(cur)) {
-        case QTYPE_QDICT:
-            assert(name);
-            qdict_put_obj(qobject_to_qdict(cur), name, value);
-            break;
-        case QTYPE_QLIST:
-            assert(!name);
-            qlist_append_obj(qobject_to_qlist(cur), value);
-            break;
-        default:
-            g_assert_not_reached();
-        }
-    }
-}
-
-static void qmp_output_start_struct(Visitor *v, const char *name, void **obj,
-                                    size_t unused, Error **errp)
-{
-    QmpOutputVisitor *qov = to_qov(v);
-    QDict *dict = qdict_new();
-
-    qmp_output_add(qov, name, dict);
-    qmp_output_push(qov, dict, obj);
-}
-
-static void qmp_output_end_struct(Visitor *v, void **obj)
-{
-    QmpOutputVisitor *qov = to_qov(v);
-    QObject *value = qmp_output_pop(qov, obj);
-    assert(qobject_type(value) == QTYPE_QDICT);
-}
-
-static void qmp_output_start_list(Visitor *v, const char *name,
-                                  GenericList **listp, size_t size,
-                                  Error **errp)
-{
-    QmpOutputVisitor *qov = to_qov(v);
-    QList *list = qlist_new();
-
-    qmp_output_add(qov, name, list);
-    qmp_output_push(qov, list, listp);
-}
-
-static GenericList *qmp_output_next_list(Visitor *v, GenericList *tail,
-                                         size_t size)
-{
-    return tail->next;
-}
-
-static void qmp_output_end_list(Visitor *v, void **obj)
-{
-    QmpOutputVisitor *qov = to_qov(v);
-    QObject *value = qmp_output_pop(qov, obj);
-    assert(qobject_type(value) == QTYPE_QLIST);
-}
-
-static void qmp_output_type_int64(Visitor *v, const char *name, int64_t *obj,
-                                  Error **errp)
-{
-    QmpOutputVisitor *qov = to_qov(v);
-    qmp_output_add(qov, name, qint_from_int(*obj));
-}
-
-static void qmp_output_type_uint64(Visitor *v, const char *name, uint64_t *obj,
-                                   Error **errp)
-{
-    /* FIXME: QMP outputs values larger than INT64_MAX as negative */
-    QmpOutputVisitor *qov = to_qov(v);
-    qmp_output_add(qov, name, qint_from_int(*obj));
-}
-
-static void qmp_output_type_bool(Visitor *v, const char *name, bool *obj,
-                                 Error **errp)
-{
-    QmpOutputVisitor *qov = to_qov(v);
-    qmp_output_add(qov, name, qbool_from_bool(*obj));
-}
-
-static void qmp_output_type_str(Visitor *v, const char *name, char **obj,
-                                Error **errp)
-{
-    QmpOutputVisitor *qov = to_qov(v);
-    if (*obj) {
-        qmp_output_add(qov, name, qstring_from_str(*obj));
-    } else {
-        qmp_output_add(qov, name, qstring_from_str(""));
-    }
-}
-
-static void qmp_output_type_number(Visitor *v, const char *name, double *obj,
-                                   Error **errp)
-{
-    QmpOutputVisitor *qov = to_qov(v);
-    qmp_output_add(qov, name, qfloat_from_double(*obj));
-}
-
-static void qmp_output_type_any(Visitor *v, const char *name, QObject **obj,
-                                Error **errp)
-{
-    QmpOutputVisitor *qov = to_qov(v);
-    qobject_incref(*obj);
-    qmp_output_add_obj(qov, name, *obj);
-}
-
-static void qmp_output_type_null(Visitor *v, const char *name, Error **errp)
-{
-    QmpOutputVisitor *qov = to_qov(v);
-    qmp_output_add_obj(qov, name, qnull());
-}
-
-/* Finish building, and return the root object.
- * The root object is never null. The caller becomes the object's
- * owner, and should use qobject_decref() when done with it.  */
-static void qmp_output_complete(Visitor *v, void *opaque)
-{
-    QmpOutputVisitor *qov = to_qov(v);
-
-    /* A visit must have occurred, with each start paired with end.  */
-    assert(qov->root && QSLIST_EMPTY(&qov->stack));
-    assert(opaque == qov->result);
-
-    qobject_incref(qov->root);
-    *qov->result = qov->root;
-    qov->result = NULL;
-}
-
-static void qmp_output_free(Visitor *v)
-{
-    QmpOutputVisitor *qov = to_qov(v);
-    QStackEntry *e;
-
-    while (!QSLIST_EMPTY(&qov->stack)) {
-        e = QSLIST_FIRST(&qov->stack);
-        QSLIST_REMOVE_HEAD(&qov->stack, node);
-        g_free(e);
-    }
-
-    qobject_decref(qov->root);
-    g_free(qov);
-}
-
-Visitor *qmp_output_visitor_new(QObject **result)
-{
-    QmpOutputVisitor *v;
-
-    v = g_malloc0(sizeof(*v));
-
-    v->visitor.type = VISITOR_OUTPUT;
-    v->visitor.start_struct = qmp_output_start_struct;
-    v->visitor.end_struct = qmp_output_end_struct;
-    v->visitor.start_list = qmp_output_start_list;
-    v->visitor.next_list = qmp_output_next_list;
-    v->visitor.end_list = qmp_output_end_list;
-    v->visitor.type_int64 = qmp_output_type_int64;
-    v->visitor.type_uint64 = qmp_output_type_uint64;
-    v->visitor.type_bool = qmp_output_type_bool;
-    v->visitor.type_str = qmp_output_type_str;
-    v->visitor.type_number = qmp_output_type_number;
-    v->visitor.type_any = qmp_output_type_any;
-    v->visitor.type_null = qmp_output_type_null;
-    v->visitor.complete = qmp_output_complete;
-    v->visitor.free = qmp_output_free;
-
-    *result = NULL;
-    v->result = result;
-
-    return &v->visitor;
-}
diff --git a/qapi/qobject-output-visitor.c b/qapi/qobject-output-visitor.c
new file mode 100644
index 0000000..df43cb8
--- /dev/null
+++ b/qapi/qobject-output-visitor.c
@@ -0,0 +1,254 @@
+/*
+ * Core Definitions for QAPI/QMP Command Registry
+ *
+ * Copyright (C) 2012-2016 Red Hat, Inc.
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/qobject-output-visitor.h"
+#include "qapi/visitor-impl.h"
+#include "qemu/queue.h"
+#include "qemu-common.h"
+#include "qapi/qmp/types.h"
+
+typedef struct QStackEntry {
+    QObject *value;
+    void *qapi; /* sanity check that caller uses same pointer */
+    QSLIST_ENTRY(QStackEntry) node;
+} QStackEntry;
+
+struct QObjectOutputVisitor {
+    Visitor visitor;
+    QSLIST_HEAD(, QStackEntry) stack; /* Stack of unfinished containers */
+    QObject *root; /* Root of the output visit */
+    QObject **result; /* User's storage location for result */
+};
+
+#define qobject_output_add(qov, name, value) \
+    qobject_output_add_obj(qov, name, QOBJECT(value))
+#define qobject_output_push(qov, value, qapi) \
+    qobject_output_push_obj(qov, QOBJECT(value), qapi)
+
+static QObjectOutputVisitor *to_qov(Visitor *v)
+{
+    return container_of(v, QObjectOutputVisitor, visitor);
+}
+
+/* Push @value onto the stack of current QObjects being built */
+static void qobject_output_push_obj(QObjectOutputVisitor *qov, QObject *value,
+                                    void *qapi)
+{
+    QStackEntry *e = g_malloc0(sizeof(*e));
+
+    assert(qov->root);
+    assert(value);
+    e->value = value;
+    e->qapi = qapi;
+    QSLIST_INSERT_HEAD(&qov->stack, e, node);
+}
+
+/* Pop a value off the stack of QObjects being built, and return it. */
+static QObject *qobject_output_pop(QObjectOutputVisitor *qov, void *qapi)
+{
+    QStackEntry *e = QSLIST_FIRST(&qov->stack);
+    QObject *value;
+
+    assert(e);
+    assert(e->qapi == qapi);
+    QSLIST_REMOVE_HEAD(&qov->stack, node);
+    value = e->value;
+    assert(value);
+    g_free(e);
+    return value;
+}
+
+/* Add @value to the current QObject being built.
+ * If the stack is visiting a dictionary or list, @value is now owned
+ * by that container. Otherwise, @value is now the root.  */
+static void qobject_output_add_obj(QObjectOutputVisitor *qov, const char *name,
+                                   QObject *value)
+{
+    QStackEntry *e = QSLIST_FIRST(&qov->stack);
+    QObject *cur = e ? e->value : NULL;
+
+    if (!cur) {
+        /* Don't allow reuse of visitor on more than one root */
+        assert(!qov->root);
+        qov->root = value;
+    } else {
+        switch (qobject_type(cur)) {
+        case QTYPE_QDICT:
+            assert(name);
+            qdict_put_obj(qobject_to_qdict(cur), name, value);
+            break;
+        case QTYPE_QLIST:
+            assert(!name);
+            qlist_append_obj(qobject_to_qlist(cur), value);
+            break;
+        default:
+            g_assert_not_reached();
+        }
+    }
+}
+
+static void qobject_output_start_struct(Visitor *v, const char *name,
+                                        void **obj, size_t unused, Error **errp)
+{
+    QObjectOutputVisitor *qov = to_qov(v);
+    QDict *dict = qdict_new();
+
+    qobject_output_add(qov, name, dict);
+    qobject_output_push(qov, dict, obj);
+}
+
+static void qobject_output_end_struct(Visitor *v, void **obj)
+{
+    QObjectOutputVisitor *qov = to_qov(v);
+    QObject *value = qobject_output_pop(qov, obj);
+    assert(qobject_type(value) == QTYPE_QDICT);
+}
+
+static void qobject_output_start_list(Visitor *v, const char *name,
+                                      GenericList **listp, size_t size,
+                                      Error **errp)
+{
+    QObjectOutputVisitor *qov = to_qov(v);
+    QList *list = qlist_new();
+
+    qobject_output_add(qov, name, list);
+    qobject_output_push(qov, list, listp);
+}
+
+static GenericList *qobject_output_next_list(Visitor *v, GenericList *tail,
+                                             size_t size)
+{
+    return tail->next;
+}
+
+static void qobject_output_end_list(Visitor *v, void **obj)
+{
+    QObjectOutputVisitor *qov = to_qov(v);
+    QObject *value = qobject_output_pop(qov, obj);
+    assert(qobject_type(value) == QTYPE_QLIST);
+}
+
+static void qobject_output_type_int64(Visitor *v, const char *name,
+                                      int64_t *obj, Error **errp)
+{
+    QObjectOutputVisitor *qov = to_qov(v);
+    qobject_output_add(qov, name, qint_from_int(*obj));
+}
+
+static void qobject_output_type_uint64(Visitor *v, const char *name,
+                                       uint64_t *obj, Error **errp)
+{
+    /* FIXME: QOBJECT outputs values larger than INT64_MAX as negative */
+    QObjectOutputVisitor *qov = to_qov(v);
+    qobject_output_add(qov, name, qint_from_int(*obj));
+}
+
+static void qobject_output_type_bool(Visitor *v, const char *name, bool *obj,
+                                     Error **errp)
+{
+    QObjectOutputVisitor *qov = to_qov(v);
+    qobject_output_add(qov, name, qbool_from_bool(*obj));
+}
+
+static void qobject_output_type_str(Visitor *v, const char *name, char **obj,
+                                    Error **errp)
+{
+    QObjectOutputVisitor *qov = to_qov(v);
+    if (*obj) {
+        qobject_output_add(qov, name, qstring_from_str(*obj));
+    } else {
+        qobject_output_add(qov, name, qstring_from_str(""));
+    }
+}
+
+static void qobject_output_type_number(Visitor *v, const char *name,
+                                       double *obj, Error **errp)
+{
+    QObjectOutputVisitor *qov = to_qov(v);
+    qobject_output_add(qov, name, qfloat_from_double(*obj));
+}
+
+static void qobject_output_type_any(Visitor *v, const char *name,
+                                    QObject **obj, Error **errp)
+{
+    QObjectOutputVisitor *qov = to_qov(v);
+    qobject_incref(*obj);
+    qobject_output_add_obj(qov, name, *obj);
+}
+
+static void qobject_output_type_null(Visitor *v, const char *name, Error **errp)
+{
+    QObjectOutputVisitor *qov = to_qov(v);
+    qobject_output_add_obj(qov, name, qnull());
+}
+
+/* Finish building, and return the root object.
+ * The root object is never null. The caller becomes the object's
+ * owner, and should use qobject_decref() when done with it.  */
+static void qobject_output_complete(Visitor *v, void *opaque)
+{
+    QObjectOutputVisitor *qov = to_qov(v);
+
+    /* A visit must have occurred, with each start paired with end.  */
+    assert(qov->root && QSLIST_EMPTY(&qov->stack));
+    assert(opaque == qov->result);
+
+    qobject_incref(qov->root);
+    *qov->result = qov->root;
+    qov->result = NULL;
+}
+
+static void qobject_output_free(Visitor *v)
+{
+    QObjectOutputVisitor *qov = to_qov(v);
+    QStackEntry *e;
+
+    while (!QSLIST_EMPTY(&qov->stack)) {
+        e = QSLIST_FIRST(&qov->stack);
+        QSLIST_REMOVE_HEAD(&qov->stack, node);
+        g_free(e);
+    }
+
+    qobject_decref(qov->root);
+    g_free(qov);
+}
+
+Visitor *qobject_output_visitor_new(QObject **result)
+{
+    QObjectOutputVisitor *v;
+
+    v = g_malloc0(sizeof(*v));
+
+    v->visitor.type = VISITOR_OUTPUT;
+    v->visitor.start_struct = qobject_output_start_struct;
+    v->visitor.end_struct = qobject_output_end_struct;
+    v->visitor.start_list = qobject_output_start_list;
+    v->visitor.next_list = qobject_output_next_list;
+    v->visitor.end_list = qobject_output_end_list;
+    v->visitor.type_int64 = qobject_output_type_int64;
+    v->visitor.type_uint64 = qobject_output_type_uint64;
+    v->visitor.type_bool = qobject_output_type_bool;
+    v->visitor.type_str = qobject_output_type_str;
+    v->visitor.type_number = qobject_output_type_number;
+    v->visitor.type_any = qobject_output_type_any;
+    v->visitor.type_null = qobject_output_type_null;
+    v->visitor.complete = qobject_output_complete;
+    v->visitor.free = qobject_output_free;
+
+    *result = NULL;
+    v->result = result;
+
+    return &v->visitor;
+}
diff --git a/qemu-img.c b/qemu-img.c
index ea52486..f4589f1 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -25,7 +25,7 @@
 #include "qemu-version.h"
 #include "qapi/error.h"
 #include "qapi-visit.h"
-#include "qapi/qmp-output-visitor.h"
+#include "qapi/qobject-output-visitor.h"
 #include "qapi/qmp/qerror.h"
 #include "qapi/qmp/qjson.h"
 #include "qemu/cutils.h"
@@ -492,7 +492,7 @@ static void dump_json_image_check(ImageCheck *check, bool quiet)
 {
     QString *str;
     QObject *obj;
-    Visitor *v = qmp_output_visitor_new(&obj);
+    Visitor *v = qobject_output_visitor_new(&obj);
 
     visit_type_ImageCheck(v, NULL, &check, &error_abort);
     visit_complete(v, &obj);
@@ -2185,7 +2185,7 @@ static void dump_json_image_info_list(ImageInfoList *list)
 {
     QString *str;
     QObject *obj;
-    Visitor *v = qmp_output_visitor_new(&obj);
+    Visitor *v = qobject_output_visitor_new(&obj);
 
     visit_type_ImageInfoList(v, NULL, &list, &error_abort);
     visit_complete(v, &obj);
@@ -2201,7 +2201,7 @@ static void dump_json_image_info(ImageInfo *info)
 {
     QString *str;
     QObject *obj;
-    Visitor *v = qmp_output_visitor_new(&obj);
+    Visitor *v = qobject_output_visitor_new(&obj);
 
     visit_type_ImageInfo(v, NULL, &info, &error_abort);
     visit_complete(v, &obj);
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index bf59846..ded4d84 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -3,7 +3,7 @@
 #include "qom/object_interfaces.h"
 #include "qemu/module.h"
 #include "qapi-visit.h"
-#include "qapi/qmp-output-visitor.h"
+#include "qapi/qobject-output-visitor.h"
 #include "qapi/opts-visitor.h"
 
 void user_creatable_complete(Object *obj, Error **errp)
diff --git a/qom/qom-qobject.c b/qom/qom-qobject.c
index 81959e0..447e4a0 100644
--- a/qom/qom-qobject.c
+++ b/qom/qom-qobject.c
@@ -16,7 +16,7 @@
 #include "qom/qom-qobject.h"
 #include "qapi/visitor.h"
 #include "qapi/qobject-input-visitor.h"
-#include "qapi/qmp-output-visitor.h"
+#include "qapi/qobject-output-visitor.h"
 
 void object_property_set_qobject(Object *obj, QObject *value,
                                  const char *name, Error **errp)
@@ -35,7 +35,7 @@ QObject *object_property_get_qobject(Object *obj, const char *name,
     Error *local_err = NULL;
     Visitor *v;
 
-    v = qmp_output_visitor_new(&ret);
+    v = qobject_output_visitor_new(&ret);
     object_property_get(obj, v, name, &local_err);
     if (!local_err) {
         visit_complete(v, &ret);
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 3010163..bf8372f 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -68,7 +68,7 @@ static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in, QObject **ret_out,
     Error *err = NULL;
     Visitor *v;
 
-    v = qmp_output_visitor_new(ret_out);
+    v = qobject_output_visitor_new(ret_out);
     visit_type_%(c_name)s(v, "unused", &ret_in, &err);
     if (!err) {
         visit_complete(v, ret_out);
@@ -278,7 +278,7 @@ fdef.write(mcgen('''
 #include "qapi/qmp/types.h"
 #include "qapi/qmp/dispatch.h"
 #include "qapi/visitor.h"
-#include "qapi/qmp-output-visitor.h"
+#include "qapi/qobject-output-visitor.h"
 #include "qapi/qobject-input-visitor.h"
 #include "qapi/dealloc-visitor.h"
 #include "%(prefix)sqapi-types.h"
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index 38d8211..f4eb7f8 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -98,7 +98,7 @@ def gen_event_send(name, arg_type, boxed):
 
     if arg_type and not arg_type.is_empty():
         ret += mcgen('''
-    v = qmp_output_visitor_new(&obj);
+    v = qobject_output_visitor_new(&obj);
 ''')
         if not arg_type.is_implicit():
             ret += mcgen('''
@@ -209,7 +209,7 @@ fdef.write(mcgen('''
 #include "qemu-common.h"
 #include "%(prefix)sqapi-event.h"
 #include "%(prefix)sqapi-visit.h"
-#include "qapi/qmp-output-visitor.h"
+#include "qapi/qobject-output-visitor.h"
 #include "qapi/qmp-event.h"
 
 ''',
diff --git a/tests/.gitignore b/tests/.gitignore
index f65dac0..b850b24 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -61,7 +61,7 @@ test-qobject-input-strict
 test-qobject-input-visitor
 test-qmp-introspect.[ch]
 test-qmp-marshal.c
-test-qmp-output-visitor
+test-qobject-output-visitor
 test-rcu-list
 test-replication
 test-rfifolock
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 7306196..b770bc2 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -20,8 +20,8 @@ check-unit-y += tests/check-qnull$(EXESUF)
 gcov-files-check-qnull-y = qobject/qnull.c
 check-unit-y += tests/check-qjson$(EXESUF)
 gcov-files-check-qjson-y = qobject/qjson.c
-check-unit-y += tests/test-qmp-output-visitor$(EXESUF)
-gcov-files-test-qmp-output-visitor-y = qapi/qmp-output-visitor.c
+check-unit-y += tests/test-qobject-output-visitor$(EXESUF)
+gcov-files-test-qobject-output-visitor-y = qapi/qobject-output-visitor.c
 check-unit-y += tests/test-clone-visitor$(EXESUF)
 gcov-files-test-clone-visitor-y = qapi/qapi-clone-visitor.c
 check-unit-y += tests/test-qobject-input-visitor$(EXESUF)
@@ -435,7 +435,7 @@ test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.o \
 	tests/check-qlist.o tests/check-qfloat.o tests/check-qnull.o \
 	tests/check-qjson.o \
 	tests/test-coroutine.o tests/test-string-output-visitor.o \
-	tests/test-string-input-visitor.o tests/test-qmp-output-visitor.o \
+	tests/test-string-input-visitor.o tests/test-qobject-output-visitor.o \
 	tests/test-clone-visitor.o \
 	tests/test-qobject-input-visitor.o tests/test-qobject-input-strict.o \
 	tests/test-qmp-commands.o tests/test-visitor-serialization.o \
@@ -538,7 +538,7 @@ $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-int
 tests/test-string-output-visitor$(EXESUF): tests/test-string-output-visitor.o $(test-qapi-obj-y)
 tests/test-string-input-visitor$(EXESUF): tests/test-string-input-visitor.o $(test-qapi-obj-y)
 tests/test-qmp-event$(EXESUF): tests/test-qmp-event.o $(test-qapi-obj-y)
-tests/test-qmp-output-visitor$(EXESUF): tests/test-qmp-output-visitor.o $(test-qapi-obj-y)
+tests/test-qobject-output-visitor$(EXESUF): tests/test-qobject-output-visitor.o $(test-qapi-obj-y)
 tests/test-clone-visitor$(EXESUF): tests/test-clone-visitor.o $(test-qapi-obj-y)
 tests/test-qobject-input-visitor$(EXESUF): tests/test-qobject-input-visitor.o $(test-qapi-obj-y)
 tests/test-qobject-input-strict$(EXESUF): tests/test-qobject-input-strict.o $(test-qapi-obj-y)
diff --git a/tests/check-qnull.c b/tests/check-qnull.c
index eeb803a..b50bb8a 100644
--- a/tests/check-qnull.c
+++ b/tests/check-qnull.c
@@ -11,7 +11,7 @@
 #include "qapi/qmp/qobject.h"
 #include "qemu-common.h"
 #include "qapi/qobject-input-visitor.h"
-#include "qapi/qmp-output-visitor.h"
+#include "qapi/qobject-output-visitor.h"
 #include "qapi/error.h"
 
 /*
@@ -52,7 +52,7 @@ static void qnull_visit_test(void)
     visit_type_null(v, NULL, &error_abort);
     visit_free(v);
 
-    v = qmp_output_visitor_new(&obj);
+    v = qobject_output_visitor_new(&obj);
     visit_type_null(v, NULL, &error_abort);
     visit_complete(v, &obj);
     g_assert(obj == &qnull_);
diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qobject-output-visitor.c
similarity index 99%
rename from tests/test-qmp-output-visitor.c
rename to tests/test-qobject-output-visitor.c
index 513d71f..4e2d79c 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qobject-output-visitor.c
@@ -1,5 +1,5 @@
 /*
- * QMP Output Visitor unit-tests.
+ * QObject Output Visitor unit-tests.
  *
  * Copyright (C) 2011-2016 Red Hat Inc.
  *
@@ -14,7 +14,7 @@
 
 #include "qemu-common.h"
 #include "qapi/error.h"
-#include "qapi/qmp-output-visitor.h"
+#include "qapi/qobject-output-visitor.h"
 #include "test-qapi-types.h"
 #include "test-qapi-visit.h"
 #include "qapi/qmp/types.h"
@@ -28,7 +28,7 @@ typedef struct TestOutputVisitorData {
 static void visitor_output_setup(TestOutputVisitorData *data,
                                  const void *unused)
 {
-    data->ov = qmp_output_visitor_new(&data->obj);
+    data->ov = qobject_output_visitor_new(&data->obj);
     g_assert(data->ov);
 }
 
diff --git a/tests/test-string-output-visitor.c b/tests/test-string-output-visitor.c
index 444844a..e736db3 100644
--- a/tests/test-string-output-visitor.c
+++ b/tests/test-string-output-visitor.c
@@ -4,7 +4,7 @@
  * Copyright (C) 2012 Red Hat Inc.
  *
  * Authors:
- *  Paolo Bonzini <pbonzini@redhat.com> (based on test-qmp-output-visitor)
+ *  Paolo Bonzini <pbonzini@redhat.com> (based on test-qobject-output-visitor)
  *
  * This work is licensed under the terms of the GNU GPL, version 2 or later.
  * See the COPYING file in the top-level directory.
diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c
index 51df428..66b2b1c 100644
--- a/tests/test-visitor-serialization.c
+++ b/tests/test-visitor-serialization.c
@@ -21,7 +21,7 @@
 #include "qapi/qmp/types.h"
 #include "qapi/qmp/qjson.h"
 #include "qapi/qobject-input-visitor.h"
-#include "qapi/qmp-output-visitor.h"
+#include "qapi/qobject-output-visitor.h"
 #include "qapi/string-input-visitor.h"
 #include "qapi/string-output-visitor.h"
 #include "qapi-types.h"
@@ -1022,7 +1022,7 @@ static void qmp_serialize(void *native_in, void **datap,
 {
     QmpSerializeData *d = g_malloc0(sizeof(*d));
 
-    d->qov = qmp_output_visitor_new(&d->obj);
+    d->qov = qobject_output_visitor_new(&d->obj);
     visit(d->qov, &native_in, errp);
     *datap = d;
 }
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 9e30a21..4cef549 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -22,7 +22,7 @@
 #include "qemu/sockets.h"
 #include "qemu/main-loop.h"
 #include "qapi/qobject-input-visitor.h"
-#include "qapi/qmp-output-visitor.h"
+#include "qapi/qobject-output-visitor.h"
 #include "qapi-visit.h"
 #include "qemu/cutils.h"
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH v13 5/6] qapi: add a QObjectInputVisitor that does string conversion
  2016-09-19 11:58 [Qemu-devel] [PATCH v13 0/6] QAPI/QOM work for non-scalar object properties Daniel P. Berrange
                   ` (3 preceding siblings ...)
  2016-09-19 11:58 ` [Qemu-devel] [PATCH v13 4/6] qapi: rename QmpOutputVisitor to QObjectOutputVisitor Daniel P. Berrange
@ 2016-09-19 11:58 ` Daniel P. Berrange
  2016-09-19 11:58 ` [Qemu-devel] [PATCH v13 6/6] qom: support arbitrary non-scalar properties with -object Daniel P. Berrange
  5 siblings, 0 replies; 18+ messages in thread
From: Daniel P. Berrange @ 2016-09-19 11:58 UTC (permalink / raw)
  To: qemu-devel
  Cc: Markus Armbruster, Max Reitz, Marc-André Lureau,
	Paolo Bonzini, Andreas Färber, Kevin Wolf,
	Daniel P. Berrange

Currently the QObjectInputVisitor 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 constructor for QObjectInputVisitor
that will set it up such that it expects a QString for
all scalar types instead.

This makes it possible to use QObjectInputVisitor with a
QDict produced from QemuOpts, where everything is in
string format.

Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 include/qapi/qobject-input-visitor.h |  32 +++++-
 qapi/qobject-input-visitor.c         | 128 ++++++++++++++++++++++
 tests/test-qobject-input-visitor.c   | 203 ++++++++++++++++++++++++++++++++++-
 3 files changed, 355 insertions(+), 8 deletions(-)

diff --git a/include/qapi/qobject-input-visitor.h b/include/qapi/qobject-input-visitor.h
index cde328d..a17a453 100644
--- a/include/qapi/qobject-input-visitor.h
+++ b/include/qapi/qobject-input-visitor.h
@@ -19,12 +19,36 @@
 
 typedef struct QObjectInputVisitor QObjectInputVisitor;
 
-/*
- * Return a new input visitor that converts a QObject to a QAPI object.
+/**
+ * Create a new input visitor that converts @obj to a QAPI object.
+ *
+ * Any scalar values in the @obj input data structure should be in the
+ * required type already. i.e. if visiting a bool, the value should
+ * already be a QBool instance.
  *
- * Set @strict to reject a parse that doesn't consume all keys of a
- * dictionary; otherwise excess input is ignored.
+ * If @strict is set to true, then an error will be reported if any
+ * dict keys are not consumed during visitation. If @strict is false
+ * then extra dict keys are silently ignored.
+ *
+ * The returned input visitor should be released by calling
+ * visit_free() when no longer required.
  */
 Visitor *qobject_input_visitor_new(QObject *obj, bool strict);
 
+/**
+ * Create a new input visitor that converts a QObject to a QAPI object.
+ *
+ * Any scalar values in the @obj input data structure should always be
+ * represented as strings. i.e. if visiting a boolean, the value should
+ * be a QString whose contents represent a valid boolean.
+ *
+ * The visitor always operates in strict mode, requiring all dict keys
+ * to be consumed during visitation. An error will be reported if this
+ * does not happen.
+ *
+ * The returned input visitor should be released by calling
+ * visit_free() when no longer required.
+ */
+Visitor *qobject_string_input_visitor_new(QObject *obj);
+
 #endif
diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index 5ff3db3..07ce525 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-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
 
@@ -263,6 +264,29 @@ static void qobject_input_type_int64(Visitor *v, const char *name, int64_t *obj,
     *obj = qint_get_int(qint);
 }
 
+static void qobject_input_type_int64_str(Visitor *v, const char *name,
+                                         int64_t *obj, Error **errp)
+{
+    QObjectInputVisitor *qiv = to_qiv(v);
+    QString *qstr = qobject_to_qstring(qobject_input_get_object(qiv, name,
+                                                                true));
+    uint64_t ret;
+    Error *local_err = NULL;
+
+    if (!qstr || !qstr->string) {
+        error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
+                   "string");
+        return;
+    }
+
+    parse_option_number(name, qstr->string, &ret, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+    } else {
+        *obj = ret;
+    }
+}
+
 static void qobject_input_type_uint64(Visitor *v, const char *name,
                                       uint64_t *obj, Error **errp)
 {
@@ -279,6 +303,22 @@ static void qobject_input_type_uint64(Visitor *v, const char *name,
     *obj = qint_get_int(qint);
 }
 
+static void qobject_input_type_uint64_str(Visitor *v, const char *name,
+                                          uint64_t *obj, Error **errp)
+{
+    QObjectInputVisitor *qiv = to_qiv(v);
+    QString *qstr = qobject_to_qstring(qobject_input_get_object(qiv, name,
+                                                                true));
+
+    if (!qstr || !qstr->string) {
+        error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
+                   "string");
+        return;
+    }
+
+    parse_option_number(name, qstr->string, obj, errp);
+}
+
 static void qobject_input_type_bool(Visitor *v, const char *name, bool *obj,
                                     Error **errp)
 {
@@ -294,6 +334,22 @@ static void qobject_input_type_bool(Visitor *v, const char *name, bool *obj,
     *obj = qbool_get_bool(qbool);
 }
 
+static void qobject_input_type_bool_str(Visitor *v, const char *name, bool *obj,
+                                        Error **errp)
+{
+    QObjectInputVisitor *qiv = to_qiv(v);
+    QString *qstr = qobject_to_qstring(qobject_input_get_object(qiv, name,
+                                                                true));
+
+    if (!qstr || !qstr->string) {
+        error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
+                   "string");
+        return;
+    }
+
+    parse_option_bool(name, qstr->string, obj, errp);
+}
+
 static void qobject_input_type_str(Visitor *v, const char *name, char **obj,
                                    Error **errp)
 {
@@ -335,6 +391,30 @@ static void qobject_input_type_number(Visitor *v, const char *name, double *obj,
                "number");
 }
 
+static void qobject_input_type_number_str(Visitor *v, const char *name,
+                                          double *obj, Error **errp)
+{
+    QObjectInputVisitor *qiv = to_qiv(v);
+    QString *qstr = qobject_to_qstring(qobject_input_get_object(qiv, name,
+                                                                true));
+    char *endp;
+
+    if (!qstr || !qstr->string) {
+        error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
+                   "string");
+        return;
+    }
+
+    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");
+}
+
 static void qobject_input_type_any(Visitor *v, const char *name, QObject **obj,
                                    Error **errp)
 {
@@ -356,6 +436,22 @@ static void qobject_input_type_null(Visitor *v, const char *name, Error **errp)
     }
 }
 
+static void qobject_input_type_size_str(Visitor *v, const char *name,
+                                        uint64_t *obj, Error **errp)
+{
+    QObjectInputVisitor *qiv = to_qiv(v);
+    QString *qstr = qobject_to_qstring(qobject_input_get_object(qiv, name,
+                                                                true));
+
+    if (!qstr || !qstr->string) {
+        error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
+                   "string");
+        return;
+    }
+
+    parse_option_size(name, qstr->string, obj, errp);
+}
+
 static void qobject_input_optional(Visitor *v, const char *name, bool *present)
 {
     QObjectInputVisitor *qiv = to_qiv(v);
@@ -413,3 +509,35 @@ Visitor *qobject_input_visitor_new(QObject *obj, bool strict)
 
     return &v->visitor;
 }
+
+Visitor *qobject_string_input_visitor_new(QObject *obj)
+{
+    QObjectInputVisitor *v;
+
+    v = g_malloc0(sizeof(*v));
+
+    v->visitor.type = VISITOR_INPUT;
+    v->visitor.start_struct = qobject_input_start_struct;
+    v->visitor.check_struct = qobject_input_check_struct;
+    v->visitor.end_struct = qobject_input_pop;
+    v->visitor.start_list = qobject_input_start_list;
+    v->visitor.next_list = qobject_input_next_list;
+    v->visitor.end_list = qobject_input_pop;
+    v->visitor.start_alternate = qobject_input_start_alternate;
+    v->visitor.type_int64 = qobject_input_type_int64_str;
+    v->visitor.type_uint64 = qobject_input_type_uint64_str;
+    v->visitor.type_bool = qobject_input_type_bool_str;
+    v->visitor.type_str = qobject_input_type_str;
+    v->visitor.type_number = qobject_input_type_number_str;
+    v->visitor.type_any = qobject_input_type_any;
+    v->visitor.type_null = qobject_input_type_null;
+    v->visitor.type_size = qobject_input_type_size_str;
+    v->visitor.optional = qobject_input_optional;
+    v->visitor.free = qobject_input_free;
+    v->strict = true;
+
+    v->root = obj;
+    qobject_incref(obj);
+
+    return &v->visitor;
+}
diff --git a/tests/test-qobject-input-visitor.c b/tests/test-qobject-input-visitor.c
index 0e65e63..5d0c2bd 100644
--- a/tests/test-qobject-input-visitor.c
+++ b/tests/test-qobject-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,31 @@ static Visitor *visitor_input_test_init_internal(TestInputVisitorData *data,
     data->obj = qobject_from_jsonv(json_string, ap);
     g_assert(data->obj);
 
-    data->qiv = qobject_input_visitor_new(data->obj, false);
+    if (autocast) {
+        assert(strict);
+        data->qiv = qobject_string_input_visitor_new(data->obj);
+    } else {
+        data->qiv = qobject_input_visitor_new(data->obj, strict);
+    }
     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 +83,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, true, false,
+                                         json_string, &ap);
     va_end(ap);
     return v;
 }
@@ -77,7 +99,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, true, false,
+                                            json_string, NULL);
 }
 
 static void test_visitor_in_int(TestInputVisitorData *data,
@@ -109,6 +132,45 @@ 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;
+    Error *err = NULL;
+    Visitor *v;
+
+    v = visitor_input_test_init_full(data, true, true,
+                                     "%" PRId64, value);
+    visit_type_int(v, NULL, &res, &err);
+    error_free_or_abort(&err);
+}
+
+static void test_visitor_in_int_str_autocast(TestInputVisitorData *data,
+                                             const void *unused)
+{
+    int64_t res = 0, value = -42;
+    Visitor *v;
+
+    v = visitor_input_test_init_full(data, true, true,
+                                     "\"-42\"");
+
+    visit_type_int(v, NULL, &res, &error_abort);
+    g_assert_cmpint(res, ==, value);
+}
+
+static void test_visitor_in_int_str_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);
+    error_free_or_abort(&err);
+}
+
 static void test_visitor_in_bool(TestInputVisitorData *data,
                                  const void *unused)
 {
@@ -121,6 +183,44 @@ 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;
+    Error *err = NULL;
+    Visitor *v;
+
+    v = visitor_input_test_init_full(data, true, true, "true");
+
+    visit_type_bool(v, NULL, &res, &err);
+    error_free_or_abort(&err);
+}
+
+static void test_visitor_in_bool_str_autocast(TestInputVisitorData *data,
+                                              const void *unused)
+{
+    bool res = false;
+    Visitor *v;
+
+    v = visitor_input_test_init_full(data, true, true, "\"yes\"");
+
+    visit_type_bool(v, NULL, &res, &error_abort);
+    g_assert_cmpint(res, ==, true);
+}
+
+static void test_visitor_in_bool_str_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);
+    error_free_or_abort(&err);
+}
+
 static void test_visitor_in_number(TestInputVisitorData *data,
                                    const void *unused)
 {
@@ -133,6 +233,69 @@ 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;
+    Error *err = NULL;
+    Visitor *v;
+
+    v = visitor_input_test_init_full(data, true, true, "%f", value);
+
+    visit_type_number(v, NULL, &res, &err);
+    error_free_or_abort(&err);
+}
+
+static void test_visitor_in_number_str_autocast(TestInputVisitorData *data,
+                                                const void *unused)
+{
+    double res = 0, value = 3.14;
+    Visitor *v;
+
+    v = visitor_input_test_init_full(data, true, true, "\"3.14\"");
+
+    visit_type_number(v, NULL, &res, &error_abort);
+    g_assert_cmpfloat(res, ==, value);
+}
+
+static void test_visitor_in_number_str_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);
+    error_free_or_abort(&err);
+}
+
+static void test_visitor_in_size_str_autocast(TestInputVisitorData *data,
+                                              const void *unused)
+{
+    uint64_t res, value = 500 * 1024 * 1024;
+    Visitor *v;
+
+    v = visitor_input_test_init_full(data, true, true, "\"500M\"");
+
+    visit_type_size(v, NULL, &res, &error_abort);
+    g_assert_cmpfloat(res, ==, value);
+}
+
+static void test_visitor_in_size_str_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);
+    error_free_or_abort(&err);
+}
+
 static void test_visitor_in_string(TestInputVisitorData *data,
                                    const void *unused)
 {
@@ -289,7 +452,8 @@ static void test_visitor_in_null(TestInputVisitorData *data,
      * when input is not null.
      */
 
-    v = visitor_input_test_init(data, "{ 'a': null, 'b': '' }");
+    v = visitor_input_test_init_full(data, false, false,
+                                     "{ 'a': null, 'b': '' }");
     visit_start_struct(v, NULL, NULL, 0, &error_abort);
     visit_type_null(v, "a", &error_abort);
     visit_type_str(v, "a", &tmp, &err);
@@ -841,10 +1005,41 @@ 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_str_autocast",
+                           &in_visitor_data, test_visitor_in_int_str_autocast);
+    input_visitor_test_add("/visitor/input/int_str_noautocast",
+                           &in_visitor_data,
+                           test_visitor_in_int_str_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_str_autocast",
+                           &in_visitor_data,
+                           test_visitor_in_bool_str_autocast);
+    input_visitor_test_add("/visitor/input/bool_str_noautocast",
+                           &in_visitor_data,
+                           test_visitor_in_bool_str_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_str_autocast",
+                           &in_visitor_data,
+                           test_visitor_in_number_str_autocast);
+    input_visitor_test_add("/visitor/input/number_str_noautocast",
+                           &in_visitor_data,
+                           test_visitor_in_number_str_noautocast);
+    input_visitor_test_add("/visitor/input/size_str_autocast",
+                           &in_visitor_data,
+                           test_visitor_in_size_str_autocast);
+    input_visitor_test_add("/visitor/input/size_str_noautocast",
+                           &in_visitor_data,
+                           test_visitor_in_size_str_noautocast);
     input_visitor_test_add("/visitor/input/string",
                            &in_visitor_data, test_visitor_in_string);
     input_visitor_test_add("/visitor/input/enum",
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH v13 6/6] qom: support arbitrary non-scalar properties with -object
  2016-09-19 11:58 [Qemu-devel] [PATCH v13 0/6] QAPI/QOM work for non-scalar object properties Daniel P. Berrange
                   ` (4 preceding siblings ...)
  2016-09-19 11:58 ` [Qemu-devel] [PATCH v13 5/6] qapi: add a QObjectInputVisitor that does string conversion Daniel P. Berrange
@ 2016-09-19 11:58 ` Daniel P. Berrange
  2016-09-19 12:12   ` Daniel P. Berrange
  5 siblings, 1 reply; 18+ messages in thread
From: Daniel P. Berrange @ 2016-09-19 11:58 UTC (permalink / raw)
  To: qemu-devel
  Cc: Markus Armbruster, Max Reitz, Marc-André Lureau,
	Paolo Bonzini, Andreas Färber, Kevin Wolf,
	Daniel P. Berrange

The current -object command line syntax only allows for
creation of objects with scalar properties, or a list
with a fixed scalar element type. Objects which have
properties that are represented as structs in the QAPI
schema cannot be created using -object.

This is a design limitation of the way the OptsVisitor
is written. It simply iterates over the QemuOpts values
as a flat list. The support for lists is enabled by
allowing the same key to be repeated in the opts string.

It is not practical to extend the OptsVisitor to support
more complex data structures while also maintaining
the existing list handling behaviour that is relied upon
by other areas of QEMU.

Fortunately there is no existing object that implements
the UserCreatable interface that relies on the list
handling behaviour, so it is possible to swap out the
OptsVisitor for a different visitor implementation, so
-object supports non-scalar properties, thus leaving
other users of OptsVisitor unaffected.

The previously added qdict_crumple() method is able to
take a qdict containing a flat set of properties and
turn that into a arbitrarily nested set of dicts and
lists. By combining qemu_opts_to_qdict and qdict_crumple()
together, we can turn the opt string into a data structure
that is practically identical to that passed over QMP
when defining an object. The only difference is that all
the scalar values are represented as strings, rather than
strings, ints and bools. This is sufficient to let us
replace the OptsVisitor with the QObjectInputVisitor for
use with -object.

Thus -object can now support non-scalar properties,
for example the QMP object

  {
    "execute": "object-add",
    "arguments": {
      "qom-type": "demo",
      "id": "demo0",
      "parameters": {
        "foo": [
          { "bar": "one", "wizz": "1" },
          { "bar": "two", "wizz": "2" }
        ]
      }
    }
  }

Would be creatable via the CLI now using

    $QEMU \
      -object demo,id=demo0,\
              foo.0.bar=one,foo.0.wizz=1,\
              foo.1.bar=two,foo.1.wizz=2

Notice that this syntax is intentionally compatible
with that currently used by block drivers.

This is also wired up to work for the 'object_add' command
in the HMP monitor with the same syntax.

  (hmp) object_add demo,id=demo0,\
                   foo.0.bar=one,foo.0.wizz=1,\
                   foo.1.bar=two,foo.1.wizz=2

NB indentation should not be used with HMP commands, this
is just for convenient formatting in this commit message.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 hmp.c                           |  16 +-
 include/qom/object_interfaces.h |  10 +-
 qmp.c                           |   2 +-
 qom/object_interfaces.c         |  45 ++++--
 tests/check-qom-proplist.c      | 334 +++++++++++++++++++++++++++++++++++++++-
 5 files changed, 378 insertions(+), 29 deletions(-)

diff --git a/hmp.c b/hmp.c
index ad33b44..834b6ae 100644
--- a/hmp.c
+++ b/hmp.c
@@ -25,7 +25,7 @@
 #include "qemu/sockets.h"
 #include "monitor/monitor.h"
 #include "monitor/qdev.h"
-#include "qapi/opts-visitor.h"
+#include "qapi/qobject-input-visitor.h"
 #include "qapi/qmp/qerror.h"
 #include "qapi/string-output-visitor.h"
 #include "qapi/util.h"
@@ -1695,20 +1695,14 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
 void hmp_object_add(Monitor *mon, const QDict *qdict)
 {
     Error *err = NULL;
-    QemuOpts *opts;
     Visitor *v;
     Object *obj = NULL;
+    QDict *pdict = qdict_clone_shallow(qdict);
 
-    opts = qemu_opts_from_qdict(qemu_find_opts("object"), qdict, &err);
-    if (err) {
-        hmp_handle_error(mon, &err);
-        return;
-    }
-
-    v = opts_visitor_new(opts);
-    obj = user_creatable_add(qdict, v, &err);
+    v = qobject_string_input_visitor_new(QOBJECT(pdict));
+    obj = user_creatable_add(pdict, v, &err);
     visit_free(v);
-    qemu_opts_del(opts);
+    QDECREF(pdict);
 
     if (err) {
         hmp_handle_error(mon, &err);
diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h
index 8b17f4d..2466e53 100644
--- a/include/qom/object_interfaces.h
+++ b/include/qom/object_interfaces.h
@@ -96,18 +96,24 @@ Object *user_creatable_add(const QDict *qdict,
  * user_creatable_add_type:
  * @type: the object type name
  * @id: the unique ID for the object
+ * @nested: whether to recurse into the visitor for properties
  * @qdict: the object properties
  * @v: the visitor
  * @errp: if an error occurs, a pointer to an area to store the error
  *
  * Create an instance of the user creatable object @type, placing
  * it in the object composition tree with name @id, initializing
- * it with properties from @qdict
+ * it with properties from @qdict.
+ *
+ * If the visitor is already positioned to read the properties
+ * in @qdict, @nested should be false. Conversely, if it is
+ * necessary to open/close a struct to read the properties in
+ * @qdict, @nested should be true.
  *
  * Returns: the newly created object or NULL on error
  */
 Object *user_creatable_add_type(const char *type, const char *id,
-                                const QDict *qdict,
+                                bool nested, const QDict *qdict,
                                 Visitor *v, Error **errp);
 
 /**
diff --git a/qmp.c b/qmp.c
index 8786c1b..a074224 100644
--- a/qmp.c
+++ b/qmp.c
@@ -688,7 +688,7 @@ void qmp_object_add(const char *type, const char *id,
     }
 
     v = qobject_input_visitor_new(props, true);
-    obj = user_creatable_add_type(type, id, pdict, v, errp);
+    obj = user_creatable_add_type(type, id, true, pdict, v, errp);
     visit_free(v);
     if (obj) {
         object_unref(obj);
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index ded4d84..a68c39d 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -4,6 +4,7 @@
 #include "qemu/module.h"
 #include "qapi-visit.h"
 #include "qapi/qobject-output-visitor.h"
+#include "qapi/qobject-input-visitor.h"
 #include "qapi/opts-visitor.h"
 
 void user_creatable_complete(Object *obj, Error **errp)
@@ -63,12 +64,16 @@ Object *user_creatable_add(const QDict *qdict,
     if (local_err) {
         goto out_visit;
     }
-    visit_check_struct(v, &local_err);
+
+    obj = user_creatable_add_type(type, id, false, pdict, v, &local_err);
     if (local_err) {
         goto out_visit;
     }
 
-    obj = user_creatable_add_type(type, id, pdict, v, &local_err);
+    visit_check_struct(v, &local_err);
+    if (local_err) {
+        goto out_visit;
+    }
 
 out_visit:
     visit_end_struct(v, NULL);
@@ -87,7 +92,7 @@ out:
 
 
 Object *user_creatable_add_type(const char *type, const char *id,
-                                const QDict *qdict,
+                                bool nested, const QDict *qdict,
                                 Visitor *v, Error **errp)
 {
     Object *obj;
@@ -114,9 +119,11 @@ Object *user_creatable_add_type(const char *type, const char *id,
 
     assert(qdict);
     obj = object_new(type);
-    visit_start_struct(v, NULL, NULL, 0, &local_err);
-    if (local_err) {
-        goto out;
+    if (nested) {
+        visit_start_struct(v, NULL, NULL, 0, &local_err);
+        if (local_err) {
+            goto out;
+        }
     }
     for (e = qdict_first(qdict); e; e = qdict_next(qdict, e)) {
         object_property_set(obj, v, e->key, &local_err);
@@ -124,12 +131,14 @@ Object *user_creatable_add_type(const char *type, const char *id,
             break;
         }
     }
-    if (!local_err) {
-        visit_check_struct(v, &local_err);
-    }
-    visit_end_struct(v, NULL);
-    if (local_err) {
-        goto out;
+    if (nested) {
+        if (!local_err) {
+            visit_check_struct(v, &local_err);
+        }
+        visit_end_struct(v, NULL);
+        if (local_err) {
+            goto out;
+        }
     }
 
     object_property_add_child(object_get_objects_root(),
@@ -158,13 +167,21 @@ Object *user_creatable_add_opts(QemuOpts *opts, Error **errp)
 {
     Visitor *v;
     QDict *pdict;
+    QObject *pobj;
     Object *obj = NULL;
 
-    v = opts_visitor_new(opts);
     pdict = qemu_opts_to_qdict(opts, NULL);
 
-    obj = user_creatable_add(pdict, v, errp);
+    pobj = qdict_crumple(pdict, true, errp);
+    if (!pobj) {
+        goto cleanup;
+    }
+    v = qobject_string_input_visitor_new(pobj);
+
+    obj = user_creatable_add(qobject_to_qdict(pobj), v, errp);
     visit_free(v);
+    qobject_decref(pobj);
+ cleanup:
     QDECREF(pdict);
     return obj;
 }
diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
index a16cefc..0bff55f 100644
--- a/tests/check-qom-proplist.c
+++ b/tests/check-qom-proplist.c
@@ -23,6 +23,14 @@
 #include "qapi/error.h"
 #include "qom/object.h"
 #include "qemu/module.h"
+#include "qapi/visitor.h"
+#include "qom/object_interfaces.h"
+#include "qemu/option.h"
+#include "qemu/config-file.h"
+#include "qapi/qmp/qjson.h"
+#include "qapi/qobject-input-visitor.h"
+#include "qapi-visit.h"
+#include "qapi/dealloc-visitor.h"
 
 
 #define TYPE_DUMMY "qemu-dummy"
@@ -30,6 +38,11 @@
 typedef struct DummyObject DummyObject;
 typedef struct DummyObjectClass DummyObjectClass;
 
+typedef struct DummyPerson DummyPerson;
+typedef struct DummyAddr DummyAddr;
+typedef struct DummyAddrList DummyAddrList;
+typedef struct DummySizeList DummySizeList;
+
 #define DUMMY_OBJECT(obj)                               \
     OBJECT_CHECK(DummyObject, (obj), TYPE_DUMMY)
 
@@ -50,12 +63,35 @@ static const char *const dummy_animal_map[DUMMY_LAST + 1] = {
     [DUMMY_LAST] = NULL,
 };
 
+
+struct DummyAddr {
+    char *ip;
+    int64_t prefix;
+    bool ipv6only;
+};
+
+struct DummyAddrList {
+    DummyAddrList *next;
+    struct DummyAddr *value;
+};
+
+struct DummyPerson {
+    char *name;
+    int64_t age;
+};
+
 struct DummyObject {
     Object parent_obj;
 
     bool bv;
     DummyAnimal av;
     char *sv;
+
+    intList *sizes;
+
+    DummyPerson *person;
+
+    DummyAddrList *addrs;
 };
 
 struct DummyObjectClass {
@@ -117,6 +153,157 @@ static char *dummy_get_sv(Object *obj,
     return g_strdup(dobj->sv);
 }
 
+static void visit_type_DummyPerson_fields(Visitor *v, DummyPerson **obj,
+                                          Error **errp)
+{
+    Error *err = NULL;
+
+    visit_type_str(v, "name", &(*obj)->name, &err);
+    if (err) {
+        goto out;
+    }
+    visit_type_int(v, "age", &(*obj)->age, &err);
+    if (err) {
+        goto out;
+    }
+
+out:
+    error_propagate(errp, err);
+}
+
+static void visit_type_DummyPerson(Visitor *v, const char *name,
+                                   DummyPerson **obj, Error **errp)
+{
+    Error *err = NULL;
+
+    visit_start_struct(v, name, (void **)obj, sizeof(DummyPerson), &err);
+    if (err) {
+        goto out;
+    }
+    if (!*obj) {
+        goto out_obj;
+    }
+    visit_type_DummyPerson_fields(v, obj, &err);
+    error_propagate(errp, err);
+    err = NULL;
+out_obj:
+    visit_end_struct(v, (void **)obj);
+out:
+    error_propagate(errp, err);
+}
+
+static void visit_type_DummyAddr_members(Visitor *v, DummyAddr **obj,
+                                         Error **errp)
+{
+    Error *err = NULL;
+
+    visit_type_str(v, "ip", &(*obj)->ip, &err);
+    if (err) {
+        goto out;
+    }
+    visit_type_int(v, "prefix", &(*obj)->prefix, &err);
+    if (err) {
+        goto out;
+    }
+    visit_type_bool(v, "ipv6only", &(*obj)->ipv6only, &err);
+    if (err) {
+        goto out;
+    }
+
+out:
+    error_propagate(errp, err);
+}
+
+static void visit_type_DummyAddr(Visitor *v, const char *name,
+                                 DummyAddr **obj, Error **errp)
+{
+    Error *err = NULL;
+
+    visit_start_struct(v, name, (void **)obj, sizeof(DummyAddr), &err);
+    if (err) {
+        goto out;
+    }
+    if (!*obj) {
+        goto out_obj;
+    }
+    visit_type_DummyAddr_members(v, obj, &err);
+    error_propagate(errp, err);
+    err = NULL;
+out_obj:
+    visit_end_struct(v, (void **)obj);
+out:
+    error_propagate(errp, err);
+}
+
+static void qapi_free_DummyAddrList(DummyAddrList *obj);
+
+static void visit_type_DummyAddrList(Visitor *v, const char *name,
+                                     DummyAddrList **obj, Error **errp)
+{
+    Error *err = NULL;
+    DummyAddrList *tail;
+    size_t size = sizeof(**obj);
+
+    visit_start_list(v, name, (GenericList **)obj, size, &err);
+    if (err) {
+        goto out;
+    }
+
+    for (tail = *obj; tail;
+         tail = (DummyAddrList *)visit_next_list(v,
+                                                 (GenericList *)tail,
+                                                 size)) {
+        visit_type_DummyAddr(v, NULL, &tail->value, &err);
+        if (err) {
+            break;
+        }
+    }
+
+    visit_end_list(v, (void **)obj);
+    if (err && visit_is_input(v)) {
+        qapi_free_DummyAddrList(*obj);
+        *obj = NULL;
+    }
+out:
+    error_propagate(errp, err);
+}
+
+static void qapi_free_DummyAddrList(DummyAddrList *obj)
+{
+    Visitor *v;
+
+    if (!obj) {
+        return;
+    }
+
+    v = qapi_dealloc_visitor_new();
+    visit_type_DummyAddrList(v, NULL, &obj, NULL);
+    visit_free(v);
+}
+
+static void dummy_set_sizes(Object *obj, Visitor *v, const char *name,
+                            void *opaque, Error **errp)
+{
+    DummyObject *dobj = DUMMY_OBJECT(obj);
+
+    visit_type_intList(v, name, &dobj->sizes, errp);
+}
+
+static void dummy_set_person(Object *obj, Visitor *v, const char *name,
+                            void *opaque, Error **errp)
+{
+    DummyObject *dobj = DUMMY_OBJECT(obj);
+
+    visit_type_DummyPerson(v, name, &dobj->person, errp);
+}
+
+static void dummy_set_addrs(Object *obj, Visitor *v, const char *name,
+                            void *opaque, Error **errp)
+{
+    DummyObject *dobj = DUMMY_OBJECT(obj);
+
+    visit_type_DummyAddrList(v, name, &dobj->addrs, errp);
+}
 
 static void dummy_init(Object *obj)
 {
@@ -126,9 +313,16 @@ static void dummy_init(Object *obj)
                              NULL);
 }
 
+static void
+dummy_complete(UserCreatable *uc, Error **errp)
+{
+}
 
 static void dummy_class_init(ObjectClass *cls, void *data)
 {
+    UserCreatableClass *ucc = USER_CREATABLE_CLASS(cls);
+    ucc->complete = dummy_complete;
+
     object_class_property_add_bool(cls, "bv",
                                    dummy_get_bv,
                                    dummy_set_bv,
@@ -143,12 +337,34 @@ static void dummy_class_init(ObjectClass *cls, void *data)
                                    dummy_get_av,
                                    dummy_set_av,
                                    NULL);
+    object_class_property_add(cls, "sizes",
+                              "int[]",
+                              NULL,
+                              dummy_set_sizes,
+                              NULL, NULL, NULL);
+    object_class_property_add(cls, "person",
+                              "DummyPerson",
+                              NULL,
+                              dummy_set_person,
+                              NULL, NULL, NULL);
+    object_class_property_add(cls, "addrs",
+                              "DummyAddrList",
+                              NULL,
+                              dummy_set_addrs,
+                              NULL, NULL, NULL);
 }
 
 
 static void dummy_finalize(Object *obj)
 {
     DummyObject *dobj = DUMMY_OBJECT(obj);
+    Visitor *v;
+
+    v = qapi_dealloc_visitor_new();
+    visit_type_intList(v, NULL, &dobj->sizes, NULL);
+    visit_type_DummyAddrList(v, NULL, &dobj->addrs, NULL);
+    visit_type_DummyPerson(v, NULL, &dobj->person, NULL);
+    visit_free(v);
 
     g_free(dobj->sv);
 }
@@ -162,6 +378,10 @@ static const TypeInfo dummy_info = {
     .instance_finalize = dummy_finalize,
     .class_size = sizeof(DummyObjectClass),
     .class_init = dummy_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_USER_CREATABLE },
+        { }
+    }
 };
 
 
@@ -473,7 +693,9 @@ static void test_dummy_iterator(void)
 
     ObjectProperty *prop;
     ObjectPropertyIterator iter;
-    bool seenbv = false, seensv = false, seenav = false, seentype;
+    bool seenbv = false, seensv = false, seenav = false,
+        seentype = false, seenaddrs = false, seenperson = false,
+        seensizes = false;
 
     object_property_iter_init(&iter, OBJECT(dobj));
     while ((prop = object_property_iter_next(&iter))) {
@@ -486,6 +708,12 @@ static void test_dummy_iterator(void)
         } else if (g_str_equal(prop->name, "type")) {
             /* This prop comes from the base Object class */
             seentype = true;
+        } else if (g_str_equal(prop->name, "addrs")) {
+            seenaddrs = true;
+        } else if (g_str_equal(prop->name, "person")) {
+            seenperson = true;
+        } else if (g_str_equal(prop->name, "sizes")) {
+            seensizes = true;
         } else {
             g_printerr("Found prop '%s'\n", prop->name);
             g_assert_not_reached();
@@ -495,6 +723,9 @@ static void test_dummy_iterator(void)
     g_assert(seenav);
     g_assert(seensv);
     g_assert(seentype);
+    g_assert(seenaddrs);
+    g_assert(seenperson);
+    g_assert(seensizes);
 
     object_unparent(OBJECT(dobj));
 }
@@ -513,11 +744,109 @@ static void test_dummy_delchild(void)
     object_unparent(OBJECT(dev));
 }
 
+
+static QemuOptsList dummy_opts = {
+    .name = "object",
+    .implied_opt_name = "qom-type",
+    .head = QTAILQ_HEAD_INITIALIZER(dummy_opts.head),
+    .desc = {
+        { }
+    },
+};
+
+static void test_dummy_create_complex(DummyObject *dummy)
+{
+    g_assert(dummy->person != NULL);
+    g_assert_cmpstr(dummy->person->name, ==, "fred");
+    g_assert_cmpint(dummy->person->age, ==, 52);
+
+    g_assert(dummy->sizes != NULL);
+    g_assert_cmpint(dummy->sizes->value, ==, 12);
+    g_assert_cmpint(dummy->sizes->next->value, ==, 65);
+    g_assert_cmpint(dummy->sizes->next->next->value, ==, 8139);
+
+    g_assert(dummy->addrs != NULL);
+    g_assert_cmpstr(dummy->addrs->value->ip, ==, "127.0.0.1");
+    g_assert_cmpint(dummy->addrs->value->prefix, ==, 24);
+    g_assert(dummy->addrs->value->ipv6only);
+    g_assert_cmpstr(dummy->addrs->next->value->ip, ==, "0.0.0.0");
+    g_assert_cmpint(dummy->addrs->next->value->prefix, ==, 16);
+    g_assert(!dummy->addrs->next->value->ipv6only);
+}
+
+
+static void test_dummy_createopts(void)
+{
+    const char *optstr = "qemu-dummy,id=dummy0,bv=yes,av=alligator,sv=hiss,"
+        "person.name=fred,person.age=52,sizes.0=12,sizes.1=65,sizes.2=8139,"
+        "addrs.0.ip=127.0.0.1,addrs.0.prefix=24,addrs.0.ipv6only=yes,"
+        "addrs.1.ip=0.0.0.0,addrs.1.prefix=16,addrs.1.ipv6only=no";
+    QemuOpts *opts;
+    DummyObject *dummy;
+
+    opts = qemu_opts_parse_noisily(&dummy_opts,
+                                   optstr, true);
+    g_assert(opts != NULL);
+
+    dummy = DUMMY_OBJECT(user_creatable_add_opts(opts, &error_abort));
+
+    test_dummy_create_complex(dummy);
+
+    object_unparent(OBJECT(dummy));
+    object_unref(OBJECT(dummy));
+}
+
+
+static void test_dummy_createopts_bad(void)
+{
+    /* Something that tries to create a QList at the top level
+     * should be invalid. */
+    const char *optstr = "qemu-dummy,1=foo,2=bar,3=wizz";
+    QemuOpts *opts;
+    DummyObject *dummy;
+    Error *err = NULL;
+
+    opts = qemu_opts_parse_noisily(&dummy_opts,
+                                   optstr, true);
+    g_assert(opts != NULL);
+
+    dummy = DUMMY_OBJECT(user_creatable_add_opts(opts, &err));
+    error_free_or_abort(&err);
+    g_assert(!dummy);
+}
+
+
+static void test_dummy_createqmp(void)
+{
+    const char *jsonstr =
+        "{ 'qom-type': 'qemu-dummy', 'id': 'dummy0', "
+        "  'bv': true, 'av': 'alligator', 'sv': 'hiss', "
+        "  'person': { 'name': 'fred', 'age': 52 }, "
+        "  'sizes': [12, 65, 8139], "
+        "  'addrs': [ { 'ip': '127.0.0.1', 'prefix': 24, 'ipv6only': true }, "
+        "             { 'ip': '0.0.0.0', 'prefix': 16, 'ipv6only': false } ] }";
+    QObject *obj = qobject_from_json(jsonstr);
+    Visitor *v = qobject_input_visitor_new(obj, true);
+    DummyObject *dummy;
+    g_assert(obj);
+    dummy = DUMMY_OBJECT(user_creatable_add(qobject_to_qdict(obj), v,
+                                            &error_abort));
+
+    test_dummy_create_complex(dummy);
+    visit_free(v);
+    object_unparent(OBJECT(dummy));
+    object_unref(OBJECT(dummy));
+    qobject_decref(obj);
+}
+
 int main(int argc, char **argv)
 {
     g_test_init(&argc, &argv, NULL);
 
     module_call_init(MODULE_INIT_QOM);
+
+    qemu_add_opts(&dummy_opts);
+
     type_register_static(&dummy_info);
     type_register_static(&dummy_dev_info);
     type_register_static(&dummy_bus_info);
@@ -529,6 +858,9 @@ int main(int argc, char **argv)
     g_test_add_func("/qom/proplist/getenum", test_dummy_getenum);
     g_test_add_func("/qom/proplist/iterator", test_dummy_iterator);
     g_test_add_func("/qom/proplist/delchild", test_dummy_delchild);
+    g_test_add_func("/qom/proplist/createopts", test_dummy_createopts);
+    g_test_add_func("/qom/proplist/createoptsbad", test_dummy_createopts_bad);
+    g_test_add_func("/qom/proplist/createqmp", test_dummy_createqmp);
 
     return g_test_run();
 }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v13 6/6] qom: support arbitrary non-scalar properties with -object
  2016-09-19 11:58 ` [Qemu-devel] [PATCH v13 6/6] qom: support arbitrary non-scalar properties with -object Daniel P. Berrange
@ 2016-09-19 12:12   ` Daniel P. Berrange
  2016-09-19 12:19     ` Paolo Bonzini
  2016-09-23 10:29     ` Kevin Wolf
  0 siblings, 2 replies; 18+ messages in thread
From: Daniel P. Berrange @ 2016-09-19 12:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: Markus Armbruster, Max Reitz, Marc-André Lureau,
	Paolo Bonzini, Andreas Färber, Kevin Wolf

On Mon, Sep 19, 2016 at 12:58:30PM +0100, Daniel P. Berrange wrote:
> The current -object command line syntax only allows for
> creation of objects with scalar properties, or a list
> with a fixed scalar element type. Objects which have
> properties that are represented as structs in the QAPI
> schema cannot be created using -object.
> 
> This is a design limitation of the way the OptsVisitor
> is written. It simply iterates over the QemuOpts values
> as a flat list. The support for lists is enabled by
> allowing the same key to be repeated in the opts string.
> 
> It is not practical to extend the OptsVisitor to support
> more complex data structures while also maintaining
> the existing list handling behaviour that is relied upon
> by other areas of QEMU.
> 
> Fortunately there is no existing object that implements
> the UserCreatable interface that relies on the list
> handling behaviour, so it is possible to swap out the
> OptsVisitor for a different visitor implementation, so
> -object supports non-scalar properties, thus leaving
> other users of OptsVisitor unaffected.

Urgh, I've just discovered that this is not in fact true.

The 'memory-backend' object type uses uint16List which
has the hacky list syntax

  -object memory-backend-ram,\
          id=ram-node2,size=24578621440,policy=bind,\
          host-nodes=1-2,host-nodes=5,host-nodes=7,

So I'll need to figure out a way to preserve this syntax...

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v13 6/6] qom: support arbitrary non-scalar properties with -object
  2016-09-19 12:12   ` Daniel P. Berrange
@ 2016-09-19 12:19     ` Paolo Bonzini
  2016-09-19 12:29       ` Daniel P. Berrange
  2016-09-23 10:29     ` Kevin Wolf
  1 sibling, 1 reply; 18+ messages in thread
From: Paolo Bonzini @ 2016-09-19 12:19 UTC (permalink / raw)
  To: Daniel P. Berrange, qemu-devel
  Cc: Markus Armbruster, Max Reitz, Marc-André Lureau,
	Andreas Färber, Kevin Wolf



On 19/09/2016 14:12, Daniel P. Berrange wrote:
> On Mon, Sep 19, 2016 at 12:58:30PM +0100, Daniel P. Berrange wrote:
>> The current -object command line syntax only allows for
>> creation of objects with scalar properties, or a list
>> with a fixed scalar element type. Objects which have
>> properties that are represented as structs in the QAPI
>> schema cannot be created using -object.
>>
>> This is a design limitation of the way the OptsVisitor
>> is written. It simply iterates over the QemuOpts values
>> as a flat list. The support for lists is enabled by
>> allowing the same key to be repeated in the opts string.
>>
>> It is not practical to extend the OptsVisitor to support
>> more complex data structures while also maintaining
>> the existing list handling behaviour that is relied upon
>> by other areas of QEMU.
>>
>> Fortunately there is no existing object that implements
>> the UserCreatable interface that relies on the list
>> handling behaviour, so it is possible to swap out the
>> OptsVisitor for a different visitor implementation, so
>> -object supports non-scalar properties, thus leaving
>> other users of OptsVisitor unaffected.
> 
> Urgh, I've just discovered that this is not in fact true.
> 
> The 'memory-backend' object type uses uint16List which
> has the hacky list syntax
> 
>   -object memory-backend-ram,\
>           id=ram-node2,size=24578621440,policy=bind,\
>           host-nodes=1-2,host-nodes=5,host-nodes=7,
> 
> So I'll need to figure out a way to preserve this syntax...

Is there a usecase for qdict_crumple without the following
de-stringification pass?  If not, qdict_crumple could use a
StringInputVisitor on the values directly.

Paolo

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v13 6/6] qom: support arbitrary non-scalar properties with -object
  2016-09-19 12:19     ` Paolo Bonzini
@ 2016-09-19 12:29       ` Daniel P. Berrange
  2016-09-19 12:40         ` Paolo Bonzini
  0 siblings, 1 reply; 18+ messages in thread
From: Daniel P. Berrange @ 2016-09-19 12:29 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: qemu-devel, Markus Armbruster, Max Reitz, Marc-André Lureau,
	Andreas Färber, Kevin Wolf

On Mon, Sep 19, 2016 at 02:19:19PM +0200, Paolo Bonzini wrote:
> 
> 
> On 19/09/2016 14:12, Daniel P. Berrange wrote:
> > On Mon, Sep 19, 2016 at 12:58:30PM +0100, Daniel P. Berrange wrote:
> >> The current -object command line syntax only allows for
> >> creation of objects with scalar properties, or a list
> >> with a fixed scalar element type. Objects which have
> >> properties that are represented as structs in the QAPI
> >> schema cannot be created using -object.
> >>
> >> This is a design limitation of the way the OptsVisitor
> >> is written. It simply iterates over the QemuOpts values
> >> as a flat list. The support for lists is enabled by
> >> allowing the same key to be repeated in the opts string.
> >>
> >> It is not practical to extend the OptsVisitor to support
> >> more complex data structures while also maintaining
> >> the existing list handling behaviour that is relied upon
> >> by other areas of QEMU.
> >>
> >> Fortunately there is no existing object that implements
> >> the UserCreatable interface that relies on the list
> >> handling behaviour, so it is possible to swap out the
> >> OptsVisitor for a different visitor implementation, so
> >> -object supports non-scalar properties, thus leaving
> >> other users of OptsVisitor unaffected.
> > 
> > Urgh, I've just discovered that this is not in fact true.
> > 
> > The 'memory-backend' object type uses uint16List which
> > has the hacky list syntax
> > 
> >   -object memory-backend-ram,\
> >           id=ram-node2,size=24578621440,policy=bind,\
> >           host-nodes=1-2,host-nodes=5,host-nodes=7,
> > 
> > So I'll need to figure out a way to preserve this syntax...
> 
> Is there a usecase for qdict_crumple without the following
> de-stringification pass?  If not, qdict_crumple could use a
> StringInputVisitor on the values directly.

I'm not sure I understand how that would help and in any case, the
string-input-visitor itself is incapable of dealing with compound
properties. It too could/should be ultimately replaced by the
qobject-input-visitor combined with a pre-processing step to
turn the input string into a qdict.

The problem I'm facing with the above scenario though occurs
before we get anywhere near the visitors, or the crumple step.

When qemu_opts_to_qdict() turns QemuOpts into QDict, it looses
repeated options. So given a command line


   -object memory-backend-ram,\
           id=ram-node2,size=24578621440,policy=bind,\
           host-nodes=1-2,host-nodes=5,host-nodes=7,

we're getting an initial QDict containing

    type=memory-backend-ram
    id=ram-node2
    size=24578621440
    policy=bind
    host-nodes=7

What I need to do is make qemu_opts_to_qdict more intelligent
so that if it sees repeated strings, it inserts them into the
qdict using list index style. eg, the QemuOpts -> QDict
conversion should have at minimum given us

    type=memory-backend-ram
    id=ram-node2
    size=24578621440
    policy=bind
    host-nodes.0=1-2
    host-nodes.1=5
    host-nodes.2=7

although even then there's some magic in the range value seen there. I'm
unclear on whether we should try to deal with the range "1-2" in the
qemu_opts_to_qdict() method, the qdict_crumple() method or the qobject
input visitor code for parsing ints.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v13 6/6] qom: support arbitrary non-scalar properties with -object
  2016-09-19 12:29       ` Daniel P. Berrange
@ 2016-09-19 12:40         ` Paolo Bonzini
  0 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2016-09-19 12:40 UTC (permalink / raw)
  To: Daniel P. Berrange
  Cc: qemu-devel, Markus Armbruster, Max Reitz, Marc-André Lureau,
	Andreas Färber, Kevin Wolf



On 19/09/2016 14:29, Daniel P. Berrange wrote:
> On Mon, Sep 19, 2016 at 02:19:19PM +0200, Paolo Bonzini wrote:
>>
>>
>> On 19/09/2016 14:12, Daniel P. Berrange wrote:
>>> On Mon, Sep 19, 2016 at 12:58:30PM +0100, Daniel P. Berrange wrote:
>>>> The current -object command line syntax only allows for
>>>> creation of objects with scalar properties, or a list
>>>> with a fixed scalar element type. Objects which have
>>>> properties that are represented as structs in the QAPI
>>>> schema cannot be created using -object.
>>>>
>>>> This is a design limitation of the way the OptsVisitor
>>>> is written. It simply iterates over the QemuOpts values
>>>> as a flat list. The support for lists is enabled by
>>>> allowing the same key to be repeated in the opts string.
>>>>
>>>> It is not practical to extend the OptsVisitor to support
>>>> more complex data structures while also maintaining
>>>> the existing list handling behaviour that is relied upon
>>>> by other areas of QEMU.
>>>>
>>>> Fortunately there is no existing object that implements
>>>> the UserCreatable interface that relies on the list
>>>> handling behaviour, so it is possible to swap out the
>>>> OptsVisitor for a different visitor implementation, so
>>>> -object supports non-scalar properties, thus leaving
>>>> other users of OptsVisitor unaffected.
>>>
>>> Urgh, I've just discovered that this is not in fact true.
>>>
>>> The 'memory-backend' object type uses uint16List which
>>> has the hacky list syntax
>>>
>>>   -object memory-backend-ram,\
>>>           id=ram-node2,size=24578621440,policy=bind,\
>>>           host-nodes=1-2,host-nodes=5,host-nodes=7,
>>>
>>> So I'll need to figure out a way to preserve this syntax...
>>
>> Is there a usecase for qdict_crumple without the following
>> de-stringification pass?  If not, qdict_crumple could use a
>> StringInputVisitor on the values directly.
> 
> I'm not sure I understand how that would help and in any case, the
> string-input-visitor itself is incapable of dealing with compound
> properties.

> It too could/should be ultimately replaced by the
> qobject-input-visitor combined with a pre-processing step to
> turn the input string into a qdict.
> 
> The problem I'm facing with the above scenario though occurs
> before we get anywhere near the visitors, or the crumple step.
> 
> When qemu_opts_to_qdict() turns QemuOpts into QDict, it looses
> repeated options.

Ouch, I thought the problem was only with "1-2", not with the repeated
host-nodes.  Wow, that does really work?!?

What might work is if you convert the repeated options to
"host-nodes=1-2,5,7".  Then StringInputVisitor is able to parse that
(but OptsVisitor isn't).

I'm not sure if there are other uses of repeated options.  Probably not
based on a limited inspection of callers of qemu_opt_foreach.

Paolo

> What I need to do is make qemu_opts_to_qdict more intelligent
> so that if it sees repeated strings, it inserts them into the
> qdict using list index style. eg, the QemuOpts -> QDict
> conversion should have at minimum given us
> 
>     type=memory-backend-ram
>     id=ram-node2
>     size=24578621440
>     policy=bind
>     host-nodes.0=1-2
>     host-nodes.1=5
>     host-nodes.2=7
> 
> although even then there's some magic in the range value seen there. I'm
> unclear on whether we should try to deal with the range "1-2" in the
> qemu_opts_to_qdict() method, the qdict_crumple() method or the qobject
> input visitor code for parsing ints.
> 
> Regards,
> Daniel
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v13 2/6] option: make parse_option_bool/number non-static
  2016-09-19 11:58 ` [Qemu-devel] [PATCH v13 2/6] option: make parse_option_bool/number non-static Daniel P. Berrange
@ 2016-09-23 10:24   ` Kevin Wolf
  2016-09-23 11:07     ` Daniel P. Berrange
  0 siblings, 1 reply; 18+ messages in thread
From: Kevin Wolf @ 2016-09-23 10:24 UTC (permalink / raw)
  To: Daniel P. Berrange
  Cc: qemu-devel, Markus Armbruster, Max Reitz, Marc-André Lureau,
	Paolo Bonzini, Andreas Färber

Am 19.09.2016 um 13:58 hat Daniel P. Berrange geschrieben:
> The opts-visitor.c opts_type_bool() method has code for
> parsing a string to set a bool value, as does the
> qemu-option.c parse_option_bool() method, except it
> handles fewer cases.
> 
> To enable consistency across the codebase, extend
> parse_option_bool() to handle "yes", "no", "y" and
> "n", and make it non-static. Convert the opts
> visitor to call this method directly.
> 
> Also make parse_option_number() non-static to allow
> for similar reuse later.
> 
> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

> --- a/util/qemu-option.c
> +++ b/util/qemu-option.c
> @@ -125,25 +125,30 @@ int get_param_value(char *buf, int buf_size,
>      return get_next_param_value(buf, buf_size, tag, &str);
>  }
>  
> -static void parse_option_bool(const char *name, const char *value, bool *ret,
> -                              Error **errp)
> +void parse_option_bool(const char *name, const char *value, bool *ret,
> +                       Error **errp)
>  {
>      if (value != NULL) {
> -        if (!strcmp(value, "on")) {
> -            *ret = 1;
> -        } else if (!strcmp(value, "off")) {
> -            *ret = 0;
> +        if (strcmp(value, "on") == 0 ||
> +            strcmp(value, "yes") == 0 ||
> +            strcmp(value, "y") == 0) {
> +            *ret = true;
> +        } else if (strcmp(value, "off") == 0 ||
> +            strcmp(value, "no") == 0 ||
> +            strcmp(value, "n") == 0) {
> +            *ret = false;
>          } else {
> -            error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
> -                       name, "'on' or 'off'");
> +            error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name,
> +                       "on|yes|y|off|no|n");

This change requires an update to the reference output of some
qemu-iotests (at least 051 and 137).

Kevin

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v13 6/6] qom: support arbitrary non-scalar properties with -object
  2016-09-19 12:12   ` Daniel P. Berrange
  2016-09-19 12:19     ` Paolo Bonzini
@ 2016-09-23 10:29     ` Kevin Wolf
  2016-09-23 11:07       ` Daniel P. Berrange
  1 sibling, 1 reply; 18+ messages in thread
From: Kevin Wolf @ 2016-09-23 10:29 UTC (permalink / raw)
  To: Daniel P. Berrange
  Cc: qemu-devel, Markus Armbruster, Max Reitz, Marc-André Lureau,
	Paolo Bonzini, Andreas Färber

Am 19.09.2016 um 14:12 hat Daniel P. Berrange geschrieben:
> On Mon, Sep 19, 2016 at 12:58:30PM +0100, Daniel P. Berrange wrote:
> > Fortunately there is no existing object that implements
> > the UserCreatable interface that relies on the list
> > handling behaviour, so it is possible to swap out the
> > OptsVisitor for a different visitor implementation, so
> > -object supports non-scalar properties, thus leaving
> > other users of OptsVisitor unaffected.
> 
> Urgh, I've just discovered that this is not in fact true.
> 
> The 'memory-backend' object type uses uint16List which
> has the hacky list syntax
> 
>   -object memory-backend-ram,\
>           id=ram-node2,size=24578621440,policy=bind,\
>           host-nodes=1-2,host-nodes=5,host-nodes=7,
> 
> So I'll need to figure out a way to preserve this syntax...

If this turns out rather hard, would it make sense to merge just patches
1 to 5 for now to enable -blockdev, which doesn't need this syntax? Then
you can add the magic list syntax as a follow-up together with the
-object work.

Kevin

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v13 6/6] qom: support arbitrary non-scalar properties with -object
  2016-09-23 10:29     ` Kevin Wolf
@ 2016-09-23 11:07       ` Daniel P. Berrange
  2016-09-23 11:12         ` Kevin Wolf
  0 siblings, 1 reply; 18+ messages in thread
From: Daniel P. Berrange @ 2016-09-23 11:07 UTC (permalink / raw)
  To: Kevin Wolf
  Cc: qemu-devel, Markus Armbruster, Max Reitz, Marc-André Lureau,
	Paolo Bonzini, Andreas Färber

On Fri, Sep 23, 2016 at 12:29:21PM +0200, Kevin Wolf wrote:
> Am 19.09.2016 um 14:12 hat Daniel P. Berrange geschrieben:
> > On Mon, Sep 19, 2016 at 12:58:30PM +0100, Daniel P. Berrange wrote:
> > > Fortunately there is no existing object that implements
> > > the UserCreatable interface that relies on the list
> > > handling behaviour, so it is possible to swap out the
> > > OptsVisitor for a different visitor implementation, so
> > > -object supports non-scalar properties, thus leaving
> > > other users of OptsVisitor unaffected.
> > 
> > Urgh, I've just discovered that this is not in fact true.
> > 
> > The 'memory-backend' object type uses uint16List which
> > has the hacky list syntax
> > 
> >   -object memory-backend-ram,\
> >           id=ram-node2,size=24578621440,policy=bind,\
> >           host-nodes=1-2,host-nodes=5,host-nodes=7,
> > 
> > So I'll need to figure out a way to preserve this syntax...
> 
> If this turns out rather hard, would it make sense to merge just patches
> 1 to 5 for now to enable -blockdev, which doesn't need this syntax? Then
> you can add the magic list syntax as a follow-up together with the
> -object work.

I've got code to deal with it and will be sending a new series, probably
on monday once i've done more testing.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v13 2/6] option: make parse_option_bool/number non-static
  2016-09-23 10:24   ` Kevin Wolf
@ 2016-09-23 11:07     ` Daniel P. Berrange
  2016-09-23 14:16       ` Eric Blake
  0 siblings, 1 reply; 18+ messages in thread
From: Daniel P. Berrange @ 2016-09-23 11:07 UTC (permalink / raw)
  To: Kevin Wolf
  Cc: qemu-devel, Markus Armbruster, Max Reitz, Marc-André Lureau,
	Paolo Bonzini, Andreas Färber

On Fri, Sep 23, 2016 at 12:24:30PM +0200, Kevin Wolf wrote:
> Am 19.09.2016 um 13:58 hat Daniel P. Berrange geschrieben:
> > The opts-visitor.c opts_type_bool() method has code for
> > parsing a string to set a bool value, as does the
> > qemu-option.c parse_option_bool() method, except it
> > handles fewer cases.
> > 
> > To enable consistency across the codebase, extend
> > parse_option_bool() to handle "yes", "no", "y" and
> > "n", and make it non-static. Convert the opts
> > visitor to call this method directly.
> > 
> > Also make parse_option_number() non-static to allow
> > for similar reuse later.
> > 
> > Reviewed-by: Kevin Wolf <kwolf@redhat.com>
> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Reviewed-by: Eric Blake <eblake@redhat.com>
> > Reviewed-by: Markus Armbruster <armbru@redhat.com>
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> 
> > --- a/util/qemu-option.c
> > +++ b/util/qemu-option.c
> > @@ -125,25 +125,30 @@ int get_param_value(char *buf, int buf_size,
> >      return get_next_param_value(buf, buf_size, tag, &str);
> >  }
> >  
> > -static void parse_option_bool(const char *name, const char *value, bool *ret,
> > -                              Error **errp)
> > +void parse_option_bool(const char *name, const char *value, bool *ret,
> > +                       Error **errp)
> >  {
> >      if (value != NULL) {
> > -        if (!strcmp(value, "on")) {
> > -            *ret = 1;
> > -        } else if (!strcmp(value, "off")) {
> > -            *ret = 0;
> > +        if (strcmp(value, "on") == 0 ||
> > +            strcmp(value, "yes") == 0 ||
> > +            strcmp(value, "y") == 0) {
> > +            *ret = true;
> > +        } else if (strcmp(value, "off") == 0 ||
> > +            strcmp(value, "no") == 0 ||
> > +            strcmp(value, "n") == 0) {
> > +            *ret = false;
> >          } else {
> > -            error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
> > -                       name, "'on' or 'off'");
> > +            error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name,
> > +                       "on|yes|y|off|no|n");
> 
> This change requires an update to the reference output of some
> qemu-iotests (at least 051 and 137).

Opps, yes, so it does.


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v13 6/6] qom: support arbitrary non-scalar properties with -object
  2016-09-23 11:07       ` Daniel P. Berrange
@ 2016-09-23 11:12         ` Kevin Wolf
  0 siblings, 0 replies; 18+ messages in thread
From: Kevin Wolf @ 2016-09-23 11:12 UTC (permalink / raw)
  To: Daniel P. Berrange
  Cc: qemu-devel, Markus Armbruster, Max Reitz, Marc-André Lureau,
	Paolo Bonzini, Andreas Färber

Am 23.09.2016 um 13:07 hat Daniel P. Berrange geschrieben:
> On Fri, Sep 23, 2016 at 12:29:21PM +0200, Kevin Wolf wrote:
> > Am 19.09.2016 um 14:12 hat Daniel P. Berrange geschrieben:
> > > On Mon, Sep 19, 2016 at 12:58:30PM +0100, Daniel P. Berrange wrote:
> > > > Fortunately there is no existing object that implements
> > > > the UserCreatable interface that relies on the list
> > > > handling behaviour, so it is possible to swap out the
> > > > OptsVisitor for a different visitor implementation, so
> > > > -object supports non-scalar properties, thus leaving
> > > > other users of OptsVisitor unaffected.
> > > 
> > > Urgh, I've just discovered that this is not in fact true.
> > > 
> > > The 'memory-backend' object type uses uint16List which
> > > has the hacky list syntax
> > > 
> > >   -object memory-backend-ram,\
> > >           id=ram-node2,size=24578621440,policy=bind,\
> > >           host-nodes=1-2,host-nodes=5,host-nodes=7,
> > > 
> > > So I'll need to figure out a way to preserve this syntax...
> > 
> > If this turns out rather hard, would it make sense to merge just patches
> > 1 to 5 for now to enable -blockdev, which doesn't need this syntax? Then
> > you can add the magic list syntax as a follow-up together with the
> > -object work.
> 
> I've got code to deal with it and will be sending a new series, probably
> on monday once i've done more testing.

Even better then. :-)

Kevin

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v13 2/6] option: make parse_option_bool/number non-static
  2016-09-23 11:07     ` Daniel P. Berrange
@ 2016-09-23 14:16       ` Eric Blake
  2016-09-23 14:21         ` Daniel P. Berrange
  0 siblings, 1 reply; 18+ messages in thread
From: Eric Blake @ 2016-09-23 14:16 UTC (permalink / raw)
  To: Daniel P. Berrange, Kevin Wolf
  Cc: Markus Armbruster, qemu-devel, Max Reitz, Marc-André Lureau,
	Paolo Bonzini, Andreas Färber

[-- Attachment #1: Type: text/plain, Size: 694 bytes --]

On 09/23/2016 06:07 AM, Daniel P. Berrange wrote:

>>> -            error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
>>> -                       name, "'on' or 'off'");
>>> +            error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name,
>>> +                       "on|yes|y|off|no|n");
>>
>> This change requires an update to the reference output of some
>> qemu-iotests (at least 051 and 137).
> 
> Opps, yes, so it does.

Do any of the docker tests or buildbots catch this automatically? (I
often forget to run qemu-iotests, since they are not part of 'make check')

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v13 2/6] option: make parse_option_bool/number non-static
  2016-09-23 14:16       ` Eric Blake
@ 2016-09-23 14:21         ` Daniel P. Berrange
  0 siblings, 0 replies; 18+ messages in thread
From: Daniel P. Berrange @ 2016-09-23 14:21 UTC (permalink / raw)
  To: Eric Blake
  Cc: Kevin Wolf, Markus Armbruster, qemu-devel, Max Reitz,
	Marc-André Lureau, Paolo Bonzini, Andreas Färber

On Fri, Sep 23, 2016 at 09:16:35AM -0500, Eric Blake wrote:
> On 09/23/2016 06:07 AM, Daniel P. Berrange wrote:
> 
> >>> -            error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
> >>> -                       name, "'on' or 'off'");
> >>> +            error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name,
> >>> +                       "on|yes|y|off|no|n");
> >>
> >> This change requires an update to the reference output of some
> >> qemu-iotests (at least 051 and 137).
> > 
> > Opps, yes, so it does.
> 
> Do any of the docker tests or buildbots catch this automatically? (I
> often forget to run qemu-iotests, since they are not part of 'make check')

FWIW, as mentioned here:

 https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg04392.html

I've tried adding qemu-iotests to our travis config for raw, qcow2
and luks, but hit many non-deterministic failures in it. We'll have
to get it stable under high load before we can have it run automatically
unless we want high false-failure rate.


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2016-09-23 14:21 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-19 11:58 [Qemu-devel] [PATCH v13 0/6] QAPI/QOM work for non-scalar object properties Daniel P. Berrange
2016-09-19 11:58 ` [Qemu-devel] [PATCH v13 1/6] qdict: implement a qdict_crumple method for un-flattening a dict Daniel P. Berrange
2016-09-19 11:58 ` [Qemu-devel] [PATCH v13 2/6] option: make parse_option_bool/number non-static Daniel P. Berrange
2016-09-23 10:24   ` Kevin Wolf
2016-09-23 11:07     ` Daniel P. Berrange
2016-09-23 14:16       ` Eric Blake
2016-09-23 14:21         ` Daniel P. Berrange
2016-09-19 11:58 ` [Qemu-devel] [PATCH v13 3/6] qapi: rename QmpInputVisitor to QObjectInputVisitor Daniel P. Berrange
2016-09-19 11:58 ` [Qemu-devel] [PATCH v13 4/6] qapi: rename QmpOutputVisitor to QObjectOutputVisitor Daniel P. Berrange
2016-09-19 11:58 ` [Qemu-devel] [PATCH v13 5/6] qapi: add a QObjectInputVisitor that does string conversion Daniel P. Berrange
2016-09-19 11:58 ` [Qemu-devel] [PATCH v13 6/6] qom: support arbitrary non-scalar properties with -object Daniel P. Berrange
2016-09-19 12:12   ` Daniel P. Berrange
2016-09-19 12:19     ` Paolo Bonzini
2016-09-19 12:29       ` Daniel P. Berrange
2016-09-19 12:40         ` Paolo Bonzini
2016-09-23 10:29     ` Kevin Wolf
2016-09-23 11:07       ` Daniel P. Berrange
2016-09-23 11:12         ` Kevin Wolf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.