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?
> + 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?
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],
Try to avoid magic indexes: lambda (key, _): pnode in key
> + reverse=True)]
> +
> + self._Error(self.EINSTANCESPLITGROUPS, instance,
Please use ErrorIf with the condition above (“len(instance_groups) > 1”).
> + "instance has primary and secondary nodes in different"
> + " groups: %s", utils.CommaJoin(pretty_node_groups),
> + code=self.ETYPE_WARNING)
Michael
