diff mbox series

[qemu,v3,1/2] qmp: Merge ObjectPropertyInfo and DevicePropertyInfo

Message ID 20180301130939.15875-2-aik@ozlabs.ru
State New
Headers show
Series qmp: Add qom-list-properties to list QOM object properties | expand

Commit Message

Alexey Kardashevskiy March 1, 2018, 1:09 p.m. UTC
ObjectPropertyInfo is more generic and only missing @description.
This adds a description to ObjectPropertyInfo and removes
DevicePropertyInfo so the resulting ObjectPropertyInfo can be used
elsewhere.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 qapi-schema.json | 23 +++++------------------
 qdev-monitor.c   |  6 +++---
 qmp.c            | 20 ++++++++++----------
 3 files changed, 18 insertions(+), 31 deletions(-)

Comments

David Gibson March 2, 2018, 1 a.m. UTC | #1
On Fri, Mar 02, 2018 at 12:09:38AM +1100, Alexey Kardashevskiy wrote:
> ObjectPropertyInfo is more generic and only missing @description.
> This adds a description to ObjectPropertyInfo and removes
> DevicePropertyInfo so the resulting ObjectPropertyInfo can be used
> elsewhere.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  qapi-schema.json | 23 +++++------------------
>  qdev-monitor.c   |  6 +++---
>  qmp.c            | 20 ++++++++++----------
>  3 files changed, 18 insertions(+), 31 deletions(-)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 0262b9f..87327e5 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1266,10 +1266,12 @@
>  #        3) A link type in the form 'link<subtype>' where subtype is a qdev
>  #           device type name.  Link properties form the device model graph.
>  #
> +# @description: if specified, the description of the property.
> +#
>  # Since: 1.2
>  ##
>  { 'struct': 'ObjectPropertyInfo',
> -  'data': { 'name': 'str', 'type': 'str' } }
> +  'data': { 'name': 'str', 'type': 'str', '*description': 'str' } }
>  
>  ##
>  # @qom-list:
> @@ -1425,34 +1427,19 @@
>    'returns': [ 'ObjectTypeInfo' ] }
>  
>  ##
> -# @DevicePropertyInfo:
> -#
> -# Information about device properties.
> -#
> -# @name: the name of the property
> -# @type: the typename of the property
> -# @description: if specified, the description of the property.
> -#               (since 2.2)
> -#
> -# Since: 1.2
> -##
> -{ 'struct': 'DevicePropertyInfo',
> -  'data': { 'name': 'str', 'type': 'str', '*description': 'str' } }
> -
> -##
>  # @device-list-properties:
>  #
>  # List properties associated with a device.
>  #
>  # @typename: the type name of a device
>  #
> -# Returns: a list of DevicePropertyInfo describing a devices properties
> +# Returns: a list of ObjectPropertyInfo describing a devices properties
>  #
>  # Since: 1.2
>  ##
>  { 'command': 'device-list-properties',
>    'data': { 'typename': 'str'},
> -  'returns': [ 'DevicePropertyInfo' ] }
> +  'returns': [ 'ObjectPropertyInfo' ] }
>  
>  ##
>  # @xen-set-global-dirty-log:
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index 8462381..ab9c46c 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -258,8 +258,8 @@ int qdev_device_help(QemuOpts *opts)
>  {
>      Error *local_err = NULL;
>      const char *driver;
> -    DevicePropertyInfoList *prop_list;
> -    DevicePropertyInfoList *prop;
> +    ObjectPropertyInfoList *prop_list;
> +    ObjectPropertyInfoList *prop;
>  
>      driver = qemu_opt_get(opts, "driver");
>      if (driver && is_help_option(driver)) {
> @@ -295,7 +295,7 @@ int qdev_device_help(QemuOpts *opts)
>          }
>      }
>  
> -    qapi_free_DevicePropertyInfoList(prop_list);
> +    qapi_free_ObjectPropertyInfoList(prop_list);
>      return 1;
>  
>  error:
> diff --git a/qmp.c b/qmp.c
> index 793f6f3..8a74038 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -463,12 +463,12 @@ ObjectTypeInfoList *qmp_qom_list_types(bool has_implements,
>   *
>   * The caller must free the return value.
>   */
> -static DevicePropertyInfo *make_device_property_info(ObjectClass *klass,
> -                                                     const char *name,
> -                                                     const char *default_type,
> -                                                     const char *description)
> +static ObjectPropertyInfo *make_device_property_info(ObjectClass *klass,
> +                                                  const char *name,
> +                                                  const char *default_type,
> +                                                  const char *description)
>  {
> -    DevicePropertyInfo *info;
> +    ObjectPropertyInfo *info;
>      Property *prop;
>  
>      do {
> @@ -508,14 +508,14 @@ static DevicePropertyInfo *make_device_property_info(ObjectClass *klass,
>      return info;
>  }
>  
> -DevicePropertyInfoList *qmp_device_list_properties(const char *typename,
> -                                                   Error **errp)
> +ObjectPropertyInfoList *qmp_device_list_properties(const char *typename,
> +                                                Error **errp)
>  {
>      ObjectClass *klass;
>      Object *obj;
>      ObjectProperty *prop;
>      ObjectPropertyIterator iter;
> -    DevicePropertyInfoList *prop_list = NULL;
> +    ObjectPropertyInfoList *prop_list = NULL;
>  
>      klass = object_class_by_name(typename);
>      if (klass == NULL) {
> @@ -540,8 +540,8 @@ DevicePropertyInfoList *qmp_device_list_properties(const char *typename,
>  
>      object_property_iter_init(&iter, obj);
>      while ((prop = object_property_iter_next(&iter))) {
> -        DevicePropertyInfo *info;
> -        DevicePropertyInfoList *entry;
> +        ObjectPropertyInfo *info;
> +        ObjectPropertyInfoList *entry;
>  
>          /* Skip Object and DeviceState properties */
>          if (strcmp(prop->name, "type") == 0 ||
Eric Blake March 2, 2018, 1:37 p.m. UTC | #2
On 03/01/2018 07:09 AM, Alexey Kardashevskiy wrote:
> ObjectPropertyInfo is more generic and only missing @description.
> This adds a description to ObjectPropertyInfo and removes
> DevicePropertyInfo so the resulting ObjectPropertyInfo can be used
> elsewhere.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>   qapi-schema.json | 23 +++++------------------
>   qdev-monitor.c   |  6 +++---
>   qmp.c            | 20 ++++++++++----------
>   3 files changed, 18 insertions(+), 31 deletions(-)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 0262b9f..87327e5 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1266,10 +1266,12 @@
>   #        3) A link type in the form 'link<subtype>' where subtype is a qdev
>   #           device type name.  Link properties form the device model graph.
>   #
> +# @description: if specified, the description of the property.

Missing a '(since 2.12)' tag.

> +#
>   # Since: 1.2
>   ##
>   { 'struct': 'ObjectPropertyInfo',
> -  'data': { 'name': 'str', 'type': 'str' } }
> +  'data': { 'name': 'str', 'type': 'str', '*description': 'str' } }
>   
> +++ b/qmp.c
> @@ -463,12 +463,12 @@ ObjectTypeInfoList *qmp_qom_list_types(bool has_implements,
>    *
>    * The caller must free the return value.
>    */
> -static DevicePropertyInfo *make_device_property_info(ObjectClass *klass,
> -                                                     const char *name,
> -                                                     const char *default_type,
> -                                                     const char *description)
> +static ObjectPropertyInfo *make_device_property_info(ObjectClass *klass,
> +                                                  const char *name,
> +                                                  const char *default_type,
> +                                                  const char *description)

Why the indentation change?

> @@ -508,14 +508,14 @@ static DevicePropertyInfo *make_device_property_info(ObjectClass *klass,
>       return info;
>   }
>   
> -DevicePropertyInfoList *qmp_device_list_properties(const char *typename,
> -                                                   Error **errp)
> +ObjectPropertyInfoList *qmp_device_list_properties(const char *typename,
> +                                                Error **errp)

and again

Otherwise looks okay
Paolo Bonzini March 2, 2018, 1:42 p.m. UTC | #3
On 02/03/2018 14:37, Eric Blake wrote:
>>
>> index 0262b9f..87327e5 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -1266,10 +1266,12 @@
>>   #        3) A link type in the form 'link<subtype>' where subtype is
>> a qdev
>>   #           device type name.  Link properties form the device model
>> graph.
>>   #
>> +# @description: if specified, the description of the property.
> 
> Missing a '(since 2.12)' tag.

Some of the users had it (in other types that are now unified) before
2.12.  I'm not sure whether it is more accurate to have the annotation
or not, especially considering that it is optional.  Protocol-wise it is
never an issue to add optional fields, since you cannot distinguish an
implementation that lacks the field from one that never fills it.

Paolo
Eric Blake March 2, 2018, 2:10 p.m. UTC | #4
On 03/02/2018 07:42 AM, Paolo Bonzini wrote:
> On 02/03/2018 14:37, Eric Blake wrote:
>>>
>>> index 0262b9f..87327e5 100644
>>> --- a/qapi-schema.json
>>> +++ b/qapi-schema.json
>>> @@ -1266,10 +1266,12 @@
>>>    #        3) A link type in the form 'link<subtype>' where subtype is
>>> a qdev
>>>    #           device type name.  Link properties form the device model
>>> graph.
>>>    #
>>> +# @description: if specified, the description of the property.
>>
>> Missing a '(since 2.12)' tag.
> 
> Some of the users had it (in other types that are now unified) before
> 2.12.  I'm not sure whether it is more accurate to have the annotation
> or not, especially considering that it is optional.  Protocol-wise it is
> never an issue to add optional fields, since you cannot distinguish an
> implementation that lacks the field from one that never fills it.

True; you could write it as '(since 2.2)', as that is the earliest 
version that provided the optional field between the structs that you 
are merging.
Alexey Kardashevskiy March 3, 2018, 12:16 a.m. UTC | #5
On 03/03/18 00:37, Eric Blake wrote:
> On 03/01/2018 07:09 AM, Alexey Kardashevskiy wrote:
>> ObjectPropertyInfo is more generic and only missing @description.
>> This adds a description to ObjectPropertyInfo and removes
>> DevicePropertyInfo so the resulting ObjectPropertyInfo can be used
>> elsewhere.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>>   qapi-schema.json | 23 +++++------------------
>>   qdev-monitor.c   |  6 +++---
>>   qmp.c            | 20 ++++++++++----------
>>   3 files changed, 18 insertions(+), 31 deletions(-)
>>
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index 0262b9f..87327e5 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -1266,10 +1266,12 @@
>>   #        3) A link type in the form 'link<subtype>' where subtype is a
>> qdev
>>   #           device type name.  Link properties form the device model
>> graph.
>>   #
>> +# @description: if specified, the description of the property.
> 
> Missing a '(since 2.12)' tag.
> 
>> +#
>>   # Since: 1.2
>>   ##
>>   { 'struct': 'ObjectPropertyInfo',
>> -  'data': { 'name': 'str', 'type': 'str' } }
>> +  'data': { 'name': 'str', 'type': 'str', '*description': 'str' } }
>>   +++ b/qmp.c
>> @@ -463,12 +463,12 @@ ObjectTypeInfoList *qmp_qom_list_types(bool
>> has_implements,
>>    *
>>    * The caller must free the return value.
>>    */
>> -static DevicePropertyInfo *make_device_property_info(ObjectClass *klass,
>> -                                                     const char *name,
>> -                                                     const char
>> *default_type,
>> -                                                     const char
>> *description)
>> +static ObjObjectPropertyInfoectPropertyInfo *make_device_property_info(ObjectClass *klass,
>> +                                                  const char *name,
>> +                                                  const char *default_type,
>> +                                                  const char *description)
> 
> Why the indentation change?


Oh. Leftover from DevicePropertyInfo->(non-existng) OOMPropertyInfo. I'll
repost.


> 
>> @@ -508,14 +508,14 @@ static DevicePropertyInfo
>> *make_device_property_info(ObjectClass *klass,
>>       return info;
>>   }
>>   -DevicePropertyInfoList *qmp_device_list_properties(const char *typename,
>> -                                                   Error **errp)
>> +ObjectPropertyInfoList *qmp_device_list_properties(const char *typename,
>> +                                                Error **errp)
> 
> and again
> 
> Otherwise looks okay
>
diff mbox series

Patch

diff --git a/qapi-schema.json b/qapi-schema.json
index 0262b9f..87327e5 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1266,10 +1266,12 @@ 
 #        3) A link type in the form 'link<subtype>' where subtype is a qdev
 #           device type name.  Link properties form the device model graph.
 #
+# @description: if specified, the description of the property.
+#
 # Since: 1.2
 ##
 { 'struct': 'ObjectPropertyInfo',
-  'data': { 'name': 'str', 'type': 'str' } }
+  'data': { 'name': 'str', 'type': 'str', '*description': 'str' } }
 
 ##
 # @qom-list:
@@ -1425,34 +1427,19 @@ 
   'returns': [ 'ObjectTypeInfo' ] }
 
 ##
-# @DevicePropertyInfo:
-#
-# Information about device properties.
-#
-# @name: the name of the property
-# @type: the typename of the property
-# @description: if specified, the description of the property.
-#               (since 2.2)
-#
-# Since: 1.2
-##
-{ 'struct': 'DevicePropertyInfo',
-  'data': { 'name': 'str', 'type': 'str', '*description': 'str' } }
-
-##
 # @device-list-properties:
 #
 # List properties associated with a device.
 #
 # @typename: the type name of a device
 #
-# Returns: a list of DevicePropertyInfo describing a devices properties
+# Returns: a list of ObjectPropertyInfo describing a devices properties
 #
 # Since: 1.2
 ##
 { 'command': 'device-list-properties',
   'data': { 'typename': 'str'},
-  'returns': [ 'DevicePropertyInfo' ] }
+  'returns': [ 'ObjectPropertyInfo' ] }
 
 ##
 # @xen-set-global-dirty-log:
diff --git a/qdev-monitor.c b/qdev-monitor.c
index 8462381..ab9c46c 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -258,8 +258,8 @@  int qdev_device_help(QemuOpts *opts)
 {
     Error *local_err = NULL;
     const char *driver;
-    DevicePropertyInfoList *prop_list;
-    DevicePropertyInfoList *prop;
+    ObjectPropertyInfoList *prop_list;
+    ObjectPropertyInfoList *prop;
 
     driver = qemu_opt_get(opts, "driver");
     if (driver && is_help_option(driver)) {
@@ -295,7 +295,7 @@  int qdev_device_help(QemuOpts *opts)
         }
     }
 
-    qapi_free_DevicePropertyInfoList(prop_list);
+    qapi_free_ObjectPropertyInfoList(prop_list);
     return 1;
 
 error:
diff --git a/qmp.c b/qmp.c
index 793f6f3..8a74038 100644
--- a/qmp.c
+++ b/qmp.c
@@ -463,12 +463,12 @@  ObjectTypeInfoList *qmp_qom_list_types(bool has_implements,
  *
  * The caller must free the return value.
  */
-static DevicePropertyInfo *make_device_property_info(ObjectClass *klass,
-                                                     const char *name,
-                                                     const char *default_type,
-                                                     const char *description)
+static ObjectPropertyInfo *make_device_property_info(ObjectClass *klass,
+                                                  const char *name,
+                                                  const char *default_type,
+                                                  const char *description)
 {
-    DevicePropertyInfo *info;
+    ObjectPropertyInfo *info;
     Property *prop;
 
     do {
@@ -508,14 +508,14 @@  static DevicePropertyInfo *make_device_property_info(ObjectClass *klass,
     return info;
 }
 
-DevicePropertyInfoList *qmp_device_list_properties(const char *typename,
-                                                   Error **errp)
+ObjectPropertyInfoList *qmp_device_list_properties(const char *typename,
+                                                Error **errp)
 {
     ObjectClass *klass;
     Object *obj;
     ObjectProperty *prop;
     ObjectPropertyIterator iter;
-    DevicePropertyInfoList *prop_list = NULL;
+    ObjectPropertyInfoList *prop_list = NULL;
 
     klass = object_class_by_name(typename);
     if (klass == NULL) {
@@ -540,8 +540,8 @@  DevicePropertyInfoList *qmp_device_list_properties(const char *typename,
 
     object_property_iter_init(&iter, obj);
     while ((prop = object_property_iter_next(&iter))) {
-        DevicePropertyInfo *info;
-        DevicePropertyInfoList *entry;
+        ObjectPropertyInfo *info;
+        ObjectPropertyInfoList *entry;
 
         /* Skip Object and DeviceState properties */
         if (strcmp(prop->name, "type") == 0 ||