Change in vdsm[ovirt-3.5]: mounts: Optimize mount loop device resolution

ybronhei at redhat.com ybronhei at redhat.com
Fri Sep 5 23:32:07 UTC 2014


Hello Saggi Mizrahi, Dima Kuznetsov,

I'd like you to do a code review.  Please visit

    http://gerrit.ovirt.org/32535

to review the following change.

Change subject: mounts: Optimize mount loop device resolution
......................................................................

mounts: Optimize mount loop device resolution

Existing code was re-reading mtab for each /proc/mounts entry.
Added lookup dictionary that translates /dev/loop spec to actual file
spec.

Lookup is reinitialized each time mtab is changed.

Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1112779
Change-Id: I54f11786b45782cedd994d52e1e506292132fa47
Signed-off-by: Dima Kuznetsov <dkuznets at redhat.com>
Reviewed-on: http://gerrit.ovirt.org/28586
Reviewed-by: Saggi Mizrahi <smizrahi at redhat.com>
Tested-by: Saggi Mizrahi <smizrahi at redhat.com>
---
M tests/mountTests.py
M vdsm/storage/mount.py
2 files changed, 152 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/35/32535/1

diff --git a/tests/mountTests.py b/tests/mountTests.py
index 04aa753..6eb6832 100644
--- a/tests/mountTests.py
+++ b/tests/mountTests.py
@@ -20,8 +20,10 @@
 
 from contextlib import contextmanager
 import errno
-from tempfile import mkstemp
+from tempfile import mkstemp, mkdtemp
 import os
+import shutil
+import stat
 
 from nose.plugins.skip import SkipTest
 
@@ -30,6 +32,7 @@
 from storage.misc import execCmd
 import storage.mount as mount
 from testValidation import checkSudo
+import monkeypatch
 
 FLOPPY_SIZE = (2 ** 20) * 4
 
@@ -72,3 +75,117 @@
                     self.assertTrue(m.isMounted())
                 finally:
                     m.umount()
+
+
+class IterMountsPerfTests(TestCaseBase):
+    line_fmt = ('%(fs_spec)s\t%(fs_file)s\t%(fs_vfstype)s'
+                '\t%(fs_mntops)s\t%(fs_freq)s\t%(fs_passno)s\n')
+
+    @classmethod
+    def _createFiles(cls, path, size):
+        mounts_path = os.path.join(path, 'mounts')
+        mtab_path = os.path.join(path, 'mtab')
+
+        with open(mounts_path, 'w') as mounts:
+            with open(mtab_path, 'w') as mtab:
+                for i in xrange(100):
+                    mnt = mount.MountRecord('/dev/sda%d' % i,
+                                            '/some/path/%d' % i,
+                                            'btrfs',
+                                            'rw,relatime',
+                                            '0', '0')
+                    mounts.write(cls.line_fmt % mnt._asdict())
+                    mtab.write(cls.line_fmt % mnt._asdict())
+
+                for i in xrange(size):
+                    mounts_mnt = mount.MountRecord('/dev/loop%d' % i,
+                                                   '/mnt/loop%d' % i,
+                                                   'xfs',
+                                                   'rw,foobar,errors=continue',
+                                                   '0', '0')
+                    mtab_mnt = mount.MountRecord('/images/loop%d.img' % i,
+                                                 '/mnt/loop%d' % i,
+                                                 'xfs',
+                                                 'rw,loop=/dev/loop%d' % i,
+                                                 '0', '0')
+                    mounts.write(cls.line_fmt % mounts_mnt._asdict())
+                    mtab.write(cls.line_fmt % mtab_mnt._asdict())
+
+        return (mounts_path, mtab_path)
+
+    def setUp(self):
+        self._temp_dir = mkdtemp()
+        mounts, mtab = self._createFiles(self._temp_dir, 1000)
+        old_stat = os.stat
+
+        def mock_stat(path):
+            if path.startswith('/dev/loop'):
+                return old_stat('/')
+            return old_stat(path)
+        self._patch = monkeypatch.Patch([(mount, '_PROC_MOUNTS_PATH', mounts),
+                                         (mount, '_ETC_MTAB_PATH', mtab),
+                                         (mount, '_SYS_DEV_BLOCK_PATH',
+                                          self._temp_dir),
+                                         (mount, '_loopFsSpecs', {}),
+                                         (os, 'stat', mock_stat),
+                                         (stat, 'S_ISBLK', lambda x: True)])
+        self._patch.apply()
+
+    def tearDown(self):
+        shutil.rmtree(self._temp_dir)
+        self._patch.revert()
+
+    def test1000EntriesValidate(self):
+        mounts = list(mount.iterMounts())
+        for entry in mounts:
+            mountpoint = entry.fs_file
+            if mountpoint.startswith('/mnt/loop'):
+                expected_spec = ('/images/loop%s.img' %
+                                 mountpoint[len('/mnt/loop'):])
+                self.assertEquals(entry.fs_spec, expected_spec)
+
+    def test1000EntriesTwice(self):
+        list(mount.iterMounts())
+        list(mount.iterMounts())
+
+    def testLookupWipe(self):
+        with open(mount._PROC_MOUNTS_PATH) as f:
+            old_mounts = f.read()
+        with open(mount._ETC_MTAB_PATH) as f:
+            old_mtab = f.read()
+
+        self.assertEquals(len(list(mount.iterMounts())), 1100)
+        mounts_mnt = mount.MountRecord('/dev/loop10001',
+                                       '/mnt/loop10001',
+                                       'xfs',
+                                       'rw,foobar,errors=continue',
+                                       '0', '0')
+        mtab_mnt = mount.MountRecord('/images/loop10001.img',
+                                     '/mnt/loop%d',
+                                     'xfs',
+                                     'rw,loop=/dev/loop10001',
+                                     '0', '0')
+
+        with open(mount._PROC_MOUNTS_PATH, 'a') as f:
+            f.write(self.line_fmt % mounts_mnt._asdict())
+        with open(mount._ETC_MTAB_PATH, 'a') as f:
+            f.write(self.line_fmt % mtab_mnt._asdict())
+
+        self.assertEquals(len(list(mount.iterMounts())), 1101)
+        self.assertEquals(
+            len(filter(lambda x: x.fs_spec == '/images/loop10001.img',
+                       mount.iterMounts())),
+            1)
+        self.assertTrue('/dev/loop10001' in mount._getLoopFsSpecs())
+
+        with open(mount._PROC_MOUNTS_PATH, 'w') as f:
+            f.write(old_mounts)
+        with open(mount._ETC_MTAB_PATH, 'w') as f:
+            f.write(old_mtab)
+
+        self.assertEquals(len(list(mount.iterMounts())), 1100)
+        self.assertEquals(
+            len(filter(lambda x: x.fs_spec == '/images/loop10001.img',
+                       mount.iterMounts())),
+            0)
+        self.assertFalse('/dev/loop10001' in mount._getLoopFsSpecs())
diff --git a/vdsm/storage/mount.py b/vdsm/storage/mount.py
index a475a6f..99b2299 100644
--- a/vdsm/storage/mount.py
+++ b/vdsm/storage/mount.py
@@ -23,6 +23,7 @@
 import re
 import os
 import stat
