[Qemu-devel] [PATCH] machine: replace underscores in machine's property names

[Qemu-devel] [PATCH] machine: replace underscores in machine's property names

[Marcel Apfelbaum]

Replaced '_' with '-' to comply with QOM guidelines.
Made the conversion from HMP to QMP in vl.c

Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
 hw/core/machine.c |  8 ++++----
 vl.c              | 12 +++++++++++-
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index cbba679..7a66c57 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -239,11 +239,11 @@ static void machine_initfn(Object *obj)
 {
     object_property_add_str(obj, "accel",
                             machine_get_accel, machine_set_accel, NULL);
-    object_property_add_bool(obj, "kernel_irqchip",
+    object_property_add_bool(obj, "kernel-irqchip",
                              machine_get_kernel_irqchip,
                              machine_set_kernel_irqchip,
                              NULL);
-    object_property_add(obj, "kvm_shadow_mem", "int",
+    object_property_add(obj, "kvm-shadow-mem", "int",
                         machine_get_kvm_shadow_mem,
                         machine_set_kvm_shadow_mem,
                         NULL, NULL, NULL);
@@ -257,11 +257,11 @@ static void machine_initfn(Object *obj)
                             machine_get_dtb, machine_set_dtb, NULL);
     object_property_add_str(obj, "dumpdtb",
                             machine_get_dumpdtb, machine_set_dumpdtb, NULL);
-    object_property_add(obj, "phandle_start", "int",
+    object_property_add(obj, "phandle-start", "int",
                         machine_get_phandle_start,
                         machine_set_phandle_start,
                         NULL, NULL, NULL);
-    object_property_add_str(obj, "dt_compatible",
+    object_property_add_str(obj, "dt-compatible",
                             machine_get_dt_compatible,
                             machine_set_dt_compatible,
                             NULL);
diff --git a/vl.c b/vl.c
index a1686ef..7587c97 100644
--- a/vl.c
+++ b/vl.c
@@ -2820,15 +2820,25 @@ static int object_set_property(const char *name, const char *value, void *opaque
     Object *obj = OBJECT(opaque);
     StringInputVisitor *siv;
     Error *local_err = NULL;
+    char *c, *qom_name;
 
     if (strcmp(name, "qom-type") == 0 || strcmp(name, "id") == 0 ||
         strcmp(name, "type") == 0) {
         return 0;
     }
 
+    qom_name = g_strdup(name);
+    c = qom_name;
+    while (*c++) {
+        if (*c == '_') {
+            *c = '-';
+        }
+    }
+
     siv = string_input_visitor_new(value);
-    object_property_set(obj, string_input_get_visitor(siv), name, &local_err);
+    object_property_set(obj, string_input_get_visitor(siv), qom_name, &local_err);
     string_input_visitor_cleanup(siv);
+    free(qom_name);
 
     if (local_err) {
         qerror_report_err(local_err);
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] machine: replace underscores in machine's property names

[Michael S. Tsirkin]

On Sun, Jun 29, 2014 at 12:09:15PM +0300, Marcel Apfelbaum wrote:
> Replaced '_' with '-' to comply with QOM guidelines.
> Made the conversion from HMP to QMP in vl.c
> 
> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>

Nothing to do with me, pls merge through Andrea's or Paolo's tree.
FWIW

Acked-by: Michael S. Tsirkin <mst@redhat.com>


> ---
>  hw/core/machine.c |  8 ++++----
>  vl.c              | 12 +++++++++++-
>  2 files changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index cbba679..7a66c57 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -239,11 +239,11 @@ static void machine_initfn(Object *obj)
>  {
>      object_property_add_str(obj, "accel",
>                              machine_get_accel, machine_set_accel, NULL);
> -    object_property_add_bool(obj, "kernel_irqchip",
> +    object_property_add_bool(obj, "kernel-irqchip",
>                               machine_get_kernel_irqchip,
>                               machine_set_kernel_irqchip,
>                               NULL);
> -    object_property_add(obj, "kvm_shadow_mem", "int",
> +    object_property_add(obj, "kvm-shadow-mem", "int",
>                          machine_get_kvm_shadow_mem,
>                          machine_set_kvm_shadow_mem,
>                          NULL, NULL, NULL);
> @@ -257,11 +257,11 @@ static void machine_initfn(Object *obj)
>                              machine_get_dtb, machine_set_dtb, NULL);
>      object_property_add_str(obj, "dumpdtb",
>                              machine_get_dumpdtb, machine_set_dumpdtb, NULL);
> -    object_property_add(obj, "phandle_start", "int",
> +    object_property_add(obj, "phandle-start", "int",
>                          machine_get_phandle_start,
>                          machine_set_phandle_start,
>                          NULL, NULL, NULL);
> -    object_property_add_str(obj, "dt_compatible",
> +    object_property_add_str(obj, "dt-compatible",
>                              machine_get_dt_compatible,
>                              machine_set_dt_compatible,
>                              NULL);
> diff --git a/vl.c b/vl.c
> index a1686ef..7587c97 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2820,15 +2820,25 @@ static int object_set_property(const char *name, const char *value, void *opaque
>      Object *obj = OBJECT(opaque);
>      StringInputVisitor *siv;
>      Error *local_err = NULL;
> +    char *c, *qom_name;
>  
>      if (strcmp(name, "qom-type") == 0 || strcmp(name, "id") == 0 ||
>          strcmp(name, "type") == 0) {
>          return 0;
>      }
>  
> +    qom_name = g_strdup(name);
> +    c = qom_name;
> +    while (*c++) {
> +        if (*c == '_') {
> +            *c = '-';
> +        }
> +    }
> +
>      siv = string_input_visitor_new(value);
> -    object_property_set(obj, string_input_get_visitor(siv), name, &local_err);
> +    object_property_set(obj, string_input_get_visitor(siv), qom_name, &local_err);
>      string_input_visitor_cleanup(siv);
> +    free(qom_name);
>  
>      if (local_err) {
>          qerror_report_err(local_err);
> -- 
> 1.8.3.1

Re: [Qemu-devel] [PATCH] machine: replace underscores in machine's property names

[Marcel Apfelbaum]

On Sun, 2014-06-29 at 14:37 +0300, Michael S. Tsirkin wrote:
> On Sun, Jun 29, 2014 at 12:09:15PM +0300, Marcel Apfelbaum wrote:
> > Replaced '_' with '-' to comply with QOM guidelines.
> > Made the conversion from HMP to QMP in vl.c
> > 
> > Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> 
> Nothing to do with me, pls merge through Andrea's or Paolo's tree.
> FWIW
Ping.
I thought we want this in 2.1

Thanks,
Marcel

> 
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> 
> 
> > ---
> >  hw/core/machine.c |  8 ++++----
> >  vl.c              | 12 +++++++++++-
> >  2 files changed, 15 insertions(+), 5 deletions(-)
> > 
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index cbba679..7a66c57 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -239,11 +239,11 @@ static void machine_initfn(Object *obj)
> >  {
> >      object_property_add_str(obj, "accel",
> >                              machine_get_accel, machine_set_accel, NULL);
> > -    object_property_add_bool(obj, "kernel_irqchip",
> > +    object_property_add_bool(obj, "kernel-irqchip",
> >                               machine_get_kernel_irqchip,
> >                               machine_set_kernel_irqchip,
> >                               NULL);
> > -    object_property_add(obj, "kvm_shadow_mem", "int",
> > +    object_property_add(obj, "kvm-shadow-mem", "int",
> >                          machine_get_kvm_shadow_mem,
> >                          machine_set_kvm_shadow_mem,
> >                          NULL, NULL, NULL);
> > @@ -257,11 +257,11 @@ static void machine_initfn(Object *obj)
> >                              machine_get_dtb, machine_set_dtb, NULL);
> >      object_property_add_str(obj, "dumpdtb",
> >                              machine_get_dumpdtb, machine_set_dumpdtb, NULL);
> > -    object_property_add(obj, "phandle_start", "int",
> > +    object_property_add(obj, "phandle-start", "int",
> >                          machine_get_phandle_start,
> >                          machine_set_phandle_start,
> >                          NULL, NULL, NULL);
> > -    object_property_add_str(obj, "dt_compatible",
> > +    object_property_add_str(obj, "dt-compatible",
> >                              machine_get_dt_compatible,
> >                              machine_set_dt_compatible,
> >                              NULL);
> > diff --git a/vl.c b/vl.c
> > index a1686ef..7587c97 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -2820,15 +2820,25 @@ static int object_set_property(const char *name, const char *value, void *opaque
> >      Object *obj = OBJECT(opaque);
> >      StringInputVisitor *siv;
> >      Error *local_err = NULL;
> > +    char *c, *qom_name;
> >  
> >      if (strcmp(name, "qom-type") == 0 || strcmp(name, "id") == 0 ||
> >          strcmp(name, "type") == 0) {
> >          return 0;
> >      }
> >  
> > +    qom_name = g_strdup(name);
> > +    c = qom_name;
> > +    while (*c++) {
> > +        if (*c == '_') {
> > +            *c = '-';
> > +        }
> > +    }
> > +
> >      siv = string_input_visitor_new(value);
> > -    object_property_set(obj, string_input_get_visitor(siv), name, &local_err);
> > +    object_property_set(obj, string_input_get_visitor(siv), qom_name, &local_err);
> >      string_input_visitor_cleanup(siv);
> > +    free(qom_name);
> >  
> >      if (local_err) {
> >          qerror_report_err(local_err);
> > -- 
> > 1.8.3.1
> 

Re: [Qemu-devel] [PATCH] machine: replace underscores in machine's property names

[Paolo Bonzini]

Il 17/07/2014 16:15, Marcel Apfelbaum ha scritto:
> On Sun, 2014-06-29 at 14:37 +0300, Michael S. Tsirkin wrote:
>> On Sun, Jun 29, 2014 at 12:09:15PM +0300, Marcel Apfelbaum wrote:
>>> Replaced '_' with '-' to comply with QOM guidelines.
>>> Made the conversion from HMP to QMP in vl.c
>>>
>>> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
>>
>> Nothing to do with me, pls merge through Andrea's or Paolo's tree.
>> FWIW
> Ping.
> I thought we want this in 2.1

Renaming properties is fine according to the QOM guidelines, so I think 
it can be left for 2.2.

Sorry for the delay, this patch escaped me completely.

Paolo

Re: [Qemu-devel] [PATCH] machine: replace underscores in machine's property names

[Andreas Färber]

Am 17.07.2014 16:20, schrieb Paolo Bonzini:
> Il 17/07/2014 16:15, Marcel Apfelbaum ha scritto:
>> On Sun, 2014-06-29 at 14:37 +0300, Michael S. Tsirkin wrote:
>>> On Sun, Jun 29, 2014 at 12:09:15PM +0300, Marcel Apfelbaum wrote:
>>>> Replaced '_' with '-' to comply with QOM guidelines.
>>>> Made the conversion from HMP to QMP in vl.c
>>>>
>>>> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
>>>
>>> Nothing to do with me, pls merge through Andrea's or Paolo's tree.
>>> FWIW
>> Ping.
>> I thought we want this in 2.1
> 
> Renaming properties is fine according to the QOM guidelines, so I think
> it can be left for 2.2.
> 
> Sorry for the delay, this patch escaped me completely.

Sorry, just seeing this patch now, too.

My argument for getting this into 2.1 had been to avoid tools picking up
these to-be-renamed property names from the start. At this point, I'm
not so sure whether it's worse to break management tools or potentially
some rarely used/tested option - if we decide for 2.2, is backporting to
2.1.1 an option if we document it in the release notes?

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] machine: replace underscores in machine's property names

[Michael Roth]

Quoting Andreas Färber (2014-07-17 10:48:59)
> Am 17.07.2014 16:20, schrieb Paolo Bonzini:
> > Il 17/07/2014 16:15, Marcel Apfelbaum ha scritto:
> >> On Sun, 2014-06-29 at 14:37 +0300, Michael S. Tsirkin wrote:
> >>> On Sun, Jun 29, 2014 at 12:09:15PM +0300, Marcel Apfelbaum wrote:
> >>>> Replaced '_' with '-' to comply with QOM guidelines.
> >>>> Made the conversion from HMP to QMP in vl.c
> >>>>
> >>>> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> >>>
> >>> Nothing to do with me, pls merge through Andrea's or Paolo's tree.
> >>> FWIW
> >> Ping.
> >> I thought we want this in 2.1
> > 
> > Renaming properties is fine according to the QOM guidelines, so I think
> > it can be left for 2.2.
> > 
> > Sorry for the delay, this patch escaped me completely.
> 
> Sorry, just seeing this patch now, too.
> 
> My argument for getting this into 2.1 had been to avoid tools picking up
> these to-be-renamed property names from the start. At this point, I'm
> not so sure whether it's worse to break management tools or potentially
> some rarely used/tested option - if we decide for 2.2, is backporting to
> 2.1.1 an option if we document it in the release notes?

IMO, if there's some risk to breaking management or other tools, I'd
rather it be left to major releases. And if these values are already misnamed
for 2.1.0 and prior, I don't think we stop it from poliferating much more by
pushing the fix up by a few months.

I do think it makes sense to pull it in for 2.1.0-rc3, but I think that's a
priority call and this seems fairly low risk.

> 
> Regards,
> Andreas
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] machine: replace underscores in machine's property names

[Paolo Bonzini]

Il 17/07/2014 18:47, Michael Roth ha scritto:
>> > My argument for getting this into 2.1 had been to avoid tools picking up
>> > these to-be-renamed property names from the start. At this point, I'm
>> > not so sure whether it's worse to break management tools or potentially
>> > some rarely used/tested option - if we decide for 2.2, is backporting to
>> > 2.1.1 an option if we document it in the release notes?
> IMO, if there's some risk to breaking management or other tools, I'd
> rather it be left to major releases. And if these values are already misnamed
> for 2.1.0 and prior, I don't think we stop it from poliferating much more by
> pushing the fix up by a few months.

I'm not sure in which case management could break (except for qom-get). 
  Andreas, can you explain?

Paolo

Re: [Qemu-devel] [PATCH] machine: replace underscores in machine's property names

[Andreas Färber]

Am 17.07.2014 19:00, schrieb Paolo Bonzini:
> Il 17/07/2014 18:47, Michael Roth ha scritto:
>>> > My argument for getting this into 2.1 had been to avoid tools
>>> picking up
>>> > these to-be-renamed property names from the start. At this point, I'm
>>> > not so sure whether it's worse to break management tools or
>>> potentially
>>> > some rarely used/tested option - if we decide for 2.2, is
>>> backporting to
>>> > 2.1.1 an option if we document it in the release notes?
>> IMO, if there's some risk to breaking management or other tools, I'd
>> rather it be left to major releases. And if these values are already
>> misnamed
>> for 2.1.0 and prior, I don't think we stop it from poliferating much
>> more by
>> pushing the fix up by a few months.
> 
> I'm not sure in which case management could break (except for qom-get).
>  Andreas, can you explain?

I was mainly concerned about qom-set, but same goes for qom-get. The
breakage would be in 2.2, if in 2.1 we introduce properties with foo_bar
and rename them to foo-bar in 2.2. Since they're not in 2.0, I had asked
Marcel to rename them for 2.1 on a KVM call.

I checked that sPAPR is not affected, so the only issue is the trivial
g_free(). Since apart from sPAPR we have a compact snippet of properties
being added, grep'ing for occurrences of the old strings and verifying
that the patch changes all properties should be safe for -rc3 if Peter
would be willing to take a pull.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] machine: replace underscores in machine's property names

[Paolo Bonzini]

Il 18/07/2014 16:15, Andreas Färber ha scritto:
> I was mainly concerned about qom-set, but same goes for qom-get. The
> breakage would be in 2.2, if in 2.1 we introduce properties with foo_bar
> and rename them to foo-bar in 2.2. Since they're not in 2.0, I had asked
> Marcel to rename them for 2.1 on a KVM call.
> 
> I checked that sPAPR is not affected, so the only issue is the trivial
> g_free(). Since apart from sPAPR we have a compact snippet of properties
> being added, grep'ing for occurrences of the old strings and verifying
> that the patch changes all properties should be safe for -rc3 if Peter
> would be willing to take a pull.

Okay, I agree.

Paolo

Re: [Qemu-devel] [PATCH] machine: replace underscores in machine's property names

[Marcel Apfelbaum]

On Fri, 2014-07-18 at 16:15 +0200, Andreas Färber wrote:
> Am 17.07.2014 19:00, schrieb Paolo Bonzini:
> > Il 17/07/2014 18:47, Michael Roth ha scritto:
> >>> > My argument for getting this into 2.1 had been to avoid tools
> >>> picking up
> >>> > these to-be-renamed property names from the start. At this point, I'm
> >>> > not so sure whether it's worse to break management tools or
> >>> potentially
> >>> > some rarely used/tested option - if we decide for 2.2, is
> >>> backporting to
> >>> > 2.1.1 an option if we document it in the release notes?
> >> IMO, if there's some risk to breaking management or other tools, I'd
> >> rather it be left to major releases. And if these values are already
> >> misnamed
> >> for 2.1.0 and prior, I don't think we stop it from poliferating much
> >> more by
> >> pushing the fix up by a few months.
> > 
> > I'm not sure in which case management could break (except for qom-get).
> >  Andreas, can you explain?
> 
> I was mainly concerned about qom-set, but same goes for qom-get. The
> breakage would be in 2.2, if in 2.1 we introduce properties with foo_bar
> and rename them to foo-bar in 2.2. Since they're not in 2.0, I had asked
> Marcel to rename them for 2.1 on a KVM call.
> 
> I checked that sPAPR is not affected, so the only issue is the trivial
> g_free(). Since apart from sPAPR we have a compact snippet of properties
> being added, grep'ing for occurrences of the old strings and verifying
> that the patch changes all properties should be safe for -rc3 if Peter
> would be willing to take a pull.
Hi,

The patch only affects machine properties.
The patch will be upstream in a few minutes. Sorry for the delay.

Thanks,
Marcel
> 
> Andreas
> 

Re: [Qemu-devel] [PATCH] machine: replace underscores in machine's property names

[Peter Maydell]

On 29 June 2014 10:09, Marcel Apfelbaum <marcel.a@redhat.com> wrote:
> Replaced '_' with '-' to comply with QOM guidelines.
> Made the conversion from HMP to QMP in vl.c
>
> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>

> index a1686ef..7587c97 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2820,15 +2820,25 @@ static int object_set_property(const char *name, const char *value, void *opaque
>      Object *obj = OBJECT(opaque);
>      StringInputVisitor *siv;
>      Error *local_err = NULL;
> +    char *c, *qom_name;
>
>      if (strcmp(name, "qom-type") == 0 || strcmp(name, "id") == 0 ||
>          strcmp(name, "type") == 0) {
>          return 0;
>      }
>
> +    qom_name = g_strdup(name);

Memory allocated with g_strdup...

> +    c = qom_name;
> +    while (*c++) {
> +        if (*c == '_') {
> +            *c = '-';
> +        }
> +    }
> +
>      siv = string_input_visitor_new(value);
> -    object_property_set(obj, string_input_get_visitor(siv), name, &local_err);
> +    object_property_set(obj, string_input_get_visitor(siv), qom_name, &local_err);
>      string_input_visitor_cleanup(siv);
> +    free(qom_name);

...but freed with free rather than g_free.

>
>      if (local_err) {
>          qerror_report_err(local_err);
> --
> 1.8.3.1

thanks
-- PMM

Re: [Qemu-devel] [PATCH] machine: replace underscores in machine's property names

[Marcel Apfelbaum]

On Thu, 2014-07-17 at 17:55 +0100, Peter Maydell wrote:
> On 29 June 2014 10:09, Marcel Apfelbaum <marcel.a@redhat.com> wrote:
> > Replaced '_' with '-' to comply with QOM guidelines.
> > Made the conversion from HMP to QMP in vl.c
> >
> > Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> 
> > index a1686ef..7587c97 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -2820,15 +2820,25 @@ static int object_set_property(const char *name, const char *value, void *opaque
> >      Object *obj = OBJECT(opaque);
> >      StringInputVisitor *siv;
> >      Error *local_err = NULL;
> > +    char *c, *qom_name;
> >
> >      if (strcmp(name, "qom-type") == 0 || strcmp(name, "id") == 0 ||
> >          strcmp(name, "type") == 0) {
> >          return 0;
> >      }
> >
> > +    qom_name = g_strdup(name);
> 
> Memory allocated with g_strdup...
> 
> > +    c = qom_name;
> > +    while (*c++) {
> > +        if (*c == '_') {
> > +            *c = '-';
> > +        }
> > +    }
> > +
> >      siv = string_input_visitor_new(value);
> > -    object_property_set(obj, string_input_get_visitor(siv), name, &local_err);
> > +    object_property_set(obj, string_input_get_visitor(siv), qom_name, &local_err);
> >      string_input_visitor_cleanup(siv);
> > +    free(qom_name);
> 
> ...but freed with free rather than g_free.
Thanks, I'll take care of that.
Marcel

> 
> >
> >      if (local_err) {
> >          qerror_report_err(local_err);
> > --
> > 1.8.3.1
> 
> thanks
> -- PMM

Re: [Qemu-devel] [PATCH] machine: replace underscores in machine's property names

[Andreas Färber]

Am 29.06.2014 11:09, schrieb Marcel Apfelbaum:
> Replaced '_' with '-' to comply with QOM guidelines.
> Made the conversion from HMP to QMP in vl.c
> 
> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> ---
>  hw/core/machine.c |  8 ++++----
>  vl.c              | 12 +++++++++++-
>  2 files changed, 15 insertions(+), 5 deletions(-)
[snip]
> diff --git a/vl.c b/vl.c
> index a1686ef..7587c97 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2820,15 +2820,25 @@ static int object_set_property(const char *name, const char *value, void *opaque
>      Object *obj = OBJECT(opaque);
>      StringInputVisitor *siv;
>      Error *local_err = NULL;
> +    char *c, *qom_name;
>  
>      if (strcmp(name, "qom-type") == 0 || strcmp(name, "id") == 0 ||
>          strcmp(name, "type") == 0) {
>          return 0;
>      }
>  
> +    qom_name = g_strdup(name);
> +    c = qom_name;
> +    while (*c++) {
> +        if (*c == '_') {
> +            *c = '-';
> +        }
> +    }

Actually, is this really safe? By my reading, this function handles
-object as well, which in turn allows - in theory - to instantiate any
device, where some will still have underscores in their property names.
Not sure if all non-device objects such as virtio-rng backends have been
checked?

Since it's really late, I would be more comfortable to copy this
function with a large TODO and only apply this fixup for -machine, where
we are more easily able to review and test. Opinions?

Regards,
Andreas

> +
>      siv = string_input_visitor_new(value);
> -    object_property_set(obj, string_input_get_visitor(siv), name, &local_err);
> +    object_property_set(obj, string_input_get_visitor(siv), qom_name, &local_err);
>      string_input_visitor_cleanup(siv);
> +    free(qom_name);
>  
>      if (local_err) {
>          qerror_report_err(local_err);
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] machine: replace underscores in machine's property names

[Paolo Bonzini]

Il 18/07/2014 16:25, Andreas Färber ha scritto:
> Actually, is this really safe? By my reading, this function handles
> -object as well, which in turn allows - in theory - to instantiate any
> device, where some will still have underscores in their property names.
> Not sure if all non-device objects such as virtio-rng backends have been
> checked?

I guess you would have to change devices from _ to - before adding
UserCreatable to devices.

-object backends are safe, they only use - (virtio-rng doesn't even have
dashes).

Paolo

Re: [Qemu-devel] [PATCH] machine: replace underscores in machine's property names

[Marcel Apfelbaum]

On Fri, 2014-07-18 at 16:25 +0200, Andreas Färber wrote:
> Am 29.06.2014 11:09, schrieb Marcel Apfelbaum:
> > Replaced '_' with '-' to comply with QOM guidelines.
> > Made the conversion from HMP to QMP in vl.c
> > 
> > Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> > ---
> >  hw/core/machine.c |  8 ++++----
> >  vl.c              | 12 +++++++++++-
> >  2 files changed, 15 insertions(+), 5 deletions(-)
> [snip]
> > diff --git a/vl.c b/vl.c
> > index a1686ef..7587c97 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -2820,15 +2820,25 @@ static int object_set_property(const char *name, const char *value, void *opaque
> >      Object *obj = OBJECT(opaque);
> >      StringInputVisitor *siv;
> >      Error *local_err = NULL;
> > +    char *c, *qom_name;
> >  
> >      if (strcmp(name, "qom-type") == 0 || strcmp(name, "id") == 0 ||
> >          strcmp(name, "type") == 0) {
> >          return 0;
> >      }
> >  
> > +    qom_name = g_strdup(name);
> > +    c = qom_name;
> > +    while (*c++) {
> > +        if (*c == '_') {
> > +            *c = '-';
> > +        }
> > +    }
> 
> Actually, is this really safe? By my reading, this function handles
> -object as well, which in turn allows - in theory - to instantiate any
> device, where some will still have underscores in their property names.
> Not sure if all non-device objects such as virtio-rng backends have been
> checked?
Hi Andreas,

I checked and object_set_property is used only be machine right now, so
no problem here.

Thanks,
Marcel

> 
> Since it's really late, I would be more comfortable to copy this
> function with a large TODO and only apply this fixup for -machine, where
> we are more easily able to review and test. Opinions?
> 
> Regards,
> Andreas
> 
> > +
> >      siv = string_input_visitor_new(value);
> > -    object_property_set(obj, string_input_get_visitor(siv), name, &local_err);
> > +    object_property_set(obj, string_input_get_visitor(siv), qom_name, &local_err);
> >      string_input_visitor_cleanup(siv);
> > +    free(qom_name);
> >  
> >      if (local_err) {
> >          qerror_report_err(local_err);
> > 
> 
> 

Re: [Qemu-devel] [PATCH] machine: replace underscores in machine's property names

[Andreas Färber]

Hi,

Am 18.07.2014 17:59, schrieb Marcel Apfelbaum:
> On Fri, 2014-07-18 at 16:25 +0200, Andreas Färber wrote:
>> Am 29.06.2014 11:09, schrieb Marcel Apfelbaum:
>>> Replaced '_' with '-' to comply with QOM guidelines.
>>> Made the conversion from HMP to QMP in vl.c
>>>
>>> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
>>> ---
>>>  hw/core/machine.c |  8 ++++----
>>>  vl.c              | 12 +++++++++++-
>>>  2 files changed, 15 insertions(+), 5 deletions(-)
>> [snip]
>>> diff --git a/vl.c b/vl.c
>>> index a1686ef..7587c97 100644
>>> --- a/vl.c
>>> +++ b/vl.c
>>> @@ -2820,15 +2820,25 @@ static int object_set_property(const char *name, const char *value, void *opaque
>>>      Object *obj = OBJECT(opaque);
>>>      StringInputVisitor *siv;
>>>      Error *local_err = NULL;
>>> +    char *c, *qom_name;
>>>  
>>>      if (strcmp(name, "qom-type") == 0 || strcmp(name, "id") == 0 ||
>>>          strcmp(name, "type") == 0) {
>>>          return 0;
>>>      }
>>>  
>>> +    qom_name = g_strdup(name);
>>> +    c = qom_name;
>>> +    while (*c++) {
>>> +        if (*c == '_') {
>>> +            *c = '-';
>>> +        }
>>> +    }
>>
>> Actually, is this really safe? By my reading, this function handles
>> -object as well, which in turn allows - in theory - to instantiate any
>> device, where some will still have underscores in their property names.
>> Not sure if all non-device objects such as virtio-rng backends have been
>> checked?
> Hi Andreas,
> 
> I checked and object_set_property is used only be machine right now, so
> no problem here.

Indeed you are right. If -object is no longer using it, can we drop
qom-type handling? What changed there?

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] machine: replace underscores in machine's property names

[Marcel Apfelbaum]

On Fri, 2014-07-18 at 18:10 +0200, Andreas Färber wrote:
> Hi,
> 
> Am 18.07.2014 17:59, schrieb Marcel Apfelbaum:
> > On Fri, 2014-07-18 at 16:25 +0200, Andreas Färber wrote:
> >> Am 29.06.2014 11:09, schrieb Marcel Apfelbaum:
> >>> Replaced '_' with '-' to comply with QOM guidelines.
> >>> Made the conversion from HMP to QMP in vl.c
> >>>
> >>> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> >>> ---
> >>>  hw/core/machine.c |  8 ++++----
> >>>  vl.c              | 12 +++++++++++-
> >>>  2 files changed, 15 insertions(+), 5 deletions(-)
> >> [snip]
> >>> diff --git a/vl.c b/vl.c
> >>> index a1686ef..7587c97 100644
> >>> --- a/vl.c
> >>> +++ b/vl.c
> >>> @@ -2820,15 +2820,25 @@ static int object_set_property(const char *name, const char *value, void *opaque
> >>>      Object *obj = OBJECT(opaque);
> >>>      StringInputVisitor *siv;
> >>>      Error *local_err = NULL;
> >>> +    char *c, *qom_name;
> >>>  
> >>>      if (strcmp(name, "qom-type") == 0 || strcmp(name, "id") == 0 ||
> >>>          strcmp(name, "type") == 0) {
> >>>          return 0;
> >>>      }
> >>>  
> >>> +    qom_name = g_strdup(name);
> >>> +    c = qom_name;
> >>> +    while (*c++) {
> >>> +        if (*c == '_') {
> >>> +            *c = '-';
> >>> +        }
> >>> +    }
> >>
> >> Actually, is this really safe? By my reading, this function handles
> >> -object as well, which in turn allows - in theory - to instantiate any
> >> device, where some will still have underscores in their property names.
> >> Not sure if all non-device objects such as virtio-rng backends have been
> >> checked?
> > Hi Andreas,
> > 
> > I checked and object_set_property is used only be machine right now, so
> > no problem here.
> 
> Indeed you are right. If -object is no longer using it, can we drop
> qom-type handling? What changed there?
Hi Andreas,

The check was originally placed there by Paolo for -object handling.
We need to find out where the "qom-type" property is coming from. (What code is adding it)
If is added automatically at parse/init time we can't get rid of it.
If is object specific, it is ok.
Paolo, can you please advise?

Thanks,
Marcel 


> Regards,
> Andreas
> 

Re: [Qemu-devel] [PATCH] machine: replace underscores in machine's property names

[Andreas Färber]

Am 18.07.2014 18:23, schrieb Marcel Apfelbaum:
> On Fri, 2014-07-18 at 18:10 +0200, Andreas Färber wrote:
>> Hi,
>>
>> Am 18.07.2014 17:59, schrieb Marcel Apfelbaum:
>>> On Fri, 2014-07-18 at 16:25 +0200, Andreas Färber wrote:
>>>> Am 29.06.2014 11:09, schrieb Marcel Apfelbaum:
>>>>> Replaced '_' with '-' to comply with QOM guidelines.
>>>>> Made the conversion from HMP to QMP in vl.c
>>>>>
>>>>> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
>>>>> ---
>>>>>  hw/core/machine.c |  8 ++++----
>>>>>  vl.c              | 12 +++++++++++-
>>>>>  2 files changed, 15 insertions(+), 5 deletions(-)
>>>> [snip]
>>>>> diff --git a/vl.c b/vl.c
>>>>> index a1686ef..7587c97 100644
>>>>> --- a/vl.c
>>>>> +++ b/vl.c
>>>>> @@ -2820,15 +2820,25 @@ static int object_set_property(const char *name, const char *value, void *opaque
>>>>>      Object *obj = OBJECT(opaque);
>>>>>      StringInputVisitor *siv;
>>>>>      Error *local_err = NULL;
>>>>> +    char *c, *qom_name;
>>>>>  
>>>>>      if (strcmp(name, "qom-type") == 0 || strcmp(name, "id") == 0 ||
>>>>>          strcmp(name, "type") == 0) {
>>>>>          return 0;
>>>>>      }
>>>>>  
>>>>> +    qom_name = g_strdup(name);
>>>>> +    c = qom_name;
>>>>> +    while (*c++) {
>>>>> +        if (*c == '_') {
>>>>> +            *c = '-';
>>>>> +        }
>>>>> +    }
>>>>
>>>> Actually, is this really safe? By my reading, this function handles
>>>> -object as well, which in turn allows - in theory - to instantiate any
>>>> device, where some will still have underscores in their property names.
>>>> Not sure if all non-device objects such as virtio-rng backends have been
>>>> checked?
>>> Hi Andreas,
>>>
>>> I checked and object_set_property is used only be machine right now, so
>>> no problem here.
>>
>> Indeed you are right. If -object is no longer using it, can we drop
>> qom-type handling? What changed there?
> Hi Andreas,
> 
> The check was originally placed there by Paolo for -object handling.
> We need to find out where the "qom-type" property is coming from. (What code is adding it)
> If is added automatically at parse/init time we can't get rid of it.
> If is object specific, it is ok.

It was not a QOM property, it was a QemuOpt parameter for -object and
therefore excluded from the handling like your type property is. Paolo
had dicussed to rename qom-type to type for simplicity and consistency,
but what I don't know is why this function is no longer used.

Andreas

> Paolo, can you please advise?
> 
> Thanks,
> Marcel

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] machine: replace underscores in machine's property names

[Marcel Apfelbaum]

On Fri, 2014-07-18 at 18:32 +0200, Andreas Färber wrote:
> Am 18.07.2014 18:23, schrieb Marcel Apfelbaum:
> > On Fri, 2014-07-18 at 18:10 +0200, Andreas Färber wrote:
> >> Hi,
> >>
> >> Am 18.07.2014 17:59, schrieb Marcel Apfelbaum:
> >>> On Fri, 2014-07-18 at 16:25 +0200, Andreas Färber wrote:
> >>>> Am 29.06.2014 11:09, schrieb Marcel Apfelbaum:
> >>>>> Replaced '_' with '-' to comply with QOM guidelines.
> >>>>> Made the conversion from HMP to QMP in vl.c
> >>>>>
> >>>>> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> >>>>> ---
> >>>>>  hw/core/machine.c |  8 ++++----
> >>>>>  vl.c              | 12 +++++++++++-
> >>>>>  2 files changed, 15 insertions(+), 5 deletions(-)
> >>>> [snip]
> >>>>> diff --git a/vl.c b/vl.c
> >>>>> index a1686ef..7587c97 100644
> >>>>> --- a/vl.c
> >>>>> +++ b/vl.c
> >>>>> @@ -2820,15 +2820,25 @@ static int object_set_property(const char *name, const char *value, void *opaque
> >>>>>      Object *obj = OBJECT(opaque);
> >>>>>      StringInputVisitor *siv;
> >>>>>      Error *local_err = NULL;
> >>>>> +    char *c, *qom_name;
> >>>>>  
> >>>>>      if (strcmp(name, "qom-type") == 0 || strcmp(name, "id") == 0 ||
> >>>>>          strcmp(name, "type") == 0) {
> >>>>>          return 0;
> >>>>>      }
> >>>>>  
> >>>>> +    qom_name = g_strdup(name);
> >>>>> +    c = qom_name;
> >>>>> +    while (*c++) {
> >>>>> +        if (*c == '_') {
> >>>>> +            *c = '-';
> >>>>> +        }
> >>>>> +    }
> >>>>
> >>>> Actually, is this really safe? By my reading, this function handles
> >>>> -object as well, which in turn allows - in theory - to instantiate any
> >>>> device, where some will still have underscores in their property names.
> >>>> Not sure if all non-device objects such as virtio-rng backends have been
> >>>> checked?
> >>> Hi Andreas,
> >>>
> >>> I checked and object_set_property is used only be machine right now, so
> >>> no problem here.
> >>
> >> Indeed you are right. If -object is no longer using it, can we drop
> >> qom-type handling? What changed there?
> > Hi Andreas,
> > 
> > The check was originally placed there by Paolo for -object handling.
> > We need to find out where the "qom-type" property is coming from. (What code is adding it)
> > If is added automatically at parse/init time we can't get rid of it.
> > If is object specific, it is ok.
> 
> It was not a QOM property, it was a QemuOpt parameter for -object and
> therefore excluded from the handling like your type property is. Paolo
> had dicussed to rename qom-type to type for simplicity and consistency,
> but what I don't know is why this function is no longer used.
Paolo, can you help?
I posted a V2, but I can resend in case it is needed, of course.

Thanks,
Marcel
> 
> Andreas
> 
> > Paolo, can you please advise?
> > 
> > Thanks,
> > Marcel
> 

Re: [Qemu-devel] [PATCH] machine: replace underscores in machine's property names

[Paolo Bonzini]

Il 18/07/2014 18:32, Andreas Färber ha scritto:
> Am 18.07.2014 18:23, schrieb Marcel Apfelbaum:
>> On Fri, 2014-07-18 at 18:10 +0200, Andreas Färber wrote:
>>> Hi,
>>>
>>> Am 18.07.2014 17:59, schrieb Marcel Apfelbaum:
>>>> On Fri, 2014-07-18 at 16:25 +0200, Andreas Färber wrote:
>>>>> Am 29.06.2014 11:09, schrieb Marcel Apfelbaum:
>>>>>> Replaced '_' with '-' to comply with QOM guidelines.
>>>>>> Made the conversion from HMP to QMP in vl.c
>>>>>>
>>>>>> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
>>>>>> ---
>>>>>>  hw/core/machine.c |  8 ++++----
>>>>>>  vl.c              | 12 +++++++++++-
>>>>>>  2 files changed, 15 insertions(+), 5 deletions(-)
>>>>> [snip]
>>>>>> diff --git a/vl.c b/vl.c
>>>>>> index a1686ef..7587c97 100644
>>>>>> --- a/vl.c
>>>>>> +++ b/vl.c
>>>>>> @@ -2820,15 +2820,25 @@ static int object_set_property(const char *name, const char *value, void *opaque
>>>>>>      Object *obj = OBJECT(opaque);
>>>>>>      StringInputVisitor *siv;
>>>>>>      Error *local_err = NULL;
>>>>>> +    char *c, *qom_name;
>>>>>>  
>>>>>>      if (strcmp(name, "qom-type") == 0 || strcmp(name, "id") == 0 ||
>>>>>>          strcmp(name, "type") == 0) {
>>>>>>          return 0;
>>>>>>      }
>>>>>>  
>>>>>> +    qom_name = g_strdup(name);
>>>>>> +    c = qom_name;
>>>>>> +    while (*c++) {
>>>>>> +        if (*c == '_') {
>>>>>> +            *c = '-';
>>>>>> +        }
>>>>>> +    }
>>>>>
>>>>> Actually, is this really safe? By my reading, this function handles
>>>>> -object as well, which in turn allows - in theory - to instantiate any
>>>>> device, where some will still have underscores in their property names.
>>>>> Not sure if all non-device objects such as virtio-rng backends have been
>>>>> checked?
>>>> Hi Andreas,
>>>>
>>>> I checked and object_set_property is used only be machine right now, so
>>>> no problem here.
>>>
>>> Indeed you are right. If -object is no longer using it, can we drop
>>> qom-type handling? What changed there?
>> Hi Andreas,
>>
>> The check was originally placed there by Paolo for -object handling.
>> We need to find out where the "qom-type" property is coming from. (What code is adding it)
>> If is added automatically at parse/init time we can't get rid of it.
>> If is object specific, it is ok.
> 
> It was not a QOM property, it was a QemuOpt parameter for -object and
> therefore excluded from the handling like your type property is. Paolo
> had dicussed to rename qom-type to type for simplicity and consistency,
> but what I don't know is why this function is no longer used.

It is not used anymore because -object is now parsed using OptsVisitor;
see function object_create.

Paolo