diff --git a/lib/mcpu.py b/lib/mcpu.py
index 8f4946932d0425546f7ade206d47be19c9ce34a0..2851f2977047af724eb1842b4cb73ae591753115 100644
--- a/lib/mcpu.py
+++ b/lib/mcpu.py
@@ -48,6 +48,19 @@ from ganeti import pathutils
_OP_PREFIX = "Op"
_LU_PREFIX = "LU"
+#: LU classes which don't need to acquire the node allocation lock
+#: (L{locking.NAL}) when they acquire all node or node resource locks
+_NODE_ALLOC_WHITELIST = frozenset()
+
+#: LU classes which don't need to acquire the node allocation lock
+#: (L{locking.NAL}) in the same mode (shared/exclusive) as the node
+#: or node resource locks
+_NODE_ALLOC_MODE_WHITELIST = frozenset([
+ cmdlib.LUBackupExport,
+ cmdlib.LUBackupRemove,
+ cmdlib.LUOobCommand,
+ ])
+
class LockAcquireTimeout(Exception):
"""Exception to report timeouts on acquiring locks.
@@ -246,6 +259,44 @@ def _RpcResultsToHooksResults(rpc_results):
for (node, rpc_res) in rpc_results.items())
+def _VerifyLocks(lu, glm, _mode_whitelist=_NODE_ALLOC_MODE_WHITELIST,
+ _nal_whitelist=_NODE_ALLOC_WHITELIST):
+ """Performs consistency checks on locks acquired by a logical unit.
+
+ @type lu: L{cmdlib.LogicalUnit}
+ @param lu: Logical unit instance
+ @type glm: L{locking.GanetiLockManager}
+ @param glm: Lock manager
+
+ """
+ if not __debug__:
+ return
+
+ have_nal = glm.check_owned(locking.LEVEL_NODE_ALLOC, locking.NAL)
+
+ for level in [locking.LEVEL_NODE, locking.LEVEL_NODE_RES]:
+ # TODO: Verify using actual lock mode, not using LU variables
+ if level in lu.needed_locks:
+ share_node_alloc = lu.share_locks[locking.LEVEL_NODE_ALLOC]
+ share_level = lu.share_locks[level]
+
+ if lu.__class__ in _mode_whitelist:
+ assert share_node_alloc != share_level, \
+ "LU is whitelisted to use different modes for node allocation lock"
+ else:
+ assert bool(share_node_alloc) == bool(share_level), \
+ ("Node allocation lock must be acquired using the same mode as nodes"
+ " and node resources")
+
+ if lu.__class__ in _nal_whitelist:
+ assert not have_nal, \
+ "LU is whitelisted for not acquiring the node allocation lock"
+ elif lu.needed_locks[level] == locking.ALL_SET or glm.owning_all(level):
+ assert have_nal, \
+ ("Node allocation lock must be used if an LU acquires all nodes"
+ " or node resources")
+
+
class Processor(object):
"""Object which runs OpCodes"""
DISPATCH_TABLE = _ComputeDispatchTable()
@@ -356,9 +407,13 @@ class Processor(object):
given LU and its opcodes.
"""
+ glm = self.context.glm
adding_locks = level in lu.add_locks
acquiring_locks = level in lu.needed_locks
+
if level not in locking.LEVELS:
+ _VerifyLocks(lu, glm)
+
if self._cbs:
self._cbs.NotifyStart()
@@ -405,7 +460,7 @@ class Processor(object):
lu.remove_locks[level] = add_locks
try:
- self.context.glm.add(level, add_locks, acquired=1, shared=share)
+ glm.add(level, add_locks, acquired=1, shared=share)
except errors.LockError:
logging.exception("Detected lock error in level %s for locks"
" %s, shared=%s", level, add_locks, share)
@@ -418,10 +473,10 @@ class Processor(object):
result = self._LockAndExecLU(lu, level + 1, calc_timeout)
finally:
if level in lu.remove_locks:
- self.context.glm.remove(level, lu.remove_locks[level])
+ glm.remove(level, lu.remove_locks[level])
finally:
- if self.context.glm.is_owned(level):
- self.context.glm.release(level)
+ if glm.is_owned(level):
+ glm.release(level)
else:
result = self._LockAndExecLU(lu, level + 1, calc_timeout)
diff --git a/test/ganeti.mcpu_unittest.py b/test/ganeti.mcpu_unittest.py
index 0678140390de541d020afc757c3b786555b1a605..33d2d670a119096ee89c30b9fa47ca943de66872 100755
--- a/test/ganeti.mcpu_unittest.py
+++ b/test/ganeti.mcpu_unittest.py
@@ -28,6 +28,7 @@ import itertools
from ganeti import mcpu
from ganeti import opcodes
from ganeti import cmdlib
+from ganeti import locking
from ganeti import constants
from ganeti.constants import \
LOCK_ATTEMPTS_TIMEOUT, \
@@ -166,5 +167,128 @@ class TestProcessResult(unittest.TestCase):
self.assertEqual(op2.debug_level, 3)
+class _FakeLuWithLocks:
+ def __init__(self, needed_locks, share_locks):
+ self.needed_locks = needed_locks
+ self.share_locks = share_locks
+
+
+class _FakeGlm:
+ def __init__(self, owning_nal):
+ self._owning_nal = owning_nal
+
+ def check_owned(self, level, names):
+ assert level == locking.LEVEL_NODE_ALLOC
+ assert names == locking.NAL
+ return self._owning_nal
+
+ def owning_all(self, level):
+ return False
+
+
+class TestVerifyLocks(unittest.TestCase):
+ def testNoLocks(self):
+ lu = _FakeLuWithLocks({}, {})
+ glm = _FakeGlm(False)
+ mcpu._VerifyLocks(lu, glm,
+ _mode_whitelist=NotImplemented,
+ _nal_whitelist=NotImplemented)
+
+ def testNotAllSameMode(self):
+ for level in [locking.LEVEL_NODE, locking.LEVEL_NODE_RES]:
+ lu = _FakeLuWithLocks({
+ level: ["foo"],
+ }, {
+ level: 0,
+ locking.LEVEL_NODE_ALLOC: 0,
+ })
+ glm = _FakeGlm(False)
+ mcpu._VerifyLocks(lu, glm, _mode_whitelist=[], _nal_whitelist=[])
+
+ def testDifferentMode(self):
+ for level in [locking.LEVEL_NODE, locking.LEVEL_NODE_RES]:
+ lu = _FakeLuWithLocks({
+ level: ["foo"],
+ }, {
+ level: 0,
+ locking.LEVEL_NODE_ALLOC: 1,
+ })
+ glm = _FakeGlm(False)
+ try:
+ mcpu._VerifyLocks(lu, glm, _mode_whitelist=[], _nal_whitelist=[])
+ except AssertionError, err:
+ self.assertTrue("using the same mode as nodes" in str(err))
+ else:
+ self.fail("Exception not raised")
+
+ # Once more with the whitelist
+ mcpu._VerifyLocks(lu, glm, _mode_whitelist=[_FakeLuWithLocks],
+ _nal_whitelist=[])
+
+ def testSameMode(self):
+ for level in [locking.LEVEL_NODE, locking.LEVEL_NODE_RES]:
+ lu = _FakeLuWithLocks({
+ level: ["foo"],
+ locking.LEVEL_NODE_ALLOC: locking.ALL_SET,
+ }, {
+ level: 1,
+ locking.LEVEL_NODE_ALLOC: 1,
+ })
+ glm = _FakeGlm(True)
+
+ try:
+ mcpu._VerifyLocks(lu, glm, _mode_whitelist=[_FakeLuWithLocks],
+ _nal_whitelist=[])
+ except AssertionError, err:
+ self.assertTrue("whitelisted to use different modes" in str(err))
+ else:
+ self.fail("Exception not raised")
+
+ # Once more without the whitelist
+ mcpu._VerifyLocks(lu, glm, _mode_whitelist=[], _nal_whitelist=[])
+
+ def testAllWithoutAllocLock(self):
+ for level in [locking.LEVEL_NODE, locking.LEVEL_NODE_RES]:
+ lu = _FakeLuWithLocks({
+ level: locking.ALL_SET,
+ }, {
+ level: 0,
+ locking.LEVEL_NODE_ALLOC: 0,
+ })
+ glm = _FakeGlm(False)
+ try:
+ mcpu._VerifyLocks(lu, glm, _mode_whitelist=[], _nal_whitelist=[])
+ except AssertionError, err:
+ self.assertTrue("allocation lock must be used if" in str(err))
+ else:
+ self.fail("Exception not raised")
+
+ # Once more with the whitelist
+ mcpu._VerifyLocks(lu, glm, _mode_whitelist=[],
+ _nal_whitelist=[_FakeLuWithLocks])
+
+ def testAllWithAllocLock(self):
+ for level in [locking.LEVEL_NODE, locking.LEVEL_NODE_RES]:
+ lu = _FakeLuWithLocks({
+ level: locking.ALL_SET,
+ locking.LEVEL_NODE_ALLOC: locking.ALL_SET,
+ }, {
+ level: 0,
+ locking.LEVEL_NODE_ALLOC: 0,
+ })
+ glm = _FakeGlm(True)
+
+ try:
+ mcpu._VerifyLocks(lu, glm, _mode_whitelist=[],
+ _nal_whitelist=[_FakeLuWithLocks])
+ except AssertionError, err:
+ self.assertTrue("whitelisted for not acquiring" in str(err))
+ else:
+ self.fail("Exception not raised")
+
+ # Once more without the whitelist
+ mcpu._VerifyLocks(lu, glm, _mode_whitelist=[], _nal_whitelist=[])
+
+
if __name__ == "__main__":
testutils.GanetiTestProgram()