Nir Soffer has posted comments on this change. Change subject: json-rpc: Protocol detection ......................................................................
Patch Set 24: (1 comment) http://gerrit.ovirt.org/#/c/26300/24/vdsm/BindingXMLRPC.py File vdsm/BindingXMLRPC.py: Line 204: Line 205: server = utils.SimpleThreadedXMLRPCServer( Line 206: requestHandler=RequestHandler, Line 207: logRequests=True) Line 208: utils.closeOnExec(server.socket.fileno()) > I am not really sure what you mean here. We are using Python XMLRPCServer, which is baed on a base tcp server, creating a socket in its __init__. See /usr/lib64/python2.7/SocketServer.py line 413. We do not need this socket because we separated nicely the accept part from the request handling part. So our server should override __init__ to make sure we do not create a listening socket. Looking at the horrible Python code we are using , I think this should be deferred to another patch, and the correct solution is not inheriting from XMLRPCServer, but instead write 20 lines server that take request from a queue and process them in a new thread. The way it is implemented here is fragile and impossible to understand with so many level of inheritance through so many modules (this, utils, SimpleXMLRCPServer, BaseHTTPServer, SocketServer). Line 209: Line 210: return server Line 211: Line 212: def _registerFunctions(self): -- To view, visit http://gerrit.ovirt.org/26300 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id739a40e2b37dcc175137ec91cd5ec166ad24a75 Gerrit-PatchSet: 24 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski <piotr.kliczew...@gmail.com> Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczew...@gmail.com> Gerrit-Reviewer: Saggi Mizrahi <smizr...@redhat.com> Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: Yeela Kaplan <ykap...@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
