Commit 6683bba2 authored by Guido Trotter's avatar Guido Trotter
Browse files

Fix issue when acquiring empty lock sets

By design if an empty list of locks is acquired from a set, no locks are
acquired, and thus release() cannot be called on the set. On the other
hand if None is passed instead of the list, the whole set is acquired,
and must later be released. When acquiring whole empty sets, a release
must happen too, because the set-lock is acquired.

Since we used to overwrite the required locks (needed_locks) with the
acquired ones, we weren't able to distinguish the two cases (empty list
of locks required, and all locks required, but an empty list returned
because the set is empty). Valid solutions include:
  (1) forbidding the acquire of empty lists of locks
  (2) skipping the acquire/release on empty lists of locks
  (3) separating the to-acquire and the acquired list

This patch implements the third approach, and thus LUs will find
acquired locks in the acquired_locks dict, rather than in needed_locks.
The LUs which used this feature before have been updated. This makes it
easier because it doesn't force LUs to do more checks on corner cases,
which are easily forgettable (1) and allows more flexibility if we want
LUs to release (part-of) the locks (which is still a possibly scary
operation, but anyway). This easily combines with (2) should we choose
to implement it.

Reviewed-by: imsnah
parent 5685c1a5
......@@ -81,6 +81,7 @@ class LogicalUnit(object):
self.sstore = sstore
self.context = context
self.needed_locks = None
self.acquired_locks = {}
self.share_locks = dict(((i, 0) for i in locking.LEVELS))
# Used to force good behavior when calling helper functions
self.recalculate_locks = {}
......@@ -291,7 +292,7 @@ class LogicalUnit(object):
# future we might want to have different behaviors depending on the value
# of self.recalculate_locks[locking.LEVEL_NODE]
wanted_nodes = []
for instance_name in self.needed_locks[locking.LEVEL_INSTANCE]:
for instance_name in self.acquired_locks[locking.LEVEL_INSTANCE]:
instance = self.context.cfg.GetInstanceInfo(instance_name)
wanted_nodes.append(instance.primary_node)
wanted_nodes.extend(instance.secondary_nodes)
......@@ -1398,7 +1399,7 @@ class LUQueryNodes(NoHooksLU):
"""
# This of course is valid only if we locked the nodes
self.wanted = self.needed_locks[locking.LEVEL_NODE]
self.wanted = self.acquired_locks[locking.LEVEL_NODE]
def Exec(self, feedback_fn):
"""Computes the list of nodes and their attributes.
......@@ -2501,7 +2502,7 @@ class LUQueryInstances(NoHooksLU):
"""
# This of course is valid only if we locked the instances
self.wanted = self.needed_locks[locking.LEVEL_INSTANCE]
self.wanted = self.acquired_locks[locking.LEVEL_INSTANCE]
def Exec(self, feedback_fn):
"""Computes the list of nodes and their attributes.
......
......@@ -139,13 +139,15 @@ class Processor(object):
share = lu.share_locks[level]
# This is always safe to do, as we can't acquire more/less locks than
# what was requested.
lu.needed_locks[level] = self.context.glm.acquire(level,
lu.acquired_locks[level] = self.context.glm.acquire(level,
needed_locks,
shared=share)
try:
result = self._LockAndExecLU(lu, level + 1)
finally:
if lu.needed_locks[level]:
# We need to release the current level if we acquired any lock, or if
# we acquired the set-lock (needed_locks is None)
if lu.needed_locks[level] is None or lu.acquired_locks[level]:
self.context.glm.release(level)
else:
result = self._LockAndExecLU(lu, level + 1)
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment