On Thu, Apr 17, 2014 at 01:43PM, Jose A. Lopes wrote:
> On Apr 17 14:42, Ilias Tsitsimpis wrote:
> > 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?
> 
> Yup.

Interdiff:

diff --git a/lib/config.py b/lib/config.py
index 9e01cd5..5d3d3bd 100644
--- a/lib/config.py
+++ b/lib/config.py
@@ -417,6 +417,7 @@ class ConfigWriter(object):
     @type lvmap: dict
     @param lvmap: Optional dictionary to receive the
         'node' : ['lv', ...] data.
+    @rtype: dict or None
     @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
@@ -424,7 +425,7 @@ class ConfigWriter(object):
 
     """
     def _MapLVsByNode(lvmap, devices, node_uuid):
-      """Recursively helper function."""
+      """Recursive helper function."""
       if not node_uuid in lvmap:
         lvmap[node_uuid] = []
 
@@ -450,9 +451,7 @@ class ConfigWriter(object):
     else:
       ret = None
 
-    node_uuid = instance.primary_node
-    devices = instance.disks
-    _MapLVsByNode(lvmap, devices, node_uuid)
+    _MapLVsByNode(lvmap, instance.disks, instance.primary_node)
     return ret
 
   @_ConfigSync(shared=1)


Thanks,
Ilias

> Thanks,
> Jose
> 
> > 
> > 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
> > > > 
> 
> 
> 
> -- 
> 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

-- 
Ilias Tsitsimpis
OpenPGP public key ID:
pub  4096R/25F3E3EE 2012-11-02 Ilias Tsitsimpis <[email protected]>
  Key fingerprint = 1986 21F5 7024 9B25 F4E2  4EF7 6FB8 90BA 25F3 E3EE


Tiger got to hunt, Bird got to fly;
Lisper got to sit and wonder, (Y (Y Y))?

Tiger got to sleep, Bird got to land;
Lisper got to tell himself he understand.

    — Kurt Vonnegut, modified by Darius Bacon

Attachment: signature.asc
Description: Digital signature

Reply via email to