Re: [libvirt] [Qemu-devel] [PATCH v4 4/5] qmp: full introspection support for QMP
CC Libvirt-list Original discussion: http://marc.info/?l=qemu-devel&m=139048842504757&w=2 [Qemu-devel] [PATCH v4 0/5] QMP full introspection On Fri, Jan 24, 2014 at 06:48:31PM +0800, Fam Zheng wrote:
On Thu, 01/23 22:46, Amos Kong wrote:
This patch introduces a new monitor command to query QMP schema information, the return data is a range of schema structs, which contains the useful metadata to help management to check supported features, QMP commands detail, etc.
We use qapi-introspect.py to parse all json definition in qapi-schema.json, and generate a range of dictionaries with metadata. The query command will visit the dictionaries and fill the data to allocated struct tree. Then QMP infrastructure will convert the tree to json string and return to QMP client.
TODO: Wenchao Xia is working to convert QMP events to qapi-schema.json, then event can also be queried by this interface.
I will introduce another command 'query-qga-schema' to query QGA schema information, it's easy to add this support based on this patch.
Signed-off-by: Amos Kong <akong@redhat.com> --- qapi-schema.json | 11 +++ qmp-commands.hx | 42 +++++++++++ qmp.c | 215 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 268 insertions(+)
diff --git a/qapi-schema.json b/qapi-schema.json index c63f0ca..6033383 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -4411,3 +4411,14 @@ 'reference-type': 'String', 'type': 'DataObjectType', 'unionobj': 'DataObjectUnion' } } + +## +# @query-qmp-schema +# +# Query QMP schema information +# +# @returns: list of @DataObject +# +# Since: 1.8 +## +{ 'command': 'query-qmp-schema', 'returns': ['DataObject'] } diff --git a/qmp-commands.hx b/qmp-commands.hx index 02cc815..b83762d 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -3291,6 +3291,48 @@ Example: }
EQMP + { + .name = "query-qmp-schema", + .args_type = "", + .mhandler.cmd_new = qmp_marshal_input_query_qmp_schema, + }, + + +SQMP +query-qmp-schema +---------------- + +query qmp schema information + +Return a json-object with the following information: + +- "name": qmp schema name (json-string) +- "type": qmp schema type, it can be 'comand', 'type', 'enum', 'union' +- "returns": return data of qmp command (json-object, optional) + +Example: + +-> { "execute": "query-qmp-schema" } +-> { "return": [ + { + "name": "query-name", + "type": "command", + "returns": { + "name": "NameInfo", + "type": "type", + "data": [ + { + "name": "name", + "optional": true, + "recursive": false, + "type": "str" + } + ] + } + } + } + +EQMP
{ .name = "blockdev-add", diff --git a/qmp.c b/qmp.c index 0f46171..a64ae6d 100644 --- a/qmp.c +++ b/qmp.c @@ -27,6 +27,8 @@ #include "qapi/qmp/qobject.h" #include "qapi/qmp-input-visitor.h" #include "hw/boards.h" +#include "qapi/qmp/qjson.h" +#include "qapi-introspect.h"
NameInfo *qmp_query_name(Error **errp) { @@ -488,6 +490,219 @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp) return arch_query_cpu_definitions(errp); }
+static strList *qobject_to_strlist(QObject *data) +{ + strList *list = NULL; + strList **plist = &list; + QList *qlist; + const QListEntry *lent; + + qlist = qobject_to_qlist(data); + for (lent = qlist_first(qlist); lent; lent = qlist_next(lent)) { + strList *entry = g_malloc0(sizeof(strList)); + entry->value = g_strdup(qobject_get_str(lent->value)); + *plist = entry; + plist = &entry->next; + } + + return list; +} + +static DataObject *qobject_to_dataobj(QObject *data); + +static DataObjectMember *qobject_to_dataobjmem(QObject *data) +{ + + DataObjectMember *member = g_malloc0(sizeof(DataObjectMember)); + + member->type = g_malloc0(sizeof(DataObjectMemberType)); + if (data->type->code == QTYPE_QDICT) { + member->type->kind = DATA_OBJECT_MEMBER_TYPE_KIND_EXTEND; + member->type->extend = qobject_to_dataobj(data); + } else { + member->type->kind = DATA_OBJECT_MEMBER_TYPE_KIND_REFERENCE; + member->type->reference = g_strdup(qobject_get_str(data)); + } + + return member; +} + +static DataObjectMemberList *qobject_to_dict_memlist(QObject *data) +{ + DataObjectMemberList *list = NULL; + DataObjectMemberList **plist = &list; + QDict *qdict = qobject_to_qdict(data); + const QDictEntry *dent; + + for (dent = qdict_first(qdict); dent; dent = qdict_next(qdict, dent)) { + DataObjectMemberList *entry = g_malloc0(sizeof(DataObjectMemberList)); + entry->value = qobject_to_dataobjmem(dent->value); + + entry->value->has_optional = true; + entry->value->has_name = true; + if (dent->key[0] == '*') { + entry->value->optional = true; + entry->value->name = g_strdup(dent->key + 1); + } else { + entry->value->name = g_strdup(dent->key); + } + *plist = entry; + plist = &entry->next; + } + + return list; +} + +static DataObjectMemberList *qobject_to_list_memlist(QObject *data) +{ + const QListEntry *lent; + DataObjectMemberList *list = NULL; + DataObjectMemberList **plist = &list; + QList *qlist = qobject_to_qlist(data); + + for (lent = qlist_first(qlist); lent; lent = qlist_next(lent)) { + DataObjectMemberList *entry = g_malloc0(sizeof(DataObjectMemberList)); + entry->value = qobject_to_dataobjmem(lent->value); + entry->value->has_optional = true; + entry->value->has_name = true; + *plist = entry; + plist = &entry->next; + } + + return list; +} + +static DataObjectMemberList *qobject_to_memlist(QObject *data)
This whole converting is cumbersome. You already did all the traversing through the type jungle in python when generating this, it's not necessary to do the similar thing again here.
We can parse raw schemas and generate json string table, we can't directly return the string / qobject to monitor, C code has to convert the json to qobject, we have to revisit the qobject and convert them to DataObject/DataObjectMember/DataObject... structs.
Alternatively, I think we have a good reason to extend QMP framework as necessary here, as we are doing "QMP introspection", which is a part of the framework:
* Define final output into qmp_schema_table[], no need to box it like:
"{'_obj_member': 'False', '_obj_type': 'enum', '_obj_name': 'ErrorClass', '_obj_data': {'data': ...
just put it content of "qmp-introspection.output.txt" as a long string in the header,
like you would generate in qobject_to_memlist:
const char *qmp_schema_table = "{ 'name': 'ErrorClass', 'type': 'enumeration', 'data': [...]}," "{ 'name': '...', ...},"
The keys are used for metadata might be 'recursive', 'optional', etc. It might exists problem in namespace, let's use '_obj_' or '_' prefix for the metadata keys. I used a nested dictionary to describe a DataObject, because we can store the metadata and definition to different level, it's helpful in parse the output by Libvirt. example: "{ 'type': 'NameInfo', 'data': {'*name': 'str', '*job': 'str'} }" It's good to store _type, _name, data to same level, but the metadata of items of data's value dictionary can't be appended to same level. "{ '_name': 'NameInfo', '_type': 'type', 'data': { 'name': 'str', '_name_optional': 'True', 'job': 'str', '_job_optional': 'True' } }" A better solution, but I don't know if it will cause trouble for Libvirt to parse the output. "{'_type': 'type', '_name': 'NameInfo', 'data': { 'job': {'_value': 'str', '_recursive': 'True'}, 'name': {'_value': 'str', '_recursive': 'True'} }, '_recursive': 'False' }" When we describe a DataObject (dict/list/str, one schema, extened schema, schema member, etc), so I we generally use a nested dictionary to describe a DataObject, it will split the metadata with original data two different dictionary level, it's convenient for parse of Libvirt. Here I just use a dict member as an example, actually it's more complex to parse all kinds of data objects.
... ;
* Add a new type of qmp command, that returns a QString as a json literal. query-qmp-schema is defined as this type. (This wouldn't be much code, but may be abused in the future, I'm afraid. However we can review, limit its use to introspection only)
* And return qmp_schema_table from query-qmp-shema, which will be copied to the wire.
Feel free to disagree, it's not a perfect solution. But I really think we need to avoid duplicating "enum", "base", "type", "union", "discriminator", ...
In the past, we didn't consider to extend and add metadata by Python, so Libvirt wasn't happy to just get a raw schema(not extended, no metadata). But now, we already successfully to transfer this work to Python, and we can adjust to make management to be happy for the metadata and format. The problem is that Libvirt should parse the output twice, the json items are also json object string. Eric, Is it acceptabled? Example: * as suggested by Fam to put the metadta with schema data in same dict level * process some special cases by nested dictionary (eg: '*tls': 'bool' ==> 'tls': {'_value': 'bool', '_optional': 'True'} ) * return a strList to Libvirt, the content of string is json object, that contains the metadata. { "return": [ "{'_type': 'enum', '_name': 'ErrorClass', 'data': ['GenericError', 'CommandNotFound', 'DeviceEncrypted', 'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap'], '_recursive': 'False'}", "{'_type': 'command', '_name': 'add_client', 'data': {'tls': {'_value': 'bool', '_optional': 'True'}, 'skipauth': {'_value': 'bool', '_optional': 'True'}, 'protocol': 'str', 'fdname': 'str'}, '_recursive': 'False'}", "{'_type': 'type', '_name': 'NameInfo', 'data': {'job': {'_value': 'str', '_optional': 'True'}, 'name': {'_value': 'str', '_optional': 'True'}}, '_recursive': 'False'}", "{'returns': {'_type': 'type', '_recursive': 'False', 'data': {'job': {'_value': 'str', '_optional': 'True'}, 'name': {'_value': 'str', '_optional': 'True'}}, '_name': 'NameInfo'}, '_name': 'query-name', '_type': 'command', '_recursive': 'False'}", "......", "......", "......" ] }
Fam
-- Amos.
On Mon, Jan 27, 2014 at 04:17:56PM +0800, Amos Kong wrote:
CC Libvirt-list
Original discussion: http://marc.info/?l=qemu-devel&m=139048842504757&w=2 [Qemu-devel] [PATCH v4 0/5] QMP full introspection
On Fri, Jan 24, 2014 at 06:48:31PM +0800, Fam Zheng wrote:
On Thu, 01/23 22:46, Amos Kong wrote:
This patch introduces a new monitor command to query QMP schema information, the return data is a range of schema structs, which contains the useful metadata to help management to check supported features, QMP commands detail, etc.
We use qapi-introspect.py to parse all json definition in qapi-schema.json, and generate a range of dictionaries with metadata. The query command will visit the dictionaries and fill the data to allocated struct tree. Then QMP infrastructure will convert the tree to json string and return to QMP client.
TODO: Wenchao Xia is working to convert QMP events to qapi-schema.json, then event can also be queried by this interface.
I will introduce another command 'query-qga-schema' to query QGA schema information, it's easy to add this support based on this patch.
Signed-off-by: Amos Kong <akong@redhat.com> --- qapi-schema.json | 11 +++ qmp-commands.hx | 42 +++++++++++ qmp.c | 215 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 268 insertions(+)
+static DataObjectMemberList *qobject_to_list_memlist(QObject *data) +{ + const QListEntry *lent; + DataObjectMemberList *list = NULL; + DataObjectMemberList **plist = &list; + QList *qlist = qobject_to_qlist(data); + + for (lent = qlist_first(qlist); lent; lent = qlist_next(lent)) { + DataObjectMemberList *entry = g_malloc0(sizeof(DataObjectMemberList)); + entry->value = qobject_to_dataobjmem(lent->value); + entry->value->has_optional = true; + entry->value->has_name = true; + *plist = entry; + plist = &entry->next; + } + + return list; +} + +static DataObjectMemberList *qobject_to_memlist(QObject *data)
This whole converting is cumbersome. You already did all the traversing through the type jungle in python when generating this, it's not necessary to do the similar thing again here.
We can parse raw schemas and generate json string table, we can't directly return the string / qobject to monitor, C code has to convert the json to qobject, we have to revisit the qobject and convert them to DataObject/DataObjectMember/DataObject... structs.
Alternatively, I think we have a good reason to extend QMP framework as necessary here, as we are doing "QMP introspection", which is a part of the framework:
* Define final output into qmp_schema_table[], no need to box it like:
"{'_obj_member': 'False', '_obj_type': 'enum', '_obj_name': 'ErrorClass', '_obj_data': {'data': ...
just put it content of "qmp-introspection.output.txt" as a long string in the header,
like you would generate in qobject_to_memlist:
const char *qmp_schema_table = "{ 'name': 'ErrorClass', 'type': 'enumeration', 'data': [...]}," "{ 'name': '...', ...},"
The keys are used for metadata might be 'recursive', 'optional', etc. It might exists problem in namespace, let's use '_obj_' or '_' prefix for the metadata keys.
I used a nested dictionary to describe a DataObject, because we can store the metadata and definition to different level, it's helpful in parse the output by Libvirt.
example: "{ 'type': 'NameInfo', 'data': {'*name': 'str', '*job': 'str'} }"
It's good to store _type, _name, data to same level, but the metadata of items of data's value dictionary can't be appended to same level.
"{ '_name': 'NameInfo', '_type': 'type', 'data': { 'name': 'str', '_name_optional': 'True', 'job': 'str', '_job_optional': 'True' } }"
A better solution, but I don't know if it will cause trouble for Libvirt to parse the output.
"{'_type': 'type', '_name': 'NameInfo', 'data': { 'job': {'_value': 'str', '_recursive': 'True'}, 'name': {'_value': 'str', '_recursive': 'True'} }, '_recursive': 'False' }"
When we describe a DataObject (dict/list/str, one schema, extened schema, schema member, etc), so I we generally use a nested dictionary to describe a DataObject, it will split the metadata with original data two different dictionary level, it's convenient for parse of Libvirt. Here I just use a dict member as an example, actually it's more complex to parse all kinds of data objects.
... ;
* Add a new type of qmp command, that returns a QString as a json literal. query-qmp-schema is defined as this type. (This wouldn't be much code, but may be abused in the future, I'm afraid. However we can review, limit its use to introspection only)
* And return qmp_schema_table from query-qmp-shema, which will be copied to the wire.
Feel free to disagree, it's not a perfect solution. But I really think we need to avoid duplicating "enum", "base", "type", "union", "discriminator", ...
In the past, we didn't consider to extend and add metadata by Python, so Libvirt wasn't happy to just get a raw schema(not extended, no metadata). But now, we already successfully to transfer this work to Python, and we can adjust to make management to be happy for the metadata and format.
The problem is that Libvirt should parse the output twice, the json items are also json object string.
Eric, Is it acceptabled?
Example: * as suggested by Fam to put the metadta with schema data in same dict level * process some special cases by nested dictionary (eg: '*tls': 'bool' ==> 'tls': {'_value': 'bool', '_optional': 'True'} ) * return a strList to Libvirt, the content of string is json object, that contains the metadata.
{ "return": [ "{'_type': 'enum', '_name': 'ErrorClass', 'data': ['GenericError', 'CommandNotFound', 'DeviceEncrypted', 'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap'], '_recursive': 'False'}", "{'_type': 'command', '_name': 'add_client', 'data': {'tls': {'_value': 'bool', '_optional': 'True'}, 'skipauth': {'_value': 'bool', '_optional': 'True'}, 'protocol': 'str', 'fdname': 'str'}, '_recursive': 'False'}", "{'_type': 'type', '_name': 'NameInfo', 'data': {'job': {'_value': 'str', '_optional': 'True'}, 'name': {'_value': 'str', '_optional': 'True'}}, '_recursive': 'False'}", "{'returns': {'_type': 'type', '_recursive': 'False', 'data': {'job': {'_value': 'str', '_optional': 'True'}, 'name': {'_value': 'str', '_optional': 'True'}}, '_name': 'NameInfo'}, '_name': 'query-name', '_type': 'command', '_recursive': 'False'}", "......", "......", "......" ] }
Provide two txt files: https://i-kvm.rhcloud.com/static/pub/v5/qapi-introspect.h https://i-kvm.rhcloud.com/static/pub/v5/query-output.txt -- Amos.
Il 27/01/2014 09:17, Amos Kong ha scritto:
CC Libvirt-list
Original discussion: http://marc.info/?l=qemu-devel&m=139048842504757&w=2 [Qemu-devel] [PATCH v4 0/5] QMP full introspection
On Fri, Jan 24, 2014 at 06:48:31PM +0800, Fam Zheng wrote:
On Thu, 01/23 22:46, Amos Kong wrote:
This patch introduces a new monitor command to query QMP schema information, the return data is a range of schema structs, which contains the useful metadata to help management to check supported features, QMP commands detail, etc.
We use qapi-introspect.py to parse all json definition in qapi-schema.json, and generate a range of dictionaries with metadata. The query command will visit the dictionaries and fill the data to allocated struct tree. Then QMP infrastructure will convert the tree to json string and return to QMP client.
TODO: Wenchao Xia is working to convert QMP events to qapi-schema.json, then event can also be queried by this interface.
I will introduce another command 'query-qga-schema' to query QGA schema information, it's easy to add this support based on this patch.
Signed-off-by: Amos Kong <akong@redhat.com> --- qapi-schema.json | 11 +++ qmp-commands.hx | 42 +++++++++++ qmp.c | 215 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 268 insertions(+)
diff --git a/qapi-schema.json b/qapi-schema.json index c63f0ca..6033383 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -4411,3 +4411,14 @@ 'reference-type': 'String', 'type': 'DataObjectType', 'unionobj': 'DataObjectUnion' } } + +## +# @query-qmp-schema +# +# Query QMP schema information +# +# @returns: list of @DataObject +# +# Since: 1.8 +## +{ 'command': 'query-qmp-schema', 'returns': ['DataObject'] } diff --git a/qmp-commands.hx b/qmp-commands.hx index 02cc815..b83762d 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -3291,6 +3291,48 @@ Example: }
EQMP + { + .name = "query-qmp-schema", + .args_type = "", + .mhandler.cmd_new = qmp_marshal_input_query_qmp_schema, + }, + + +SQMP +query-qmp-schema +---------------- + +query qmp schema information + +Return a json-object with the following information: + +- "name": qmp schema name (json-string) +- "type": qmp schema type, it can be 'comand', 'type', 'enum', 'union' +- "returns": return data of qmp command (json-object, optional) + +Example: + +-> { "execute": "query-qmp-schema" } +-> { "return": [ + { + "name": "query-name", + "type": "command", + "returns": { + "name": "NameInfo", + "type": "type", + "data": [ + { + "name": "name", + "optional": true, + "recursive": false, + "type": "str" + } + ] + } + } + } + +EQMP
{ .name = "blockdev-add", diff --git a/qmp.c b/qmp.c index 0f46171..a64ae6d 100644 --- a/qmp.c +++ b/qmp.c @@ -27,6 +27,8 @@ #include "qapi/qmp/qobject.h" #include "qapi/qmp-input-visitor.h" #include "hw/boards.h" +#include "qapi/qmp/qjson.h" +#include "qapi-introspect.h"
NameInfo *qmp_query_name(Error **errp) { @@ -488,6 +490,219 @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp) return arch_query_cpu_definitions(errp); }
+static strList *qobject_to_strlist(QObject *data) +{ + strList *list = NULL; + strList **plist = &list; + QList *qlist; + const QListEntry *lent; + + qlist = qobject_to_qlist(data); + for (lent = qlist_first(qlist); lent; lent = qlist_next(lent)) { + strList *entry = g_malloc0(sizeof(strList)); + entry->value = g_strdup(qobject_get_str(lent->value)); + *plist = entry; + plist = &entry->next; + } + + return list; +} + +static DataObject *qobject_to_dataobj(QObject *data); + +static DataObjectMember *qobject_to_dataobjmem(QObject *data) +{ + + DataObjectMember *member = g_malloc0(sizeof(DataObjectMember)); + + member->type = g_malloc0(sizeof(DataObjectMemberType)); + if (data->type->code == QTYPE_QDICT) { + member->type->kind = DATA_OBJECT_MEMBER_TYPE_KIND_EXTEND; + member->type->extend = qobject_to_dataobj(data); + } else { + member->type->kind = DATA_OBJECT_MEMBER_TYPE_KIND_REFERENCE; + member->type->reference = g_strdup(qobject_get_str(data)); + } + + return member; +} + +static DataObjectMemberList *qobject_to_dict_memlist(QObject *data) +{ + DataObjectMemberList *list = NULL; + DataObjectMemberList **plist = &list; + QDict *qdict = qobject_to_qdict(data); + const QDictEntry *dent; + + for (dent = qdict_first(qdict); dent; dent = qdict_next(qdict, dent)) { + DataObjectMemberList *entry = g_malloc0(sizeof(DataObjectMemberList)); + entry->value = qobject_to_dataobjmem(dent->value); + + entry->value->has_optional = true; + entry->value->has_name = true; + if (dent->key[0] == '*') { + entry->value->optional = true; + entry->value->name = g_strdup(dent->key + 1); + } else { + entry->value->name = g_strdup(dent->key); + } + *plist = entry; + plist = &entry->next; + } + + return list; +} + +static DataObjectMemberList *qobject_to_list_memlist(QObject *data) +{ + const QListEntry *lent; + DataObjectMemberList *list = NULL; + DataObjectMemberList **plist = &list; + QList *qlist = qobject_to_qlist(data); + + for (lent = qlist_first(qlist); lent; lent = qlist_next(lent)) { + DataObjectMemberList *entry = g_malloc0(sizeof(DataObjectMemberList)); + entry->value = qobject_to_dataobjmem(lent->value); + entry->value->has_optional = true; + entry->value->has_name = true; + *plist = entry; + plist = &entry->next; + } + + return list; +} + +static DataObjectMemberList *qobject_to_memlist(QObject *data)
This whole converting is cumbersome. You already did all the traversing through the type jungle in python when generating this, it's not necessary to do the similar thing again here.
We can parse raw schemas and generate json string table, we can't directly return the string / qobject to monitor, C code has to convert the json to qobject, we have to revisit the qobject and convert them to DataObject/DataObjectMember/DataObject... structs.
Alternatively, I think we have a good reason to extend QMP framework as necessary here, as we are doing "QMP introspection", which is a part of the framework:
* Define final output into qmp_schema_table[], no need to box it like:
"{'_obj_member': 'False', '_obj_type': 'enum', '_obj_name': 'ErrorClass', '_obj_data': {'data': ...
just put it content of "qmp-introspection.output.txt" as a long string in the header,
like you would generate in qobject_to_memlist:
const char *qmp_schema_table = "{ 'name': 'ErrorClass', 'type': 'enumeration', 'data': [...]}," "{ 'name': '...', ...},"
The keys are used for metadata might be 'recursive', 'optional', etc. It might exists problem in namespace, let's use '_obj_' or '_' prefix for the metadata keys.
I used a nested dictionary to describe a DataObject, because we can store the metadata and definition to different level, it's helpful in parse the output by Libvirt.
example: "{ 'type': 'NameInfo', 'data': {'*name': 'str', '*job': 'str'} }"
It's good to store _type, _name, data to same level, but the metadata of items of data's value dictionary can't be appended to same level.
"{ '_name': 'NameInfo', '_type': 'type', 'data': { 'name': 'str', '_name_optional': 'True', 'job': 'str', '_job_optional': 'True' } }"
A better solution, but I don't know if it will cause trouble for Libvirt to parse the output.
"{'_type': 'type', '_name': 'NameInfo', 'data': { 'job': {'_value': 'str', '_recursive': 'True'}, 'name': {'_value': 'str', '_recursive': 'True'} }, '_recursive': 'False' }"
When we describe a DataObject (dict/list/str, one schema, extened schema, schema member, etc), so I we generally use a nested dictionary to describe a DataObject, it will split the metadata with original data two different dictionary level, it's convenient for parse of Libvirt. Here I just use a dict member as an example, actually it's more complex to parse all kinds of data objects.
... ;
* Add a new type of qmp command, that returns a QString as a json literal. query-qmp-schema is defined as this type. (This wouldn't be much code, but may be abused in the future, I'm afraid. However we can review, limit its use to introspection only)
* And return qmp_schema_table from query-qmp-shema, which will be copied to the wire.
Feel free to disagree, it's not a perfect solution. But I really think we need to avoid duplicating "enum", "base", "type", "union", "discriminator", ...
In the past, we didn't consider to extend and add metadata by Python, so Libvirt wasn't happy to just get a raw schema(not extended, no metadata). But now, we already successfully to transfer this work to Python, and we can adjust to make management to be happy for the metadata and format.
The problem is that Libvirt should parse the output twice, the json items are also json object string.
Eric, Is it acceptabled?
Example: * as suggested by Fam to put the metadta with schema data in same dict level * process some special cases by nested dictionary (eg: '*tls': 'bool' ==> 'tls': {'_value': 'bool', '_optional': 'True'} ) * return a strList to Libvirt, the content of string is json object, that contains the metadata.
{ "return": [ "{'_type': 'enum', '_name': 'ErrorClass', 'data': ['GenericError', 'CommandNotFound', 'DeviceEncrypted', 'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap'], '_recursive': 'False'}", "{'_type': 'command', '_name': 'add_client', 'data': {'tls': {'_value': 'bool', '_optional': 'True'}, 'skipauth': {'_value': 'bool', '_optional': 'True'}, 'protocol': 'str', 'fdname': 'str'}, '_recursive': 'False'}", "{'_type': 'type', '_name': 'NameInfo', 'data': {'job': {'_value': 'str', '_optional': 'True'}, 'name': {'_value': 'str', '_optional': 'True'}}, '_recursive': 'False'}", "{'returns': {'_type': 'type', '_recursive': 'False', 'data': {'job': {'_value': 'str', '_optional': 'True'}, 'name': {'_value': 'str', '_optional': 'True'}}, '_name': 'NameInfo'}, '_name': 'query-name', '_type': 'command', '_recursive': 'False'}", "......", "......", "......" ] }
Fam
No, I don't like this. QAPI types are perfectly able to "describe themselves", there is no need to escape to JSON. Let's just do what this series is doing, minus the unnecessary recursion. Paolo
On Mon, Jan 27, 2014 at 10:38:24AM +0100, Paolo Bonzini wrote:
Il 27/01/2014 09:17, Amos Kong ha scritto:
CC Libvirt-list
Original discussion: http://marc.info/?l=qemu-devel&m=139048842504757&w=2 [Qemu-devel] [PATCH v4 0/5] QMP full introspection
On Fri, Jan 24, 2014 at 06:48:31PM +0800, Fam Zheng wrote:
On Thu, 01/23 22:46, Amos Kong wrote:
This patch introduces a new monitor command to query QMP schema information, the return data is a range of schema structs, which contains the useful metadata to help management to check supported features, QMP commands detail, etc.
We use qapi-introspect.py to parse all json definition in qapi-schema.json, and generate a range of dictionaries with metadata. The query command will visit the dictionaries and fill the data to allocated struct tree. Then QMP infrastructure will convert the tree to json string and return to QMP client.
TODO: Wenchao Xia is working to convert QMP events to qapi-schema.json, then event can also be queried by this interface.
I will introduce another command 'query-qga-schema' to query QGA schema information, it's easy to add this support based on this patch.
Signed-off-by: Amos Kong <akong@redhat.com> --- qapi-schema.json | 11 +++ qmp-commands.hx | 42 +++++++++++ qmp.c | 215 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 268 insertions(+)
diff --git a/qapi-schema.json b/qapi-schema.json index c63f0ca..6033383 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -4411,3 +4411,14 @@ 'reference-type': 'String', 'type': 'DataObjectType', 'unionobj': 'DataObjectUnion' } } + +## +# @query-qmp-schema +# +# Query QMP schema information +# +# @returns: list of @DataObject +# +# Since: 1.8 +## +{ 'command': 'query-qmp-schema', 'returns': ['DataObject'] } diff --git a/qmp-commands.hx b/qmp-commands.hx index 02cc815..b83762d 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -3291,6 +3291,48 @@ Example: }
EQMP + { + .name = "query-qmp-schema", + .args_type = "", + .mhandler.cmd_new = qmp_marshal_input_query_qmp_schema, + }, + + +SQMP +query-qmp-schema +---------------- + +query qmp schema information + +Return a json-object with the following information: + +- "name": qmp schema name (json-string) +- "type": qmp schema type, it can be 'comand', 'type', 'enum', 'union' +- "returns": return data of qmp command (json-object, optional) + +Example: + +-> { "execute": "query-qmp-schema" } +-> { "return": [ + { + "name": "query-name", + "type": "command", + "returns": { + "name": "NameInfo", + "type": "type", + "data": [ + { + "name": "name", + "optional": true, + "recursive": false, + "type": "str" + } + ] + } + } + } + +EQMP
{ .name = "blockdev-add", diff --git a/qmp.c b/qmp.c index 0f46171..a64ae6d 100644 --- a/qmp.c +++ b/qmp.c @@ -27,6 +27,8 @@ #include "qapi/qmp/qobject.h" #include "qapi/qmp-input-visitor.h" #include "hw/boards.h" +#include "qapi/qmp/qjson.h" +#include "qapi-introspect.h"
NameInfo *qmp_query_name(Error **errp) { @@ -488,6 +490,219 @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp) return arch_query_cpu_definitions(errp); }
+static strList *qobject_to_strlist(QObject *data) +{ + strList *list = NULL; + strList **plist = &list; + QList *qlist; + const QListEntry *lent; + + qlist = qobject_to_qlist(data); + for (lent = qlist_first(qlist); lent; lent = qlist_next(lent)) { + strList *entry = g_malloc0(sizeof(strList)); + entry->value = g_strdup(qobject_get_str(lent->value)); + *plist = entry; + plist = &entry->next; + } + + return list; +} + +static DataObject *qobject_to_dataobj(QObject *data); + +static DataObjectMember *qobject_to_dataobjmem(QObject *data) +{ + + DataObjectMember *member = g_malloc0(sizeof(DataObjectMember)); + + member->type = g_malloc0(sizeof(DataObjectMemberType)); + if (data->type->code == QTYPE_QDICT) { + member->type->kind = DATA_OBJECT_MEMBER_TYPE_KIND_EXTEND; + member->type->extend = qobject_to_dataobj(data); + } else { + member->type->kind = DATA_OBJECT_MEMBER_TYPE_KIND_REFERENCE; + member->type->reference = g_strdup(qobject_get_str(data)); + } + + return member; +} + +static DataObjectMemberList *qobject_to_dict_memlist(QObject *data) +{ + DataObjectMemberList *list = NULL; + DataObjectMemberList **plist = &list; + QDict *qdict = qobject_to_qdict(data); + const QDictEntry *dent; + + for (dent = qdict_first(qdict); dent; dent = qdict_next(qdict, dent)) { + DataObjectMemberList *entry = g_malloc0(sizeof(DataObjectMemberList)); + entry->value = qobject_to_dataobjmem(dent->value); + + entry->value->has_optional = true; + entry->value->has_name = true; + if (dent->key[0] == '*') { + entry->value->optional = true; + entry->value->name = g_strdup(dent->key + 1); + } else { + entry->value->name = g_strdup(dent->key); + } + *plist = entry; + plist = &entry->next; + } + + return list; +} + +static DataObjectMemberList *qobject_to_list_memlist(QObject *data) +{ + const QListEntry *lent; + DataObjectMemberList *list = NULL; + DataObjectMemberList **plist = &list; + QList *qlist = qobject_to_qlist(data); + + for (lent = qlist_first(qlist); lent; lent = qlist_next(lent)) { + DataObjectMemberList *entry = g_malloc0(sizeof(DataObjectMemberList)); + entry->value = qobject_to_dataobjmem(lent->value); + entry->value->has_optional = true; + entry->value->has_name = true; + *plist = entry; + plist = &entry->next; + } + + return list; +} + +static DataObjectMemberList *qobject_to_memlist(QObject *data)
This whole converting is cumbersome. You already did all the traversing through the type jungle in python when generating this, it's not necessary to do the similar thing again here.
We can parse raw schemas and generate json string table, we can't directly return the string / qobject to monitor, C code has to convert the json to qobject, we have to revisit the qobject and convert them to DataObject/DataObjectMember/DataObject... structs.
Alternatively, I think we have a good reason to extend QMP framework as necessary here, as we are doing "QMP introspection", which is a part of the framework:
* Define final output into qmp_schema_table[], no need to box it like:
"{'_obj_member': 'False', '_obj_type': 'enum', '_obj_name': 'ErrorClass', '_obj_data': {'data': ...
just put it content of "qmp-introspection.output.txt" as a long string in the header,
like you would generate in qobject_to_memlist:
const char *qmp_schema_table = "{ 'name': 'ErrorClass', 'type': 'enumeration', 'data': [...]}," "{ 'name': '...', ...},"
The keys are used for metadata might be 'recursive', 'optional', etc. It might exists problem in namespace, let's use '_obj_' or '_' prefix for the metadata keys.
I used a nested dictionary to describe a DataObject, because we can store the metadata and definition to different level, it's helpful in parse the output by Libvirt.
example: "{ 'type': 'NameInfo', 'data': {'*name': 'str', '*job': 'str'} }"
It's good to store _type, _name, data to same level, but the metadata of items of data's value dictionary can't be appended to same level.
"{ '_name': 'NameInfo', '_type': 'type', 'data': { 'name': 'str', '_name_optional': 'True', 'job': 'str', '_job_optional': 'True' } }"
A better solution, but I don't know if it will cause trouble for Libvirt to parse the output.
"{'_type': 'type', '_name': 'NameInfo', 'data': { 'job': {'_value': 'str', '_recursive': 'True'}, 'name': {'_value': 'str', '_recursive': 'True'} }, '_recursive': 'False' }"
When we describe a DataObject (dict/list/str, one schema, extened schema, schema member, etc), so I we generally use a nested dictionary to describe a DataObject, it will split the metadata with original data two different dictionary level, it's convenient for parse of Libvirt. Here I just use a dict member as an example, actually it's more complex to parse all kinds of data objects.
... ;
* Add a new type of qmp command, that returns a QString as a json literal. query-qmp-schema is defined as this type. (This wouldn't be much code, but may be abused in the future, I'm afraid. However we can review, limit its use to introspection only)
* And return qmp_schema_table from query-qmp-shema, which will be copied to the wire.
Feel free to disagree, it's not a perfect solution. But I really think we need to avoid duplicating "enum", "base", "type", "union", "discriminator", ...
In the past, we didn't consider to extend and add metadata by Python, so Libvirt wasn't happy to just get a raw schema(not extended, no metadata). But now, we already successfully to transfer this work to Python, and we can adjust to make management to be happy for the metadata and format.
The problem is that Libvirt should parse the output twice, the json items are also json object string.
Eric, Is it acceptabled?
Example: * as suggested by Fam to put the metadta with schema data in same dict level * process some special cases by nested dictionary (eg: '*tls': 'bool' ==> 'tls': {'_value': 'bool', '_optional': 'True'} ) * return a strList to Libvirt, the content of string is json object, that contains the metadata.
{ "return": [ "{'_type': 'enum', '_name': 'ErrorClass', 'data': ['GenericError', 'CommandNotFound', 'DeviceEncrypted', 'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap'], '_recursive': 'False'}", "{'_type': 'command', '_name': 'add_client', 'data': {'tls': {'_value': 'bool', '_optional': 'True'}, 'skipauth': {'_value': 'bool', '_optional': 'True'}, 'protocol': 'str', 'fdname': 'str'}, '_recursive': 'False'}", "{'_type': 'type', '_name': 'NameInfo', 'data': {'job': {'_value': 'str', '_optional': 'True'}, 'name': {'_value': 'str', '_optional': 'True'}}, '_recursive': 'False'}", "{'returns': {'_type': 'type', '_recursive': 'False', 'data': {'job': {'_value': 'str', '_optional': 'True'}, 'name': {'_value': 'str', '_optional': 'True'}}, '_name': 'NameInfo'}, '_name': 'query-name', '_type': 'command', '_recursive': 'False'}", "......", "......", "......" ] }
Fam
No, I don't like this.
QAPI types are perfectly able to "describe themselves", there is no need to escape to JSON.
Let's just do what this series is doing, minus the unnecessary recursion.
The recursion extend was requested by Libvirt, they want to get a extended output with metadata to avoid heavy parsing in Libvirt. Let's see the response of libvirt people.
Paolo
-- Amos.
Il 27/01/2014 11:07, Amos Kong ha scritto:
No, I don't like this.
QAPI types are perfectly able to "describe themselves", there is no need to escape to JSON.
Let's just do what this series is doing, minus the unnecessary recursion.
The recursion extend was requested by Libvirt, they want to get a extended output with metadata to avoid heavy parsing in Libvirt.
Let's see the response of libvirt people.
I think Eric already answered. In any case, 4 seconds to produce 1.7 MB is a sign of a bug in my opinion. What does the profile say? Paolo
On Mon, 01/27 10:38, Paolo Bonzini wrote:
Il 27/01/2014 09:17, Amos Kong ha scritto:
CC Libvirt-list
Original discussion: http://marc.info/?l=qemu-devel&m=139048842504757&w=2 [Qemu-devel] [PATCH v4 0/5] QMP full introspection
On Fri, Jan 24, 2014 at 06:48:31PM +0800, Fam Zheng wrote:
On Thu, 01/23 22:46, Amos Kong wrote:
This patch introduces a new monitor command to query QMP schema information, the return data is a range of schema structs, which contains the useful metadata to help management to check supported features, QMP commands detail, etc.
We use qapi-introspect.py to parse all json definition in qapi-schema.json, and generate a range of dictionaries with metadata. The query command will visit the dictionaries and fill the data to allocated struct tree. Then QMP infrastructure will convert the tree to json string and return to QMP client.
TODO: Wenchao Xia is working to convert QMP events to qapi-schema.json, then event can also be queried by this interface.
I will introduce another command 'query-qga-schema' to query QGA schema information, it's easy to add this support based on this patch.
Signed-off-by: Amos Kong <akong@redhat.com> --- qapi-schema.json | 11 +++ qmp-commands.hx | 42 +++++++++++ qmp.c | 215 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 268 insertions(+)
diff --git a/qapi-schema.json b/qapi-schema.json index c63f0ca..6033383 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -4411,3 +4411,14 @@ 'reference-type': 'String', 'type': 'DataObjectType', 'unionobj': 'DataObjectUnion' } } + +## +# @query-qmp-schema +# +# Query QMP schema information +# +# @returns: list of @DataObject +# +# Since: 1.8 +## +{ 'command': 'query-qmp-schema', 'returns': ['DataObject'] } diff --git a/qmp-commands.hx b/qmp-commands.hx index 02cc815..b83762d 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -3291,6 +3291,48 @@ Example: }
EQMP + { + .name = "query-qmp-schema", + .args_type = "", + .mhandler.cmd_new = qmp_marshal_input_query_qmp_schema, + }, + + +SQMP +query-qmp-schema +---------------- + +query qmp schema information + +Return a json-object with the following information: + +- "name": qmp schema name (json-string) +- "type": qmp schema type, it can be 'comand', 'type', 'enum', 'union' +- "returns": return data of qmp command (json-object, optional) + +Example: + +-> { "execute": "query-qmp-schema" } +-> { "return": [ + { + "name": "query-name", + "type": "command", + "returns": { + "name": "NameInfo", + "type": "type", + "data": [ + { + "name": "name", + "optional": true, + "recursive": false, + "type": "str" + } + ] + } + } + } + +EQMP
{ .name = "blockdev-add", diff --git a/qmp.c b/qmp.c index 0f46171..a64ae6d 100644 --- a/qmp.c +++ b/qmp.c @@ -27,6 +27,8 @@ #include "qapi/qmp/qobject.h" #include "qapi/qmp-input-visitor.h" #include "hw/boards.h" +#include "qapi/qmp/qjson.h" +#include "qapi-introspect.h"
NameInfo *qmp_query_name(Error **errp) { @@ -488,6 +490,219 @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp) return arch_query_cpu_definitions(errp); }
+static strList *qobject_to_strlist(QObject *data) +{ + strList *list = NULL; + strList **plist = &list; + QList *qlist; + const QListEntry *lent; + + qlist = qobject_to_qlist(data); + for (lent = qlist_first(qlist); lent; lent = qlist_next(lent)) { + strList *entry = g_malloc0(sizeof(strList)); + entry->value = g_strdup(qobject_get_str(lent->value)); + *plist = entry; + plist = &entry->next; + } + + return list; +} + +static DataObject *qobject_to_dataobj(QObject *data); + +static DataObjectMember *qobject_to_dataobjmem(QObject *data) +{ + + DataObjectMember *member = g_malloc0(sizeof(DataObjectMember)); + + member->type = g_malloc0(sizeof(DataObjectMemberType)); + if (data->type->code == QTYPE_QDICT) { + member->type->kind = DATA_OBJECT_MEMBER_TYPE_KIND_EXTEND; + member->type->extend = qobject_to_dataobj(data); + } else { + member->type->kind = DATA_OBJECT_MEMBER_TYPE_KIND_REFERENCE; + member->type->reference = g_strdup(qobject_get_str(data)); + } + + return member; +} + +static DataObjectMemberList *qobject_to_dict_memlist(QObject *data) +{ + DataObjectMemberList *list = NULL; + DataObjectMemberList **plist = &list; + QDict *qdict = qobject_to_qdict(data); + const QDictEntry *dent; + + for (dent = qdict_first(qdict); dent; dent = qdict_next(qdict, dent)) { + DataObjectMemberList *entry = g_malloc0(sizeof(DataObjectMemberList)); + entry->value = qobject_to_dataobjmem(dent->value); + + entry->value->has_optional = true; + entry->value->has_name = true; + if (dent->key[0] == '*') { + entry->value->optional = true; + entry->value->name = g_strdup(dent->key + 1); + } else { + entry->value->name = g_strdup(dent->key); + } + *plist = entry; + plist = &entry->next; + } + + return list; +} + +static DataObjectMemberList *qobject_to_list_memlist(QObject *data) +{ + const QListEntry *lent; + DataObjectMemberList *list = NULL; + DataObjectMemberList **plist = &list; + QList *qlist = qobject_to_qlist(data); + + for (lent = qlist_first(qlist); lent; lent = qlist_next(lent)) { + DataObjectMemberList *entry = g_malloc0(sizeof(DataObjectMemberList)); + entry->value = qobject_to_dataobjmem(lent->value); + entry->value->has_optional = true; + entry->value->has_name = true; + *plist = entry; + plist = &entry->next; + } + + return list; +} + +static DataObjectMemberList *qobject_to_memlist(QObject *data)
This whole converting is cumbersome. You already did all the traversing through the type jungle in python when generating this, it's not necessary to do the similar thing again here.
We can parse raw schemas and generate json string table, we can't directly return the string / qobject to monitor, C code has to convert the json to qobject, we have to revisit the qobject and convert them to DataObject/DataObjectMember/DataObject... structs.
Alternatively, I think we have a good reason to extend QMP framework as necessary here, as we are doing "QMP introspection", which is a part of the framework:
* Define final output into qmp_schema_table[], no need to box it like:
"{'_obj_member': 'False', '_obj_type': 'enum', '_obj_name': 'ErrorClass', '_obj_data': {'data': ...
just put it content of "qmp-introspection.output.txt" as a long string in the header,
like you would generate in qobject_to_memlist:
const char *qmp_schema_table = "{ 'name': 'ErrorClass', 'type': 'enumeration', 'data': [...]}," "{ 'name': '...', ...},"
The keys are used for metadata might be 'recursive', 'optional', etc. It might exists problem in namespace, let's use '_obj_' or '_' prefix for the metadata keys.
I used a nested dictionary to describe a DataObject, because we can store the metadata and definition to different level, it's helpful in parse the output by Libvirt.
example: "{ 'type': 'NameInfo', 'data': {'*name': 'str', '*job': 'str'} }"
It's good to store _type, _name, data to same level, but the metadata of items of data's value dictionary can't be appended to same level.
"{ '_name': 'NameInfo', '_type': 'type', 'data': { 'name': 'str', '_name_optional': 'True', 'job': 'str', '_job_optional': 'True' } }"
A better solution, but I don't know if it will cause trouble for Libvirt to parse the output.
"{'_type': 'type', '_name': 'NameInfo', 'data': { 'job': {'_value': 'str', '_recursive': 'True'}, 'name': {'_value': 'str', '_recursive': 'True'} }, '_recursive': 'False' }"
When we describe a DataObject (dict/list/str, one schema, extened schema, schema member, etc), so I we generally use a nested dictionary to describe a DataObject, it will split the metadata with original data two different dictionary level, it's convenient for parse of Libvirt. Here I just use a dict member as an example, actually it's more complex to parse all kinds of data objects.
... ;
* Add a new type of qmp command, that returns a QString as a json literal. query-qmp-schema is defined as this type. (This wouldn't be much code, but may be abused in the future, I'm afraid. However we can review, limit its use to introspection only)
* And return qmp_schema_table from query-qmp-shema, which will be copied to the wire.
Feel free to disagree, it's not a perfect solution. But I really think we need to avoid duplicating "enum", "base", "type", "union", "discriminator", ...
In the past, we didn't consider to extend and add metadata by Python, so Libvirt wasn't happy to just get a raw schema(not extended, no metadata). But now, we already successfully to transfer this work to Python, and we can adjust to make management to be happy for the metadata and format.
The problem is that Libvirt should parse the output twice, the json items are also json object string.
Eric, Is it acceptabled?
Example: * as suggested by Fam to put the metadta with schema data in same dict level * process some special cases by nested dictionary (eg: '*tls': 'bool' ==> 'tls': {'_value': 'bool', '_optional': 'True'} ) * return a strList to Libvirt, the content of string is json object, that contains the metadata.
{ "return": [ "{'_type': 'enum', '_name': 'ErrorClass', 'data': ['GenericError', 'CommandNotFound', 'DeviceEncrypted', 'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap'], '_recursive': 'False'}", "{'_type': 'command', '_name': 'add_client', 'data': {'tls': {'_value': 'bool', '_optional': 'True'}, 'skipauth': {'_value': 'bool', '_optional': 'True'}, 'protocol': 'str', 'fdname': 'str'}, '_recursive': 'False'}", "{'_type': 'type', '_name': 'NameInfo', 'data': {'job': {'_value': 'str', '_optional': 'True'}, 'name': {'_value': 'str', '_optional': 'True'}}, '_recursive': 'False'}", "{'returns': {'_type': 'type', '_recursive': 'False', 'data': {'job': {'_value': 'str', '_optional': 'True'}, 'name': {'_value': 'str', '_optional': 'True'}}, '_name': 'NameInfo'}, '_name': 'query-name', '_type': 'command', '_recursive': 'False'}", "......", "......", "......" ] }
Fam
No, I don't like this.
QAPI types are perfectly able to "describe themselves", there is no need to escape to JSON.
Let's just do what this series is doing, minus the unnecessary recursion.
I think the interface is fine with v4, no need to change to string array. It's just the implementation in this series shows some duplication: generator in python parser in C qapi-schema.json ----------------------> qapi-introspect.h ------------------> qmp result When you look at "qmp result", it is qapi-schema.json rewritten in a formal syntax (a real schema?). But when you look at qapi-introspect.h, that is generated by python and parsed by C, it's a schema of the qapi-schema. So the python code and the C code, on the arrows, are just (to a big degree) reversal to each other, as they both implement the "schema's schema" logic: one for generation and the other for parsing. They both write code for each type like "command", "union", "discriminator", etc. My question is why is this generate-and-parse necessary? Will it be reused in the future? Can we achieve it with less duplication? Fam
On Mon, Jan 27, 2014 at 06:46:31PM +0800, Fam Zheng wrote:
On Mon, 01/27 10:38, Paolo Bonzini wrote:
Il 27/01/2014 09:17, Amos Kong ha scritto:
CC Libvirt-list
Original discussion: http://marc.info/?l=qemu-devel&m=139048842504757&w=2 [Qemu-devel] [PATCH v4 0/5] QMP full introspection
On Fri, Jan 24, 2014 at 06:48:31PM +0800, Fam Zheng wrote:
On Thu, 01/23 22:46, Amos Kong wrote:
This patch introduces a new monitor command to query QMP schema information, the return data is a range of schema structs, which contains the useful metadata to help management to check supported features, QMP commands detail, etc.
We use qapi-introspect.py to parse all json definition in qapi-schema.json, and generate a range of dictionaries with metadata. The query command will visit the dictionaries and fill the data to allocated struct tree. Then QMP infrastructure will convert the tree to json string and return to QMP client.
TODO: Wenchao Xia is working to convert QMP events to qapi-schema.json, then event can also be queried by this interface.
I will introduce another command 'query-qga-schema' to query QGA schema information, it's easy to add this support based on this patch.
Signed-off-by: Amos Kong <akong@redhat.com> --- qapi-schema.json | 11 +++ qmp-commands.hx | 42 +++++++++++ qmp.c | 215 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 268 insertions(+)
diff --git a/qapi-schema.json b/qapi-schema.json index c63f0ca..6033383 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -4411,3 +4411,14 @@ 'reference-type': 'String', 'type': 'DataObjectType', 'unionobj': 'DataObjectUnion' } } + +## +# @query-qmp-schema +# +# Query QMP schema information +# +# @returns: list of @DataObject +# +# Since: 1.8 +## +{ 'command': 'query-qmp-schema', 'returns': ['DataObject'] } diff --git a/qmp-commands.hx b/qmp-commands.hx index 02cc815..b83762d 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -3291,6 +3291,48 @@ Example: }
EQMP + { + .name = "query-qmp-schema", + .args_type = "", + .mhandler.cmd_new = qmp_marshal_input_query_qmp_schema, + }, + + +SQMP +query-qmp-schema +---------------- + +query qmp schema information + +Return a json-object with the following information: + +- "name": qmp schema name (json-string) +- "type": qmp schema type, it can be 'comand', 'type', 'enum', 'union' +- "returns": return data of qmp command (json-object, optional) + +Example: + +-> { "execute": "query-qmp-schema" } +-> { "return": [ + { + "name": "query-name", + "type": "command", + "returns": { + "name": "NameInfo", + "type": "type", + "data": [ + { + "name": "name", + "optional": true, + "recursive": false, + "type": "str" + } + ] + } + } + } + +EQMP
{ .name = "blockdev-add", diff --git a/qmp.c b/qmp.c index 0f46171..a64ae6d 100644 --- a/qmp.c +++ b/qmp.c @@ -27,6 +27,8 @@ #include "qapi/qmp/qobject.h" #include "qapi/qmp-input-visitor.h" #include "hw/boards.h" +#include "qapi/qmp/qjson.h" +#include "qapi-introspect.h"
NameInfo *qmp_query_name(Error **errp) { @@ -488,6 +490,219 @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp) return arch_query_cpu_definitions(errp); }
+static strList *qobject_to_strlist(QObject *data) +{ + strList *list = NULL; + strList **plist = &list; + QList *qlist; + const QListEntry *lent; + + qlist = qobject_to_qlist(data); + for (lent = qlist_first(qlist); lent; lent = qlist_next(lent)) { + strList *entry = g_malloc0(sizeof(strList)); + entry->value = g_strdup(qobject_get_str(lent->value)); + *plist = entry; + plist = &entry->next; + } + + return list; +} + +static DataObject *qobject_to_dataobj(QObject *data); + +static DataObjectMember *qobject_to_dataobjmem(QObject *data) +{ + + DataObjectMember *member = g_malloc0(sizeof(DataObjectMember)); + + member->type = g_malloc0(sizeof(DataObjectMemberType)); + if (data->type->code == QTYPE_QDICT) { + member->type->kind = DATA_OBJECT_MEMBER_TYPE_KIND_EXTEND; + member->type->extend = qobject_to_dataobj(data); + } else { + member->type->kind = DATA_OBJECT_MEMBER_TYPE_KIND_REFERENCE; + member->type->reference = g_strdup(qobject_get_str(data)); + } + + return member; +} + +static DataObjectMemberList *qobject_to_dict_memlist(QObject *data) +{ + DataObjectMemberList *list = NULL; + DataObjectMemberList **plist = &list; + QDict *qdict = qobject_to_qdict(data); + const QDictEntry *dent; + + for (dent = qdict_first(qdict); dent; dent = qdict_next(qdict, dent)) { + DataObjectMemberList *entry = g_malloc0(sizeof(DataObjectMemberList)); + entry->value = qobject_to_dataobjmem(dent->value); + + entry->value->has_optional = true; + entry->value->has_name = true; + if (dent->key[0] == '*') { + entry->value->optional = true; + entry->value->name = g_strdup(dent->key + 1); + } else { + entry->value->name = g_strdup(dent->key); + } + *plist = entry; + plist = &entry->next; + } + + return list; +} + +static DataObjectMemberList *qobject_to_list_memlist(QObject *data) +{ + const QListEntry *lent; + DataObjectMemberList *list = NULL; + DataObjectMemberList **plist = &list; + QList *qlist = qobject_to_qlist(data); + + for (lent = qlist_first(qlist); lent; lent = qlist_next(lent)) { + DataObjectMemberList *entry = g_malloc0(sizeof(DataObjectMemberList)); + entry->value = qobject_to_dataobjmem(lent->value); + entry->value->has_optional = true; + entry->value->has_name = true; + *plist = entry; + plist = &entry->next; + } + + return list; +} + +static DataObjectMemberList *qobject_to_memlist(QObject *data)
This whole converting is cumbersome. You already did all the traversing through the type jungle in python when generating this, it's not necessary to do the similar thing again here.
We can parse raw schemas and generate json string table, we can't directly return the string / qobject to monitor, C code has to convert the json to qobject, we have to revisit the qobject and convert them to DataObject/DataObjectMember/DataObject... structs.
Alternatively, I think we have a good reason to extend QMP framework as necessary here, as we are doing "QMP introspection", which is a part of the framework:
* Define final output into qmp_schema_table[], no need to box it like:
"{'_obj_member': 'False', '_obj_type': 'enum', '_obj_name': 'ErrorClass', '_obj_data': {'data': ...
just put it content of "qmp-introspection.output.txt" as a long string in the header,
like you would generate in qobject_to_memlist:
const char *qmp_schema_table = "{ 'name': 'ErrorClass', 'type': 'enumeration', 'data': [...]}," "{ 'name': '...', ...},"
The keys are used for metadata might be 'recursive', 'optional', etc. It might exists problem in namespace, let's use '_obj_' or '_' prefix for the metadata keys.
I used a nested dictionary to describe a DataObject, because we can store the metadata and definition to different level, it's helpful in parse the output by Libvirt.
example: "{ 'type': 'NameInfo', 'data': {'*name': 'str', '*job': 'str'} }"
It's good to store _type, _name, data to same level, but the metadata of items of data's value dictionary can't be appended to same level.
"{ '_name': 'NameInfo', '_type': 'type', 'data': { 'name': 'str', '_name_optional': 'True', 'job': 'str', '_job_optional': 'True' } }"
A better solution, but I don't know if it will cause trouble for Libvirt to parse the output.
"{'_type': 'type', '_name': 'NameInfo', 'data': { 'job': {'_value': 'str', '_recursive': 'True'}, 'name': {'_value': 'str', '_recursive': 'True'} }, '_recursive': 'False' }"
When we describe a DataObject (dict/list/str, one schema, extened schema, schema member, etc), so I we generally use a nested dictionary to describe a DataObject, it will split the metadata with original data two different dictionary level, it's convenient for parse of Libvirt. Here I just use a dict member as an example, actually it's more complex to parse all kinds of data objects.
... ;
* Add a new type of qmp command, that returns a QString as a json literal. query-qmp-schema is defined as this type. (This wouldn't be much code, but may be abused in the future, I'm afraid. However we can review, limit its use to introspection only)
* And return qmp_schema_table from query-qmp-shema, which will be copied to the wire.
Feel free to disagree, it's not a perfect solution. But I really think we need to avoid duplicating "enum", "base", "type", "union", "discriminator", ...
In the past, we didn't consider to extend and add metadata by Python, so Libvirt wasn't happy to just get a raw schema(not extended, no metadata). But now, we already successfully to transfer this work to Python, and we can adjust to make management to be happy for the metadata and format.
The problem is that Libvirt should parse the output twice, the json items are also json object string.
Eric, Is it acceptabled?
Example: * as suggested by Fam to put the metadta with schema data in same dict level * process some special cases by nested dictionary (eg: '*tls': 'bool' ==> 'tls': {'_value': 'bool', '_optional': 'True'} ) * return a strList to Libvirt, the content of string is json object, that contains the metadata.
{ "return": [ "{'_type': 'enum', '_name': 'ErrorClass', 'data': ['GenericError', 'CommandNotFound', 'DeviceEncrypted', 'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap'], '_recursive': 'False'}", "{'_type': 'command', '_name': 'add_client', 'data': {'tls': {'_value': 'bool', '_optional': 'True'}, 'skipauth': {'_value': 'bool', '_optional': 'True'}, 'protocol': 'str', 'fdname': 'str'}, '_recursive': 'False'}", "{'_type': 'type', '_name': 'NameInfo', 'data': {'job': {'_value': 'str', '_optional': 'True'}, 'name': {'_value': 'str', '_optional': 'True'}}, '_recursive': 'False'}", "{'returns': {'_type': 'type', '_recursive': 'False', 'data': {'job': {'_value': 'str', '_optional': 'True'}, 'name': {'_value': 'str', '_optional': 'True'}}, '_name': 'NameInfo'}, '_name': 'query-name', '_type': 'command', '_recursive': 'False'}", "......", "......", "......" ] }
Fam
No, I don't like this.
QAPI types are perfectly able to "describe themselves", there is no need to escape to JSON.
Let's just do what this series is doing, minus the unnecessary recursion.
I think the interface is fine with v4, no need to change to string array. It's just the implementation in this series shows some duplication:
The V4 output is very close to original conclusion about DataObject/metadata.
generator in python parser in C qapi-schema.json ----------------------> qapi-introspect.h ------------------> qmp result
When you look at "qmp result", it is qapi-schema.json rewritten in a formal syntax (a real schema?). But when you look at qapi-introspect.h, that is generated by python and parsed by C, it's a schema of the qapi-schema. So the python code and the C code, on the arrows, are just (to a big degree) reversal to each other, as they both implement the "schema's schema" logic: one for generation and the other for parsing. They both write code for each type like "command", "union", "discriminator", etc.
Right, we can't avoid the duplicate if we want to connect multiple interfaces (Python, QMP, QAPI, JSON).
My question is why is this generate-and-parse necessary?
It's request of Libvirt, actually we can directly return the raw schema to Libvirt without extending/parsing, then Libvirt parse by itself.
Will it be reused in the future?
It seems not.
Can we achieve it with less duplication?
Fam
Let's see the feedback of Eric. Thanks, Amos
Il 28/01/2014 11:45, Amos Kong ha scritto:
My question is why is this generate-and-parse necessary?
It's request of Libvirt, actually we can directly return the raw schema to Libvirt without extending/parsing, then Libvirt parse by itself.
Can we achieve it with less duplication?
Let's see the feedback of Eric.
Eric's feedback is certainly useful, but I think we need to look at it from the QEMU perspective more than the libvirt perspective. Passing the raw schema and letting libvirt parse it is a Really Bad idea from the QEMU perspective, in my opinion, even if it means a little more work now and even if libvirt is willing to add the parser. First and foremost, the current "pseudo-JSON" encoding of the schema is nothing but a QEMU implementation detail. The "pseudo-JSON" syntax definitely shouldn't percolate to the QAPI documentation. Using normal QAPI structs means that the normal tool for documentation (qapi-schema.json doc comments) applies just as well to QAPI schema introspection Second, if one day we were to change the schema representation from "pseudo-JSON" to something else, we would have to carry a "pseudo-JSON" serializer for backwards compatibility. Building QAPI structs and relying on the normal formatting machinery is very different from putting together strings manually. The schema must be emitted as JSON data, not as a string. I'm not willing to compromise on this point. :) Paolo
On 01/28/2014 04:14 AM, Paolo Bonzini wrote:
Let's see the feedback of Eric.
Eric's feedback is certainly useful, but I think we need to look at it from the QEMU perspective more than the libvirt perspective.
Passing the raw schema and letting libvirt parse it is a Really Bad idea from the QEMU perspective, in my opinion, even if it means a little more work now and even if libvirt is willing to add the parser.
Libvirt wants to parse formal qapi, not pseudo-JSON. I still have on my to-do list to read the v4 schema and make sure that libvirt can live with it.
First and foremost, the current "pseudo-JSON" encoding of the schema is nothing but a QEMU implementation detail. The "pseudo-JSON" syntax definitely shouldn't percolate to the QAPI documentation. Using normal QAPI structs means that the normal tool for documentation (qapi-schema.json doc comments) applies just as well to QAPI schema introspection
Agreed - I definitely want the output of the query command to be fully described by qapi. Which means we DO have to convert from the pseudo-JSON of the qapi file into the final formal qapi format. But the conversion is known at code generation time, so you should do it as part of your python code generator, and not repeat the conversion at runtime in the C code. That is, the C code should have everything already split out into the data structures it needs to just output the qapi structure as documented for the formal qapi definition.
Second, if one day we were to change the schema representation from "pseudo-JSON" to something else, we would have to carry a "pseudo-JSON" serializer for backwards compatibility. Building QAPI structs and relying on the normal formatting machinery is very different from putting together strings manually.
The schema must be emitted as JSON data, not as a string. I'm not willing to compromise on this point. :)
Nor am I. The output MUST be formal JSON, described by the qapi (whether we change the syntax of qapi-schema.json and tweak the .py code generators in the future should not matter, the output of the QMP command will be the same formal JSON). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Tue, 01/28 06:58, Eric Blake wrote:
On 01/28/2014 04:14 AM, Paolo Bonzini wrote:
Let's see the feedback of Eric.
Eric's feedback is certainly useful, but I think we need to look at it from the QEMU perspective more than the libvirt perspective.
Passing the raw schema and letting libvirt parse it is a Really Bad idea from the QEMU perspective, in my opinion, even if it means a little more work now and even if libvirt is willing to add the parser.
Libvirt wants to parse formal qapi, not pseudo-JSON. I still have on my to-do list to read the v4 schema and make sure that libvirt can live with it.
First and foremost, the current "pseudo-JSON" encoding of the schema is nothing but a QEMU implementation detail. The "pseudo-JSON" syntax definitely shouldn't percolate to the QAPI documentation. Using normal QAPI structs means that the normal tool for documentation (qapi-schema.json doc comments) applies just as well to QAPI schema introspection
Agreed - I definitely want the output of the query command to be fully described by qapi. Which means we DO have to convert from the pseudo-JSON of the qapi file into the final formal qapi format. But the conversion is known at code generation time, so you should do it as part of your python code generator, and not repeat the conversion at runtime in the C code. That is, the C code should have everything already split out into the data structures it needs to just output the qapi structure as documented for the formal qapi definition.
Yes, that's exactly what I mean. If we use python to generate code for qobject_to_dataobj() or even generate object_to_$type() for each qapi type, the C code needed will be minimal anyway. Thanks, Fam
participants (4)
-
Amos Kong -
Eric Blake -
Fam Zheng -
Paolo Bonzini