[Qemu-devel] [PATCH v3 0/3] qmp: Return extra information on qom-list-types

[Qemu-devel] [PATCH v3 0/3] qmp: Return extra information on qom-list-types

[Eduardo Habkost]

This series adds 'abstract' and 'parent' fields to the output of
qom-list-types.

For reference, below are the sizes of the output of
"qom-list-types abstract=true" on qemu-system-x86_64, before and
after applying the patches:

* before: 11724 bytes
* with 'abstract' field: 13146 bytes
* with 'abstract' and 'parent': 24046 bytes

This is just a rebase of v2.  I'm sending it as v3 because a long
time has passed since v2 was submitted.

Changes v2 -> v3:
* Rebase to latest qemu.git master, only

Changes v1 (RFC) -> v2:
* Make 'parent' field optional, to make output shorter
* Return only immediate parent type on 'parent' field, instead of
  all parent-types on a 'parent-types' field

Eduardo Habkost (3):
  tests: Simplify abstract-interfaces check with a helper
  qmp: Include 'abstract' field on 'qom-list-types' output
  qmp: Include parent type on 'qom-list-types' output

 qapi-schema.json               |   7 +-
 qmp.c                          |   6 ++
 tests/device-introspect-test.c | 156 +++++++++++++++++++++++++++++++++--------
 3 files changed, 139 insertions(+), 30 deletions(-)

-- 
2.9.4

[Qemu-devel] [PATCH v3 1/3] tests: Simplify abstract-interfaces check with a helper

[Eduardo Habkost]

Add a new type_list_find() helper to device-introspect-test.c, to
simplify the code at test_abstract_interfaces().

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v2 -> v3:
* None, rebase only.

Changes v1 -> v2:
* (none)
---
 tests/device-introspect-test.c | 43 ++++++++++++++++++++++++------------------
 1 file changed, 25 insertions(+), 18 deletions(-)

diff --git a/tests/device-introspect-test.c b/tests/device-introspect-test.c
index b1abb2a..f1e6ddd 100644
--- a/tests/device-introspect-test.c
+++ b/tests/device-introspect-test.c
@@ -45,6 +45,22 @@ static QList *qom_list_types(const char *implements, bool abstract)
     return ret;
 }
 
+/* Find an entry on a list returned by qom-list-types */
+static QDict *type_list_find(QList *types, const char *name)
+{
+    QListEntry *e;
+
+    QLIST_FOREACH_ENTRY(types, e) {
+        QDict *d = qobject_to_qdict(qlist_entry_obj(e));
+        const char *ename = qdict_get_str(d, "name");
+        if (!strcmp(ename, name)) {
+            return d;
+        }
+    }
+
+    return NULL;
+}
+
 static QList *device_type_list(bool abstract)
 {
     return qom_list_types("device", abstract);
@@ -125,7 +141,7 @@ static void test_abstract_interfaces(void)
 {
     QList *all_types;
     QList *obj_types;
-    QListEntry *ae;
+    QListEntry *e;
 
     qtest_start(common_args);
     /* qom-list-types implements=interface would return any type
@@ -138,24 +154,15 @@ static void test_abstract_interfaces(void)
     all_types = qom_list_types(NULL, false);
     obj_types = qom_list_types("object", false);
 
-    QLIST_FOREACH_ENTRY(all_types, ae) {
-        QDict *at = qobject_to_qdict(qlist_entry_obj(ae));
-        const char *aname = qdict_get_str(at, "name");
-        QListEntry *oe;
-        const char *found = NULL;
-
-        QLIST_FOREACH_ENTRY(obj_types, oe) {
-            QDict *ot = qobject_to_qdict(qlist_entry_obj(oe));
-            const char *oname = qdict_get_str(ot, "name");
-            if (!strcmp(aname, oname)) {
-                found = oname;
-                break;
-            }
-        }
+    QLIST_FOREACH_ENTRY(all_types, e) {
+        QDict *d = qobject_to_qdict(qlist_entry_obj(e));
+
+        /*
+         * An interface (incorrectly) marked as non-abstract would
+         * appear on all_types, but not on obj_types:
+         */
+        g_assert(type_list_find(obj_types, qdict_get_str(d, "name")));
 
-        /* Using g_assert_cmpstr() will give more useful failure
-         * messages than g_assert(found) */
-        g_assert_cmpstr(aname, ==, found);
     }
 
     QDECREF(all_types);
-- 
2.9.4

[Qemu-devel] [PATCH v3 2/3] qmp: Include 'abstract' field on 'qom-list-types' output

[Eduardo Habkost]

A client may be interested in getting the list of both abstract and
non-abstract types.  Instead of requiring them to make multiple queries
with different filter arguments, just return an 'abstract' field in
'qom-list-types'.

In addition to the new test code for validating this field, update the
abstract-interfaces test case to query for all 'interface' subtypes
(including abstract ones), and to look at the 'abstract' field directly.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v2 -> v3:
* None, rebase only.

Changes v1 -> v2:
* Omit field if false, to keep command output smaller.
---
 qapi-schema.json               |  5 +++-
 qmp.c                          |  1 +
 tests/device-introspect-test.c | 53 +++++++++++++++++++++++++++++++-----------
 3 files changed, 45 insertions(+), 14 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 37c4b95..48d613f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3059,10 +3059,13 @@
 #
 # @name: the type name found in the search
 #
+# @abstract: the type is abstract and can't be directly instantiated.
+#            Omitted if false. (since 2.10)
+#
 # Since: 1.1
 ##
 { 'struct': 'ObjectTypeInfo',
-  'data': { 'name': 'str' } }
+  'data': { 'name': 'str', '*abstract': 'bool' } }
 
 ##
 # @qom-list-types:
