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