New subject: Change in vdsm[master]: netinfo: _devinfo, _nicinfo, etc. to deal with Link objs

19 Nov 2013

Antoni Segura Puimedon has uploaded a new change for review.
Change subject: netinfo: improvements from using Link devices
......................................................................
netinfo: improvements from using Link devices
Make use of the new linkwrapper features to simplify netinfo code.
Change-Id: I7a51c0d3bcec1358521846238e0cbd4c13ae1d17
Signed-off-by: Antoni S. Puimedon asegurap@redhat.com
---
M lib/vdsm/ipwrapper.py
M lib/vdsm/netinfo.py
M tests/netinfoTests.py
3 files changed, 79 insertions(+), 115 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/08/21408/1

diff --git a/lib/vdsm/ipwrapper.py b/lib/vdsm/ipwrapper.py
index ebfda41..69be2aa 100644
--- a/lib/vdsm/ipwrapper.py
+++ b/lib/vdsm/ipwrapper.py
@@ -37,6 +37,7 @@
                               '/sbin/ethtool',  # EL6, ubuntu and Debian
                               '/usr/bin/ethtool',  # Arch
                               )
+NET_PATH = '/sys/class/net'
def _isValid(ip, verifier):
@@ -74,6 +75,7 @@
 class Link(object):
     """Represents link information obtained from iproute2"""
     _fakeNics = config.get('vars', 'fake_nics').split(',')
+    _hiddenBonds = config.get('vars', 'hidden_bonds').split(',')
     _hiddenNics = config.get('vars', 'hidden_nics').split(',')
     _hiddenVlans = config.get('vars', 'hidden_vlans').split(',')
@@ -149,7 +151,7 @@
         detectedType = None
         rc, out, _ = execCmd([_ETHTOOL_BINARY.cmd, '-i', name])
         if rc == 71:  # Unkown driver, usually dummy or loopback
-            if not os.path.exists('/sys/class/net/' + name):
+            if not os.path.exists(os.path.join(NET_PATH, name)):
                 raise ValueError('Device %s does not exist' % name)
             if name == 'lo':
                 detectedType = LinkType.LOOPBACK
@@ -178,6 +180,12 @@
             detectedType = LinkType.NIC
         return detectedType
+    def isBOND(self):
+        return self.type == LinkType.BOND
+
+    def isBRIDGE(self):
+        return self.type == LinkType.BRIDGE
+
     def isDUMMY(self):
         return self.type == LinkType.DUMMY
@@ -204,9 +212,16 @@
         if self.isVLAN():
             return anyFnmatch(self.name, self._hiddenVlans)
         elif self.isDUMMY() or self.isVETH() or self.isNIC():
-            return anyFnmatch(self.name, self._hiddenNics)
-        else:
-            raise NotImplementedError
+            return (anyFnmatch(self.name, self._hiddenNics) or
+                    (self.master and _bondExists(self.master) and
+                     anyFnmatch(self.master, self._hiddenBonds)))
+        elif self.isBOND():
+            return anyFnmatch(self.name, self._hiddenBonds)
+        return False
+
+
+def _bondExists(bondName):
+    return os.path.exists(os.path.join(NET_PATH, bondName, 'bonding'))
def getLinks():
diff --git a/lib/vdsm/netinfo.py b/lib/vdsm/netinfo.py
index dd54a96..39b2d84 100644
--- a/lib/vdsm/netinfo.py
+++ b/lib/vdsm/netinfo.py
@@ -34,6 +34,7 @@
 from .config import config
 from . import constants
 from .ipwrapper import getLink, getLinks
+from .ipwrapper import NET_PATH
 from .ipwrapper import Route
 from .ipwrapper import routeShowAllDefaultGateways
 from . import libvirtconnection
@@ -49,7 +50,6 @@
NET_CONF_PREF = NET_CONF_DIR + 'ifcfg-'
 PROC_NET_VLAN = '/proc/net/vlan/'
-NET_PATH = '/sys/class/net'
 BONDING_MASTERS = '/sys/class/net/bonding_masters'
 BONDING_SLAVES = '/sys/class/net/%s/bonding/slaves'
 BONDING_OPT = '/sys/class/net/%s/bonding/%s'
@@ -67,42 +67,11 @@
 OPERSTATE_UP = 'up'
-def _nic_devices():
-    """
-    Returns an iterable of nic devices real or fakes, available on the host.
-    """
-    deviceLinks = getLinks()
-    for device in deviceLinks:
-        if device.isNIC() or device.isFakeNIC():
-            yield device
-
-
 def nics():
