commit 843094ad8639b29419a3668e4e8208f643bafe17
Author: Thomas Thrainer <thomasth@google.com>
Date:   Mon Jul 1 08:08:19 2013 +0200

    Make names more descriptive
    
    LUGroupVerifyDisks had some not very descriptive names which made it
    hard to read and understand the code. This patch renames some methods
    and variables to make it more readable.
    
    Also, the return type of MapInstanceLvsToNodes was changed to contain
    Instance objects instead of instance names. This enables easier
    processing of the result.
    
    Signed-off-by: Thomas Thrainer <thomasth@google.com>
    Reviewed-by: Guido Trotter <ultrotter@google.com>

diff --git a/lib/cmdlib/common.py b/lib/cmdlib/common.py
index 7621f63..4bcc65e 100644
--- a/lib/cmdlib/common.py
+++ b/lib/cmdlib/common.py
@@ -907,14 +907,15 @@ def _SetOpEarlyRelease(early_release, op):
   return op
 
 
-def MapInstanceDisksToNodes(instances):
+def MapInstanceLvsToNodes(instances):
   """Creates a map from (node, volume) to instance name.
 
   @type instances: list of L{objects.Instance}
-  @rtype: dict; tuple of (node uuid, volume name) as key, instance name as value
+  @rtype: dict; tuple of (node uuid, volume name) as key, L{objects.Instance}
+          object as value
 
   """
