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>
---
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>