CP-49720 Move LOCK_TYPE_RUNNING from cleanup.py to lock.py

drivers/FileSR.py

@@ -378,31 +378,8 @@ def _process_replay(self, data):
             index += 1
 
     def _kickGC(self):
-        # don't bother if an instance already running (this is just an
-        # optimization to reduce the overhead of forking a new process if we
-        # don't have to, but the process will check the lock anyways)
-        lockRunning = Lock(cleanup.LOCK_TYPE_RUNNING, self.uuid)
-        if not lockRunning.acquireNoblock():
-            if cleanup.should_preempt(self.session, self.uuid):
-                util.SMlog("Aborting currently-running coalesce of garbage VDI")
-                try:
-                    if not cleanup.abort(self.uuid, soft=True):
-                        util.SMlog("The GC has already been scheduled to "
-                                "re-start")
-                except util.CommandException as e:
-                    if e.code != errno.ETIMEDOUT:
-                        raise
-                    util.SMlog('failed to abort the GC')
-                finally:
-                    return
-            else:
-                util.SMlog("A GC instance already running, not kicking")
-                return
-        else:
-            lockRunning.release()
-
         util.SMlog("Kicking GC")
-        cleanup.start_gc(self.uuid)
+        cleanup.start_gc(self.session, self.uuid)
 
     def _isbind(self):
         # os.path.ismount can't deal with bind mount

drivers/LVHDSR.py

@@ -1289,29 +1289,8 @@ def _prepareTestMode(self):
             util.SMlog("Setting env %s" % self.ENV_VAR_VHD_TEST[self.testMode])
 
     def _kickGC(self):
-        # don't bother if an instance already running (this is just an
-        # optimization to reduce the overhead of forking a new process if we
-        # don't have to, but the process will check the lock anyways)
-        lockRunning = Lock(cleanup.LOCK_TYPE_RUNNING, self.uuid)
-        if not lockRunning.acquireNoblock():
-            if cleanup.should_preempt(self.session, self.uuid):
-                util.SMlog("Aborting currently-running coalesce of garbage VDI")
-                try:
-                    if not cleanup.abort(self.uuid, soft=True):
-                        util.SMlog("The GC has already been scheduled to "
-                                "re-start")
-                except util.CommandException as e:
-                    if e.code != errno.ETIMEDOUT:
-                        raise
-                    util.SMlog('failed to abort the GC')
-            else:
-                util.SMlog("A GC instance already running, not kicking")
-                return
-        else:
-            lockRunning.release()
-
         util.SMlog("Kicking GC")
-        cleanup.start_gc(self.uuid)
+        cleanup.start_gc(self.session, self.uuid)
 
     def ensureCBTSpace(self):
         # Ensure we have space for at least one LV

drivers/cleanup.py

@@ -62,13 +62,12 @@
 # process "lock", used simply as an indicator that a process already exists
 # that is doing GC/coalesce on this SR (such a process holds the lock, and we
 # check for the fact by trying the lock).
-LOCK_TYPE_RUNNING = "running"
-lockRunning = None
+lockGCRunning = None
 
 # process "lock" to indicate that the GC process has been activated but may not
 # yet be running, stops a second process from being started.
 LOCK_TYPE_GC_ACTIVE = "gc_active"
-lockActive = None
+lockGCActive = None
 
 # Default coalesce error rate limit, in messages per minute. A zero value
 # disables throttling, and a negative value disables error reporting.
@@ -2934,7 +2933,7 @@ def abortTest():
 
 
 def _gcLoop(sr, dryRun=False, immediate=False):
-    if not lockActive.acquireNoblock():
+    if not lockGCActive.acquireNoblock():
         Util.log("Another GC instance already active, exiting")
         return
 
@@ -2971,7 +2970,7 @@ def _gcLoop(sr, dryRun=False, immediate=False):
                 Util.log("No work, exiting")
                 break
 
