[Qemu-devel] [PATCH v4 0/5] object: Add 'help' option for all available backends and properties

[Qemu-devel] [PATCH v4 0/5] object: Add 'help' option for all available backends and properties

[Lin Ma]

V3->V4:
* drop the code about manually define interface 'user-creatable' as abstract.
* add test case for the generated enum value str.
* split the code about adding descriptions to class properties into a separate patch.
* minor changes in user_creatable_help_func to ignore interface TYPE_USER_CREATABLE.

V2->v3:
* make type user-creatable abstract.
* auto generate enum value strings during qemu configuration.(Borrowwed Daniel's code)
* save the generated enum value strings into member description of ObjectProperty.
* drop the judgement logic of whether a property has an enumeration type anymore,
  output member description of ObjectProperty directly.
* at least, user_creatable_help_func should be put after
  'object_property_add_child(object_get_root(), "machine",OBJECT(current_machine), ...)',
  because host_memory_backend_init needs to access an instance of type machine.

V1->V2:
* Output the acceptable values of enum types by "-object TYPE-NAME,help"

Lin Ma (5):
  qom: Add interface check in object_class_is_abstract
  qapi: auto generate enum value strings
  qapi: add test case for the generated enum value str
  backends: add description for enum class properties
  object: Add 'help' option for all available backends and properties

 backends/hostmem.c              |  4 +++
 crypto/secret.c                 |  4 +++
 crypto/tlscreds.c               |  4 +++
 include/qom/object_interfaces.h |  2 ++
 net/filter.c                    |  4 +++
 qemu-options.hx                 |  7 +++++-
 qom/object.c                    |  6 ++++-
 qom/object_interfaces.c         | 55 +++++++++++++++++++++++++++++++++++++++++
 scripts/qapi-types.py           |  2 ++
 scripts/qapi.py                 |  9 +++++++
 tests/test-qmp-commands.c       | 18 ++++++++++++++
 vl.c                            |  5 ++++
 12 files changed, 118 insertions(+), 2 deletions(-)

-- 
2.9.2

[Qemu-devel] [PATCH v4 1/5] qom: Add interface check in object_class_is_abstract

[Lin Ma]

Signed-off-by: Lin Ma <lma@suse.com>
---
 qom/object.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/qom/object.c b/qom/object.c
index 7a05e35..4096645 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -747,7 +747,11 @@ ObjectClass *object_get_class(Object *obj)
 
 bool object_class_is_abstract(ObjectClass *klass)
 {
-    return klass->type->abstract;
+    if (type_is_ancestor(klass->type, type_interface)) {
+        return true;
+    } else {
+        return klass->type->abstract;
+    }
 }
 
 const char *object_class_get_name(ObjectClass *klass)
-- 
2.9.2

[Qemu-devel] 答复： [PATCH v4 1/5] qom: Add interface check in object_class_is_abstract

[Lin Ma]

ping...

>>> Lin Ma <lma@suse.com> 2016/10/20 星期四 下午 7:28 >>>
Signed-off-by: Lin Ma <lma@suse.com>
---
qom/object.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/qom/object.c b/qom/object.c
index 7a05e35..4096645 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -747,7 +747,11 @@ ObjectClass *object_get_class(Object *obj)

bool object_class_is_abstract(ObjectClass *klass)
{
-    return klass->type->abstract;
+    if (type_is_ancestor(klass->type, type_interface)) {
+	    return true;
+    } else {
+	    return klass->type->abstract;
+    }
}

const char *object_class_get_name(ObjectClass *klass)
-- 
2.9.2

Re: [Qemu-devel] [PATCH v4 1/5] qom: Add interface check in object_class_is_abstract

[Markus Armbruster]

Lin Ma <lma@suse.com> writes:

> Signed-off-by: Lin Ma <lma@suse.com>
> ---
>  qom/object.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/qom/object.c b/qom/object.c
> index 7a05e35..4096645 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -747,7 +747,11 @@ ObjectClass *object_get_class(Object *obj)
>  
>  bool object_class_is_abstract(ObjectClass *klass)
>  {
> -    return klass->type->abstract;
> +    if (type_is_ancestor(klass->type, type_interface)) {
> +        return true;
> +    } else {
> +        return klass->type->abstract;
> +    }
>  }
>  
>  const char *object_class_get_name(ObjectClass *klass)

Pardon my ignorance...

If all types derived from type_interface are abstract, why aren't we
setting ->abstract right when such a type is defined?

Hmm, perhaps we do?  type_initialize_interface() sets info.abstract =
true...

In case we don't: what about other uses of ->abstract?  Why is it okay
not to check whether type_interface is an ancestore there?

[Qemu-devel] 答复： Re: [PATCH v4 1/5] qom: Add interface check in object_class_is_abstract

[Lin Ma]

>>> Markus Armbruster <armbru@redhat.com> 2016/11/4 星期五 上午 2:18 >>>
>Lin Ma <lma@suse.com> writes:
>
>> Signed-off-by: Lin Ma <lma@suse.com>
>> ---
>>  qom/object.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/qom/object.c b/qom/object.c
>> index 7a05e35..4096645 100644
>> --- a/qom/object.c
>> +++ b/qom/object.c
>> @@ -747,7 +747,11 @@ ObjectClass *object_get_class(Object *obj)
>>  
>>  bool object_class_is_abstract(ObjectClass *klass)
>>  {
>> -    return klass->type->abstract;
>> +    if (type_is_ancestor(klass->type, type_interface)) {
>> +	    return true;
>> +    } else {
>> +	    return klass->type->abstract;
>> +    }
>>  }
>>  
>>  const char *object_class_get_name(ObjectClass *klass)
>
>Pardon my ignorance...
>
>If all types derived from type_interface are abstract, why aren't we
>setting ->abstract right when such a type is defined?
For lots of interfaces, they already explicitly define abstract as true, say:
generic-pc-machine in hw/i386/pc.c, or memory-backend in backends/hostmem.c
 
Please refer to http://lists.nongnu.org/archive/html/qemu-devel/2016-10/msg01955.html
hope it could give you some clue.
but I see that other interfaces (I randomly picked generic-pc-machine and fw_cfg as examples)
still explicitly define 'abstract = true'.

>Hmm, perhaps we do?  type_initialize_interface() sets info.abstract =
>true...
For interfaces, I guess not.

>In case we don't: what about other uses of ->abstract?  Why is it okay
>not to check whether type_interface is an ancestore there?
For lots of interfaces, they already explicitly define abstract as true, say:
generic-pc-machine in hw/i386/pc.c, or memory-backend in backends/hostmem.c
So they can be checked through  ->abstract, dont need to  check whether type_interface is
an ancestore.
 
Thanks,
Lin

[Qemu-devel] [PATCH v4 2/5] qapi: auto generate enum value strings

[Lin Ma]

Automatically generate enum value strings that containing the acceptable values.
(Borrowed Daniel's code.)

Signed-off-by: Lin Ma <lma@suse.com>
---
 scripts/qapi-types.py | 2 ++
 scripts/qapi.py       | 9 +++++++++
 2 files changed, 11 insertions(+)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index dabc42e..0446839 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -202,9 +202,11 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
             self._btin += gen_enum(name, values, prefix)
             if do_builtins:
                 self.defn += gen_enum_lookup(name, values, prefix)
+                self._btin += gen_enum_value_str(name, values)
         else:
             self._fwdecl += gen_enum(name, values, prefix)
             self.defn += gen_enum_lookup(name, values, prefix)
+            self._fwdecl += gen_enum_value_str(name, values)
 
     def visit_array_type(self, name, info, element_type):
         if isinstance(element_type, QAPISchemaBuiltinType):
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 21bc32f..d11c414 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1649,6 +1649,15 @@ const char *const %(c_name)s_lookup[] = {
     return ret
 
 
+def gen_enum_value_str(name, values):
+    return mcgen('''
+
+#define %(c_name)s_value_str "%(value_str)s"
+''',
+                c_name=c_name(name),
+                value_str=", ".join(["'%s'" % c for c in values]))
+
+
 def gen_enum(name, values, prefix=None):
     # append automatically generated _MAX value
     enum_values = values + ['_MAX']
-- 
2.9.2

[Qemu-devel] 答复： [PATCH v4 2/5] qapi: auto generate enum value strings

[Lin Ma]

ping...

>>> Lin Ma <lma@suse.com> 2016/10/20 星期四 下午 7:28 >>>
Automatically generate enum value strings that containing the acceptable values.
(Borrowed Daniel's code.)

Signed-off-by: Lin Ma <lma@suse.com>
---
scripts/qapi-types.py | 2 ++
scripts/qapi.py	   | 9 +++++++++
2 files changed, 11 insertions(+)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index dabc42e..0446839 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -202,9 +202,11 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
			 self._btin += gen_enum(name, values, prefix)
			 if do_builtins:
				 self.defn += gen_enum_lookup(name, values, prefix)
+			    self._btin += gen_enum_value_str(name, values)
		 else:
			 self._fwdecl += gen_enum(name, values, prefix)
			 self.defn += gen_enum_lookup(name, values, prefix)
+		    self._fwdecl += gen_enum_value_str(name, values)

	 def visit_array_type(self, name, info, element_type):
		 if isinstance(element_type, QAPISchemaBuiltinType):
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 21bc32f..d11c414 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1649,6 +1649,15 @@ const char *const %(c_name)s_lookup[] = {
	 return ret


+def gen_enum_value_str(name, values):
+    return mcgen('''
+
+#define %(c_name)s_value_str "%(value_str)s"
+''',
+			    c_name=c_name(name),
+			    value_str=", ".join(["'%s'" % c for c in values]))
+
+
def gen_enum(name, values, prefix=None):
	 # append automatically generated _MAX value
	 enum_values = values + ['_MAX']
-- 
2.9.2

Re: [Qemu-devel] [PATCH v4 2/5] qapi: auto generate enum value strings

[Markus Armbruster]

Lin Ma <lma@suse.com> writes:

> Automatically generate enum value strings that containing the acceptable values.
> (Borrowed Daniel's code.)
>
> Signed-off-by: Lin Ma <lma@suse.com>
> ---
>  scripts/qapi-types.py | 2 ++
>  scripts/qapi.py       | 9 +++++++++
>  2 files changed, 11 insertions(+)
>
> diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
> index dabc42e..0446839 100644
> --- a/scripts/qapi-types.py
> +++ b/scripts/qapi-types.py
> @@ -202,9 +202,11 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
>              self._btin += gen_enum(name, values, prefix)
>              if do_builtins:
>                  self.defn += gen_enum_lookup(name, values, prefix)
> +                self._btin += gen_enum_value_str(name, values)
>          else:
>              self._fwdecl += gen_enum(name, values, prefix)
>              self.defn += gen_enum_lookup(name, values, prefix)
> +            self._fwdecl += gen_enum_value_str(name, values)
>  
>      def visit_array_type(self, name, info, element_type):
>          if isinstance(element_type, QAPISchemaBuiltinType):
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 21bc32f..d11c414 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -1649,6 +1649,15 @@ const char *const %(c_name)s_lookup[] = {
>      return ret
>  
>  
> +def gen_enum_value_str(name, values):
> +    return mcgen('''
> +
> +#define %(c_name)s_value_str "%(value_str)s"
> +''',
> +                c_name=c_name(name),
> +                value_str=", ".join(["'%s'" % c for c in values]))
> +
> +
>  def gen_enum(name, values, prefix=None):
>      # append automatically generated _MAX value
>      enum_values = values + ['_MAX']

This function is generating a macro definition, not a string.  We could
call it gen_enum_values_define().  But I'd simply fold it into
gen_enum().

Adds another 9KiB to qapi-types.h, which is included widely.  Instead of
defining these macros, we could also iterate over FOO_lookup[] at
run-time.  But let's first review how the macro is used.

[Qemu-devel] 答复： Re: [PATCH v4 2/5] qapi: auto generate enum value strings

[Lin Ma]

>>> Markus Armbruster <armbru@redhat.com> 2016/11/4 星期五 上午 3:17 >>>
>Lin Ma <lma@suse.com> writes:
>
>> Automatically generate enum value strings that containing the acceptable values.
>> (Borrowed Daniel's code.)
>>
>> Signed-off-by: Lin Ma <lma@suse.com>
>> ---
>>  scripts/qapi-types.py | 2 ++
>>  scripts/qapi.py       | 9 +++++++++
>>  2 files changed, 11 insertions(+)
>>
>> diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
>> index dabc42e..0446839 100644
>> --- a/scripts/qapi-types.py
>> +++ b/scripts/qapi-types.py
>> @@ -202,9 +202,11 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
>>			  self._btin += gen_enum(name, values, prefix)
>>			  if do_builtins:
>>				  self.defn += gen_enum_lookup(name, values, prefix)
>> +			    self._btin += gen_enum_value_str(name, values)
>>		  else:
>>			  self._fwdecl += gen_enum(name, values, prefix)
>>			  self.defn += gen_enum_lookup(name, values, prefix)
>> +		    self._fwdecl += gen_enum_value_str(name, values)
>>  
>>	  def visit_array_type(self, name, info, element_type):
>>		  if isinstance(element_type, QAPISchemaBuiltinType):
>> diff --git a/scripts/qapi.py b/scripts/qapi.py
>> index 21bc32f..d11c414 100644
>> --- a/scripts/qapi.py
>> +++ b/scripts/qapi.py
>> @@ -1649,6 +1649,15 @@ const char *const %(c_name)s_lookup[] = {
>>	  return ret
>>  
>>  
>> +def gen_enum_value_str(name, values):
>> +    return mcgen('''
>> +
>> +#define %(c_name)s_value_str "%(value_str)s"
>> +''',
>> +			    c_name=c_name(name),
>> +			    value_str=", ".join(["'%s'" % c for c in values]))
>> +
>> +
>>  def gen_enum(name, values, prefix=None):
>>	  # append automatically generated _MAX value
>>	  enum_values = values + ['_MAX']
>
>This function is generating a macro definition, not a string.  We could
>call it gen_enum_values_define().  But I'd simply fold it into
>gen_enum().
>
>Adds another 9KiB to qapi-types.h, which is included widely.  Instead of
>defining these macros, we could also iterate over FOO_lookup[] at
>run-time.  But let's first review how the macro is used.
Sorry, I'm not familiar with writing qemu test code. I didn't understand it.
What's the meaning of 'Adds another 9KiB to qapi-types.h'? What is '9KiB'?
 
Thanks,
Lin
 

 

[Qemu-devel] [PATCH v4 3/5] qapi: add test case for the generated enum value str

[Lin Ma]

Signed-off-by: Lin Ma <lma@suse.com>
---
 tests/test-qmp-commands.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
index 81cbe54..9cd61b2 100644
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -262,6 +262,23 @@ static void test_dealloc_partial(void)
     qapi_free_UserDefTwo(ud2);
 }
 
+/* test generated enum value str */
+static void test_enum_value_str(void)
+{
+    EnumOne i;
+    char *expected_str = NULL;
+
+    for (i = 0; i < ENUM_ONE__MAX; i++) {
+        if (i == 0) {
+            expected_str = g_strdup_printf("\'%s\'", EnumOne_lookup[i]);
+        } else {
+            expected_str = g_strdup_printf("%s, \'%s\'",
+                                            expected_str, EnumOne_lookup[i]);
+        }
+    }
+    g_assert_cmpstr(EnumOne_value_str, ==, expected_str);
+}
+
 
 int main(int argc, char **argv)
 {
@@ -272,6 +289,7 @@ int main(int argc, char **argv)
     g_test_add_func("/0.15/dispatch_cmd_io", test_dispatch_cmd_io);
     g_test_add_func("/0.15/dealloc_types", test_dealloc_types);
     g_test_add_func("/0.15/dealloc_partial", test_dealloc_partial);
+    g_test_add_func("/0.15/enum_value_str", test_enum_value_str);
 
     module_call_init(MODULE_INIT_QAPI);
     g_test_run();
-- 
2.9.2

[Qemu-devel] 答复： [PATCH v4 3/5] qapi: add test case for the generated enum value str

[Lin Ma]

ping...

>>> Lin Ma <lma@suse.com> 2016/10/20 星期四 下午 7:28 >>>
Signed-off-by: Lin Ma <lma@suse.com>
---
tests/test-qmp-commands.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
index 81cbe54..9cd61b2 100644
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -262,6 +262,23 @@ static void test_dealloc_partial(void)
	 qapi_free_UserDefTwo(ud2);
}

+/* test generated enum value str */
+static void test_enum_value_str(void)
+{
+    EnumOne i;
+    char *expected_str = NULL;
+
+    for (i = 0; i < ENUM_ONE__MAX; i++) {
+	    if (i == 0) {
+		    expected_str = g_strdup_printf("\'%s\'", EnumOne_lookup[i]);
+	    } else {
+		    expected_str = g_strdup_printf("%s, \'%s\'",
+										    expected_str, EnumOne_lookup[i]);
+	    }
+    }
+    g_assert_cmpstr(EnumOne_value_str, ==, expected_str);
+}
+

int main(int argc, char **argv)
{
@@ -272,6 +289,7 @@ int main(int argc, char **argv)
	 g_test_add_func("/0.15/dispatch_cmd_io", test_dispatch_cmd_io);
	 g_test_add_func("/0.15/dealloc_types", test_dealloc_types);
	 g_test_add_func("/0.15/dealloc_partial", test_dealloc_partial);
+    g_test_add_func("/0.15/enum_value_str", test_enum_value_str);

	 module_call_init(MODULE_INIT_QAPI);
	 g_test_run();
-- 
2.9.2

[Qemu-devel] [PATCH v4 4/5] backends: add description for enum class properties

[Lin Ma]

Signed-off-by: Lin Ma <lma@suse.com>
---
 backends/hostmem.c | 4 ++++
 crypto/secret.c    | 4 ++++
 crypto/tlscreds.c  | 4 ++++
 net/filter.c       | 4 ++++
 4 files changed, 16 insertions(+)

diff --git a/backends/hostmem.c b/backends/hostmem.c
index 4256d24..25f303d 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -377,6 +377,10 @@ host_memory_backend_class_init(ObjectClass *oc, void *data)
         HostMemPolicy_lookup,
         host_memory_backend_get_policy,
         host_memory_backend_set_policy, &error_abort);
+    object_class_property_set_description(oc, "policy",
+                                    "Data format: one of "
+                                    HostMemPolicy_value_str,
+                                    &error_abort);
 }
 
 static const TypeInfo host_memory_backend_info = {
diff --git a/crypto/secret.c b/crypto/secret.c
index 285ab7a..71d06e3 100644
--- a/crypto/secret.c
+++ b/crypto/secret.c
@@ -382,6 +382,10 @@ qcrypto_secret_class_init(ObjectClass *oc, void *data)
                                    qcrypto_secret_prop_get_format,
                                    qcrypto_secret_prop_set_format,
                                    NULL);
+    object_class_property_set_description(oc, "format",
+                                          "Data format: one of "
+                                          QCryptoSecretFormat_value_str,
+                                          &error_abort);
     object_class_property_add_str(oc, "data",
                                   qcrypto_secret_prop_get_data,
                                   qcrypto_secret_prop_set_data,
diff --git a/crypto/tlscreds.c b/crypto/tlscreds.c
index a896553..d3af38a 100644
--- a/crypto/tlscreds.c
+++ b/crypto/tlscreds.c
@@ -237,6 +237,10 @@ qcrypto_tls_creds_class_init(ObjectClass *oc, void *data)
                                    qcrypto_tls_creds_prop_get_endpoint,
                                    qcrypto_tls_creds_prop_set_endpoint,
                                    NULL);
+    object_class_property_set_description(oc, "endpoint",
+                                          "Data format: one of "
+                                          QCryptoTLSCredsEndpoint_value_str,
+                                          &error_abort);
     object_class_property_add_str(oc, "priority",
                                   qcrypto_tls_creds_prop_get_priority,
                                   qcrypto_tls_creds_prop_set_priority,
diff --git a/net/filter.c b/net/filter.c
index 1dfd2ca..205fb49 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -182,6 +182,10 @@ static void netfilter_init(Object *obj)
                              NetFilterDirection_lookup,
                              netfilter_get_direction, netfilter_set_direction,
                              NULL);
+    object_property_set_description(obj, "queue",
+                                    "Data format: one of "
+                                    NetFilterDirection_value_str,
+                                    &error_abort);
     object_property_add_str(obj, "status",
                             netfilter_get_status, netfilter_set_status,
                             NULL);
-- 
2.9.2

[Qemu-devel] 答复： [PATCH v4 4/5] backends: add description for enum class properties

[Lin Ma]

ping...

>>> Lin Ma <lma@suse.com> 2016/10/20 星期四 下午 7:28 >>>
Signed-off-by: Lin Ma <lma@suse.com>
---
backends/hostmem.c | 4 ++++
crypto/secret.c    | 4 ++++
crypto/tlscreds.c  | 4 ++++
net/filter.c	   | 4 ++++
4 files changed, 16 insertions(+)

diff --git a/backends/hostmem.c b/backends/hostmem.c
index 4256d24..25f303d 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -377,6 +377,10 @@ host_memory_backend_class_init(ObjectClass *oc, void *data)
		 HostMemPolicy_lookup,
		 host_memory_backend_get_policy,
		 host_memory_backend_set_policy, &error_abort);
+    object_class_property_set_description(oc, "policy",
+								    "Data format: one of "
+								    HostMemPolicy_value_str,
+								    &error_abort);
}

static const TypeInfo host_memory_backend_info = {
diff --git a/crypto/secret.c b/crypto/secret.c
index 285ab7a..71d06e3 100644
--- a/crypto/secret.c
+++ b/crypto/secret.c
@@ -382,6 +382,10 @@ qcrypto_secret_class_init(ObjectClass *oc, void *data)
								    qcrypto_secret_prop_get_format,
								    qcrypto_secret_prop_set_format,
								    NULL);
+    object_class_property_set_description(oc, "format",
+										  "Data format: one of "
+										  QCryptoSecretFormat_value_str,
+										  &error_abort);
	 object_class_property_add_str(oc, "data",
								   qcrypto_secret_prop_get_data,
								   qcrypto_secret_prop_set_data,
diff --git a/crypto/tlscreds.c b/crypto/tlscreds.c
index a896553..d3af38a 100644
--- a/crypto/tlscreds.c
+++ b/crypto/tlscreds.c
@@ -237,6 +237,10 @@ qcrypto_tls_creds_class_init(ObjectClass *oc, void *data)
								    qcrypto_tls_creds_prop_get_endpoint,
								    qcrypto_tls_creds_prop_set_endpoint,
								    NULL);
+    object_class_property_set_description(oc, "endpoint",
+										  "Data format: one of "
+										  QCryptoTLSCredsEndpoint_value_str,
+										  &error_abort);
	 object_class_property_add_str(oc, "priority",
								   qcrypto_tls_creds_prop_get_priority,
								   qcrypto_tls_creds_prop_set_priority,
diff --git a/net/filter.c b/net/filter.c
index 1dfd2ca..205fb49 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -182,6 +182,10 @@ static void netfilter_init(Object *obj)
							  NetFilterDirection_lookup,
							  netfilter_get_direction, netfilter_set_direction,
							  NULL);
+    object_property_set_description(obj, "queue",
+								    "Data format: one of "
+								    NetFilterDirection_value_str,
+								    &error_abort);
	 object_property_add_str(obj, "status",
							 netfilter_get_status, netfilter_set_status,
							 NULL);
-- 
2.9.2

Re: [Qemu-devel] [PATCH v4 4/5] backends: add description for enum class properties

[Markus Armbruster]

Lin Ma <lma@suse.com> writes:

> Signed-off-by: Lin Ma <lma@suse.com>
> ---
>  backends/hostmem.c | 4 ++++
>  crypto/secret.c    | 4 ++++
>  crypto/tlscreds.c  | 4 ++++
>  net/filter.c       | 4 ++++
>  4 files changed, 16 insertions(+)
>
> diff --git a/backends/hostmem.c b/backends/hostmem.c
> index 4256d24..25f303d 100644
> --- a/backends/hostmem.c
> +++ b/backends/hostmem.c
> @@ -377,6 +377,10 @@ host_memory_backend_class_init(ObjectClass *oc, void *data)
>          HostMemPolicy_lookup,
>          host_memory_backend_get_policy,
>          host_memory_backend_set_policy, &error_abort);
> +    object_class_property_set_description(oc, "policy",
> +                                    "Data format: one of "
> +                                    HostMemPolicy_value_str,
> +                                    &error_abort);
>  }
>  
>  static const TypeInfo host_memory_backend_info = {
> diff --git a/crypto/secret.c b/crypto/secret.c
> index 285ab7a..71d06e3 100644
> --- a/crypto/secret.c
> +++ b/crypto/secret.c
> @@ -382,6 +382,10 @@ qcrypto_secret_class_init(ObjectClass *oc, void *data)
>                                     qcrypto_secret_prop_get_format,
>                                     qcrypto_secret_prop_set_format,
>                                     NULL);
> +    object_class_property_set_description(oc, "format",
> +                                          "Data format: one of "
> +                                          QCryptoSecretFormat_value_str,
> +                                          &error_abort);
>      object_class_property_add_str(oc, "data",
>                                    qcrypto_secret_prop_get_data,
>                                    qcrypto_secret_prop_set_data,
> diff --git a/crypto/tlscreds.c b/crypto/tlscreds.c
> index a896553..d3af38a 100644
> --- a/crypto/tlscreds.c
> +++ b/crypto/tlscreds.c
> @@ -237,6 +237,10 @@ qcrypto_tls_creds_class_init(ObjectClass *oc, void *data)
>                                     qcrypto_tls_creds_prop_get_endpoint,
>                                     qcrypto_tls_creds_prop_set_endpoint,
>                                     NULL);
> +    object_class_property_set_description(oc, "endpoint",
> +                                          "Data format: one of "
> +                                          QCryptoTLSCredsEndpoint_value_str,
> +                                          &error_abort);
>      object_class_property_add_str(oc, "priority",
>                                    qcrypto_tls_creds_prop_get_priority,
>                                    qcrypto_tls_creds_prop_set_priority,
> diff --git a/net/filter.c b/net/filter.c
> index 1dfd2ca..205fb49 100644
> --- a/net/filter.c
> +++ b/net/filter.c
> @@ -182,6 +182,10 @@ static void netfilter_init(Object *obj)
>                               NetFilterDirection_lookup,
>                               netfilter_get_direction, netfilter_set_direction,
>                               NULL);
> +    object_property_set_description(obj, "queue",
> +                                    "Data format: one of "
> +                                    NetFilterDirection_value_str,
> +                                    &error_abort);
>      object_property_add_str(obj, "status",
>                              netfilter_get_status, netfilter_set_status,
>                              NULL);

After  each

    object_class_property_add_enum(OC, PROP_NAME", ENUM_NAME, ...);

you add

    object_class_property_set_description(OC, PROP_NAME,
                                          "Data format: one of "
                                          ENUM_NAME_value_str,
                                          &error_abort);

Whenever somebody adds another object_class_property_add_enum(), he
should also add an object_class_property_set_description().  Easily
forgotten.

Why not do it in object_class_property_add_enum() automatically?
We got strings[] there...

[Qemu-devel] 答复： Re: [PATCH v4 4/5] backends: add description for enum class properties

[Lin Ma]

>>> Markus Armbruster <armbru@redhat.com> 2016/11/4 星期五 上午 3:58 >>>
>Lin Ma <lma@suse.com> writes:
>
>> Signed-off-by: Lin Ma <lma@suse.com>
>> ---
>>  backends/hostmem.c | 4 ++++
>>  crypto/secret.c    | 4 ++++
>>  crypto/tlscreds.c  | 4 ++++
>>  net/filter.c       | 4 ++++
>>  4 files changed, 16 insertions(+)
>>
>> diff --git a/backends/hostmem.c b/backends/hostmem.c
>> index 4256d24..25f303d 100644
>> --- a/backends/hostmem.c
>> +++ b/backends/hostmem.c
>> @@ -377,6 +377,10 @@ host_memory_backend_class_init(ObjectClass *oc, void *data)
>>		  HostMemPolicy_lookup,
>>		  host_memory_backend_get_policy,
>>		  host_memory_backend_set_policy, &error_abort);
>> +    object_class_property_set_description(oc, "policy",
>> +								    "Data format: one of "
>> +								    HostMemPolicy_value_str,
>> +								    &error_abort);
>>  }
>>  
>>  static const TypeInfo host_memory_backend_info = {
>> diff --git a/crypto/secret.c b/crypto/secret.c
>> index 285ab7a..71d06e3 100644
>> --- a/crypto/secret.c
>> +++ b/crypto/secret.c
>> @@ -382,6 +382,10 @@ qcrypto_secret_class_init(ObjectClass *oc, void *data)
>>									 qcrypto_secret_prop_get_format,
>>									 qcrypto_secret_prop_set_format,
>>									 NULL);
>> +    object_class_property_set_description(oc, "format",
>> +										  "Data format: one of "
>> +										  QCryptoSecretFormat_value_str,
>> +										  &error_abort);
>>	  object_class_property_add_str(oc, "data",
>>								    qcrypto_secret_prop_get_data,
>>								    qcrypto_secret_prop_set_data,
>> diff --git a/crypto/tlscreds.c b/crypto/tlscreds.c
>> index a896553..d3af38a 100644
>> --- a/crypto/tlscreds.c
>> +++ b/crypto/tlscreds.c
>> @@ -237,6 +237,10 @@ qcrypto_tls_creds_class_init(ObjectClass *oc, void *data)
>>									 qcrypto_tls_creds_prop_get_endpoint,
>>									 qcrypto_tls_creds_prop_set_endpoint,
>>									 NULL);
>> +    object_class_property_set_description(oc, "endpoint",
>> +										  "Data format: one of "
>> +										  QCryptoTLSCredsEndpoint_value_str,
>> +										  &error_abort);
>>	  object_class_property_add_str(oc, "priority",
>>								    qcrypto_tls_creds_prop_get_priority,
>>								    qcrypto_tls_creds_prop_set_priority,
>> diff --git a/net/filter.c b/net/filter.c
>> index 1dfd2ca..205fb49 100644
>> --- a/net/filter.c
>> +++ b/net/filter.c
>> @@ -182,6 +182,10 @@ static void netfilter_init(Object *obj)
>>							   NetFilterDirection_lookup,
>>							   netfilter_get_direction, netfilter_set_direction,
>>							   NULL);
>> +    object_property_set_description(obj, "queue",
>> +								    "Data format: one of "
>> +								    NetFilterDirection_value_str,
>> +								    &error_abort);
>>	  object_property_add_str(obj, "status",
>>							  netfilter_get_status, netfilter_set_status,
>>							  NULL);
>
>After  each
>
>    object_class_property_add_enum(OC, PROP_NAME", ENUM_NAME, ...);
>
>you add
>
>    object_class_property_set_description(OC, PROP_NAME,
>										  "Data format: one of "
>										  ENUM_NAME_value_str,
>										  &error_abort);
>
>Whenever somebody adds another object_class_property_add_enum(), he
>should also add an object_class_property_set_description().  Easily
>forgotten.
>
>Why not do it in object_class_property_add_enum() automatically?
>We got strings[] there...
Because in the help output, I'd like to treat the output as two part:
enum type name + description of enum type name.
So I didn't add it in object_class_property_add_enum().
Yes, it is easy to forget calling object_class_property_set_description().
But is it really better that combine it into object_class_property_add_enum()?
 
Thanks,
Lin

Re: [Qemu-devel] [PATCH v4 4/5] backends: add description for enum class properties

[Daniel P. Berrange]

On Thu, Nov 03, 2016 at 08:58:53PM +0100, Markus Armbruster wrote:
> Lin Ma <lma@suse.com> writes:
> 
> > Signed-off-by: Lin Ma <lma@suse.com>
> > ---
> >  backends/hostmem.c | 4 ++++
> >  crypto/secret.c    | 4 ++++
> >  crypto/tlscreds.c  | 4 ++++
> >  net/filter.c       | 4 ++++
> >  4 files changed, 16 insertions(+)
> >
> > diff --git a/backends/hostmem.c b/backends/hostmem.c
> > index 4256d24..25f303d 100644
> > --- a/backends/hostmem.c
> > +++ b/backends/hostmem.c
> > @@ -377,6 +377,10 @@ host_memory_backend_class_init(ObjectClass *oc, void *data)
> >          HostMemPolicy_lookup,
> >          host_memory_backend_get_policy,
> >          host_memory_backend_set_policy, &error_abort);
> > +    object_class_property_set_description(oc, "policy",
> > +                                    "Data format: one of "
> > +                                    HostMemPolicy_value_str,
> > +                                    &error_abort);
> >  }
> >  
> >  static const TypeInfo host_memory_backend_info = {
> > diff --git a/crypto/secret.c b/crypto/secret.c
> > index 285ab7a..71d06e3 100644
> > --- a/crypto/secret.c
> > +++ b/crypto/secret.c
> > @@ -382,6 +382,10 @@ qcrypto_secret_class_init(ObjectClass *oc, void *data)
> >                                     qcrypto_secret_prop_get_format,
> >                                     qcrypto_secret_prop_set_format,
> >                                     NULL);
> > +    object_class_property_set_description(oc, "format",
> > +                                          "Data format: one of "
> > +                                          QCryptoSecretFormat_value_str,
> > +                                          &error_abort);
> >      object_class_property_add_str(oc, "data",
> >                                    qcrypto_secret_prop_get_data,
> >                                    qcrypto_secret_prop_set_data,
> > diff --git a/crypto/tlscreds.c b/crypto/tlscreds.c
> > index a896553..d3af38a 100644
> > --- a/crypto/tlscreds.c
> > +++ b/crypto/tlscreds.c
> > @@ -237,6 +237,10 @@ qcrypto_tls_creds_class_init(ObjectClass *oc, void *data)
> >                                     qcrypto_tls_creds_prop_get_endpoint,
> >                                     qcrypto_tls_creds_prop_set_endpoint,
> >                                     NULL);
> > +    object_class_property_set_description(oc, "endpoint",
> > +                                          "Data format: one of "
> > +                                          QCryptoTLSCredsEndpoint_value_str,
> > +                                          &error_abort);
> >      object_class_property_add_str(oc, "priority",
> >                                    qcrypto_tls_creds_prop_get_priority,
> >                                    qcrypto_tls_creds_prop_set_priority,
> > diff --git a/net/filter.c b/net/filter.c
> > index 1dfd2ca..205fb49 100644
> > --- a/net/filter.c
> > +++ b/net/filter.c
> > @@ -182,6 +182,10 @@ static void netfilter_init(Object *obj)
> >                               NetFilterDirection_lookup,
> >                               netfilter_get_direction, netfilter_set_direction,
> >                               NULL);
> > +    object_property_set_description(obj, "queue",
> > +                                    "Data format: one of "
> > +                                    NetFilterDirection_value_str,
> > +                                    &error_abort);
> >      object_property_add_str(obj, "status",
> >                              netfilter_get_status, netfilter_set_status,
> >                              NULL);
> 
> After  each
> 
>     object_class_property_add_enum(OC, PROP_NAME", ENUM_NAME, ...);
> 
> you add
> 
>     object_class_property_set_description(OC, PROP_NAME,
>                                           "Data format: one of "
>                                           ENUM_NAME_value_str,
>                                           &error_abort);
> 
> Whenever somebody adds another object_class_property_add_enum(), he
> should also add an object_class_property_set_description().  Easily
> forgotten.
> 
> Why not do it in object_class_property_add_enum() automatically?
> We got strings[] there...

We could make every single object_property_add_* method accept a
description, but it'd be a huge code-churn to retrofit.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|

[Qemu-devel] [PATCH v4 5/5] object: Add 'help' option for all available backends and properties

[Lin Ma]

'-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         | 55 +++++++++++++++++++++++++++++++++++++++++
 vl.c                            |  5 ++++
 4 files changed, 68 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 b1fbdb0..b9573ce 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3761,7 +3761,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},...]