-    """
-    Returns a list of nics devices available to be used by vdsm.
-    The list of nics is built by filtering out nics defined
-    as hidden, fake or hidden bonds (with related nics'slaves).
-    """
-    def isEnslavedByHiddenBond(device):
-        """
-        Returns boolean stating if a nic device is enslaved to an hidden bond.
-        """
-        hidden_bonds = config.get('vars', 'hidden_bonds').split(',')
-        valid_bonds_fn = (bond_fn for bond_fn in hidden_bonds if bond_fn)
-
-        for bond_fn in valid_bonds_fn:
-            for bond_path in iglob(NET_PATH + '/' + bond_fn):
-                bond = os.path.basename(bond_path)
-                if device in slaves(bond):
-                    return True
-        return False
-
-    def isManagedByVdsm(device):
-        """Returns boolean stating if a device should be managed by vdsm."""
-        return (not device.isHidden() and
-                not isEnslavedByHiddenBond(device.name))
-
-    res = [dev.name for dev in _nic_devices() if isManagedByVdsm(dev)]
+    """Returns a list of nics and fake nics devices available (not hidden) to
+    be used by vdsm."""
+    res = [dev.name for dev in getLinks() if
+           (dev.isNIC() or dev.isFakeNIC) and not dev.isHidden()]
     return res
@@ -514,64 +483,72 @@
def _bridgeinfo(bridge, gateways, ipv6routes):
     info = _devinfo(bridge)
-    info.update({'gateway': getgateway(gateways, bridge),
-                'ipv6gateway': ipv6routes.get(bridge, '::'),
-                'ports': ports(bridge), 'stp': bridge_stp_state(bridge)})
-    return (bridge, info)
+    info.update({'gateway': getgateway(gateways, bridge.name),
+                'ipv6gateway': ipv6routes.get(bridge.name, '::'),
+                'ports': ports(bridge.name),
+                'stp': bridge_stp_state(bridge.name)})
+    return info
def _nicinfo(nic, paddr):
     info = _devinfo(nic)
-    info.update({'hwaddr': gethwaddr(nic), 'speed': speed(nic)})
-    if paddr.get(nic):
-        info['permhwaddr'] = paddr[nic]
-    return (nic, info)
+    info.update({'hwaddr': nic.address, 'speed': speed(nic.name)})
+    if nic.name in paddr:
+        info['permhwaddr'] = paddr[nic.name]
+    return info
def _bondinfo(bond):
     info = _devinfo(bond)
-    info.update({'hwaddr': gethwaddr(bond), 'slaves': slaves(bond)})
-    return (bond, info)
+    info.update({'hwaddr': bond.address, 'slaves': slaves(bond.name)})
+    return info
def _vlaninfo(vlan):
     info = _devinfo(vlan)
-    info.update({'iface': getVlanDevice(vlan), 'vlanid': getVlanID(vlan)})
-    return (vlan, info)
+    info.update({'iface': vlan.device, 'vlanid': vlan.vlanid})
+    return info
def _devinfo(dev):
-    ipv4addr, ipv4netmask, ipv6addrs = getIpInfo(dev)
+    ipv4addr, ipv4netmask, ipv6addrs = getIpInfo(dev.name)
     return {'addr': ipv4addr,
-            'cfg': getIfaceCfg(dev),
+            'cfg': getIfaceCfg(dev.name),
             'ipv6addrs': ipv6addrs,
-            'mtu': str(getMtu(dev)),
+            'mtu': dev.mtu,
             'netmask': ipv4netmask}
def get():
-    d = {}
+    devInfo = {'networks': {}, 'bridges': {}, 'nics': {}, 'bondings': {},
+               'vlans': {}}
     gateways = getRoutes()
     ipv6routes = getIPv6Routes()
-    paddr = permAddr()
-    d['networks'] = {}
for net, netAttr in networks().iteritems():
         try:
-            d['networks'][net] = _getNetInfo(netAttr.get('iface', net),
-                                             netAttr['bridged'], gateways,
-                                             ipv6routes,
-                                             netAttr.get('qosInbound'),
-                                             netAttr.get('qosOutbound'))
+            devInfo['networks'][net] = _getNetInfo(netAttr.get('iface', net),
+                                                   netAttr['bridged'],
+                                                   gateways, ipv6routes,
+                                                   netAttr.get('qosInbound'),
+                                                   netAttr.get('qosOutbound'))
         except KeyError:
             continue  # Do not report missing libvirt networks.
