On Thu, Apr 17, 2014 at 12:55PM, Jose A. Lopes wrote: > On Apr 16 15:19, Ilias Tsitsimpis wrote: > > Provide a mapping of node to LVs a given instance owns. > > > > Signed-off-by: Ilias Tsitsimpis <[email protected]> > > --- > > lib/config.py | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 56 insertions(+) > > > > diff --git a/lib/config.py b/lib/config.py > > index 7147b50..9e01cd5 100644 > > --- a/lib/config.py > > +++ b/lib/config.py > > @@ -408,6 +408,62 @@ class ConfigWriter(object): > > """ > > return self._UnlockedGetInstanceSecondaryNodes(inst_uuid) > > > > + def _UnlockedGetInstanceLVsByNode(self, inst_uuid, lvmap=None): > > + """Provide a mapping of node to LVs a given instance owns. > > + > > + @type inst_uuid: string > > + @param inst_uuid: The UUID of the instance we want to > > + compute the LVsByNode for > > + @type lvmap: dict > > + @param lvmap: Optional dictionary to receive the > > + 'node' : ['lv', ...] data. > > + @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() > > + > > + """ > > + def _MapLVsByNode(lvmap, devices, node_uuid): > > + """Recursively helper function.""" > > Maybe you meant this? > > s/Recursively/Recursive/
ACK.
> > + if not node_uuid in lvmap:
> > + lvmap[node_uuid] = []
> > +
> > + for dev in devices:
> > + 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:
> > + _MapLVsByNode(lvmap, dev.children, dev.logical_id[0])
> > + _MapLVsByNode(lvmap, dev.children, dev.logical_id[1])
> > +
> > + elif dev.children:
> > + _MapLVsByNode(lvmap, dev.children, node_uuid)
> > +
> > + instance = self._UnlockedGetInstanceInfo(inst_uuid)
> > + if instance is None:
> > + raise errors.ConfigurationError("Unknown instance '%s'" % inst_uuid)
> > +
> > + if lvmap is None:
> > + lvmap = {}
> > + ret = lvmap
> > + else:
> > + ret = None
> > +
> > + node_uuid = instance.primary_node
> > + devices = instance.disks
> > + _MapLVsByNode(lvmap, devices, node_uuid)
>
> We don't really need the local variables:
>
> _MapLVsByNode(lvmap, instance.disks, instance.primary_node)
ACK. Should I send an interdiff?
Thanks,
Ilias
> Rest LGTM.
>
> Thanks,
> Jose
>
> > + return ret
> > +
> > + @_ConfigSync(shared=1)
> > + def GetInstanceLVsByNode(self, inst_uuid, lvmap=None):
> > + """Provide a mapping of node to LVs a given instance owns.
> > +
> > + This is a simple wrapper over L{_UnlockedGetInstanceLVsByNode}
> > +
> > + """
> > + return self._UnlockedGetInstanceLVsByNode(inst_uuid, lvmap=lvmap)
> > +
> > @_ConfigSync(shared=1)
> > def GetGroupDiskParams(self, group):
> > """Get the disk params populated with inherit chain.
> > --
> > 1.9.1
> >
signature.asc
Description: Digital signature
