| Message ID | 1373971062-28909-3-git-send-email-akong@redhat.com |
|---|---|
| State | New |
| Headers | show |
Il 16/07/2013 12:37, Amos Kong ha scritto: > So here I defined a 'DataObject' type in qapi-schema.json, > it's used to describe the dynamical dictionary/list/string. > > { 'type': 'DataObject', > 'data': { '*key': 'str', '*type': 'str', '*data': ['DataObject'] } } This is missing '*optional': 'bool'. Also, how do you distinguish these: { 'command': 'query-tpm-types', 'returns': 'TpmType] } { 'command': 'query-tpm-types', 'returns': ['TpmType'] } Could it have to be like this? 'data': { '*key': 'str', '*type': 'str', '*list': 'bool', '*optional': 'bool', '*data': ['DataObject'] } } Can you document, in the commit message or the code, how you avoid infinite loops (possible with optional or list fields)? Paolo > Not all the keys in data will be used. > # List: type > # Dict: key, type > # nested List: type, data > # nested Dict: key, type, data > > The DataObject is described in docs/qmp-full-introspection.txt in > detail. > > The following content gives an example of query-tpm-types: > > ## Define example in qapi-schema.json: > > { 'enum': 'TpmType', 'data': [ 'passthrough' ] } > { 'command': 'query-tpm-types', 'returns': ['TpmType'] } > > ## Returned description: > > { > "name": "query-tpm-types", > "type": "Command", > "returns": [ > { > "type": "TpmType", > "data": [ > { > "type": "passthrough" > } > ] > } > ] > },
On Tue, Jul 16, 2013 at 12:48:36PM +0200, Paolo Bonzini wrote: > Il 16/07/2013 12:37, Amos Kong ha scritto: > > So here I defined a 'DataObject' type in qapi-schema.json, > > it's used to describe the dynamical dictionary/list/string. > > > > { 'type': 'DataObject', > > 'data': { '*key': 'str', '*type': 'str', '*data': ['DataObject'] } } Hi Paolo, > This is missing '*optional': 'bool'. Also, how do you distinguish these: > > { 'command': 'query-tpm-types', 'returns': 'TpmType] } do you mean 'TpmType' ? not 'TpmType] { "name": "query-tpm-types", "type": "Command", "returns": [ > { > "type": "passthrough" > } ] }, > { 'command': 'query-tpm-types', 'returns': ['TpmType'] } { "name": "query-tpm-types", "type": "Command", "returns": [ > { > "type": "TpmType", > "data": [ > { > "type": "passthrough" > } > ] > } ] }, > > Could it have to be like this? > > 'data': { '*key': 'str', '*type': 'str', '*list': 'bool', > '*optional': 'bool', > '*data': ['DataObject'] } } there are three conditions: 1) list 2) dict 3) string > Can you document, in the commit message or the code, I added a document for QMP introspection support. (docs/qmp-full-introspection.txt) The DataObject is described in docs/qmp-full-introspection.txt in detail. > how you avoid infinite loops (possible with optional or list fields)? +We have four types (ImageInfo, BlockStats, PciDeviceInfo, SchemaData) +that uses themself in their own define data directly or indirectly, +we will not repeatedly extend them to avoid dead loop. +/* + * Use a string to record the visit path, type index of each node + * will be saved to the string, indexes are split by ':'. + */ related functions: pop_id() push_id() The detail needs to be added to qmp-full-introspection.txt > Paolo > > > Not all the keys in data will be used. > > # List: type > > # Dict: key, type > > # nested List: type, data > > # nested Dict: key, type, data > > > > The DataObject is described in docs/qmp-full-introspection.txt in > > detail. > > > > The following content gives an example of query-tpm-types: > > > > ## Define example in qapi-schema.json: > > > > { 'enum': 'TpmType', 'data': [ 'passthrough' ] } > > { 'command': 'query-tpm-types', 'returns': ['TpmType'] } > > > > ## Returned description: > > > > { > > "name": "query-tpm-types", > > "type": "Command", > > "returns": [ > > { > > "type": "TpmType", > > "data": [ > > { > > "type": "passthrough" > > } > > ] > > } > > ] > > },
Il 16/07/2013 13:04, Amos Kong ha scritto: >>> > > So here I defined a 'DataObject' type in qapi-schema.json, >>> > > it's used to describe the dynamical dictionary/list/string. >>> > > >>> > > { 'type': 'DataObject', >>> > > 'data': { '*key': 'str', '*type': 'str', '*data': ['DataObject'] } } > > Hi Paolo, > >> > This is missing '*optional': 'bool'. Also, how do you distinguish these: >> > >> > { 'command': 'query-tpm-types', 'returns': 'TpmType] } > do you mean 'TpmType' ? not 'TpmType] Yes. > { > "name": "query-tpm-types", > "type": "Command", > "returns": [ > > { > > "type": "passthrough" > > } > ] > }, > >> > { 'command': 'query-tpm-types', 'returns': ['TpmType'] } > { > "name": "query-tpm-types", > "type": "Command", > "returns": [ > > { > > "type": "TpmType", > > "data": [ > > { > > "type": "passthrough" > > } > > ] > > } > ] > }, Thanks. I see this is unique, but it is also not too intuitive. So, could you add a "kind" field to DataObject that is an enum (list/dict/scalar, or something like that)? This would make it easier to parse (for humans at least, but I guess also for programs). Paolo >> > >> > Could it have to be like this? >> > >> > 'data': { '*key': 'str', '*type': 'str', '*list': 'bool', >> > '*optional': 'bool', >> > '*data': ['DataObject'] } } > there are three conditions: > 1) list > 2) dict > 3) string > >> > Can you document, in the commit message or the code, > > I added a document for QMP introspection support. > (docs/qmp-full-introspection.txt) > > The DataObject is described in docs/qmp-full-introspection.txt in > detail. > >> > how you avoid infinite loops (possible with optional or list fields)? > > +We have four types (ImageInfo, BlockStats, PciDeviceInfo, SchemaData) > +that uses themself in their own define data directly or indirectly, > +we will not repeatedly extend them to avoid dead loop. > > +/* > + * Use a string to record the visit path, type index of each node > + * will be saved to the string, indexes are split by ':'. > + */ > > related functions: > pop_id() > push_id() > > The detail needs to be added to qmp-full-introspection.txt >
On Tue, Jul 16, 2013 at 01:08:55PM +0200, Paolo Bonzini wrote: > Il 16/07/2013 13:04, Amos Kong ha scritto: > >>> > > So here I defined a 'DataObject' type in qapi-schema.json, > >>> > > it's used to describe the dynamical dictionary/list/string. > >>> > > > >>> > > { 'type': 'DataObject', > >>> > > 'data': { '*key': 'str', '*type': 'str', '*data': ['DataObject'] } } > > > > Hi Paolo, > > > >> > This is missing '*optional': 'bool'. Also, how do you distinguish these: > >> > > >> > { 'command': 'query-tpm-types', 'returns': 'TpmType] } > > do you mean 'TpmType' ? not 'TpmType] > > Yes. > > > { > > "name": "query-tpm-types", > > "type": "Command", > > "returns": [ > > > { > > > "type": "passthrough" > > > } > > ] > > }, > > > >> > { 'command': 'query-tpm-types', 'returns': ['TpmType'] } > > { > > "name": "query-tpm-types", > > "type": "Command", > > "returns": [ > > > { > > > "type": "TpmType", > > > "data": [ > > > { > > > "type": "passthrough" > > > } > > > ] > > > } > > ] > > }, > > Thanks. I see this is unique, but it is also not too intuitive. > > So, could you add a "kind" field to DataObject that is an enum > (list/dict/scalar, or something like that)? This would make it easier > to parse (for humans at least, but I guess also for programs). I thought we can identify the kind by some judgment. if the dict has key 'key', it's a dict if no 'key', have 'type', it's a list if only have 'type', it's a buildin type (or extended type that doesn't need to be extended) if no 'key', have 'type' & 'data', it's extended list type if have 'key', 'type', 'data', it's extended dict type I will added a 'kind' field to make it clearer. KIND enum: list dict str scalar(bool): Or just simplely check if have 'data' key? true/false Amos > Paolo
Il 16/07/2013 14:04, Amos Kong ha scritto: >> > Thanks. I see this is unique, but it is also not too intuitive. >> > >> > So, could you add a "kind" field to DataObject that is an enum >> > (list/dict/scalar, or something like that)? This would make it easier >> > to parse (for humans at least, but I guess also for programs). > I thought we can identify the kind by some judgment. Yes, I understood that. Strictly speaking the kind is redundant, but it seems to me that it makes the API easier to understand and use. > if the dict has key 'key', it's a dict > if no 'key', have 'type', it's a list > if only have 'type', it's a buildin type (or extended type that > doesn't need to be extended) > if no 'key', have 'type' & 'data', it's extended list type > if have 'key', 'type', 'data', it's extended dict type > > I will added a 'kind' field to make it clearer. > > KIND enum: > list > dict > str Why "str" and not "scalar" for a builtin type? It's not necessarily a string, is it? Paolo > scalar(bool): Or just simplely check if have 'data' key? > true/false
On Tue, 16 Jul 2013 18:37:42 +0800 Amos Kong <akong@redhat.com> wrote: > Introduces new monitor command to query QMP schema information, > the return data is a dynamical and nested dict/list, it contains > the useful metadata to help management to check feature support, > QMP commands detail, etc. > > I added a document for QMP introspection support. > (docs/qmp-full-introspection.txt) > > We need to parse all commands json definition, and generated a > dynamical tree, QMP infrastructure will convert the tree to > json string and return to QMP client. > > So here I defined a 'DataObject' type in qapi-schema.json, > it's used to describe the dynamical dictionary/list/string. I skimmed over the patch and made some comments, but my impression is that this is getting too complex... Why did we move from letting mngt query type by type (last version) to this version which returns all commands and their input types? Does this satisfy libvirt needs? > > { 'type': 'DataObject', > 'data': { '*key': 'str', '*type': 'str', '*data': ['DataObject'] } } > > Not all the keys in data will be used. > # List: type > # Dict: key, type > # nested List: type, data > # nested Dict: key, type, data > > The DataObject is described in docs/qmp-full-introspection.txt in > detail. > > The following content gives an example of query-tpm-types: > > ## Define example in qapi-schema.json: > > { 'enum': 'TpmType', 'data': [ 'passthrough' ] } > { 'command': 'query-tpm-types', 'returns': ['TpmType'] } > > ## Returned description: > > { > "name": "query-tpm-types", > "type": "Command", > "returns": [ > { > "type": "TpmType", > "data": [ > { > "type": "passthrough" > } > ] > } > ] > }, > > 'TpmType' is a defined type, it will be extended in returned > description. [ 'passthrough' ] is a list, so 'type' and 'data' > will be used. > > TODO: > We can add events definations to qapi-schema.json by another > patch, then event can also be queried. > > Introduce another command 'query-qga-schema' to query QGA schema > information, it's easy to add this support with current current > patch. > > Signed-off-by: Amos Kong <akong@redhat.com> > --- > docs/qmp-full-introspection.txt | 143 +++++++++++++++++++ > qapi-schema.json | 69 +++++++++ > qmp-commands.hx | 39 +++++ > qmp.c | 307 ++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 558 insertions(+) > create mode 100644 docs/qmp-full-introspection.txt > > diff --git a/docs/qmp-full-introspection.txt b/docs/qmp-full-introspection.txt > new file mode 100644 > index 0000000..cc0fb80 > --- /dev/null > +++ b/docs/qmp-full-introspection.txt > @@ -0,0 +1,143 @@ > += full introspection support for QMP = > + > +== Purpose == > + > +Add a new interface to provide QMP schema information to management, > +the return data is a dynamical and nested dict/list, it contains > +the useful metadata to help management to check feature support, > +QMP commands detail, etc. > + > +== Usage == > + > +Execute QMP command: > + > + { "execute": "query-qmp-schema" } > + > +Returns: > + > + { "return": [ > + { > + "name": "query-name", > + "type": "Command", > + "returns": [ > + { > + "key": "*name", > + "type": "str" > + } > + ] > + }, > + ... > + } > + > +The whole schema information will be returned in one go, it contains > +commands and event. It doesn't support to be filtered by type or name. > + > +We have four types (ImageInfo, BlockStats, PciDeviceInfo, SchemaData) > +that uses themself in their own define data directly or indirectly, > +we will not repeatedly extend them to avoid dead loop. > + > +== more description of 'DataObject' type == > + > +We use 'DataObject' to describe dynamical data struct, it might be > +nested dictionary, list or string. > + > +'DataObject' itself is a arbitrary and nested dictionary, the > +dictionary has three keys ('key', 'type', 'data'), 'key' and > +'data' are optional. > + > +* For describing Dictionary, we set the key to 'key', and set the > + value to 'type' > +* For describing List, we don't set 'key', just set the value to > + 'type' > +* If the value of dictionary or list is non-native type, we extend > + the non-native type to dictionary, set it to 'data', and set the > + non-native type's name to 'type'. > +* If the value of dictionary or list is dictionary or list, 'type' > + won't be set. > + > +== examples == > + > +1) Dict, value is native type > +{ 'id': 'str', ... } > +-------------------- > +[ > + { > + "key": "id", > + "type": "str" > + }, > + ..... > +] > + > +2) Dict, value is defined types > +{ 'options': 'TpmTypeOptions' } > +------------------------------- > +[ > + { > + "key": "options", > + "type": "TpmTypeOptions", > + "data": [ > + { > + "key": "passthrough", > + "type": "str", > + } > + ] > + }, > + ..... > +] > + > +3) List, value is native type > +['str', ... ] > +------------- > +[ > + { > + "type": "str" > + }, > + .... > +] > + > +4) List, value is defined types > +['TpmTypeOptions', ... ] > +------------------------ > +[ > + { > + "type": "TpmTypeOptions", > + "data": [ > + { > + "key": "passthrough", > + "type": "str", > + } > + ] > + }, > + ..... > +] > + > +5) Dict, value is dictionary > +{ 'info': { 'age': 'init', ... } } > +----------------------------- > +[ > + { > + "key": "info", > + "data": [ > + { > + "key": "age", > + "type": "init", > + }, > + ... > + ] > + }, > +] > + > +6) Dict, value is list > +{ 'info': [ 'str', ... } } > +----------------------------- > +[ > + { > + "key": "info", > + "data": [ > + { > + "type": "str", > + }, > + ... > + ] > + }, > +] > diff --git a/qapi-schema.json b/qapi-schema.json > index 7b9fef1..cf03391 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -3679,3 +3679,72 @@ > '*cpuid-input-ecx': 'int', > 'cpuid-register': 'X86CPURegister32', > 'features': 'int' } } > + > +## > +# @DataObject > +# > +# Details of a data object, it can be nested dictionary/list > +# > +# @key: #optional Data object key > +# > +# @type: Data object type name > +# > +# @optional: #optional whether the data object is optional > +# > +# @data: #optional DataObject list, can be a dictionary or list type > +# > +# Since: 1.6 > +## > +{ 'type': 'DataObject', > + 'data': { '*key': 'str', '*type': 'str', '*optional': 'bool', '*data': ['DataObject'] } } > + > +## > +# @SchemaMetaType > +# > +# Possible meta types of a schema entry > +# > +# @Command: QMP command > +# > +# @Type: defined new data type > +# > +# @Enumeration: enumeration data type > +# > +# @Union: union data type > +# > +# Since: 1.6 > +## > +{ 'enum': 'SchemaMetaType', > + 'data': ['Command', 'Type', 'Enumeration', 'Union'] } > + > +## > +# @SchemaEntry > +# > +# Details of schema items > +# > +# @type: Entry's type in string format It's not a string, it's a SchemaMetaType. > +# > +# @name: Entry name > +# > +# @data: #optional list of DataObject. This can have different meaning > +# depending on the 'type' value. For example, for a QMP command, > +# this member contains an argument listing. For an enumeration, > +# it contains the enum's values and so on > +# > +# @returns: #optional list of DataObject, return data after executing > +# QMP command > +# > +# Since: 1.6 > +## > +{ 'type': 'SchemaEntry', 'data': { 'type': 'SchemaMetaType', > + 'name': 'str', '*data': ['DataObject'], '*returns': ['DataObject'] } } > + > +## > +# @query-qmp-schema > +# > +# Query QMP schema information > +# > +# Returns: list of @SchemaEntry. Returns an error if json string is invalid. > +# > +# Since: 1.6 > +## > +{ 'command': 'query-qmp-schema', 'returns': ['SchemaEntry'] } > diff --git a/qmp-commands.hx b/qmp-commands.hx > index e075df4..e3cbe93 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -3047,3 +3047,42 @@ Example: > <- { "return": {} } > > 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', 'event' > +- "data": schema data (json-object, optional) > +- "returns": return data of qmp command (json-object, optional) > + > +Example: > + > +-> { "execute": "query-qmp-schema" } > +<- { "return": [ > + { > + "name": "query-name", > + "type": "Command", > + "returns": [ > + { > + "key": "*name", > + "type": "str" > + } > + ] > + } > + ] > + } > + > +EQMP > diff --git a/qmp.c b/qmp.c > index 4c149b3..3ace3a6 100644 > --- a/qmp.c > +++ b/qmp.c > @@ -25,6 +25,8 @@ > #include "sysemu/blockdev.h" > #include "qom/qom-qobject.h" > #include "hw/boards.h" > +#include "qmp-schema.h" > +#include "qapi/qmp/qjson.h" > > NameInfo *qmp_query_name(Error **errp) > { > @@ -486,6 +488,311 @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp) > return arch_query_cpu_definitions(errp); > } > > +/* > + * Use a string to record the visit path, type index of each node > + * will be saved to the string, indexes are split by ':'. > + */ > +static char visit_path_str[1024]; visit path? I don't get this. > + > +/* push the type index to visit_path_str */ > +static void push_id(int id) > +{ > + char *end = strrchr(visit_path_str, ':'); > + char type_idx[256]; > + int num; > + > + num = sprintf(type_idx, "%d:", id); > + > + if (end) { > + /* avoid overflow */ > + assert(end - visit_path_str + 1 + num < sizeof(visit_path_str)); > + sprintf(end + 1, "%d:", id); > + } else { > + sprintf(visit_path_str, "%d:", id); > + } > +} > + > +/* pop the type index from visit_path_str */ > +static void pop_id(void) > +{ > + char *p = strrchr(visit_path_str, ':'); > + > + assert(p != NULL); > + *p = '\0'; > + p = strrchr(visit_path_str, ':'); > + if (p) { > + *(p + 1) = '\0'; > + } else { > + visit_path_str[0] = '\0'; > + } > +} > + > +static const char *qstring_copy_str(QObject *data) > +{ > + QString *qstr; > + > + if (!data) { > + return NULL; > + } > + qstr = qobject_to_qstring(data); > + if (qstr) { > + return qstring_get_str(qstr); > + } else { > + return NULL; > + } > +} > + > +static DataObjectList *visit_qobj_dict(QObject *data); > +static DataObjectList *visit_qobj_list(QObject *data); > + > +/* extend defined type to json object */ > +static DataObjectList *extend_type(const char* str) > +{ > + DataObjectList *data_list; > + QObject *data; > + QDict *qdict; > + const QDictEntry *ent; > + int i; > + > + /* don't extend builtin types */ > + if (!strcmp(str, "str") || !strcmp(str, "int") || > + !strcmp(str, "number") || !strcmp(str, "bool") || > + !strcmp(str, "int8") || !strcmp(str, "int16") || > + !strcmp(str, "int32") || !strcmp(str, "int64") || > + !strcmp(str, "uint8") || !strcmp(str, "uint16") || > + !strcmp(str, "uint32") || !strcmp(str, "uint64")) { > + return NULL; > + } > + > + for (i = 0; qmp_schema_table[i]; i++) { > + data = qobject_from_json(qmp_schema_table[i]); > + assert(data != NULL); > + > + qdict = qobject_to_qdict(data); > + assert(qdict != NULL); > + > + ent = qdict_first(qdict); > + if (!qdict_get(qdict, "enum") && !qdict_get(qdict, "type") > + && !qdict_get(qdict, "union")) { > + continue; > + } > + > + if (!strcmp(str, qstring_copy_str(ent->value))) { > + char *start, *end; > + char cur_idx[256]; > + char type_idx[256]; > + > + start = visit_path_str; > + sprintf(type_idx, "%d", i); > + while(start) { > + end = strchr(start, ':'); > + if (!end) { > + break; > + } > + snprintf(cur_idx, end - start + 1, "%s", start); > + start = end + 1; > + /* if the type was already extended in parent node, > + * we don't extend it again to avoid dead loop. */ > + if (!strcmp(cur_idx, type_idx)) { > + return NULL; > + } > + } > + /* push index to visit_path_str before extending */ > + push_id(i); > + > + data = qdict_get(qdict, "data"); > + if(data) { > + if (data->type->code == QTYPE_QDICT) { > + data_list = visit_qobj_dict(data); > + } else if (data->type->code == QTYPE_QLIST) { > + data_list = visit_qobj_list(data); > + } > + /* pop index from visit_path_str after extending */ > + pop_id(); > + > + return data_list; > + } > + } > + } > + > + return NULL; > +} > + > +static DataObjectList *visit_qobj_list(QObject *data) > +{ > + DataObjectList *obj_list, *obj_last_entry, *obj_entry; > + DataObject *obj_info; > + const QListEntry *ent; > + QList *qlist; > + > + qlist = qobject_to_qlist(data); > + assert(qlist != NULL); > + > + obj_list = NULL; > + for (ent = qlist_first(qlist); ent; ent = qlist_next(ent)) { > + obj_info = g_malloc0(sizeof(*obj_info)); > + obj_entry = g_malloc0(sizeof(*obj_entry)); > + obj_entry->value = obj_info; > + obj_entry->next = NULL; > + > + if (!obj_list) { > + obj_list = obj_entry; > + } else { > + obj_last_entry->next = obj_entry; > + } > + obj_last_entry = obj_entry; > + > + if (ent->value->type->code == QTYPE_QDICT) { > + obj_info->data = visit_qobj_dict(ent->value); > + } else if (ent->value->type->code == QTYPE_QLIST) { > + obj_info->data = visit_qobj_list(ent->value); > + } else if (ent->value->type->code == QTYPE_QSTRING) { > + obj_info->has_type = true; > + obj_info->type = g_strdup(qstring_copy_str(ent->value)); > + obj_info->data = extend_type(qstring_copy_str(ent->value)); > + } > + if (obj_info->data) { > + obj_info->has_data = true; > + } > + } > + > + return obj_list; > +} > + > +static DataObjectList *visit_qobj_dict(QObject *data) > +{ > + DataObjectList *obj_list, *obj_last_entry, *obj_entry; > + DataObject *obj_info; > + const QDictEntry *ent; > + QDict *qdict; > + > + qdict = qobject_to_qdict(data); > + assert(qdict != NULL); > + > + obj_list = NULL; > + for (ent = qdict_first(qdict); ent; ent = qdict_next(qdict, ent)) { > + obj_info = g_malloc0(sizeof(*obj_info)); > + obj_entry = g_malloc0(sizeof(*obj_entry)); > + obj_entry->value = obj_info; > + obj_entry->next = NULL; > + > + if (!obj_list) { > + obj_list = obj_entry; > + } else { > + obj_last_entry->next = obj_entry; > + } > + obj_last_entry = obj_entry; > + > + if (ent->key[0] == '*') { > + obj_info->key = g_strdup(ent->key + 1); > + obj_info->has_optional = true; > + obj_info->optional = true; > + } else { > + obj_info->key = g_strdup(ent->key); > + } > + obj_info->has_key = true; > + > + if (ent->value->type->code == QTYPE_QDICT) { > + obj_info->data = visit_qobj_dict(ent->value); > + } else if (ent->value->type->code == QTYPE_QLIST) { > + obj_info->data = visit_qobj_list(ent->value); > + } else if (ent->value->type->code == QTYPE_QSTRING) { > + obj_info->has_type = true; > + obj_info->type = g_strdup(qstring_copy_str(ent->value)); > + obj_info->data = extend_type(qstring_copy_str(ent->value)); > + } > + if (obj_info->data) { > + obj_info->has_data = true; > + } > + } > + > + return obj_list; > +} > + > +SchemaEntryList *qmp_query_qmp_schema(Error **errp) > +{ > + SchemaEntryList *list, *last_entry, *entry; > + SchemaEntry *info; > + DataObjectList *obj_entry; > + DataObject *obj_info; > + QObject *data; > + QDict *qdict; > + int i; > + > + list = NULL; > + for (i = 0; qmp_schema_table[i]; i++) { > + data = qobject_from_json(qmp_schema_table[i]); > + assert(data != NULL); > + > + qdict = qobject_to_qdict(data); > + assert(qdict != NULL); > + > + if (qdict_get(qdict, "command")) { > + info = g_malloc0(sizeof(*info)); > + info->type = SCHEMA_META_TYPE_COMMAND; > + info->name = strdup(qdict_get_str(qdict, "command")); s/strdup/g_strdup > + } else { > + continue; > + } You return only commands. That is, types are returned as part of the command input. ErrorClass can't be queried then? I'm not judging, only observing. Eric, this is good enough for libvirt? Btw, you're leaking data on the else clause. > + > + memset(visit_path_str, 0, sizeof(visit_path_str)); > + data = qdict_get(qdict, "data"); > + if (data) { > + info->has_data = true; > + if (data->type->code == QTYPE_QLIST) { > + info->data = visit_qobj_list(data); > + } else if (data->type->code == QTYPE_QDICT) { > + info->data = visit_qobj_dict(data); > + } else if (data->type->code == QTYPE_QSTRING) { > + info->data = extend_type(qstring_get_str(qobject_to_qstring(data))); > + if (!info->data) { > + obj_info = g_malloc0(sizeof(*obj_info)); > + obj_entry = g_malloc0(sizeof(*obj_entry)); > + obj_entry->value = obj_info; > + obj_info->has_type = true; > + obj_info->type = g_strdup(qdict_get_str(qdict, "data")); > + info->data = obj_entry; > + } > + } else { > + abort(); Pleae, explain why you're aborting here. > + } > + } > + > + memset(visit_path_str, 0, sizeof(visit_path_str)); > + data = qdict_get(qdict, "returns"); > + if (data) { > + info->has_returns = true; > + if (data->type->code == QTYPE_QLIST) { > + info->returns = visit_qobj_list(data); > + } else if (data->type->code == QTYPE_QDICT) { > + info->returns = visit_qobj_dict(data); > + } else if (data->type->code == QTYPE_QSTRING) { > + info->returns = extend_type(qstring_copy_str(data)); > + if (!info->returns) { > + obj_info = g_malloc0(sizeof(*obj_info)); > + obj_entry = g_malloc0(sizeof(*obj_entry)); > + obj_entry->value = obj_info; > + obj_info->has_type = true; > + obj_info->type = g_strdup(qdict_get_str(qdict, "returns")); > + info->returns = obj_entry; > + } > + } > + } > + > + entry = g_malloc0(sizeof(DataObjectList *)); > + entry->value = info; > + entry->next = NULL; > + if (!list) { > + list = entry; > + } else { > + last_entry->next = entry; > + } > + last_entry = entry; > + } > + > + return list; > +} > + > void qmp_add_client(const char *protocol, const char *fdname, > bool has_skipauth, bool skipauth, bool has_tls, bool tls, > Error **errp)
On 07/17/2013 02:36 PM, Luiz Capitulino wrote: >> We need to parse all commands json definition, and generated a >> dynamical tree, QMP infrastructure will convert the tree to >> json string and return to QMP client. >> >> So here I defined a 'DataObject' type in qapi-schema.json, >> it's used to describe the dynamical dictionary/list/string. > > I skimmed over the patch and made some comments, but my impression > is that this is getting too complex... Why did we move from letting > mngt query type by type (last version) to this version which returns > all commands and their input types? Does this satisfy libvirt needs? Libvirt can query once, and then browse through the (giant) reply as many times as needed to drill down to a full definition. The ability to filter by passing in a name to look up is a bonus, but not mandatory. I'm also working on a reply to the full patch. > You return only commands. That is, types are returned as part of the > command input. ErrorClass can't be queried then? I'm not judging, only > observing. > > Eric, this is good enough for libvirt? It might be sufficient (after all, you can't use a type except by passing it as part of a command [argument or return] or event [which we still don't have converted into qapi...]). In one respect, it means that if we want to know if an optional field was added for command 'foo', we start by querying command foo; then regardless of what type names were used, we will see that in the results for the command. On the other hand, we've been arguing that qapi type names are part of the API, and that we shouldn't arbitrarily change type names (particularly not once introspection is in place). Thus, if I can query for the contents of type 'FooDict', that shaves a step from querying the structure of command 'foo' that uses the type 'FooDict'. In other words, it will "work" for libvirt, but it may not be optimal.
On 07/16/2013 04:37 AM, Amos Kong wrote: > Introduces new monitor command to query QMP schema information, > the return data is a dynamical and nested dict/list, it contains s/dynamical/dynamic/ > the useful metadata to help management to check feature support, > QMP commands detail, etc. > > I added a document for QMP introspection support. > (docs/qmp-full-introspection.txt) Yay - docs make a world of difference. > > We need to parse all commands json definition, and generated a mixing tense ("need" present, "generated" past); for commit messages, present tense is generally appropriate. Thus: s/generated/generate/ > dynamical tree, QMP infrastructure will convert the tree to s/dynamical/dynamic/ > json string and return to QMP client. > > So here I defined a 'DataObject' type in qapi-schema.json, > it's used to describe the dynamical dictionary/list/string. s/dynamical/dynamic/ > > { 'type': 'DataObject', > 'data': { '*key': 'str', '*type': 'str', '*data': ['DataObject'] } } > > Not all the keys in data will be used. > # List: type > # Dict: key, type > # nested List: type, data > # nested Dict: key, type, data I wonder if we can take advantage of Kevin's work on unions to make this MUCH easier to use: https://lists.gnu.org/archive/html/qemu-devel/2013-07/msg01407.html { 'enum': 'DataObjectType', 'data': [ 'command', 'struct', 'union', 'enum' ] } # extend to add 'event' later { 'type': 'DataObjectBase', 'data': { 'name': 'str' } } { 'union': 'DataObjectMemberType', 'discriminator': {}, 'data': { 'reference': 'str', 'inline': 'DataObject' } } { 'type': DataObjectMember', 'data': { 'name': 'str', 'type': 'DataObjectMemberType', '*optional': 'bool', '*recursive': 'bool' } } { 'type': 'DataObjectStruct', 'data': { 'data': [ 'DataObjectMember' ] } } { 'type': 'DataObjectEnum', 'data': { 'data': [ 'str' ] } } { 'type': 'DataObjectUnion', 'data': { 'data': [ 'DataObjectMember' ], '*base': 'str', '*discriminator': 'str' } } { 'type': 'DataObjectCommand', 'data': { 'data': [ 'DataObjectMember' ], '*returns': 'DataObject' } } { 'union': 'DataObject', 'base': 'DataObjectBase', 'discriminator': 'type', 'data': { 'struct': 'DataObjectStruct', 'enum': 'DataObjectEnum', 'union': 'DataObjectUnion', 'command': 'DataObjectCommand', 'array': {} } > > The DataObject is described in docs/qmp-full-introspection.txt in > detail. > > The following content gives an example of query-tpm-types: > > ## Define example in qapi-schema.json: > > { 'enum': 'TpmType', 'data': [ 'passthrough' ] } > { 'command': 'query-tpm-types', 'returns': ['TpmType'] } It might be more useful to have a (made-up) example that shows as many details as possible - a command that takes arguments and has a return type will exercise more of the code paths than a query command with only a return. > > ## Returned description: > > { > "name": "query-tpm-types", > "type": "Command", > "returns": [ > { > "type": "TpmType", > "data": [ > { > "type": "passthrough" > } > ] I need a way to know the difference between a TpmType returned directly in a dict, vs. a return containing an array of TpmType. > } > ] > }, Thus, using the discriminated union I set out above, I would return something like: { "name": "TpmType", "type": "enum", "data": [ "passthrough" ] }, { "name": "query-tpm-types", "type": "command", "data": [ ], "returns": { "name": "TpmType", "type": "array" } } > > 'TpmType' is a defined type, it will be extended in returned > description. [ 'passthrough' ] is a list, so 'type' and 'data' > will be used. > > TODO: > We can add events definations to qapi-schema.json by another s/definations/definitions/ > patch, then event can also be queried. > > Introduce another command 'query-qga-schema' to query QGA schema > information, it's easy to add this support with current current > patch. Such a command would be part of the QGA interface, not a QMP command. But yes, it is needed, and the ideal patch series from you will include that patch as part of the series. But I don't mind waiting until we get the interface nailed down before you actually implement the QGA repeat of the interface. > > Signed-off-by: Amos Kong <akong@redhat.com> > --- > docs/qmp-full-introspection.txt | 143 +++++++++++++++++++ > qapi-schema.json | 69 +++++++++ > qmp-commands.hx | 39 +++++ > qmp.c | 307 ++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 558 insertions(+) > create mode 100644 docs/qmp-full-introspection.txt > > diff --git a/docs/qmp-full-introspection.txt b/docs/qmp-full-introspection.txt > new file mode 100644 > index 0000000..cc0fb80 > --- /dev/null > +++ b/docs/qmp-full-introspection.txt > @@ -0,0 +1,143 @@ > += full introspection support for QMP = > + Is it worth merging this into the existing qapi-code-gen.txt, or at least having the two documents refer to one another, since they deal with related concepts (turning the .json file into generated code)? > +== Purpose == > + > +Add a new interface to provide QMP schema information to management, s/a new/an/ - after a year, the interface won't be new, but this doc will still be relevant. > +the return data is a dynamical and nested dict/list, it contains s/dynamical/dynamic/ > +the useful metadata to help management to check feature support, > +QMP commands detail, etc. > + > +== Usage == > + > +Execute QMP command: > + > + { "execute": "query-qmp-schema" } > + > +Returns: > + > + { "return": [ > + { > + "name": "query-name", > + "type": "Command", > + "returns": [ > + { > + "key": "*name", > + "type": "str" > + } > + ] Are we trying to use struct names where they exist for compactness, or trying to inline-expand everything as far as possible until we run into self-referential recursion? > + }, > + ... > + } > + > +The whole schema information will be returned in one go, it contains > +commands and event. It doesn't support to be filtered by type or name. s/event/events/ s/It/At present, it/ s/to be filtered/filtering/ > + > +We have four types (ImageInfo, BlockStats, PciDeviceInfo, SchemaData) This list will get out of date quickly. I'd just word it generically: We have several self-referential types > +that uses themself in their own define data directly or indirectly, s/uses themself/use themselves/ > +we will not repeatedly extend them to avoid dead loop. Would it be worth a flag in the QMP output when a type is not being further expanded because it is a nested occurrence of itself? That is, given my proposed layout above, ImageInfo would turn into something like: { "name": "ImageInfo", "type": "struct", "data": [ { "name": "filename", "type", "str" }, ... { "name": "backing-image", "type": "ImageInfo", "optional": true, "recursive": true } ] }, > + > +== more description of 'DataObject' type == > + > +We use 'DataObject' to describe dynamical data struct, it might be s/dynamical/dynamic/ > +nested dictionary, list or string. > + > +'DataObject' itself is a arbitrary and nested dictionary, the > +dictionary has three keys ('key', 'type', 'data'), 'key' and spacing is odd. > +'data' are optional. > + > +* For describing Dictionary, we set the key to 'key', and set the > + value to 'type' > +* For describing List, we don't set 'key', just set the value to > + 'type' Again, if you use the idea of a discriminated union, you don't need quite the complexity in describing this: dictionaries are listed with key/type/optional tuples, lists (enums) are listed with just an array of strings, and the QAPI perfectly described that difference by the discriminator telling you 'struct' vs. 'enum'. > +* If the value of dictionary or list is non-native type, we extend > + the non-native type to dictionary, set it to 'data', and set the > + non-native type's name to 'type'. Again, I tried to set up the QAPI that describes something that uses an anonymous union that either gives a string (the name of a primitive or already-defined type) or a struct (a recursion into another layer of struct describing the structure of that type in line). > diff --git a/qapi-schema.json b/qapi-schema.json > index 7b9fef1..cf03391 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -3679,3 +3679,72 @@ > '*cpuid-input-ecx': 'int', > 'cpuid-register': 'X86CPURegister32', > 'features': 'int' } } > + > +## > +# @DataObject > +# > +# Details of a data object, it can be nested dictionary/list > +# > +# @key: #optional Data object key > +# > +# @type: Data object type name > +# > +# @optional: #optional whether the data object is optional mention that the default is false. > +# > +# @data: #optional DataObject list, can be a dictionary or list type so if 'data' is present, we are describing @type in more detail; if it is absent, then @type is primitive. > +# > +# Since: 1.6 > +## > +{ 'type': 'DataObject', > + 'data': { '*key': 'str', '*type': 'str', '*optional': 'bool', '*data': ['DataObject'] } } > + '*type' should be an enum type, not open-coded string. > +## > +# @SchemaMetaType > +# > +# Possible meta types of a schema entry > +# > +# @Command: QMP command > +# > +# @Type: defined new data type > +# > +# @Enumeration: enumeration data type > +# > +# @Union: union data type > +# > +# Since: 1.6 > +## > +{ 'enum': 'SchemaMetaType', > + 'data': ['Command', 'Type', 'Enumeration', 'Union'] } Do we need to start these with a capital? On the other hand, having them as capitals may make it easier to ensure we are outputting correct information. > + > +## > +# @SchemaEntry > +# > +# Details of schema items > +# > +# @type: Entry's type in string format > +# > +# @name: Entry name > +# > +# @data: #optional list of DataObject. This can have different meaning > +# depending on the 'type' value. For example, for a QMP command, > +# this member contains an argument listing. For an enumeration, > +# it contains the enum's values and so on This argues for making it a union type. > +# > +# @returns: #optional list of DataObject, return data after executing > +# QMP command > +# > +# Since: 1.6 > +## > +{ 'type': 'SchemaEntry', 'data': { 'type': 'SchemaMetaType', > + 'name': 'str', '*data': ['DataObject'], '*returns': ['DataObject'] } } > + > +## > +# @query-qmp-schema > +# > +# Query QMP schema information > +# > +# Returns: list of @SchemaEntry. Returns an error if json string is invalid. If you don't take any arguments, then the "returns an error" statement is impossible. > +# > +# Since: 1.6 > +## > +{ 'command': 'query-qmp-schema', 'returns': ['SchemaEntry'] } > diff --git a/qmp-commands.hx b/qmp-commands.hx > index e075df4..e3cbe93 100644 > --- a/qmp-commands.hx > > +/* > + * Use a string to record the visit path, type index of each node > + * will be saved to the string, indexes are split by ':'. > + */ > +static char visit_path_str[1024]; Is a fixed width buffer really the best solution, or does glib offer us something better? For example, a hash table, or even a growable string. > + > + for (i = 0; qmp_schema_table[i]; i++) { > + data = qobject_from_json(qmp_schema_table[i]); > + assert(data != NULL); > + > + qdict = qobject_to_qdict(data); > + assert(qdict != NULL); > + > + ent = qdict_first(qdict); > + if (!qdict_get(qdict, "enum") && !qdict_get(qdict, "type") > + && !qdict_get(qdict, "union")) { > + continue; > + } Why are we doing the work in C code, every time the command is called? Wouldn't it be more efficient to do the work in python code, at the time the qmp_schema_table C code is first generated, so that the generated code is already in the right format? > +SchemaEntryList *qmp_query_qmp_schema(Error **errp) > +{ > + SchemaEntryList *list, *last_entry, *entry; > + SchemaEntry *info; > + DataObjectList *obj_entry; > + DataObject *obj_info; > + QObject *data; > + QDict *qdict; > + int i; > + > + list = NULL; > + for (i = 0; qmp_schema_table[i]; i++) { > + data = qobject_from_json(qmp_schema_table[i]); > + assert(data != NULL); > + > + qdict = qobject_to_qdict(data); > + assert(qdict != NULL); > + > + if (qdict_get(qdict, "command")) { > + info = g_malloc0(sizeof(*info)); > + info->type = SCHEMA_META_TYPE_COMMAND; > + info->name = strdup(qdict_get_str(qdict, "command")); > + } else { > + continue; > + } > + Don't we want to also output types (struct, union, enum) and eventually events, not just commands? I hope we're converging, but I'm starting to worry that we won't have a design in place in time for the 1.6 release.
On Tue, Jul 16, 2013 at 02:18:37PM +0200, Paolo Bonzini wrote: > Il 16/07/2013 14:04, Amos Kong ha scritto: > >> > Thanks. I see this is unique, but it is also not too intuitive. > >> > > >> > So, could you add a "kind" field to DataObject that is an enum > >> > (list/dict/scalar, or something like that)? This would make it easier > >> > to parse (for humans at least, but I guess also for programs). > > I thought we can identify the kind by some judgment. > > Yes, I understood that. Strictly speaking the kind is redundant, but it > seems to me that it makes the API easier to understand and use. > > > if the dict has key 'key', it's a dict > > if no 'key', have 'type', it's a list > > if only have 'type', it's a buildin type (or extended type that > > doesn't need to be extended) > > if no 'key', have 'type' & 'data', it's extended list type > > if have 'key', 'type', 'data', it's extended dict type > > > > I will added a 'kind' field to make it clearer. > > > > KIND enum: > > list > > dict > > str > > Why "str" and not "scalar" for a builtin type? It's not necessarily a > string, is it? right, 'scalar' is better. > Paolo > > > scalar(bool): Or just simplely check if have 'data' key? > > true/false
On Wed, Jul 17, 2013 at 04:36:06PM -0400, Luiz Capitulino wrote: > On Tue, 16 Jul 2013 18:37:42 +0800 > Amos Kong <akong@redhat.com> wrote: > > > Introduces new monitor command to query QMP schema information, > > the return data is a dynamical and nested dict/list, it contains > > the useful metadata to help management to check feature support, > > QMP commands detail, etc. > > > > I added a document for QMP introspection support. > > (docs/qmp-full-introspection.txt) > > > > We need to parse all commands json definition, and generated a > > dynamical tree, QMP infrastructure will convert the tree to > > json string and return to QMP client. > > > > So here I defined a 'DataObject' type in qapi-schema.json, > > it's used to describe the dynamical dictionary/list/string. > > I skimmed over the patch and made some comments, but my impression > is that this is getting too complex... Why did we move from letting > mngt query type by type (last version) to this version which returns > all commands and their input types? Does this satisfy libvirt needs? > > diff --git a/qmp.c b/qmp.c > > index 4c149b3..3ace3a6 100644 > > --- a/qmp.c > > +++ b/qmp.c > > @@ -25,6 +25,8 @@ > > #include "sysemu/blockdev.h" > > #include "qom/qom-qobject.h" > > #include "hw/boards.h" > > +#include "qmp-schema.h" > > +#include "qapi/qmp/qjson.h" > > > > NameInfo *qmp_query_name(Error **errp) > > { > > @@ -486,6 +488,311 @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp) > > return arch_query_cpu_definitions(errp); > > } > > > > +/* > > + * Use a string to record the visit path, type index of each node > > + * will be saved to the string, indexes are split by ':'. > > + */ > > +static char visit_path_str[1024]; > > visit path? I don't get this. The return data is a dynamic tree. When we found defined type, we need to extend it. But some defined type is used in its child's node. So I use a visit_path_str to record the type index in one node. We don't extend one type, if it's already been extened in parent's node. {'type': 'a', 'data': ['a', 'b', 'c']} {'command': 'cmd', 'data': 'a'} 'a' is a defined type, we will extended it. But we will not extend 'a' in data list of type 'a'. I will add the explain to the doc. > > + > > +/* push the type index to visit_path_str */ > > +static void push_id(int id) > > +{ > > + char *end = strrchr(visit_path_str, ':'); > > + char type_idx[256]; > > + int num; > > + > > + num = sprintf(type_idx, "%d:", id); > > + > > + if (end) { > > + /* avoid overflow */ > > + assert(end - visit_path_str + 1 + num < sizeof(visit_path_str)); > > + sprintf(end + 1, "%d:", id); > > + } else { > > + sprintf(visit_path_str, "%d:", id); > > + } > > +} > > + > > +/* pop the type index from visit_path_str */ > > +static void pop_id(void) > > +{ > > + char *p = strrchr(visit_path_str, ':'); > > + > > + assert(p != NULL); > > + *p = '\0'; > > + p = strrchr(visit_path_str, ':'); > > + if (p) { > > + *(p + 1) = '\0'; > > + } else { > > + visit_path_str[0] = '\0'; > > + } > > +} > > +SchemaEntryList *qmp_query_qmp_schema(Error **errp) > > +{ > > + SchemaEntryList *list, *last_entry, *entry; > > + SchemaEntry *info; > > + DataObjectList *obj_entry; > > + DataObject *obj_info; > > + QObject *data; > > + QDict *qdict; > > + int i; > > + > > + list = NULL; > > + for (i = 0; qmp_schema_table[i]; i++) { > > + data = qobject_from_json(qmp_schema_table[i]); > > + assert(data != NULL); > > + > > + qdict = qobject_to_qdict(data); > > + assert(qdict != NULL); > > + > > + if (qdict_get(qdict, "command")) { > > + info = g_malloc0(sizeof(*info)); > > + info->type = SCHEMA_META_TYPE_COMMAND; > > + info->name = strdup(qdict_get_str(qdict, "command")); > > s/strdup/g_strdup > > > + } else { > > + continue; > > + } > > You return only commands. That is, types are returned as part of the > command input. Right. > ErrorClass can't be queried then? I'm not judging, only > observing. We can return the 'type', 'enum', 'union', 'etc' if libvirt needs them. > Eric, this is good enough for libvirt? > > Btw, you're leaking data on the else clause. > > > + > > + memset(visit_path_str, 0, sizeof(visit_path_str)); > > + data = qdict_get(qdict, "data"); > > + if (data) { > > + info->has_data = true; > > + if (data->type->code == QTYPE_QLIST) { > > + info->data = visit_qobj_list(data); > > + } else if (data->type->code == QTYPE_QDICT) { > > + info->data = visit_qobj_dict(data); > > + } else if (data->type->code == QTYPE_QSTRING) { > > + info->data = extend_type(qstring_get_str(qobject_to_qstring(data))); > > + if (!info->data) { > > + obj_info = g_malloc0(sizeof(*obj_info)); > > + obj_entry = g_malloc0(sizeof(*obj_entry)); > > + obj_entry->value = obj_info; > > + obj_info->has_type = true; > > + obj_info->type = g_strdup(qdict_get_str(qdict, "data")); > > + info->data = obj_entry; > > + } > > + } else { > > + abort(); > > Pleae, explain why you're aborting here. { 'command': 'query-qmp-schema', 'data': XXXX } the type of XXXX can only be list, dictionary or buildin-type. XXXX is the value in define dictionary. > > + } > > + }
On Fri, Jul 19, 2013 at 04:05:16PM -0600, Eric Blake wrote: > On 07/16/2013 04:37 AM, Amos Kong wrote: > > Introduces new monitor command to query QMP schema information, > > the return data is a dynamical and nested dict/list, it contains > > s/dynamical/dynamic/ > > > the useful metadata to help management to check feature support, > > QMP commands detail, etc. > > > > I added a document for QMP introspection support. > > (docs/qmp-full-introspection.txt) > > Yay - docs make a world of difference. > > > > > We need to parse all commands json definition, and generated a > > mixing tense ("need" present, "generated" past); for commit messages, > present tense is generally appropriate. Thus: > > s/generated/generate/ > > > dynamical tree, QMP infrastructure will convert the tree to > > s/dynamical/dynamic/ > > > json string and return to QMP client. > > > > So here I defined a 'DataObject' type in qapi-schema.json, > > it's used to describe the dynamical dictionary/list/string. > > s/dynamical/dynamic/ > > > > > { 'type': 'DataObject', > > 'data': { '*key': 'str', '*type': 'str', '*data': ['DataObject'] } } > > > > Not all the keys in data will be used. > > # List: type > > # Dict: key, type > > # nested List: type, data > > # nested Dict: key, type, data > > I wonder if we can take advantage of Kevin's work on unions to make this > MUCH easier to use: > > https://lists.gnu.org/archive/html/qemu-devel/2013-07/msg01407.html > > { 'enum': 'DataObjectType', > 'data': [ 'command', 'struct', 'union', 'enum' ] } It's necessary!! > # extend to add 'event' later > > { 'type': 'DataObjectBase', > 'data': { 'name': 'str' } } > > { 'union': 'DataObjectMemberType', > 'discriminator': {}, > 'data': { 'reference': 'str', > 'inline': 'DataObject' } } > > { 'type': DataObjectMember', > 'data': { 'name': 'str', 'type': 'DataObjectMemberType', > '*optional': 'bool', '*recursive': 'bool' } } > > { 'type': 'DataObjectStruct', > 'data': { 'data': [ 'DataObjectMember' ] } } > { 'type': 'DataObjectEnum', > 'data': { 'data': [ 'str' ] } } > { 'type': 'DataObjectUnion', > 'data': { 'data': [ 'DataObjectMember' ], '*base': 'str', > '*discriminator': 'str' } } > { 'type': 'DataObjectCommand', > 'data': { 'data': [ 'DataObjectMember' ], '*returns': 'DataObject' } } Defining DataObject to a union, it will solve some hidden problem :) > { 'union': 'DataObject', > 'base': 'DataObjectBase', > 'discriminator': 'type', > 'data': { > 'struct': 'DataObjectStruct', > 'enum': 'DataObjectEnum', > 'union': 'DataObjectUnion', > 'command': 'DataObjectCommand', > 'array': {} } > > > > > The DataObject is described in docs/qmp-full-introspection.txt in > > detail. > > > > The following content gives an example of query-tpm-types: > > > > ## Define example in qapi-schema.json: > > > > { 'enum': 'TpmType', 'data': [ 'passthrough' ] } > > { 'command': 'query-tpm-types', 'returns': ['TpmType'] } > > It might be more useful to have a (made-up) example that shows as many > details as possible - a command that takes arguments and has a return > type will exercise more of the code paths than a query command with only > a return. OK. > > > > ## Returned description: > > > > { > > "name": "query-tpm-types", > > "type": "Command", > > "returns": [ > > { > > "type": "TpmType", > > "data": [ > > { > > "type": "passthrough" > > } > > ] > > I need a way to know the difference between a TpmType returned directly > in a dict, vs. a return containing an array of TpmType. QObject is always a dictionary, it can describe list and dictionary. I will added a 'KIND' field for QObject. > > } > > ] > > }, > > Thus, using the discriminated union I set out above, I would return > something like: > { "name": "TpmType", "type": "enum", > "data": [ "passthrough" ] }, > { "name": "query-tpm-types", "type": "command", > "data": [ ], > "returns": { "name": "TpmType", "type": "array" } } > > > > > 'TpmType' is a defined type, it will be extended in returned > > description. [ 'passthrough' ] is a list, so 'type' and 'data' > > will be used. > > > > TODO: > > We can add events definations to qapi-schema.json by another > > s/definations/definitions/ > > > patch, then event can also be queried. > > > > Introduce another command 'query-qga-schema' to query QGA schema > > information, it's easy to add this support with current current > > patch. > > Such a command would be part of the QGA interface, not a QMP command. > But yes, it is needed, and the ideal patch series from you will include > that patch as part of the series. But I don't mind waiting until we get > the interface nailed down before you actually implement the QGA repeat > of the interface. > > > > > Signed-off-by: Amos Kong <akong@redhat.com> > > --- > > docs/qmp-full-introspection.txt | 143 +++++++++++++++++++ > > qapi-schema.json | 69 +++++++++ > > qmp-commands.hx | 39 +++++ > > qmp.c | 307 ++++++++++++++++++++++++++++++++++++++++ > > 4 files changed, 558 insertions(+) > > create mode 100644 docs/qmp-full-introspection.txt > > > > diff --git a/docs/qmp-full-introspection.txt b/docs/qmp-full-introspection.txt > > new file mode 100644 > > index 0000000..cc0fb80 > > --- /dev/null > > +++ b/docs/qmp-full-introspection.txt > > @@ -0,0 +1,143 @@ > > += full introspection support for QMP = > > + > > Is it worth merging this into the existing qapi-code-gen.txt, or at > least having the two documents refer to one another, since they deal > with related concepts (turning the .json file into generated code)? This doc is not just about code generating (we just generated a simple head file), most of work is about "full-introspection", the usage of DataObject, implementation detail(visit path), examples. > > +== Purpose == > > + > > +Add a new interface to provide QMP schema information to management, > > s/a new/an/ - after a year, the interface won't be new, but this doc > will still be relevant. > > > +the return data is a dynamical and nested dict/list, it contains > > s/dynamical/dynamic/ > > > +the useful metadata to help management to check feature support, > > +QMP commands detail, etc. > > + > > +== Usage == > > + > > +Execute QMP command: > > + > > + { "execute": "query-qmp-schema" } > > + > > +Returns: > > + > > + { "return": [ > > + { > > + "name": "query-name", > > + "type": "Command", > > + "returns": [ > > + { > > + "key": "*name", > > + "type": "str" > > + } > > + ] > > Are we trying to use struct names where they exist for compactness, or > trying to inline-expand everything as far as possible until we run into > self-referential recursion? inline-expand everything > > + }, > > + ... > > + } > > + > > +The whole schema information will be returned in one go, it contains > > +commands and event. It doesn't support to be filtered by type or name. > > s/event/events/ > s/It/At present, it/ > s/to be filtered/filtering/ > > > + > > +We have four types (ImageInfo, BlockStats, PciDeviceInfo, SchemaData) > > This list will get out of date quickly. I'd just word it generically: > > We have several self-referential types > > > +that uses themself in their own define data directly or indirectly, > > s/uses themself/use themselves/ > > > +we will not repeatedly extend them to avoid dead loop. > > Would it be worth a flag in the QMP output when a type is not being > further expanded because it is a nested occurrence of itself? ok. > That is, > given my proposed layout above, ImageInfo would turn into something like: > > { "name": "ImageInfo", "type": "struct", > "data": [ { "name": "filename", "type", "str" }, > ... > { "name": "backing-image", "type": "ImageInfo", > "optional": true, "recursive": true } ] }, > > > + > > +== more description of 'DataObject' type == > > + > > +We use 'DataObject' to describe dynamical data struct, it might be > > s/dynamical/dynamic/ > > > +nested dictionary, list or string. > > + > > +'DataObject' itself is a arbitrary and nested dictionary, the > > +dictionary has three keys ('key', 'type', 'data'), 'key' and > > spacing is odd. > > > +'data' are optional. > > + > > +* For describing Dictionary, we set the key to 'key', and set the > > + value to 'type' > > +* For describing List, we don't set 'key', just set the value to > > + 'type' > > Again, if you use the idea of a discriminated union, you don't need > quite the complexity in describing this: dictionaries are listed with > key/type/optional tuples, lists (enums) are listed with just an array of > strings, and the QAPI perfectly described that difference by the > discriminator telling you 'struct' vs. 'enum'. > > > +* If the value of dictionary or list is non-native type, we extend > > + the non-native type to dictionary, set it to 'data', and set the > > + non-native type's name to 'type'. > > Again, I tried to set up the QAPI that describes something that uses an > anonymous union that either gives a string (the name of a primitive or > already-defined type) or a struct (a recursion into another layer of > struct describing the structure of that type in line). > > > diff --git a/qapi-schema.json b/qapi-schema.json > > index 7b9fef1..cf03391 100644 > > --- a/qapi-schema.json > > +++ b/qapi-schema.json > > @@ -3679,3 +3679,72 @@ > > '*cpuid-input-ecx': 'int', > > 'cpuid-register': 'X86CPURegister32', > > 'features': 'int' } } > > + > > +## > > +# @DataObject > > +# > > +# Details of a data object, it can be nested dictionary/list > > +# > > +# @key: #optional Data object key > > +# > > +# @type: Data object type name > > +# > > +# @optional: #optional whether the data object is optional > > mention that the default is false. > > > +# > > +# @data: #optional DataObject list, can be a dictionary or list type > > so if 'data' is present, we are describing @type in more detail; if it > is absent, then @type is primitive. > > > +# > > +# Since: 1.6 > > +## > > +{ 'type': 'DataObject', > > + 'data': { '*key': 'str', '*type': 'str', '*optional': 'bool', '*data': ['DataObject'] } } > > + > > '*type' should be an enum type, not open-coded string. Right! > > +## > > +# @SchemaMetaType > > +# > > +# Possible meta types of a schema entry > > +# > > +# @Command: QMP command > > +# > > +# @Type: defined new data type > > +# > > +# @Enumeration: enumeration data type > > +# > > +# @Union: union data type > > +# > > +# Since: 1.6 > > +## > > +{ 'enum': 'SchemaMetaType', > > + 'data': ['Command', 'Type', 'Enumeration', 'Union'] } > > Do we need to start these with a capital? On the other hand, having > them as capitals may make it easier to ensure we are outputting correct > information. > > + > > +## > > +# @SchemaEntry > > +# > > +# Details of schema items > > +# > > +# @type: Entry's type in string format > > +# > > +# @name: Entry name > > +# > > +# @data: #optional list of DataObject. This can have different meaning > > +# depending on the 'type' value. For example, for a QMP command, > > +# this member contains an argument listing. For an enumeration, > > +# it contains the enum's values and so on > > This argues for making it a union type. > > > +# > > +# @returns: #optional list of DataObject, return data after executing > > +# QMP command > > +# > > +# Since: 1.6 > > +## > > +{ 'type': 'SchemaEntry', 'data': { 'type': 'SchemaMetaType', > > + 'name': 'str', '*data': ['DataObject'], '*returns': ['DataObject'] } } > > + > > +## > > +# @query-qmp-schema > > +# > > +# Query QMP schema information > > +# > > +# Returns: list of @SchemaEntry. Returns an error if json string is invalid. > > If you don't take any arguments, then the "returns an error" statement > is impossible. When we execute the full introspecting, we will parse the json strings in the head file (converted from .json file). Return error if the json string is invild in head file. > > +# > > +# Since: 1.6 > > +## > > +{ 'command': 'query-qmp-schema', 'returns': ['SchemaEntry'] } > > diff --git a/qmp-commands.hx b/qmp-commands.hx > > index e075df4..e3cbe93 100644 > > --- a/qmp-commands.hx > > > > +/* > > + * Use a string to record the visit path, type index of each node > > + * will be saved to the string, indexes are split by ':'. > > + */ > > +static char visit_path_str[1024]; > > Is a fixed width buffer really the best solution, or does glib offer us > something better? For example, a hash table, or even a growable string. 1024 is enough by experience, and I have a check in push_id() which will fill string in visit_path_str. +static void push_id(int id) +{ + char *end = strrchr(visit_path_str, ':'); + char type_idx[256]; + int num; + + num = sprintf(type_idx, "%d:", id); + + if (end) { + /* avoid overflow */ + assert(end - visit_path_str + 1 + num < sizeof(visit_path_str)); > > + > > + for (i = 0; qmp_schema_table[i]; i++) { > > + data = qobject_from_json(qmp_schema_table[i]); > > + assert(data != NULL); > > + > > + qdict = qobject_to_qdict(data); > > + assert(qdict != NULL); > > + > > + ent = qdict_first(qdict); > > + if (!qdict_get(qdict, "enum") && !qdict_get(qdict, "type") > > + && !qdict_get(qdict, "union")) { > > + continue; > > + } > > Why are we doing the work in C code, every time the command is called? > Wouldn't it be more efficient to do the work in python code, at the time > the qmp_schema_table C code is first generated, so that the generated > code is already in the right format? Right now, we parse the json string, and generate a dynamic data struct and set it as the output of QMP. Parse json string by qapi (python) script and generate C code ( a big tree, it's buildup by many nested structs. We need to visit the big static tree and generate another dynamic tree for QMP. I think current implement is better. I will try the python solution. > > +SchemaEntryList *qmp_query_qmp_schema(Error **errp) > > +{ > > + SchemaEntryList *list, *last_entry, *entry; > > + SchemaEntry *info; > > + DataObjectList *obj_entry; > > + DataObject *obj_info; > > + QObject *data; > > + QDict *qdict; > > + int i; > > + > > + list = NULL; > > + for (i = 0; qmp_schema_table[i]; i++) { > > + data = qobject_from_json(qmp_schema_table[i]); > > + assert(data != NULL); > > + > > + qdict = qobject_to_qdict(data); > > + assert(qdict != NULL); > > + > > + if (qdict_get(qdict, "command")) { > > + info = g_malloc0(sizeof(*info)); > > + info->type = SCHEMA_META_TYPE_COMMAND; > > + info->name = strdup(qdict_get_str(qdict, "command")); > > + } else { > > + continue; > > + } > > + > > Don't we want to also output types (struct, union, enum) and eventually > events, not just commands? I think struct, union, enum are used in commands, so I didn't repeatedly output them. If it's not repeated, I will output all of them in next version. > I hope we're converging, but I'm starting to worry that we won't have a > design in place in time for the 1.6 release.
On 07/26/2013 01:51 AM, Amos Kong wrote: >>> +# Query QMP schema information >>> +# >>> +# Returns: list of @SchemaEntry. Returns an error if json string is invalid. >> >> If you don't take any arguments, then the "returns an error" statement >> is impossible. > > When we execute the full introspecting, we will parse the json strings > in the head file (converted from .json file). Return error if the > json string is invild in head file. We should fail to compile if the conversion from .json to head file is not correct. No working qemu binary should ever encounter an invalid head file.
On Fri, Jul 19, 2013 at 04:05:16PM -0600, Eric Blake wrote: > On 07/16/2013 04:37 AM, Amos Kong wrote: > > Introduces new monitor command to query QMP schema information, > > the return data is a dynamical and nested dict/list, it contains > > s/dynamical/dynamic/ > > > the useful metadata to help management to check feature support, > > QMP commands detail, etc. > > > > I added a document for QMP introspection support. > > (docs/qmp-full-introspection.txt) > > Yay - docs make a world of difference. > > > > > We need to parse all commands json definition, and generated a > > mixing tense ("need" present, "generated" past); for commit messages, > present tense is generally appropriate. Thus: > > s/generated/generate/ > > > dynamical tree, QMP infrastructure will convert the tree to > > s/dynamical/dynamic/ > > > json string and return to QMP client. > > > > So here I defined a 'DataObject' type in qapi-schema.json, > > it's used to describe the dynamical dictionary/list/string. > > s/dynamical/dynamic/ > > > > > { 'type': 'DataObject', > > 'data': { '*key': 'str', '*type': 'str', '*data': ['DataObject'] } } > > > > Not all the keys in data will be used. > > # List: type > > # Dict: key, type > > # nested List: type, data > > # nested Dict: key, type, data > > I wonder if we can take advantage of Kevin's work on unions to make this > MUCH easier to use: > > https://lists.gnu.org/archive/html/qemu-devel/2013-07/msg01407.html > > { 'enum': 'DataObjectType', > 'data': [ 'command', 'struct', 'union', 'enum' ] } > # extend to add 'event' later > > { 'type': 'DataObjectBase', > 'data': { 'name': 'str' } } > > { 'union': 'DataObjectMemberType', > 'discriminator': {}, > 'data': { 'reference': 'str', > 'inline': 'DataObject' } } > > { 'type': DataObjectMember', > 'data': { 'name': 'str', 'type': 'DataObjectMemberType', > '*optional': 'bool', '*recursive': 'bool' } } > > { 'type': 'DataObjectStruct', > 'data': { 'data': [ 'DataObjectMember' ] } } > { 'type': 'DataObjectEnum', > 'data': { 'data': [ 'str' ] } } > { 'type': 'DataObjectUnion', > 'data': { 'data': [ 'DataObjectMember' ], '*base': 'str', > '*discriminator': 'str' } } > { 'type': 'DataObjectCommand', > 'data': { 'data': [ 'DataObjectMember' ], '*returns': 'DataObject' } } > { 'union': 'DataObject', > 'base': 'DataObjectBase', > 'discriminator': 'type', > 'data': { > 'struct': 'DataObjectStruct', > 'enum': 'DataObjectEnum', > 'union': 'DataObjectUnion', > 'command': 'DataObjectCommand', > 'array': {} } In my patch, I used a _dictionary_ to describe this kind of thing 1) dict, 2) list, 3) str The above line is used for Dictionary or List, it should be: 'array': ['DataObject'] But I touched a new error: qapi-visit.c: In function ‘visit_type_DataObject’: qapi-visit.c:7255:29: error: implicit declaration of function ‘visit_type_DataObjectList_fields’ [-Werror=implicit-function-declaration] visit_type_DataObjectList_fields(m, &(*obj)->array, &err); ---- So I try to defined { 'type': 'DataObjectArray', 'data': ['DataObject'] } 'DataObjectArrayType' } } { 'union': 'DataObject', 'base': 'DataObjectBase', 'discriminator': 'name', 'data': { 'array': 'DataObjectArray', got error: Traceback (most recent call last): File "/home/devel/qemu/scripts/qapi-types.py", line 471, in <module> ret += generate_struct(expr) + "\n" File "/home/devel/qemu/scripts/qapi-types.py", line 101, in generate_struct ret += generate_struct_fields(members) File "/home/devel/qemu/scripts/qapi-types.py", line 67, in generate_struct_fields for argname, argentry, optional, structured in parse_args(members): File "/home/devel/qemu/scripts/qapi.py", line 197, in parse_args argentry = typeinfo[member] TypeError: list indices must be integers, not str ---- a new definition: { 'enum': 'DataObjectArrayType', 'data': ['Dictionary', 'List'] } { 'type': 'DataObjectArray', 'data': {'data': ['DataObject'], 'type': 'DataObjectArrayType' } } { 'union': 'DataObject', 'base': 'DataObjectBase', 'discriminator': 'name', 'data': { 'array': 'DataObjectArray', ----- In my V2, I parse the schema just according the Format attribute (dict, str, list) Eric suggested defination is wonderful, but it's not flexible as mine ;-) The data type, format (dict/str/list), more matadata should be considered. It makes the parse very complex. I have to simple it, the matadata will also provided, just make the parse work easyer. Libvirt can still get good info as using Eric's defination. Thanks, Amos > > The DataObject is described in docs/qmp-full-introspection.txt in > > detail. > > > > The following content gives an example of query-tpm-types: > > > > ## Define example in qapi-schema.json: > > > > { 'enum': 'TpmType', 'data': [ 'passthrough' ] } > > { 'command': 'query-tpm-types', 'returns': ['TpmType'] } > > It might be more useful to have a (made-up) example that shows as many > details as possible - a command that takes arguments and has a return > type will exercise more of the code paths than a query command with only > a return. > > > > > ## Returned description: > > > > { > > "name": "query-tpm-types", > > "type": "Command", > > "returns": [ > > { > > "type": "TpmType", > > "data": [ > > { > > "type": "passthrough" > > } > > ] > > I need a way to know the difference between a TpmType returned directly > in a dict, vs. a return containing an array of TpmType. > > > } > > ] > > }, > > Thus, using the discriminated union I set out above, I would return > something like: > { "name": "TpmType", "type": "enum", > "data": [ "passthrough" ] }, > { "name": "query-tpm-types", "type": "command", > "data": [ ], > "returns": { "name": "TpmType", "type": "array" } } > > > > > 'TpmType' is a defined type, it will be extended in returned > > description. [ 'passthrough' ] is a list, so 'type' and 'data' > > will be used. > > > > TODO: > > We can add events definations to qapi-schema.json by another > > s/definations/definitions/ > > > patch, then event can also be queried. > > > > Introduce another command 'query-qga-schema' to query QGA schema > > information, it's easy to add this support with current current > > patch. > > Such a command would be part of the QGA interface, not a QMP command. > But yes, it is needed, and the ideal patch series from you will include > that patch as part of the series. But I don't mind waiting until we get > the interface nailed down before you actually implement the QGA repeat > of the interface. > > > > > Signed-off-by: Amos Kong <akong@redhat.com> > > --- > > docs/qmp-full-introspection.txt | 143 +++++++++++++++++++ > > qapi-schema.json | 69 +++++++++ > > qmp-commands.hx | 39 +++++ > > qmp.c | 307 ++++++++++++++++++++++++++++++++++++++++ > > 4 files changed, 558 insertions(+) > > create mode 100644 docs/qmp-full-introspection.txt > > > > diff --git a/docs/qmp-full-introspection.txt b/docs/qmp-full-introspection.txt > > new file mode 100644 > > index 0000000..cc0fb80 > > --- /dev/null > > +++ b/docs/qmp-full-introspection.txt > > @@ -0,0 +1,143 @@ > > += full introspection support for QMP = > > + > > Is it worth merging this into the existing qapi-code-gen.txt, or at > least having the two documents refer to one another, since they deal > with related concepts (turning the .json file into generated code)? > > > +== Purpose == > > + > > +Add a new interface to provide QMP schema information to management, > > s/a new/an/ - after a year, the interface won't be new, but this doc > will still be relevant. > > > +the return data is a dynamical and nested dict/list, it contains > > s/dynamical/dynamic/ > > > +the useful metadata to help management to check feature support, > > +QMP commands detail, etc. > > + > > +== Usage == > > + > > +Execute QMP command: > > + > > + { "execute": "query-qmp-schema" } > > + > > +Returns: > > + > > + { "return": [ > > + { > > + "name": "query-name", > > + "type": "Command", > > + "returns": [ > > + { > > + "key": "*name", > > + "type": "str" > > + } > > + ] > > Are we trying to use struct names where they exist for compactness, or > trying to inline-expand everything as far as possible until we run into > self-referential recursion? > > > + }, > > + ... > > + } > > + > > +The whole schema information will be returned in one go, it contains > > +commands and event. It doesn't support to be filtered by type or name. > > s/event/events/ > s/It/At present, it/ > s/to be filtered/filtering/ > > > + > > +We have four types (ImageInfo, BlockStats, PciDeviceInfo, SchemaData) > > This list will get out of date quickly. I'd just word it generically: > > We have several self-referential types > > > +that uses themself in their own define data directly or indirectly, > > s/uses themself/use themselves/ > > > +we will not repeatedly extend them to avoid dead loop. > > Would it be worth a flag in the QMP output when a type is not being > further expanded because it is a nested occurrence of itself? That is, > given my proposed layout above, ImageInfo would turn into something like: > > { "name": "ImageInfo", "type": "struct", > "data": [ { "name": "filename", "type", "str" }, > ... > { "name": "backing-image", "type": "ImageInfo", > "optional": true, "recursive": true } ] }, > > > + > > +== more description of 'DataObject' type == > > + > > +We use 'DataObject' to describe dynamical data struct, it might be > > s/dynamical/dynamic/ > > > +nested dictionary, list or string. > > + > > +'DataObject' itself is a arbitrary and nested dictionary, the > > +dictionary has three keys ('key', 'type', 'data'), 'key' and > > spacing is odd. > > > +'data' are optional. > > + > > +* For describing Dictionary, we set the key to 'key', and set the > > + value to 'type' > > +* For describing List, we don't set 'key', just set the value to > > + 'type' > > Again, if you use the idea of a discriminated union, you don't need > quite the complexity in describing this: dictionaries are listed with > key/type/optional tuples, lists (enums) are listed with just an array of > strings, and the QAPI perfectly described that difference by the > discriminator telling you 'struct' vs. 'enum'. > > > +* If the value of dictionary or list is non-native type, we extend > > + the non-native type to dictionary, set it to 'data', and set the > > + non-native type's name to 'type'. > > Again, I tried to set up the QAPI that describes something that uses an > anonymous union that either gives a string (the name of a primitive or > already-defined type) or a struct (a recursion into another layer of > struct describing the structure of that type in line). > > > diff --git a/qapi-schema.json b/qapi-schema.json > > index 7b9fef1..cf03391 100644 > > --- a/qapi-schema.json > > +++ b/qapi-schema.json > > @@ -3679,3 +3679,72 @@ > > '*cpuid-input-ecx': 'int', > > 'cpuid-register': 'X86CPURegister32', > > 'features': 'int' } } > > + > > +## > > +# @DataObject > > +# > > +# Details of a data object, it can be nested dictionary/list > > +# > > +# @key: #optional Data object key > > +# > > +# @type: Data object type name > > +# > > +# @optional: #optional whether the data object is optional > > mention that the default is false. > > > +# > > +# @data: #optional DataObject list, can be a dictionary or list type > > so if 'data' is present, we are describing @type in more detail; if it > is absent, then @type is primitive. > > > +# > > +# Since: 1.6 > > +## > > +{ 'type': 'DataObject', > > + 'data': { '*key': 'str', '*type': 'str', '*optional': 'bool', '*data': ['DataObject'] } } > > + > > '*type' should be an enum type, not open-coded string. > > > > +## > > +# @SchemaMetaType > > +# > > +# Possible meta types of a schema entry > > +# > > +# @Command: QMP command > > +# > > +# @Type: defined new data type > > +# > > +# @Enumeration: enumeration data type > > +# > > +# @Union: union data type > > +# > > +# Since: 1.6 > > +## > > +{ 'enum': 'SchemaMetaType', > > + 'data': ['Command', 'Type', 'Enumeration', 'Union'] } > > Do we need to start these with a capital? On the other hand, having > them as capitals may make it easier to ensure we are outputting correct > information. > > + > > +## > > +# @SchemaEntry > > +# > > +# Details of schema items > > +# > > +# @type: Entry's type in string format > > +# > > +# @name: Entry name > > +# > > +# @data: #optional list of DataObject. This can have different meaning > > +# depending on the 'type' value. For example, for a QMP command, > > +# this member contains an argument listing. For an enumeration, > > +# it contains the enum's values and so on > > This argues for making it a union type. > > > +# > > +# @returns: #optional list of DataObject, return data after executing > > +# QMP command > > +# > > +# Since: 1.6 > > +## > > +{ 'type': 'SchemaEntry', 'data': { 'type': 'SchemaMetaType', > > + 'name': 'str', '*data': ['DataObject'], '*returns': ['DataObject'] } } > > + > > +## > > +# @query-qmp-schema > > +# > > +# Query QMP schema information > > +# > > +# Returns: list of @SchemaEntry. Returns an error if json string is invalid. > > If you don't take any arguments, then the "returns an error" statement > is impossible. > > > +# > > +# Since: 1.6 > > +## > > +{ 'command': 'query-qmp-schema', 'returns': ['SchemaEntry'] } > > diff --git a/qmp-commands.hx b/qmp-commands.hx > > index e075df4..e3cbe93 100644 > > --- a/qmp-commands.hx > > > > +/* > > + * Use a string to record the visit path, type index of each node > > + * will be saved to the string, indexes are split by ':'. > > + */ > > +static char visit_path_str[1024]; > > Is a fixed width buffer really the best solution, or does glib offer us > something better? For example, a hash table, or even a growable string. > > > + > > + for (i = 0; qmp_schema_table[i]; i++) { > > + data = qobject_from_json(qmp_schema_table[i]); > > + assert(data != NULL); > > + > > + qdict = qobject_to_qdict(data); > > + assert(qdict != NULL); > > + > > + ent = qdict_first(qdict); > > + if (!qdict_get(qdict, "enum") && !qdict_get(qdict, "type") > > + && !qdict_get(qdict, "union")) { > > + continue; > > + } > > Why are we doing the work in C code, every time the command is called? > Wouldn't it be more efficient to do the work in python code, at the time > the qmp_schema_table C code is first generated, so that the generated > code is already in the right format? > > > +SchemaEntryList *qmp_query_qmp_schema(Error **errp) > > +{ > > + SchemaEntryList *list, *last_entry, *entry; > > + SchemaEntry *info; > > + DataObjectList *obj_entry; > > + DataObject *obj_info; > > + QObject *data; > > + QDict *qdict; > > + int i; > > + > > + list = NULL; > > + for (i = 0; qmp_schema_table[i]; i++) { > > + data = qobject_from_json(qmp_schema_table[i]); > > + assert(data != NULL); > > + > > + qdict = qobject_to_qdict(data); > > + assert(qdict != NULL); > > + > > + if (qdict_get(qdict, "command")) { > > + info = g_malloc0(sizeof(*info)); > > + info->type = SCHEMA_META_TYPE_COMMAND; > > + info->name = strdup(qdict_get_str(qdict, "command")); > > + } else { > > + continue; > > + } > > + > > Don't we want to also output types (struct, union, enum) and eventually > events, not just commands? > > I hope we're converging, but I'm starting to worry that we won't have a > design in place in time for the 1.6 release. > > -- > Eric Blake eblake redhat com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org >
Am 27.11.2013 um 03:32 hat Amos Kong geschrieben: > On Fri, Jul 19, 2013 at 04:05:16PM -0600, Eric Blake wrote: > > On 07/16/2013 04:37 AM, Amos Kong wrote: > > > { 'type': 'DataObject', > > > 'data': { '*key': 'str', '*type': 'str', '*data': ['DataObject'] } } > > > > > > Not all the keys in data will be used. > > > # List: type > > > # Dict: key, type > > > # nested List: type, data > > > # nested Dict: key, type, data > > > > I wonder if we can take advantage of Kevin's work on unions to make this > > MUCH easier to use: > > > > https://lists.gnu.org/archive/html/qemu-devel/2013-07/msg01407.html > > > > { 'enum': 'DataObjectType', > > 'data': [ 'command', 'struct', 'union', 'enum' ] } > > # extend to add 'event' later > > > > { 'type': 'DataObjectBase', > > 'data': { 'name': 'str' } } > > > > { 'union': 'DataObjectMemberType', > > 'discriminator': {}, > > 'data': { 'reference': 'str', > > 'inline': 'DataObject' } } > > > > { 'type': DataObjectMember', > > 'data': { 'name': 'str', 'type': 'DataObjectMemberType', > > '*optional': 'bool', '*recursive': 'bool' } } > > > > { 'type': 'DataObjectStruct', > > 'data': { 'data': [ 'DataObjectMember' ] } } > > { 'type': 'DataObjectEnum', > > 'data': { 'data': [ 'str' ] } } > > { 'type': 'DataObjectUnion', > > 'data': { 'data': [ 'DataObjectMember' ], '*base': 'str', > > '*discriminator': 'str' } } > > { 'type': 'DataObjectCommand', > > 'data': { 'data': [ 'DataObjectMember' ], '*returns': 'DataObject' } } > > > > { 'union': 'DataObject', > > 'base': 'DataObjectBase', > > 'discriminator': 'type', > > 'data': { > > 'struct': 'DataObjectStruct', > > 'enum': 'DataObjectEnum', > > 'union': 'DataObjectUnion', > > 'command': 'DataObjectCommand', > > > 'array': {} } > > > In my patch, I used a _dictionary_ to describe this kind of thing > 1) dict, 2) list, 3) str > > The above line is used for Dictionary or List, it should be: > 'array': ['DataObject'] > > But I touched a new error: > qapi-visit.c: In function ‘visit_type_DataObject’: > qapi-visit.c:7255:29: error: implicit declaration of function ‘visit_type_DataObjectList_fields’ [-Werror=implicit-function-declaration] > visit_type_DataObjectList_fields(m, &(*obj)->array, &err); > > ---- > So I try to defined > > { 'type': 'DataObjectArray', 'data': ['DataObject'] } > 'DataObjectArrayType' } } > > { 'union': 'DataObject', > 'base': 'DataObjectBase', > 'discriminator': 'name', > 'data': { > 'array': 'DataObjectArray', > > got error: > Traceback (most recent call last): > File "/home/devel/qemu/scripts/qapi-types.py", line 471, in <module> > ret += generate_struct(expr) + "\n" > File "/home/devel/qemu/scripts/qapi-types.py", line 101, in generate_struct > ret += generate_struct_fields(members) > File "/home/devel/qemu/scripts/qapi-types.py", line 67, in generate_struct_fields > for argname, argentry, optional, structured in parse_args(members): > File "/home/devel/qemu/scripts/qapi.py", line 197, in parse_args > argentry = typeinfo[member] > TypeError: list indices must be integers, not str I've never had a need for unions inside an array so far, but that doesn't make them less useful. We'll need them in more places than just here. So instead of working around the current limitations, the right thing to do is to modify the QAPI generator to allow this kind of definitions. Kevin
(resend without big attachment)
Hello Eric, other
We had "command, enumeration, type, unionobj" in Eric suggested DataObject
union, it's helpful for us to provide meaningful metadata in the output.
but there still exists some problem.
We should describe some arbitrary data struct, I would like to call it "undefined struct"
eg 1:
{ 'type': 'VersionInfo',
'data': {'qemu': {'major': 'int', 'minor': 'int', 'micro': 'int'},
'package': 'str'} }
it's same as:
{ 'type': 'newtype',
'data': {'major': 'int', 'minor': 'int', 'micro': 'int'} }
{ 'type': 'VersionInfo',
'data': { 'qemu': 'newtype', 'package': 'str'} }
The difference between original 'DataObjectType' and 'DataObjectUndefinedStruct'
is that we don't have 'name' for the DataObject union for undefined struct.
so I set the 'name' item in DataObjectBase to be optional.
eg 2:
{ 'command': 'human-monitor-command',
'data': {'command-line': 'str', '*cpu-index': 'int'},
'returns': 'str' }
... 'returns': ['RxFilterInfo'] }
... 'returns': 'ChardevReturn' }
We returns str (native type), list or extended dict here. Sometimes we
returns a defined type, but it doesn't need to be extended. And we need
to describe this kind of data (type str, dict or list) by "DataObject"
in schema definition of DataObject** type.
So I added a "'reference-type': 'String'" in DataObject union.
list, dict will still be described by "DataObjectType"
You can find the draft patches here:
https://github.com/kongove/qemu/commits/qmp-introspection
I will post the V3 when I finish the cleanup.
Thanks, Amos
Command output
==============
https://raw.github.com/kongove/misc/master/txt/qmp-introspection.output.txt (1.6M)
Latest schema definition
========================
{ 'type': 'DataObjectBase',
'data': { '*name': 'str', 'type': 'str' } }
{ 'union': 'DataObjectMemberType',
'discriminator': {},
'data': { 'reference': 'str',
'undefined': 'DataObject',
'extend': 'DataObject' } }
{ 'type': 'DataObjectMember',
'data': { 'type': 'DataObjectMemberType', '*name': 'str',
'*optional': 'bool', '*recursive': 'bool' } }
{ 'type': 'DataObjectCommand',
'data': { '*data': [ 'DataObjectMember' ],
'*returns': 'DataObject',
'*gen': 'bool' } }
{ 'type': 'DataObjectEnumeration',
'data': { 'data': [ 'str' ] } }
{ 'type': 'DataObjectType',
'data': { 'data': [ 'DataObjectMember' ] } }
{ 'type': 'DataObjectUndefinedStruct',
'data': { 'data': [ 'DataObjectMember' ] } }
{ 'type': 'DataObjectUnion',
'data': { 'data': [ 'DataObjectMember' ], '*base': 'str',
'*discriminator': 'str' } }
{ 'union': 'DataObject',
'base': 'DataObjectBase',
'discriminator': 'type',
'data': {
'command': 'DataObjectCommand',
'enumeration': 'DataObjectEnumeration',
'type': 'DataObjectType',
'undefined-struct': 'DataObjectUndefinedStruct',
'reference-type': 'String',
'unionobj': 'DataObjectUnion' } }
{ 'command': 'query-qmp-schema', 'returns': ['DataObject'] }
Il 20/12/2013 12:57, Amos Kong ha scritto: > { 'type': 'DataObjectBase', > 'data': { '*name': 'str', 'type': 'str' } } > { 'union': 'DataObjectMemberType', > 'discriminator': {}, > 'data': { 'reference': 'str', > 'undefined': 'DataObject', > 'extend': 'DataObject' } } What is the purpose of "undefined"? I don't see any occurrence of any of "undefined" or "extend" in the sample. > { 'type': 'DataObjectMember', > 'data': { 'type': 'DataObjectMemberType', '*name': 'str', > '*optional': 'bool', '*recursive': 'bool' } } > { 'type': 'DataObjectCommand', > 'data': { '*data': [ 'DataObjectMember' ], > '*returns': 'DataObject', > '*gen': 'bool' } } > { 'type': 'DataObjectEnumeration', > 'data': { 'data': [ 'str' ] } } > { 'type': 'DataObjectType', > 'data': { 'data': [ 'DataObjectMember' ] } } > { 'type': 'DataObjectUndefinedStruct', Perhaps Unnamed or Anonymous? > 'data': { 'data': [ 'DataObjectMember' ] } } > { 'type': 'DataObjectUnion', > 'data': { 'data': [ 'DataObjectMember' ], '*base': 'str', > '*discriminator': 'str' } } > { 'union': 'DataObject', > 'base': 'DataObjectBase', > 'discriminator': 'type', > 'data': { > 'command': 'DataObjectCommand', > 'enumeration': 'DataObjectEnumeration', > 'type': 'DataObjectType', > 'undefined-struct': 'DataObjectUndefinedStruct', > 'reference-type': 'String', > 'unionobj': 'DataObjectUnion' } } > { 'command': 'query-qmp-schema', 'returns': ['DataObject'] } I think forcing expansion of everything that isn't unnamed/anonymous makes the schema much larger and unwieldy. Otherwise looks great! Paolo
Hi, Amos > (resend without big attachment) > > Hello Eric, other > > We had "command, enumeration, type, unionobj" in Eric suggested DataObject > union, it's helpful for us to provide meaningful metadata in the output. > but there still exists some problem. > > We should describe some arbitrary data struct, I would like to call it "undefined struct" > If user have defined an arbitrary or embbed data struct, I think it is better leave as it is, instead of generate a new struct for it. Since qapi-visit.c and qapi-types.c doesn't have a "undefined" struct for it now, it is a bit risk to do it only in QMP introspection. Maybe leave it now, and support it when we found it is really useful?(and add it in qapi-visit.c and qapi-types.c correspondly) > eg 1: > { 'type': 'VersionInfo', > 'data': {'qemu': {'major': 'int', 'minor': 'int', 'micro': 'int'}, > 'package': 'str'} } > it's same as: > { 'type': 'newtype', > 'data': {'major': 'int', 'minor': 'int', 'micro': 'int'} } > { 'type': 'VersionInfo', > 'data': { 'qemu': 'newtype', 'package': 'str'} } > > The difference between original 'DataObjectType' and 'DataObjectUndefinedStruct' > is that we don't have 'name' for the DataObject union for undefined struct. > so I set the 'name' item in DataObjectBase to be optional. > > eg 2: > { 'command': 'human-monitor-command', > 'data': {'command-line': 'str', '*cpu-index': 'int'}, > 'returns': 'str' } > ... 'returns': ['RxFilterInfo'] } > ... 'returns': 'ChardevReturn' } > > We returns str (native type), list or extended dict here. Sometimes we > returns a defined type, but it doesn't need to be extended. And we need > to describe this kind of data (type str, dict or list) by "DataObject" > in schema definition of DataObject** type. > > So I added a "'reference-type': 'String'" in DataObject union. > list, dict will still be described by "DataObjectType" > I guess you want to tip what type may be returned? It seems a bit too agressive, since the qapi-schema.json didn't tip that those types may be returned. > > You can find the draft patches here: > https://github.com/kongove/qemu/commits/qmp-introspection > I will post the V3 when I finish the cleanup. > > Thanks, Amos > > > Command output > ============== > https://raw.github.com/kongove/misc/master/txt/qmp-introspection.output.txt (1.6M) > > Latest schema definition > ======================== > { 'type': 'DataObjectBase', > 'data': { '*name': 'str', 'type': 'str' } } > { 'union': 'DataObjectMemberType', > 'discriminator': {}, > 'data': { 'reference': 'str', > 'undefined': 'DataObject', > 'extend': 'DataObject' } } > { 'type': 'DataObjectMember', > 'data': { 'type': 'DataObjectMemberType', '*name': 'str', > '*optional': 'bool', '*recursive': 'bool' } } > { 'type': 'DataObjectCommand', > 'data': { '*data': [ 'DataObjectMember' ], > '*returns': 'DataObject', > '*gen': 'bool' } } > { 'type': 'DataObjectEnumeration', > 'data': { 'data': [ 'str' ] } } > { 'type': 'DataObjectType', > 'data': { 'data': [ 'DataObjectMember' ] } } > { 'type': 'DataObjectUndefinedStruct', > 'data': { 'data': [ 'DataObjectMember' ] } } > { 'type': 'DataObjectUnion', > 'data': { 'data': [ 'DataObjectMember' ], '*base': 'str', > '*discriminator': 'str' } } > { 'union': 'DataObject', > 'base': 'DataObjectBase', > 'discriminator': 'type', > 'data': { > 'command': 'DataObjectCommand', > 'enumeration': 'DataObjectEnumeration', > 'type': 'DataObjectType', > 'undefined-struct': 'DataObjectUndefinedStruct', > 'reference-type': 'String', > 'unionobj': 'DataObjectUnion' } } > { 'command': 'query-qmp-schema', 'returns': ['DataObject'] } >
On Mon, Dec 23, 2013 at 02:32:46PM +0800, Wenchao Xia wrote: > Hi, Amos > > >(resend without big attachment) > > > >Hello Eric, other > > > >We had "command, enumeration, type, unionobj" in Eric suggested DataObject > >union, it's helpful for us to provide meaningful metadata in the output. > >but there still exists some problem. > > > >We should describe some arbitrary data struct, I would like to call it "undefined struct" > > > If user have defined an arbitrary or embbed data struct, I think it > is better leave as it is, instead of generate a new struct for it. I don't really generate a new struct for it, just try the arbitrary data as an abstract 'undefined' struct. In the output, it's "leave as it is". > Since qapi-visit.c and qapi-types.c doesn't have a "undefined" struct > for it now, it is a bit risk to do it only in QMP introspection. Maybe > leave it now, and support it when we found it is really useful?(and add > it in qapi-visit.c and qapi-types.c correspondly) > > >eg 1: > > { 'type': 'VersionInfo', > > 'data': {'qemu': {'major': 'int', 'minor': 'int', 'micro': 'int'}, > > 'package': 'str'} } > > it's same as: > > { 'type': 'newtype', > > 'data': {'major': 'int', 'minor': 'int', 'micro': 'int'} } > > { 'type': 'VersionInfo', > > 'data': { 'qemu': 'newtype', 'package': 'str'} } > > > > The difference between original 'DataObjectType' and 'DataObjectUndefinedStruct' > > is that we don't have 'name' for the DataObject union for undefined struct. > > so I set the 'name' item in DataObjectBase to be optional. > > > >eg 2: > > { 'command': 'human-monitor-command', > > 'data': {'command-line': 'str', '*cpu-index': 'int'}, > > 'returns': 'str' } > > ... 'returns': ['RxFilterInfo'] } > > ... 'returns': 'ChardevReturn' } > > > > We returns str (native type), list or extended dict here. Sometimes we > > returns a defined type, but it doesn't need to be extended. And we need > > to describe this kind of data (type str, dict or list) by "DataObject" > > in schema definition of DataObject** type. > > > > So I added a "'reference-type': 'String'" in DataObject union. > > list, dict will still be described by "DataObjectType" > > > I guess you want to tip what type may be returned? It seems a bit too > agressive, since the qapi-schema.json didn't tip that those types may > be returned. In command schema, the value of 'returns' tips the model of return data. It can be string(defined type/union/enum/etc) or undefined list/dict. > >You can find the draft patches here: > > https://github.com/kongove/qemu/commits/qmp-introspection > >I will post the V3 when I finish the cleanup.
On Fri, Dec 20, 2013 at 07:03:57PM +0100, Paolo Bonzini wrote: > Il 20/12/2013 12:57, Amos Kong ha scritto: > > { 'type': 'DataObjectBase', > > 'data': { '*name': 'str', 'type': 'str' } } > > { 'union': 'DataObjectMemberType', > > 'discriminator': {}, > > 'data': { 'reference': 'str', > > 'undefined': 'DataObject', > > 'extend': 'DataObject' } } > > What is the purpose of "undefined"? I don't see any occurrence of any > of "undefined" or "extend" in the sample. | { 'type': 'VersionInfo', | 'data': {'qemu': {'major': 'int', 'minor': 'int', 'micro': 'int'}, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | { | "name": "VersionInfo", | "type": "type", | "data": [ |~ { |~ "name": "qemu", |~ "optional": false, |~ "type": { |~ "type": "undefined-struct", |~ "data": [ |~ { |~ "name": "micro", |~ "optional": false, |~ "recursive": false, |~ "type": "int" |~ }, |~ { |~ "name": "minor", |~ "optional": false, |~ "recursive": false, |~ "type": "int" |~ }, |~ { |~ "name": "major", |~ "optional": false, |~ "recursive": false, |~ "type": "int" |~ } |~ ] |~ } |~ }, > > { 'type': 'DataObjectMember', > > 'data': { 'type': 'DataObjectMemberType', '*name': 'str', > > '*optional': 'bool', '*recursive': 'bool' } } > > { 'type': 'DataObjectCommand', > > 'data': { '*data': [ 'DataObjectMember' ], > > '*returns': 'DataObject', > > '*gen': 'bool' } } > > { 'type': 'DataObjectEnumeration', > > 'data': { 'data': [ 'str' ] } } > > { 'type': 'DataObjectType', > > 'data': { 'data': [ 'DataObjectMember' ] } } > > { 'type': 'DataObjectUndefinedStruct', > > Perhaps Unnamed or Anonymous? Anonymous is good. > > 'data': { 'data': [ 'DataObjectMember' ] } } > > { 'type': 'DataObjectUnion', > > 'data': { 'data': [ 'DataObjectMember' ], '*base': 'str', > > '*discriminator': 'str' } } > > { 'union': 'DataObject', > > 'base': 'DataObjectBase', > > 'discriminator': 'type', > > 'data': { > > 'command': 'DataObjectCommand', > > 'enumeration': 'DataObjectEnumeration', > > 'type': 'DataObjectType', > > 'undefined-struct': 'DataObjectUndefinedStruct', > > 'reference-type': 'String', > > 'unionobj': 'DataObjectUnion' } } > > { 'command': 'query-qmp-schema', 'returns': ['DataObject'] } > > I think forcing expansion of everything that isn't unnamed/anonymous > makes the schema much larger and unwieldy. Otherwise looks great! We want to provide more useful metadata, and used some enum/unions to indicate the dynamic type. In the output, some simple data is processed too unwieldy. In another side, some complex data is described clearly. It's also caused by some limitation of QAPI infrastructure. > Paolo
diff --git a/docs/qmp-full-introspection.txt b/docs/qmp-full-introspection.txt new file mode 100644 index 0000000..cc0fb80 --- /dev/null +++ b/docs/qmp-full-introspection.txt @@ -0,0 +1,143 @@ += full introspection support for QMP = + +== Purpose == + +Add a new interface to provide QMP schema information to management, +the return data is a dynamical and nested dict/list, it contains +the useful metadata to help management to check feature support, +QMP commands detail, etc. + +== Usage == + +Execute QMP command: + + { "execute": "query-qmp-schema" } + +Returns: + + { "return": [ + { + "name": "query-name", + "type": "Command", + "returns": [ + { + "key": "*name", + "type": "str" + } + ] + }, + ... + } + +The whole schema information will be returned in one go, it contains +commands and event. It doesn't support to be filtered by type or name. + +We have four types (ImageInfo, BlockStats, PciDeviceInfo, SchemaData) +that uses themself in their own define data directly or indirectly, +we will not repeatedly extend them to avoid dead loop. + +== more description of 'DataObject' type == + +We use 'DataObject' to describe dynamical data struct, it might be +nested dictionary, list or string. + +'DataObject' itself is a arbitrary and nested dictionary, the +dictionary has three keys ('key', 'type', 'data'), 'key' and +'data' are optional. + +* For describing Dictionary, we set the key to 'key', and set the + value to 'type' +* For describing List, we don't set 'key', just set the value to + 'type' +* If the value of dictionary or list is non-native type, we extend + the non-native type to dictionary, set it to 'data', and set the + non-native type's name to 'type'. +* If the value of dictionary or list is dictionary or list, 'type' + won't be set. + +== examples == + +1) Dict, value is native type +{ 'id': 'str', ... } +-------------------- +[ + { + "key": "id", + "type": "str" + }, + ..... +] + +2) Dict, value is defined types +{ 'options': 'TpmTypeOptions' } +------------------------------- +[ + { + "key": "options", + "type": "TpmTypeOptions", + "data": [ + { + "key": "passthrough", + "type": "str", + } + ] + }, + ..... +] + +3) List, value is native type +['str', ... ] +------------- +[ + { + "type": "str" + }, + .... +] + +4) List, value is defined types +['TpmTypeOptions', ... ] +------------------------ +[ + { + "type": "TpmTypeOptions", + "data": [ + { + "key": "passthrough", + "type": "str", + } + ] + }, + ..... +] + +5) Dict, value is dictionary +{ 'info': { 'age': 'init', ... } } +----------------------------- +[ + { + "key": "info", + "data": [ + { + "key": "age", + "type": "init", + }, + ... + ] + }, +] + +6) Dict, value is list +{ 'info': [ 'str', ... } } +----------------------------- +[ + { + "key": "info", + "data": [ + { + "type": "str", + }, + ... + ] + }, +] diff --git a/qapi-schema.json b/qapi-schema.json index 7b9fef1..cf03391 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -3679,3 +3679,72 @@ '*cpuid-input-ecx': 'int', 'cpuid-register': 'X86CPURegister32', 'features': 'int' } } + +## +# @DataObject +# +# Details of a data object, it can be nested dictionary/list +# +# @key: #optional Data object key +# +# @type: Data object type name +# +# @optional: #optional whether the data object is optional +# +# @data: #optional DataObject list, can be a dictionary or list type +# +# Since: 1.6 +## +{ 'type': 'DataObject', + 'data': { '*key': 'str', '*type': 'str', '*optional': 'bool', '*data': ['DataObject'] } } + +## +# @SchemaMetaType +# +# Possible meta types of a schema entry +# +# @Command: QMP command +# +# @Type: defined new data type +# +# @Enumeration: enumeration data type +# +# @Union: union data type +# +# Since: 1.6 +## +{ 'enum': 'SchemaMetaType', + 'data': ['Command', 'Type', 'Enumeration', 'Union'] } + +## +# @SchemaEntry +# +# Details of schema items +# +# @type: Entry's type in string format +# +# @name: Entry name +# +# @data: #optional list of DataObject. This can have different meaning +# depending on the 'type' value. For example, for a QMP command, +# this member contains an argument listing. For an enumeration, +# it contains the enum's values and so on +# +# @returns: #optional list of DataObject, return data after executing +# QMP command +# +# Since: 1.6 +## +{ 'type': 'SchemaEntry', 'data': { 'type': 'SchemaMetaType', + 'name': 'str', '*data': ['DataObject'], '*returns': ['DataObject'] } } + +## +# @query-qmp-schema +# +# Query QMP schema information +# +# Returns: list of @SchemaEntry. Returns an error if json string is invalid. +# +# Since: 1.6 +## +{ 'command': 'query-qmp-schema', 'returns': ['SchemaEntry'] } diff --git a/qmp-commands.hx b/qmp-commands.hx index e075df4..e3cbe93 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -3047,3 +3047,42 @@ Example: <- { "return": {} } 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', 'event' +- "data": schema data (json-object, optional) +- "returns": return data of qmp command (json-object, optional) + +Example: + +-> { "execute": "query-qmp-schema" } +<- { "return": [ + { + "name": "query-name", + "type": "Command", + "returns": [ + { + "key": "*name", + "type": "str" + } + ] + } + ] + } + +EQMP diff --git a/qmp.c b/qmp.c index 4c149b3..3ace3a6 100644 --- a/qmp.c +++ b/qmp.c @@ -25,6 +25,8 @@ #include "sysemu/blockdev.h" #include "qom/qom-qobject.h" #include "hw/boards.h" +#include "qmp-schema.h" +#include "qapi/qmp/qjson.h" NameInfo *qmp_query_name(Error **errp) { @@ -486,6 +488,311 @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp) return arch_query_cpu_definitions(errp); } +/* + * Use a string to record the visit path, type index of each node + * will be saved to the string, indexes are split by ':'. + */ +static char visit_path_str[1024]; + +/* push the type index to visit_path_str */ +static void push_id(int id) +{ + char *end = strrchr(visit_path_str, ':'); + char type_idx[256]; + int num; + + num = sprintf(type_idx, "%d:", id); + + if (end) { + /* avoid overflow */ + assert(end - visit_path_str + 1 + num < sizeof(visit_path_str)); + sprintf(end + 1, "%d:", id); + } else { + sprintf(visit_path_str, "%d:", id); + } +} + +/* pop the type index from visit_path_str */ +static void pop_id(void) +{ + char *p = strrchr(visit_path_str, ':'); + + assert(p != NULL); + *p = '\0'; + p = strrchr(visit_path_str, ':'); + if (p) { + *(p + 1) = '\0'; + } else { + visit_path_str[0] = '\0'; + } +} + +static const char *qstring_copy_str(QObject *data) +{ + QString *qstr; + + if (!data) { + return NULL; + } + qstr = qobject_to_qstring(data); + if (qstr) { + return qstring_get_str(qstr); + } else { + return NULL; + } +} + +static DataObjectList *visit_qobj_dict(QObject *data); +static DataObjectList *visit_qobj_list(QObject *data); + +/* extend defined type to json object */ +static DataObjectList *extend_type(const char* str) +{ + DataObjectList *data_list; + QObject *data; + QDict *qdict; + const QDictEntry *ent; + int i; + + /* don't extend builtin types */ + if (!strcmp(str, "str") || !strcmp(str, "int") || + !strcmp(str, "number") || !strcmp(str, "bool") || + !strcmp(str, "int8") || !strcmp(str, "int16") || + !strcmp(str, "int32") || !strcmp(str, "int64") || + !strcmp(str, "uint8") || !strcmp(str, "uint16") || + !strcmp(str, "uint32") || !strcmp(str, "uint64")) { + return NULL; + } + + for (i = 0; qmp_schema_table[i]; i++) { + data = qobject_from_json(qmp_schema_table[i]); + assert(data != NULL); + + qdict = qobject_to_qdict(data); + assert(qdict != NULL); + + ent = qdict_first(qdict); + if (!qdict_get(qdict, "enum") && !qdict_get(qdict, "type") + && !qdict_get(qdict, "union")) { + continue; + } + + if (!strcmp(str, qstring_copy_str(ent->value))) { + char *start, *end; + char cur_idx[256]; + char type_idx[256]; + + start = visit_path_str; + sprintf(type_idx, "%d", i); + while(start) { + end = strchr(start, ':'); + if (!end) { + break; + } + snprintf(cur_idx, end - start + 1, "%s", start); + start = end + 1; + /* if the type was already extended in parent node, + * we don't extend it again to avoid dead loop. */ + if (!strcmp(cur_idx, type_idx)) { + return NULL; + } + } + /* push index to visit_path_str before extending */ + push_id(i); + + data = qdict_get(qdict, "data"); + if(data) { + if (data->type->code == QTYPE_QDICT) { + data_list = visit_qobj_dict(data); + } else if (data->type->code == QTYPE_QLIST) { + data_list = visit_qobj_list(data); + } + /* pop index from visit_path_str after extending */ + pop_id(); + + return data_list; + } + } + } + + return NULL; +} + +static DataObjectList *visit_qobj_list(QObject *data) +{ + DataObjectList *obj_list, *obj_last_entry, *obj_entry; + DataObject *obj_info; + const QListEntry *ent; + QList *qlist; + + qlist = qobject_to_qlist(data); + assert(qlist != NULL); + + obj_list = NULL; + for (ent = qlist_first(qlist); ent; ent = qlist_next(ent)) { + obj_info = g_malloc0(sizeof(*obj_info)); + obj_entry = g_malloc0(sizeof(*obj_entry)); + obj_entry->value = obj_info; + obj_entry->next = NULL; + + if (!obj_list) { + obj_list = obj_entry; + } else { + obj_last_entry->next = obj_entry; + } + obj_last_entry = obj_entry; + + if (ent->value->type->code == QTYPE_QDICT) { + obj_info->data = visit_qobj_dict(ent->value); + } else if (ent->value->type->code == QTYPE_QLIST) { + obj_info->data = visit_qobj_list(ent->value); + } else if (ent->value->type->code == QTYPE_QSTRING) { + obj_info->has_type = true; + obj_info->type = g_strdup(qstring_copy_str(ent->value)); + obj_info->data = extend_type(qstring_copy_str(ent->value)); + } + if (obj_info->data) { + obj_info->has_data = true; + } + } + + return obj_list; +} + +static DataObjectList *visit_qobj_dict(QObject *data) +{ + DataObjectList *obj_list, *obj_last_entry, *obj_entry; + DataObject *obj_info; + const QDictEntry *ent; + QDict *qdict; + + qdict = qobject_to_qdict(data); + assert(qdict != NULL); + + obj_list = NULL; + for (ent = qdict_first(qdict); ent; ent = qdict_next(qdict, ent)) { + obj_info = g_malloc0(sizeof(*obj_info)); + obj_entry = g_malloc0(sizeof(*obj_entry)); + obj_entry->value = obj_info; + obj_entry->next = NULL; + + if (!obj_list) { + obj_list = obj_entry; + } else { + obj_last_entry->next = obj_entry; + } + obj_last_entry = obj_entry; + + if (ent->key[0] == '*') { + obj_info->key = g_strdup(ent->key + 1); + obj_info->has_optional = true; + obj_info->optional = true; + } else { + obj_info->key = g_strdup(ent->key); + } + obj_info->has_key = true; + + if (ent->value->type->code == QTYPE_QDICT) { + obj_info->data = visit_qobj_dict(ent->value); + } else if (ent->value->type->code == QTYPE_QLIST) { + obj_info->data = visit_qobj_list(ent->value); + } else if (ent->value->type->code == QTYPE_QSTRING) { + obj_info->has_type = true; + obj_info->type = g_strdup(qstring_copy_str(ent->value)); + obj_info->data = extend_type(qstring_copy_str(ent->value)); + } + if (obj_info->data) { + obj_info->has_data = true; + } + } + + return obj_list; +} + +SchemaEntryList *qmp_query_qmp_schema(Error **errp) +{ + SchemaEntryList *list, *last_entry, *entry; + SchemaEntry *info; + DataObjectList *obj_entry; + DataObject *obj_info; + QObject *data; + QDict *qdict; + int i; + + list = NULL; + for (i = 0; qmp_schema_table[i]; i++) { + data = qobject_from_json(qmp_schema_table[i]); + assert(data != NULL); + + qdict = qobject_to_qdict(data); + assert(qdict != NULL); + + if (qdict_get(qdict, "command")) { + info = g_malloc0(sizeof(*info)); + info->type = SCHEMA_META_TYPE_COMMAND; + info->name = strdup(qdict_get_str(qdict, "command")); + } else { + continue; + } + + memset(visit_path_str, 0, sizeof(visit_path_str)); + data = qdict_get(qdict, "data"); + if (data) { + info->has_data = true; + if (data->type->code == QTYPE_QLIST) { + info->data = visit_qobj_list(data); + } else if (data->type->code == QTYPE_QDICT) { + info->data = visit_qobj_dict(data); + } else if (data->type->code == QTYPE_QSTRING) { + info->data = extend_type(qstring_get_str(qobject_to_qstring(data))); + if (!info->data) { + obj_info = g_malloc0(sizeof(*obj_info)); + obj_entry = g_malloc0(sizeof(*obj_entry)); + obj_entry->value = obj_info; + obj_info->has_type = true; + obj_info->type = g_strdup(qdict_get_str(qdict, "data")); + info->data = obj_entry; + } + } else { + abort(); + } + } + + memset(visit_path_str, 0, sizeof(visit_path_str)); + data = qdict_get(qdict, "returns"); + if (data) { + info->has_returns = true; + if (data->type->code == QTYPE_QLIST) { + info->returns = visit_qobj_list(data); + } else if (data->type->code == QTYPE_QDICT) { + info->returns = visit_qobj_dict(data); + } else if (data->type->code == QTYPE_QSTRING) { + info->returns = extend_type(qstring_copy_str(data)); + if (!info->returns) { + obj_info = g_malloc0(sizeof(*obj_info)); + obj_entry = g_malloc0(sizeof(*obj_entry)); + obj_entry->value = obj_info; + obj_info->has_type = true; + obj_info->type = g_strdup(qdict_get_str(qdict, "returns")); + info->returns = obj_entry; + } + } + } + + entry = g_malloc0(sizeof(DataObjectList *)); + entry->value = info; + entry->next = NULL; + if (!list) { + list = entry; + } else { + last_entry->next = entry; + } + last_entry = entry; + } + + return list; +} + void qmp_add_client(const char *protocol, const char *fdname, bool has_skipauth, bool skipauth, bool has_tls, bool tls, Error **errp)
Introduces new monitor command to query QMP schema information, the return data is a dynamical and nested dict/list, it contains the useful metadata to help management to check feature support, QMP commands detail, etc. I added a document for QMP introspection support. (docs/qmp-full-introspection.txt) We need to parse all commands json definition, and generated a dynamical tree, QMP infrastructure will convert the tree to json string and return to QMP client. So here I defined a 'DataObject' type in qapi-schema.json, it's used to describe the dynamical dictionary/list/string. { 'type': 'DataObject', 'data': { '*key': 'str', '*type': 'str', '*data': ['DataObject'] } } Not all the keys in data will be used. # List: type # Dict: key, type # nested List: type, data # nested Dict: key, type, data The DataObject is described in docs/qmp-full-introspection.txt in detail. The following content gives an example of query-tpm-types: ## Define example in qapi-schema.json: { 'enum': 'TpmType', 'data': [ 'passthrough' ] } { 'command': 'query-tpm-types', 'returns': ['TpmType'] } ## Returned description: { "name": "query-tpm-types", "type": "Command", "returns": [ { "type": "TpmType", "data": [ { "type": "passthrough" } ] } ] }, 'TpmType' is a defined type, it will be extended in returned description. [ 'passthrough' ] is a list, so 'type' and 'data' will be used. TODO: We can add events definations to qapi-schema.json by another patch, then event can also be queried. Introduce another command 'query-qga-schema' to query QGA schema information, it's easy to add this support with current current patch. Signed-off-by: Amos Kong <akong@redhat.com> --- docs/qmp-full-introspection.txt | 143 +++++++++++++++++++ qapi-schema.json | 69 +++++++++ qmp-commands.hx | 39 +++++ qmp.c | 307 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 558 insertions(+) create mode 100644 docs/qmp-full-introspection.txt