Edward Haas has posted comments on this change.

Change subject: net: Enable VDSM to listen on IPv6 addresses
......................................................................


Patch Set 4:

(4 comments)

https://gerrit.ovirt.org/#/c/51319/4/lib/vdsm/config.py.in
File lib/vdsm/config.py.in:

Line 364: XMPRPC
> We do not need it. We are going to drop it soon so it is enough to say "vds
Done


Line 361:             'clients.'),
Line 362: 
Line 363:         ('management_ip', '::',
Line 364:             'IP address on which the vdsmd XMPRPC server listens to 
network '
Line 365:             'clients.'
> keep a trailing space inside the string, or the help message would look ugl
Done
Line 366:             'Set to "0.0.0.0" to listen only on IPv4.'
Line 367:             'Set to "::" to listen on both IPv4 & IPv6'),
Line 368: 
Line 369:         ('guests_gateway_ip', '', None),


https://gerrit.ovirt.org/#/c/51319/4/tests/protocoldetectorTests.py
File tests/protocoldetectorTests.py:

Line 227:         t.start()
Line 228: 
Line 229:     @contextmanager
Line 230:     def connect(self, use_ssl):
Line 231:         host, port = self.acceptor_address
> it looks like we could reuse utils.create_connected_socket() here.
Almost, the other one uses an SSL wrapper (SSLContext) while this one is 
simple, used for the test.
We should seperate between the socket creation, the ssl wrapping and the actual 
connection in order to safely reuse code here.

Anyway, I suggest doing so in a future patch.
Line 232:         addrinfo = socket.getaddrinfo(host, port,
Line 233:                                       socket.AF_UNSPEC, 
socket.SOCK_STREAM)
Line 234:         family, socktype, proto, _, sockaddr = addrinfo[0]
Line 235:         s = socket.socket(family, socktype, proto)


https://gerrit.ovirt.org/#/c/51319/4/vdsm/protocoldetector.py
File vdsm/protocoldetector.py:

Line 168:     ):
Line 169:         self._sslctx = sslctx
Line 170:         self._reactor = reactor
Line 171:         sock = _create_socket(host, port)
Line 172:         self._host, self._port = sock.getsockname()[0:2]
> in a future patch you can clean all usages of self._host and self._port sim
Adding a TODO
Line 173:         self.log.info("Listening at %s:%d", self._host, self._port)
Line 174:         self._acceptor = self._reactor.create_dispatcher(
Line 175:             sock, _AcceptorImpl(self.handle_accept))
Line 176:         self._acceptor.listen(5)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9c893d8f38f6abf183dcccbc2a5e328b492235e
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas <edwa...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Edward Haas <edwa...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda <osvob...@redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczew...@gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com>
Gerrit-Reviewer: Yevgeny Zaspitsky <yzasp...@redhat.com>
Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to