[3/4] vl.c: do not set 'type' property in obj_set_property

Filter out also 'type' property when setting
object's properties

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
 vl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Am 07.05.2014 16:42, schrieb Marcel Apfelbaum:
> Filter out also 'type' property when setting
> object's properties
> 
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> ---
>  vl.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/vl.c b/vl.c
> index 58673bd..6ec6c1a 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2889,7 +2889,8 @@ static int object_set_property(const char *name, const char *value, void *opaque
>      StringInputVisitor *siv;
>      Error *local_err = NULL;
>  
> -    if (strcmp(name, "qom-type") == 0 || strcmp(name, "id") == 0) {
> +    if (strcmp(name, "qom-type") == 0 || strcmp(name, "id") == 0 ||
> +        strcmp(name, "type") == 0) {
>          return 0;
>      }
>  

Seems sensible, although qom-type and type looks kind of odd at first.

Given the Rb on this v1 patch, is this applied in any tree already?

Regards,
Andreas

Marcel Apfelbaum <marcel.a@redhat.com> writes:

> Filter out also 'type' property when setting
> object's properties
>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> ---
>  vl.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/vl.c b/vl.c
> index 58673bd..6ec6c1a 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2889,7 +2889,8 @@ static int object_set_property(const char *name, const char *value, void *opaque
>      StringInputVisitor *siv;
>      Error *local_err = NULL;
>  
> -    if (strcmp(name, "qom-type") == 0 || strcmp(name, "id") == 0) {
> +    if (strcmp(name, "qom-type") == 0 || strcmp(name, "id") == 0 ||
> +        strcmp(name, "type") == 0) {
>          return 0;
>      }

I can see why "id" and "qom-type" need to be filtered out: they're
consumed by object_create() before object_set_property() gets called.

I can't see why "type".  Explain?

Am 15.05.2014 18:15, schrieb Markus Armbruster:
> Marcel Apfelbaum <marcel.a@redhat.com> writes:
> 
>> Filter out also 'type' property when setting
>> object's properties
>>
>> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
>> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
>> ---
>>  vl.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/vl.c b/vl.c
>> index 58673bd..6ec6c1a 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -2889,7 +2889,8 @@ static int object_set_property(const char *name, const char *value, void *opaque
>>      StringInputVisitor *siv;
>>      Error *local_err = NULL;
>>  
>> -    if (strcmp(name, "qom-type") == 0 || strcmp(name, "id") == 0) {
>> +    if (strcmp(name, "qom-type") == 0 || strcmp(name, "id") == 0 ||
>> +        strcmp(name, "type") == 0) {
>>          return 0;
>>      }
> 
> I can see why "id" and "qom-type" need to be filtered out: they're
> consumed by object_create() before object_set_property() gets called.
> 
> I can't see why "type".  Explain?

My understanding is,

-machine [type=]name[,...]

coincides with every Object exposing a read-only property "type".

Not only would setting it fail, but the values differ (display name vs.
unique type name).

On that matter, our help output does not seem to indicate the name of
the corresponding -object parameter, my unverified guess is that that is
called "qom-type".

OTOH there's no reason to suppress "qom-type" or "id" for -machine, as
implied when I suggested using a separate function.

Regards,
Andreas

Il 15/05/2014 18:38, Andreas Färber ha scritto:
> On that matter, our help output does not seem to indicate the name of
> the corresponding -object parameter, my unverified guess is that that is
> called "qom-type".
>
> OTOH there's no reason to suppress "qom-type" or "id" for -machine, as
> implied when I suggested using a separate function.

"id" should be suppressed for backwards compatibility for "-machine id=foo".

"qom-type" is not needed indeed.  But it is so ugly that I would suggest 
doing a backwards-incompatible change to the name the default argument 
of -object, from "qom-type" to "type".  Libvirt doesn't use "qom-type", 
and "type" nicely matches the QOM property name:

   $ qemu-system-x86_64 -qmp unix:/tmp/m1,server,nowait -S \
        -object rng-random,id=foo &
   $ ./qom-get -s /tmp/m1 /objects/foo.type
   rng-random

Paolo