Re: [ovs-dev] [PATCH] ovn: Properly set the index for chassis lookup

2019-05-24 Thread Dumitru Ceara
On Thu, May 23, 2019 at 10:58 PM Ben Pfaff  wrote:
>
> On Thu, May 23, 2019 at 01:11:46PM -0700, Han Zhou wrote:
> > On Thu, May 9, 2019 at 1:09 AM Dumitru Ceara  wrote:
> > >
> > > The chassis_lookup_by_name function now calls
> > > sbrec_chassis_index_set_name instead of sbrec_chassis_set_name. Due to
> > > the use of the wrong API memory was leaked every time a chassis was
> > > looked up by name. This was mostly visible when chassis lookups had to
> > > be done continuously (e.g., when two chassis were misconfigured
> > > with the same system-id).
> > >
> > > Reported-at: https://bugzilla.redhat.com/1698462
> > > Reported-by: Alexander 
> > > Signed-off-by: Dumitru Ceara 
> > > ---
> > >  ovn/lib/chassis-index.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/ovn/lib/chassis-index.c b/ovn/lib/chassis-index.c
> > > index 34d4a31..423 100644
> > > --- a/ovn/lib/chassis-index.c
> > > +++ b/ovn/lib/chassis-index.c
> > > @@ -30,7 +30,7 @@ chassis_lookup_by_name(struct ovsdb_idl_index
> > *sbrec_chassis_by_name,
> > >  {
> > >  struct sbrec_chassis *target = sbrec_chassis_index_init_row(
> > >  sbrec_chassis_by_name);
> > > -sbrec_chassis_set_name(target, name);
> > > +sbrec_chassis_index_set_name(target, name);
> > >
> > >  struct sbrec_chassis *retval = sbrec_chassis_index_find(
> > >  sbrec_chassis_by_name, target);
> > > --
> > > 1.8.3.1
> > >
> > > ___
> > > dev mailing list
> > > d...@openvswitch.org
> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
> > Thanks Dumitru for fixing this. I see same wrong way of using index in the
> > same file:
> >
> > struct sbrec_ha_chassis_group *target =
> > sbrec_ha_chassis_group_index_init_row(sbrec_ha_chassis_grp_by_name);
> > sbrec_ha_chassis_group_set_name(target, name);
> >
> > May it be fixed together?
> > I did check all the other files under ovn, and it seems these are the only
> > places with this problem.
> >
> > Acked-by: Han Zhou 
>
> I folded in the additional fix and applied this to master.  Thank you!

Thanks Han for the additional fix (I completely missed it) and thanks
Ben for applying it!

Cheers,
Dumitru

>
> Here is the final version:
>
> --8<--cut here-->8--
>
> From: Dumitru Ceara 
> Date: Thu, 9 May 2019 10:09:23 +0200
> Subject: [PATCH] ovn: Properly set the index for chassis lookup
>
> The chassis_lookup_by_name function now calls
> sbrec_chassis_index_set_name instead of sbrec_chassis_set_name. Due to
> the use of the wrong API memory was leaked every time a chassis was
> looked up by name. This was mostly visible when chassis lookups had to
> be done continuously (e.g., when two chassis were misconfigured
> with the same system-id).
>
> Acked-by: Han Zhou 
> Reported-at: https://bugzilla.redhat.com/1698462
> Reported-by: Alexander 
> Signed-off-by: Dumitru Ceara 
> Signed-off-by: Ben Pfaff 
> ---
>  ovn/lib/chassis-index.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/ovn/lib/chassis-index.c b/ovn/lib/chassis-index.c
> index 34d4a31eb339..10f70fb4a18f 100644
> --- a/ovn/lib/chassis-index.c
> +++ b/ovn/lib/chassis-index.c
> @@ -30,7 +30,7 @@ chassis_lookup_by_name(struct ovsdb_idl_index 
> *sbrec_chassis_by_name,
>  {
>  struct sbrec_chassis *target = sbrec_chassis_index_init_row(
>  sbrec_chassis_by_name);
> -sbrec_chassis_set_name(target, name);
> +sbrec_chassis_index_set_name(target, name);
>
>  struct sbrec_chassis *retval = sbrec_chassis_index_find(
>  sbrec_chassis_by_name, target);
> @@ -55,7 +55,7 @@ ha_chassis_group_lookup_by_name(
>  {
>  struct sbrec_ha_chassis_group *target =
>  sbrec_ha_chassis_group_index_init_row(sbrec_ha_chassis_grp_by_name);
> -sbrec_ha_chassis_group_set_name(target, name);
> +sbrec_ha_chassis_group_index_set_name(target, name);
>
>  struct sbrec_ha_chassis_group *retval =
>  sbrec_ha_chassis_group_index_find(sbrec_ha_chassis_grp_by_name,
> --
> 2.20.1
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] ovn: Properly set the index for chassis lookup

2019-05-23 Thread Ben Pfaff
On Thu, May 23, 2019 at 01:11:46PM -0700, Han Zhou wrote:
> On Thu, May 9, 2019 at 1:09 AM Dumitru Ceara  wrote:
> >
> > The chassis_lookup_by_name function now calls
> > sbrec_chassis_index_set_name instead of sbrec_chassis_set_name. Due to
> > the use of the wrong API memory was leaked every time a chassis was
> > looked up by name. This was mostly visible when chassis lookups had to
> > be done continuously (e.g., when two chassis were misconfigured
> > with the same system-id).
> >
> > Reported-at: https://bugzilla.redhat.com/1698462
> > Reported-by: Alexander 
> > Signed-off-by: Dumitru Ceara 
> > ---
> >  ovn/lib/chassis-index.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/ovn/lib/chassis-index.c b/ovn/lib/chassis-index.c
> > index 34d4a31..423 100644
> > --- a/ovn/lib/chassis-index.c
> > +++ b/ovn/lib/chassis-index.c
> > @@ -30,7 +30,7 @@ chassis_lookup_by_name(struct ovsdb_idl_index
> *sbrec_chassis_by_name,
> >  {
> >  struct sbrec_chassis *target = sbrec_chassis_index_init_row(
> >  sbrec_chassis_by_name);
> > -sbrec_chassis_set_name(target, name);
> > +sbrec_chassis_index_set_name(target, name);
> >
> >  struct sbrec_chassis *retval = sbrec_chassis_index_find(
> >  sbrec_chassis_by_name, target);
> > --
> > 1.8.3.1
> >
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 
> Thanks Dumitru for fixing this. I see same wrong way of using index in the
> same file:
> 
> struct sbrec_ha_chassis_group *target =
> sbrec_ha_chassis_group_index_init_row(sbrec_ha_chassis_grp_by_name);
> sbrec_ha_chassis_group_set_name(target, name);
> 
> May it be fixed together?
> I did check all the other files under ovn, and it seems these are the only
> places with this problem.
> 
> Acked-by: Han Zhou 

I folded in the additional fix and applied this to master.  Thank you!

Here is the final version:

--8<--cut here-->8--

From: Dumitru Ceara 
Date: Thu, 9 May 2019 10:09:23 +0200
Subject: [PATCH] ovn: Properly set the index for chassis lookup

The chassis_lookup_by_name function now calls
sbrec_chassis_index_set_name instead of sbrec_chassis_set_name. Due to
the use of the wrong API memory was leaked every time a chassis was
looked up by name. This was mostly visible when chassis lookups had to
be done continuously (e.g., when two chassis were misconfigured
with the same system-id).

Acked-by: Han Zhou 
Reported-at: https://bugzilla.redhat.com/1698462
Reported-by: Alexander 
Signed-off-by: Dumitru Ceara 
Signed-off-by: Ben Pfaff 
---
 ovn/lib/chassis-index.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ovn/lib/chassis-index.c b/ovn/lib/chassis-index.c
index 34d4a31eb339..10f70fb4a18f 100644
--- a/ovn/lib/chassis-index.c
+++ b/ovn/lib/chassis-index.c
@@ -30,7 +30,7 @@ chassis_lookup_by_name(struct ovsdb_idl_index 
*sbrec_chassis_by_name,
 {
 struct sbrec_chassis *target = sbrec_chassis_index_init_row(
 sbrec_chassis_by_name);
-sbrec_chassis_set_name(target, name);
+sbrec_chassis_index_set_name(target, name);
 
 struct sbrec_chassis *retval = sbrec_chassis_index_find(
 sbrec_chassis_by_name, target);
@@ -55,7 +55,7 @@ ha_chassis_group_lookup_by_name(
 {
 struct sbrec_ha_chassis_group *target =
 sbrec_ha_chassis_group_index_init_row(sbrec_ha_chassis_grp_by_name);
-sbrec_ha_chassis_group_set_name(target, name);
+sbrec_ha_chassis_group_index_set_name(target, name);
 
 struct sbrec_ha_chassis_group *retval =
 sbrec_ha_chassis_group_index_find(sbrec_ha_chassis_grp_by_name,
-- 
2.20.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] ovn: Properly set the index for chassis lookup

2019-05-23 Thread Han Zhou
On Thu, May 9, 2019 at 1:09 AM Dumitru Ceara  wrote:
>
> The chassis_lookup_by_name function now calls
> sbrec_chassis_index_set_name instead of sbrec_chassis_set_name. Due to
> the use of the wrong API memory was leaked every time a chassis was
> looked up by name. This was mostly visible when chassis lookups had to
> be done continuously (e.g., when two chassis were misconfigured
> with the same system-id).
>
> Reported-at: https://bugzilla.redhat.com/1698462
> Reported-by: Alexander 
> Signed-off-by: Dumitru Ceara 
> ---
>  ovn/lib/chassis-index.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ovn/lib/chassis-index.c b/ovn/lib/chassis-index.c
> index 34d4a31..423 100644
> --- a/ovn/lib/chassis-index.c
> +++ b/ovn/lib/chassis-index.c
> @@ -30,7 +30,7 @@ chassis_lookup_by_name(struct ovsdb_idl_index
*sbrec_chassis_by_name,
>  {
>  struct sbrec_chassis *target = sbrec_chassis_index_init_row(
>  sbrec_chassis_by_name);
> -sbrec_chassis_set_name(target, name);
> +sbrec_chassis_index_set_name(target, name);
>
>  struct sbrec_chassis *retval = sbrec_chassis_index_find(
>  sbrec_chassis_by_name, target);
> --
> 1.8.3.1
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Thanks Dumitru for fixing this. I see same wrong way of using index in the
same file:

struct sbrec_ha_chassis_group *target =
sbrec_ha_chassis_group_index_init_row(sbrec_ha_chassis_grp_by_name);
sbrec_ha_chassis_group_set_name(target, name);

May it be fixed together?
I did check all the other files under ovn, and it seems these are the only
places with this problem.

Acked-by: Han Zhou 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] ovn: Properly set the index for chassis lookup

2019-05-22 Thread Dumitru Ceara
On Fri, May 10, 2019 at 10:58 AM Numan Siddique  wrote:
>
>
>
> On Thu, May 9, 2019 at 1:40 PM Dumitru Ceara  wrote:
>>
>> The chassis_lookup_by_name function now calls
>> sbrec_chassis_index_set_name instead of sbrec_chassis_set_name. Due to
>> the use of the wrong API memory was leaked every time a chassis was
>> looked up by name. This was mostly visible when chassis lookups had to
>> be done continuously (e.g., when two chassis were misconfigured
>> with the same system-id).
>>
>> Reported-at: https://bugzilla.redhat.com/1698462
>> Reported-by: Alexander 
>> Signed-off-by: Dumitru Ceara 
>> ---
>
>
> Acked-by: Numan Siddique 
>

Hi,

Does this patch need more reviews in order to get committed?

Thanks,
Dumitru


>>
>>  ovn/lib/chassis-index.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/ovn/lib/chassis-index.c b/ovn/lib/chassis-index.c
>> index 34d4a31..423 100644
>> --- a/ovn/lib/chassis-index.c
>> +++ b/ovn/lib/chassis-index.c
>> @@ -30,7 +30,7 @@ chassis_lookup_by_name(struct ovsdb_idl_index 
>> *sbrec_chassis_by_name,
>>  {
>>  struct sbrec_chassis *target = sbrec_chassis_index_init_row(
>>  sbrec_chassis_by_name);
>> -sbrec_chassis_set_name(target, name);
>> +sbrec_chassis_index_set_name(target, name);
>>
>>  struct sbrec_chassis *retval = sbrec_chassis_index_find(
>>  sbrec_chassis_by_name, target);
>> --
>> 1.8.3.1
>>
>> ___
>> dev mailing list
>> d...@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] ovn: Properly set the index for chassis lookup

2019-05-10 Thread Numan Siddique
On Thu, May 9, 2019 at 1:40 PM Dumitru Ceara  wrote:

> The chassis_lookup_by_name function now calls
> sbrec_chassis_index_set_name instead of sbrec_chassis_set_name. Due to
> the use of the wrong API memory was leaked every time a chassis was
> looked up by name. This was mostly visible when chassis lookups had to
> be done continuously (e.g., when two chassis were misconfigured
> with the same system-id).
>
> Reported-at: https://bugzilla.redhat.com/1698462
> Reported-by: Alexander 
> Signed-off-by: Dumitru Ceara 
> ---
>

Acked-by: Numan Siddique 


>  ovn/lib/chassis-index.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ovn/lib/chassis-index.c b/ovn/lib/chassis-index.c
> index 34d4a31..423 100644
> --- a/ovn/lib/chassis-index.c
> +++ b/ovn/lib/chassis-index.c
> @@ -30,7 +30,7 @@ chassis_lookup_by_name(struct ovsdb_idl_index
> *sbrec_chassis_by_name,
>  {
>  struct sbrec_chassis *target = sbrec_chassis_index_init_row(
>  sbrec_chassis_by_name);
> -sbrec_chassis_set_name(target, name);
> +sbrec_chassis_index_set_name(target, name);
>
>  struct sbrec_chassis *retval = sbrec_chassis_index_find(
>  sbrec_chassis_by_name, target);
> --
> 1.8.3.1
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] ovn: Properly set the index for chassis lookup

2019-05-09 Thread Dumitru Ceara
The chassis_lookup_by_name function now calls
sbrec_chassis_index_set_name instead of sbrec_chassis_set_name. Due to
the use of the wrong API memory was leaked every time a chassis was
looked up by name. This was mostly visible when chassis lookups had to
be done continuously (e.g., when two chassis were misconfigured
with the same system-id).

Reported-at: https://bugzilla.redhat.com/1698462
Reported-by: Alexander 
Signed-off-by: Dumitru Ceara 
---
 ovn/lib/chassis-index.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ovn/lib/chassis-index.c b/ovn/lib/chassis-index.c
index 34d4a31..423 100644
--- a/ovn/lib/chassis-index.c
+++ b/ovn/lib/chassis-index.c
@@ -30,7 +30,7 @@ chassis_lookup_by_name(struct ovsdb_idl_index 
*sbrec_chassis_by_name,
 {
 struct sbrec_chassis *target = sbrec_chassis_index_init_row(
 sbrec_chassis_by_name);
-sbrec_chassis_set_name(target, name);
+sbrec_chassis_index_set_name(target, name);
 
 struct sbrec_chassis *retval = sbrec_chassis_index_find(
 sbrec_chassis_by_name, target);
-- 
1.8.3.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev