Commit 9d5b1371 authored by Michael Hanselmann's avatar Michael Hanselmann
Browse files

Fix race condition in LUGroupAssignNodes



The original code would get all node information and their groups
without before acquiring the necessary locks. With this patch the node
information is only retrieved once all locks have been acquired. Groups
are locked optimistically and verified after acquiring the node locks.
Signed-off-by: default avatarMichael Hanselmann <hansmi@google.com>
Reviewed-by: default avatarIustin Pop <iustin@google.com>
parent 9d0671ba
......@@ -10220,20 +10220,40 @@ class LUGroupAssignNodes(NoHooksLU):
# We want to lock all the affected nodes and groups. We have readily
# available the list of nodes, and the *destination* group. To gather the
# list of "source" groups, we need to fetch node information.
self.node_data = self.cfg.GetAllNodesInfo()
affected_groups = set(self.node_data[node].group for node in self.op.nodes)
affected_groups.add(self.group_uuid)
# list of "source" groups, we need to fetch node information later on.
self.needed_locks = {
locking.LEVEL_NODEGROUP: list(affected_groups),
locking.LEVEL_NODEGROUP: set([self.group_uuid]),
locking.LEVEL_NODE: self.op.nodes,
}
def DeclareLocks(self, level):
if level == locking.LEVEL_NODEGROUP:
assert len(self.needed_locks[locking.LEVEL_NODEGROUP]) == 1
# Try to get all affected nodes' groups without having the group or node
# lock yet. Needs verification later in the code flow.
groups = self.cfg.GetNodeGroupsFromNodes(self.op.nodes)
self.needed_locks[locking.LEVEL_NODEGROUP].update(groups)
def CheckPrereq(self):
"""Check prerequisites.
"""
assert self.needed_locks[locking.LEVEL_NODEGROUP]
assert (frozenset(self.acquired_locks[locking.LEVEL_NODE]) ==
frozenset(self.op.nodes))
expected_locks = (set([self.group_uuid]) |
self.cfg.GetNodeGroupsFromNodes(self.op.nodes))
actual_locks = self.acquired_locks[locking.LEVEL_NODEGROUP]
if actual_locks != expected_locks:
raise errors.OpExecError("Nodes changed groups since locks were acquired,"
" current groups are '%s', used to be '%s'" %
(utils.CommaJoin(expected_locks),
utils.CommaJoin(actual_locks)))
self.node_data = self.cfg.GetAllNodesInfo()
self.group = self.cfg.GetNodeGroup(self.group_uuid)
instance_data = self.cfg.GetAllInstancesInfo()
......@@ -10269,6 +10289,9 @@ class LUGroupAssignNodes(NoHooksLU):
for node in self.op.nodes:
self.node_data[node].group = self.group_uuid
# FIXME: Depends on side-effects of modifying the result of
# C{cfg.GetAllNodesInfo}
self.cfg.Update(self.group, feedback_fn) # Saves all modified nodes.
@staticmethod
......
......@@ -1400,6 +1400,17 @@ class ConfigWriter:
for node in self._UnlockedGetNodeList()])
return my_dict
@locking.ssynchronized(_config_lock, shared=1)
def GetNodeGroupsFromNodes(self, nodes):
"""Returns groups for a list of nodes.
@type nodes: list of string
@param nodes: List of node names
@rtype: frozenset
"""
return frozenset(self._UnlockedGetNodeInfo(name).group for name in nodes)
def _UnlockedGetMasterCandidateStats(self, exceptions=None):
"""Get the number of current and maximum desired and possible candidates.
......
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