Change in vdsm[master]: ksm: janitorial: with construct, file() to open()

fromani at redhat.com fromani at redhat.com
Thu Dec 5 12:38:25 UTC 2013


Francesco Romani has uploaded a new change for review.

Change subject: ksm: janitorial: with construct, file() to open()
......................................................................

ksm: janitorial: with construct, file() to open()

Summary: use open() and with wherever feasible.

Details:
- move from file() to open(), as the direct usage of file()
  is deprecated:
  http://docs.python.org/2/library/functions.html#file
  http://docs.python.org/release/3.0/whatsnew/3.0.html#builtins
- use the with construct wherever possible, with files and
  threading.Locks
  http://docs.python.org/2/library/threading.html#using-locks-conditions-and-semaphores-in-the-with-statement
- factor the code which read the procfs entries in a couple
  of helper functions, and a few tests for them.

Change-Id: I47213b3d85eac9afd4cb316c50b192b9c1dfbbd8
Signed-off-by: Francesco Romani <fromani at redhat.com>
---
M tests/miscTests.py
M vdsm/ksm.py
2 files changed, 37 insertions(+), 19 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/84/22084/1

diff --git a/tests/miscTests.py b/tests/miscTests.py
index c836e55..4f3a2dc 100644
--- a/tests/miscTests.py
+++ b/tests/miscTests.py
@@ -31,6 +31,7 @@
 from multiprocessing import Process
 from vdsm import utils
 
+import ksm
 import storage.outOfProcess as oop
 import storage.misc as misc
 import storage.fileUtils as fileUtils
@@ -1303,3 +1304,19 @@
         proc.start()
         self._noIntrWatchFd(myPipe, isEpoll=False, mask=select.POLLIN)
         proc.join()
+
+
+class KsmTests(TestCaseBase):
+    def testReadProcInexistent(self):
+        # Do we deal correctly with not-existent paths?
+        self.assertEquals(ksm._readProcFSInt('/proc/inexistent'), 0)
+
+    def testReadProcNotInt(self):
+        # what about unexpected content?
+        self.assertEquals(ksm._readProcFSInt('/proc/version'), 0)
+
+    def testReadProcValid(self):
+        # we need a procfs entry which exists on every system and
+        # with a predictable value.
+        threadsMax = ksm._readProcFSInt('/proc/sys/kernel/threads-max')
+        self.assertTrue(threadsMax > 0)
diff --git a/vdsm/ksm.py b/vdsm/ksm.py
index c5db89b..c144359 100644
--- a/vdsm/ksm.py
+++ b/vdsm/ksm.py
@@ -27,6 +27,18 @@
 from vdsm.config import config
 
 
+def _readFile(path):
+    with open(path) as f:
+        return f.read()
+
+
+def _readProcFSInt(path):
+    try:
+        return int(_readFile(path))
+    except (IOError, ValueError):
+        return 0
+
+
 class KsmMonitorThread(threading.Thread):
     def __init__(self, cif):
         threading.Thread.__init__(self, name='KsmMonitor')
@@ -49,8 +61,8 @@
         self.cpuUsage = 0
 
     def _getKsmdJiffies(self):
-        return sum(map(int, file('/proc/%s/stat' % self._pid)
-                       .read().split()[13:15]))
+        return sum(map(int, _readFile('/proc/%s/stat' % self._pid)
+                       .split()[13:15]))
 
     def run(self):
         start()
@@ -73,33 +85,22 @@
         current memory stress.
         Return whether ksm is running"""
 
-        self._lock.acquire()
-        try:
+        with self._lock:
             utils.execCmd([constants.EXT_SERVICE, 'ksmtuned', 'retune'],
                           sudo=True)
-        finally:
-            self._lock.release()
-        return running()
+            return running()
 
     def memsharing(self):
-        try:
-            return (int(file('/sys/kernel/mm/ksm/pages_sharing').read()))
-        except:
-            return 0
+        return _readProcFSInt('/sys/kernel/mm/ksm/pages_sharing')
 
 
 def running():
-    try:
-        return int(file('/sys/kernel/mm/ksm/run').read()) & 1 == 1
-    except:
-        return False
+    value = _readProcFSInt('/sys/kernel/mm/ksm/pages_to_scan')
+    return (value & 1) == 1
 
 
 def npages():
-    try:
-        return int(file('/sys/kernel/mm/ksm/pages_to_scan').read())
-    except:
-        return 0
+    return _readProcFSInt('/sys/kernel/mm/ksm/pages_to_scan')
 
 
 def start():


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I47213b3d85eac9afd4cb316c50b192b9c1dfbbd8
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>


More information about the vdsm-patches mailing list