Change in vdsm[master]: netinfo: implement functions gathering ipv6 information
danken at redhat.com
danken at redhat.com
Thu Nov 22 07:32:12 UTC 2012
Dan Kenigsberg has posted comments on this change.
Change subject: netinfo: implement functions gathering ipv6 information
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(5 inline comments)
thanks! gathering ipv6 addresses is a first fine step for ipv6 support.
It would be more complex to configure ipv6...
....................................................
File vdsm/netinfo.py
Line 180: return bitmask_to_address(netmask)
Line 181:
Line 182:
Line 183: def getipv6addrs(dev):
Line 184: """Return a list of ipv6 addresses in the format of 'address/prefixlen'."""
I'm very weak when it comes to ipv6, but shouldn't we report the addess's scope, too?
Line 185: dev_info_list = ethtool.get_interfaces_info(dev.encode('utf8'))
Line 186: ipv6addrs = dev_info_list[0].get_ipv6_addresses()
Line 187: return ['/'.join([addr.address, str(addr.netmask)]) for addr in ipv6addrs]
Line 188:
Line 231:
Line 232: return '.'.join(ip_address)
Line 233:
Line 234:
Line 235: def ipv6StrToAddress(ipv6_str):
this seems a bit over-complex to me.
would you consider
socket.inet_ntop(socket.AF_INET6, struct.pack('QQ', *divmod(int(ipv5_str, 16), 2**64)))
instead (UNTESTED)?
would you add a unit test for this function?
Line 236: chr_list = [chr(int(ipv6_str[x:x + 2], 16)) for x in xrange(0, 32, 2)]
Line 237: return socket.inet_ntop(socket.AF_INET6, struct.pack('16c', *chr_list))
Line 238:
Line 239:
Line 252: ip_num = int(route_parm[2], 16)
Line 253: gateways[route_parm[0]] = intToAddress(ip_num)
Line 254:
Line 255: try:
Line 256: with open("/proc/net/ipv6_route") as route_file:
I see no reason why this new code should be bundled in the existing function.
Please move to a separate getRoutes6 (or a better name).
Line 257: for route_line in route_file.xreadlines():
Line 258: route_parm = route_line.rstrip().split(' ')
Line 259: dest = route_parm[0]
Line 260: prefix = route_parm[1]
Line 262: device = route_parm[-1]
Line 263: if dest == '0' * 32 and prefix == '00' and nexthop != '0' * 32:
Line 264: ipv6gateways[device] = ipv6StrToAddress(nexthop)
Line 265: except IOError as e:
Line 266: if e.errno == 2:
errno.ENOENT is more readable than "2".
Line 267: # ipv6 module not loaded
Line 268: pass
Line 269: else:
Line 270: raise e
Line 266: if e.errno == 2:
Line 267: # ipv6 module not loaded
Line 268: pass
Line 269: else:
Line 270: raise e
use a simple "raise" (no arg) to keep the original exception with its traceback.
Line 271:
Line 272: return gateways, ipv6gateways
Line 273:
Line 274:
--
To view, visit http://gerrit.ovirt.org/9381
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2318ffa7d71abe3f57cb5e480bc46565e29f3894
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: Mark Wu <wudxw at linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
More information about the vdsm-patches
mailing list