From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52433) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aMYbM-0003Ze-Lg for qemu-devel@nongnu.org; Fri, 22 Jan 2016 05:05:57 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aMYbJ-00018f-FQ for qemu-devel@nongnu.org; Fri, 22 Jan 2016 05:05:56 -0500 Received: from mx1.redhat.com ([209.132.183.28]:56991) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aMYbJ-00018M-79 for qemu-devel@nongnu.org; Fri, 22 Jan 2016 05:05:53 -0500 Date: Fri, 22 Jan 2016 10:05:48 +0000 From: "Daniel P. Berrange" Message-ID: <20160122100548.GC14825@redhat.com> References: <1453454812-29232-1-git-send-email-valentin.rakush@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1453454812-29232-1-git-send-email-valentin.rakush@gmail.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v3] qom, qmp, hmp, qapi: create qom-type-prop-list for class properties Reply-To: "Daniel P. Berrange" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Valentin Rakush Cc: armbru@redhat.com, qemu-devel@nongnu.org, lcapitulino@redhat.com, asmetanin@virtuozzo.com, den@openvz.org, afaerber@suse.de On Fri, Jan 22, 2016 at 12:26:52PM +0300, Valentin Rakush wrote: > This patch adds support for qom-type-prop-list command to list object > class properties. A later patch will use this functionality to > implement x86_64-cpu properties. >=20 > Signed-off-by: Valentin Rakush > Cc: Luiz Capitulino > Cc: Eric Blake > Cc: Markus Armbruster > Cc: Andreas F=C3=A4rber > Cc: Daniel P. Berrange > --- > V2: Fixes after first review > - changed command name from qom-type-list to qom-type-prop-list > - changed memory allocation from g_malloc0 to g_new0 > - changed parameter name from path to typename > - fixed wordings and comments > - fixed source code formatting > - registered the command in monitor > V3: commit message fix=20 > - commit message changed to reflect actual command name >=20 >=20 > hmp-commands.hx | 13 +++++++++++++ > hmp.c | 26 ++++++++++++++++++++++++++ > hmp.h | 1 + > include/qom/object.h | 31 +++++++++++++++++++++++++++++++ > qapi-schema.json | 19 +++++++++++++++++++ > qmp-commands.hx | 6 ++++++ > qmp.c | 32 ++++++++++++++++++++++++++++++++ > qom/object.c | 7 +++++++ > 8 files changed, 135 insertions(+) >=20 > diff --git a/hmp-commands.hx b/hmp-commands.hx > index bb52e4d..0aca653 100644 > --- a/hmp-commands.hx > +++ b/hmp-commands.hx > @@ -1734,6 +1734,19 @@ Print QOM properties of object at location @var{= path} > ETEXI > =20 > { > + .name =3D "qom-type-prop-list", > + .args_type =3D "typename:s?", The typename should be mandatory so remove the '?' here, then..... > + .params =3D "typename", > + .help =3D "list QOM class properties", > + .mhandler.cmd =3D hmp_qom_type_prop_list, > + }, > + > +STEXI > +@item qom-type-prop-list [@var{typename}] > +Print QOM properties of the type @var{typename} > +ETEXI > + > + { > .name =3D "qom-set", > .args_type =3D "path:s,property:s,value:s", > .params =3D "path property value", > diff --git a/hmp.c b/hmp.c > index 54f2620..4bad6a1 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -2052,6 +2052,32 @@ void hmp_qom_list(Monitor *mon, const QDict *qdi= ct) > hmp_handle_error(mon, &err); > } > =20 > +void hmp_qom_type_prop_list(Monitor *mon, const QDict *qdict) > +{ > + const char *path =3D qdict_get_try_str(qdict, "path"); > + ObjectPropertyInfoList *list; > + Error *err =3D NULL; > + > + if (!path) { > + monitor_printf(mon, "/\n"); > + return; > + } ...you don't need this check for path being NULL, you can assume it is non-NULL > + > + list =3D qmp_qom_type_prop_list(path, &err); > + if (!err) { > + ObjectPropertyInfoList *start =3D list; > + while (list) { > + ObjectPropertyInfo *value =3D list->value; > + > + monitor_printf(mon, "%s (%s)\n", > + value->name, value->type); > + list =3D list->next; > + } > + qapi_free_ObjectPropertyInfoList(start); > + } > + hmp_handle_error(mon, &err); > +} > + Aside from that I think this change looks good. Regards, Daniel --=20 |: http://berrange.com -o- http://www.flickr.com/photos/dberrange= / :| |: http://libvirt.org -o- http://virt-manager.or= g :| |: http://autobuild.org -o- http://search.cpan.org/~danberr= / :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vn= c :|