Change in vdsm[ovirt-3.5]: netinfo:nicSpeed(): fix nicSpeed condition

Petr Horáček has uploaded a new change for review.

Change subject: netinfo:nicSpeed(): fix nicSpeed condition
......................................................................

netinfo:nicSpeed(): fix nicSpeed condition

In `if s not in (2 ** 16 - 1, 2 ** 32 - 1) or s > 0` first part doesn't
make sense with OR. Changed to AND.

I changed conditions to make the function more readable and created unit
test, to be sure that function returns correct values.

Change-Id: I34cfca16909f9695441e26d3ddd508e7a4210c12
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1157224
Signed-off-by: Petr Horáček <phoracek(a)redhat.com&gt;
---
M lib/vdsm/netinfo.py
M tests/netinfoTests.py
2 files changed, 28 insertions(+), 13 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/01/34701/1

diff --git a/lib/vdsm/netinfo.py b/lib/vdsm/netinfo.py
index 174e5e4..74c9f6c 100644
--- a/lib/vdsm/netinfo.py
+++ b/lib/vdsm/netinfo.py
@@ -246,19 +246,17 @@
 
 
 def nicSpeed(nicName):
-    """Returns the nic speed if it is a legal value and nicName refers to
a
-    nic, 0 otherwise."""
+    """Returns the nic speed if it is a legal value, nicName refers to a
+    nic and nic is UP, 0 otherwise."""
     try:
-        # if the device is not up we must report 0
-        if operstate(nicName) != OPERSTATE_UP:
-            return 0
-        with open('/sys/class/net/%s/speed' % nicName) as speedFile:
-            s = int(speedFile.read())
-        # the device may have been disabled/downed after checking
-        # so we validate the return value as sysfs may return
-        # special values to indicate the device is down/disabled
-        if s not in (2 ** 16 - 1, 2 ** 32 - 1) or s > 0:
-            return s
+        if operstate(nicName) == OPERSTATE_UP:
+            with open('/sys/class/net/%s/speed' % nicName) as speedFile:
+                s = int(speedFile.read())
+            # the device may have been disabled/downed after checking
+            # so we validate the return value as sysfs may return
+            # special values to indicate the device is down/disabled
+            if s not in (2 ** 16 - 1, 2 ** 32 - 1) and s > 0:
+                return s
     except IOError as ose:
         if ose.errno == errno.EINVAL:
             return _ibHackedSpeed(nicName)
diff --git a/tests/netinfoTests.py b/tests/netinfoTests.py
index 8900b24..62c46b6 100644
--- a/tests/netinfoTests.py
+++ b/tests/netinfoTests.py
@@ -18,9 +18,11 @@
 #
 # Refer to the README and COPYING files for full details of the license
 #
+import __builtin__
 import os
 from datetime import datetime
 from functools import partial
+import io
 import time
 
 import ethtool
@@ -29,7 +31,7 @@
 from vdsm import netconfpersistence
 from vdsm import netinfo
 from vdsm.netinfo import (getBootProtocol, getDhclientIfaces, BONDING_MASTERS,
-                          BONDING_OPT, _getBondingOptions)
+                          BONDING_OPT, _getBondingOptions, OPERSTATE_UP)
 from vdsm.tool.dump_bonding_defaults import _random_iface_name
 
 from functional import dummy, veth
@@ -93,6 +95,21 @@
         for s, addr in zip(inputs, ip):
             self.assertEqual(addr, netinfo.ipv6StrToAddress(s))
 
+    def testValidNicSpeed(self):
+        values = ((0,           OPERSTATE_UP, 0),
+                  (-10,         OPERSTATE_UP, 0),
+                  (2 ** 16 - 1, OPERSTATE_UP, 0),
+                  (2 ** 32 - 1, OPERSTATE_UP, 0)
+                  (123,         OPERSTATE_UP, 123),
+                  (123,         'unknown',    0))
+
+        for passed, operstate, expected in values:
+            with MonkeyPatchScope([(__builtin__, 'open',
+                                    lambda x: io.BytesIO(str(passed))),
+                                   (netinfo, 'operstate',
+                                    lambda x: operstate)]):
+                self.assertEqual(netinfo.nicSpeed('fake_nic'), expected)
+
     @MonkeyPatch(ipwrapper.Link, '_detectType',
                  partial(_fakeTypeDetection, ipwrapper.Link))
     @MonkeyPatch(netinfo, 'networks', lambda: {'fake':
{'bridged': True}})


-- 
To view, visit http://gerrit.ovirt.org/34701
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I34cfca16909f9695441e26d3ddd508e7a4210c12
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Petr Horáček <phoracek(a)redhat.com&gt;