Re: [ovs-dev] [PATCH] tests-windows: Fix SSL ovsdb test which is hanging

2018-03-07 Thread aserdean


> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Ben Pfaff
> Trimis: Thursday, March 8, 2018 3:07 AM
> Către: aserd...@ovn.org
> Cc: d...@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH] tests-windows: Fix SSL ovsdb test which is
> hanging
> 
> On Thu, Mar 08, 2018 at 02:22:45AM +0200, aserd...@ovn.org wrote:
> > > -Mesaj original-
> > > De la: ovs-dev-boun...@openvswitch.org  > > boun...@openvswitch.org> În numele Ben Pfaff
> > > Trimis: Thursday, March 8, 2018 2:11 AM
> > > Către: aserd...@ovn.org
> > > Cc: d...@openvswitch.org
> > > Subiect: Re: [ovs-dev] [PATCH] tests-windows: Fix SSL ovsdb test
> > > which is hanging
> > >
> > > On Thu, Mar 08, 2018 at 01:45:51AM +0200, aserd...@ovn.org wrote:
> > > > > -Mesaj original-
> > > > > De la: ovs-dev-boun...@openvswitch.org  > > > > boun...@openvswitch.org> În numele Ben Pfaff
> > > > > Trimis: Wednesday, March 7, 2018 11:20 PM
> > > > > Către: Alin Gabriel Serdean 
> > > > > Cc: d...@openvswitch.org
> > > > > Subiect: Re: [ovs-dev] [PATCH] tests-windows: Fix SSL ovsdb test
> > > > > which is hanging
> > > > >
> > > > > On Tue, Mar 06, 2018 at 01:17:12PM +0200, Alin Gabriel Serdean
> wrote:
> > > > > > The test:
> > > > > > `1827. ovsdb-server.at:490: testing SSL db: implementation
> > > > > > ...` is hanging on Windows because the returned in the case
> > > > > > the client failed to connect is "Unknown error" vs the normal
> "Protocol error".
> > > > > >
> > > > > > Update the test to accommodate for this.
> > > > > >
> > > > > > Signed-off-by: Alin Gabriel Serdean 
> > > > >
> > > > > Thanks for finding and fixing the error.
> > > > >
> > > > > I think that there is an underlying problem here: the code is
> > > > > bad at error handling in general.  It's supposed to kill the
> > > > > ovsdb-server if anything fails, but it didn't do it right: it
> > > > > killed $(cat pid) but the pidfile is actually in ovsdb-
> > > > > server.pid.  This meant that the issue showed up as a hang instead of
> a test failure, which seems bad.
> > > > >
> > > > > Also, I'm kind of inclined to just disregard the particular
> > > > > error message.  It might save time later.  (I guess there's a
> > > > > risk that the connection fails happens for some other reason
> > > > > that should fail the test
> > > > > though.)
> > > > >
> > > > > Anyway, I sent a pair of patches that implement my suggestions:
> > > > > https://patchwork.ozlabs.org/patch/882796/
> > > > > https://patchwork.ozlabs.org/patch/882798/
> > > > >
> > > > > What are your thoughts?
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Ben.
> > > > [Alin Serdean] I'm dropping this change and acking yours.
> > > > They are much cleaner and better 😊.
> > >
> > > Thanks!  I applied both of these fixes to master.
> > >
> > > Do you want me to backport them to branch-2.9 (or earlier)?  I have
> > > not done that yet.
> >
> > branch-2.9 also please.
> >
> 
> Done.
Thank you!

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


Re: [ovs-dev] [PATCH] tests-windows: Fix SSL ovsdb test which is hanging

