Thanks for the patch!
On Thu, Nov 06, 2014 at 12:30:36AM +0200, Alex Pyrgiotis wrote:
> 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.py b/lib/cmdlib/cluster.py
> index 2c229c4..f932386 100644
> --- a/lib/cmdlib/cluster.py
> +++ b/lib/cmdlib/cluster.py
> @@ -2175,6 +2175,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])
> @@ -2913,14 +2914,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}
> @@ -2930,18 +2932,22 @@ 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, False)
> else:
> - instance = instanceinfo[inst_uuid]
> - node_drbd[minor] = (inst_uuid, instance.disks_active)
> + disk_active = False
> + for (inst_uuid, inst) in instanceinfo.items():
> + if disk_uuid in inst.disks:
> + disk_active = inst.disks_active
> + break
> + node_drbd[minor] = (disk_uuid, disk_active)
This seems to be something useful to have in other contexts too. Can you
extract this?
>
> # and now check them
> used_minors = nresult.get(constants.NV_DRBDLIST, [])
> @@ -2952,11 +2958,10 @@ class LUClusterVerifyGroup(LogicalUnit,
_VerifyErrors):
> # we cannot check drbd status
> return
>
> - for minor, (inst_uuid, must_exist) in node_drbd.items():
> + for minor, (disk_uuid, must_exist) in node_drbd.items():
> test = minor not in used_minors and must_exist
> 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 is not active", minor,
disk_uuid)
Can you also provide the instance (if any) in the error message? A user
wouldn't want to check all disks.
> for minor in used_minors:
> test = minor not in node_drbd
> self._ErrorIf(test, constants.CV_ENODEDRBD, ninfo.name,
> @@ -3768,8 +3773,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 5d9c6f0..b8e23b5 100644
> --- a/lib/cmdlib/instance.py
> +++ b/lib/cmdlib/instance.py
> @@ -459,7 +459,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 = []
> @@ -492,7 +493,8 @@ class LUInstanceMove(LogicalUnit):
> try:
> RemoveDisks(self, self.instance, target_node_uuid=target_node.u
uid)
> 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 a5201f3..fe23f22 100644
> --- a/lib/cmdlib/instance_create.py
> +++ b/lib/cmdlib/instance_create.py
> @@ -1452,7 +1452,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 f6fd4bf..37ece5b 100644
> --- a/lib/cmdlib/instance_set_params.py
> +++ b/lib/cmdlib/instance_set_params.py
> @@ -1223,7 +1223,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.
> @@ -1255,7 +1256,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
> @@ -1700,7 +1702,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 1489e0f..dedbba8 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):
> + iv_name):
> """Generate a drbd8 device complete with its children.
>
> """
> @@ -557,14 +557,18 @@ def _GenerateDRBD8Branch(lu, primary_uuid,
secondary_uuid, size, vgnames, names,
> nodes=[primary_uuid, secondary_uuid],
> params={})
> 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={})
> - drbd_dev.uuid = lu.cfg.GenerateUniqueID(lu.proc.GetECId())
> + drbd_dev.uuid = drbd_uuid
> return drbd_dev
>
>
> @@ -587,8 +591,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)
> @@ -607,8 +609,7 @@ def GenerateDiskTemplate(
> disk[constants.IDISK_SIZE],
> [data_vg, meta_vg],
> names[idx * 2:idx * 2 + 2],
> - "disk/%d" % disk_index,
> - minors[idx * 2], minors[idx * 2 +
1])
> + "disk/%d" % disk_index)
> disk_dev.mode = disk[constants.IDISK_MODE]
> disk_dev.name = disk.get(constants.IDISK_NAME, None)
> disks.append(disk_dev)
> @@ -1009,7 +1010,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)
> @@ -2776,9 +2777,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 = {}
> @@ -2817,7 +2819,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
> @@ -2838,7 +2841,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.py b/lib/config.py
> index 09ea190..9ad33fe 100644
> --- a/lib/config.py
> +++ b/lib/config.py
> @@ -450,6 +450,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.
> @@ -1379,53 +1380,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
> @@ -1433,12 +1437,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):
> @@ -1888,7 +1891,6 @@ class ConfigWriter(object):
> instance.ctime = instance.mtime = time.time()
> self._ConfigData().instances[instance.uuid] = instance
> self._ConfigData().cluster.serial_no += 1
> - self._UnlockedReleaseDRBDMinors(instance.uuid)
> # FIXME: After RemoveInstance is moved to WConfd, use its internal
> # function from TempRes module instead.
> self._UnlockedCommitTemporaryIps(ec_id)
> @@ -3309,7 +3311,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 d31b23e..5d9c0e0 100644
> --- a/src/Ganeti/Config.hs
> +++ b/src/Ganeti/Config.hs
> @@ -64,6 +64,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 e3de0e0..492af60 100644
> --- a/src/Ganeti/WConfd/Core.hs
> +++ b/src/Ganeti/WConfd/Core.hs
> @@ -148,22 +148,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..fdd6594 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.
Am I understanding this correctly that this was an error in the
documentation?
> -- 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)
Indent!
> 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/cmdlib/testsupport/config_mock.py
b/test/py/cmdlib/testsupport/config_mock.py
> index 85375af..984a14d 100644
> --- a/test/py/cmdlib/testsupport/config_mock.py
> +++ b/test/py/cmdlib/testsupport/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
>
Otherwise LGTM
--
Google Germany GmbH
Dienerstr. 12
80331 München
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores
