Re: [Qemu-devel] [PATCH] qmp: using genneric string paring function for 'qom-set'

[Paolo Bonzini]

Il 28/08/2014 07:19, address@hidden ha scritto:
> From: Gonglei <address@hidden>
> 
> At present, 'qom-set' only can set string type property,
> which will restrict this qmp command's function.
> Using genneric string paring function can handle different
> types, such as int/bool/string etc.

Actually, that's not true.  You can pass JSON integers or booleans to
qom-set; they will work and, most important, will be type safe!

It's a bug in qom-set that it doesn't let you pass integers or booleans.
 Perhaps qom-set could have a -j option where the value argument is not
treated as a string, but rather as a JSON value?  Or it could special
case numbers and true/false itself?

Paolo

> Signed-off-by: Gonglei <address@hidden>
> ---
> Example:
> # ./scripts/qmp/qom-set nic1.realized false
> Traceback (most recent call last):
>   File "./scripts/qmp/qom-set", line 64, in <module>
>     print srv.command('qom-set', path=path, property=prop, value=sys.argv[2])
>   File "/mnt/sdb/gonglei/qemu.git/qemu/scripts/qmp/qmp.py", line 136, in 
> command
>     raise Exception(ret['error']['desc'])
> Exception: Invalid parameter type for 'realized', expected: boolean
> ---
>  qmp.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/qmp.c b/qmp.c
> index c6767c4..0a67c01 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -219,7 +219,7 @@ int qmp_qom_set(Monitor *mon, const QDict *qdict, QObject 
> **ret)
>  {
>      const char *path = qdict_get_str(qdict, "path");
>      const char *property = qdict_get_str(qdict, "property");
> -    QObject *value = qdict_get(qdict, "value");
> +    const char *value = qdict_get_str(qdict, "value");
>      Error *local_err = NULL;
>      Object *obj;
>  
> @@ -229,7 +229,7 @@ int qmp_qom_set(Monitor *mon, const QDict *qdict, QObject 
> **ret)
>          goto out;
>      }
>  
> -    object_property_set_qobject(obj, value, property, &local_err);
> +    object_property_parse(obj, value, property, &local_err);
>  
>  out:
>      if (local_err) {
>