Mark Wu has posted comments on this change.
Change subject: netinfo: get an interface using the ip associated with it
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(3 inline comments)
....................................................
File vdsm/BindingXMLRPC.py
Line 88: last = self.server.lastClient
Line 89: lastserver = self.server.lastServerIP
Line 90: return {'management_ip': self.serverIP,
Line 91: 'lastClient': last,
Line 92: 'lastClientIface': getIfaceByIP(lastserver)}
I agree with that using a mapping of interface and ip address cause less
overhead than parsing the routing table. But I don't understand why we need log
the 'lastClientIface'. Even if the network packet isn't transported by the
interface which has the server ip address because of routing table, does vdsm
need care about that?
Line 93:
Line 94: def _getKeyCertFilenames(self):
Line 95: """
Line 96: Get the locations of key and certificate files.
Line 113: def log_request(self, code='-', size='-'):
Line 114: """Track from where client connections are coming."""
Line 115: self.server.lastClient = self.client_address[0]
Line 116: self.server.lastClientTime = time.time()
Line 117: self.server.lastServerIP =
self.server.socket.getsockname()[0]
why do you need call getsockname() to get server ip address? Isn't it kept as
self.serverIP?
Line 118: # FIXME: The editNetwork API uses this log file to
Line 119: # determine if this host is still accessible. We use
a
Line 120: # file (rather than an event) because editNetwork is
Line 121: # performed by a separate, root process. To clean
this
....................................................
File vdsm/netinfo.py
Line 433: def getIfaceByIP(ip):
Line 434: for iface in ethtool.get_active_devices():
Line 435: ipaddrs = [
Line 436: a.address for a in
Line 437:
ethtool.get_interfaces_info(iface)[0].get_ipv6_addresses()]
I suggest you extract the line above into a separate function, like getaddr6.
Then it could be reused for other cases.
Line 438: ipaddrs.append(getaddr(iface))
Line 439: if ip in ipaddrs:
Line 440: return iface
Line 441: return ''
--
To view, visit http://gerrit.ovirt.org/11519
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I239f92c527524e5651f51712841c3eafcf659e53
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Hunt Xu <[email protected]>
Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Igor Lvovsky <[email protected]>
Gerrit-Reviewer: Livnat Peer <[email protected]>
Gerrit-Reviewer: Mark Wu <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
