| Message ID | 20160911055301.26650-1-lma@suse.com |
|---|---|
| State | New |
| Headers | show |
Lin Ma <lma@suse.com> writes: > '-object help' prints available user creatable backends. > '-object $typename,help' prints relevant properties. > > Signed-off-by: Lin Ma <lma@suse.com> > --- > 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}[,@var{prop1}=@var{value1},...] > @@ -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 +@code{-object @var{typename},help} to print relevant properties. + > @table @option > > @item -object memory-backend-file,id=@var{id},size=@var{size},mem-path=@var{dir},share=@var{on|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)) {
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 <lma@suse.com> > --- > 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
>>> Markus Armbruster <armbru@redhat.com> 2016/9/12 星期一 下午 11:42 >>> >Lin Ma <lma@suse.com> writes: > >> '-object help' prints available user creatable backends. >> '-object $typename,help' prints relevant properties. >> >> Signed-off-by: Lin Ma <lma@suse.com> >> --- >> 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}[,@var{prop1}=@var{value1},...] >> @@ -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 > +@code{-object @var{typename},help} to print relevant properties. > + > Ok. >> @table @option >> >> @item -object memory-backend-file,id=@var{id},size=@var{size},mem-path=@var{dir},share=@var{on|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? > I'll remove it. >> + 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. > Yes, this way is ugly. What I want to do is parsing the enum <-> string mappings through the EnumProperty to make the output more reasonable. It should be parsed in object.c, But I can't assume the size of enum str list, then can't pre-alloc it in user_creatable_help_func. So far I havn't figured out a good way to return a string array that including the enum str list to user_creatable_help_func for printing. May I have your suggestions? >> + >> + 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)) { > Yes, it is much simpler, I'll use this instead of visitor code. >> + 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? > Yes, The object type user-creatable is abstract, But obviously it missed the abstract property. I'll add it in next patch(patch 1/2), Then I dont need to skip it at here anymore. >> + 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) { > ... > } > Will do it. >> + 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. > ok. >> + entry->value = g_malloc0(sizeof(ObjectPropertyInfo)); > >Either g_malloc0(sizeof(entry->value), or g_new0(ObjectPropertyInfo). > will use g_malloc0(sizeof(entry->value). >> + 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? > Yes, After filter all the other types above, I assume the result here is the enum case. I knew this way does not make sense, But I havn't figured out a proper way yet to judge whether the type is an enum or not. May I have your ideas? >> + } 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? > I agreed. will merge 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. > How about move it to here? object_property_add_child(object_get_root(), "machine", OBJECT(current_machine), &error_abort); + + if (qemu_opts_foreach(qemu_find_opts("object"), user_creatable_help_func, + NULL, NULL)) { + exit(1); + } + cpu_exec_init_all(); >> if (qemu_opts_foreach(qemu_find_opts("object"), >> user_creatable_add_opts_foreach, >> object_create_initial, NULL)) { Thank you for reviewing the code. Lin
>>> "Daniel P. Berrange" <berrange@redhat.com> 2016/9/12 星期一 下午 11:56 >>> >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 <lma@suse.com> >> --- >> 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. > Yes, this way is ugly. What I want to do is parsing the enum <-> string mappings through the EnumProperty to make the output more reasonable. It should be parsed in object.c, But I can't assume the size of enum str list, then can't pre-alloc it in user_creatable_help_func. So far I havn't figured out a good way to return a string array that including the enum str list to user_creatable_help_func for printing. May I have your suggestions? >> + 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. > Yes, It does not make sense and is not safe. Writing this way becasue I can't figure out a way to judge whether the type is an enum or not. May I have your ideas? >> + while (enumprop->strings[i] != NULL) { >> + if (i != 0) { >> + values = g_strdup_printf("%s/%s", >> + values, enumprop->strings[i]); > >Leaking the old memory for 'values'. > Ok, I'll fix it. >> + } 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. > I used to consider to put it in user_creatable_add_opts_foreach, but I think that keeping it as a separate function maybe more clear. Meanwhile, I'd like to add these help output to monitor command 'object_add' later, So puting the code into user_creatable_help_func may help as well. >> + >> if (qemu_opts_foreach(qemu_find_opts("object"), >> user_creatable_add_opts_foreach, >> object_create_initial, NULL)) { > >Regards, >Daniel Thanks for helping to review the code. Lin
This is about QOM use. Cc: Andreas in case he has smart ideas. Andreas, you may want to skip ahead to "EnumProperty". "Lin Ma" <lma@suse.com> writes: >>>> Markus Armbruster <armbru@redhat.com> 2016/9/12 星期一 下午 11:42 >>> >>Lin Ma <lma@suse.com> writes: >> >>> '-object help' prints available user creatable backends. >>> '-object $typename,help' prints relevant properties. >>> >>> Signed-off-by: Lin Ma <lma@suse.com> >>> --- >>> 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; >>> + >> >>Why this blank line? >> > I'll remove it. > >>> + 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. >> > Yes, this way is ugly. > What I want to do is parsing the enum <-> string mappings through the EnumProperty > to make the output more reasonable. > It should be parsed in object.c, But I can't assume the size of enum str list, then can't > pre-alloc it in user_creatable_help_func. So far I havn't figured out a good way to return > a string array that including the enum str list to user_creatable_help_func for printing. > May I have your suggestions? See below. >>> + >>> + > 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)) { >> > Yes, it is much simpler, I'll use this instead of visitor code. > >>> + 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? >> > Yes, The object type user-creatable is abstract, But obviously it missed the abstract property. > I'll add it in next patch(patch 1/2), Then I dont need to skip it at here anymore. Yes, please. >>> + 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) { >> ... >> } >> > Will do it. > >>> + 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. >> > ok. > >>> + entry->value = g_malloc0(sizeof(ObjectPropertyInfo)); >> >>Either g_malloc0(sizeof(entry->value), or g_new0(ObjectPropertyInfo). >> > will use g_malloc0(sizeof(entry->value). > >>> + 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? >> > Yes, After filter all the other types above, I assume the result here is the enum case. > I knew this way does not make sense, But I havn't figured out a proper way yet to judge > whether the type is an enum or not. > May I have your ideas? You're messing with struct EnumProperty because you want more help than what ObjectPropertyInfo can provice. Digression on the meaning of ObjectPropertyInfo. This is its definition: ## # @ObjectPropertyInfo: # # @name: the name of the property # # @type: the type of the property. This will typically come in one of four # forms: # # 1) A primitive type such as 'u8', 'u16', 'bool', 'str', or 'double'. # These types are mapped to the appropriate JSON type. # # 2) A child type in the form 'child<subtype>' where subtype is a qdev # device type name. Child properties create the composition tree. # # 3) A link type in the form 'link<subtype>' where subtype is a qdev # device type name. Link properties form the device model graph. # # Since: 1.2 ## { 'struct': 'ObjectPropertyInfo', 'data': { 'name': 'str', 'type': 'str' } } @type is documented to be either a primitive type, a child type or a link. "Primitive type" isn't defined. The examples given suggest it's a QAPI built-in type. If that's correct, clause 1) does not cover enumeration types. However, it doesn't seem to be correct: I observe 'string', not 'str'. Paolo, Andreas, any idea what @type is supposed to mean? End of digression. All ObjectPropertyInfo gives you is two strings: a name and a type. If you need more information than that, you have to add a proper interface to get it! Say a function that takes an object and a property name, and returns information about the property's type. Probably should be two functions, one that maps QOM object and property name to the property's QOM type, one that maps a QOM type to QOM type information. In other words, you need QOM object and type introspection. Paolo, Andreas, if that already exists in some form, please point us to it. >>> + > } 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? >> > I agreed. will merge 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. >> > How about move it to here? > object_property_add_child(object_get_root(), "machine", > OBJECT(current_machine), &error_abort); > + > + if (qemu_opts_foreach(qemu_find_opts("object"), user_creatable_help_func, > + NULL, NULL)) { > + exit(1); > + } > + > cpu_exec_init_all(); I wouldn't put it in the middle of the machine stuff. Perhaps before current_machine = MACHINE(...)? >>> if (qemu_opts_foreach(qemu_find_opts("object"), >>> user_creatable_add_opts_foreach, >>> object_create_initial, NULL)) { > > Thank you for reviewing the code. > Lin
Hi Lin and Markus, Am 19.09.2016 um 13:58 schrieb Markus Armbruster: > This is about QOM use. Cc: Andreas in case he has smart ideas. > Andreas, you may want to skip ahead to "EnumProperty". > > "Lin Ma" <lma@suse.com> writes: > >>>>> Markus Armbruster <armbru@redhat.com> 2016/9/12 星期一 下午 11:42 >>> >>> Lin Ma <lma@suse.com> writes: >>> >>>> '-object help' prints available user creatable backends. >>>> '-object $typename,help' prints relevant properties. >>>> >>>> Signed-off-by: Lin Ma <lma@suse.com> >>>> --- >>>> 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; >>>> + >>> >>> Why this blank line? >>> >> I'll remove it. >> >>>> + 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. >>> >> Yes, this way is ugly. >> What I want to do is parsing the enum <-> string mappings through the EnumProperty >> to make the output more reasonable. >> It should be parsed in object.c, But I can't assume the size of enum str list, then can't >> pre-alloc it in user_creatable_help_func. So far I havn't figured out a good way to return >> a string array that including the enum str list to user_creatable_help_func for printing. >> May I have your suggestions? > > See below. > >>>> + >>>> + >> 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)) { >>> >> Yes, it is much simpler, I'll use this instead of visitor code. >> >>>> + 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? >>> >> Yes, The object type user-creatable is abstract, But obviously it missed the abstract property. >> I'll add it in next patch(patch 1/2), Then I dont need to skip it at here anymore. > > Yes, please. > >>>> + 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) { >>> ... >>> } >>> >> Will do it. >> >>>> + 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. >>> >> ok. >> >>>> + entry->value = g_malloc0(sizeof(ObjectPropertyInfo)); >>> >>> Either g_malloc0(sizeof(entry->value), or g_new0(ObjectPropertyInfo). >>> >> will use g_malloc0(sizeof(entry->value). >> >>>> + 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? >>> >> Yes, After filter all the other types above, I assume the result here is the enum case. >> I knew this way does not make sense, But I havn't figured out a proper way yet to judge >> whether the type is an enum or not. >> May I have your ideas? > > You're messing with struct EnumProperty because you want more help than > what ObjectPropertyInfo can provice. > > Digression on the meaning of ObjectPropertyInfo. This is its > definition: > > ## > # @ObjectPropertyInfo: > # > # @name: the name of the property > # > # @type: the type of the property. This will typically come in one of four > # forms: > # > # 1) A primitive type such as 'u8', 'u16', 'bool', 'str', or 'double'. > # These types are mapped to the appropriate JSON type. > # > # 2) A child type in the form 'child<subtype>' where subtype is a qdev > # device type name. Child properties create the composition tree. > # > # 3) A link type in the form 'link<subtype>' where subtype is a qdev > # device type name. Link properties form the device model graph. > # > # Since: 1.2 > ## > { 'struct': 'ObjectPropertyInfo', > 'data': { 'name': 'str', 'type': 'str' } } > > @type is documented to be either a primitive type, a child type or a > link. "Primitive type" isn't defined. The examples given suggest it's > a QAPI built-in type. If that's correct, clause 1) does not cover > enumeration types. However, it doesn't seem to be correct: I observ > 'string', not 'str'. Paolo, Andreas, any idea what @type is supposed to > mean? > > End of digression. > > All ObjectPropertyInfo gives you is two strings: a name and a type. If > you need more information than that, you have to add a proper interface > to get it! Say a function that takes an object and a property name, and > returns information about the property's type. Probably should be two > functions, one that maps QOM object and property name to the property's > QOM type, one that maps a QOM type to QOM type information. > > In other words, you need QOM object and type introspection. Paolo, > Andreas, if that already exists in some form, please point us to it. Could you say what exactly you want to introspect here? Both ObjectClass and Object have a list of properties that together form the list of properties that can be set on an instance. So you'll need to instantiate the object like I think we do for devices. Then you can recursively enumerate the properties to get their names and types (and possibly put them into a new list for alphabetical sorting if desired). Regards, Andreas
Andreas Färber <afaerber@suse.de> writes: > Hi Lin and Markus, > > Am 19.09.2016 um 13:58 schrieb Markus Armbruster: [...] >> You're messing with struct EnumProperty because you want more help than >> what ObjectPropertyInfo can provice. >> >> Digression on the meaning of ObjectPropertyInfo. This is its >> definition: >> >> ## >> # @ObjectPropertyInfo: >> # >> # @name: the name of the property >> # >> # @type: the type of the property. This will typically come in one of four >> # forms: >> # >> # 1) A primitive type such as 'u8', 'u16', 'bool', 'str', or 'double'. >> # These types are mapped to the appropriate JSON type. >> # >> # 2) A child type in the form 'child<subtype>' where subtype is a qdev >> # device type name. Child properties create the composition tree. >> # >> # 3) A link type in the form 'link<subtype>' where subtype is a qdev >> # device type name. Link properties form the device model graph. >> # >> # Since: 1.2 >> ## >> { 'struct': 'ObjectPropertyInfo', >> 'data': { 'name': 'str', 'type': 'str' } } >> >> @type is documented to be either a primitive type, a child type or a >> link. "Primitive type" isn't defined. The examples given suggest it's >> a QAPI built-in type. If that's correct, clause 1) does not cover >> enumeration types. However, it doesn't seem to be correct: I observ >> 'string', not 'str'. Paolo, Andreas, any idea what @type is supposed to >> mean? >> >> End of digression. >> >> All ObjectPropertyInfo gives you is two strings: a name and a type. If >> you need more information than that, you have to add a proper interface >> to get it! Say a function that takes an object and a property name, and >> returns information about the property's type. Probably should be two >> functions, one that maps QOM object and property name to the property's >> QOM type, one that maps a QOM type to QOM type information. >> >> In other words, you need QOM object and type introspection. Paolo, >> Andreas, if that already exists in some form, please point us to it. > > Could you say what exactly you want to introspect here? Context: Lin wants to implement "-object TYPE-NAME,help", similar to "-device DRIVER-NAME,help", i.e. list the available properties of TYPE-NAME. His proposed patch tries to give better help for enumeration types by listing the acceptable values. The code that does it is an unacceptable hack, though. We're trying to figure out a way to provide such help cleanly. One way to do it would be introspecting QOM *types*. Find the property's type, figure out what kind of type it is, if it's an enumeration type, find its members, ... > Both ObjectClass and Object have a list of properties that together form > the list of properties that can be set on an instance. So you'll need to > instantiate the object like I think we do for devices. Then you can > recursively enumerate the properties to get their names and types (and > possibly put them into a new list for alphabetical sorting if desired). Lin's code uses object_new() to instantiate a dummy object, object_property_iter_init() and object_property_iter_next() to find the properties. Sounds okay so far, doesn't it? Hmm, ObjectProperty has members name, type, description. Could description be useful? I guess it's set with object_property_set_description(). I can see only a few dozen calls, which makes me suspect it's null more often than not. When it's null we could still fall back to a description of the type. Does such a thing exist? Enumeration types could provide one listing their values. Other ideas?
"Lin Ma" <lma@suse.com> writes: >>>> Markus Armbruster <armbru@redhat.com> 2016/9/20 星期二 上午 1:13 >>> >>Andreas Färber <afaerber@suse.de> writes: >> >>> Hi Lin and Markus, >>> >>> Am 19.09.2016 um 13:58 schrieb Markus Armbruster: >>[...] >>>> You're messing with struct EnumProperty because you want more help than >>>> what ObjectPropertyInfo can provice. >>>> >>>> Digression on the meaning of ObjectPropertyInfo. This is its >>>> definition: >>>> >>>> ## >>>> # @ObjectPropertyInfo: >>>> # >>>> # @name: the name of the property >>>> # >>>> # @type: the type of the property. This will typically come in one of four >>>> # forms: >>>> # >>>> # 1) A primitive type such as 'u8', 'u16', 'bool', 'str', or 'double'. >>>> # These types are mapped to the appropriate JSON type. >>>> # >>>> # 2) A child type in the form 'child<subtype>' where subtype is a qdev >>>> # device type name. Child properties create the composition tree. >>>> # >>>> # 3) A link type in the form 'link<subtype>' where subtype is a qdev >>>> # device type name. Link properties form the device model graph. >>>> # >>>> # Since: 1.2 >>>> ## >>>> { 'struct': 'ObjectPropertyInfo', >>>> 'data': { 'name': 'str', 'type': 'str' } } >>>> >>>> @type is documented to be either a primitive type, a child type or a >>>> link. "Primitive type" isn't defined. The examples given suggest it's >>>> a QAPI built-in type. If that's correct, clause 1) does not cover >>>> enumeration types. However, it doesn't seem to be correct: I observ >>>> 'string', not 'str'. Paolo, Andreas, any idea what @type is supposed to >>>> mean? >>>> >>>> End of digression. >>>> >>>> All ObjectPropertyInfo gives you is two strings: a name and a type. If >>>> you need more information than that, you have to add a proper interface >>>> to get it! Say a function that takes an object and a property name, and >>>> returns information about the property's type. Probably should be two >>>> functions, one that maps QOM object and property name to the property's >>>> QOM type, one that maps a QOM type to QOM type information. >>>> >>>> In other words, you need QOM object and type introspection. Paolo, >>>> Andreas, if that already exists in some form, please point us to it. >>> >>> Could you say what exactly you want to introspect here? >> >>Context: Lin wants to implement "-object TYPE-NAME,help", similar to >>"-device DRIVER-NAME,help", i.e. list the available properties of >>TYPE-NAME. >> >>His proposed patch tries to give better help for enumeration types by >>listing the acceptable values. The code that does it is an unacceptable >>hack, though. We're trying to figure out a way to provide such help >>cleanly. >> >>One way to do it would be introspecting QOM *types*. Find the >>property's type, figure out what kind of type it is, if it's an >>enumeration type, find its members, ... >> >>> Both ObjectClass and Object have a list of properties that together form >>> the list of properties that can be set on an instance. So you'll need to >>> instantiate the object like I think we do for devices. Then you can >>> recursively enumerate the properties to get their names and types (and >>> possibly put them into a new list for alphabetical sorting if desired). >> >>Lin's code uses object_new() to instantiate a dummy object, >>object_property_iter_init() and object_property_iter_next() to find the >>properties. Sounds okay so far, doesn't it? >> >>Hmm, ObjectProperty has members name, type, description. Could >>description be useful? I guess it's set with >>object_property_set_description(). I can see only a few dozen calls, >>which makes me suspect it's null more often than not. > > Saving acceptable values of enumeration types into member description > of ObjectProperty is a good idea. > > The member description of ObjectProperty instance of any user-creatable > object is NULL so far, It's null until set with object_property_set_description(). We do that for a few properties, and may do it more. > If I use object_property_set_description() to fill the > acceptable values of enumeration type into the description in function > object_property_add_enum and object_class_property_add_enum, Then I > can use this description to judge whether a ObjectProperty instance' type > is enumeration or not in function user_creatable_help_func. In this case, If > member description is not NULL, it means this ObjectProperty instance is > an enumeration. No. If you need to decide in user_creatable_help_func() whether a property has an enumeration type, something's wrong at the infrastructure level. > Any suggestions? Yes: >>When it's null we could still fall back to a description of the type. >>Does such a thing exist? Enumeration types could provide one listing >>their values. Don't make up a description in user_creatable_help_func(), improve the description infrastructure and its use so you get more useful ones there. The existing description infrastructure is just Property member description and object_property_set_description(). Rarely used, so description is generally null. Calling object_property_set_description() more often could be helpful, but to come up with a sensible description string, you need to know what the property does. Needs to be left to people actually familiar with the objects. Aside: historically, we add properties to *instances*. All the property meta-data gets duplicated for every instance, including property descriptions. This is more flexible than adding the meta-data to the class. The flexibility is rarely needed, but the price in wasted memory is always paid. Only since commit 16bf7f5, we can add it to classes. Adding lots of helpful property descriptions would increase the cost of instance properties further. What you could perhaps do is adding a *type* description. For enums, that would show the set of acceptable values. Then if the property has no description, fall back to the description of its type. Unfortunately, I can't see a "property type" abstraction in the code. Perhaps Andreas or Paolo can provide some guidance. > If this way makes sense, Then Should I add a member description for > ObjectPropertyInfo as well? I suspect you won't even need ObjectPropertyInfo here once you've fused your loops. ObjectPropertyInfo is a QAPI/QMP thing. Adding a description to it needs to make sense for QMP. I haven't thought about it. >>Other ideas?
On Thu, Sep 22, 2016 at 10:36:45AM +0200, Markus Armbruster wrote: > Don't make up a description in user_creatable_help_func(), improve the > description infrastructure and its use so you get more useful ones > there. > > The existing description infrastructure is just Property member > description and object_property_set_description(). Rarely used, so > description is generally null. > > Calling object_property_set_description() more often could be helpful, > but to come up with a sensible description string, you need to know what > the property does. Needs to be left to people actually familiar with > the objects. > > Aside: historically, we add properties to *instances*. All the property > meta-data gets duplicated for every instance, including property > descriptions. This is more flexible than adding the meta-data to the > class. The flexibility is rarely needed, but the price in wasted memory > is always paid. Only since commit 16bf7f5, we can add it to classes. > Adding lots of helpful property descriptions would increase the cost of > instance properties further. FWIW, we could easily optimize handling of description strings by applying the same trick that GLib has done for GObject property descriptions. Almost certainly every call to object_property_set_description is going to be passing a string literal, not a dynamically allocated string. So we take advantage of that and in fact mandate that it is a string literal, and thus avoid the strdup() of description. We can place a fun game to enforce this at compile time thus: - Rename object_property_set_description() to object_property_set_description_internal() - Add the macro #define object_property_set_description(obj, name, desc, errp) \ object_property_set_description_internal(obj, name, "" desc "", errp) None the less, we really should make an effort to switch things over to use class properties instead of instance properties, as its going to save us allocating a 64 byte struct per property per instance Regards, Daniel
"Daniel P. Berrange" <berrange@redhat.com> writes: > On Thu, Sep 22, 2016 at 10:36:45AM +0200, Markus Armbruster wrote: >> Don't make up a description in user_creatable_help_func(), improve the >> description infrastructure and its use so you get more useful ones >> there. >> >> The existing description infrastructure is just Property member >> description and object_property_set_description(). Rarely used, so >> description is generally null. >> >> Calling object_property_set_description() more often could be helpful, >> but to come up with a sensible description string, you need to know what >> the property does. Needs to be left to people actually familiar with >> the objects. >> >> Aside: historically, we add properties to *instances*. All the property >> meta-data gets duplicated for every instance, including property >> descriptions. This is more flexible than adding the meta-data to the >> class. The flexibility is rarely needed, but the price in wasted memory >> is always paid. Only since commit 16bf7f5, we can add it to classes. >> Adding lots of helpful property descriptions would increase the cost of >> instance properties further. > > FWIW, we could easily optimize handling of description strings by > applying the same trick that GLib has done for GObject property > descriptions. > > Almost certainly every call to object_property_set_description is > going to be passing a string literal, not a dynamically allocated > string. So we take advantage of that and in fact mandate that it > is a string literal, and thus avoid the strdup() of description. > > We can place a fun game to enforce this at compile time thus: > > - Rename object_property_set_description() to > object_property_set_description_internal() > > - Add the macro > > #define object_property_set_description(obj, name, desc, errp) \ > object_property_set_description_internal(obj, name, "" desc "", errp) Cute :) > None the less, we really should make an effort to switch things > over to use class properties instead of instance properties, as > its going to save us allocating a 64 byte struct per property > per instance Yes, please. Related: a way to define a bunch of properties as *data*, i.e. an array of property descriptions, commonly static. Reasoning about static data is so much easier than reasoning about code.
On Thu, Sep 22, 2016 at 01:12:22PM +0200, Markus Armbruster wrote: > "Daniel P. Berrange" <berrange@redhat.com> writes: > > > On Thu, Sep 22, 2016 at 10:36:45AM +0200, Markus Armbruster wrote: > >> Don't make up a description in user_creatable_help_func(), improve the > >> description infrastructure and its use so you get more useful ones > >> there. > >> > >> The existing description infrastructure is just Property member > >> description and object_property_set_description(). Rarely used, so > >> description is generally null. > >> > >> Calling object_property_set_description() more often could be helpful, > >> but to come up with a sensible description string, you need to know what > >> the property does. Needs to be left to people actually familiar with > >> the objects. > >> > >> Aside: historically, we add properties to *instances*. All the property > >> meta-data gets duplicated for every instance, including property > >> descriptions. This is more flexible than adding the meta-data to the > >> class. The flexibility is rarely needed, but the price in wasted memory > >> is always paid. Only since commit 16bf7f5, we can add it to classes. > >> Adding lots of helpful property descriptions would increase the cost of > >> instance properties further. > > > > FWIW, we could easily optimize handling of description strings by > > applying the same trick that GLib has done for GObject property > > descriptions. > > > > Almost certainly every call to object_property_set_description is > > going to be passing a string literal, not a dynamically allocated > > string. So we take advantage of that and in fact mandate that it > > is a string literal, and thus avoid the strdup() of description. > > > > We can place a fun game to enforce this at compile time thus: > > > > - Rename object_property_set_description() to > > object_property_set_description_internal() > > > > - Add the macro > > > > #define object_property_set_description(obj, name, desc, errp) \ > > object_property_set_description_internal(obj, name, "" desc "", errp) > > Cute :) > > > None the less, we really should make an effort to switch things > > over to use class properties instead of instance properties, as > > its going to save us allocating a 64 byte struct per property > > per instance > > Yes, please. > > Related: a way to define a bunch of properties as *data*, i.e. an array > of property descriptions, commonly static. Reasoning about static data > is so much easier than reasoning about code. IMHO we should go further and leverage QAPI schema to auto-generate all the tedious boilerplate code for QOM objects eg, consider the crypto/secret.c object file. We could declare it as { 'object': 'QCryptoSecret', 'parent': 'Object', 'properties': { 'format': 'QCryptoSecretFormat', 'data': 'str', 'file': 'str', 'keyid': 'str', 'iv': 'str' } } Based on that it would have enough knowledge to generate - struct QCryptoSecret definition + typedef - struct QCryptoSecretClass definition + typedef - TYPE_CRYPTO_SECRET macro - QCRYPT_SECRET() cast macro - Setters & getters aka qcrypto_secret_prop_set_format qcrypto_secret_prop_get_format qcrypto_secret_prop_set_data qcrypto_secret_prop_get_data qcrypto_secret_prop_set_file qcrypto_secret_prop_get_file qcrypto_secret_prop_set_keyid qcrypto_secret_prop_get_keyid qcrypto_secret_prop_set_iv qcrypto_secret_prop_get_iv - qcrypto_secret_finalize - qcrypto_secret_class_init - TypeInfo qcrypto_secret_info variable - qcrypto_secret_register_types() method - type_init(qcrypto_secret_register_types); That'd massively reduce the work to create new objects, to just filling in the semantically useful logic Regards, Daniel
"Daniel P. Berrange" <berrange@redhat.com> writes: > On Thu, Sep 22, 2016 at 01:12:22PM +0200, Markus Armbruster wrote: >> "Daniel P. Berrange" <berrange@redhat.com> writes: >> >> > On Thu, Sep 22, 2016 at 10:36:45AM +0200, Markus Armbruster wrote: >> >> Don't make up a description in user_creatable_help_func(), improve the >> >> description infrastructure and its use so you get more useful ones >> >> there. >> >> >> >> The existing description infrastructure is just Property member >> >> description and object_property_set_description(). Rarely used, so >> >> description is generally null. >> >> >> >> Calling object_property_set_description() more often could be helpful, >> >> but to come up with a sensible description string, you need to know what >> >> the property does. Needs to be left to people actually familiar with >> >> the objects. >> >> >> >> Aside: historically, we add properties to *instances*. All the property >> >> meta-data gets duplicated for every instance, including property >> >> descriptions. This is more flexible than adding the meta-data to the >> >> class. The flexibility is rarely needed, but the price in wasted memory >> >> is always paid. Only since commit 16bf7f5, we can add it to classes. >> >> Adding lots of helpful property descriptions would increase the cost of >> >> instance properties further. >> > >> > FWIW, we could easily optimize handling of description strings by >> > applying the same trick that GLib has done for GObject property >> > descriptions. >> > >> > Almost certainly every call to object_property_set_description is >> > going to be passing a string literal, not a dynamically allocated >> > string. So we take advantage of that and in fact mandate that it >> > is a string literal, and thus avoid the strdup() of description. >> > >> > We can place a fun game to enforce this at compile time thus: >> > >> > - Rename object_property_set_description() to >> > object_property_set_description_internal() >> > >> > - Add the macro >> > >> > #define object_property_set_description(obj, name, desc, errp) \ >> > object_property_set_description_internal(obj, name, "" desc "", errp) >> >> Cute :) >> >> > None the less, we really should make an effort to switch things >> > over to use class properties instead of instance properties, as >> > its going to save us allocating a 64 byte struct per property >> > per instance >> >> Yes, please. >> >> Related: a way to define a bunch of properties as *data*, i.e. an array >> of property descriptions, commonly static. Reasoning about static data >> is so much easier than reasoning about code. > > IMHO we should go further and leverage QAPI schema to auto-generate all > the tedious boilerplate code for QOM objects > > eg, consider the crypto/secret.c object file. > > We could declare it as > > { 'object': 'QCryptoSecret', > 'parent': 'Object', > 'properties': { > 'format': 'QCryptoSecretFormat', > 'data': 'str', > 'file': 'str', > 'keyid': 'str', > 'iv': 'str' > } } > > Based on that it would have enough knowledge to generate > > - struct QCryptoSecret definition + typedef > - struct QCryptoSecretClass definition + typedef > - TYPE_CRYPTO_SECRET macro > - QCRYPT_SECRET() cast macro > - Setters & getters aka > qcrypto_secret_prop_set_format > qcrypto_secret_prop_get_format > qcrypto_secret_prop_set_data > qcrypto_secret_prop_get_data > qcrypto_secret_prop_set_file > qcrypto_secret_prop_get_file > qcrypto_secret_prop_set_keyid > qcrypto_secret_prop_get_keyid > qcrypto_secret_prop_set_iv > qcrypto_secret_prop_get_iv > - qcrypto_secret_finalize > - qcrypto_secret_class_init > - TypeInfo qcrypto_secret_info variable > - qcrypto_secret_register_types() method > - type_init(qcrypto_secret_register_types); > > That'd massively reduce the work to create new objects, to just > filling in the semantically useful logic Promising idea. Sadly, the existing QAPI queue is eating all my QAPI cycles.
On Thu, Sep 22, 2016 at 02:03:39PM +0200, Markus Armbruster wrote: > "Daniel P. Berrange" <berrange@redhat.com> writes: > > > > IMHO we should go further and leverage QAPI schema to auto-generate all > > the tedious boilerplate code for QOM objects > > > > eg, consider the crypto/secret.c object file. > > > > We could declare it as > > > > { 'object': 'QCryptoSecret', > > 'parent': 'Object', > > 'properties': { > > 'format': 'QCryptoSecretFormat', > > 'data': 'str', > > 'file': 'str', > > 'keyid': 'str', > > 'iv': 'str' > > } } > > > > Based on that it would have enough knowledge to generate > > > > - struct QCryptoSecret definition + typedef > > - struct QCryptoSecretClass definition + typedef > > - TYPE_CRYPTO_SECRET macro > > - QCRYPT_SECRET() cast macro > > - Setters & getters aka > > qcrypto_secret_prop_set_format > > qcrypto_secret_prop_get_format > > qcrypto_secret_prop_set_data > > qcrypto_secret_prop_get_data > > qcrypto_secret_prop_set_file > > qcrypto_secret_prop_get_file > > qcrypto_secret_prop_set_keyid > > qcrypto_secret_prop_get_keyid > > qcrypto_secret_prop_set_iv > > qcrypto_secret_prop_get_iv > > - qcrypto_secret_finalize > > - qcrypto_secret_class_init > > - TypeInfo qcrypto_secret_info variable > > - qcrypto_secret_register_types() method > > - type_init(qcrypto_secret_register_types); > > > > That'd massively reduce the work to create new objects, to just > > filling in the semantically useful logic > > Promising idea. Sadly, the existing QAPI queue is eating all my QAPI > cycles. Indeed, I've no time either. Perhaps this is something nice for a "lucky" GSoC / OutReachy student :-) Copying Stefan.... Regards, Daniel
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}[,@var{prop1}=@var{value1},...] @@ -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. + @table @option @item -object memory-backend-file,id=@var{id},size=@var{size},mem-path=@var{dir},share=@var{on|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; + + 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; + + 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")) { + 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); + } 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; +} + 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); + } + if (qemu_opts_foreach(qemu_find_opts("object"), user_creatable_add_opts_foreach, object_create_initial, NULL)) {
'-object help' prints available user creatable backends. '-object $typename,help' prints relevant properties. Signed-off-by: Lin Ma <lma@suse.com> --- 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(-)