> diff --git a/lib/cmdlib/cluster.py b/lib/cmdlib/cluster.py
> index e352ce4..52b1cd4 100644
> --- a/lib/cmdlib/cluster.py
> +++ b/lib/cmdlib/cluster.py
> @@ -538,8 +538,6 @@ class LUClusterRepairDiskSizes(NoHooksLU):
> continue
>
> newl = [v[2].Copy() for v in dskl]
> - for dsk in newl:
> - self.cfg.SetDiskID(dsk, node_uuid)
> instance = dskl[0][0]
> node_name = self.cfg.GetNodeName(node_uuid)
> result = self.rpc.call_blockdev_getdimensions(node_uuid,
> @@ -2697,7 +2695,6 @@ class LUClusterVerifyGroup(LogicalUnit, _VerifyErrors):
> for (inst_uuid, dev) in disks:
> (anno_disk,) = AnnotateDiskParams(instanceinfo[inst_uuid], [dev],
> self.cfg)
> - self.cfg.SetDiskID(anno_disk, nuuid)
> dev_inst_only.append((anno_disk, instanceinfo[inst_uuid]))
Perhaps a 'map' or list comprehension instead of 'empty list', 'for',
'append'.
>
> node_disks_dev_inst_only[nuuid] = dev_inst_only
> diff --git a/lib/cmdlib/common.py b/lib/cmdlib/common.py
> index d1e428f..80334e9 100644
> --- a/lib/cmdlib/common.py
> +++ b/lib/cmdlib/common.py
> @@ -1036,9 +1036,6 @@ def CheckIAllocatorOrNode(lu, iallocator_slot,
> node_slot):
> def FindFaultyInstanceDisks(cfg, rpc_runner, instance, node_uuid, prereq):
> faulty = []
This can be moved closer to where it's used (i.e., a few lines below).
And the 'for' code that uses 'faulty' can be rewritten as a list
comprehension. What do you think ?
>
> - for dev in instance.disks:
> - cfg.SetDiskID(dev, node_uuid)
> -
> result = rpc_runner.call_blockdev_getmirrorstatus(
> node_uuid, (instance.disks, instance))
> result.Raise("Failed to get disk status from node %s" %
> diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py
> index 7d33cbe..005004b 100644
> --- a/lib/cmdlib/instance.py
> +++ b/lib/cmdlib/instance.py
> @@ -1272,7 +1272,6 @@ class LUInstanceCreate(LogicalUnit):
> for t_dsk, a_dsk in zip(tmp_disks, self.disks):
> rename_to.append(t_dsk.logical_id)
> t_dsk.logical_id = (t_dsk.logical_id[0],
> a_dsk[constants.IDISK_ADOPT])
> - self.cfg.SetDiskID(t_dsk, self.pnode.uuid)
Given that we remove the call to 'SetDiskID', does the previous
assignment to 't_dsk' still make sense ?
If so, is it possible to make to make the assignment more clear, that
is, instead of doing the assignment
> @@ -572,15 +570,10 @@ class ConfigWriter(object):
> result.append("duplicate logical id %s" % str(disk.logical_id))
> else:
> l_ids.append(disk.logical_id)
This piece of code ^ is of the form
if ... :
if ...:
...
else:
...
Can we use 'and' ?
if ... and ...:
...
else:
...
Thanks,
Jose
--
Jose Antonio Lopes
Ganeti Engineering
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
Steuernummer: 48/725/00206
Umsatzsteueridentifikationsnummer: DE813741370
