From: Ilias Tsitsimpis <[email protected]> AllocateDRBDMinor used to allocate some DRBD minors for a given instance using ComputeDRBDMap. ComputeDRBDMap didn't take into account, the DRBD minors for disks that are detached (don't belong to any instance). This patch fixes the above problem by modifying AllocateDRBDMinor and ReleaseDRBDMinors in order to accept the disk for which we want to allocate the minors (and not the instance). In addition, ComputeDRBDMap now examines all the disks (even the detached ones) in order to compute the minors.
Signed-off-by: Ilias Tsitsimpis <[email protected]> Signed-off-by: Alex Pyrgiotis <[email protected]> diff --git a/lib/cmdlib/cluster/verify.py b/lib/cmdlib/cluster/verify.py index 222b4ca..2a40b86 100644 --- a/lib/cmdlib/cluster/verify.py +++ b/lib/cmdlib/cluster/verify.py @@ -437,6 +437,7 @@ class LUClusterVerifyGroup(LogicalUnit, _VerifyErrors): self.all_node_info = self.cfg.GetAllNodesInfo() self.all_inst_info = self.cfg.GetAllInstancesInfo() + self.all_disks_info = self.cfg.GetAllDisksInfo() self.my_node_uuids = group_node_uuids self.my_node_info = dict((node_uuid, self.all_node_info[node_uuid]) @@ -1178,14 +1179,15 @@ class LUClusterVerifyGroup(LogicalUnit, _VerifyErrors): self._ErrorIf(test, constants.CV_ENODEDRBDHELPER, ninfo.name, "wrong drbd usermode helper: %s", payload) - def _VerifyNodeDrbd(self, ninfo, nresult, instanceinfo, drbd_helper, - drbd_map): + def _VerifyNodeDrbd(self, ninfo, nresult, instanceinfo, disks_info, + drbd_helper, drbd_map): """Verifies and the node DRBD status. @type ninfo: L{objects.Node} @param ninfo: the node to check @param nresult: the remote results for the node @param instanceinfo: the dict of instances + @param disks_info: the dict of disks @param drbd_helper: the configured DRBD usermode helper @param drbd_map: the DRBD map as returned by L{ganeti.config.ConfigWriter.ComputeDRBDMap} @@ -1195,18 +1197,24 @@ class LUClusterVerifyGroup(LogicalUnit, _VerifyErrors): # compute the DRBD minors node_drbd = {} - for minor, inst_uuid in drbd_map[ninfo.uuid].items(): - test = inst_uuid not in instanceinfo + for minor, disk_uuid in drbd_map[ninfo.uuid].items(): + test = disk_uuid not in disks_info self._ErrorIf(test, constants.CV_ECLUSTERCFG, None, - "ghost instance '%s' in temporary DRBD map", inst_uuid) - # ghost instance should not be running, but otherwise we - # don't give double warnings (both ghost instance and + "ghost disk '%s' in temporary DRBD map", disk_uuid) + # ghost disk should not be active, but otherwise we + # don't give double warnings (both ghost disk and # unallocated minor in use) if test: - node_drbd[minor] = (inst_uuid, False) + node_drbd[minor] = (disk_uuid, None, False) else: - instance = instanceinfo[inst_uuid] - node_drbd[minor] = (inst_uuid, instance.disks_active) + disk_active = False + disk_instance = None + for (inst_uuid, inst) in instanceinfo.items(): + if disk_uuid in inst.disks: + disk_active = inst.disks_active + disk_instance = inst.name + break + node_drbd[minor] = (disk_uuid, disk_instance, disk_active) # and now check them used_minors = nresult.get(constants.NV_DRBDLIST, []) @@ -1217,11 +1225,16 @@ class LUClusterVerifyGroup(LogicalUnit, _VerifyErrors): # we cannot check drbd status return - for minor, (inst_uuid, must_exist) in node_drbd.items(): + for minor, (disk_uuid, inst_uuid, must_exist) in node_drbd.items(): test = minor not in used_minors and must_exist + if inst_uuid is not None: + attached = "(attached in instance '%s')" % \ + self.cfg.GetInstanceName(inst_uuid) + else: + attached = "(detached)" self._ErrorIf(test, constants.CV_ENODEDRBD, ninfo.name, - "drbd minor %d of instance %s is not active", minor, - self.cfg.GetInstanceName(inst_uuid)) + "drbd minor %d of disk %s %s is not active", + minor, disk_uuid, attached) for minor in used_minors: test = minor not in node_drbd self._ErrorIf(test, constants.CV_ENODEDRBD, ninfo.name, @@ -2033,8 +2046,8 @@ class LUClusterVerifyGroup(LogicalUnit, _VerifyErrors): if nimg.vm_capable: self._UpdateVerifyNodeLVM(node_i, nresult, vg_name, nimg) if constants.DT_DRBD8 in cluster.enabled_disk_templates: - self._VerifyNodeDrbd(node_i, nresult, self.all_inst_info, drbd_helper, - all_drbd_map) + self._VerifyNodeDrbd(node_i, nresult, self.all_inst_info, + self.all_disks_info, drbd_helper, all_drbd_map) if (constants.DT_PLAIN in cluster.enabled_disk_templates) or \ (constants.DT_DRBD8 in cluster.enabled_disk_templates): diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py index 989f6b3..a8403f6 100644 --- a/lib/cmdlib/instance.py +++ b/lib/cmdlib/instance.py @@ -467,7 +467,8 @@ class LUInstanceMove(LogicalUnit): CreateDisks(self, self.instance, target_node_uuid=target_node.uuid) except errors.OpExecError: self.LogWarning("Device creation failed") - self.cfg.ReleaseDRBDMinors(self.instance.uuid) + for disk_uuid in self.instance.disks: + self.cfg.ReleaseDRBDMinors(disk_uuid) raise errs = [] @@ -500,7 +501,8 @@ class LUInstanceMove(LogicalUnit): try: RemoveDisks(self, self.instance, target_node_uuid=target_node.uuid) finally: - self.cfg.ReleaseDRBDMinors(self.instance.uuid) + for disk_uuid in self.instance.disks: + self.cfg.ReleaseDRBDMinors(disk_uuid) raise errors.OpExecError("Errors during disk copy: %s" % (",".join(errs),)) diff --git a/lib/cmdlib/instance_create.py b/lib/cmdlib/instance_create.py index 92cacac..cbb8c39 100644 --- a/lib/cmdlib/instance_create.py +++ b/lib/cmdlib/instance_create.py @@ -1525,7 +1525,8 @@ class LUInstanceCreate(LogicalUnit): CreateDisks(self, iobj, disks=disks) except errors.OpExecError: self.LogWarning("Device creation failed") - self.cfg.ReleaseDRBDMinors(instance_uuid) + for disk in disks: + self.cfg.ReleaseDRBDMinors(disk.uuid) raise feedback_fn("adding instance %s to cluster config" % self.op.instance_name) diff --git a/lib/cmdlib/instance_set_params.py b/lib/cmdlib/instance_set_params.py index 7b3ff9d..9e0009e 100644 --- a/lib/cmdlib/instance_set_params.py +++ b/lib/cmdlib/instance_set_params.py @@ -1219,7 +1219,8 @@ class LUInstanceSetParams(LogicalUnit): disks=new_disks) except errors.OpExecError: self.LogWarning("Device creation failed") - self.cfg.ReleaseDRBDMinors(self.instance.uuid) + for disk in new_disks: + self.cfg.ReleaseDRBDMinors(disk.uuid) raise # Transfer the data from the old to the newly created disks of the instance. @@ -1251,7 +1252,8 @@ class LUInstanceSetParams(LogicalUnit): disks=new_disks) self.LogInfo("Newly created disks removed successfully") finally: - self.cfg.ReleaseDRBDMinors(self.instance.uuid) + for disk in new_disks: + self.cfg.ReleaseDRBDMinors(disk.uuid) result.Raise("Error while converting the instance's template") # In case of DRBD disk, return its port to the pool @@ -1696,7 +1698,8 @@ class LUInstanceSetParams(LogicalUnit): else: self._ConvertInstanceTemplate(feedback_fn) except: - self.cfg.ReleaseDRBDMinors(self.instance.uuid) + for disk in inst_disks: + self.cfg.ReleaseDRBDMinors(disk.uuid) raise result.append(("disk_template", self.op.disk_template)) diff --git a/lib/cmdlib/instance_storage.py b/lib/cmdlib/instance_storage.py index 446d7c0..9354fc1 100644 --- a/lib/cmdlib/instance_storage.py +++ b/lib/cmdlib/instance_storage.py @@ -538,7 +538,7 @@ def CheckRADOSFreeSpace(): def _GenerateDRBD8Branch(lu, primary_uuid, secondary_uuid, size, vgnames, names, - iv_name, p_minor, s_minor, forthcoming=False): + iv_name, forthcoming=False): """Generate a drbd8 device complete with its children. """ @@ -557,15 +557,19 @@ def _GenerateDRBD8Branch(lu, primary_uuid, secondary_uuid, size, vgnames, names, nodes=[primary_uuid, secondary_uuid], params={}, forthcoming=forthcoming) dev_meta.uuid = lu.cfg.GenerateUniqueID(lu.proc.GetECId()) + + drbd_uuid = lu.cfg.GenerateUniqueID(lu.proc.GetECId()) + minors = lu.cfg.AllocateDRBDMinor([primary_uuid, secondary_uuid], drbd_uuid) + assert len(minors) == 2 drbd_dev = objects.Disk(dev_type=constants.DT_DRBD8, size=size, logical_id=(primary_uuid, secondary_uuid, port, - p_minor, s_minor, + minors[0], minors[1], shared_secret), children=[dev_data, dev_meta], nodes=[primary_uuid, secondary_uuid], iv_name=iv_name, params={}, forthcoming=forthcoming) - drbd_dev.uuid = lu.cfg.GenerateUniqueID(lu.proc.GetECId()) + drbd_dev.uuid = drbd_uuid return drbd_dev @@ -588,8 +592,6 @@ def GenerateDiskTemplate( if len(secondary_node_uuids) != 1: raise errors.ProgrammerError("Wrong template configuration") remote_node_uuid = secondary_node_uuids[0] - minors = lu.cfg.AllocateDRBDMinor( - [primary_node_uuid, remote_node_uuid] * len(disk_info), instance_uuid) (drbd_params, _, _) = objects.Disk.ComputeLDParams(template_name, full_disk_params) @@ -609,7 +611,6 @@ def GenerateDiskTemplate( [data_vg, meta_vg], names[idx * 2:idx * 2 + 2], "disk/%d" % disk_index, - minors[idx * 2], minors[idx * 2 + 1], forthcoming=forthcoming) disk_dev.mode = disk[constants.IDISK_MODE] disk_dev.name = disk.get(constants.IDISK_NAME, None) @@ -1018,7 +1019,7 @@ class LUInstanceRecreateDisks(LogicalUnit): # have changed (_, _, old_port, _, _, old_secret) = disk.logical_id new_minors = self.cfg.AllocateDRBDMinor(self.op.node_uuids, - self.instance.uuid) + disk.uuid) new_id = (self.op.node_uuids[0], self.op.node_uuids[1], old_port, new_minors[0], new_minors[1], old_secret) assert len(disk.logical_id) == len(new_id) @@ -2855,9 +2856,10 @@ class TLReplaceDisks(Tasklet): # after this, we must manually remove the drbd minors on both the # error and the success paths self.lu.LogStep(4, steps_total, "Changing drbd configuration") - minors = self.cfg.AllocateDRBDMinor([self.new_node_uuid - for _ in inst_disks], - self.instance.uuid) + minors = [] + for disk in inst_disks: + minor = self.cfg.AllocateDRBDMinor([self.new_node_uuid], disk.uuid) + minors.append(minor) logging.debug("Allocated minors %r", minors) iv_names = {} @@ -2896,7 +2898,8 @@ class TLReplaceDisks(Tasklet): GetInstanceInfoText(self.instance), False, excl_stor) except errors.GenericError: - self.cfg.ReleaseDRBDMinors(self.instance.uuid) + for disk in inst_disks: + self.cfg.ReleaseDRBDMinors(disk.uuid) raise # We have new devices, shutdown the drbd on the old secondary @@ -2917,7 +2920,8 @@ class TLReplaceDisks(Tasklet): msg = result.fail_msg if msg: # detaches didn't succeed (unlikely) - self.cfg.ReleaseDRBDMinors(self.instance.uuid) + for disk in inst_disks: + self.cfg.ReleaseDRBDMinors(disk.uuid) raise errors.OpExecError("Can't detach the disks from the network on" " old node: %s" % (msg,)) diff --git a/lib/config/__init__.py b/lib/config/__init__.py index 16313b1..8ed237e 100644 --- a/lib/config/__init__.py +++ b/lib/config/__init__.py @@ -350,6 +350,7 @@ class ConfigWriter(object): disk.UpgradeConfig() self._ConfigData().disks[disk.uuid] = disk self._ConfigData().cluster.serial_no += 1 + self._UnlockedReleaseDRBDMinors(disk.uuid) def _UnlockedAttachInstanceDisk(self, inst_uuid, disk_uuid, idx=None): """Attach a disk to an instance. @@ -1280,53 +1281,56 @@ class ConfigWriter(object): return dict(map(lambda (k, v): (k, dict(v)), self._wconfd.ComputeDRBDMap())) - def AllocateDRBDMinor(self, node_uuids, inst_uuid): + def AllocateDRBDMinor(self, node_uuids, disk_uuid): """Allocate a drbd minor. This is just a wrapper over a call to WConfd. The free minor will be automatically computed from the existing - devices. A node can be given multiple times in order to allocate - multiple minors. The result is the list of minors, in the same + devices. A node can not be given multiple times. + The result is the list of minors, in the same order as the passed nodes. - @type inst_uuid: string - @param inst_uuid: the instance for which we allocate minors + @type node_uuids: list of strings + @param node_uuids: the nodes in which we allocate minors + @type disk_uuid: string + @param disk_uuid: the disk for which we allocate minors + @rtype: list of ints + @return: A list of minors in the same order as the passed nodes """ - assert isinstance(inst_uuid, basestring), \ - "Invalid argument '%s' passed to AllocateDRBDMinor" % inst_uuid + assert isinstance(disk_uuid, basestring), \ + "Invalid argument '%s' passed to AllocateDRBDMinor" % disk_uuid if self._offline: raise errors.ProgrammerError("Can't call AllocateDRBDMinor" " in offline mode") - result = self._wconfd.AllocateDRBDMinor(inst_uuid, node_uuids) + result = self._wconfd.AllocateDRBDMinor(disk_uuid, node_uuids) logging.debug("Request to allocate drbd minors, input: %s, returning %s", node_uuids, result) return result - def _UnlockedReleaseDRBDMinors(self, inst_uuid): - """Release temporary drbd minors allocated for a given instance. + def _UnlockedReleaseDRBDMinors(self, disk_uuid): + """Release temporary drbd minors allocated for a given disk. This is just a wrapper over a call to WConfd. - @type inst_uuid: string - @param inst_uuid: the instance for which temporary minors should be - released + @type disk_uuid: string + @param disk_uuid: the disk for which temporary minors should be released """ - assert isinstance(inst_uuid, basestring), \ + assert isinstance(disk_uuid, basestring), \ "Invalid argument passed to ReleaseDRBDMinors" # in offline mode we allow the calls to release DRBD minors, # because then nothing can be allocated anyway; # this is useful for testing if not self._offline: - self._wconfd.ReleaseDRBDMinors(inst_uuid) + self._wconfd.ReleaseDRBDMinors(disk_uuid) @ConfigSync() - def ReleaseDRBDMinors(self, inst_uuid): - """Release temporary drbd minors allocated for a given instance. + def ReleaseDRBDMinors(self, disk_uuid): + """Release temporary drbd minors allocated for a given disk. This should be called on the error paths, on the success paths it's automatically called by the ConfigWriter add and update @@ -1334,12 +1338,11 @@ class ConfigWriter(object): This function is just a wrapper over L{_UnlockedReleaseDRBDMinors}. - @type inst_uuid: string - @param inst_uuid: the instance for which temporary minors should be - released + @type disk_uuid: string + @param disk_uuid: the disk for which temporary minors should be released """ - self._UnlockedReleaseDRBDMinors(inst_uuid) + self._UnlockedReleaseDRBDMinors(disk_uuid) @ConfigSync(shared=1) def GetConfigVersion(self): @@ -3226,7 +3229,7 @@ class ConfigWriter(object): self._ConfigData().cluster.serial_no += 1 self._ConfigData().cluster.mtime = now - if isinstance(target, objects.Instance): + if isinstance(target, objects.Disk): self._UnlockedReleaseDRBDMinors(target.uuid) if ec_id is not None: diff --git a/src/Ganeti/Config.hs b/src/Ganeti/Config.hs index 340be97..5be157b 100644 --- a/src/Ganeti/Config.hs +++ b/src/Ganeti/Config.hs @@ -66,6 +66,7 @@ module Ganeti.Config , getInstAllNodes , getInstDisks , getInstDisksFromObj + , getDrbdMinorsForDisk , getDrbdMinorsForInstance , getFilledInstHvParams , getFilledInstBeParams diff --git a/src/Ganeti/WConfd/Core.hs b/src/Ganeti/WConfd/Core.hs index 59ce9c3..1c00463 100644 --- a/src/Ganeti/WConfd/Core.hs +++ b/src/Ganeti/WConfd/Core.hs @@ -150,22 +150,22 @@ computeDRBDMap = uncurry T.computeDRBDMap =<< readTempResState -- Allocate a drbd minor. -- -- The free minor will be automatically computed from the existing devices. --- A node can be given multiple times in order to allocate multiple minors. +-- A node can not be given multiple times. -- The result is the list of minors, in the same order as the passed nodes. allocateDRBDMinor - :: T.InstanceUUID -> [T.NodeUUID] -> WConfdMonad [T.DRBDMinor] -allocateDRBDMinor inst nodes = - modifyTempResStateErr (\cfg -> T.allocateDRBDMinor cfg inst nodes) + :: T.DiskUUID -> [T.NodeUUID] -> WConfdMonad [T.DRBDMinor] +allocateDRBDMinor disk nodes = + modifyTempResStateErr (\cfg -> T.allocateDRBDMinor cfg disk nodes) --- Release temporary drbd minors allocated for a given instance using +-- Release temporary drbd minors allocated for a given disk using -- 'allocateDRBDMinor'. -- -- This should be called on the error paths, on the success paths -- it's automatically called by the ConfigWriter add and update -- functions. releaseDRBDMinors - :: T.InstanceUUID -> WConfdMonad () -releaseDRBDMinors inst = modifyTempResState (const $ T.releaseDRBDMinors inst) + :: T.DiskUUID -> WConfdMonad () +releaseDRBDMinors disk = modifyTempResState (const $ T.releaseDRBDMinors disk) -- *** MACs diff --git a/src/Ganeti/WConfd/TempRes.hs b/src/Ganeti/WConfd/TempRes.hs index 020aee8..ef152ea 100644 --- a/src/Ganeti/WConfd/TempRes.hs +++ b/src/Ganeti/WConfd/TempRes.hs @@ -44,6 +44,7 @@ module Ganeti.WConfd.TempRes , emptyTempResState , NodeUUID , InstanceUUID + , DiskUUID , NetworkUUID , DRBDMinor , DRBDMap @@ -111,15 +112,17 @@ type NodeUUID = String type InstanceUUID = String +type DiskUUID = String + type NetworkUUID = String type DRBDMinor = Int -- | A map of the usage of DRBD minors -type DRBDMap = Map NodeUUID (Map DRBDMinor InstanceUUID) +type DRBDMap = Map NodeUUID (Map DRBDMinor DiskUUID) -- | A map of the usage of DRBD minors with possible duplicates -type DRBDMap' = Map NodeUUID (Map DRBDMinor [InstanceUUID]) +type DRBDMap' = Map NodeUUID (Map DRBDMinor [DiskUUID]) -- * The state data structure @@ -218,16 +221,15 @@ computeDRBDMap' :: (MonadError GanetiException m) => ConfigData -> TempResState -> m DRBDMap' computeDRBDMap' cfg trs = flip execStateT (fmap (fmap (: [])) (trsDRBD trs)) - $ F.forM_ (configInstances cfg) addDisks + $ F.forM_ (configDisks cfg) addMinors where -- | Creates a lens for modifying the list of instances - nodeMinor :: NodeUUID -> DRBDMinor -> Lens' DRBDMap' [InstanceUUID] + nodeMinor :: NodeUUID -> DRBDMinor -> Lens' DRBDMap' [DiskUUID] nodeMinor node minor = maybeLens (at node) . maybeLens (at minor) - -- | Adds disks of an instance within the state monad - addDisks inst = do - disks <- toError $ getDrbdMinorsForInstance cfg inst - forM_ disks $ \(minor, node) -> nodeMinor node minor - %= (uuidOf inst :) + -- | Adds minors of a disk within the state monad + addMinors disk = do + let minors = getDrbdMinorsForDisk disk + forM_ minors $ \(minor, node) -> nodeMinor node minor %= (uuidOf disk :) -- | Compute the map of used DRBD minor/nodes. -- Report any duplicate entries as an error. @@ -246,25 +248,27 @@ computeDRBDMap cfg trs = do -- Allocate a drbd minor. -- -- The free minor will be automatically computed from the existing devices. --- A node can be given multiple times in order to allocate multiple minors. +-- A node can not be given multiple times. -- The result is the list of minors, in the same order as the passed nodes. allocateDRBDMinor :: (MonadError GanetiException m, MonadState TempResState m) - => ConfigData -> InstanceUUID -> [NodeUUID] + => ConfigData -> DiskUUID -> [NodeUUID] -> m [DRBDMinor] -allocateDRBDMinor cfg inst nodes = do +allocateDRBDMinor cfg disk nodes = do + unless (nodes == ordNub nodes) . resError + $ "Duplicate nodes detected in list '" ++ show nodes ++ "'" dMap <- computeDRBDMap' cfg =<< get let usedMap = fmap M.keysSet dMap - let alloc :: S.Set DRBDMinor -> Map DRBDMinor InstanceUUID - -> (DRBDMinor, Map DRBDMinor InstanceUUID) + let alloc :: S.Set DRBDMinor -> Map DRBDMinor DiskUUID + -> (DRBDMinor, Map DRBDMinor DiskUUID) alloc used m = let k = findFirst 0 (M.keysSet m `S.union` used) - in (k, M.insert k inst m) + in (k, M.insert k disk m) forM nodes $ \node -> trsDRBDL . maybeLens (at node) %%= alloc (M.findWithDefault mempty node usedMap) --- Release temporary drbd minors allocated for a given instance using +-- Release temporary drbd minors allocated for a given disk using -- 'allocateDRBDMinor'. -releaseDRBDMinors :: (MonadState TempResState m) => InstanceUUID -> m () -releaseDRBDMinors inst = trsDRBDL %= filterNested (/= inst) +releaseDRBDMinors :: (MonadState TempResState m) => DiskUUID -> m () +releaseDRBDMinors disk = trsDRBDL %= filterNested (/= disk) -- * Other temporary resources diff --git a/test/py/cmdlib/instance_unittest.py b/test/py/cmdlib/instance_unittest.py index 7a6e258..bc44dd4 100644 --- a/test/py/cmdlib/instance_unittest.py +++ b/test/py/cmdlib/instance_unittest.py @@ -1133,7 +1133,7 @@ class _FakeConfigForGenDiskTemplate(ConfigMock): def GenerateUniqueID(self, ec_id): return "ec%s-uq%s" % (ec_id, self._unique_id.next()) - def AllocateDRBDMinor(self, nodes, instance): + def AllocateDRBDMinor(self, nodes, disk): return [self._drbd_minor.next() for _ in nodes] diff --git a/test/py/testutils/config_mock.py b/test/py/testutils/config_mock.py index fed7362..62b02e5 100644 --- a/test/py/testutils/config_mock.py +++ b/test/py/testutils/config_mock.py @@ -581,10 +581,10 @@ class ConfigMock(config.ConfigWriter): def ComputeDRBDMap(self): return dict((node_uuid, {}) for node_uuid in self._ConfigData().nodes) - def AllocateDRBDMinor(self, node_uuids, inst_uuid): + def AllocateDRBDMinor(self, node_uuids, disk_uuid): return map(lambda _: 0, node_uuids) - def _UnlockedReleaseDRBDMinors(self, inst_uuid): + def _UnlockedReleaseDRBDMinors(self, disk_uuid): pass def _CreateConfig(self): -- 1.7.10.4