2018-03-07 Thread Ben Pfaff
On Thu, Mar 08, 2018 at 02:22:45AM +0200, aserd...@ovn.org wrote:
> > -Mesaj original-
> > De la: ovs-dev-boun...@openvswitch.org  > boun...@openvswitch.org> În numele Ben Pfaff
> > Trimis: Thursday, March 8, 2018 2:11 AM
> > Către: aserd...@ovn.org
> > Cc: d...@openvswitch.org
> > Subiect: Re: [ovs-dev] [PATCH] tests-windows: Fix SSL ovsdb test which is
> > hanging
> > 
> > On Thu, Mar 08, 2018 at 01:45:51AM +0200, aserd...@ovn.org wrote:
> > > > -Mesaj original-
> > > > De la: ovs-dev-boun...@openvswitch.org  > > > boun...@openvswitch.org> În numele Ben Pfaff
> > > > Trimis: Wednesday, March 7, 2018 11:20 PM
> > > > Către: Alin Gabriel Serdean 
> > > > Cc: d...@openvswitch.org
> > > > Subiect: Re: [ovs-dev] [PATCH] tests-windows: Fix SSL ovsdb test
> > > > which is hanging
> > > >
> > > > On Tue, Mar 06, 2018 at 01:17:12PM +0200, Alin Gabriel Serdean wrote:
> > > > > The test:
> > > > > `1827. ovsdb-server.at:490: testing SSL db: implementation ...` is
> > > > > hanging on Windows because the returned in the case the client
> > > > > failed to connect is "Unknown error" vs the normal "Protocol error".
> > > > >
> > > > > Update the test to accommodate for this.
> > > > >
> > > > > Signed-off-by: Alin Gabriel Serdean 
> > > >
> > > > Thanks for finding and fixing the error.
> > > >
> > > > I think that there is an underlying problem here: the code is bad at
> > > > error handling in general.  It's supposed to kill the ovsdb-server
> > > > if anything fails, but it didn't do it right: it killed $(cat pid)
> > > > but the pidfile is actually in ovsdb- server.pid.  This meant that
> > > > the issue showed up as a hang instead of a test failure, which seems 
> > > > bad.
> > > >
> > > > Also, I'm kind of inclined to just disregard the particular error
> > > > message.  It might save time later.  (I guess there's a risk that
> > > > the connection fails happens for some other reason that should fail
> > > > the test
> > > > though.)
> > > >
> > > > Anyway, I sent a pair of patches that implement my suggestions:
> > > > https://patchwork.ozlabs.org/patch/882796/
> > > > https://patchwork.ozlabs.org/patch/882798/
> > > >
> > > > What are your thoughts?
> > > >
> > > > Thanks,
> > > >
> > > > Ben.
> > > [Alin Serdean] I'm dropping this change and acking yours.
> > > They are much cleaner and better 😊.
> > 
> > Thanks!  I applied both of these fixes to master.
> > 
> > Do you want me to backport them to branch-2.9 (or earlier)?  I have not done
> > that yet.
> 
> branch-2.9 also please.
> 

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


Re: [ovs-dev] [PATCH] tests-windows: Fix SSL ovsdb test which is hanging

2018-03-07 Thread aserdean
> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Ben Pfaff
> Trimis: Thursday, March 8, 2018 2:11 AM
> Către: aserd...@ovn.org
> Cc: d...@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH] tests-windows: Fix SSL ovsdb test which is
> hanging
> 
> On Thu, Mar 08, 2018 at 01:45:51AM +0200, aserd...@ovn.org wrote:
> > > -Mesaj original-
> > > De la: ovs-dev-boun...@openvswitch.org  > > boun...@openvswitch.org> În numele Ben Pfaff
> > > Trimis: Wednesday, March 7, 2018 11:20 PM
> > > Către: Alin Gabriel Serdean 
> > > Cc: d...@openvswitch.org
> > > Subiect: Re: [ovs-dev] [PATCH] tests-windows: Fix SSL ovsdb test
> > > which is hanging
> > >
> > > On Tue, Mar 06, 2018 at 01:17:12PM +0200, Alin Gabriel Serdean wrote:
> > > > The test:
> > > > `1827. ovsdb-server.at:490: testing SSL db: implementation ...` is
> > > > hanging on Windows because the returned in the case the client
> > > > failed to connect is "Unknown error" vs the normal "Protocol error".
> > > >
> > > > Update the test to accommodate for this.
> > > >
> > > > Signed-off-by: Alin Gabriel Serdean 
> > >
> > > Thanks for finding and fixing the error.
> > >
> > > I think that there is an underlying problem here: the code is bad at
> > > error handling in general.  It's supposed to kill the ovsdb-server
> > > if anything fails, but it didn't do it right: it killed $(cat pid)
> > > but the pidfile is actually in ovsdb- server.pid.  This meant that
> > > the issue showed up as a hang instead of a test failure, which seems bad.
> > >
> > > Also, I'm kind of inclined to just disregard the particular error
> > > message.  It might save time later.  (I guess there's a risk that
> > > the connection fails happens for some other reason that should fail
> > > the test
> > > though.)
> > >
> > > Anyway, I sent a pair of patches that implement my suggestions:
> > > https://patchwork.ozlabs.org/patch/882796/
> > > https://patchwork.ozlabs.org/patch/882798/
> > >
> > > What are your thoughts?
> > >
> > > Thanks,
> > >
> > > Ben.
> > [Alin Serdean] I'm dropping this change and acking yours.
> > They are much cleaner and better 😊.
> 
> Thanks!  I applied both of these fixes to master.
> 
> Do you want me to backport them to branch-2.9 (or earlier)?  I have not done
> that yet.

branch-2.9 also please.

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


Re: [ovs-dev] [PATCH] tests-windows: Fix SSL ovsdb test which is hanging