diff --git a/qmp.c b/qmp.c
index 7ee9bcf..5e9b563 100644
--- a/qmp.c
+++ b/qmp.c
@@ -443,6 +443,7 @@ static void qom_list_types_tramp(ObjectClass *klass, void *data)
 
     info = g_malloc0(sizeof(*info));
     info->name = g_strdup(object_class_get_name(klass));
+    info->has_abstract = info->abstract = object_class_is_abstract(klass);
 
     e = g_malloc0(sizeof(*e));
     e->value = info;
diff --git a/tests/device-introspect-test.c b/tests/device-introspect-test.c
index f1e6ddd..78d4811 100644
--- a/tests/device-introspect-test.c
+++ b/tests/device-introspect-test.c
@@ -103,6 +103,33 @@ static void test_device_intro_list(void)
     qtest_end();
 }
 
+static void test_qom_list_fields(void)
+{
+    QList *all_types;
+    QList *non_abstract;
+    QListEntry *e;
+
+    qtest_start(common_args);
+
+    all_types = qom_list_types(NULL, true);
+    non_abstract = qom_list_types(NULL, false);
+
+    QLIST_FOREACH_ENTRY(all_types, e) {
+        QDict *d = qobject_to_qdict(qlist_entry_obj(e));
+        const char *name = qdict_get_str(d, "name");
+        bool abstract = qdict_haskey(d, "abstract") ?
+                        qdict_get_bool(d, "abstract") :
+                        false;
+        bool expected_abstract = !type_list_find(non_abstract, name);
+
+        g_assert(abstract == expected_abstract);
+    }
+
+    QDECREF(all_types);
+    QDECREF(non_abstract);
+    qtest_end();
+}
+
 static void test_device_intro_none(void)
 {
     qtest_start(common_args);
@@ -144,25 +171,24 @@ static void test_abstract_interfaces(void)
     QListEntry *e;
 
     qtest_start(common_args);
-    /* qom-list-types implements=interface would return any type
-     * that implements _any_ interface (not just interface types),
-     * so use a trick to find the interface type names:
-     * - list all object types
-     * - list all types, and look for items that are not
-     *   on the first list
+    /*
+     * qom-list-types implements=interface returns all types that
+     * implement _any_ interface (not just interface types), but
+     * we can filter them out because interfaces don't implement
+     * the "object" type.
      */
-    all_types = qom_list_types(NULL, false);
-    obj_types = qom_list_types("object", false);
+    all_types = qom_list_types("interface", true);
+    obj_types = qom_list_types("object", true);
 
     QLIST_FOREACH_ENTRY(all_types, e) {
         QDict *d = qobject_to_qdict(qlist_entry_obj(e));
 
-        /*
-         * An interface (incorrectly) marked as non-abstract would
-         * appear on all_types, but not on obj_types:
-         */
-        g_assert(type_list_find(obj_types, qdict_get_str(d, "name")));
+        if (type_list_find(obj_types, qdict_get_str(d, "name"))) {
+            /* Not an interface type */
+            continue;
+        }
 
+        g_assert(qdict_haskey(d, "abstract") && qdict_get_bool(d, "abstract"));
     }
 
     QDECREF(all_types);
@@ -175,6 +201,7 @@ int main(int argc, char **argv)
     g_test_init(&argc, &argv, NULL);
 
     qtest_add_func("device/introspect/list", test_device_intro_list);
+    qtest_add_func("device/introspect/list-fields", test_qom_list_fields);
     qtest_add_func("device/introspect/none", test_device_intro_none);
     qtest_add_func("device/introspect/abstract", test_device_intro_abstract);
     qtest_add_func("device/introspect/concrete", test_device_intro_concrete);
-- 
2.9.4

[Qemu-devel] [PATCH v3 3/3] qmp: Include parent type on 'qom-list-types' output

[Eduardo Habkost]

Include name of parent type of each type on 'qom-list-types' output.

Without this, there's no way to figure out the parents of a given type
without making additional 'qom-list-types' queries.

In addition to the test case for the new feature, update the
abstract-interface test case to use the new field and avoid the
"qom-list-types implements=object" trick.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v2 -> v3:
* None, rebase only.

Changes v1 -> v2:
* Include only immediate parent type, rename new field to 'parent'
---
 qapi-schema.json               |  4 +-
 qmp.c                          |  5 +++
 tests/device-introspect-test.c | 84 +++++++++++++++++++++++++++++++++++++-----
 3 files changed, 82 insertions(+), 11 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 48d613f..0432827 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3062,10 +3062,12 @@
 # @abstract: the type is abstract and can't be directly instantiated.
 #            Omitted if false. (since 2.10)
 #
+# @parent: Name of parent type, if any (since 2.10)
+#
 # Since: 1.1
 ##
 { 'struct': 'ObjectTypeInfo',
-  'data': { 'name': 'str', '*abstract': 'bool' } }
+  'data': { 'name': 'str', '*abstract': 'bool', '*parent': 'str' } }
 
 ##
 # @qom-list-types:
