[Ovirt-devel] [PATCH] display ipv6 address in networking details page, also fix ipv6 netmask configurations

Alan Pevec apevec at gmail.com
Wed Aug 3 09:07:30 UTC 2011


subject is too long - 1. line should be 50 chars or less, put detailed
description in 3. and following lines
http://spheredev.org/wiki/Git_for_the_lazy#Writing_good_commit_messages

> -                self.BR_CONFIG += "set %s/IPV6ADDR %s\n" % (BR_ROOT, OVIRT_VARS["OVIRT_IPV6_ADDRESS"])
> +                self.BR_CONFIG += "set %s/IPV6ADDR %s/%s\n" % (BR_ROOT, OVIRT_VARS["OVIRT_IPV6_ADDRESS"], OVIRT_VARS["OVIRT_IPV6_NETMASK"])

While at it, this whole section could be made more readable by using
multi-line string:
map=OVIRT_VARS.copy()
map["BR_ROOT"]=BR_ROOT
self.BR_CONFIG = """
set %(BR_ROOT)s/IPV6ADDR %(OVIRT_IPV6_ADDRESS)s/%(OVIRT_IPV6_NETMASK)s
set %(BR_ROOT)s/IPV6_AUTOCONF no
set %(BR_ROOT)s/IPV6FORWARDING no
set %(BR_ROOT)s/IPV6_DEFAULTGW %(OVIRT_IPV6_GATEWAY)s
""" % map

>                 for key in sorted(self.network_status.iterkeys()):
>                     if key.startswith("br"):
>                         parent_dev = key[+2:]
> -                        del self.network_status[parent_dev]
> +                        try:
> +                            del self.network_status[parent_dev]
> +                        except:
> +                            pass

Why not network_status.pop(parent_dev) ?

>  def get_ipv6_address(interface):
> -    inet6_lookup_cmd = 'ifconfig ' + interface + ' |grep inet6|grep -v Global|awk {\'print $3\'}'
> +    inet6_lookup_cmd = 'ifconfig ' + interface + ' |grep inet6|grep Global|awk {\'print $3\'}'

Better use ip command, ifconfig is obsolete:
inet6_lookup_cmd = "ip addr show dev %s | awk '$1==\"inet6\" &&
$4==\"global\" { print $2 }'" % interface
BTW, a general note, when parsing command output you must ensure
locale but in case of ovirt/rhevh that's fixed in the image.
Best would be to replace it with python-only code, there's
http://pypi.python.org/pypi/netifaces - it's in F16 so we could have a
look at it.

Alan




More information about the ovirt-devel mailing list