Change in vdsm[master]: jsonrpc: make sure not to block when processing i/o

nsoffer at redhat.com nsoffer at redhat.com
Fri Apr 8 15:47:31 UTC 2016


Nir Soffer has posted comments on this change.

Change subject: jsonrpc: make sure not to block when processing i/o
......................................................................


Patch Set 1: Code-Review-1

(6 comments)

https://gerrit.ovirt.org/#/c/55872/1/lib/vdsm/protocoldetector.py
File lib/vdsm/protocoldetector.py:

Line 120
Line 121
Line 122
Line 123
Line 124
We cannot do this. The server handling this socket is set the socket. Protocol detector should only detect the protocol and pass the socket to the next layer.


Line 37:     server_socket = socket.socket(family, socktype, proto)
Line 38:     filecontrol.set_close_on_exec(server_socket.fileno())
Line 39:     server_socket.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
Line 40:     server_socket.bind(sockaddr)
Line 41:     server_socket.setblocking(0)
This is not needed, since asyncore dispatcher will make the socket non blocking when you create a dispatcher with the socket.
Line 42: 
Line 43:     return server_socket
Line 44: 
Line 45: 


https://gerrit.ovirt.org/#/c/55872/1/lib/vdsm/utils.py
File lib/vdsm/utils.py:

Line 849: 
Line 850: def create_connected_socket(host, port, sslctx=None, timeout=None):
Line 851:     addrinfo = socket.getaddrinfo(host, port,
Line 852:                                   socket.AF_UNSPEC, socket.SOCK_STREAM)
Line 853:     family, socktype, proto, _, _ = addrinfo[0]
Not related
Line 854:     sock = socket.socket(family, socktype, proto)
Line 855: 
Line 856:     if sslctx:
Line 857:         sock = sslctx.wrapSocket(sock)


Line 857:         sock = sslctx.wrapSocket(sock)
Line 858: 
Line 859:     sock.settimeout(timeout)
Line 860:     sock.connect((host, port))
Line 861:     sock.setblocking(0)
This is too late, we already blocked during connect.
Line 862:     return sock
Line 863: 
Line 864: 
Line 865: @contextmanager


https://gerrit.ovirt.org/#/c/55872/1/lib/yajsonrpc/stompreactor.py
File lib/yajsonrpc/stompreactor.py:

Line 278
Line 279
Line 280
Line 281
Line 282
+1


Line 373
Line 374
Line 375
Line 376
Line 377
+1


-- 
To view, visit https://gerrit.ovirt.org/55872
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8dee39c7ec518721e809de1cb542173161612ffb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list