Add locking around configfs save/restore/clear operationslocking

One concern raised on the list recently was that concurrent readers and
writers to configfs could result in things blowing up. This patch adds
a lock, which should serialize save, restore, and clear operations that
are used by targetctl, and potentially other clients via the rtslib API.

However, targetcli uses rtslib objects returned from other RTSRoot methods,
and could also potentially cause issues. We need to determine if we are OK
with that, or if more steps need to be taken to avoid this.

Signed-off-by: Andy Grover <agrover@redhat.com>

2 files changed, 35 insertions, 1 deletions

diff --git a/rtslib/root.py b/rtslib/root.py
index d558445..13d7f2a 100644
--- a/rtslib/root.py
+++ b/rtslib/root.py
@@ -28,7 +28,7 @@ from fabric import FabricModule
 from tcm import (StorageObject, FileIOStorageObject, BlockStorageObject,
                  PSCSIStorageObject, RDMCPStorageObject)
 from utils import RTSLibError, RTSLibBrokenLink, modprobe, mount_configfs
-from utils import dict_remove, set_attributes
+from utils import dict_remove, set_attributes, locked
 
 storageobjects = dict(
     fileio=FileIOStorageObject,
@@ -126,6 +126,7 @@ class RTSRoot(CFSNode):
 
     # RTSRoot public stuff
 
+    @locked
     def dump(self):
         '''
         Returns a dict representing the complete state of the target
@@ -140,6 +141,7 @@ class RTSRoot(CFSNode):
                                if f.discovery_enable_auth]
         return d
 
+    @locked
     def clear_existing(self, confirm=False):
         '''
         Remove entire current configuration.
@@ -155,6 +157,7 @@ class RTSRoot(CFSNode):
         for so in self.storage_objects:
             so.delete()
 
+    @locked
     def restore(self, config, clear_existing=False, abort_on_error=False):
         '''
         Takes a dict generated by dump() and reconfigures the target to match.
diff --git a/rtslib/utils.py b/rtslib/utils.py
index 8f86d6d..1f36946 100644
--- a/rtslib/utils.py
+++ b/rtslib/utils.py
@@ -23,6 +23,8 @@ import stat
 import uuid
 import socket
 import subprocess
+import fcntl
+import time
 from contextlib import contextmanager
 
 class RTSLibError(Exception):
@@ -391,6 +393,35 @@ def ignored(*exceptions):
     except exceptions:
         pass
 
+@contextmanager
+def file_lock(lock_file, timeout=10):
+    '''
+    A context manager for locking a file, with timeout.
+    '''
+    with open(lock_file, "w") as f:
+        for x in xrange(timeout):
+            try:
+                fcntl.lockf(f, fcntl.LOCK_EX | fcntl.LOCK_NB)
+            except IOError:
+                time.sleep(1)
+            else:
+                try:
+                    yield
+                finally:
+                    fcntl.lockf(f, fcntl.LOCK_UN)
+                break
+        else:
+            raise IOError("Acquiring lock timed out after %d seconds" % timeout)
+
+def locked(func):
+    '''
+    A decorator that enforces mutual exclusion between methods using it.
+    '''
+    def inner(*args, **kwargs):
+        with file_lock("/var/run/target.lock"):
+            return func(*args, **kwargs)
+    return inner
+
 #
 # These two functions are meant to be used with functools.partial and
 # properties.