* [Qemu-devel] [PATCH v11 0/6] QAPI/QOM work for non-scalar object properties
@ 2016-09-05 15:16 Daniel P. Berrange
2016-09-05 15:16 ` [Qemu-devel] [PATCH v11 1/6] qdict: implement a qdict_crumple method for un-flattening a dict Daniel P. Berrange
` (6 more replies)
0 siblings, 7 replies; 30+ messages in thread
From: Daniel P. Berrange @ 2016-09-05 15:16 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Max Reitz, Marc-André Lureau,
Paolo Bonzini, Andreas Färber, Eric Blake,
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
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 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 QmpInputVisitor 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 | 63 +++
...p-output-visitor.h => qobject-output-visitor.h} | 10 +-
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 | 526 +++++++++++++++++++++
qapi/qobject-output-visitor.c | 254 ++++++++++
qemu-img.c | 8 +-
qmp.c | 6 +-
qobject/qdict.c | 283 +++++++++++
qom/object_interfaces.c | 47 +-
qom/qom-qobject.c | 8 +-
scripts/qapi-commands.py | 8 +-
scripts/qapi-event.py | 4 +-
tests/.gitignore | 6 +-
tests/Makefile.include | 20 +-
tests/check-qdict.c | 241 ++++++++++
tests/check-qnull.c | 8 +-
tests/check-qom-proplist.c | 314 +++++++++++-
tests/test-qmp-commands.c | 4 +-
...-input-strict.c => test-qobject-input-strict.c} | 4 +-
...nput-visitor.c => test-qobject-input-visitor.c} | 154 +++++-
...put-visitor.c => test-qobject-output-visitor.c} | 4 +-
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 +-
39 files changed, 1959 insertions(+), 824 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} (99%)
rename tests/{test-qmp-input-visitor.c => test-qobject-input-visitor.c} (86%)
rename tests/{test-qmp-output-visitor.c => test-qobject-output-visitor.c} (99%)
--
2.7.4
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH v11 1/6] qdict: implement a qdict_crumple method for un-flattening a dict
2016-09-05 15:16 [Qemu-devel] [PATCH v11 0/6] QAPI/QOM work for non-scalar object properties Daniel P. Berrange
@ 2016-09-05 15:16 ` Daniel P. Berrange
2016-09-14 14:18 ` Kevin Wolf
2016-09-05 15:16 ` [Qemu-devel] [PATCH v11 2/6] option: make parse_option_bool/number non-static Daniel P. Berrange
` (5 subsequent siblings)
6 siblings, 1 reply; 30+ messages in thread
From: Daniel P. Berrange @ 2016-09-05 15:16 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Max Reitz, Marc-André Lureau,
Paolo Bonzini, Andreas Färber, Eric Blake,
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: 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 | 283 +++++++++++++++++++++++++++++++++++++++++++++++
tests/check-qdict.c | 241 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 525 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..a075eb1 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,288 @@ 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 crumple a dictionary with a mix of 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 crumple a dictionary with a mix of list "
+ "and non-list keys");
+ return -1;
+ }
+ }
+ }
+
+ if (is_list == -1) {
+ assert(!qdict_size(maybe_list));
+ is_list = 0;
+ }
+
+ 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)) {
+ 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);
+ assert(child);
+
+ 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..be4a132 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,235 @@ static void qdict_join_test(void)
QDECREF(dict2);
}
+
+static void qdict_crumple_test_nonrecursive(void)
+{
+ QDict *src, *dst, *rules, *vnc;
+ 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"));
+
+ 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), ==, 2);
+
+ 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"));
+
+ 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"));
+
+ 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);
+
+ 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", qdict_new());
+ 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", qdict_new());
+ 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 +972,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] 30+ messages in thread
* [Qemu-devel] [PATCH v11 2/6] option: make parse_option_bool/number non-static
2016-09-05 15:16 [Qemu-devel] [PATCH v11 0/6] QAPI/QOM work for non-scalar object properties Daniel P. Berrange
2016-09-05 15:16 ` [Qemu-devel] [PATCH v11 1/6] qdict: implement a qdict_crumple method for un-flattening a dict Daniel P. Berrange
@ 2016-09-05 15:16 ` Daniel P. Berrange
2016-09-14 14:33 ` Kevin Wolf
2016-09-05 15:16 ` [Qemu-devel] [PATCH v11 3/6] qapi: rename QmpInputVisitor to QObjectInputVisitor Daniel P. Berrange
` (4 subsequent siblings)
6 siblings, 1 reply; 30+ messages in thread
From: Daniel P. Berrange @ 2016-09-05 15:16 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Max Reitz, Marc-André Lureau,
Paolo Bonzini, Andreas Färber, Eric Blake,
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: 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] 30+ messages in thread
* [Qemu-devel] [PATCH v11 3/6] qapi: rename QmpInputVisitor to QObjectInputVisitor
2016-09-05 15:16 [Qemu-devel] [PATCH v11 0/6] QAPI/QOM work for non-scalar object properties Daniel P. Berrange
2016-09-05 15:16 ` [Qemu-devel] [PATCH v11 1/6] qdict: implement a qdict_crumple method for un-flattening a dict Daniel P. Berrange
2016-09-05 15:16 ` [Qemu-devel] [PATCH v11 2/6] option: make parse_option_bool/number non-static Daniel P. Berrange
@ 2016-09-05 15:16 ` Daniel P. Berrange
2016-09-12 16:19 ` Markus Armbruster
2016-09-05 15:16 ` [Qemu-devel] [PATCH v11 4/6] qapi: rename QmpOutputVisitor to QObjectOutputVisitor Daniel P. Berrange
` (3 subsequent siblings)
6 siblings, 1 reply; 30+ messages in thread
From: Daniel P. Berrange @ 2016-09-05 15:16 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Max Reitz, Marc-André Lureau,
Paolo Bonzini, Andreas Färber, Eric Blake,
Daniel P. Berrange
The QmpInputVisitor has no direct dependancy 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 convertor.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
docs/qapi-code-gen.txt | 2 +-
...qmp-input-visitor.h => qobject-input-visitor.h} | 10 +-
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 +-
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} | 4 +-
...nput-visitor.c => test-qobject-input-visitor.c} | 4 +-
tests/test-string-input-visitor.c | 2 +-
tests/test-visitor-serialization.c | 4 +-
util/qemu-sockets.c | 2 +-
17 files changed, 121 insertions(+), 118 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} (99%)
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/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 b6d531e..ffc8828 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"
@@ -666,7 +666,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/tests/.gitignore b/tests/.gitignore
index dbb5263..4ec0a1a 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 14be491..5ac61b7 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)
@@ -415,7 +415,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 \
@@ -514,8 +514,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 99%
rename from tests/test-qmp-input-strict.c
rename to tests/test-qobject-input-strict.c
index 814550a..546f8af 100644
--- a/tests/test-qmp-input-strict.c
+++ b/tests/test-qobject-input-strict.c
@@ -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..02895f0 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qobject-input-visitor.c
@@ -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 d837ebe..9bdcd74 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 2aed799..dd9ad44 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] 30+ messages in thread
* [Qemu-devel] [PATCH v11 4/6] qapi: rename QmpOutputVisitor to QObjectOutputVisitor
2016-09-05 15:16 [Qemu-devel] [PATCH v11 0/6] QAPI/QOM work for non-scalar object properties Daniel P. Berrange
` (2 preceding siblings ...)
2016-09-05 15:16 ` [Qemu-devel] [PATCH v11 3/6] qapi: rename QmpInputVisitor to QObjectInputVisitor Daniel P. Berrange
@ 2016-09-05 15:16 ` Daniel P. Berrange
2016-09-12 16:20 ` Markus Armbruster
2016-09-12 18:24 ` Eric Blake
2016-09-05 15:16 ` [Qemu-devel] [PATCH v11 5/6] qapi: add a QmpInputVisitor that does string conversion Daniel P. Berrange
` (2 subsequent siblings)
6 siblings, 2 replies; 30+ messages in thread
From: Daniel P. Berrange @ 2016-09-05 15:16 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Max Reitz, Marc-André Lureau,
Paolo Bonzini, Andreas Färber, Eric Blake,
Daniel P. Berrange
The QmpOutputVisitor has no direct dependancy 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 convertor.
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} | 4 +-
tests/test-string-output-visitor.c | 2 +-
tests/test-visitor-serialization.c | 4 +-
util/qemu-sockets.c | 2 +-
20 files changed, 290 insertions(+), 292 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 2161400..2d388d6 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"
@@ -3907,7 +3907,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 f204d041..a6e9c1e 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);
@@ -2183,7 +2183,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);
@@ -2199,7 +2199,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 4ec0a1a..576b87b 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-rfifolock
test-string-input-visitor
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 5ac61b7..716fafd 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)
@@ -413,7 +413,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 \
@@ -512,7 +512,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..8f46c71 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qobject-output-visitor.c
@@ -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 dd9ad44..3a11e43 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] 30+ messages in thread
* [Qemu-devel] [PATCH v11 5/6] qapi: add a QmpInputVisitor that does string conversion
2016-09-05 15:16 [Qemu-devel] [PATCH v11 0/6] QAPI/QOM work for non-scalar object properties Daniel P. Berrange
` (3 preceding siblings ...)
2016-09-05 15:16 ` [Qemu-devel] [PATCH v11 4/6] qapi: rename QmpOutputVisitor to QObjectOutputVisitor Daniel P. Berrange
@ 2016-09-05 15:16 ` Daniel P. Berrange
2016-09-12 16:21 ` Markus Armbruster
` (3 more replies)
2016-09-05 15:16 ` [Qemu-devel] [PATCH v11 6/6] qom: support arbitrary non-scalar properties with -object Daniel P. Berrange
2016-09-14 15:04 ` [Qemu-devel] [PATCH v11 0/6] QAPI/QOM work for non-scalar object properties Kevin Wolf
6 siblings, 4 replies; 30+ messages in thread
From: Daniel P. Berrange @ 2016-09-05 15:16 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Max Reitz, Marc-André Lureau,
Paolo Bonzini, Andreas Färber, Eric Blake,
Daniel P. Berrange
Currently the QmpInputVisitor assumes that all scalar
values are directly represented as their final types.
ie it assumes an 'int' is using QInt, and a 'bool' is
using QBool.
This adds an alternative constructor for QmpInputVisitor
that will set it up such that it expects a QString for
all scalar types instead.
This makes it possible to use QmpInputVisitor with a
QDict produced from QemuOpts, where everything is in
string format.
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
include/qapi/qobject-input-visitor.h | 41 +++++++++-
qapi/qobject-input-visitor.c | 115 +++++++++++++++++++++++++-
tests/test-qobject-input-visitor.c | 152 ++++++++++++++++++++++++++++++++++-
3 files changed, 298 insertions(+), 10 deletions(-)
diff --git a/include/qapi/qobject-input-visitor.h b/include/qapi/qobject-input-visitor.h
index cde328d..aa911cb 100644
--- a/include/qapi/qobject-input-visitor.h
+++ b/include/qapi/qobject-input-visitor.h
@@ -19,12 +19,45 @@
typedef struct QObjectInputVisitor QObjectInputVisitor;
-/*
- * Return a new input visitor that converts a QObject to a QAPI object.
+/**
+ * qobject_input_visitor_new:
+ * @obj: the input object to visit
+ * @strict: whether to require that all input keys are consumed
+ *
+ * Create a new input visitor that converts a QObject 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.
+ *
+ * The returned input visitor should be released by calling
+ * visit_free() when no longer required.
+ *
+ * Returns: a new input visitor
*/
Visitor *qobject_input_visitor_new(QObject *obj, bool strict);
+/**
+ * qobject_string_input_visitor_new:
+ * @obj: the input object to visit
+ *
+ * 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.
+ *
+ * The returned input visitor should be released by calling
+ * visit_free() when no longer required.
+ *
+ * Returns: a new input visitor
+ */
+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..b79c229 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,18 @@ 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;
+
+ parse_option_number(name, qstr ? qstr->string : NULL, &ret, errp);
+ *obj = ret;
+}
+
static void qobject_input_type_uint64(Visitor *v, const char *name,
uint64_t *obj, Error **errp)
{
@@ -279,6 +292,16 @@ 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));
+
+ parse_option_number(name, qstr ? qstr->string : NULL, obj, errp);
+}
+
static void qobject_input_type_bool(Visitor *v, const char *name, bool *obj,
Error **errp)
{
@@ -294,6 +317,16 @@ 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));
+
+ parse_option_bool(name, qstr ? qstr->string : NULL, obj, errp);
+}
+
static void qobject_input_type_str(Visitor *v, const char *name, char **obj,
Error **errp)
{
@@ -312,8 +345,8 @@ static void qobject_input_type_str(Visitor *v, const char *name, char **obj,
}
static void qobject_input_type_number(Visitor *v, const char *name, double *obj,
- Error **errp)
-{
+ Error **errp){
+
QObjectInputVisitor *qiv = to_qiv(v);
QObject *qobj = qobject_input_get_object(qiv, name, true);
QInt *qint;
@@ -335,6 +368,26 @@ 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) {
+ 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 +409,32 @@ 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));
+ int64_t val;
+ char *endptr;
+
+ if (qstr && qstr->string) {
+ val = qemu_strtosz_suffix(qstr->string, &endptr,
+ QEMU_STRTOSZ_DEFSUFFIX_B);
+ if (val < 0 || *endptr) {
+ error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name,
+ "a size value representible as a non-negative int64");
+ return;
+ }
+
+ *obj = val;
+ return;
+ }
+
+ error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
+ "size");
+}
+
static void qobject_input_optional(Visitor *v, const char *name, bool *present)
{
QObjectInputVisitor *qiv = to_qiv(v);
@@ -413,3 +492,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 02895f0..1f60ad8 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,33 @@ static void test_visitor_in_int_overflow(TestInputVisitorData *data,
error_free_or_abort(&err);
}
+static void test_visitor_in_int_autocast(TestInputVisitorData *data,
+ const void *unused)
+{
+ int64_t res = 0, value = -42;
+ Visitor *v;
+
+ v = visitor_input_test_init_full(data, true, true,
+ "\"-42\"");
+
+ visit_type_int(v, NULL, &res, &error_abort);
+ g_assert_cmpint(res, ==, value);
+}
+
+static void test_visitor_in_int_noautocast(TestInputVisitorData *data,
+ const void *unused)
+{
+ int64_t res = 0;
+ Visitor *v;
+ Error *err = NULL;
+
+ v = visitor_input_test_init(data, "\"-42\"");
+
+ visit_type_int(v, NULL, &res, &err);
+ g_assert(err != NULL);
+ error_free(err);
+}
+
static void test_visitor_in_bool(TestInputVisitorData *data,
const void *unused)
{
@@ -121,6 +171,32 @@ static void test_visitor_in_bool(TestInputVisitorData *data,
g_assert_cmpint(res, ==, true);
}
+static void test_visitor_in_bool_autocast(TestInputVisitorData *data,
+ const void *unused)
+{
+ bool res = false;
+ Visitor *v;
+
+ v = visitor_input_test_init_full(data, true, true, "\"yes\"");
+
+ visit_type_bool(v, NULL, &res, &error_abort);
+ g_assert_cmpint(res, ==, true);
+}
+
+static void test_visitor_in_bool_noautocast(TestInputVisitorData *data,
+ const void *unused)
+{
+ bool res = false;
+ Visitor *v;
+ Error *err = NULL;
+
+ v = visitor_input_test_init(data, "\"true\"");
+
+ visit_type_bool(v, NULL, &res, &err);
+ g_assert(err != NULL);
+ error_free(err);
+}
+
static void test_visitor_in_number(TestInputVisitorData *data,
const void *unused)
{
@@ -133,6 +209,58 @@ static void test_visitor_in_number(TestInputVisitorData *data,
g_assert_cmpfloat(res, ==, value);
}
+static void test_visitor_in_number_autocast(TestInputVisitorData *data,
+ const void *unused)
+{
+ double res = 0, value = 3.14;
+ Visitor *v;
+
+ v = visitor_input_test_init_full(data, true, true, "\"3.14\"");
+
+ visit_type_number(v, NULL, &res, &error_abort);
+ g_assert_cmpfloat(res, ==, value);
+}
+
+static void test_visitor_in_number_noautocast(TestInputVisitorData *data,
+ const void *unused)
+{
+ double res = 0;
+ Visitor *v;
+ Error *err = NULL;
+
+ v = visitor_input_test_init(data, "\"3.14\"");
+
+ visit_type_number(v, NULL, &res, &err);
+ g_assert(err != NULL);
+ error_free(err);
+}
+
+static void test_visitor_in_size_autocast(TestInputVisitorData *data,
+ const void *unused)
+{
+ uint64_t res, value = 500 * 1024 * 1024;
+ Visitor *v;
+
+ v = visitor_input_test_init_full(data, true, true, "\"500M\"");
+
+ visit_type_size(v, NULL, &res, &error_abort);
+ g_assert_cmpfloat(res, ==, value);
+}
+
+static void test_visitor_in_size_noautocast(TestInputVisitorData *data,
+ const void *unused)
+{
+ uint64_t res = 0;
+ Visitor *v;
+ Error *err = NULL;
+
+ v = visitor_input_test_init(data, "\"500M\"");
+
+ visit_type_size(v, NULL, &res, &err);
+ g_assert(err != NULL);
+ error_free(err);
+}
+
static void test_visitor_in_string(TestInputVisitorData *data,
const void *unused)
{
@@ -289,7 +417,7 @@ 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 +969,26 @@ int main(int argc, char **argv)
&in_visitor_data, test_visitor_in_int);
input_visitor_test_add("/visitor/input/int_overflow",
&in_visitor_data, test_visitor_in_int_overflow);
+ input_visitor_test_add("/visitor/input/int_autocast",
+ &in_visitor_data, test_visitor_in_int_autocast);
+ input_visitor_test_add("/visitor/input/int_noautocast",
+ &in_visitor_data, test_visitor_in_int_noautocast);
input_visitor_test_add("/visitor/input/bool",
&in_visitor_data, test_visitor_in_bool);
+ input_visitor_test_add("/visitor/input/bool_autocast",
+ &in_visitor_data, test_visitor_in_bool_autocast);
+ input_visitor_test_add("/visitor/input/bool_noautocast",
+ &in_visitor_data, test_visitor_in_bool_noautocast);
input_visitor_test_add("/visitor/input/number",
&in_visitor_data, test_visitor_in_number);
+ input_visitor_test_add("/visitor/input/number_autocast",
+ &in_visitor_data, test_visitor_in_number_autocast);
+ input_visitor_test_add("/visitor/input/number_noautocast",
+ &in_visitor_data, test_visitor_in_number_noautocast);
+ input_visitor_test_add("/visitor/input/size_autocast",
+ &in_visitor_data, test_visitor_in_size_autocast);
+ input_visitor_test_add("/visitor/input/size_noautocast",
+ &in_visitor_data, test_visitor_in_size_noautocast);
input_visitor_test_add("/visitor/input/string",
&in_visitor_data, test_visitor_in_string);
input_visitor_test_add("/visitor/input/enum",
--
2.7.4
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH v11 6/6] qom: support arbitrary non-scalar properties with -object
2016-09-05 15:16 [Qemu-devel] [PATCH v11 0/6] QAPI/QOM work for non-scalar object properties Daniel P. Berrange
` (4 preceding siblings ...)
2016-09-05 15:16 ` [Qemu-devel] [PATCH v11 5/6] qapi: add a QmpInputVisitor that does string conversion Daniel P. Berrange
@ 2016-09-05 15:16 ` Daniel P. Berrange
2016-09-12 18:20 ` Eric Blake
2016-09-14 15:04 ` [Qemu-devel] [PATCH v11 0/6] QAPI/QOM work for non-scalar object properties Kevin Wolf
6 siblings, 1 reply; 30+ messages in thread
From: Daniel P. Berrange @ 2016-09-05 15:16 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Max Reitz, Marc-André Lureau,
Paolo Bonzini, Andreas Färber, Eric Blake,
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 QMPInputVisitor 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 | 314 +++++++++++++++++++++++++++++++++++++++-
5 files changed, 358 insertions(+), 29 deletions(-)
diff --git a/hmp.c b/hmp.c
index cc2056e..f1f75d0 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"
@@ -1693,20 +1693,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 ffc8828..a530272 100644
--- a/qmp.c
+++ b/qmp.c
@@ -667,7 +667,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..3a157af 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((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 42defe7..b2a27f1 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 },
+ { }
+ }
};
@@ -457,7 +677,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))) {
@@ -470,6 +692,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();
@@ -479,6 +707,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));
}
@@ -497,11 +728,90 @@ 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_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);
@@ -513,6 +823,8 @@ 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/createqmp", test_dummy_createqmp);
return g_test_run();
}
--
2.7.4
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v11 3/6] qapi: rename QmpInputVisitor to QObjectInputVisitor
2016-09-05 15:16 ` [Qemu-devel] [PATCH v11 3/6] qapi: rename QmpInputVisitor to QObjectInputVisitor Daniel P. Berrange
@ 2016-09-12 16:19 ` Markus Armbruster
2016-09-13 10:25 ` Daniel P. Berrange
0 siblings, 1 reply; 30+ messages in thread
From: Markus Armbruster @ 2016-09-12 16:19 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: qemu-devel, Max Reitz, Marc-André Lureau, Paolo Bonzini,
Andreas Färber
"Daniel P. Berrange" <berrange@redhat.com> writes:
> The QmpInputVisitor has no direct dependancy 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 convertor.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> docs/qapi-code-gen.txt | 2 +-
> ...qmp-input-visitor.h => qobject-input-visitor.h} | 10 +-
> 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 +-
> 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} | 4 +-
> ...nput-visitor.c => test-qobject-input-visitor.c} | 4 +-
> tests/test-string-input-visitor.c | 2 +-
> tests/test-visitor-serialization.c | 4 +-
> util/qemu-sockets.c | 2 +-
> 17 files changed, 121 insertions(+), 118 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} (99%)
> rename tests/{test-qmp-input-visitor.c => test-qobject-input-visitor.c} (99%)
$ git-grep -Ei 'qmp.?input.*?vis'
include/qapi/visitor.h: * the dealloc and QMP input/output visitors are general, the string,
target-s390x/cpu_models.c:#include "qapi/qmp-input-visitor.h"
target-s390x/cpu_models.c: visitor = qmp_input_visitor_new(info->props, true);
tests/test-qobject-input-strict.c: * QMP Input Visitor unit-tests (strict mode).
tests/test-qobject-input-visitor.c: * QMP Input Visitor unit-tests.
>
> 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/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)
The replacement of qmp_input by qobject_input makes our long function
names even longer. Oh well.
> {
> 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 b6d531e..ffc8828 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"
> @@ -666,7 +666,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/tests/.gitignore b/tests/.gitignore
> index dbb5263..4ec0a1a 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 14be491..5ac61b7 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)
> @@ -415,7 +415,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 \
> @@ -514,8 +514,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 99%
> rename from tests/test-qmp-input-strict.c
> rename to tests/test-qobject-input-strict.c
> index 814550a..546f8af 100644
> --- a/tests/test-qmp-input-strict.c
> +++ b/tests/test-qobject-input-strict.c
> @@ -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..02895f0 100644
> --- a/tests/test-qmp-input-visitor.c
> +++ b/tests/test-qobject-input-visitor.c
> @@ -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 d837ebe..9bdcd74 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 2aed799..dd9ad44 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"
I guess we could simply live with the badly chosen name (and avoid the
churn), but since you did the work already...
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v11 4/6] qapi: rename QmpOutputVisitor to QObjectOutputVisitor
2016-09-05 15:16 ` [Qemu-devel] [PATCH v11 4/6] qapi: rename QmpOutputVisitor to QObjectOutputVisitor Daniel P. Berrange
@ 2016-09-12 16:20 ` Markus Armbruster
2016-09-13 10:25 ` Daniel P. Berrange
2016-09-12 18:24 ` Eric Blake
1 sibling, 1 reply; 30+ messages in thread
From: Markus Armbruster @ 2016-09-12 16:20 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: qemu-devel, Max Reitz, Marc-André Lureau, Paolo Bonzini,
Andreas Färber
"Daniel P. Berrange" <berrange@redhat.com> writes:
> The QmpOutputVisitor has no direct dependancy 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 convertor.
>
> 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} | 4 +-
> tests/test-string-output-visitor.c | 2 +-
> tests/test-visitor-serialization.c | 4 +-
> util/qemu-sockets.c | 2 +-
> 20 files changed, 290 insertions(+), 292 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%)
$ git-grep -Ei 'qmp.?output.*?vis'
tests/test-qobject-output-visitor.c: * QMP Output Visitor unit-tests.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v11 5/6] qapi: add a QmpInputVisitor that does string conversion
2016-09-05 15:16 ` [Qemu-devel] [PATCH v11 5/6] qapi: add a QmpInputVisitor that does string conversion Daniel P. Berrange
@ 2016-09-12 16:21 ` Markus Armbruster
2016-09-12 16:23 ` Daniel P. Berrange
2016-09-12 18:39 ` Eric Blake
` (2 subsequent siblings)
3 siblings, 1 reply; 30+ messages in thread
From: Markus Armbruster @ 2016-09-12 16:21 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: qemu-devel, Max Reitz, Marc-André Lureau, Paolo Bonzini,
Andreas Färber
"Daniel P. Berrange" <berrange@redhat.com> writes:
> Currently the QmpInputVisitor assumes that all scalar
> values are directly represented as their final types.
> ie it assumes an 'int' is using QInt, and a 'bool' is
> using QBool.
>
> This adds an alternative constructor for QmpInputVisitor
> that will set it up such that it expects a QString for
> all scalar types instead.
>
> This makes it possible to use QmpInputVisitor with a
> QDict produced from QemuOpts, where everything is in
> string format.
>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Ignorant question: is this used by the next patch?
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v11 5/6] qapi: add a QmpInputVisitor that does string conversion
2016-09-12 16:21 ` Markus Armbruster
@ 2016-09-12 16:23 ` Daniel P. Berrange
2016-09-12 16:30 ` Daniel P. Berrange
0 siblings, 1 reply; 30+ messages in thread
From: Daniel P. Berrange @ 2016-09-12 16:23 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-devel, Max Reitz, Marc-André Lureau, Paolo Bonzini,
Andreas Färber
On Mon, Sep 12, 2016 at 06:21:32PM +0200, Markus Armbruster wrote:
> "Daniel P. Berrange" <berrange@redhat.com> writes:
>
> > Currently the QmpInputVisitor assumes that all scalar
> > values are directly represented as their final types.
> > ie it assumes an 'int' is using QInt, and a 'bool' is
> > using QBool.
> >
> > This adds an alternative constructor for QmpInputVisitor
> > that will set it up such that it expects a QString for
> > all scalar types instead.
> >
> > This makes it possible to use QmpInputVisitor with a
> > QDict produced from QemuOpts, where everything is in
> > string format.
> >
> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
>
> Ignorant question: is this used by the next patch?
Yes indeed :-)
We use the qdict_crumpl() method to turn the command line QemuOpts
into a QDict, and then this new mode for QmpInputVisitor for the
visiting this QDict, thus eliminating need to use OptsVisitor in
QOM.
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] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v11 5/6] qapi: add a QmpInputVisitor that does string conversion
2016-09-12 16:23 ` Daniel P. Berrange
@ 2016-09-12 16:30 ` Daniel P. Berrange
0 siblings, 0 replies; 30+ messages in thread
From: Daniel P. Berrange @ 2016-09-12 16:30 UTC (permalink / raw)
To: Markus Armbruster
Cc: Paolo Bonzini, Marc-André Lureau, qemu-devel,
Andreas Färber, Max Reitz
On Mon, Sep 12, 2016 at 05:23:47PM +0100, Daniel P. Berrange wrote:
> On Mon, Sep 12, 2016 at 06:21:32PM +0200, Markus Armbruster wrote:
> > "Daniel P. Berrange" <berrange@redhat.com> writes:
> >
> > > Currently the QmpInputVisitor assumes that all scalar
> > > values are directly represented as their final types.
> > > ie it assumes an 'int' is using QInt, and a 'bool' is
> > > using QBool.
> > >
> > > This adds an alternative constructor for QmpInputVisitor
> > > that will set it up such that it expects a QString for
> > > all scalar types instead.
> > >
> > > This makes it possible to use QmpInputVisitor with a
> > > QDict produced from QemuOpts, where everything is in
> > > string format.
> > >
> > > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> >
> > Ignorant question: is this used by the next patch?
>
> Yes indeed :-)
>
> We use the qdict_crumpl() method to turn the command line QemuOpts
> into a QDict, and then this new mode for QmpInputVisitor for the
> visiting this QDict, thus eliminating need to use OptsVisitor in
> QOM.
Oh and obviously my commit message needs fixing to say
QObjectInputVisitor not QmpInputVisitor now :-)
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] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v11 6/6] qom: support arbitrary non-scalar properties with -object
2016-09-05 15:16 ` [Qemu-devel] [PATCH v11 6/6] qom: support arbitrary non-scalar properties with -object Daniel P. Berrange
@ 2016-09-12 18:20 ` Eric Blake
2016-09-13 10:32 ` Daniel P. Berrange
0 siblings, 1 reply; 30+ messages in thread
From: Eric Blake @ 2016-09-12 18:20 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel
Cc: Markus Armbruster, Max Reitz, Marc-André Lureau,
Paolo Bonzini, Andreas Färber
[-- Attachment #1: Type: text/plain, Size: 4073 bytes --]
On 09/05/2016 10:16 AM, 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.
>
> 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 QMPInputVisitor for
QObjectInputVisitor
> 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>
I still haven't looked closely at the intermediate patches in this
series, but I do like the end result, so I'll start with a review of
this one.
> @@ -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((QDict *)pobj, v, errp);
This cast looks fishy; I think you want qobject_to_qdict(pobj) for
absolute safety (if some future change causes QDict to not contain
QObject at offset 0). Besides, qdict_crumple() can return a QList
instead of a QDict in some cases, so asserting that qobject_to_qdict()
did not return NULL may be worthwhile to prove that this particular
crumple never gives us something unexpected.
> +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;
Actually, what happens if I do an optstr of "qemu-dummy,1=foo,2=bar"?
Is that rejected (because '1' and '2' are not valid key names) or does
it try to create a 2-element list instead of the usual dict? That is,
if a list is something the user can trigger at the CLI, then we need
better error handling than just an assertion that we actually get a
QDict above.
But overall I like this patch.
--
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] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v11 4/6] qapi: rename QmpOutputVisitor to QObjectOutputVisitor
2016-09-05 15:16 ` [Qemu-devel] [PATCH v11 4/6] qapi: rename QmpOutputVisitor to QObjectOutputVisitor Daniel P. Berrange
2016-09-12 16:20 ` Markus Armbruster
@ 2016-09-12 18:24 ` Eric Blake
1 sibling, 0 replies; 30+ messages in thread
From: Eric Blake @ 2016-09-12 18:24 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel
Cc: Markus Armbruster, Max Reitz, Marc-André Lureau,
Paolo Bonzini, Andreas Färber
[-- Attachment #1: Type: text/plain, Size: 710 bytes --]
On 09/05/2016 10:16 AM, Daniel P. Berrange wrote:
> The QmpOutputVisitor has no direct dependancy on QMP. It is
s/dependancy/dependency/
> valid to use it anywhere that one wants a QObject. Rename it
> to better reflect its functionality as a generic QAPI
> to QObject convertor.
c/convertor/converter/
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
Other than the last couple spots found by Markus' grep, both 3/6 and
this 4/6 are fairly mechanical and verified by the compiler, so on the
respin, you can add:
Reviewed-by: Eric Blake <eblake@redhat.com>
--
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] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v11 5/6] qapi: add a QmpInputVisitor that does string conversion
2016-09-05 15:16 ` [Qemu-devel] [PATCH v11 5/6] qapi: add a QmpInputVisitor that does string conversion Daniel P. Berrange
2016-09-12 16:21 ` Markus Armbruster
@ 2016-09-12 18:39 ` Eric Blake
2016-09-13 10:22 ` Daniel P. Berrange
2016-09-13 9:05 ` Markus Armbruster
2016-09-14 14:59 ` Kevin Wolf
3 siblings, 1 reply; 30+ messages in thread
From: Eric Blake @ 2016-09-12 18:39 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel
Cc: Markus Armbruster, Max Reitz, Marc-André Lureau,
Paolo Bonzini, Andreas Färber
[-- Attachment #1: Type: text/plain, Size: 6957 bytes --]
On 09/05/2016 10:16 AM, Daniel P. Berrange wrote:
> Currently the QmpInputVisitor assumes that all scalar
> values are directly represented as their final types.
> ie it assumes an 'int' is using QInt, and a 'bool' is
> using QBool.
>
> This adds an alternative constructor for QmpInputVisitor
> that will set it up such that it expects a QString for
> all scalar types instead.
>
> This makes it possible to use QmpInputVisitor with a
> QDict produced from QemuOpts, where everything is in
> string format.
>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> include/qapi/qobject-input-visitor.h | 41 +++++++++-
> qapi/qobject-input-visitor.c | 115 +++++++++++++++++++++++++-
> tests/test-qobject-input-visitor.c | 152 ++++++++++++++++++++++++++++++++++-
> 3 files changed, 298 insertions(+), 10 deletions(-)
>
> +/**
> + * qobject_string_input_visitor_new:
> + * @obj: the input object to visit
> + *
> + * 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.
Good; I like that strict mode on the new constructor is not optional.
> +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;
Uninitialized...
> +
> + parse_option_number(name, qstr ? qstr->string : NULL, &ret, errp);
...and parse_option_number() explicitly leaves *ret untouched on error...
> + *obj = ret;
so if errp was set, then *obj now contains uninitialized memory. I
guess valgrind is smart enough to only complain if callers then try to
branch based on that value (that is, assigning one uninit location to
another silently propagates uninit status to the new location, but it is
only when you branch or otherwise USE uninit data, not just copy it,
that valgrind complains). On the other hand, if the caller explicitly
set value = 0 before calling qobject_input_type_int64_str(&value), we've
now messed with the caller's expectations of value being at its pre-set
value on error.
> @@ -312,8 +345,8 @@ static void qobject_input_type_str(Visitor *v, const char *name, char **obj,
> }
>
> static void qobject_input_type_number(Visitor *v, const char *name, double *obj,
> - Error **errp)
> -{
> + Error **errp){
> +
Spurious hunk.
> +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));
> + int64_t val;
> + char *endptr;
> +
> + if (qstr && qstr->string) {
> + val = qemu_strtosz_suffix(qstr->string, &endptr,
> + QEMU_STRTOSZ_DEFSUFFIX_B);
> + if (val < 0 || *endptr) {
> + error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name,
> + "a size value representible as a non-negative int64");
s/representible/representable/
> +++ b/tests/test-qobject-input-visitor.c
>
> +static void test_visitor_in_int_autocast(TestInputVisitorData *data,
> + const void *unused)
> +{
> + int64_t res = 0, value = -42;
> + Visitor *v;
> +
> + v = visitor_input_test_init_full(data, true, true,
> + "\"-42\"");
> +
> + visit_type_int(v, NULL, &res, &error_abort);
> + g_assert_cmpint(res, ==, value);
> +}
> +
> +static void test_visitor_in_int_noautocast(TestInputVisitorData *data,
> + const void *unused)
> +{
> + int64_t res = 0;
> + Visitor *v;
> + Error *err = NULL;
> +
> + v = visitor_input_test_init(data, "\"-42\"");
> +
> + visit_type_int(v, NULL, &res, &err);
> + g_assert(err != NULL);
> + error_free(err);
> +}
So we've previously tested that int->int works without autocast, and you
add that str->int works with autocast, and that str->int fails without
autocast. Is it also worth testing that int->int fails with autocast
(that is, when doing string parsing, a QInt is intentionally rejected
even though we are parsing to get an int result)?
> @@ -841,10 +969,26 @@ int main(int argc, char **argv)
> &in_visitor_data, test_visitor_in_int);
> input_visitor_test_add("/visitor/input/int_overflow",
> &in_visitor_data, test_visitor_in_int_overflow);
> + input_visitor_test_add("/visitor/input/int_autocast",
> + &in_visitor_data, test_visitor_in_int_autocast);
> + input_visitor_test_add("/visitor/input/int_noautocast",
> + &in_visitor_data, test_visitor_in_int_noautocast);
> input_visitor_test_add("/visitor/input/bool",
> &in_visitor_data, test_visitor_in_bool);
> + input_visitor_test_add("/visitor/input/bool_autocast",
> + &in_visitor_data, test_visitor_in_bool_autocast);
> + input_visitor_test_add("/visitor/input/bool_noautocast",
> + &in_visitor_data, test_visitor_in_bool_noautocast);
> input_visitor_test_add("/visitor/input/number",
> &in_visitor_data, test_visitor_in_number);
> + input_visitor_test_add("/visitor/input/number_autocast",
> + &in_visitor_data, test_visitor_in_number_autocast);
> + input_visitor_test_add("/visitor/input/number_noautocast",
> + &in_visitor_data, test_visitor_in_number_noautocast);
> + input_visitor_test_add("/visitor/input/size_autocast",
> + &in_visitor_data, test_visitor_in_size_autocast);
> + input_visitor_test_add("/visitor/input/size_noautocast",
> + &in_visitor_data, test_visitor_in_size_noautocast);
Similar question for autocast causing QBool->bool, QInt->int under size,
and QFloat->number failures.
--
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] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v11 5/6] qapi: add a QmpInputVisitor that does string conversion
2016-09-05 15:16 ` [Qemu-devel] [PATCH v11 5/6] qapi: add a QmpInputVisitor that does string conversion Daniel P. Berrange
2016-09-12 16:21 ` Markus Armbruster
2016-09-12 18:39 ` Eric Blake
@ 2016-09-13 9:05 ` Markus Armbruster
2016-09-13 9:33 ` Daniel P. Berrange
2016-09-14 14:59 ` Kevin Wolf
3 siblings, 1 reply; 30+ messages in thread
From: Markus Armbruster @ 2016-09-13 9:05 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: qemu-devel, Max Reitz, Marc-André Lureau, Paolo Bonzini,
Andreas Färber
"Daniel P. Berrange" <berrange@redhat.com> writes:
> Currently the QmpInputVisitor assumes that all scalar
> values are directly represented as their final types.
> ie it assumes an 'int' is using QInt, and a 'bool' is
> using QBool.
>
> This adds an alternative constructor for QmpInputVisitor
> that will set it up such that it expects a QString for
> all scalar types instead.
>
> This makes it possible to use QmpInputVisitor with a
> QDict produced from QemuOpts, where everything is in
> string format.
Can you explain how this is related to the Options visitor?
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> include/qapi/qobject-input-visitor.h | 41 +++++++++-
> qapi/qobject-input-visitor.c | 115 +++++++++++++++++++++++++-
> tests/test-qobject-input-visitor.c | 152 ++++++++++++++++++++++++++++++++++-
> 3 files changed, 298 insertions(+), 10 deletions(-)
>
> diff --git a/include/qapi/qobject-input-visitor.h b/include/qapi/qobject-input-visitor.h
> index cde328d..aa911cb 100644
> --- a/include/qapi/qobject-input-visitor.h
> +++ b/include/qapi/qobject-input-visitor.h
> @@ -19,12 +19,45 @@
>
> typedef struct QObjectInputVisitor QObjectInputVisitor;
>
> -/*
> - * Return a new input visitor that converts a QObject to a QAPI object.
> +/**
> + * qobject_input_visitor_new:
> + * @obj: the input object to visit
> + * @strict: whether to require that all input keys are consumed
> + *
> + * Create a new input visitor that converts a QObject 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.
> + *
> + * The returned input visitor should be released by calling
> + * visit_free() when no longer required.
> + *
> + * Returns: a new input visitor
> */
GDK-Doc style is an egregious waste of screen space and reader
bandwidth. Much of the text is spent on restating the obvious, drowning
out the parts that are actually interesting.
Compare:
/*
- * Return a new input visitor that converts a QObject to a QAPI object.
+ * Create an input visitor that converts @obj to a QAPI object.
*
- * Set @strict to reject a parse that doesn't consume all keys of a
- * dictionary; otherwise excess input is ignored.
+ *
+ * If @strict is true, the visit is expected to consume all of @obj's
+ * keys. Else, excess keys are silently ignored.
+ *
+ * Return the input visitor. It should be destroyed with visit_free().
+ */
Like your version, this is vague on what exactly happens when a strict
visit fails to consume all keys. It's more explicitly vague, though :)
GDK-Doc is that way so that a fairly dumb tool can generate fairly
usable library reference documentation. We're not using that tool.
This is not a library. I loathe having my read bandwidth wasted.
> Visitor *qobject_input_visitor_new(QObject *obj, bool strict);
>
> +/**
> + * qobject_string_input_visitor_new:
> + * @obj: the input object to visit
> + *
> + * 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.
> + *
> + * The returned input visitor should be released by calling
> + * visit_free() when no longer required.
> + *
> + * Returns: a new input visitor
> + */
> +Visitor *qobject_string_input_visitor_new(QObject *obj);
> +
> #endif
[...]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v11 5/6] qapi: add a QmpInputVisitor that does string conversion
2016-09-13 9:05 ` Markus Armbruster
@ 2016-09-13 9:33 ` Daniel P. Berrange
2016-09-13 13:32 ` Markus Armbruster
0 siblings, 1 reply; 30+ messages in thread
From: Daniel P. Berrange @ 2016-09-13 9:33 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-devel, Max Reitz, Marc-André Lureau, Paolo Bonzini,
Andreas Färber
On Tue, Sep 13, 2016 at 11:05:08AM +0200, Markus Armbruster wrote:
> "Daniel P. Berrange" <berrange@redhat.com> writes:
>
> > Currently the QmpInputVisitor assumes that all scalar
> > values are directly represented as their final types.
> > ie it assumes an 'int' is using QInt, and a 'bool' is
> > using QBool.
> >
> > This adds an alternative constructor for QmpInputVisitor
> > that will set it up such that it expects a QString for
> > all scalar types instead.
> >
> > This makes it possible to use QmpInputVisitor with a
> > QDict produced from QemuOpts, where everything is in
> > string format.
>
> Can you explain how this is related to the Options visitor?
The intention is that this can replace the existing OptsVisitor,
for cases that don't rely on the magic "List of scalars" semantics
of OptsVisitor - eg where 'foo=3,foo=5,foo=533' gets turned into
a QList.
When using QemuOpts w/ qdict_crumple + QmpInputVisitor, you would
do list of scalars in different manner 'foo.1=3,foo.2=5,foo.3=533'
since this syntax is extendable to deal with arbitrary nesting of
dicts + lists, where as the OptsVisitor syntax cannot be extended
in a back-compatible manner.
This series converts -object to use QmpInputVisitor and this is
safe todo right now, since no currently created QOM object has
a property which is a list of scalars and this the changed syntax
is not going to affect any existing usage.
The -drive arg can be converted to QmpInputVisitor too, since
the qdict_crumple/QmpInputVisitor combination was explicitly
designed to be 100% compatible with -drive syntax for blockdevs
nested options.
Other args would have to be considered on a case-by-case basis
to see if they rely on the magic list of scalars syntax used
by OptsVisitor. Probably only a few of them need this.
> > ---
> > include/qapi/qobject-input-visitor.h | 41 +++++++++-
> > qapi/qobject-input-visitor.c | 115 +++++++++++++++++++++++++-
> > tests/test-qobject-input-visitor.c | 152 ++++++++++++++++++++++++++++++++++-
> > 3 files changed, 298 insertions(+), 10 deletions(-)
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] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v11 5/6] qapi: add a QmpInputVisitor that does string conversion
2016-09-12 18:39 ` Eric Blake
@ 2016-09-13 10:22 ` Daniel P. Berrange
0 siblings, 0 replies; 30+ messages in thread
From: Daniel P. Berrange @ 2016-09-13 10:22 UTC (permalink / raw)
To: Eric Blake
Cc: qemu-devel, Markus Armbruster, Max Reitz, Marc-André Lureau,
Paolo Bonzini, Andreas Färber
On Mon, Sep 12, 2016 at 01:39:50PM -0500, Eric Blake wrote:
> On 09/05/2016 10:16 AM, Daniel P. Berrange wrote:
> > Currently the QmpInputVisitor assumes that all scalar
> > values are directly represented as their final types.
> > ie it assumes an 'int' is using QInt, and a 'bool' is
> > using QBool.
> >
> > This adds an alternative constructor for QmpInputVisitor
> > that will set it up such that it expects a QString for
> > all scalar types instead.
> >
> > This makes it possible to use QmpInputVisitor with a
> > QDict produced from QemuOpts, where everything is in
> > string format.
> >
> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> > include/qapi/qobject-input-visitor.h | 41 +++++++++-
> > qapi/qobject-input-visitor.c | 115 +++++++++++++++++++++++++-
> > tests/test-qobject-input-visitor.c | 152 ++++++++++++++++++++++++++++++++++-
> > 3 files changed, 298 insertions(+), 10 deletions(-)
> >
>
> > +/**
> > + * qobject_string_input_visitor_new:
> > + * @obj: the input object to visit
> > + *
> > + * 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.
>
> Good; I like that strict mode on the new constructor is not optional.
>
>
> > +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;
>
> Uninitialized...
>
> > +
> > + parse_option_number(name, qstr ? qstr->string : NULL, &ret, errp);
>
> ...and parse_option_number() explicitly leaves *ret untouched on error...
>
> > + *obj = ret;
>
> so if errp was set, then *obj now contains uninitialized memory. I
> guess valgrind is smart enough to only complain if callers then try to
> branch based on that value (that is, assigning one uninit location to
> another silently propagates uninit status to the new location, but it is
> only when you branch or otherwise USE uninit data, not just copy it,
> that valgrind complains). On the other hand, if the caller explicitly
> set value = 0 before calling qobject_input_type_int64_str(&value), we've
> now messed with the caller's expectations of value being at its pre-set
> value on error.
I'll use a local Error, so we can avoid the uninitialized
value and also avoid splattering *obj on failure.
> > +++ b/tests/test-qobject-input-visitor.c
> >
> > +static void test_visitor_in_int_autocast(TestInputVisitorData *data,
> > + const void *unused)
> > +{
> > + int64_t res = 0, value = -42;
> > + Visitor *v;
> > +
> > + v = visitor_input_test_init_full(data, true, true,
> > + "\"-42\"");
> > +
> > + visit_type_int(v, NULL, &res, &error_abort);
> > + g_assert_cmpint(res, ==, value);
> > +}
> > +
> > +static void test_visitor_in_int_noautocast(TestInputVisitorData *data,
> > + const void *unused)
> > +{
> > + int64_t res = 0;
> > + Visitor *v;
> > + Error *err = NULL;
> > +
> > + v = visitor_input_test_init(data, "\"-42\"");
> > +
> > + visit_type_int(v, NULL, &res, &err);
> > + g_assert(err != NULL);
> > + error_free(err);
> > +}
>
> So we've previously tested that int->int works without autocast, and you
> add that str->int works with autocast, and that str->int fails without
> autocast. Is it also worth testing that int->int fails with autocast
> (that is, when doing string parsing, a QInt is intentionally rejected
> even though we are parsing to get an int result)?
[snip]
> Similar question for autocast causing QBool->bool, QInt->int under size,
> and QFloat->number failures.
You always notice all the edge cases :-)
I've added such tests and it exposed a bug in my impl which is nice :-)
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] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v11 3/6] qapi: rename QmpInputVisitor to QObjectInputVisitor
2016-09-12 16:19 ` Markus Armbruster
@ 2016-09-13 10:25 ` Daniel P. Berrange
0 siblings, 0 replies; 30+ messages in thread
From: Daniel P. Berrange @ 2016-09-13 10:25 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-devel, Max Reitz, Marc-André Lureau, Paolo Bonzini,
Andreas Färber
On Mon, Sep 12, 2016 at 06:19:08PM +0200, Markus Armbruster wrote:
> "Daniel P. Berrange" <berrange@redhat.com> writes:
>
> > The QmpInputVisitor has no direct dependancy 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 convertor.
> >
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> > docs/qapi-code-gen.txt | 2 +-
> > ...qmp-input-visitor.h => qobject-input-visitor.h} | 10 +-
> > 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 +-
> > 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} | 4 +-
> > ...nput-visitor.c => test-qobject-input-visitor.c} | 4 +-
> > tests/test-string-input-visitor.c | 2 +-
> > tests/test-visitor-serialization.c | 4 +-
> > util/qemu-sockets.c | 2 +-
> > 17 files changed, 121 insertions(+), 118 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} (99%)
> > rename tests/{test-qmp-input-visitor.c => test-qobject-input-visitor.c} (99%)
>
> $ git-grep -Ei 'qmp.?input.*?vis'
> include/qapi/visitor.h: * the dealloc and QMP input/output visitors are general, the string,
> target-s390x/cpu_models.c:#include "qapi/qmp-input-visitor.h"
> target-s390x/cpu_models.c: visitor = qmp_input_visitor_new(info->props, true);
> tests/test-qobject-input-strict.c: * QMP Input Visitor unit-tests (strict mode).
> tests/test-qobject-input-visitor.c: * QMP Input Visitor unit-tests.
Opps, yes will fix those.
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] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v11 4/6] qapi: rename QmpOutputVisitor to QObjectOutputVisitor
2016-09-12 16:20 ` Markus Armbruster
@ 2016-09-13 10:25 ` Daniel P. Berrange
0 siblings, 0 replies; 30+ messages in thread
From: Daniel P. Berrange @ 2016-09-13 10:25 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-devel, Max Reitz, Marc-André Lureau, Paolo Bonzini,
Andreas Färber
On Mon, Sep 12, 2016 at 06:20:36PM +0200, Markus Armbruster wrote:
> "Daniel P. Berrange" <berrange@redhat.com> writes:
>
> > The QmpOutputVisitor has no direct dependancy 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 convertor.
> >
> > 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} | 4 +-
> > tests/test-string-output-visitor.c | 2 +-
> > tests/test-visitor-serialization.c | 4 +-
> > util/qemu-sockets.c | 2 +-
> > 20 files changed, 290 insertions(+), 292 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%)
>
> $ git-grep -Ei 'qmp.?output.*?vis'
> tests/test-qobject-output-visitor.c: * QMP Output Visitor unit-tests.
Yep, will fix that.
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] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v11 6/6] qom: support arbitrary non-scalar properties with -object
2016-09-12 18:20 ` Eric Blake
@ 2016-09-13 10:32 ` Daniel P. Berrange
0 siblings, 0 replies; 30+ messages in thread
From: Daniel P. Berrange @ 2016-09-13 10:32 UTC (permalink / raw)
To: Eric Blake
Cc: qemu-devel, Markus Armbruster, Max Reitz, Marc-André Lureau,
Paolo Bonzini, Andreas Färber
On Mon, Sep 12, 2016 at 01:20:25PM -0500, Eric Blake wrote:
> On 09/05/2016 10:16 AM, 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.
> >
>
> > 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 QMPInputVisitor for
>
> QObjectInputVisitor
>
> > 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>
>
> I still haven't looked closely at the intermediate patches in this
> series, but I do like the end result, so I'll start with a review of
> this one.
>
> > @@ -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((QDict *)pobj, v, errp);
>
> This cast looks fishy; I think you want qobject_to_qdict(pobj) for
> absolute safety (if some future change causes QDict to not contain
> QObject at offset 0). Besides, qdict_crumple() can return a QList
> instead of a QDict in some cases, so asserting that qobject_to_qdict()
> did not return NULL may be worthwhile to prove that this particular
> crumple never gives us something unexpected.
>
> > +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;
>
> Actually, what happens if I do an optstr of "qemu-dummy,1=foo,2=bar"?
> Is that rejected (because '1' and '2' are not valid key names) or does
> it try to create a 2-element list instead of the usual dict? That is,
> if a list is something the user can trigger at the CLI, then we need
> better error handling than just an assertion that we actually get a
> QDict above.
With that example you give, qdict_crumple() will raise an error because
you have a mixture of list and non-list keys at the top level. IOW, the
crumple method is enforcing that you must only have a dict at the top.
I'll add a test for this to validate the behaviour too.
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] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v11 5/6] qapi: add a QmpInputVisitor that does string conversion
2016-09-13 9:33 ` Daniel P. Berrange
@ 2016-09-13 13:32 ` Markus Armbruster
2016-09-13 13:47 ` Daniel P. Berrange
0 siblings, 1 reply; 30+ messages in thread
From: Markus Armbruster @ 2016-09-13 13:32 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: Paolo Bonzini, Marc-André Lureau, qemu-devel,
Andreas Färber, Max Reitz
"Daniel P. Berrange" <berrange@redhat.com> writes:
> On Tue, Sep 13, 2016 at 11:05:08AM +0200, Markus Armbruster wrote:
>> "Daniel P. Berrange" <berrange@redhat.com> writes:
>>
>> > Currently the QmpInputVisitor assumes that all scalar
>> > values are directly represented as their final types.
>> > ie it assumes an 'int' is using QInt, and a 'bool' is
>> > using QBool.
>> >
>> > This adds an alternative constructor for QmpInputVisitor
>> > that will set it up such that it expects a QString for
>> > all scalar types instead.
>> >
>> > This makes it possible to use QmpInputVisitor with a
>> > QDict produced from QemuOpts, where everything is in
>> > string format.
>>
>> Can you explain how this is related to the Options visitor?
>
> The intention is that this can replace the existing OptsVisitor,
> for cases that don't rely on the magic "List of scalars" semantics
> of OptsVisitor - eg where 'foo=3,foo=5,foo=533' gets turned into
> a QList.
There's also the even more magical foo=3-5,foo=533.
> When using QemuOpts w/ qdict_crumple + QmpInputVisitor, you would
> do list of scalars in different manner 'foo.1=3,foo.2=5,foo.3=533'
> since this syntax is extendable to deal with arbitrary nesting of
> dicts + lists, where as the OptsVisitor syntax cannot be extended
> in a back-compatible manner.
>
> This series converts -object to use QmpInputVisitor and this is
> safe todo right now, since no currently created QOM object has
> a property which is a list of scalars and this the changed syntax
> is not going to affect any existing usage.
Peeking at the patch... aha! Instead of having the options visitor
visit the QemuOpts, you convert the QemuOpts to a QDict, which you then
visit with your new visitor. Less efficient, because you have to build
and destroy the intermediate QDict. Not an issue when processing
configuration or even monitor commands, of course.
I guess you convert to QDict so that the work of going from a
QemuOpts-style string using -drive conventions to a visit splits into
manageable chunks more easily, preferably into chunks that already
exist: parse string into QemuOpts, convert to QDict, crumple, visit,
destroy QDict.
Still, I take the conversion as a signal that our data structures are
wrong. Wild idea: should QemuOpts use a QDict rather than a QTAILQ of
QemuOpt to store options?
The pipeline then becomes parse string into QemuOpts, clone its QDict,
crumple, visit, destroy QDict. Next step would be crumpling in place,
i.e. parse string into nested QemuOpts, get its QDict, visit. Mind, I'm
not demanding you to do that now, I'm just trying to figure out whether
this series is shoveling into or out of the QemuOpts hole :)
> The -drive arg can be converted to QmpInputVisitor too, since
> the qdict_crumple/QmpInputVisitor combination was explicitly
> designed to be 100% compatible with -drive syntax for blockdevs
> nested options.
You abstaining from inventing yet another option syntax dialect is
appreciated.
> Other args would have to be considered on a case-by-case basis
> to see if they rely on the magic list of scalars syntax used
> by OptsVisitor. Probably only a few of them need this.
Obvious maintainer question: what would it take to get rid of the
options visitor entirely? Because this maintainer looks on additions to
his fiefdom much more kindly when they're balanced by subtractions.
I guess all the uses that don't rely on the "list of scalars" magic are
easy prey: replace by conversion to intermediate QDict + your new
visitor.
What about the ones that do rely on it? Which ones do? Any ideas on
how to change them so that they don't require a complete options
visitor?
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v11 5/6] qapi: add a QmpInputVisitor that does string conversion
2016-09-13 13:32 ` Markus Armbruster
@ 2016-09-13 13:47 ` Daniel P. Berrange
0 siblings, 0 replies; 30+ messages in thread
From: Daniel P. Berrange @ 2016-09-13 13:47 UTC (permalink / raw)
To: Markus Armbruster
Cc: Paolo Bonzini, Marc-André Lureau, qemu-devel,
Andreas Färber, Max Reitz
On Tue, Sep 13, 2016 at 03:32:33PM +0200, Markus Armbruster wrote:
> "Daniel P. Berrange" <berrange@redhat.com> writes:
> > When using QemuOpts w/ qdict_crumple + QmpInputVisitor, you would
> > do list of scalars in different manner 'foo.1=3,foo.2=5,foo.3=533'
> > since this syntax is extendable to deal with arbitrary nesting of
> > dicts + lists, where as the OptsVisitor syntax cannot be extended
> > in a back-compatible manner.
> >
> > This series converts -object to use QmpInputVisitor and this is
> > safe todo right now, since no currently created QOM object has
> > a property which is a list of scalars and this the changed syntax
> > is not going to affect any existing usage.
>
> Peeking at the patch... aha! Instead of having the options visitor
> visit the QemuOpts, you convert the QemuOpts to a QDict, which you then
> visit with your new visitor. Less efficient, because you have to build
> and destroy the intermediate QDict. Not an issue when processing
> configuration or even monitor commands, of course.
>
> I guess you convert to QDict so that the work of going from a
> QemuOpts-style string using -drive conventions to a visit splits into
> manageable chunks more easily, preferably into chunks that already
> exist: parse string into QemuOpts, convert to QDict, crumple, visit,
> destroy QDict.
FWIW, I did originally try to modify the QemuOpts visitor directly
to support visiting nested data structure, but I basically ended
up re-inventing alot of code from qdict and indeed having to use
a qdict internally to QemuOpts to store intermediate bits. At
that point I realized it was clearly silly to try to shoe horn
it into QemuOpts visitor directly, since I was basically creating
qdict_crumple() indirectly and then essentially making QemuOpts
visitor have the same logic as QmpInputVisitor.
> Still, I take the conversion as a signal that our data structures are
> wrong. Wild idea: should QemuOpts use a QDict rather than a QTAILQ of
> QemuOpt to store options?
If QemuOpts used a QDict internally, then you avoid the first
qemu_opts_to_qdict() call before qdict_crumple(), but everything
else basically stays the same. So a minor improvement, but nothing
ground-shaking.
> The pipeline then becomes parse string into QemuOpts, clone its QDict,
> crumple, visit, destroy QDict. Next step would be crumpling in place,
> i.e. parse string into nested QemuOpts, get its QDict, visit. Mind, I'm
> not demanding you to do that now, I'm just trying to figure out whether
> this series is shoveling into or out of the QemuOpts hole :)
Yes, I guess the interesting win would be if qemu_opts_do_parse()
could directly populate a qdict in crumpled format from the start,
thus avoiding the late qdict_crumple call altogether.
> > Other args would have to be considered on a case-by-case basis
> > to see if they rely on the magic list of scalars syntax used
> > by OptsVisitor. Probably only a few of them need this.
>
> Obvious maintainer question: what would it take to get rid of the
> options visitor entirely? Because this maintainer looks on additions to
> his fiefdom much more kindly when they're balanced by subtractions.
>
> I guess all the uses that don't rely on the "list of scalars" magic are
> easy prey: replace by conversion to intermediate QDict + your new
> visitor.
>
> What about the ones that do rely on it? Which ones do? Any ideas on
> how to change them so that they don't require a complete options
> visitor?
I guess you could have some code that re-processes the stored QemuOpt
in-place, changing repeated keys such as 'foo=400,foo=342' into the
preferred format 'foo.1=400,foo.2=342', at which point you can use
the qdict_crumple facility directly.
That's probably not even very hard.
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] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v11 1/6] qdict: implement a qdict_crumple method for un-flattening a dict
2016-09-05 15:16 ` [Qemu-devel] [PATCH v11 1/6] qdict: implement a qdict_crumple method for un-flattening a dict Daniel P. Berrange
@ 2016-09-14 14:18 ` Kevin Wolf
2016-09-15 11:30 ` Daniel P. Berrange
0 siblings, 1 reply; 30+ messages in thread
From: Kevin Wolf @ 2016-09-14 14:18 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: qemu-devel, Markus Armbruster, Max Reitz, Marc-André Lureau,
Paolo Bonzini, Andreas Färber
Am 05.09.2016 um 17:16 hat Daniel P. Berrange geschrieben:
> 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.
> [...]
> +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);
This fits in a single line.
> + *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 crumple a dictionary with a mix of list "
> + "and non-list keys");
This is a message that users will see if they pass a bad command line
option. I don't think they will understand what it means to "crumple a
dictionary" or that they tried to do this.
Maybe simply "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 crumple a dictionary with a mix of list "
> + "and non-list keys");
Same here.
> + return -1;
> + }
> + }
> + }
> +
> + if (is_list == -1) {
> + assert(!qdict_size(maybe_list));
> + is_list = 0;
> + }
> +
> + if (len != (max + 1)) {
> + error_setg(errp, "List indexes are not contiguous, "
> + "saw %zd elements but %zd largest index",
> + len, max);
> + return -1;
> + }
I don't think this is catching everything that isn't contiguous, but I'm
not sure whether we care.
One reason is that you accept negative indexes above, the other one is
that the keys are strings, so I could pass "1", "+1", "01" and "3" and it
would be accepted.
It's probably reasonable enough to say that in such cases you get what
you get, and if future versions behave differently, that's your problem.
> + 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"
Hm, so you thought of the case I mentioned above, but I don't see it
handled properly in the code.
> + *
> + * 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)) {
> + 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);
> + assert(child);
So this is the place where the %zu requirement for key names is
enforced. But where do we check this before to return an error instead
of running into an assertion failure?
If you turn this into another non-contiguous error, we should be fine.
> +
> + 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..be4a132 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,235 @@ static void qdict_join_test(void)
> QDECREF(dict2);
> }
>
> +
> +static void qdict_crumple_test_nonrecursive(void)
> +{
> + QDict *src, *dst, *rules, *vnc;
> + 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"));
Worth testing an escaped . as well? Possibly both in the prefix (where
it should get unescaped) and in the suffix (where the doubling is still
expected.
> + 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), ==, 2);
> +
> + 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"));
> +
> + 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"));
Here some tests with escaped . would be nice, too.
> + 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);
> +
> + 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", qdict_new());
You want to use something valid here (i.e. a scalar)
> + 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", qdict_new());
And here too. This invalid entry is the reason why you error out early
instead of running into the assertion failure I mentioned above.
> + 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);
> +}
Kevin
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v11 2/6] option: make parse_option_bool/number non-static
2016-09-05 15:16 ` [Qemu-devel] [PATCH v11 2/6] option: make parse_option_bool/number non-static Daniel P. Berrange
@ 2016-09-14 14:33 ` Kevin Wolf
0 siblings, 0 replies; 30+ messages in thread
From: Kevin Wolf @ 2016-09-14 14:33 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: qemu-devel, Markus Armbruster, Max Reitz, Marc-André Lureau,
Paolo Bonzini, Andreas Färber
Am 05.09.2016 um 17:16 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: 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>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v11 5/6] qapi: add a QmpInputVisitor that does string conversion
2016-09-05 15:16 ` [Qemu-devel] [PATCH v11 5/6] qapi: add a QmpInputVisitor that does string conversion Daniel P. Berrange
` (2 preceding siblings ...)
2016-09-13 9:05 ` Markus Armbruster
@ 2016-09-14 14:59 ` Kevin Wolf
2016-09-15 11:33 ` Daniel P. Berrange
3 siblings, 1 reply; 30+ messages in thread
From: Kevin Wolf @ 2016-09-14 14:59 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: qemu-devel, Markus Armbruster, Max Reitz, Marc-André Lureau,
Paolo Bonzini, Andreas Färber
Am 05.09.2016 um 17:16 hat Daniel P. Berrange geschrieben:
> Currently the QmpInputVisitor assumes that all scalar
> values are directly represented as their final types.
> ie it assumes an 'int' is using QInt, and a 'bool' is
> using QBool.
>
> This adds an alternative constructor for QmpInputVisitor
> that will set it up such that it expects a QString for
> all scalar types instead.
>
> This makes it possible to use QmpInputVisitor with a
> QDict produced from QemuOpts, where everything is in
> string format.
>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> +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));
> + int64_t val;
> + char *endptr;
> +
> + if (qstr && qstr->string) {
> + val = qemu_strtosz_suffix(qstr->string, &endptr,
> + QEMU_STRTOSZ_DEFSUFFIX_B);
> + if (val < 0 || *endptr) {
> + error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name,
> + "a size value representible as a non-negative int64");
> + return;
> + }
> +
> + *obj = val;
> + return;
> + }
> +
> + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
> + "size");
> +}
Why not use parse_option_size() here when you use the QemuOpts parser
for the other functions?
Of course, parse_option_size() could be switched to internally use
qemu_strtosz_suffix() sooner or later, too, but that's a different
problem...
Kevin
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v11 0/6] QAPI/QOM work for non-scalar object properties
2016-09-05 15:16 [Qemu-devel] [PATCH v11 0/6] QAPI/QOM work for non-scalar object properties Daniel P. Berrange
` (5 preceding siblings ...)
2016-09-05 15:16 ` [Qemu-devel] [PATCH v11 6/6] qom: support arbitrary non-scalar properties with -object Daniel P. Berrange
@ 2016-09-14 15:04 ` Kevin Wolf
2016-09-15 11:34 ` Daniel P. Berrange
6 siblings, 1 reply; 30+ messages in thread
From: Kevin Wolf @ 2016-09-14 15:04 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: qemu-devel, Markus Armbruster, Max Reitz, Marc-André Lureau,
Paolo Bonzini, Andreas Färber
Am 05.09.2016 um 17:16 hat Daniel P. Berrange geschrieben:
> 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
>
> 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.
I haven't looked at the final patch because that one is irrelevant for
me, but otherwise nice series.
Please keep me copied on new versions of it, I noticed that
qdict_crumple() and qobject_string_visitor_new() are exactly what I need
for implementing the new '-blockdev' option, so I'll build on top of
this.
Kevin
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v11 1/6] qdict: implement a qdict_crumple method for un-flattening a dict
2016-09-14 14:18 ` Kevin Wolf
@ 2016-09-15 11:30 ` Daniel P. Berrange
0 siblings, 0 replies; 30+ messages in thread
From: Daniel P. Berrange @ 2016-09-15 11:30 UTC (permalink / raw)
To: Kevin Wolf
Cc: qemu-devel, Markus Armbruster, Max Reitz, Marc-André Lureau,
Paolo Bonzini, Andreas Färber
On Wed, Sep 14, 2016 at 04:18:42PM +0200, Kevin Wolf wrote:
> Am 05.09.2016 um 17:16 hat Daniel P. Berrange geschrieben:
> > 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.
> > [...]
>
> > +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);
>
> This fits in a single line.
>
> > + *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 crumple a dictionary with a mix of list "
> > + "and non-list keys");
>
> This is a message that users will see if they pass a bad command line
> option. I don't think they will understand what it means to "crumple a
> dictionary" or that they tried to do this.
>
> Maybe simply "Cannot mix list and non-list keys"?
Oh yes, good point.
> > + if (is_list == -1) {
> > + assert(!qdict_size(maybe_list));
> > + is_list = 0;
> > + }
> > +
> > + if (len != (max + 1)) {
> > + error_setg(errp, "List indexes are not contiguous, "
> > + "saw %zd elements but %zd largest index",
> > + len, max);
> > + return -1;
> > + }
>
> I don't think this is catching everything that isn't contiguous, but I'm
> not sure whether we care.
>
> One reason is that you accept negative indexes above, the other one is
> that the keys are strings, so I could pass "1", "+1", "01" and "3" and it
> would be accepted.
>
> It's probably reasonable enough to say that in such cases you get what
> you get, and if future versions behave differently, that's your problem.
In the context of this 'qdict_is_list()' method I think that's
ok - even if they use "1", "+1", "01" and "3", from the POV of
this method it is a list.
The qdict_crumple method could however then detect if there
are keys that are duplicated and/or missing, which will handle
the case you illustrate. I'll add a checks and a test for this.
> > +/**
> > + * 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"
>
> Hm, so you thought of the case I mentioned above, but I don't see it
> handled properly in the code.
Yeah, I've not handled it well enough.
> > + /* 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);
> > + assert(child);
>
> So this is the place where the %zu requirement for key names is
> enforced. But where do we check this before to return an error instead
> of running into an assertion failure?
>
> If you turn this into another non-contiguous error, we should be fine.
Yes, I should turn the assert into an reported error.
> > +
> > + qobject_incref(child);
> > + qlist_append_obj(qobject_to_qlist(dst), child);
> > + }
> > + QDECREF(multi_level);
> > + multi_level = NULL;
> > + } else {
> > + dst = QOBJECT(multi_level);
> > +static void qdict_crumple_test_nonrecursive(void)
> > +{
> > + QDict *src, *dst, *rules, *vnc;
> > + 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"));
>
> Worth testing an escaped . as well? Possibly both in the prefix (where
> it should get unescaped) and in the suffix (where the doubling is still
> expected.
Yes, we should validate escaping.
> > +
> > +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", qdict_new());
>
> You want to use something valid here (i.e. a scalar)
>
> > + 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", qdict_new());
>
> And here too. This invalid entry is the reason why you error out early
> instead of running into the assertion failure I mentioned above.
OK
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] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v11 5/6] qapi: add a QmpInputVisitor that does string conversion
2016-09-14 14:59 ` Kevin Wolf
@ 2016-09-15 11:33 ` Daniel P. Berrange
0 siblings, 0 replies; 30+ messages in thread
From: Daniel P. Berrange @ 2016-09-15 11:33 UTC (permalink / raw)
To: Kevin Wolf
Cc: qemu-devel, Markus Armbruster, Max Reitz, Marc-André Lureau,
Paolo Bonzini, Andreas Färber
On Wed, Sep 14, 2016 at 04:59:50PM +0200, Kevin Wolf wrote:
> Am 05.09.2016 um 17:16 hat Daniel P. Berrange geschrieben:
> > Currently the QmpInputVisitor assumes that all scalar
> > values are directly represented as their final types.
> > ie it assumes an 'int' is using QInt, and a 'bool' is
> > using QBool.
> >
> > This adds an alternative constructor for QmpInputVisitor
> > that will set it up such that it expects a QString for
> > all scalar types instead.
> >
> > This makes it possible to use QmpInputVisitor with a
> > QDict produced from QemuOpts, where everything is in
> > string format.
> >
> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
>
> > +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));
> > + int64_t val;
> > + char *endptr;
> > +
> > + if (qstr && qstr->string) {
> > + val = qemu_strtosz_suffix(qstr->string, &endptr,
> > + QEMU_STRTOSZ_DEFSUFFIX_B);
> > + if (val < 0 || *endptr) {
> > + error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name,
> > + "a size value representible as a non-negative int64");
> > + return;
> > + }
> > +
> > + *obj = val;
> > + return;
> > + }
> > +
> > + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
> > + "size");
> > +}
>
> Why not use parse_option_size() here when you use the QemuOpts parser
> for the other functions?
No idea why i didn't use that.
> Of course, parse_option_size() could be switched to internally use
> qemu_strtosz_suffix() sooner or later, too, but that's a different
> problem...
Yes, it really should. I might just do that later.
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] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v11 0/6] QAPI/QOM work for non-scalar object properties
2016-09-14 15:04 ` [Qemu-devel] [PATCH v11 0/6] QAPI/QOM work for non-scalar object properties Kevin Wolf
@ 2016-09-15 11:34 ` Daniel P. Berrange
0 siblings, 0 replies; 30+ messages in thread
From: Daniel P. Berrange @ 2016-09-15 11:34 UTC (permalink / raw)
To: Kevin Wolf
Cc: qemu-devel, Markus Armbruster, Max Reitz, Marc-André Lureau,
Paolo Bonzini, Andreas Färber
On Wed, Sep 14, 2016 at 05:04:05PM +0200, Kevin Wolf wrote:
> Am 05.09.2016 um 17:16 hat Daniel P. Berrange geschrieben:
> > 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
> >
> > 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.
>
> I haven't looked at the final patch because that one is irrelevant for
> me, but otherwise nice series.
>
> Please keep me copied on new versions of it, I noticed that
> qdict_crumple() and qobject_string_visitor_new() are exactly what I need
> for implementing the new '-blockdev' option, so I'll build on top of
> this.
That's entirely intentional :-)
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] 30+ messages in thread
end of thread, other threads:[~2016-09-15 11:34 UTC | newest]
Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-05 15:16 [Qemu-devel] [PATCH v11 0/6] QAPI/QOM work for non-scalar object properties Daniel P. Berrange
2016-09-05 15:16 ` [Qemu-devel] [PATCH v11 1/6] qdict: implement a qdict_crumple method for un-flattening a dict Daniel P. Berrange
2016-09-14 14:18 ` Kevin Wolf
2016-09-15 11:30 ` Daniel P. Berrange
2016-09-05 15:16 ` [Qemu-devel] [PATCH v11 2/6] option: make parse_option_bool/number non-static Daniel P. Berrange
2016-09-14 14:33 ` Kevin Wolf
2016-09-05 15:16 ` [Qemu-devel] [PATCH v11 3/6] qapi: rename QmpInputVisitor to QObjectInputVisitor Daniel P. Berrange
2016-09-12 16:19 ` Markus Armbruster
2016-09-13 10:25 ` Daniel P. Berrange
2016-09-05 15:16 ` [Qemu-devel] [PATCH v11 4/6] qapi: rename QmpOutputVisitor to QObjectOutputVisitor Daniel P. Berrange
2016-09-12 16:20 ` Markus Armbruster
2016-09-13 10:25 ` Daniel P. Berrange
2016-09-12 18:24 ` Eric Blake
2016-09-05 15:16 ` [Qemu-devel] [PATCH v11 5/6] qapi: add a QmpInputVisitor that does string conversion Daniel P. Berrange
2016-09-12 16:21 ` Markus Armbruster
2016-09-12 16:23 ` Daniel P. Berrange
2016-09-12 16:30 ` Daniel P. Berrange
2016-09-12 18:39 ` Eric Blake
2016-09-13 10:22 ` Daniel P. Berrange
2016-09-13 9:05 ` Markus Armbruster
2016-09-13 9:33 ` Daniel P. Berrange
2016-09-13 13:32 ` Markus Armbruster
2016-09-13 13:47 ` Daniel P. Berrange
2016-09-14 14:59 ` Kevin Wolf
2016-09-15 11:33 ` Daniel P. Berrange
2016-09-05 15:16 ` [Qemu-devel] [PATCH v11 6/6] qom: support arbitrary non-scalar properties with -object Daniel P. Berrange
2016-09-12 18:20 ` Eric Blake
2016-09-13 10:32 ` Daniel P. Berrange
2016-09-14 15:04 ` [Qemu-devel] [PATCH v11 0/6] QAPI/QOM work for non-scalar object properties Kevin Wolf
2016-09-15 11:34 ` Daniel P. Berrange
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.