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

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

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

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.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
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) {
-- 
1.7.12.4

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

[Paolo Bonzini]

Il 28/08/2014 07:19, arei.gonglei@huawei.com ha scritto:
> From: Gonglei <arei.gonglei@huawei.com>
> 
> 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 <arei.gonglei@huawei.com>
> ---
> 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) {
> 

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

[Gonglei (Arei)]

> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> Subject: Re: [PATCH] qmp: using genneric string paring function for 'qom-set'
> 
> Il 28/08/2014 07:19, arei.gonglei@huawei.com ha scritto:
> > From: Gonglei <arei.gonglei@huawei.com>
> >
> > 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!
> 
OK. It's my fault. Thanks!

> 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?
> 
Sorry, I don't have an idea for your questions.
Maybe Luiz can give us an answer? Thanks.

Best regards,
-Gonglei

> Paolo
> 
> > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > ---
> > 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) {
> >

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 800 bytes --]

On 08/27/2014 11:19 PM, arei.gonglei@huawei.com wrote:
> From: Gonglei <arei.gonglei@huawei.com>
> 

In the subject: s/genneric/generic/, s/paring/parsing/

> At present, 'qom-set' only can set string type property,

qom-set is typed as taking arbitrary JSON 9one of the few commands with
'gen':'no', which means it is completely type-unsafe, and can therefore
accept EVERY type, including JSON integers, not just strings).

> which will restrict this qmp command's function.
> Using genneric string paring function can handle different

Same typos here as in subject.

> types, such as int/bool/string etc.
> 
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

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

[Gonglei (Arei)]

> From: Eric Blake [mailto:eblake@redhat.com]
> Subject: Re: [Qemu-devel] [PATCH] qmp: using genneric string paring function
> for 'qom-set'
> 
> On 08/27/2014 11:19 PM, arei.gonglei@huawei.com wrote:
> > From: Gonglei <arei.gonglei@huawei.com>
> >
> 
> In the subject: s/genneric/generic/, s/paring/parsing/
> 
OK.

> > At present, 'qom-set' only can set string type property,
> 
> qom-set is typed as taking arbitrary JSON 9one of the few commands with
> 'gen':'no', which means it is completely type-unsafe, and can therefore
> accept EVERY type, including JSON integers, not just strings).
> 
Yep. Thanks, Eric. This is my fault.

> > which will restrict this qmp command's function.
> > Using genneric string paring function can handle different
> 
> Same typos here as in subject.
> 
Yep. Thanks for your review.

Best regards,
-Gonglei

> > types, such as int/bool/string etc.
> >
> > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > ---
> 
> --
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org