diff --git a/qmp.c b/qmp.c
index 5e9b563..ca5f446 100644
--- a/qmp.c
+++ b/qmp.c
@@ -440,10 +440,15 @@ static void qom_list_types_tramp(ObjectClass *klass, void *data)
 {
     ObjectTypeInfoList *e, **pret = data;
     ObjectTypeInfo *info;
+    ObjectClass *parent = object_class_get_parent(klass);
 
     info = g_malloc0(sizeof(*info));
     info->name = g_strdup(object_class_get_name(klass));
     info->has_abstract = info->abstract = object_class_is_abstract(klass);
+    if (parent) {
+        info->has_parent = true;
+        info->parent = g_strdup(object_class_get_name(parent));
+    }
 
     e = g_malloc0(sizeof(*e));
     e->value = info;
diff --git a/tests/device-introspect-test.c b/tests/device-introspect-test.c
index 78d4811..f7162c0 100644
--- a/tests/device-introspect-test.c
+++ b/tests/device-introspect-test.c
@@ -45,6 +45,40 @@ static QList *qom_list_types(const char *implements, bool abstract)
     return ret;
 }
 
+/* Build a name -> ObjectTypeInfo index from a ObjectTypeInfo list */
+static QDict *qom_type_index(QList *types)
+{
+    QDict *index = qdict_new();
+    QListEntry *e;
+
+    QLIST_FOREACH_ENTRY(types, e) {
+        QDict *d = qobject_to_qdict(qlist_entry_obj(e));
+        const char *name = qdict_get_str(d, "name");
+        QINCREF(d);
+        qdict_put(index, name, d);
+    }
+    return index;
+}
+
+/* Check if @parent is present in the parent chain of @type */
+static bool qom_has_parent(QDict *index, const char *type, const char *parent)
+{
+    while (type) {
+        QDict *d = qdict_get_qdict(index, type);
+        const char *p = d && qdict_haskey(d, "parent") ?
+                        qdict_get_str(d, "parent") :
+                        NULL;
+
+        if (!strcmp(type, parent)) {
+            return true;
+        }
+
+        type = p;
+    }
+
+    return false;
+}
+
 /* Find an entry on a list returned by qom-list-types */
 static QDict *type_list_find(QList *types, const char *name)
 {
@@ -103,6 +137,30 @@ static void test_device_intro_list(void)
     qtest_end();
 }
 