@@ -3771,6 +3773,9 @@ in the order they are specified.  Note that the 'id'
 property must be set.  These objects are placed in the
 '/objects' path.
 
+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..da8be39 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,60 @@ void user_creatable_del(const char *id, Error **errp)
     object_unparent(obj);
 }
 
+int user_creatable_help_func(void *opaque, QemuOpts *opts, Error **errp)
+{
+    const char *type = NULL;
+    Object *obj = NULL;
+    ObjectClass *klass;
+    ObjectProperty *prop;
+    ObjectPropertyIterator iter;
+
+    type = qemu_opt_get(opts, "qom-type");
+    if (type && is_help_option(type)) {
+        GSList *list;
+        printf("Available object backend types:\n");
+        for (list = object_class_get_list(TYPE_USER_CREATABLE, false);  \
+                list;                                                   \
+                list = list->next) {
+            const char *name;
+            name = object_class_get_name(OBJECT_CLASS(list->data));
+            if (strcmp(name, TYPE_USER_CREATABLE)) {
+                printf("%s\n", name);
+            }
+        }
+        g_slist_free(list);
+        goto out;
+    }
+
+    if (!type || !qemu_opt_has_help_opt(opts)) {
+        return 0;
+    }
+
+    klass = object_class_by_name(type);
+    if (!klass) {
+        printf("invalid object type: %s\n", type);
+        goto out;
+    }
+    if (object_class_is_abstract(klass)) {
+        printf("object type '%s' is abstract\n", type);
+        goto out;
+    }
+    obj = object_new(type);
+    object_property_iter_init(&iter, obj);
+
+    while ((prop = object_property_iter_next(&iter))) {
+        if (prop->description) {
+            printf("%s (%s, %s)\n", prop->name, prop->type, prop->description);
+        } else {
+            printf("%s (%s)\n", prop->name, prop->type);
+        }
+    }
+
+out:
+    object_unref(obj);
+    return 1;
+}
+
 static void register_types(void)
 {
     static const TypeInfo uc_interface_info = {
diff --git a/vl.c b/vl.c
index ebd47af..1456eca 100644
--- a/vl.c
+++ b/vl.c
@@ -4100,6 +4100,11 @@ int main(int argc, char **argv, char **envp)
         exit(0);
     }
 
+    if (qemu_opts_foreach(qemu_find_opts("object"), user_creatable_help_func,
+                          NULL, NULL)) {
+        exit(1);
+    }
+
     if (!trace_init_backends()) {
         exit(1);
     }
-- 
2.9.2

[Qemu-devel] 答复： [PATCH v4 5/5] object: Add 'help' option for all available backends and properties

[Lin Ma]

ping...

>>> Lin Ma <lma@suse.com> 2016/10/20 星期四 下午 7:28 >>>
'-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		 | 55 +++++++++++++++++++++++++++++++++++++++++
vl.c						    |  5 ++++
4 files changed, 68 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 b1fbdb0..b9573ce 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3761,7 +3761,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},...]
@@ -3771,6 +3773,9 @@ in the order they are specified.  Note that the 'id'
property must be set.  These objects are placed in the
'/objects' path.

