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 19:16:32 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:

(2 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 are setting it later to non-blocking so I do not see why we can't have i
When passing the socket to the xmlrpc server, it must be in blocking mode.


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

Line 857:         sock = sslctx.wrapSocket(sock)
Line 858: 
Line 859:     sock.settimeout(timeout)
Line 860:     sock.connect((host, port))
Line 861:     sock.setblocking(0)
> We could have it earlier but that would mean we need to abandon connected s
If the client side it not running in the reactor thread, then it should be harmless to block in connect.

Anyway, since the client is using asyncore, and it take care of this, we dont' have to do this here. It will also break any other code that is not using asyncore and expecting that a new socket is in blocking mode.
Line 862:     return sock
Line 863: 
Line 864: 
Line 865: @contextmanager


-- 
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