From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48944) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W06cz-0004kG-LD for qemu-devel@nongnu.org; Mon, 06 Jan 2014 04:37:51 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W06cu-0007Pw-7S for qemu-devel@nongnu.org; Mon, 06 Jan 2014 04:37:45 -0500 Received: from mx1.redhat.com ([209.132.183.28]:64544) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W06ct-0007Pr-Rx for qemu-devel@nongnu.org; Mon, 06 Jan 2014 04:37:40 -0500 Message-ID: <52CA794F.7090505@redhat.com> Date: Mon, 06 Jan 2014 17:37:19 +0800 From: Fam Zheng MIME-Version: 1.0 References: <1388923351-10556-1-git-send-email-akong@redhat.com> <1388923351-10556-4-git-send-email-akong@redhat.com> In-Reply-To: <1388923351-10556-4-git-send-email-akong@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v3 3/3] qmp: full introspection support for QMP List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Amos Kong , qemu-devel@nongnu.org Cc: qiaonuohan@cn.fujitsu.com, lcapitulino@redhat.com, mdroth@linux.vnet.ibm.com, xiawenc@linux.vnet.ibm.com On 2014=E5=B9=B401=E6=9C=8805=E6=97=A5 20:02, 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. > > It parses all json definition in qapi-schema.json, and generate a > dynamic struct tree, QMP infrastructure will convert the tree to > json string and return to QMP client. > > I defined a 'DataObject' union in qapi-schema.json, it's used to > describe the dynamic data struct. > > I also added a document about QMP full introspection support > (docs/qmp-full-introspection.txt), it helps to use the new interface > and understand the abstract method in describing the dynamic struct. > > 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 > --- I have a few comments on the current implementation below, there are a=20 few things to improve. However I agree to what Eric suggested in reply=20 to V2: it may be better to generate most of the response data in python=20 code at compile time and simplify the logic in C. Because this=20 implementation is slow and it is unnecessary runtime computation. It=20 also duplicates much of existing qapi.py logic (data types and other=20 semantics parsing). > docs/qmp-full-introspection.txt | 97 ++++++++++ > qapi-schema.json | 150 ++++++++++++++++ > qmp-commands.hx | 43 ++++- > qmp.c | 382 +++++++++++++++++++++++++++++++= +++++++++ > 4 files changed, 671 insertions(+), 1 deletion(-) > create mode 100644 docs/qmp-full-introspection.txt > > diff --git a/docs/qmp-full-introspection.txt b/docs/qmp-full-introspect= ion.txt > new file mode 100644 > index 0000000..1617df7 > --- /dev/null > +++ b/docs/qmp-full-introspection.txt > @@ -0,0 +1,97 @@ > +=3D Full introspection support for QMP =3D > + > + > +=3D=3D Purpose =3D=3D > + > +Add a new monitor command for management to query QMP schema > +information, it returns a range of schema structs, which contain the > +useful metadata to help management to check supported features, QMP > +commands detail, etc. > + > +=3D=3D Usage =3D=3D > + > +Json schema: > + { 'type': 'NameInfo', 'data': {'*name': 'str'} } > + { 'command': 'query-name', 'returns': 'NameInfo' } > + > +Execute QMP command: > + > + { "execute": "query-qmp-schema" } > + > +Returns: > + > + { "return": [ > + { > + "name": "query-name", > + "type": "command", > + "returns": { > + "name": "NameInfo", > + "type": "type", > + "data": [ > + { > + "name": "name", > + "optional": true, > + "recursive": false, > + "type": "str" > + } > + ] > + } > + }, > + ... > + } > + > +The whole schema information will be returned in one go, it contains > +all the schema entries. It doesn't support to be filtered by type > +or name. Currently it takes about 5 seconds to return about 1.5M strin= g. > + > +=3D=3D 'DataObject' union =3D=3D > + > +{ 'union': 'DataObject', > + 'base': 'DataObjectBase', > + 'discriminator': 'type', > + 'data': { > + 'anonymous-struct': 'DataObjectAnonymousStruct', > + 'command': 'DataObjectCommand', > + 'enumeration': 'DataObjectEnumeration', > + 'reference-type': 'String', > + 'type': 'DataObjectType', > + 'unionobj': 'DataObjectUnion' } } > + > +Currently we have schema difinitions for type, command, enumeration, > +union. Some arbitrary structs (dictionary, list or string) and native > +types are also used in the body of definitions. > + > +Here we use "DataObject" union to abstract all above schema. 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. > + > +So we define 'DataObject' to be an union, it always has an object name > +except anonymous struct. > + > +'command', 'enumeration', 'type', 'unionobj' are common schema type, > +'union' is a build-in type, so I used unionobj here. > + > +'reference-type' will be used to describe native types and unextended > +types. > + > +'anonymous-struct' will be used to describe arbitrary structs > +(dictionary, list or string). > + > +=3D=3D Avoid dead loop in recursive extending =3D=3D > + > +We have four types (ImageInfo, BlockStats, PciDeviceInfo, ObjectData) > +that uses themself in their own define data directly or indirectly, > +we will not repeatedly extend them to avoid dead loop. > + > +We use a string to record the visit path, type index of each node > +will be saved to the string, indexes are split by ':'. > + > +Push index to visit_path_str before extending, and pop index from > +visit_path_str after extending. > + > +If the type was already extended in parent node, we don't extend it > +again to avoid dead loop. > + > +'recursive' indicates if the type is extended or not. > diff --git a/qapi-schema.json b/qapi-schema.json > index c3c939c..db500ab 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -4235,3 +4235,153 @@ > # Since: 1.7 > ## > { 'command': 'blockdev-add', 'data': { 'options': 'BlockdevOptions' }= } > + > +## > +# @DataObjectBase > +# > +# Base of @DataObject > +# > +# @name: #optional @DataObject name > +# @type: @DataObject type > +# > +# Since: 1.8 > +## > +{ 'type': 'DataObjectBase', > + 'data': { '*name': 'str', 'type': 'str' } } > + > +## > +# @DataObjectMemberType > +# > +# Type of @DabaObjectMember > +# > +# @reference: reference string > +# @anonymous: arbitrary struct > +# @extend: the @DataObjectMember > +# > +# Since: 1.8 > +## > +{ 'union': 'DataObjectMemberType', > + 'discriminator': {}, > + 'data': { 'reference': 'str', > + 'anonymous': 'DataObject', > + 'extend': 'DataObject' } } > + > +## > +# @DataObjectMember > +# > +# General member of @DataObject > +# > +# @type: type of @DataObjectMember > +# @name: #optional name > +# @optional: #optional key to indicate if the @DataObjectMember is opt= ional > +# @recursive: #optional key to indicate if it's defined recursively > +# > +# Since: 1.8 > +## > +{ 'type': 'DataObjectMember', > + 'data': { 'type': 'DataObjectMemberType', '*name': 'str', > + '*optional': 'bool', '*recursive': 'bool' } } > + > +## > +# @DataObjectAnonymousStruct > +# > +# Arbitrary struct, it can be dictionary, list or string > +# > +# @data: content of arbitrary struct > +# > +# Since: 1.8 > +## > +{ 'type': 'DataObjectAnonymousStruct', > + 'data': { 'data': [ 'DataObjectMember' ] } } > + > +## > +# @DataObjectCommand > +# > +# QMP Command schema > +# > +# @data: QMP command content > +# @returns: returns of executing command > +# @gen: a key to suppress code generation > +# > +# Since: 1.8 > +## > +{ 'type': 'DataObjectCommand', > + 'data': { '*data': [ 'DataObjectMember' ], > + '*returns': 'DataObject', > + '*gen': 'bool' } } > + > +## > +# @DataObjectEnumeration > +# > +# Enumeration schema > +# > +# @data: enumeration content, it's a string list > +# > +# Since: 1.8 > +## > +{ 'type': 'DataObjectEnumeration', > + 'data': { 'data': [ 'str' ] } } > + > +## > +# @DataObjectType > +# > +# Type schema > +# > +# @data: type content > +# > +# Since: 1.8 > +## > +{ 'type': 'DataObjectType', > + 'data': { 'data': [ 'DataObjectMember' ] } } > + > +## > +# @DataObjectUnion > +# > +# Union schema > +# > +# @data: union content > +# @base: union base > +# @discriminator: union discriminator > +# > +# Since: 1.8 > +## > +{ 'type': 'DataObjectUnion', > + 'data': { 'data': [ 'DataObjectMember' ], '*base': 'str', > + '*discriminator': 'str' } } > + > +## > +# @DataObject > +# > +# Dynamic data struct, it can be command, enumeration, type, union, ar= bitrary > +# struct or native type. > +# > +# @anonymous-struct: arbitrary struct, it can be dictionary, list or s= tring > +# @command: QMP command schema > +# @enumeration: enumeration schema > +# @reference-type: native type or unextended type > +# @type: type schema, it will be extended > +# @unionobj: union schema > +# > +# Since: 1.8 > +## > +{ 'union': 'DataObject', > + 'base': 'DataObjectBase', > + 'discriminator': 'type', > + 'data': { > + 'anonymous-struct': 'DataObjectAnonymousStruct', > + 'command': 'DataObjectCommand', > + 'enumeration': 'DataObjectEnumeration', > + '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 fba15cd..cf9c4aa 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -3237,7 +3237,48 @@ Example: > "broadcast-allowed": false > } > ] > - } > + > +EQMP > + { > + .name =3D "query-qmp-schema", > + .args_type =3D "", > + .mhandler.cmd_new =3D 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 > > diff --git a/qmp.c b/qmp.c > index 1d7a04d..e9bba06 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,386 @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(= Error **errp) > return arch_query_cpu_definitions(errp); > } > > +/* > + * use a passed string to record the visit path, schema index of > + * each extended node will be appended to the string, indexes are > + * split by ':' > + */ > +static char visit_path_str[1024]; Is it safe to use a global variable here? Is there any race condition=20 (i.e. multiple monitor commands run parallel)? IIUC this serves as a queue of number? Why not use array of int? > + > +/* push the type index into visit_path_str */ > +static void push_id(int id) > +{ > + char *end =3D strrchr(visit_path_str, ':'); > + char type_idx[256]; > + int num; > + > + num =3D 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 =3D strrchr(visit_path_str, ':'); > + > + assert(p !=3D NULL); > + *p =3D '\0'; > + p =3D strrchr(visit_path_str, ':'); > + if (p) { > + *(p + 1) =3D '\0'; > + } else { > + visit_path_str[0] =3D '\0'; > + } > +} > + > +static const char *qstring_copy_str(QObject *data) The string is not copied, it's misleading. And I think you could just=20 use qstring_get_str in the caller or define a new qstring_ util function=20 in qobject/qstring.c. > +{ > + QString *qstr; > + > + if (!data) { > + return NULL; > + } > + qstr =3D qobject_to_qstring(data); > + if (qstr) { > + return qstring_get_str(qstr); > + } else { > + return NULL; > + } > +} > + > +static QObject *get_definition(const char *str, bool update_path) > +{ > + QObject *data, *value; > + QDict *qdict; > + int i; > + > + 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") || > + !strcmp(str, "visitor") || !strcmp(str, "**") || > + !strcmp(str, "size")) { > + return NULL; > + } > + > + for (i =3D 0; qmp_schema_table[i]; i++) { > + data =3D qobject_from_json(qmp_schema_table[i]); > + qdict =3D qobject_to_qdict(data); > + assert(qdict !=3D NULL); > + > + if (qdict_get(qdict, "enum")) { > + value =3D qdict_get(qdict, "enum"); > + } else if (qdict_get(qdict, "type")) { > + value =3D qdict_get(qdict, "type"); > + } else if (qdict_get(qdict, "union")) { > + value =3D qdict_get(qdict, "union"); > + } else { > + continue; > + } > + > + if (!strcmp(str, qstring_copy_str(value)) && update_path) { Simply: if (strcmp(str, qstring_copy_str(value)) { continue; } Then avoid another check for this. > + char *start, *end; > + char cur_idx[256]; > + char type_idx[256]; > + > + start =3D visit_path_str; > + sprintf(type_idx, "%d", i); Let's avoid sprintf completely. > + while (start) { > + end =3D strchr(start, ':'); > + if (!end) { > + break; > + } > + snprintf(cur_idx, end - start + 1, "%s", start); > + start =3D 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; > + } > + } Using array of int will get you a simpler code here. Furthermore, you could avoid linear lookup by using a bitmap beside=20 visit_path_str to keep track of what is visited. > + /* push index to visit_path_str before extending */ > + push_id(i); > + } > + if (!strcmp(str, qstring_copy_str(value))) { > + > + return qobject_from_json(qmp_schema_table[i]); > + } > + } > + return NULL; > +} > + > +static DataObject *visit_qobj_dict(QObject *data); > +static DataObject *visit_qobj_list(QObject *data); > + > +/* extend defined type to json object */ > +static DataObject *extend_type(const char *str) > +{ > + QObject *data; > + DataObject *obj; > + > + data =3D get_definition(str, true); > + > + if (data) { > + obj =3D visit_qobj_dict(data); > + pop_id(); > + } else { > + obj =3D g_malloc0(sizeof(struct DataObject)); > + obj->kind =3D DATA_OBJECT_KIND_REFERENCE_TYPE; > + obj->reference_type =3D g_malloc0(sizeof(String)); > + obj->reference_type->str =3D g_strdup(str); > + } > + > + return obj; > +} > + > +static DataObjectMemberList *list_to_memberlist(QObject *data) > +{ > + DataObjectMemberList *mem_list, *entry, *last_entry; > + QList *qlist; > + const QListEntry *lent; > + > + qlist =3D qobject_to_qlist(data); Bad indentation starting from here. > + > + mem_list =3D NULL; > + for (lent =3D qlist_first(qlist); lent; lent =3D qlist_next(le= nt)) { > + entry =3D g_malloc0(sizeof(DataObjectMemberList *)); Pointer size allocated instead of structure. > + entry->value =3D g_malloc0(sizeof(DataObjectMember)); > + entry->value->type =3D g_malloc0(sizeof(DataObjectMemberTy= pe)); > + > + if (get_definition(qstring_copy_str(lent->value), false)) = { > + entry->value->type->kind =3D DATA_OBJECT_MEMBER_TYPE_K= IND_EXTEND; > + entry->value->has_recursive =3D true; > + entry->value->recursive =3D true; > + entry->value->type->extend =3D > + extend_type(qstring_copy_str(lent->value)); > + } else { > + entry->value->type->kind =3D > + DATA_OBJECT_MEMBER_TYPE_KIND_REFERENCE; > + entry->value->has_recursive =3D true; > + entry->value->recursive =3D false; > + entry->value->type->reference =3D > + g_strdup(qstring_copy_str(lent->value)); > + } > + > + entry->next =3D NULL; > + if (!mem_list) { > + mem_list =3D entry; > + } else { > + last_entry->next =3D entry; > + } > + last_entry =3D entry; > + } > + return mem_list; > +} > + > +static DataObjectMemberList *dict_to_memberlist(QObject *data) > +{ > + DataObjectMemberList *mem_list, *entry, *last_entry; > + QDict *qdict; > + const QDictEntry *dent; > + > + qdict =3D qobject_to_qdict(data); > + > + mem_list =3D NULL; > + for (dent =3D qdict_first(qdict); dent; dent =3D qdict_next(qd= ict, dent)) { > + entry =3D g_malloc0(sizeof(DataObjectMemberList *)); Same here. > + entry->value =3D g_malloc0(sizeof(DataObjectMember)); > + > + entry->value->type =3D g_malloc0(sizeof(DataObjectMemberTy= pe)); > + > + if (dent->value->type->code =3D=3D QTYPE_QDICT) { > + entry->value->type->kind =3D DATA_OBJECT_MEMBER_TYPE_K= IND_EXTEND; > + entry->value->type->extend =3D visit_qobj_dict(dent->v= alue); > + } else if (dent->value->type->code =3D=3D QTYPE_QLIST) { > + entry->value->type->kind =3D DATA_OBJECT_MEMBER_TYPE_K= IND_EXTEND; > + entry->value->type->extend =3D visit_qobj_list(dent->v= alue); > + } else if (get_definition(qstring_copy_str(dent->value), f= alse)) { > + entry->value->type->kind =3D DATA_OBJECT_MEMBER_TYPE_K= IND_EXTEND; > + entry->value->has_recursive =3D true; > + entry->value->recursive =3D true; > + entry->value->type->extend =3D > + extend_type(qstring_copy_str(dent->value)); > + } else { > + entry->value->type->kind =3D > + DATA_OBJECT_MEMBER_TYPE_KIND_REFERENCE; > + entry->value->has_recursive =3D true; > + entry->value->recursive =3D false; > + entry->value->type->reference =3D > + g_strdup(qstring_copy_str(dent->value)); > + } > + entry->value->has_optional =3D true; > + entry->value->has_name =3D true; > + if (dent->key[0] =3D=3D '*') { > + entry->value->optional =3D true; > + entry->value->name =3D g_strdup(dent->key + 1); > + } else { > + entry->value->name =3D g_strdup(dent->key); > + } > + > + entry->next =3D NULL; > + if (!mem_list) { > + mem_list =3D entry; > + } else { > + last_entry->next =3D entry; > + } > + last_entry =3D entry; > + } > + return mem_list; > +} > + > +static DataObject *visit_qobj_list(QObject *data) > +{ > + DataObject *obj; > + > + obj =3D g_malloc0(sizeof(struct DataObject)); > + obj->kind =3D DATA_OBJECT_KIND_ANONYMOUS_STRUCT; > + obj->anonymous_struct =3D g_malloc0(sizeof(struct > + DataObjectAnonymousStruct= )); > + obj->anonymous_struct->data =3D list_to_memberlist(data); > + > + return obj; > +} > + > +static strList *get_str_list(QObject *data) > +{ > + strList *str_list, *last_str_entry, *str_entry; > + QList *qlist; > + const QListEntry *lent; > + > + qlist =3D qobject_to_qlist(data); > + str_list =3D NULL; > + for (lent =3D qlist_first(qlist); lent; lent =3D qlist_next(lent))= { > + str_entry =3D g_malloc0(sizeof(strList *)); And here. > + str_entry->value =3D g_strdup(qstring_copy_str(lent->value)); > + str_entry->next =3D NULL; > + if (!str_list) { > + str_list =3D str_entry; > + } else { > + last_str_entry->next =3D str_entry; > + } > + last_str_entry =3D str_entry; > + } > + > + return str_list; > +} > + > +static DataObject *visit_qobj_dict(QObject *data) > +{ > + DataObject *obj; > + QObject *subdata; > + QDict *qdict; > + > + qdict =3D qobject_to_qdict(data); > + assert(qdict !=3D NULL); > + obj =3D g_malloc0(sizeof(*obj)); > + > + if (qdict_get(qdict, "command")) { > + obj->kind =3D DATA_OBJECT_KIND_COMMAND; > + obj->has_name =3D true; > + obj->name =3D g_strdup(qstring_copy_str(qdict_get(qdict, "comm= and"))); > + obj->command =3D g_malloc0(sizeof(struct DataObjectCommand)); > + > + subdata =3D qdict_get(qdict, "data"); > + if (subdata && subdata->type->code =3D=3D QTYPE_QDICT) { > + obj->command->has_data =3D true; > + obj->command->data =3D dict_to_memberlist(subdata); > + } else if (subdata && subdata->type->code =3D=3D QTYPE_QLIST) = { > + abort(); > + } else if (subdata) { > + obj->command->has_data =3D true; > + obj->command->data =3D > + dict_to_memberlist(get_definition(qstring_copy_str(sub= data), > + true)); > + pop_id(); > + } > + > + subdata =3D qdict_get(qdict, "returns"); > + if (subdata && subdata->type->code =3D=3D QTYPE_QDICT) { > + abort(); > + } else if (subdata && subdata->type->code =3D=3D QTYPE_QLIST) = { > + obj->command->has_returns =3D true; > + obj->command->returns =3D visit_qobj_list(subdata); > + } else if (subdata && subdata->type->code =3D=3D QTYPE_QSTRING= ) { > + obj->command->has_returns =3D true; > + obj->command->returns =3D extend_type(qstring_copy_str(sub= data)); > + } > + > + subdata =3D qdict_get(qdict, "gen"); > + if (subdata && subdata->type->code =3D=3D QTYPE_QSTRING) { > + obj->command->has_gen =3D true; > + if (!strcmp(qstring_copy_str(subdata), "no")) { > + obj->command->gen =3D false; > + } else { > + obj->command->gen =3D true; > + } > + } > + } else if (qdict_get(qdict, "union")) { > + obj->kind =3D DATA_OBJECT_KIND_UNIONOBJ; > + obj->has_name =3D true; > + obj->name =3D g_strdup(qstring_copy_str(qdict_get(qdict, "unio= n"))); > + obj->unionobj =3D g_malloc0(sizeof(struct DataObjectUnion)); > + subdata =3D qdict_get(qdict, "data"); > + obj->unionobj->data =3D dict_to_memberlist(subdata); > + } else if (qdict_get(qdict, "type")) { > + obj->kind =3D DATA_OBJECT_KIND_TYPE; > + obj->has_name =3D true; > + obj->name =3D g_strdup(qstring_copy_str(qdict_get(qdict, "type= "))); > + obj->type =3D g_malloc0(sizeof(struct DataObjectType)); > + subdata =3D qdict_get(qdict, "data"); > + obj->type->data =3D dict_to_memberlist(subdata); > + } else if (qdict_get(qdict, "enum")) { > + obj->kind =3D DATA_OBJECT_KIND_ENUMERATION; > + obj->has_name =3D true; > + obj->name =3D g_strdup(qstring_copy_str(qdict_get(qdict, "enum= "))); > + obj->enumeration =3D g_malloc0(sizeof(struct DataObjectEnumera= tion)); > + subdata =3D qdict_get(qdict, "data"); > + obj->enumeration->data =3D get_str_list(subdata); > + } else { > + obj->kind =3D DATA_OBJECT_KIND_ANONYMOUS_STRUCT; > + obj->anonymous_struct =3D g_malloc0(sizeof(struct > + DataObjectAnonymousSt= ruct)); > + obj->anonymous_struct->data =3D dict_to_memberlist(data); > + } > + > + return obj; > +} > + > +DataObjectList *qmp_query_qmp_schema(Error **errp) > +{ > + DataObjectList *list, *last_entry, *entry; > + QObject *data; > + int i; > + > + list =3D NULL; > + for (i =3D 0; qmp_schema_table[i]; i++) { > + data =3D qobject_from_json(qmp_schema_table[i]); > + assert(data !=3D NULL); > + > + entry =3D g_malloc0(sizeof(DataObjectList *)); And here. > + memset(visit_path_str, 0, sizeof(visit_path_str)); > + entry->value =3D visit_qobj_dict(data); > + entry->next =3D NULL; > + if (!list) { Why not use a pointer to pointer to avoid this if branch? > + list =3D entry; > + } else { > + last_entry->next =3D entry; > + } > + last_entry =3D entry; > + } > + > + return list; > +} > + > void qmp_add_client(const char *protocol, const char *fdname, > bool has_skipauth, bool skipauth, bool has_tls, b= ool tls, > Error **errp) > Fam