[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] object: Add 'help' option for all available
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v2] object: Add 'help' option for all available backends and properties |
Date: |
Mon, 12 Sep 2016 17:42:11 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Lin Ma <address@hidden> writes:
> '-object help' prints available user creatable backends.
> '-object $typename,help' prints relevant properties.
>
> Signed-off-by: Lin Ma <address@hidden>
> ---
> 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/include/qom/object_interfaces.h b/include/qom/object_interfaces.h
> index 8b17f4d..197cd5f 100644
> --- a/include/qom/object_interfaces.h
> +++ b/include/qom/object_interfaces.h
> @@ -165,4 +165,6 @@ int user_creatable_add_opts_foreach(void *opaque,
> */
> void user_creatable_del(const char *id, Error **errp);
>
> +int user_creatable_help_func(void *opaque, QemuOpts *opts, Error **errp);
> +
> #endif
> diff --git a/qemu-options.hx b/qemu-options.hx
> index a71aaf8..fade53c 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -3752,7 +3752,9 @@ DEF("object", HAS_ARG, QEMU_OPTION_object,
> " create a new object of type TYPENAME setting
> properties\n"
> " in the order they are specified. Note that the 'id'\n"
> " property must be set. These objects are placed in
> the\n"
> - " '/objects' path.\n",
> + " '/objects' path.\n"
> + " Use '-object help' to print available backend types
> and\n"
> + " '-object typename,help' to print relevant
> properties.\n",
> QEMU_ARCH_ALL)
> STEXI
> @item -object @var{typename}[,@address@hidden,...]
> @@ -3762,6 +3764,9 @@ in the order they are specified. Note that the 'id'
> property must be set. These objects are placed in the
> '/objects' path.
>
> +Use '-object help' to print available backend types and
> +'-object typename,help' to print relevant properties.
> +
Make that
+Use @code{-object help} to print available backend types and
address@hidden @var{typename},help} to print relevant properties.
+
> @table @option
>
> @item -object
> memory-backend-file,address@hidden,address@hidden,address@hidden,address@hidden|off}
> 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;
> +
Why this blank line?
> + 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;
Copied from object.c. Big no-no. Whatever you do with this probably
belongs into object.c instead.
> +
> + 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)) {
I think the Options visitor is overkill. Much simpler:
type = qemu_opt_get(opts, "qom-type");
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)) {
Why do you need to skip TYPE_USER_CREATABLE?
Hmm...
$ qemu-system-x86_64 -object user-creatable,id=foo
**
ERROR:/work/armbru/qemu/qom/object.c:361:object_initialize_with_type:
assertion failed (type->instance_size >= sizeof(Object)): (0 >= 40)
Aborted (core dumped)
Should this type be abstract?
> + printf("%s\n", name);
> + }
> + list = list->next;
> + }
Recommend to keep the loop control in one place:
for (list = object_class_get_list(TYPE_USER_CREATABLE, false);
list;
list = list->next) {
...
}
If you hate multi-line for (...), you can do
GSList *head = object_class_get_list(TYPE_USER_CREATABLE, false);
for (list = head; 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));
Blank line between declarations and statements, please.
> + entry->value = g_malloc0(sizeof(ObjectPropertyInfo));
Either g_malloc0(sizeof(entry->value), or g_new0(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")) {
Uh, what are you trying to test with this condition? It's not obvious
to me...
> + while (enumprop->strings[i] != NULL) {
> + if (i != 0) {
> + values = g_strdup_printf("%s/%s",
> + values, enumprop->strings[i]);
> + } else {
> + values = g_strdup_printf("%s", enumprop->strings[i]);
> + }
> + i++;
> + }
> + entry->value->type = g_strdup_printf("%s, available values: %s",
> + prop->type, values);
Looks like this is the enum case. But why is the condition true exactly
for enums?
> + } else {
> + entry->value->type = g_strdup(prop->type);
> + }
> + }
The loop above collects properties, and ...
> +
> + start = props;
> + while (props != NULL) {
> + ObjectPropertyInfo *value = props->value;
> + printf("%s (%s)\n", value->name, value->type);
> + props = props->next;
> + }
> + qapi_free_ObjectPropertyInfoList(start);
... this loop prints them. Have you considered fusing the two loops?
> +
> +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;
> +}
> +
> static void register_types(void)
> {
> static const TypeInfo uc_interface_info = {
> 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);
> + }
> +
I think this should be done as early as possible. However, we already
do -device help nearby. Not fair to ask you to clean up this mess.
> if (qemu_opts_foreach(qemu_find_opts("object"),
> user_creatable_add_opts_foreach,
> object_create_initial, NULL)) {
- [Qemu-devel] [PATCH v2] object: Add 'help' option for all available backends and properties, Lin Ma, 2016/09/11
- Re: [Qemu-devel] [PATCH v2] object: Add 'help' option for all available backends and properties,
Markus Armbruster <=
- [Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all available backends and properties, Lin Ma, 2016/09/17
- Re: [Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all available backends and properties, Markus Armbruster, 2016/09/19
- Re: [Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all available backends and properties, Andreas Färber, 2016/09/19
- Re: [Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all available backends and properties, Markus Armbruster, 2016/09/19
- Re: [Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all available backends and properties, Lin Ma, 2016/09/21
- Re: [Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all available backends and properties, Markus Armbruster, 2016/09/22
- Re: [Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all available backends and properties, Daniel P. Berrange, 2016/09/22
- Re: [Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all available backends and properties, Markus Armbruster, 2016/09/22
- Re: [Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all available backends and properties, Daniel P. Berrange, 2016/09/22
- Re: [Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all available backends and properties, Markus Armbruster, 2016/09/22