Change in vdsm[master]: netinfo: get an interface using the ip associated with it

wudxw at linux.vnet.ibm.com wudxw at linux.vnet.ibm.com
Mon Feb 4 02:22:08 UTC 2013


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 <mhuntxu at gmail.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky at redhat.com>
Gerrit-Reviewer: Livnat Peer <lpeer at redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw at linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list