2:17 a.m.
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(a)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.
2:50 a.m.
New subject: [libvirt] [Qemu-devel] [PATCH v4 4/5] qmp: full introspection support for QMP
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(a)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.
3:38 a.m.
New subject: [libvirt] [PATCH v4 4/5] qmp: full introspection support for QMP
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(a)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
4:07 a.m.
New subject: [libvirt] [PATCH v4 4/5] qmp: full introspection support for QMP
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(a)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.
4:15 a.m.
New subject: [libvirt] [PATCH v4 4/5] qmp: full introspection support for QMP
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
4:46 a.m.
New subject: [libvirt] [PATCH v4 4/5] qmp: full introspection support for QMP
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(a)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
4:45 a.m.
New subject: [libvirt] [PATCH v4 4/5] qmp: full introspection support for QMP
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(a)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
5:14 a.m.
New subject: [libvirt] [PATCH v4 4/5] qmp: full introspection support for QMP
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
7:58 a.m.
New subject: [libvirt] [Qemu-devel] [PATCH v4 4/5] qmp: full introspection support for QMP
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
2:12 a.m.
New subject: [libvirt] [Qemu-devel] [PATCH v4 4/5] qmp: full introspection support for QMP
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
3923
days inactive
3925
days old
9 comments
4 participants
participants (4)
-
Amos Kong -
Eric Blake -
Fam Zheng -
Paolo Bonzini