-    d['bridges'] = dict([_bridgeinfo(bridge, gateways, ipv6routes)
-                         for bridge in bridges()])
-    d['nics'] = dict([_nicinfo(nic, paddr) for nic in nics()])
-    d['bondings'] = dict([_bondinfo(bond) for bond in bondings()])
-    d['vlans'] = dict([_vlaninfo(vlan) for vlan in vlans()])
-    return d
+    paddr = permAddr()
+    for device in getLinks():
+        if device.isHidden():
+            continue
+        if device.isBRIDGE():
+            devInfo['bridges'][device.name] = _bridgeinfo(device, gateways,
+                                                          ipv6routes)
+        elif device.isBOND():
+            devInfo['bondings'][device.name] = _bondinfo(device)
+        elif device.isVLAN():
+            devInfo['vlans'][device.name] = _vlaninfo(device)
+        elif device.isNIC() or device.isFakeNIC():
+            devInfo['nics'][device.name] = _nicinfo(device, paddr)
+    return devInfo
def isVlanned(dev):
diff --git a/tests/netinfoTests.py b/tests/netinfoTests.py
index a4e5cb8..f5b81de 100644
--- a/tests/netinfoTests.py
+++ b/tests/netinfoTests.py
@@ -116,7 +116,8 @@
     def _testNics(self):
         """Creates a test fixture so that nics() reports:
         physical nics: em, me, me0, me1, hid0 and hideous
-        dummies: fake and fake0"""
+        dummies: fake and fake0
+        bonds: jbond (over me0 and me1)"""
         lines = ('2: em: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc '
                  'pfifo_fast state UP mode DEFAULT group default qlen 1000\  '
                  '  link/ether f0:de:f1:da:aa:e7 brd ff:ff:ff:ff:ff:ff '
@@ -134,13 +135,13 @@
                  '   link/ether ff:de:11:da:aa:e7 brd ff:ff:ff:ff:ff:ff '
                  'promiscuity 0 \    nic ',
                  '6: me0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc '
-                 'pfifo_fast state UP mode DEFAULT group default qlen 1000\  '
-                 '  link/ether 66:de:f1:da:aa:e7 brd ff:ff:ff:ff:ff:ff '
-                 'promiscuity 0 \    nic ',
+                 'pfifo_fast master jbond state UP mode DEFAULT group default '
+                 'qlen 1000\    link/ether 66:de:f1:da:aa:e7 brd '
+                 'ff:ff:ff:ff:ff:ff promiscuity 0 \    nic ',
                  '7: me1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc '
-                 'pfifo_fast state UP mode DEFAULT group default qlen 1000\  '
-                 '  link/ether 77:de:f1:da:aa:e7 brd ff:ff:ff:ff:ff:ff '
-                 'promiscuity 0 \    nic ',
+                 'pfifo_fast master jbond state UP mode DEFAULT group default '
+                 'qlen 1000\    link/ether 66:de:f1:da:aa:e7 brd '
+                 'ff:ff:ff:ff:ff:ff promiscuity 0 \    nic ',
                  '34: fake0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc '
                  'pfifo_fast state UP mode DEFAULT group default qlen 1000\  '
                  '  link/ether ff:aa:f1:da:aa:e7 brd ff:ff:ff:ff:ff:ff '
@@ -148,57 +149,28 @@
                  '35: fake: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc '
                  'pfifo_fast state UP mode DEFAULT group default qlen 1000\  '
                  '  link/ether ff:aa:f1:da:bb:e7 brd ff:ff:ff:ff:ff:ff '
-                 'promiscuity 0  \    dummy ')
+                 'promiscuity 0  \    dummy ',
+                 '419: jbond: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> mtu '
+                 '1500 qdisc noqueue state UP mode DEFAULT group default \    '
+                 'link/ether 66:de:f1:da:aa:e7 brd ff:ff:ff:ff:ff:ff '
+                 'promiscuity 1 \    bond')
         return [ipwrapper.Link.fromText(line) for line in lines]
-    def _dev_dirs_setup(self, dir_fixture):
+    def testNics(self):
         """
-        Creates test fixture so that the nics created by _testNics are reported
-        as:
         managed by vdsm: em, me, fake0, fake1
         not managed due to hidden bond (jbond) enslavement: me0, me1
         not managed due to being hidden nics: hid0, hideous
-
-        returns related containing dir.
         """
-        bonding_path = os.path.join(dir_fixture, 'jbond/bonding')
-        os.makedirs(bonding_path)
-        with open(os.path.join(bonding_path, 'slaves'), 'w') as f:
-            f.write('me0 me1')
-
-        return dir_fixture
-
-    def _config_setup(self):
-        """
-        Returns an instance of a config stub.
-        With patterns:
-            * hid* for hidden nics.
-            * fake* for fake nics.
-            * jb* for hidden bonds.
-        """
-        class Config(object):
-            def get(self, unused_vars, key):
-                if key == 'hidden_nics':
-                    return 'hid*'
-                elif key == 'fake_nics':
-                    return 'fake*'
-                else:
-                    return 'jb*'
-
-        return Config()
-
-    def testNics(self):
         temp_dir = tempfile.mkdtemp()
-        with MonkeyPatchScope([(netinfo, 'BONDING_SLAVES',
-                                temp_dir + '/%s/bonding/slaves'),
-                               (netinfo, 'getLinks',
+        with MonkeyPatchScope([(netinfo, 'getLinks',
                                 self._testNics),
-                               (netinfo, 'NET_PATH',
-                                self._dev_dirs_setup(temp_dir)),
+                               (ipwrapper, '_bondExists',
+                                lambda x: x == 'jbond'),
                                (ipwrapper.Link, '_detectType',
                                 partial(_fakeTypeDetection, ipwrapper.Link)),
-                               (netinfo, 'config', self._config_setup()),
                                (ipwrapper.Link, '_fakeNics', ['fake*']),
+                               (ipwrapper.Link, '_hiddenBonds', ['jb*']),
                                (ipwrapper.Link, '_hiddenNics', ['hid*'])
                                ]):
             try:
-- 
To view, visit http://gerrit.ovirt.org/21408
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7a51c0d3bcec1358521846238e0cbd4c13ae1d17
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com

    

Change in vdsm[master]: netinfo: improvements from using Link devices