+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..da8be39 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,60 @@ void user_creatable_del(const char *id, Error **errp)
	 object_unparent(obj);
}

+int user_creatable_help_func(void *opaque, QemuOpts *opts, Error **errp)
+{
+    const char *type = NULL;
+    Object *obj = NULL;
+    ObjectClass *klass;
+    ObjectProperty *prop;
+    ObjectPropertyIterator iter;
+
+    type = qemu_opt_get(opts, "qom-type");
+    if (type && is_help_option(type)) {
+	    GSList *list;
+	    printf("Available object backend types:\n");
+	    for (list = object_class_get_list(TYPE_USER_CREATABLE, false);  \
+			    list; 												  \
+			    list = list->next) {
+		    const char *name;
+		    name = object_class_get_name(OBJECT_CLASS(list->data));
+		    if (strcmp(name, TYPE_USER_CREATABLE)) {
+			    printf("%s\n", name);
+		    }
+	    }
+	    g_slist_free(list);
+	    goto out;
+    }
+
+    if (!type || !qemu_opt_has_help_opt(opts)) {
+	    return 0;
+    }
+
+    klass = object_class_by_name(type);
+    if (!klass) {
+	    printf("invalid object type: %s\n", type);
+	    goto out;
+    }
+    if (object_class_is_abstract(klass)) {
+	    printf("object type '%s' is abstract\n", type);
+	    goto out;
+    }
+    obj = object_new(type);
+    object_property_iter_init(&iter, obj);
+
+    while ((prop = object_property_iter_next(&iter))) {
+	    if (prop->description) {
+		    printf("%s (%s, %s)\n", prop->name, prop->type, prop->description);
+	    } else {
+		    printf("%s (%s)\n", prop->name, prop->type);
+	    }
+    }
+
+out:
+    object_unref(obj);
+    return 1;
+}
+
static void register_types(void)
{
	 static const TypeInfo uc_interface_info = {
diff --git a/vl.c b/vl.c
index ebd47af..145
6eca 100644
--- a/vl.c
+++ b/vl.c
@@ -4100,6 +4100,11 @@ int main(int argc, char **argv, char **envp)
		 exit(0);
	 }

+    if (qemu_opts_foreach(qemu_find_opts("object"), user_creatable_help_func,
+						  NULL, NULL)) {
+	    exit(1);
+    }
+
	 if (!trace_init_backends()) {
		 exit(1);
	 }
-- 
2.9.2

Re: [Qemu-devel] [PATCH v4 5/5] object: Add 'help' option for all available backends and properties

[Markus Armbruster]

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         | 55 +++++++++++++++++++++++++++++++++++++++++
>  vl.c                            |  5 ++++
>  4 files changed, 68 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);
> +