+import threading
 
 from vdsm import constants
 import misc
@@ -35,6 +36,10 @@
 
 MountRecord = namedtuple("MountRecord", "fs_spec fs_file fs_vfstype "
                          "fs_mntops fs_freq fs_passno")
+
+_ETC_MTAB_PATH = '/etc/mtab'
+_PROC_MOUNTS_PATH = '/proc/mounts'
+_SYS_DEV_BLOCK_PATH = '/sys/dev/block/'
 
 _RE_ESCAPE = re.compile(r"\\0\d\d")
 
@@ -62,7 +67,7 @@
 
 
 def _iterateMtab():
-    with open("/etc/mtab", "r") as f:
+    with open(_ETC_MTAB_PATH, "r") as f:
         for line in f:
             yield _parseFstabLine(line)
 
@@ -73,6 +78,26 @@
 
 class MountError(RuntimeError):
     pass
+
+
+_loopFsSpecsLock = threading.Lock()
+_loopFsSpecs = {}
+_loopFsSpecsTimestamp = None
+
+
+def _getLoopFsSpecs():
+    with _loopFsSpecsLock:
+        mtabTimestamp = os.stat(_ETC_MTAB_PATH).st_mtime
+        if _loopFsSpecsTimestamp != mtabTimestamp:
+            global _loopFsSpecs
+            _loopFsSpecs = {}
+            for entry in _iterateMtab():
+                for opt in entry.fs_mntops:
+                    if opt.startswith('loop='):
+                        _loopFsSpecs[opt[len('loop='):]] = entry.fs_spec
+            global _loopFsSpecsTimestamp
+            _loopFsSpecsTimestamp = mtabTimestamp
+    return _loopFsSpecs
 
 
 def _resolveLoopDevice(path):
@@ -93,7 +118,9 @@
 
     minor = os.minor(st.st_rdev)
     major = os.major(st.st_rdev)
-    loopdir = "/sys/dev/block/%d:%d/loop" % (major, minor)
+    loopdir = os.path.join(_SYS_DEV_BLOCK_PATH,
+                           '%d:%d' % (major, minor),
+                           'loop')
     if os.path.exists(loopdir):
         with open(loopdir + "/backing_file", "r") as f:
             # Remove trailing newline
@@ -101,19 +128,17 @@
 
     # Old kernels might not have the sysfs entry, this is a bit slower and does
     # not work on hosts that do support the above method.
-    for rec in _iterateMtab():
-        loopOpt = "loop=%s" % path
-        for opt in rec.fs_mntops:
-            if opt != loopOpt:
-                continue
 
-            return rec.fs_spec
+    lookup = _getLoopFsSpecs()
+
+    if path in lookup:
+        return lookup[path]
 
     return path
 
 
 def _iterKnownMounts():
-    with open("/proc/mounts", "r") as f:
+    with open(_PROC_MOUNTS_PATH, "r") as f:
         for line in f:
             yield _parseFstabLine(line)
 


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I54f11786b45782cedd994d52e1e506292132fa47
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Dima Kuznetsov <dkuznets at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>


More information about the vdsm-patches mailing list