targetcli: serialize multiple requests

Problem:
-------
targetcli/rtslib cannot handle parallel requests at the moment.
Read more about this at http://bit.ly/targetcli-parallel-requests-issue

$ for i in {1..10}; do \
 targetcli /backstores/fileio create ${i} /tmp/file${i} 10M& done

Created fileio 1 with size 10485760
This _Backstore already exists in configFS
This _Backstore already exists in configFS
This _Backstore already exists in configFS
This _Backstore already exists in configFS
This _Backstore already exists in configFS
Created fileio 6 with size 10485760
Created fileio 2 with size 10485760
This _Backstore already exists in configFS
Created fileio 8 with size 10485760
Created fileio 9 with size 10485760

bails-out most of the time with above errors and sometimes even crashes.

Solution:
--------
Serialize/defend the parallel requests by simply taking a wait lock at
targetcli level, so that only one request can be processed by targetcli at
any given point in time.

Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>

2 files changed, 42 insertions, 47 deletions

diff --git a/daemon/targetclid b/daemon/targetclid
index b909c6e..06ffbe8 100755
--- a/daemon/targetclid
+++ b/daemon/targetclid
@@ -44,8 +44,6 @@ class TargetCLI:
         self.socket_path = '/var/run/targetclid.sock'
         # pid file for defending on multiple daemon runs
         self.pid_file = '/var/run/targetclid.pid'
-        # lockfile for serializing multiple client requests
-        self.lock_file = '/var/run/targetclid.lock'
 
         self.NoSignal = True
 
@@ -85,16 +83,6 @@ class TargetCLI:
             self.pfd.close()
             sys.exit(1)
 
-        try:
-            self.lkfd = open(self.lock_file, 'w+');
-        except IOError as e:
-            self.display(
-                self.render(
-                    "opening lockfile failed: %s" %str(e),
-                    'red'))
-            self.pfd.close()
-            sys.exit(1)
-
         # Keep track, for later use
         self.con_stdout_ = self.con._stdout
         self.con_stderr_ = self.con._stderr
@@ -104,9 +92,6 @@ class TargetCLI:
         '''
         destructor
         '''
-        if not self.lkfd.closed:
-            self.lkfd.close()
-
         if not self.pfd.closed:
             self.pfd.close()
 
@@ -149,40 +134,10 @@ class TargetCLI:
         self.pfd.close()
 
 
-    def try_op_lock(self):
-        '''
-        acquire a blocking lock on lockfile, to serialize multiple client requests
-        '''
-        try:
-            fcntl.flock(self.lkfd, fcntl.LOCK_EX)  # wait here until ongoing request is finished
-        except Exception, e:
-            self.display(
-                self.render(
-                    "taking lock on lockfile failed: %s" %str(e),
-                    'red'))
-            sys.exit(1)
-
-
-    def release_op_lock(self):
-        '''
-        release blocking lock on lockfile, which can allow other requests process
-        '''
-        try:
-            fcntl.flock(self.lkfd, fcntl.LOCK_UN)  # allow other requests now
-        except Exception, e:
-            self.display(
-                self.render(
-                    "unlock on lockfile failed: %s" %str(e),
-                    'red'))
-            sys.exit(1)
-
-
     def client_thread(self, connection):
         '''
         Handle commands from client
         '''
-        self.try_op_lock()
-
         still_listen = True
         # Receive the data in small chunks and retransmit it
         while still_listen:
@@ -212,8 +167,6 @@ class TargetCLI:
                 connection.sendall(var)    # length of string
                 connection.sendall(output) # actual string
 
-        self.release_op_lock()
-
 
 def usage():
     print("Usage: %s [--version|--help]" % sys.argv[0], file=err)
diff --git a/scripts/targetcli b/scripts/targetcli
index 66cb5bb..9e03b0f 100755
--- a/scripts/targetcli
+++ b/scripts/targetcli
@@ -31,8 +31,11 @@ import socket
 import struct
 import readline
 import six
+import fcntl
 
 err = sys.stderr
+# lockfile for serializing multiple targetcli requests
+lock_file = '/var/run/targetcli.lock'
 socket_path = '/var/run/targetclid.sock'
 hints = ['/', 'backstores/', 'iscsi/', 'loopback/', 'vhost/', 'xen-pvscsi/',
          'cd', 'pwd', 'ls', 'set', 'get', 'help', 'refresh', 'status',
@@ -85,6 +88,33 @@ def usage_version(cmd):
     if cmd in ("version", "--version", "-v"):
         version()
 
+def try_op_lock(shell, lkfd):
+    '''
+    acquire a blocking lock on lockfile, to serialize multiple requests
+    '''
+    try:
+        fcntl.flock(lkfd, fcntl.LOCK_EX)  # wait here until ongoing request is finished
+    except Exception, e:
+        shell.con.display(
+            shell.con.render_text(
+                "taking lock on lockfile failed: %s" %str(e),
+                'red'))
+        sys.exit(1)
+
+def release_op_lock(shell, lkfd):
+    '''
+    release blocking lock on lockfile, which can allow other requests process
+    '''
+    try:
+        fcntl.flock(lkfd, fcntl.LOCK_UN)  # allow other requests now
+    except Exception, e:
+        shell.con.display(
+            shell.con.render_text(
+                "unlock on lockfile failed: %s" %str(e),
+                'red'))
+        sys.exit(1)
+    lkfd.close()
+
 def completer(text, state):
     options = [x for x in hints if x.startswith(text)]
     try:
@@ -168,6 +198,16 @@ def main():
     if getuid() == 0:
         is_root = True
 
+    try:
+        lkfd = open(lock_file, 'w+');
+    except IOError as e:
+        shell.con.display(
+                shell.con.render_text("opening lockfile failed: %s" %str(e),
+                    'red'))
+        sys.exit(1)
+
+    try_op_lock(shell, lkfd)
+
     use_daemon = False
     if shell.prefs['auto_use_daemon']:
         use_daemon = True
@@ -214,6 +254,8 @@ def main():
         shell.log.info("Global pref auto_save_on_exit=true")
         root_node.ui_command_saveconfig()
 
+    release_op_lock(shell, lkfd)
+
 
 if __name__ == "__main__":
     main()