Re: [Qemu-devel] [RFC PATCH qemu] qdev: Use "string" for QOM string properties

[Daniel P . Berrangé]

On Tue, Jun 05, 2018 at 07:15:08PM +1000, Alexey Kardashevskiy wrote:
> For QOM properties QEMU uses "string", for example:
> 
> {"execute": "qom-list", "arguments": {"path": "/machine"}}
> [{'return': [{'type': 'string', 'name': 'append'},  {'type': 'string', 
> 'name': 'kernel'}]
> 
> However for the device properties copied from DEVICE_CLASS(class)->props,
> "str" is used for a type (vnet0 is "-device virtio-net-pci,id=vnet0"):
> 
> {"execute": "qom-list", "arguments": {"path": "/machine/peripheral/vnet0"}}
> ret=[{'return': [{'name': 'tx', 'type': 'str'}]
> 
> This changes string type when adding them to QOM property list so
> we get one string type in QMP.
> 
> Signed-off-by: Alexey Kardashevskiy <address@hidden>
> ---
> 
> Not a huge improvement and might actually break something but since
> I got that far I decided to post this anyway :)
> 
> The alternatives are: 1) do nothing 2) do this:
> 
>  const PropertyInfo qdev_prop_string = {
> -    .name  = "str",
> +    .name  = "string",
>      .release = release_string,
>      .get   = get_string,
>      .set   = set_string,
>  };

As further points of reference:

 - object_property_add_str & object_class_property_add_str
   both use "string".
   
 - QAPI schema uses "str"

So the inconsistency is even bigger than just qdev. I'm inclined
to say "QAPI schema" is the canonical source of truth, and so
every use of "string" should change to "str".

CC'd Markus for 2nd opinion.

> 
> 
> ---
>  hw/core/qdev.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index f6f9247..2895693 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -687,7 +687,7 @@ static void qdev_property_add_legacy(DeviceState *dev, 
> Property *prop,
>      }
>  
>      name = g_strdup_printf("legacy-%s", prop->name);
> -    object_property_add(OBJECT(dev), name, "str",
> +    object_property_add(OBJECT(dev), name, "string",
>                          prop->info->print ? qdev_get_legacy_property : 
> prop->info->get,
>                          NULL,
>                          NULL,
> @@ -711,6 +711,7 @@ void qdev_property_add_static(DeviceState *dev, Property 
> *prop,
>  {
>      Error *local_err = NULL;
>      Object *obj = OBJECT(dev);
> +    const char *proptype;
>  
>      if (prop->info->create) {
>          prop->info->create(obj, prop, &local_err);
> @@ -723,7 +724,11 @@ void qdev_property_add_static(DeviceState *dev, Property 
> *prop,
>          if (!prop->info->get && !prop->info->set) {
>              return;
>          }
> -        object_property_add(obj, prop->name, prop->info->name,
> +        proptype = prop->info->name;
> +        if (0 == strncmp(proptype, "str", 3)) {
> +            proptype = "string";
> +        }
> +        object_property_add(obj, prop->name, proptype,
>                              prop->info->get, prop->info->set,
>                              prop->info->release,
>                              prop, &local_err);
> -- 
> 2.11.0
> 
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|