On Mon, Jan 10, 2011 at 14:06 +0000, Michael Hanselmann <[email protected]>
wrote:
> Am 7. Januar 2011 15:49 schrieb Adeodato Simo <[email protected]>:
> > + if inst_config.disk_template in constants.DTS_NET_MIRROR:
> > + pnode = inst_config.primary_node
> > + instance_nodes = [pnode] +
> > utils.NiceSort(inst_config.secondary_nodes)
> Is there a specific reason why you're not using inst_config.all_nodes?
Hm, that I hadn't noticed it was available. Changed. :-)
> > + instance_groups = {}
> > +
> > + for node in instance_nodes:
> > + nodes = instance_groups.setdefault(nodeinfo_byname[node].group,
> > [])
> > + nodes.append(node)
> This looks very confusing until one reads “setdefault”. Can't you
> rewrite it like the following?
I did it to improve wrapping, but if it decreases readability, let's
change it back (see below).
> Try to avoid magic indexes: lambda (key, _): pnode in key
Done.
> > + self._Error(self.EINSTANCESPLITGROUPS, instance,
> Please use ErrorIf with the condition above (“len(instance_groups) >
> 1”).
Done.
diff --git lib/cmdlib.py lib/cmdlib.py
index 451ab7e..6f2408d 100644
--- lib/cmdlib.py
+++ lib/cmdlib.py
@@ -2266,24 +2266,23 @@ class LUVerifyCluster(LogicalUnit):
if inst_config.disk_template in constants.DTS_NET_MIRROR:
pnode = inst_config.primary_node
- instance_nodes = [pnode] + utils.NiceSort(inst_config.secondary_nodes)
+ instance_nodes = utils.NiceSort(inst_config.all_nodes)
instance_groups = {}
for node in instance_nodes:
- nodes = instance_groups.setdefault(nodeinfo_byname[node].group, [])
- nodes.append(node)
+ instance_groups.setdefault(nodeinfo_byname[node].group,
+ []).append(node)
- if len(instance_groups) > 1:
- pretty_node_groups = [
- "%s (group %s)" % (utils.CommaJoin(nodes), groupinfo[group].name)
- # Sort so that we always list the primary node first.
- for group, nodes in sorted(instance_groups.items(),
- key=lambda x: pnode in x[1],
- reverse=True)]
+ pretty_list = [
+ "%s (group %s)" % (utils.CommaJoin(nodes), groupinfo[group].name)
+ # Sort so that we always list the primary node first.
+ for group, nodes in sorted(instance_groups.items(),
+ key=lambda (_, nodes): pnode in nodes,
+ reverse=True)]
- self._Error(self.EINSTANCESPLITGROUPS, instance,
- "instance has primary and secondary nodes in different"
- " groups: %s", utils.CommaJoin(pretty_node_groups),
+ self._ErrorIf(len(instance_groups) > 1, self.EINSTANCESPLITGROUPS,
+ instance, "instance has primary and secondary nodes in"
+ " different groups: %s", utils.CommaJoin(pretty_list),
code=self.ETYPE_WARNING)
if not cluster.FillBE(inst_config)[constants.BE_AUTO_BALANCE]:
Thanks for the reviews,
--
Adeodato Simo | [email protected]
Corp Computing Services SRE (Dublin)