Ugly interface: two arguments that aren't used, one of them void *.
Suggest to follow the device help example: do a non-ugly interface here,
and an ugly, but static wrapper in vl.c

>  #endif
> diff --git a/qemu-options.hx b/qemu-options.hx
> index b1fbdb0..b9573ce 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -3761,7 +3761,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",

The existing text talks about "object of type TYPENAME".  The new text
suddenly talks about "backend types".  Switching terminology is not a
good idea.  Call them "object types".

For comparison, here's the text we use for -device:

                use '-device help' to print all possible drivers
                use '-device driver,help' to print all possible properties

Let's make this one similar:

                use '-object help' to print all possible object types
                use '-object driver,help' to print all possible properties

>      QEMU_ARCH_ALL)
>  STEXI
>  @item -object @var{typename}[,@var{prop1}=@var{value1},...]
> @@ -3771,6 +3773,9 @@ in the order they are specified.  Note that the 'id'
>  property must be set.  These objects are placed in the
>  '/objects' path.
>  
> +Use @code{-object help} to print available backend types and
> +@code{-object @var{typename},help} to print relevant properties.
> +

-device's text:

    To get help on possible drivers and properties, use @code{-device
    help} and @code{-device @var{driver},help}.

Suggest:

    To get help on possible object types and properties, use @code{-object
    help} and @code{-object @var{type},help}.

