Re: [Qemu-devel] [patch qemu 3/3] rocker: allow user to specify rocker world by property

2016-02-25 Thread Jiri Pirko 
Thu, Feb 25, 2016 at 12:31:58PM CET, stefa...@gmail.com wrote:
>On Mon, Feb 22, 2016 at 07:06:34PM +0100, Jiri Pirko wrote:
>> Mon, Feb 22, 2016 at 06:51:50PM CET, stefa...@gmail.com wrote:
>> >On Fri, Feb 19, 2016 at 11:06:43AM +0100, Jiri Pirko wrote:
>> >> @@ -1297,7 +1310,18 @@ static int pci_rocker_init(PCIDevice *dev)
>> >>  /* allocate worlds */
>> >>  
>> >>  r->worlds[ROCKER_WORLD_TYPE_OF_DPA] = of_dpa_world_alloc(r);
>> >> -r->world_dflt = r->worlds[ROCKER_WORLD_TYPE_OF_DPA];
>> >> +
>> >> +if (!r->world_name) {
>> >> +r->world_name = 
>> >> g_strdup(world_name(r->worlds[ROCKER_WORLD_TYPE_OF_DPA]));
>> >> +}
>> >> +
>> >> +r->world_dflt = rocker_world_type_by_name(r, r->world_name);
>> >> +if (!r->world_dflt) {
>> >> +fprintf(stderr,
>> >> +"rocker: requested world \"%s\" does not exist\n",
>> >> +r->world_name);
>> >> +return -EINVAL;
>> >> +}
>> >
>> >world_name is leaked here.  Please use goto to run the appropriate
>> >cleanup code instead of returning directly.
>> 
>> I did the same what is done with "r->name = g_strdup(ROCKER)"
>> 
>> I assumed since this is a property, the caller core will take care of
>> that.
>
>You are right, the string properties will be freed by qdev property
>release.
>
>The return statement added by this patch still leaks
>r->worlds[ROCKER_WORLD_TYPE_OF_DPA] so goto err_world_alloc is needed.
>
>(pci_rocker_uninit() is not called if pci_rocker_init() fails.)

Will fix and send v2. Thanks.

Re: [Qemu-devel] [patch qemu 3/3] rocker: allow user to specify rocker world by property

2016-02-25 Thread Stefan Hajnoczi 
On Mon, Feb 22, 2016 at 07:06:34PM +0100, Jiri Pirko wrote:
> Mon, Feb 22, 2016 at 06:51:50PM CET, stefa...@gmail.com wrote:
> >On Fri, Feb 19, 2016 at 11:06:43AM +0100, Jiri Pirko wrote:
> >> @@ -1297,7 +1310,18 @@ static int pci_rocker_init(PCIDevice *dev)
> >>  /* allocate worlds */
> >>  
> >>  r->worlds[ROCKER_WORLD_TYPE_OF_DPA] = of_dpa_world_alloc(r);
> >> -r->world_dflt = r->worlds[ROCKER_WORLD_TYPE_OF_DPA];
> >> +
> >> +if (!r->world_name) {
> >> +r->world_name = 
> >> g_strdup(world_name(r->worlds[ROCKER_WORLD_TYPE_OF_DPA]));
> >> +}
> >> +
> >> +r->world_dflt = rocker_world_type_by_name(r, r->world_name);
> >> +if (!r->world_dflt) {
> >> +fprintf(stderr,
> >> +"rocker: requested world \"%s\" does not exist\n",
> >> +r->world_name);
> >> +return -EINVAL;
> >> +}
> >
> >world_name is leaked here.  Please use goto to run the appropriate
> >cleanup code instead of returning directly.
> 
> I did the same what is done with "r->name = g_strdup(ROCKER)"
> 
> I assumed since this is a property, the caller core will take care of
> that.

You are right, the string properties will be freed by qdev property
release.

The return statement added by this patch still leaks
r->worlds[ROCKER_WORLD_TYPE_OF_DPA] so goto err_world_alloc is needed.

(pci_rocker_uninit() is not called if pci_rocker_init() fails.)


signature.asc
Description: PGP signature

Re: [Qemu-devel] [patch qemu 3/3] rocker: allow user to specify rocker world by property

2016-02-22 Thread Jiri Pirko 
Mon, Feb 22, 2016 at 06:51:50PM CET, stefa...@gmail.com wrote:
>On Fri, Feb 19, 2016 at 11:06:43AM +0100, Jiri Pirko wrote:
>> @@ -1297,7 +1310,18 @@ static int pci_rocker_init(PCIDevice *dev)
>>  /* allocate worlds */
>>  
>>  r->worlds[ROCKER_WORLD_TYPE_OF_DPA] = of_dpa_world_alloc(r);
>> -r->world_dflt = r->worlds[ROCKER_WORLD_TYPE_OF_DPA];
>> +
>> +if (!r->world_name) {
>> +r->world_name = 
>> g_strdup(world_name(r->worlds[ROCKER_WORLD_TYPE_OF_DPA]));
>> +}
>> +
>> +r->world_dflt = rocker_world_type_by_name(r, r->world_name);
>> +if (!r->world_dflt) {
>> +fprintf(stderr,
>> +"rocker: requested world \"%s\" does not exist\n",
>> +r->world_name);
>> +return -EINVAL;
>> +}
>
>world_name is leaked here.  Please use goto to run the appropriate
>cleanup code instead of returning directly.

