CP-49720 Move LOCK_TYPE_RUNNING from cleanup.py to lock.py #691
Conversation
drivers/iscsilib.py
Outdated
| @@ -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_GC_RUNNING, 'iscsiadm') | |||
There was a problem hiding this comment.
This is now incorrect. IN this case the lock should be a unique LOCK_TYPE_ISCSIADM_RUNNING as it is a lock around usage of iscsiadm and nothing to do with the GC.
drivers/FileSR.py
Outdated
| @@ -31,7 +31,7 @@ | |||
| import time | |||
| import glob | |||
| from uuid import uuid4 | |||
| from lock import Lock | |||
| from lock import Lock, LOCK_TYPE_GC_RUNNING | |||
There was a problem hiding this comment.
Still needed? I suspect not.
drivers/LVHDSR.py
Outdated
| @@ -36,7 +36,8 @@ | |||
| import cleanup | |||
| import blktap2 | |||
| from journaler import Journaler | |||
| from lock import Lock | |||
| from lock import Lock, LOCK_TYPE_GC_RUNNING | |||
| import cleanup | |||
There was a problem hiding this comment.
Surely it must already have had cleanup imported as it was calling cleanup.start_gc? And the LOCK_TYPE_GC_RUNNING is no longer required
There was a problem hiding this comment.
In fact, yes, on line 36
drivers/lock.py
Outdated
| @@ -23,6 +23,8 @@ | |||
|
|
|||
| VERBOSE = True | |||
|
|
|||
| LOCK_TYPE_GC_RUNNING = "gc_running" | |||
There was a problem hiding this comment.
Do we risk having a clash with a pre-existing runing process during an upgrade where the running GC process already has the running lock and then a new process tries, and succeeds, in acquiring the gc_running lock and assumes it has free reign over the GC process? That probably results in data corruption unless we ensure the upgrade aborts any in progress GC operations, which it does not by default do.
In the process, rename LOCK_TYPE_RUNNING to LOCK_TYPE_GC_RUNNING to describe *what* is running when it's running. This matches LOCK_TYPE_GC_ACTIVE which remains in cleanup.py because it is only used there. Rename the global variables in cleanup.py to describe *what* is Active/Running. Signed-off-by: Tim Smith <tim.smith@cloud.com>
Separate the lock for iscsiadm from the GC lock Signed-off-by: Tim Smith <tim.smith@cloud.com>
Both LVHDSR and FileSR in their _kickGC() method would check to see if a GC was already in progress and avoid starting one if so. This code was duplicated and can be moved into the cleanup.start_gc() method. Signed-off-by: Tim Smith <tim.smith@cloud.com>
TimSmithCtx
force-pushed
the
private/timsmi/CP-49720
branch
from
201a1a8 to
78faa7d
Compare
| @@ -23,6 +23,9 @@ | |||
|
|
|||
| VERBOSE = True | |||
|
|
|||
| # Still just called "running" for backwards compatibility | |||
| LOCK_TYPE_GC_RUNNING = "running" | |||
| LOCK_TYPE_ISCSIADM_RUNNING = "isciadm_running" | |||
There was a problem hiding this comment.
This is fine but we will get a slightly odd lock name of iscsiadm.iscsiadm_running as a result.
There was a problem hiding this comment.
I'm fine with that. If in the future we encounter a lock named newthing.iscsiadm_running we'll know where to look for the cut & paste error...
In the process, rename LOCK_TYPE_RUNNING to LOCK_TYPE_GC_RUNNING to describe what is running when it's running. This matches LOCK_TYPE_GC_ACTIVE which remains in cleanup.py because it is only used there.
Rename the global variables in cleanup.py to describe what is Active/Running.