-            if not lockRunning.acquireNoblock():
+            if not lockGCRunning.acquireNoblock():
                 Util.log("Unable to acquire GC running lock.")
                 return
             try:
@@ -3013,15 +3012,15 @@ def _gcLoop(sr, dryRun=False, immediate=False):
                     continue
 
             finally:
-                lockRunning.release()
+                lockGCRunning.release()
     except:
         task_status = "failure"
         raise
     finally:
         sr.xapi.set_task_status(task_status)
         Util.log("GC process exiting, no work left")
         _create_init_file(sr.uuid)
-        lockActive.release()
+        lockGCActive.release()
 
 
 def _xapi_enabled(session, hostref):
@@ -3069,19 +3068,19 @@ def _abort(srUuid, soft=False):
     soft: If set to True and there is a pending abort signal, the function
     doesn't do anything. If set to False, a new abort signal is issued.
 
-    returns: If soft is set to False, we return True holding lockActive. If
+    returns: If soft is set to False, we return True holding lockGCActive. If
     soft is set to False and an abort signal is pending, we return False
-    without holding lockActive. An exception is raised in case of error."""
+    without holding lockGCActive. An exception is raised in case of error."""
     Util.log("=== SR %s: abort ===" % (srUuid))
     init(srUuid)
-    if not lockActive.acquireNoblock():
+    if not lockGCActive.acquireNoblock():
         gotLock = False
         Util.log("Aborting currently-running instance (SR %s)" % srUuid)
         abortFlag = IPCFlag(srUuid)
         if not abortFlag.set(FLAG_TYPE_ABORT, soft):
             return False
         for i in range(SR.LOCK_RETRY_ATTEMPTS):
-            gotLock = lockActive.acquireNoblock()
+            gotLock = lockGCActive.acquireNoblock()
             if gotLock:
                 break
             time.sleep(SR.LOCK_RETRY_INTERVAL)
@@ -3093,12 +3092,12 @@ def _abort(srUuid, soft=False):
 
 
 def init(srUuid):
-    global lockRunning
-    if not lockRunning:
-        lockRunning = lock.Lock(LOCK_TYPE_RUNNING, srUuid)
-    global lockActive
-    if not lockActive:
-        lockActive = LockActive(srUuid)
+    global lockGCRunning
+    if not lockGCRunning:
+        lockGCRunning = lock.Lock(lock.LOCK_TYPE_GC_RUNNING, srUuid)
+    global lockGCActive
+    if not lockGCActive:
+        lockGCActive = LockActive(srUuid)
 
 
 class LockActive:
@@ -3166,7 +3165,7 @@ def abort(srUuid, soft=False):
     """
     if _abort(srUuid, soft):
         Util.log("abort: releasing the process lock")
-        lockActive.release()
+        lockGCActive.release()
         return True
     else:
         return False
@@ -3206,7 +3205,27 @@ def gc(session, srUuid, inBackground, dryRun=False):
         _gc(session, srUuid, dryRun, immediate=True)
 
 
-def start_gc(sr_uuid):
+def start_gc(session, sr_uuid):
+    # don't bother if an instance already running (this is just an
+    # optimization to reduce the overhead of forking a new process if we
+    # don't have to, but the process will check the lock anyways)
+    lockRunning = lock.Lock(lock.LOCK_TYPE_GC_RUNNING, sr_uuid)
+    if not lockRunning.acquireNoblock():
+        if should_preempt(session, sr_uuid):
+            util.SMlog("Aborting currently-running coalesce of garbage VDI")
+            try:
+                if not abort(sr_uuid, soft=True):
+                    util.SMlog("The GC has already been scheduled to re-start")
+            except util.CommandException as e:
+                if e.code != errno.ETIMEDOUT:
+                    raise
+                util.SMlog('failed to abort the GC')
+        else:
+            util.SMlog("A GC instance already running, not kicking")
+            return
+    else:
+        lockRunning.release()
+
     util.SMlog(f"Starting GC file is {__file__}")
     subprocess.run([__file__, '-b', '-u', sr_uuid, '-g'],
                    stdout=subprocess.PIPE, stderr=subprocess.PIPE, close_fds=True)
@@ -3224,7 +3243,7 @@ def gc_force(session, srUuid, force=False, dryRun=False, lockSR=False):
     Util.log("=== SR %s: gc_force ===" % srUuid)
     init(srUuid)
     sr = SR.getInstance(srUuid, session, lockSR, True)
-    if not lockActive.acquireNoblock():
+    if not lockGCActive.acquireNoblock():
         abort(srUuid)
     else:
         Util.log("Nothing was running, clear to proceed")
@@ -3240,7 +3259,7 @@ def gc_force(session, srUuid, force=False, dryRun=False, lockSR=False):
     finally:
         sr.cleanup()
         sr.logFilter.logState()
-        lockActive.release()
+        lockGCActive.release()
 
 
 def get_state(srUuid):
@@ -3249,8 +3268,8 @@ def get_state(srUuid):
     locking.
     """
     init(srUuid)
