[libvirt] [Qemu-devel] [PATCH v4 4/5] qmp: full introspection support for QMP
Amos Kong
akong at redhat.com
Mon Jan 27 08:17:56 UTC 2014
CC Libvirt-list
Original discussion:
http://marc.info/?l=qemu-devel&m=139048842504757&w=2
[Qemu-devel] [PATCH v4 0/5] QMP full introspection
On Fri, Jan 24, 2014 at 06:48:31PM +0800, Fam Zheng wrote:
> On Thu, 01/23 22:46, Amos Kong wrote:
> > This patch introduces a new monitor command to query QMP schema
> > information, the return data is a range of schema structs, which
> > contains the useful metadata to help management to check supported
> > features, QMP commands detail, etc.
> >
> > We use qapi-introspect.py to parse all json definition in
> > qapi-schema.json, and generate a range of dictionaries with metadata.
> > The query command will visit the dictionaries and fill the data
> > to allocated struct tree. Then QMP infrastructure will convert
> > the tree to json string and return to QMP client.
> >
> > TODO:
> > Wenchao Xia is working to convert QMP events to qapi-schema.json,
> > then event can also be queried by this interface.
> >
> > I will introduce another command 'query-qga-schema' to query QGA
> > schema information, it's easy to add this support based on this
> > patch.
> >
> > Signed-off-by: Amos Kong <akong at redhat.com>
> > ---
> > qapi-schema.json | 11 +++
> > qmp-commands.hx | 42 +++++++++++
> > qmp.c | 215 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 268 insertions(+)
> >
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index c63f0ca..6033383 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -4411,3 +4411,14 @@
> > 'reference-type': 'String',
> > 'type': 'DataObjectType',
> > 'unionobj': 'DataObjectUnion' } }
> > +
> > +##
> > +# @query-qmp-schema
> > +#
> > +# Query QMP schema information
> > +#
> > +# @returns: list of @DataObject
> > +#
> > +# Since: 1.8
> > +##
> > +{ 'command': 'query-qmp-schema', 'returns': ['DataObject'] }
> > diff --git a/qmp-commands.hx b/qmp-commands.hx
> > index 02cc815..b83762d 100644
> > --- a/qmp-commands.hx
> > +++ b/qmp-commands.hx
> > @@ -3291,6 +3291,48 @@ Example:
> > }
> >
> > EQMP
> > + {
> > + .name = "query-qmp-schema",
> > + .args_type = "",
> > + .mhandler.cmd_new = qmp_marshal_input_query_qmp_schema,
> > + },
> > +
> > +
> > +SQMP
> > +query-qmp-schema
> > +----------------
> > +
> > +query qmp schema information
> > +
> > +Return a json-object with the following information:
> > +
> > +- "name": qmp schema name (json-string)
> > +- "type": qmp schema type, it can be 'comand', 'type', 'enum', 'union'
> > +- "returns": return data of qmp command (json-object, optional)
> > +
> > +Example:
> > +
> > +-> { "execute": "query-qmp-schema" }
> > +-> { "return": [
> > + {
> > + "name": "query-name",
> > + "type": "command",
> > + "returns": {
> > + "name": "NameInfo",
> > + "type": "type",
> > + "data": [
> > + {
> > + "name": "name",
> > + "optional": true,
> > + "recursive": false,
> > + "type": "str"
> > + }
> > + ]
> > + }
> > + }
> > + }
> > +
> > +EQMP
> >
> > {
> > .name = "blockdev-add",
> > diff --git a/qmp.c b/qmp.c
> > index 0f46171..a64ae6d 100644
> > --- a/qmp.c
> > +++ b/qmp.c
> > @@ -27,6 +27,8 @@
> > #include "qapi/qmp/qobject.h"
> > #include "qapi/qmp-input-visitor.h"
> > #include "hw/boards.h"
> > +#include "qapi/qmp/qjson.h"
> > +#include "qapi-introspect.h"
> >
> > NameInfo *qmp_query_name(Error **errp)
> > {
> > @@ -488,6 +490,219 @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
> > return arch_query_cpu_definitions(errp);
> > }
> >
> > +static strList *qobject_to_strlist(QObject *data)
> > +{
> > + strList *list = NULL;
> > + strList **plist = &list;
> > + QList *qlist;
> > + const QListEntry *lent;
> > +
> > + qlist = qobject_to_qlist(data);
> > + for (lent = qlist_first(qlist); lent; lent = qlist_next(lent)) {
> > + strList *entry = g_malloc0(sizeof(strList));
> > + entry->value = g_strdup(qobject_get_str(lent->value));
> > + *plist = entry;
> > + plist = &entry->next;
> > + }
> > +
> > + return list;
> > +}
> > +
> > +static DataObject *qobject_to_dataobj(QObject *data);
> > +
> > +static DataObjectMember *qobject_to_dataobjmem(QObject *data)
> > +{
> > +
> > + DataObjectMember *member = g_malloc0(sizeof(DataObjectMember));
> > +
> > + member->type = g_malloc0(sizeof(DataObjectMemberType));
> > + if (data->type->code == QTYPE_QDICT) {
> > + member->type->kind = DATA_OBJECT_MEMBER_TYPE_KIND_EXTEND;
> > + member->type->extend = qobject_to_dataobj(data);
> > + } else {
> > + member->type->kind = DATA_OBJECT_MEMBER_TYPE_KIND_REFERENCE;
> > + member->type->reference = g_strdup(qobject_get_str(data));
> > + }
> > +
> > + return member;
> > +}
> > +
> > +static DataObjectMemberList *qobject_to_dict_memlist(QObject *data)
> > +{
> > + DataObjectMemberList *list = NULL;
> > + DataObjectMemberList **plist = &list;
> > + QDict *qdict = qobject_to_qdict(data);
> > + const QDictEntry *dent;
> > +
> > + for (dent = qdict_first(qdict); dent; dent = qdict_next(qdict, dent)) {
> > + DataObjectMemberList *entry = g_malloc0(sizeof(DataObjectMemberList));
> > + entry->value = qobject_to_dataobjmem(dent->value);
> > +
> > + entry->value->has_optional = true;
> > + entry->value->has_name = true;
> > + if (dent->key[0] == '*') {
> > + entry->value->optional = true;
> > + entry->value->name = g_strdup(dent->key + 1);
> > + } else {
> > + entry->value->name = g_strdup(dent->key);
> > + }
> > + *plist = entry;
> > + plist = &entry->next;
> > + }
> > +
> > + return list;
> > +}
> > +
> > +static DataObjectMemberList *qobject_to_list_memlist(QObject *data)
> > +{
> > + const QListEntry *lent;
> > + DataObjectMemberList *list = NULL;
> > + DataObjectMemberList **plist = &list;
> > + QList *qlist = qobject_to_qlist(data);
> > +
> > + for (lent = qlist_first(qlist); lent; lent = qlist_next(lent)) {
> > + DataObjectMemberList *entry = g_malloc0(sizeof(DataObjectMemberList));
> > + entry->value = qobject_to_dataobjmem(lent->value);
> > + entry->value->has_optional = true;
> > + entry->value->has_name = true;
> > + *plist = entry;
> > + plist = &entry->next;
> > + }
> > +
> > + return list;
> > +}
> > +
> > +static DataObjectMemberList *qobject_to_memlist(QObject *data)
>
> This whole converting is cumbersome. You already did all the traversing through
> the type jungle in python when generating this, it's not necessary to do the
> similar thing again here.
We can parse raw schemas and generate json string table, we can't
directly return the string / qobject to monitor, C code has to convert
the json to qobject, we have to revisit the qobject and convert them
to DataObject/DataObjectMember/DataObject... structs.
> Alternatively, I think we have a good reason to extend QMP framework as
> necessary here, as we are doing "QMP introspection", which is a part of the
> framework:
>
> * Define final output into qmp_schema_table[], no need to box it like:
>
> "{'_obj_member': 'False', '_obj_type': 'enum', '_obj_name':
> 'ErrorClass', '_obj_data': {'data': ...
>
> just put it content of "qmp-introspection.output.txt" as a long string in
> the header,
> like you would generate in qobject_to_memlist:
>
> const char *qmp_schema_table =
> "{ 'name': 'ErrorClass', 'type': 'enumeration', 'data': [...]},"
> "{ 'name': '...', ...},"
The keys are used for metadata might be 'recursive', 'optional', etc.
It might exists problem in namespace, let's use '_obj_' or '_' prefix
for the metadata keys.
I used a nested dictionary to describe a DataObject, because we can
store the metadata and definition to different level, it's helpful
in parse the output by Libvirt.
example:
"{ 'type': 'NameInfo', 'data': {'*name': 'str', '*job': 'str'} }"
It's good to store _type, _name, data to same level, but the metadata
of items of data's value dictionary can't be appended to same level.
"{ '_name': 'NameInfo', '_type': 'type',
'data': {
'name': 'str', '_name_optional': 'True',
'job': 'str', '_job_optional': 'True'
}
}"
A better solution, but I don't know if it will cause trouble for
Libvirt to parse the output.
"{'_type': 'type', '_name': 'NameInfo',
'data': { 'job': {'_value': 'str', '_recursive': 'True'},
'name': {'_value': 'str', '_recursive': 'True'}
},
'_recursive': 'False'
}"
When we describe a DataObject (dict/list/str, one schema, extened
schema, schema member, etc), so I we generally use a nested dictionary
to describe a DataObject, it will split the metadata with original
data two different dictionary level, it's convenient for parse of
Libvirt. Here I just use a dict member as an example, actually
it's more complex to parse all kinds of data objects.
> ...
> ;
>
> * Add a new type of qmp command, that returns a QString as a json literal.
> query-qmp-schema is defined as this type. (This wouldn't be much code, but
> may be abused in the future, I'm afraid. However we can review, limit its
> use to introspection only)
>
> * And return qmp_schema_table from query-qmp-shema, which will be copied to
> the wire.
>
> Feel free to disagree, it's not a perfect solution. But I really think we need
> to avoid duplicating "enum", "base", "type", "union", "discriminator", ...
In the past, we didn't consider to extend and add metadata by Python, so
Libvirt wasn't happy to just get a raw schema(not extended, no metadata).
But now, we already successfully to transfer this work to Python, and
we can adjust to make management to be happy for the metadata and
format.
The problem is that Libvirt should parse the output twice, the json
items are also json object string.
Eric, Is it acceptabled?
Example:
* as suggested by Fam to put the metadta with schema data in same
dict level
* process some special cases by nested dictionary
(eg: '*tls': 'bool' ==> 'tls': {'_value': 'bool', '_optional': 'True'} )
* return a strList to Libvirt, the content of string is json object,
that contains the metadata.
{
"return": [
"{'_type': 'enum', '_name': 'ErrorClass', 'data': ['GenericError', 'CommandNotFound', 'DeviceEncrypted', 'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap'], '_recursive': 'False'}",
"{'_type': 'command', '_name': 'add_client', 'data': {'tls': {'_value': 'bool', '_optional': 'True'}, 'skipauth': {'_value': 'bool', '_optional': 'True'}, 'protocol': 'str', 'fdname': 'str'}, '_recursive': 'False'}",
"{'_type': 'type', '_name': 'NameInfo', 'data': {'job': {'_value': 'str', '_optional': 'True'}, 'name': {'_value': 'str', '_optional': 'True'}}, '_recursive': 'False'}",
"{'returns': {'_type': 'type', '_recursive': 'False', 'data': {'job': {'_value': 'str', '_optional': 'True'}, 'name': {'_value': 'str', '_optional': 'True'}}, '_name': 'NameInfo'}, '_name': 'query-name', '_type': 'command', '_recursive': 'False'}",
"......",
"......",
"......"
]
}
> Fam
--
Amos.
More information about the libvir-list
mailing list