[master 3/4] Manage backing store more independently in loop backed test cases.

mulkieran installerbot-noreply at redhat.com
Tue May 26 16:58:08 UTC 2015


From: mulhern <amulhern at redhat.com>

* Make the backing store before starting to use the loop devices so there
is less elapsed time between finding a free loop device and actually using
it. Get rid of it if there is a failure in locating a free loop device
or using it.

* Make self._loopMap a list of pairs, since we never do random access
anyway.

* Don't give up on deleting all loop devices the first time one loop
device is not succesfully removed.

* Give every loop device a minimum of three deletion attempts
before teardown gives up. This is apallingly ad-hoc, but almost works.
As many deletion attempts as needed is sometimes infinitely too many.

* Use tempfile.NamedTemporaryFile to get backing store. Still put the
tempfile in "/var/tmp" not "/tmp".

Signed-off-by: mulhern <amulhern at redhat.com>
---
 tests/loopbackedtestcase.py | 65 ++++++++++++++++++++++++++++++++++-----------
 1 file changed, 50 insertions(+), 15 deletions(-)

diff --git a/tests/loopbackedtestcase.py b/tests/loopbackedtestcase.py
index 9451ed1..4c7443c 100644
--- a/tests/loopbackedtestcase.py
+++ b/tests/loopbackedtestcase.py
@@ -1,13 +1,13 @@
 import unittest
 import os
 import subprocess
+import tempfile
 
 from blivet.size import Size
 
-def makeLoopDev(device_name, file_name, num_blocks=102400, block_size=1024):
-    """ Set up a loop device with a backing store.
+def makeStore(file_name, num_blocks=102400, block_size=1024):
+    """ Set up the backing store for a loop device.
 
-        :param str device_name: the path of the loop device
         :param str file_name: the path of the backing file
         :param int num_blocks: the size of file_name in number of blocks
         :param int block_size: the number of bytes in a block
@@ -23,6 +23,13 @@ def makeLoopDev(device_name, file_name, num_blocks=102400, block_size=1024):
     if rc:
         raise OSError("dd failed creating the file %s" % file_name)
 
+def makeLoopDev(device_name, file_name):
+    """ Set up a loop device with a backing store.
+
+        :param str device_name: the path of the loop device
+        :param str file_name: the path of the backing file
+    """
+
     proc = subprocess.Popen(["losetup", device_name, file_name],
                             stdout=subprocess.PIPE, stderr=subprocess.PIPE)
     while True:
@@ -33,7 +40,7 @@ def makeLoopDev(device_name, file_name, num_blocks=102400, block_size=1024):
     if rc:
         raise OSError("losetup failed setting up the loop device %s" % device_name)
 
-def removeLoopDev(device_name, file_name):
+def removeLoopDev(device_name):
     proc = subprocess.Popen(["losetup", "-d", device_name],
                             stdout=subprocess.PIPE, stderr=subprocess.PIPE)
     while True:
@@ -44,8 +51,6 @@ def removeLoopDev(device_name, file_name):
     if rc:
         raise OSError("losetup failed removing the loop device %s" % device_name)
 
-    os.unlink(file_name)
-
 def getFreeLoopDev():
     # There's a race condition here where another process could grab the loop
     # device losetup gives us before we have time to set it up, but that's just
@@ -72,8 +77,6 @@ class LoopBackedTestCase(unittest.TestCase):
     DEFAULT_BLOCK_SIZE = Size("1 KiB")
     DEFAULT_STORE_SIZE = Size("100 MiB")
     _DEFAULT_DEVICE_SPEC = [DEFAULT_STORE_SIZE, DEFAULT_STORE_SIZE]
-    _STORE_FILE_TEMPLATE = 'test-virtdev%d'
-    _STORE_FILE_PATH = '/var/tmp'
 
     def __init__(self, methodName='runTest', deviceSpec=None, block_size=None):
         """ DevicelibsTestCase manages loop devices.
@@ -88,7 +91,7 @@ def __init__(self, methodName='runTest', deviceSpec=None, block_size=None):
         """
         unittest.TestCase.__init__(self, methodName=methodName)
         self._deviceSpec = deviceSpec or self._DEFAULT_DEVICE_SPEC
-        self._loopMap = {}
+        self._loopMap = []
         self.loopDevices = []
         self.block_size = block_size or self.DEFAULT_BLOCK_SIZE
 
@@ -97,13 +100,45 @@ def __init__(self, methodName='runTest', deviceSpec=None, block_size=None):
 
     def setUp(self):
         for index, size in enumerate(self._deviceSpec):
-            store = os.path.join(self._STORE_FILE_PATH, self._STORE_FILE_TEMPLATE % index)
-            dev = getFreeLoopDev()
             num_blocks = int(size / self.block_size)
-            makeLoopDev(dev, store, num_blocks=num_blocks, block_size=int(self.block_size))
-            self._loopMap[dev] = store
+            tmpfile = tempfile.NamedTemporaryFile(suffix="-%d" % index, prefix="test-virtdev-", dir="/var/tmp", delete=False)
+            tmpfile.close()
+
+            store = tmpfile.name
+            makeStore(store, num_blocks, int(self.block_size))
+
+            dev = None
+            try:
+                dev = getFreeLoopDev()
+                makeLoopDev(dev, store)
+            except OSError:
+                os.unlink(store)
+                raise
+
+            self._loopMap.append((dev, store))
             self.loopDevices.append(dev)
 
     def tearDown(self):
-        for (dev, store) in iter(self._loopMap.items()):
-            removeLoopDev(dev, store)
+        # Guarantees that every item in _loopMap receives a minimum of
+        # three chances to be removed or deleted as appropriate.
+        num_tries = 3 * len(self._loopMap)
+        for _ in range(num_tries):
+            # If worklist is empty, quit
+            if not self._loopMap:
+                break
+
+            # get worklist item from front
+            dev, store = self._loopMap.pop(0)
+
+            # try to clean up dev, store pair
+            # if cleanup fails, push pair onto back
+            # dev is None if removal has already succeeded
+            if dev is not None:
+                try:
+                    removeLoopDev(dev)
+                    try:
+                        os.unlink(store)
+                    except OSError:
+                        self._loopMap.append((None, store))
+                except OSError:
+                    self._loopMap.append((dev, store))


-- 
To view this commit on github, visit https://github.com/rhinstaller/blivet/commit/63932bd51fa768d99caea6c7bf52ecb34a85ff82


More information about the anaconda-patches mailing list