>>> "Daniel P. Berrange" <berra...@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 <l...@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