From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55037) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bjTad-0001q1-8t for qemu-devel@nongnu.org; Mon, 12 Sep 2016 11:56:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bjTab-00057u-HZ for qemu-devel@nongnu.org; Mon, 12 Sep 2016 11:56:11 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44972) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bjTab-00057l-9B for qemu-devel@nongnu.org; Mon, 12 Sep 2016 11:56:09 -0400 Date: Mon, 12 Sep 2016 16:56:05 +0100 From: "Daniel P. Berrange" Message-ID: <20160912155605.GM16124@redhat.com> Reply-To: "Daniel P. Berrange" References: <20160911055301.26650-1-lma@suse.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20160911055301.26650-1-lma@suse.com> Subject: Re: [Qemu-devel] [PATCH v2] object: Add 'help' option for all available backends and properties List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Lin Ma Cc: qemu-devel@nongnu.org, pbonzini@redhat.com, armbru@redhat.com On Sun, Sep 11, 2016 at 01:53:01PM +0800, Lin Ma wrote: > '-object help' prints available user creatable backends. > '-object $typename,help' prints relevant properties. > > Signed-off-by: Lin Ma > --- > include/qom/object_interfaces.h | 2 + > qemu-options.hx | 7 ++- > qom/object_interfaces.c | 112 ++++++++++++++++++++++++++++++++++++++++ > vl.c | 5 ++ > 4 files changed, 125 insertions(+), 1 deletion(-) > diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c > index bf59846..4ee8643 100644 > --- a/qom/object_interfaces.c > +++ b/qom/object_interfaces.c > @@ -5,6 +5,7 @@ > #include "qapi-visit.h" > #include "qapi/qmp-output-visitor.h" > #include "qapi/opts-visitor.h" > +#include "qemu/help_option.h" > > void user_creatable_complete(Object *obj, Error **errp) > { > @@ -212,6 +213,117 @@ void user_creatable_del(const char *id, Error **errp) > object_unparent(obj); > } > > +int user_creatable_help_func(void *opaque, QemuOpts *opts, Error **errp) > +{ > + Visitor *v; > + char *type = NULL; > + Error *local_err = NULL; > + > + int i; > + char *values = NULL; > + Object *obj; > + ObjectPropertyInfoList *props = NULL; > + ObjectProperty *prop; > + ObjectPropertyIterator iter; > + ObjectPropertyInfoList *start; > + > + struct EnumProperty { > + const char * const *strings; > + int (*get)(Object *, Error **); > + void (*set)(Object *, int, Error **); > + } *enumprop; Ewww, this struct is declared privately in object.c and you're declaring it here so you can poke at private data belonging to the object internal impl. This is not ok in any way. > + v = opts_visitor_new(opts); > + visit_start_struct(v, NULL, NULL, 0, &local_err); > + if (local_err) { > + goto out; > + } > + > + visit_type_str(v, "qom-type", &type, &local_err); > + if (local_err) { > + goto out_visit; > + } > + > + if (type && is_help_option(type)) { > + printf("Available object backend types:\n"); > + GSList *list = object_class_get_list(TYPE_USER_CREATABLE, false); > + while (list) { > + const char *name; > + name = object_class_get_name(OBJECT_CLASS(list->data)); > + if (strcmp(name, TYPE_USER_CREATABLE)) { > + printf("%s\n", name); > + } > + list = list->next; > + } > + g_slist_free(list); > + goto out_visit; > + } > + > + if (!type || !qemu_opt_has_help_opt(opts)) { > + visit_end_struct(v, NULL); > + return 0; > + } > + > + if (!object_class_by_name(type)) { > + printf("invalid object type: %s\n", type); > + goto out_visit; > + } > + obj = object_new(type); > + object_property_iter_init(&iter, obj); > + > + while ((prop = object_property_iter_next(&iter))) { > + ObjectPropertyInfoList *entry = g_malloc0(sizeof(*entry)); > + entry->value = g_malloc0(sizeof(ObjectPropertyInfo)); > + entry->next = props; > + props = entry; > + entry->value->name = g_strdup(prop->name); > + i = 0; > + enumprop = prop->opaque; > + if (!g_str_equal(prop->type, "string") && \ > + !g_str_equal(prop->type, "bool") && \ > + !g_str_equal(prop->type, "struct tm") && \ > + !g_str_equal(prop->type, "int") && \ > + !g_str_equal(prop->type, "uint8") && \ > + !g_str_equal(prop->type, "uint16") && \ > + !g_str_equal(prop->type, "uint32") && \ > + !g_str_equal(prop->type, "uint64")) { It is absolutely *not* safe to assume that the result of this condition is an enum. > + while (enumprop->strings[i] != NULL) { > + if (i != 0) { > + values = g_strdup_printf("%s/%s", > + values, enumprop->strings[i]); Leaking the old memory for 'values'. > + } else { > + values = g_strdup_printf("%s", enumprop->strings[i]); > + } > + i++; > + } > + entry->value->type = g_strdup_printf("%s, available values: %s", > + prop->type, values); > + } else { > + entry->value->type = g_strdup(prop->type); > + } > + } > + > + start = props; > + while (props != NULL) { > + ObjectPropertyInfo *value = props->value; > + printf("%s (%s)\n", value->name, value->type); > + props = props->next; > + } > + qapi_free_ObjectPropertyInfoList(start); > + > +out_visit: > + visit_end_struct(v, NULL); > + > +out: > + g_free(values); > + g_free(type); > + object_unref(obj); > + if (local_err) { > + error_report_err(local_err); > + } > + return 1; > +} > + > diff --git a/vl.c b/vl.c > index ee557a1..a2230c8 100644 > --- a/vl.c > +++ b/vl.c > @@ -4251,6 +4251,11 @@ int main(int argc, char **argv, char **envp) > page_size_init(); > socket_init(); > > + if (qemu_opts_foreach(qemu_find_opts("object"), user_creatable_help_func, > + NULL, NULL)) { > + exit(1); > + } Generally 'help' is dealt with in the main option parsing, rather than adding a second iteration over the options. IOW, it would normally be done in user_creatable_add_opts_foreach, avoiding duplicating code between user_creatable_add_opts_foreach and user_creatable_help_func. > + > if (qemu_opts_foreach(qemu_find_opts("object"), > user_creatable_add_opts_foreach, > object_create_initial, NULL)) { Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|