>  @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..da8be39 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,60 @@ void user_creatable_del(const char *id, Error **errp)
>      object_unparent(obj);
>  }
>  
> +int user_creatable_help_func(void *opaque, QemuOpts *opts, Error **errp)
> +{
> +    const char *type = NULL;
> +    Object *obj = NULL;
> +    ObjectClass *klass;
> +    ObjectProperty *prop;
> +    ObjectPropertyIterator iter;
> +
> +    type = qemu_opt_get(opts, "qom-type");
> +    if (type && is_help_option(type)) {
> +        GSList *list;
> +        printf("Available object backend types:\n");

Wrong when running for an HMP monitor command, e.g. "object_add help".
Use error_printf().  That's only slightly wrong in that it prints to
stderr instead of stdout running for the command line.

> +        for (list = object_class_get_list(TYPE_USER_CREATABLE, false);  \
> +                list;                                                   \
> +                list = list->next) {

Superfluous line continuations.  Simply write

           for (list = object_class_get_list(TYPE_USER_CREATABLE, false);
                list;
                list = list->next) {

Alternatively,

           list = object_class_get_list(TYPE_USER_CREATABLE, false);
           for (elt = list; elt; elt = elt->next) {

> +            const char *name;
> +            name = object_class_get_name(OBJECT_CLASS(list->data));
> +            if (strcmp(name, TYPE_USER_CREATABLE)) {
> +                printf("%s\n", name);

error_printf()

> +            }
> +        }
> +        g_slist_free(list);

Leaks the list, because by now @list is null.  Suggest to fix by
adopting the code under "alternatively".

> +        goto out;

Suggest return 1, because ...

> +    }
> +
> +    if (!type || !qemu_opt_has_help_opt(opts)) {
> +        return 0;

... you're not going to out here.

No need to initialize @obj then.

> +    }
> +
> +    klass = object_class_by_name(type);
> +    if (!klass) {
> +        printf("invalid object type: %s\n", type);

Please use error_report().

> +        goto out;

return 1 would do.

> +    }
> +    if (object_class_is_abstract(klass)) {
> +        printf("object type '%s' is abstract\n", type);

Please use error_report().

> +        goto out;

return 1 would do.

> +    }

I'd put a blank line here ...

> +    obj = object_new(type);
> +    object_property_iter_init(&iter, obj);
> +

... instead of here.

> +    while ((prop = object_property_iter_next(&iter))) {
> +        if (prop->description) {
> +            printf("%s (%s, %s)\n", prop->name, prop->type, prop->description);
> +        } else {
> +            printf("%s (%s)\n", prop->name, prop->type);
> +        }

error_printf()

> +    }

There's a very similar loop in machine_help_func().  Can we avoid the
duplication?

> +
> +out:

You don't actually need this label.

> +    object_unref(obj);
> +    return 1;
> +}
> +
>  static void register_types(void)
>  {
>      static const TypeInfo uc_interface_info = {
> diff --git a/vl.c b/vl.c
> index ebd47af..1456eca 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4100,6 +4100,11 @@ int main(int argc, char **argv, char **envp)
>          exit(0);
>      }
>  
> +    if (qemu_opts_foreach(qemu_find_opts("object"), user_creatable_help_func,
> +                          NULL, NULL)) {
> +        exit(1);
> +    }
> +
>      if (!trace_init_backends()) {
>          exit(1);
>      }

Can you explain why you picked this spot in main()?

[Qemu-devel] 答复： Re: [PATCH v4 5/5] object: Add 'help' option for all available backends and properties

[Lin Ma]

>>> Markus Armbruster <armbru@redhat.com> 2016/11/4 星期五 上午 3:50 >>>
>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   	  | 55 +++++++++++++++++++++++++++++++++++++++++
>>  vl.c   						 |  5 ++++
>>  4 files changed, 68 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);
>> +
>
>Ugly interface: two arguments that aren't used, one of them void *.
>Suggest to follow the device help example: do a non-ugly interface here,
>and an ugly, but static wrapper in vl.c
ok, will do.

>>  #endif
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index b1fbdb0..b9573ce 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -3761,7 +3761,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",
>
>The existing text talks about "object of type TYPENAME".  The new text
>suddenly talks about "backend types".  Switching terminology is not a
>good idea.  Call them "object types".
>
>For comparison, here's the text we use for -device:
>
>			    use '-device help' to print all possible drivers
>			    use '-device driver,help' to print all possible properties
>
>Let's make this one similar:
>
>			    use '-object help' to print all possible object types
>			    use '-object driver,help' to print all possible properties
ok, will do.
 
>>	  QEMU_ARCH_ALL)
>>  STEXI
>>  @item -object @var{typename}[,@var{prop1}=@var{value1},...]
>> @@ -3771,6 +3773,9 @@ in the order they are specified.  Note that the 'id'
>>  property must be set.  These objects are placed in the
>>  '/objects' path.
>>  
>> +Use @code{-object help} to print available backend types and
>> +@code{-object @var{typename},help} to print relevant properties.
>> +
>
>-device's text:
>
>    To get help on possible drivers and properties, use @code{-device
>    help} and @code{-device @var{driver},help}.
>
>Suggest:
>
>    To get help on possible object types and properties, use @code{-object
>    help} and @code{-object @var{type},help}.
ok, will do.

>>  @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..da8be39 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,60 @@ void user_creatable_del(const char *id, Error **errp)
>>	  object_unparent(obj);
>>  }
>>  
>> +int user_creatable_help_func(void *opaque, QemuOpts *opts, Error **errp)
>> +{
>> +    const char *type = NULL;
>> +    Object *obj = NULL;
>> +    ObjectClass *klass;
>> +    ObjectProperty *prop;
>> +    ObjectPropertyIterator iter;
>> +
>> +    type = qemu_opt_get(opts, "qom-typ
e");
>> +    if (type && is_help_option(type)) {
>> +	    GSList *list;
>> +	    printf("Available object backend types:\n");
>
>Wrong when running for an HMP monitor command, e.g. "object_add help".
>Use error_printf().  That's only slightly wrong in that it prints to
>stderr instead of stdout running for the command line.
using error_printf() instead of it, no problem.
But the current code doesn't support 'object_add help', Should I modify code to
support it in this patch?
 
>> +	    for (list = object_class_get_list(TYPE_USER_CREATABLE, false);  \
>> +			    list; 												  \
>> +			    list = list->next) {
>
>Superfluous line continuations.  Simply write
>
>		   for (list = object_class_get_list(TYPE_USER_CREATABLE, false);
>			    list;
>			    list = list->next) {
>
>Alternatively,
>
>		   list = object_class_get_list(TYPE_USER_CREATABLE, false);
>		   for (elt = list; elt; elt = elt->next) {
Perhaps this one is better.

>> +		    const char *name;
>> +		    name = object_class_get_name(OBJECT_CLASS(list->data));
>> +		    if (strcmp(name, TYPE_USER_CREATABLE)) {
>> +			    printf("%s\n", name);
>
>error_printf()
>
>> +		    }
>> +	    }
>> +	    g_slist_free(list);
>
>Leaks the list, because by now @list is null.  Suggest to fix by
>adopting the code under "alternatively".
>
>> +	    goto out;
>
>Suggest return 1, because ...
>
>> +    }
>> +
>> +    if (!type || !qemu_opt_has_help_opt(opts)) {
>> +	    return 0;
>
>... you're not going to out here.
>
>No need to initialize @obj then.
>
>> +    }
>> +
>> +    klass = object_class_by_name(type);
>> +    if (!klass) {
>> +	    printf("invalid object type: %s\n", type);
>
>Please use error_report().
>
>> +	    goto out;
>
>return 1 would do.
>
>> +    }
>> +    if (object_class_is_abstract(klass)) {
>> +	    printf("object type '%s' is abstract\n", type);
>
>Please use error_report().
>
>> +	    goto out;
>
>return 1 would do.
>
>> +    }
>
>I'd put a blank line here ...
>
>> +    obj = object_new(type);
>> +    object_property_iter_init(&iter, obj);
>> +
>
>... instead of here.
>
>> +    while ((prop = object_property_iter_next(&iter))) {
>> +	    if (prop->description) {
>> +		    printf("%s (%s, %s)\n", prop->name, prop->type, prop->description);
>> +	    } else {
>> +		    printf("%s (%s)\n", prop->name, prop->type);
>> +	    }
>
>error_printf()
ok, will fix them.

>> +    }
>
>There's a very similar loop in machine_help_func().  Can we avoid the
>duplication?
I would say negative. using same code snippet in machine_help_func() and
in here, we lost flexibility, says if we want to change the output content/format
in the future.

>> +
>> +out:
>
>You don't actually need this label.
ok, will fix it.

>> +    object_unref(obj);
>> +    return 1;
>> +}
>> +
>>  static void register_types(void)
>>  {
>>	  static const TypeInfo uc_interface_info = {
>> diff --git a/vl.c b/vl.c
>> index ebd47af..1456eca 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -4100,6 +4100,11 @@ int main(int argc, char **argv, char **envp)
>>		  exit(0);
>>	  }
>>  
>> +    if (qemu_opts_foreach(qemu_find_opts("object"), user_creatable_help_func,
>> +						  NULL, NULL)) {
>> +	    exit(1);
>> +    }
>> +
>>	  if (!trace_init_backends()) {
>>		  exit(1);
>>	  }
>
>Can you explain why you picked this spot in main()?
Nothing special, just want to let it run as early as possible. and it can't be run too early
because some basic structures/objects of qemu need to be constructed first.
May I have your idea?
 
 
Thanks,
Lin

[Qemu-devel] 答复： [PATCH v4 0/5] object: Add 'help' option for all available backends and properties

[Lin Ma]

ping...

>>> Lin Ma <lma@suse.com> 2016/10/20 星期四 下午 7:28 >>>
V3->V4:
* drop the code about manually define interface 'user-creatable' as abstract.
* add test case for the generated enum value str.
* split the code about adding descriptions to class properties into a separate patch.
* minor changes in user_creatable_help_func to ignore interface TYPE_USER_CREATABLE.

V2->v3:
* make type user-creatable abstract.
* auto generate enum value strings during qemu configuration.(Borrowwed Daniel's code)
* save the generated enum value strings into member description of ObjectProperty.
* drop the judgement logic of whether a property has an enumeration type anymore,
  output member description of ObjectProperty directly.
* at least, user_creatable_help_func should be put after
  'object_property_add_child(object_get_root(), "machine",OBJECT(current_machine), ...)',
  because host_memory_backend_init needs to access an instance of type machine.

V1->V2:
* Output the acceptable values of enum types by "-object TYPE-NAME,help"

Lin Ma (5):
  qom: Add interface check in object_class_is_abstract
  qapi: auto generate enum value strings
  qapi: add test case for the generated enum value str
  backends: add description for enum class properties
  object: Add 'help' option for all available backends and properties

backends/hostmem.c			  |  4 +++
crypto/secret.c				 |  4 +++
crypto/tlscreds.c			   |  4 +++
include/qom/object_interfaces.h |  2 ++
net/filter.c				    |  4 +++
qemu-options.hx				 |  7 +++++-
qom/object.c				    |  6 ++++-
qom/object_interfaces.c		 | 55 +++++++++++++++++++++++++++++++++++++++++
scripts/qapi-types.py		   |  2 ++
scripts/qapi.py				 |  9 +++++++
tests/test-qmp-commands.c	   | 18 ++++++++++++++
vl.c						    |  5 ++++
12 files changed, 118 insertions(+), 2 deletions(-)

-- 
2.9.2