I did the same what is done with "r->name = g_strdup(ROCKER)"

I assumed since this is a property, the caller core will take care of
that.

Re: [Qemu-devel] [patch qemu 3/3] rocker: allow user to specify rocker world by property

2016-02-22 Thread Stefan Hajnoczi 
On Fri, Feb 19, 2016 at 11:06:43AM +0100, Jiri Pirko wrote:
> @@ -1297,7 +1310,18 @@ static int pci_rocker_init(PCIDevice *dev)
>  /* allocate worlds */
>  
>  r->worlds[ROCKER_WORLD_TYPE_OF_DPA] = of_dpa_world_alloc(r);
> -r->world_dflt = r->worlds[ROCKER_WORLD_TYPE_OF_DPA];
> +
> +if (!r->world_name) {
> +r->world_name = 
> g_strdup(world_name(r->worlds[ROCKER_WORLD_TYPE_OF_DPA]));
> +}
> +
> +r->world_dflt = rocker_world_type_by_name(r, r->world_name);
> +if (!r->world_dflt) {
> +fprintf(stderr,
> +"rocker: requested world \"%s\" does not exist\n",
> +r->world_name);
> +return -EINVAL;
> +}

world_name is leaked here.  Please use goto to run the appropriate
cleanup code instead of returning directly.


signature.asc
Description: PGP signature

[Qemu-devel] [patch qemu 3/3] rocker: allow user to specify rocker world by property

2016-02-19 Thread Jiri Pirko 
From: Jiri Pirko 

Add property to specify rocker world. All ports will be assigned to this
world.

Signed-off-by: Jiri Pirko 
---
 hw/net/rocker/rocker.c | 27 ++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
index a1d921d..d0bdfcb 100644
--- a/hw/net/rocker/rocker.c
+++ b/hw/net/rocker/rocker.c
@@ -43,6 +43,7 @@ struct rocker {
 
 /* switch configuration */
 char *name;  /* switch name */
+char *world_name;/* world name */
 uint32_t fp_ports;   /* front-panel port count */
 NICPeers *fp_ports_peers;
 MACAddr fp_start_macaddr;/* front-panel port 0 mac addr */
@@ -1286,6 +1287,18 @@ static void rocker_msix_uninit(Rocker *r)
 rocker_msix_vectors_unuse(r, ROCKER_MSIX_VEC_COUNT(r->fp_ports));
 }
 
+static World *rocker_world_type_by_name(Rocker *r, const char *name)
+{
+int i;
+
+for (i = 0; i < ROCKER_WORLD_TYPE_MAX; i++) {
+if (strcmp(name, world_name(r->worlds[i])) == 0) {
+return r->worlds[i];
+   }
+}
+return NULL;
+}
+
 static int pci_rocker_init(PCIDevice *dev)
 {
 Rocker *r = to_rocker(dev);
@@ -1297,7 +1310,18 @@ static int pci_rocker_init(PCIDevice *dev)
 /* allocate worlds */
 
 r->worlds[ROCKER_WORLD_TYPE_OF_DPA] = of_dpa_world_alloc(r);
-r->world_dflt = r->worlds[ROCKER_WORLD_TYPE_OF_DPA];
+
+if (!r->world_name) {
+r->world_name = 
g_strdup(world_name(r->worlds[ROCKER_WORLD_TYPE_OF_DPA]));
+}
+
+r->world_dflt = rocker_world_type_by_name(r, r->world_name);
+if (!r->world_dflt) {
+fprintf(stderr,
+"rocker: requested world \"%s\" does not exist\n",
+r->world_name);
+return -EINVAL;
+}
 
 for (i = 0; i < ROCKER_WORLD_TYPE_MAX; i++) {
 if (!r->worlds[i]) {
@@ -1509,6 +1533,7 @@ static void rocker_reset(DeviceState *dev)
 
 static Property rocker_properties[] = {
 DEFINE_PROP_STRING("name", Rocker, name),
+DEFINE_PROP_STRING("world", Rocker, world_name),
 DEFINE_PROP_MACADDR("fp_start_macaddr", Rocker,
 fp_start_macaddr),
 DEFINE_PROP_UINT64("switch_id", Rocker,
-- 
2.4.3