-  return dict(((node_uuid, vol), inst.name)
+  return dict(((node_uuid, vol), inst)
               for inst in instances
               for (node_uuid, vols) in inst.MapLVsByNode().items()
               for vol in vols)
diff --git a/lib/cmdlib/group.py b/lib/cmdlib/group.py
index 8a15234..d4610b1 100644
--- a/lib/cmdlib/group.py
+++ b/lib/cmdlib/group.py
@@ -37,7 +37,7 @@ from ganeti.cmdlib.common import MergeAndVerifyHvState, \
   MergeAndVerifyDiskState, GetWantedNodes, GetUpdatedParams, \
   CheckNodeGroupInstances, GetUpdatedIPolicy, \
   ComputeNewInstanceViolations, GetDefaultIAllocator, ShareAll, \
-  CheckInstancesNodeGroups, LoadNodeEvacResult, MapInstanceDisksToNodes
+  CheckInstancesNodeGroups, LoadNodeEvacResult, MapInstanceLvsToNodes
 
 import ganeti.masterd.instance
 
@@ -911,14 +911,14 @@ class LUGroupVerifyDisks(NoHooksLU):
         missing volumes
 
     """
-    res_nodes = {}
-    res_instances = set()
-    res_missing = {}
+    node_errors = {}
+    offline_lv_instance_names = set()
+    missing_lvs = {}
 
-    nv_dict = MapInstanceDisksToNodes(
+    node_lv_to_inst = MapInstanceLvsToNodes(
       [inst for inst in self.instances.values() if inst.disks_active])
 
-    if nv_dict:
+    if node_lv_to_inst:
       node_uuids = utils.NiceSort(set(self.owned_locks(locking.LEVEL_NODE)) &
                                   set(self.cfg.GetVmCapableNodeList()))
 
@@ -932,17 +932,17 @@ class LUGroupVerifyDisks(NoHooksLU):
         if msg:
           logging.warning("Error enumerating LVs on node %s: %s",
                           self.cfg.GetNodeName(node_uuid), msg)
-          res_nodes[node_uuid] = msg
+          node_errors[node_uuid] = msg
           continue
 
         for lv_name, (_, _, lv_online) in node_res.payload.items():
-          inst = nv_dict.pop((node_uuid, lv_name), None)
-          if not (lv_online or inst is None):
-            res_instances.add(inst)
+          inst = node_lv_to_inst.pop((node_uuid, lv_name), None)
+          if not lv_online and inst is not None:
+            offline_lv_instance_names.add(inst.name)
 
       # any leftover items in nv_dict are missing LVs, let's arrange the data
       # better
-      for key, inst in nv_dict.iteritems():
-        res_missing.setdefault(inst, []).append(list(key))
+      for key, inst in node_lv_to_inst.iteritems():
+        missing_lvs.setdefault(inst.name, []).append(list(key))
 
-    return (res_nodes, list(res_instances), res_missing)
+    return (node_errors, list(offline_lv_instance_names), missing_lvs)
diff --git a/lib/cmdlib/node.py b/lib/cmdlib/node.py
index 234b85a..e61a8b9 100644
--- a/lib/cmdlib/node.py
+++ b/lib/cmdlib/node.py
@@ -44,7 +44,7 @@ from ganeti.cmdlib.common import CheckParamsNotGlobal, \
   RedistributeAncillaryFiles, ExpandNodeUuidAndName, ShareAll, SupportsOob, \
   CheckInstanceState, INSTANCE_DOWN, GetUpdatedParams, \
   AdjustCandidatePool, CheckIAllocatorOrNode, LoadNodeEvacResult, \
-  GetWantedNodes, MapInstanceDisksToNodes, RunPostHook, \
+  GetWantedNodes, MapInstanceLvsToNodes, RunPostHook, \
   FindFaultyInstanceDisks
 
 
@@ -1316,7 +1316,7 @@ class LUNodeQueryvols(NoHooksLU):
     volumes = self.rpc.call_node_volumes(node_uuids)
 
     ilist = self.cfg.GetAllInstancesInfo()
-    vol2inst = MapInstanceDisksToNodes(ilist.values())
+    vol2inst = MapInstanceLvsToNodes(ilist.values())
 
     output = []
     for node_uuid in node_uuids:
@@ -1346,7 +1346,12 @@ class LUNodeQueryvols(NoHooksLU):
           elif field == "size":
             val = int(float(vol["size"]))
           elif field == "instance":
-            val = vol2inst.get((node_uuid, vol["vg"] + "/" + vol["name"]), "-")
+            inst = vol2inst.get((node_uuid, vol["vg"] + "/" + vol["name"]),
+                                None)
+            if inst is not None:
+              val = inst.name
+            else:
+              val = "-"
           else:
             raise errors.ParameterError(field)
           node_output.append(str(val))
diff --git a/lib/objects.py b/lib/objects.py
index 8fdce13..d1891f8 100644
--- a/lib/objects.py
+++ b/lib/objects.py
@@ -1108,32 +1108,38 @@ class Instance(TaggableObject):
   all_nodes = property(_ComputeAllNodes, None, None,
                        "List of names of all the nodes of the instance")
 
-  def MapLVsByNode(self, lvmap=None, devs=None, node=None):
+  def MapLVsByNode(self, lvmap=None, devs=None, node_uuid=None):
     """Provide a mapping of nodes to LVs this instance owns.
 
     This function figures out what logical volumes should belong on
     which nodes, recursing through a device tree.
 
+    @type lvmap: dict
     @param lvmap: optional dictionary to receive the
         'node' : ['lv', ...] data.
-
+    @type devs: list of L{Disk}
+    @param devs: disks to get the LV name for. If None, all disk of this
+        instance are used.
+    @type node_uuid: string
+    @param node_uuid: UUID of the node to get the LV names for. If None, the
+        primary node of this instance is used.
     @return: None if lvmap arg is given, otherwise, a dictionary of
         the form { 'node_uuid' : ['volume1', 'volume2', ...], ... };
         volumeN is of the form "vg_name/lv_name", compatible with
         GetVolumeList()
 
     """
-    if node is None:
-      node = self.primary_node
+    if node_uuid is None:
+      node_uuid = self.primary_node
 
     if lvmap is None:
       lvmap = {
-        node: [],
+        node_uuid: [],
         }
       ret = lvmap
     else:
-      if not node in lvmap:
-        lvmap[node] = []
+      if not node_uuid in lvmap:
+        lvmap[node_uuid] = []
       ret = None
 
     if not devs:
@@ -1141,7 +1147,7 @@ class Instance(TaggableObject):
 
     for dev in devs:
       if dev.dev_type == constants.LD_LV:
-        lvmap[node].append(dev.logical_id[0] + "/" + dev.logical_id[1])
+        lvmap[node_uuid].append(dev.logical_id[0] + "/" + dev.logical_id[1])
 
       elif dev.dev_type in constants.LDS_DRBD:
         if dev.children:
@@ -1149,7 +1155,7 @@ class Instance(TaggableObject):
           self.MapLVsByNode(lvmap, dev.children, dev.logical_id[1])
 
       elif dev.children:
-        self.MapLVsByNode(lvmap, dev.children, node)
+        self.MapLVsByNode(lvmap, dev.children, node_uuid)
 
     return ret
 
