[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all a
From: |
Andreas Färber |
Subject: |
Re: [Qemu-devel] 答复: Re: [PATCH v2] object: Add 'help' option for all available backends and properties |
Date: |
Mon, 19 Sep 2016 17:56:01 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 |
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" <address@hidden> writes:
>
>>>>> Markus Armbruster <address@hidden> 2016/9/12 星期一 下午 11:42 >>>
>>> 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/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
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
- [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, 2016/09/12
- [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 <=
- 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
- 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, Daniel P. Berrange, 2016/09/22
Re: [Qemu-devel] [PATCH v2] object: Add 'help' option for all available backends and properties, Daniel P. Berrange, 2016/09/12