-    if lockActive.acquireNoblock():
-        lockActive.release()
+    if lockGCActive.acquireNoblock():
+        lockGCActive.release()
         return False
     return True
 
@@ -3323,9 +3342,9 @@ def abort_optional_reenable(uuid):
     ret = _abort(uuid)
     input("Press enter to re-enable...")
     print("GC/coalesce re-enabled")
-    lockRunning.release()
+    lockGCRunning.release()
     if ret:
-        lockActive.release()
+        lockGCActive.release()
 
 
 ##############################################################################

drivers/iscsilib.py

@@ -26,7 +26,6 @@
 import lock
 import glob
 import tempfile
-from cleanup import LOCK_TYPE_RUNNING
 from configparser import RawConfigParser
 import io
 
@@ -54,7 +53,7 @@ def doexec_locked(cmd):
     """Executes via util.doexec the command specified whilst holding lock"""
     _lock = None
     if os.path.basename(cmd[0]) == 'iscsiadm':
-        _lock = lock.Lock(LOCK_TYPE_RUNNING, 'iscsiadm')
+        _lock = lock.Lock(lock.LOCK_TYPE_ISCSIADM_RUNNING, 'iscsiadm')
         _lock.acquire()
     # util.SMlog("%s" % cmd)
     (rc, stdout, stderr) = util.doexec(cmd)

drivers/lock.py

@@ -23,6 +23,9 @@
 
 VERBOSE = True
 
+# Still just called "running" for backwards compatibility
+LOCK_TYPE_GC_RUNNING = "running"
+LOCK_TYPE_ISCSIADM_RUNNING = "isciadm_running"
 
 class LockException(util.SMException):
     pass

drivers/resetvdis.py

@@ -26,11 +26,10 @@
 
 def reset_sr(session, host_uuid, sr_uuid, is_sr_master):
     from vhdutil import LOCK_TYPE_SR
-    from cleanup import LOCK_TYPE_RUNNING
 
     cleanup.abort(sr_uuid)
 
-    gc_lock = lock.Lock(LOCK_TYPE_RUNNING, sr_uuid)
+    gc_lock = lock.Lock(lock.LOCK_TYPE_GC_RUNNING, sr_uuid)
     sr_lock = lock.Lock(LOCK_TYPE_SR, sr_uuid)
     gc_lock.acquire()
     sr_lock.acquire()

tests/test_FileSR.py

@@ -450,6 +450,9 @@ def setUp(self):
         lock_patcher = mock.patch('FileSR.Lock')
         self.mock_lock = lock_patcher.start()
 
+        lock_patcher_cleanup = mock.patch('cleanup.lock.Lock')
+        self.mock_lock_cleanup = lock_patcher_cleanup.start()
+
         xapi_patcher = mock.patch('SR.XenAPI')
         self.mock_xapi = xapi_patcher.start()
         self.mock_session = mock.MagicMock()