On Wed, Jul 17, 2013 at 04:36:06PM -0400, Luiz Capitulino wrote:
> On Tue, 16 Jul 2013 18:37:42 +0800
> Amos Kong <ak...@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.
> > + }
> > + }
--
Amos.
