* [Qemu-devel] [RFC 1/3] tests: Simplify abstract-interfaces check with a helper
2017-05-20 2:09 [Qemu-devel] [RFC 0/3] qmp: Return extra information on qom-list-types Eduardo Habkost
@ 2017-05-20 2:09 ` Eduardo Habkost
2017-05-20 2:09 ` [Qemu-devel] [RFC 2/3] qmp: Include 'abstract' field on 'qom-list-types' output Eduardo Habkost
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Eduardo Habkost @ 2017-05-20 2:09 UTC (permalink / raw)
To: qemu-devel; +Cc: Markus Armbruster, Eric Blake
Add a new type_list_find() helper to device-introspect-test.c, to
simplify the code at test_abstract_interfaces().
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
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 b1abb2ad89..f1e6dddfa3 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.11.0.259.g40922b1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [RFC 2/3] qmp: Include 'abstract' field on 'qom-list-types' output
2017-05-20 2:09 [Qemu-devel] [RFC 0/3] qmp: Return extra information on qom-list-types Eduardo Habkost
2017-05-20 2:09 ` [Qemu-devel] [RFC 1/3] tests: Simplify abstract-interfaces check with a helper Eduardo Habkost
@ 2017-05-20 2:09 ` Eduardo Habkost
2017-05-20 2:09 ` [Qemu-devel] [RFC 3/3] qmp: Include parent types " Eduardo Habkost
2017-05-23 14:12 ` [Qemu-devel] [RFC 0/3] qmp: Return extra information on qom-list-types Markus Armbruster
3 siblings, 0 replies; 9+ messages in thread
From: Eduardo Habkost @ 2017-05-20 2:09 UTC (permalink / raw)
To: qemu-devel; +Cc: Markus Armbruster, Eric Blake
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.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
qapi-schema.json | 5 ++++-
qmp.c | 1 +
tests/device-introspect-test.c | 51 +++++++++++++++++++++++++++++++-----------
3 files changed, 43 insertions(+), 14 deletions(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index 80603cfc51..cbf1e2a837 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3034,12 +3034,15 @@
#
# @name: the type name found in the search
#
+# @abstract: the type is abstract and can't be directly instantiated
+# (since 2.10)
+#
# Since: 1.1
#
# Notes: This command is experimental and may change syntax in future releases.
##
{ 'struct': 'ObjectTypeInfo',
- 'data': { 'name': 'str' } }
+ 'data': { 'name': 'str', 'abstract': 'bool' } }
##
# @qom-list-types:
diff --git a/qmp.c b/qmp.c
index f656940769..8066705dbe 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->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 f1e6dddfa3..d5aacb4ed1 100644
--- a/tests/device-introspect-test.c
+++ b/tests/device-introspect-test.c
@@ -103,6 +103,31 @@ 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_get_bool(d, "abstract");
+ 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 +169,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_get_bool(d, "abstract"));
}
QDECREF(all_types);
@@ -175,6 +199,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.11.0.259.g40922b1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [RFC 3/3] qmp: Include parent types on 'qom-list-types' output
2017-05-20 2:09 [Qemu-devel] [RFC 0/3] qmp: Return extra information on qom-list-types Eduardo Habkost
2017-05-20 2:09 ` [Qemu-devel] [RFC 1/3] tests: Simplify abstract-interfaces check with a helper Eduardo Habkost
2017-05-20 2:09 ` [Qemu-devel] [RFC 2/3] qmp: Include 'abstract' field on 'qom-list-types' output Eduardo Habkost
@ 2017-05-20 2:09 ` Eduardo Habkost
2017-05-23 14:12 ` [Qemu-devel] [RFC 0/3] qmp: Return extra information on qom-list-types Markus Armbruster
3 siblings, 0 replies; 9+ messages in thread
From: Eduardo Habkost @ 2017-05-20 2:09 UTC (permalink / raw)
To: qemu-devel; +Cc: Markus Armbruster, Eric Blake
Include list of parent types 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.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
I would to include a "interfaces" field in the future, too, but
this would require extending the core QOM API to allow that.
---
qapi-schema.json | 6 ++++-
qmp.c | 16 +++++++++++
tests/device-introspect-test.c | 61 +++++++++++++++++++++++++++++++++++-------
3 files changed, 72 insertions(+), 11 deletions(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index cbf1e2a837..169dfd2d7f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3037,12 +3037,16 @@
# @abstract: the type is abstract and can't be directly instantiated
# (since 2.10)
#
+# @parent-types: parent types. Parent types implemented by the type. Note that
+# this doesn't include interface names also implemented by the type.
+# (since 2.10)
+#
# Since: 1.1
#
# Notes: This command is experimental and may change syntax in future releases.
##
{ 'struct': 'ObjectTypeInfo',
- 'data': { 'name': 'str', 'abstract': 'bool' } }
+ 'data': { 'name': 'str', 'abstract': 'bool', 'parent-types': [ 'str' ] } }
##
# @qom-list-types:
diff --git a/qmp.c b/qmp.c
index 8066705dbe..5b50f6f650 100644
--- a/qmp.c
+++ b/qmp.c
@@ -436,6 +436,21 @@ void qmp_change(const char *device, const char *target,
}
}
+static strList *qom_type_get_parents(ObjectClass *klass)
+{
+ ObjectClass *parent = object_class_get_parent(klass);
+ strList *r = NULL;
+
+ if (!parent) {
+ return NULL;
+ }
+
+ r = g_new0(strList, 1);
+ r->value = g_strdup(object_class_get_name(parent));
+ r->next = qom_type_get_parents(parent);
+ return r;
+}
+
static void qom_list_types_tramp(ObjectClass *klass, void *data)
{
ObjectTypeInfoList *e, **pret = data;
@@ -444,6 +459,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->abstract = object_class_is_abstract(klass);
+ info->parent_types = qom_type_get_parents(klass);
e = g_malloc0(sizeof(*e));
e->value = info;
diff --git a/tests/device-introspect-test.c b/tests/device-introspect-test.c
index d5aacb4ed1..350b862f3b 100644
--- a/tests/device-introspect-test.c
+++ b/tests/device-introspect-test.c
@@ -61,6 +61,20 @@ static QDict *type_list_find(QList *types, const char *name)
return NULL;
}
+static bool str_list_has(QList *strlist, const char *str)
+{
+ QListEntry *e;
+
+ QLIST_FOREACH_ENTRY(strlist, e) {
+ const char *s = qstring_get_str(qobject_to_qstring(qlist_entry_obj(e)));
+ if (!strcmp(s, str)) {
+ return true;
+ }
+ }
+
+ return false;
+}
+
static QList *device_type_list(bool abstract)
{
return qom_list_types("device", abstract);
@@ -103,6 +117,31 @@ static void test_device_intro_list(void)
qtest_end();
}
+/*
+ * Ensure all entries returned by qom-list-types implements=<parent>
+ * have <parent> in parent-types.
+ */
+static void test_qom_list_parents(const char *parent)
+{
+ QList *types;
+ QListEntry *e;
+
+ types = qom_list_types(parent, true);
+
+ QLIST_FOREACH_ENTRY(types, e) {
+ QDict *d = qobject_to_qdict(qlist_entry_obj(e));
+
+ /* The parent type being queried is included in the list too, skip it */
+ if (!strcmp(qdict_get_str(d, "name"), parent)) {
+ continue;
+ }
+
+ g_assert(str_list_has(qdict_get_qlist(d, "parent-types"), parent));
+ }
+
+ QDECREF(types);
+}
+
static void test_qom_list_fields(void)
{
QList *all_types;
@@ -123,6 +162,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();
@@ -165,23 +208,22 @@ static void test_device_intro_concrete(void)
static void test_abstract_interfaces(void)
{
QList *all_types;
- QList *obj_types;
QListEntry *e;
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);
QLIST_FOREACH_ENTRY(all_types, e) {
QDict *d = qobject_to_qdict(qlist_entry_obj(e));
- 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 parent-types.
+ */
+ if (!str_list_has(qdict_get_qlist(d, "parent-types"), "interface")) {
/* Not an interface type */
continue;
}
@@ -190,7 +232,6 @@ static void test_abstract_interfaces(void)
}
QDECREF(all_types);
- QDECREF(obj_types);
qtest_end();
}
--
2.11.0.259.g40922b1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [RFC 0/3] qmp: Return extra information on qom-list-types
2017-05-20 2:09 [Qemu-devel] [RFC 0/3] qmp: Return extra information on qom-list-types Eduardo Habkost
` (2 preceding siblings ...)
2017-05-20 2:09 ` [Qemu-devel] [RFC 3/3] qmp: Include parent types " Eduardo Habkost
@ 2017-05-23 14:12 ` Markus Armbruster
2017-05-23 14:34 ` Eduardo Habkost
3 siblings, 1 reply; 9+ messages in thread
From: Markus Armbruster @ 2017-05-23 14:12 UTC (permalink / raw)
To: Eduardo Habkost; +Cc: qemu-devel
Eduardo Habkost <ehabkost@redhat.com> writes:
> This series adds 'abstract' and 'parent-types' fields to the
> output of qom-list-types.
Peeking at PATCH 3, it looks like 'parent-types' is the transitive
closure of the single direct parent type (TypeImpl member parent_type).
Do we need information on interfaces as well?
> 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: 20119 bytes
> * with 'abstract' and 'parent-types': 44383 bytes
Obvious ways to save space:
* Make 'abstract' optional, default to false, present only when
necessary (79 out of 456 times right now)
* Pare down 'parent-types' to *direct* parent types. The indirect ones
are redundant.
A less obvious way:
* 'parent-types' defines a relation with adjacency lists. If the
relation is tree-shaped, we can send the tree instead, i.e. a tree of
device names rather than list of (device name, adjacency list). New
command unless we're willing to break qom-list-types.
> I'm not sure if extending qom-list-types with this info is the
> right way to go, or if we should keep qom-list-types as-is and
> add a new "query-qom-type" QMP command to query info on one QOM
> type at a time.
Might lead to more traffic rather than less. Depends on what
information the client needs to query.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [RFC 0/3] qmp: Return extra information on qom-list-types
2017-05-23 14:12 ` [Qemu-devel] [RFC 0/3] qmp: Return extra information on qom-list-types Markus Armbruster
@ 2017-05-23 14:34 ` Eduardo Habkost
2017-05-24 12:13 ` Markus Armbruster
0 siblings, 1 reply; 9+ messages in thread
From: Eduardo Habkost @ 2017-05-23 14:34 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel
On Tue, May 23, 2017 at 04:12:43PM +0200, Markus Armbruster wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
>
> > This series adds 'abstract' and 'parent-types' fields to the
> > output of qom-list-types.
>
> Peeking at PATCH 3, it looks like 'parent-types' is the transitive
> closure of the single direct parent type (TypeImpl member parent_type).
> Do we need information on interfaces as well?
I think we should, but it is more complex so I plan to do in a
separate patch.
>
> > 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: 20119 bytes
> > * with 'abstract' and 'parent-types': 44383 bytes
>
> Obvious ways to save space:
>
> * Make 'abstract' optional, default to false, present only when
> necessary (79 out of 456 times right now)
Good idea.
>
> * Pare down 'parent-types' to *direct* parent types. The indirect ones
> are redundant.
On the one hand, I assume clients don't care if a given type is a
direct parent or indirect parent, and including only the direct
parent type will require them to make extra queries.
On the other hand, if the client wants to save a few queries it
can use the "implements" argument, already? Not sure.
>
> A less obvious way:
>
> * 'parent-types' defines a relation with adjacency lists. If the
> relation is tree-shaped, we can send the tree instead, i.e. a tree of
> device names rather than list of (device name, adjacency list). New
> command unless we're willing to break qom-list-types.
>
> > I'm not sure if extending qom-list-types with this info is the
> > right way to go, or if we should keep qom-list-types as-is and
> > add a new "query-qom-type" QMP command to query info on one QOM
> > type at a time.
>
> Might lead to more traffic rather than less. Depends on what
> information the client needs to query.
I think queries that return all available QOM types are likely to
(should?) be cached by clients. I believe the size will stay
acceptable if we implement the two suggestions above.
--
Eduardo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [RFC 0/3] qmp: Return extra information on qom-list-types
2017-05-23 14:34 ` Eduardo Habkost
@ 2017-05-24 12:13 ` Markus Armbruster
2017-05-24 13:13 ` Eduardo Habkost
2017-05-24 14:31 ` Eric Blake
0 siblings, 2 replies; 9+ messages in thread
From: Markus Armbruster @ 2017-05-24 12:13 UTC (permalink / raw)
To: Eduardo Habkost; +Cc: qemu-devel
Eduardo Habkost <ehabkost@redhat.com> writes:
> On Tue, May 23, 2017 at 04:12:43PM +0200, Markus Armbruster wrote:
>> Eduardo Habkost <ehabkost@redhat.com> writes:
>>
>> > This series adds 'abstract' and 'parent-types' fields to the
>> > output of qom-list-types.
>>
>> Peeking at PATCH 3, it looks like 'parent-types' is the transitive
>> closure of the single direct parent type (TypeImpl member parent_type).
>> Do we need information on interfaces as well?
>
> I think we should, but it is more complex so I plan to do in a
> separate patch.
Okay.
>> > 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: 20119 bytes
>> > * with 'abstract' and 'parent-types': 44383 bytes
>>
>> Obvious ways to save space:
>>
>> * Make 'abstract' optional, default to false, present only when
>> necessary (79 out of 456 times right now)
>
> Good idea.
>
>>
>> * Pare down 'parent-types' to *direct* parent types. The indirect ones
>> are redundant.
>
> On the one hand, I assume clients don't care if a given type is a
> direct parent or indirect parent, and including only the direct
> parent type will require them to make extra queries.
Given the direct parents, computing their transitive closure is trivial.
So a single query should suffice.
> On the other hand, if the client wants to save a few queries it
> can use the "implements" argument, already? Not sure.
>
>>
>> A less obvious way:
>>
>> * 'parent-types' defines a relation with adjacency lists. If the
>> relation is tree-shaped, we can send the tree instead, i.e. a tree of
>> device names rather than list of (device name, adjacency list). New
>> command unless we're willing to break qom-list-types.
>>
>> > I'm not sure if extending qom-list-types with this info is the
>> > right way to go, or if we should keep qom-list-types as-is and
>> > add a new "query-qom-type" QMP command to query info on one QOM
>> > type at a time.
>>
>> Might lead to more traffic rather than less. Depends on what
>> information the client needs to query.
>
> I think queries that return all available QOM types are likely to
> (should?) be cached by clients.
Perhaps libvirt developers can help us here.
> I believe the size will stay
> acceptable if we implement the two suggestions above.
Please do.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [RFC 0/3] qmp: Return extra information on qom-list-types
2017-05-24 12:13 ` Markus Armbruster
@ 2017-05-24 13:13 ` Eduardo Habkost
2017-05-24 14:31 ` Eric Blake
1 sibling, 0 replies; 9+ messages in thread
From: Eduardo Habkost @ 2017-05-24 13:13 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel
On Wed, May 24, 2017 at 02:13:16PM +0200, Markus Armbruster wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
>
> > On Tue, May 23, 2017 at 04:12:43PM +0200, Markus Armbruster wrote:
> >> Eduardo Habkost <ehabkost@redhat.com> writes:
> >>
> >> > This series adds 'abstract' and 'parent-types' fields to the
> >> > output of qom-list-types.
> >>
> >> Peeking at PATCH 3, it looks like 'parent-types' is the transitive
> >> closure of the single direct parent type (TypeImpl member parent_type).
> >> Do we need information on interfaces as well?
> >
> > I think we should, but it is more complex so I plan to do in a
> > separate patch.
>
> Okay.
>
> >> > 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: 20119 bytes
> >> > * with 'abstract' and 'parent-types': 44383 bytes
> >>
> >> Obvious ways to save space:
> >>
> >> * Make 'abstract' optional, default to false, present only when
> >> necessary (79 out of 456 times right now)
> >
> > Good idea.
> >
> >>
> >> * Pare down 'parent-types' to *direct* parent types. The indirect ones
> >> are redundant.
> >
> > On the one hand, I assume clients don't care if a given type is a
> > direct parent or indirect parent, and including only the direct
> > parent type will require them to make extra queries.
>
> Given the direct parents, computing their transitive closure is trivial.
> So a single query should suffice.
If you have the full list, yes. I was thinking of something like
asking "does type FOO is a (direct or indirect) child of BAR?".
"qom-list-types implements=FOO" wouldn't suffice.
"qom-list-types implements=BAR" would be enough, but it would
also return all subtypes of BAR.
However, I think this example is not very realistic. libvirt,
for example, does query everything about QEMU capabilities only
once, and then cache it. Even if a client doesn't cache it, the
size of "qom-list-types implements=BAR" should be reasonable.
>
> > On the other hand, if the client wants to save a few queries it
> > can use the "implements" argument, already? Not sure.
> >
> >>
> >> A less obvious way:
> >>
> >> * 'parent-types' defines a relation with adjacency lists. If the
> >> relation is tree-shaped, we can send the tree instead, i.e. a tree of
> >> device names rather than list of (device name, adjacency list). New
> >> command unless we're willing to break qom-list-types.
> >>
> >> > I'm not sure if extending qom-list-types with this info is the
> >> > right way to go, or if we should keep qom-list-types as-is and
> >> > add a new "query-qom-type" QMP command to query info on one QOM
> >> > type at a time.
> >>
> >> Might lead to more traffic rather than less. Depends on what
> >> information the client needs to query.
> >
> > I think queries that return all available QOM types are likely to
> > (should?) be cached by clients.
>
> Perhaps libvirt developers can help us here.
The only usage of qom-list-types I found in libvirt has no
command arguments, and is called by code that checks for cached
information first.
>
> > I believe the size will stay
> > acceptable if we implement the two suggestions above.
>
> Please do.
--
Eduardo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [RFC 0/3] qmp: Return extra information on qom-list-types
2017-05-24 12:13 ` Markus Armbruster
2017-05-24 13:13 ` Eduardo Habkost
@ 2017-05-24 14:31 ` Eric Blake
1 sibling, 0 replies; 9+ messages in thread
From: Eric Blake @ 2017-05-24 14:31 UTC (permalink / raw)
To: Markus Armbruster, Eduardo Habkost; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 939 bytes --]
On 05/24/2017 07:13 AM, Markus Armbruster wrote:
>>> Might lead to more traffic rather than less. Depends on what
>>> information the client needs to query.
>>
>> I think queries that return all available QOM types are likely to
>> (should?) be cached by clients.
>
> Perhaps libvirt developers can help us here.
Libvirt is very likely going to cache things (libvirt already tries to
cache as much information as it needs from a single run of a qemu
binary, and only recomputes the cache when either libvirt or qemu
binaries are detected to have changed to a newer timestamp). Reading
the entire list, and then libvirt computing transitive closure on that
list if needed, is probably acceptable, compared to qemu having to
compute transitive closure up front for a larger QMP message.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread