LGTM. Thanks, Jose
On Apr 16 15:19, Ilias Tsitsimpis wrote: > Use 'GetInstanceLVsByNode' from config file instead of Instance's > MapLVsByNode method. > > Also remove all_lvs computation from config's 'AddInstance' method. In > order to compute the lvs we have to use the 'GetInstanceLVsByNode' > method but the instance object doesn't exist in the config yet. In > addition, the computation of all_lvs will be obsolete once the disks > object will be separated from the instance object, since when calling > 'AddInstance' (adding an instance to the config file), the instance will > not have attached any disks. > > Signed-off-by: Ilias Tsitsimpis <[email protected]> > --- > lib/cmdlib/cluster.py | 6 +++--- > lib/cmdlib/common.py | 13 ++++++++----- > lib/cmdlib/group.py | 1 + > lib/cmdlib/node.py | 2 +- > lib/config.py | 6 +----- > lib/objects.py | 51 > --------------------------------------------------- > 6 files changed, 14 insertions(+), 65 deletions(-) > > diff --git a/lib/cmdlib/cluster.py b/lib/cmdlib/cluster.py > index 63f2a6f..9f0dd3a 100644 > --- a/lib/cmdlib/cluster.py > +++ b/lib/cmdlib/cluster.py > @@ -2312,7 +2312,7 @@ class LUClusterVerifyGroup(LogicalUnit, _VerifyErrors): > groupinfo = self.cfg.GetAllNodeGroupsInfo() > > node_vol_should = {} > - instance.MapLVsByNode(node_vol_should) > + self.cfg.GetInstanceLVsByNode(instance.uuid, lvmap=node_vol_should) > > cluster = self.cfg.GetClusterInfo() > ipolicy = ganeti.masterd.instance.CalculateGroupIPolicy(cluster, > @@ -3344,7 +3344,7 @@ class LUClusterVerifyGroup(LogicalUnit, _VerifyErrors): > gnode.ghost = (nuuid not in self.all_node_info) > node_image[nuuid] = gnode > > - instance.MapLVsByNode(node_vol_should) > + self.cfg.GetInstanceLVsByNode(instance.uuid, lvmap=node_vol_should) > > pnode = instance.primary_node > node_image[pnode].pinst.append(instance.uuid) > @@ -3569,7 +3569,7 @@ class LUClusterVerifyGroup(LogicalUnit, _VerifyErrors): > for secondary in self.cfg.GetInstanceSecondaryNodes(instance.uuid): > if (secondary in self.my_node_info > and instance.name not in self.my_inst_info): > - instance.MapLVsByNode(node_vol_should) > + self.cfg.GetInstanceLVsByNode(instance.uuid, lvmap=node_vol_should) > break > > self._VerifyOrphanVolumes(node_vol_should, node_image, reserved) > diff --git a/lib/cmdlib/common.py b/lib/cmdlib/common.py > index 5e4dc52..3353e7e 100644 > --- a/lib/cmdlib/common.py > +++ b/lib/cmdlib/common.py > @@ -953,18 +953,21 @@ def _SetOpEarlyRelease(early_release, op): > return op > > > -def MapInstanceLvsToNodes(instances): > +def MapInstanceLvsToNodes(cfg, instances): > """Creates a map from (node, volume) to instance name. > > + @type cfg: L{config.ConfigWriter} > + @param cfg: The cluster configuration > @type instances: list of L{objects.Instance} > @rtype: dict; tuple of (node uuid, volume name) as key, L{objects.Instance} > object as value > > """ > - return dict(((node_uuid, vol), inst) > - for inst in instances > - for (node_uuid, vols) in inst.MapLVsByNode().items() > - for vol in vols) > + return dict( > + ((node_uuid, vol), inst) > + for inst in instances > + for (node_uuid, vols) in cfg.GetInstanceLVsByNode(inst.uuid).items() > + for vol in vols) > > > def CheckParamsNotGlobal(params, glob_pars, kind, bad_levels, good_levels): > diff --git a/lib/cmdlib/group.py b/lib/cmdlib/group.py > index e974ead..eff1407 100644 > --- a/lib/cmdlib/group.py > +++ b/lib/cmdlib/group.py > @@ -874,6 +874,7 @@ class LUGroupVerifyDisks(NoHooksLU): > def _VerifyInstanceLvs(self, node_errors, offline_disk_instance_names, > missing_disks): > node_lv_to_inst = MapInstanceLvsToNodes( > + self.cfg, > [inst for inst in self.instances.values() if inst.disks_active]) > if node_lv_to_inst: > node_uuids = utils.NiceSort(set(self.owned_locks(locking.LEVEL_NODE)) & > diff --git a/lib/cmdlib/node.py b/lib/cmdlib/node.py > index eb59f6f..17007d1 100644 > --- a/lib/cmdlib/node.py > +++ b/lib/cmdlib/node.py > @@ -1246,7 +1246,7 @@ class LUNodeQueryvols(NoHooksLU): > volumes = self.rpc.call_node_volumes(node_uuids) > > ilist = self.cfg.GetAllInstancesInfo() > - vol2inst = MapInstanceLvsToNodes(ilist.values()) > + vol2inst = MapInstanceLvsToNodes(self.cfg, ilist.values()) > > output = [] > for node_uuid in node_uuids: > diff --git a/lib/config.py b/lib/config.py > index 6d84c8c..54df32e 100644 > --- a/lib/config.py > +++ b/lib/config.py > @@ -660,7 +660,7 @@ class ConfigWriter(object): > """ > lvnames = set() > for instance in self._ConfigData().instances.values(): > - node_data = instance.MapLVsByNode() > + node_data = self._UnlockedGetInstanceLVsByNode(instance.uuid) > for lv_list in node_data.values(): > lvnames.update(lv_list) > return lvnames > @@ -1692,10 +1692,6 @@ class ConfigWriter(object): > if not isinstance(instance, objects.Instance): > raise errors.ProgrammerError("Invalid type passed to AddInstance") > > - if instance.disk_template != constants.DT_DISKLESS: > - all_lvs = instance.MapLVsByNode() > - logging.info("Instance '%s' DISK_LAYOUT: %s", instance.name, all_lvs) > - > all_macs = self._AllMACs() > for nic in instance.nics: > if nic.mac in all_macs: > diff --git a/lib/objects.py b/lib/objects.py > index 9d0f569..21890f4 100644 > --- a/lib/objects.py > +++ b/lib/objects.py > @@ -1158,57 +1158,6 @@ class Instance(TaggableObject): > "serial_no", > ] + _TIMESTAMPS + _UUID > > - 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_uuid is None: > - node_uuid = self.primary_node > - > - if lvmap is None: > - lvmap = { > - node_uuid: [], > - } > - ret = lvmap > - else: > - if not node_uuid in lvmap: > - lvmap[node_uuid] = [] > - ret = None > - > - if not devs: > - devs = self.disks > - > - for dev in devs: > - if dev.dev_type == constants.DT_PLAIN: > - lvmap[node_uuid].append(dev.logical_id[0] + "/" + dev.logical_id[1]) > - > - elif dev.dev_type in constants.DTS_DRBD: > - if dev.children: > - self.MapLVsByNode(lvmap, dev.children, dev.logical_id[0]) > - self.MapLVsByNode(lvmap, dev.children, dev.logical_id[1]) > - > - elif dev.children: > - self.MapLVsByNode(lvmap, dev.children, node_uuid) > - > - return ret > - > def FindDisk(self, idx): > """Find a disk given having a specified index. > > -- > 1.9.1 > -- 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
