Dan Kenigsberg has posted comments on this change.

Change subject: ssl: ssl_accept blocks after reboot
......................................................................


Patch Set 2: Code-Review-1

(2 comments)

http://gerrit.ovirt.org/#/c/33611/2/tests/sslTests.py
File tests/sslTests.py:

Line 109: 
Line 110: class SocketTests(TestCaseBase):
Line 111: 
Line 112:     def test_block_socket(self):
Line 113:         # this test make sure that we won't block during
I don't understand how this test works :-(

You start a server, open a TCP connection to it, but you don't check that the 
server is still responsive.

Would this test really fail on master branch (before your fix)?

I think that you should:
* start a server
* open a DoS connection to it
* open a legitimate client to the server
* if the client is not served within 10 seconds, fail the test.
Line 114:         # accept. It will hang the build if accept blocks
Line 115:         server = TestServer()
Line 116:         server.start()
Line 117:         try:


Line 119:             sock.settimeout(1)
Line 120:             sock.connect((HOST, server.port))
Line 121:         finally:
Line 122:             server.stop()
Line 123:             sock.close()
puritan: at this point, "sock" may have not been defined. better use

  with closing(socket.socket(socket.AF_INET, socket.SOCK_STREAM)) as sock:
Line 124: 
Line 125: 
Line 126: class VerifyingTransportTests(TestCaseBase):
Line 127: 


-- 
To view, visit http://gerrit.ovirt.org/33611
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I759436b5bfb6c2334d253d12806258cbe1c3720f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczew...@gmail.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Martin PeĊ™ina <mper...@redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczew...@gmail.com>
Gerrit-Reviewer: Saggi Mizrahi <smizr...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to