+/*
+ * Ensure all entries returned by qom-list-types implements=<parent>
+ * have <parent> as a parent.
+ */
+static void test_qom_list_parents(const char *parent)
+{
+    QList *types;
+    QListEntry *e;
+    QDict *index;
+
+    types = qom_list_types(parent, true);
+    index = qom_type_index(types);
+
+    QLIST_FOREACH_ENTRY(types, e) {
+        QDict *d = qobject_to_qdict(qlist_entry_obj(e));
+        const char *name = qdict_get_str(d, "name");
+
+        g_assert(qom_has_parent(index, name, parent));
+    }
+
+    QDECREF(types);
+    QDECREF(index);
+}
+
 static void test_qom_list_fields(void)
 {
     QList *all_types;
@@ -125,6 +183,10 @@ static void test_qom_list_fields(void)
         g_assert(abstract == expected_abstract);
     }
 
+    test_qom_list_parents("object");
+    test_qom_list_parents("device");
+    test_qom_list_parents("sys-bus-device");
+
     QDECREF(all_types);
     QDECREF(non_abstract);
     qtest_end();
@@ -167,23 +229,25 @@ static void test_device_intro_concrete(void)
 static void test_abstract_interfaces(void)
 {
     QList *all_types;
-    QList *obj_types;
     QListEntry *e;
+    QDict *index;
 
     qtest_start(common_args);
-    /*
-     * qom-list-types implements=interface returns all types that
-     * implement _any_ interface (not just interface types), but
-     * we can filter them out because interfaces don't implement
-     * the "object" type.
-     */
+
     all_types = qom_list_types("interface", true);
-    obj_types = qom_list_types("object", true);
+    index = qom_type_index(all_types);
 
     QLIST_FOREACH_ENTRY(all_types, e) {
         QDict *d = qobject_to_qdict(qlist_entry_obj(e));
+        const char *name = qdict_get_str(d, "name");
 
-        if (type_list_find(obj_types, qdict_get_str(d, "name"))) {
+        /*
+         * qom-list-types implements=interface returns all types
+         * that implement _any_ interface (not just interface
+         * types), so skip the ones that don't have "interface"
+         * on the parent type chain.
+         */
+        if (!qom_has_parent(index, name, "interface")) {
             /* Not an interface type */
             continue;
         }
@@ -192,7 +256,7 @@ static void test_abstract_interfaces(void)
     }
 
     QDECREF(all_types);
-    QDECREF(obj_types);
+    QDECREF(index);
     qtest_end();
 }
 
-- 
2.9.4

Re: [Qemu-devel] [PATCH v3 0/3] qmp: Return extra information on qom-list-types

[Eduardo Habkost]

On Fri, Jul 07, 2017 at 09:22:12AM -0300, Eduardo Habkost wrote:
> This series adds 'abstract' and 'parent' fields to the output of
> qom-list-types.
> 
> For reference, below are the sizes of the output of
> "qom-list-types abstract=true" on qemu-system-x86_64, before and
> after applying the patches:
> 
> * before: 11724 bytes
> * with 'abstract' field: 13146 bytes
> * with 'abstract' and 'parent': 24046 bytes
> 
> This is just a rebase of v2.  I'm sending it as v3 because a long
> time has passed since v2 was submitted.

As we're close to soft freeze, I'm queueing this on my
machine-next branch.

-- 
Eduardo