2018-03-07 Thread Ben Pfaff
On Thu, Mar 08, 2018 at 01:45:51AM +0200, aserd...@ovn.org wrote:
> > -Mesaj original-
> > De la: ovs-dev-boun...@openvswitch.org  > boun...@openvswitch.org> În numele Ben Pfaff
> > Trimis: Wednesday, March 7, 2018 11:20 PM
> > Către: Alin Gabriel Serdean 
> > Cc: d...@openvswitch.org
> > Subiect: Re: [ovs-dev] [PATCH] tests-windows: Fix SSL ovsdb test which is
> > hanging
> > 
> > On Tue, Mar 06, 2018 at 01:17:12PM +0200, Alin Gabriel Serdean wrote:
> > > The test:
> > > `1827. ovsdb-server.at:490: testing SSL db: implementation ...` is
> > > hanging on Windows because the returned in the case the client failed
> > > to connect is "Unknown error" vs the normal "Protocol error".
> > >
> > > Update the test to accommodate for this.
> > >
> > > Signed-off-by: Alin Gabriel Serdean 
> > 
> > Thanks for finding and fixing the error.
> > 
> > I think that there is an underlying problem here: the code is bad at error
> > handling in general.  It's supposed to kill the ovsdb-server if anything 
> > fails,
> > but it didn't do it right: it killed $(cat pid) but the pidfile is actually 
> > in ovsdb-
> > server.pid.  This meant that the issue showed up as a hang instead of a test
> > failure, which seems bad.
> > 
> > Also, I'm kind of inclined to just disregard the particular error message.  
> > It
> > might save time later.  (I guess there's a risk that the connection fails
> > happens for some other reason that should fail the test
> > though.)
> > 
> > Anyway, I sent a pair of patches that implement my suggestions:
> > https://patchwork.ozlabs.org/patch/882796/
> > https://patchwork.ozlabs.org/patch/882798/
> > 
> > What are your thoughts?
> > 
> > Thanks,
> > 
> > Ben.
> [Alin Serdean] I'm dropping this change and acking yours.
> They are much cleaner and better 😊.

Thanks!  I applied both of these fixes to master.

Do you want me to backport them to branch-2.9 (or earlier)?  I have not
done that yet.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] tests-windows: Fix SSL ovsdb test which is hanging

2018-03-07 Thread aserdean
> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Ben Pfaff
> Trimis: Wednesday, March 7, 2018 11:20 PM
> Către: Alin Gabriel Serdean 
> Cc: d...@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH] tests-windows: Fix SSL ovsdb test which is
> hanging
> 
> On Tue, Mar 06, 2018 at 01:17:12PM +0200, Alin Gabriel Serdean wrote:
> > The test:
> > `1827. ovsdb-server.at:490: testing SSL db: implementation ...` is
> > hanging on Windows because the returned in the case the client failed
> > to connect is "Unknown error" vs the normal "Protocol error".
> >
> > Update the test to accommodate for this.
> >
> > Signed-off-by: Alin Gabriel Serdean 
> 
> Thanks for finding and fixing the error.
> 
> I think that there is an underlying problem here: the code is bad at error
> handling in general.  It's supposed to kill the ovsdb-server if anything 
> fails,
> but it didn't do it right: it killed $(cat pid) but the pidfile is actually 
> in ovsdb-
> server.pid.  This meant that the issue showed up as a hang instead of a test
> failure, which seems bad.
> 
> Also, I'm kind of inclined to just disregard the particular error message.  It
> might save time later.  (I guess there's a risk that the connection fails
> happens for some other reason that should fail the test
> though.)
> 
> Anyway, I sent a pair of patches that implement my suggestions:
> https://patchwork.ozlabs.org/patch/882796/
> https://patchwork.ozlabs.org/patch/882798/
> 
> What are your thoughts?
> 
> Thanks,
> 
> Ben.
[Alin Serdean] I'm dropping this change and acking yours.
They are much cleaner and better 😊.

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


Re: [ovs-dev] [PATCH] tests-windows: Fix SSL ovsdb test which is hanging

2018-03-07 Thread Ben Pfaff
On Tue, Mar 06, 2018 at 01:17:12PM +0200, Alin Gabriel Serdean wrote:
> The test:
> `1827. ovsdb-server.at:490: testing SSL db: implementation ...`
> is hanging on Windows because the returned in the case the client failed to
> connect is "Unknown error" vs the normal "Protocol error".
> 
> Update the test to accommodate for this.
> 
> Signed-off-by: Alin Gabriel Serdean 

Thanks for finding and fixing the error.

I think that there is an underlying problem here: the code is bad at
error handling in general.  It's supposed to kill the ovsdb-server if
anything fails, but it didn't do it right: it killed $(cat pid) but the
pidfile is actually in ovsdb-server.pid.  This meant that the issue
showed up as a hang instead of a test failure, which seems bad.

Also, I'm kind of inclined to just disregard the particular error
message.  It might save time later.  (I guess there's a risk that the
connection fails happens for some other reason that should fail the test
though.)

Anyway, I sent a pair of patches that implement my suggestions:
https://patchwork.ozlabs.org/patch/882796/
https://patchwork.ozlabs.org/patch/882798/

What are your thoughts?

Thanks,

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