On Thu, Apr 17, 2014 at 12:53PM, Jose A. Lopes wrote:
> On Apr 16 15:19, Ilias Tsitsimpis wrote:
> > Get all disk-related nodes for an instance.
> > Also use 'GetInstanceSecondaryNodes' to get the
> > list of secondary nodes.
> > 
> > Signed-off-by: Ilias Tsitsimpis <[email protected]>
> > ---
> >  lib/config.py | 58 
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 58 insertions(+)
> > 
> > diff --git a/lib/config.py b/lib/config.py
> > index b86df08..7147b50 100644
> > --- a/lib/config.py
> > +++ b/lib/config.py
> > @@ -350,6 +350,64 @@ class ConfigWriter(object):
> >      nodegroup = self._UnlockedGetNodeGroup(node.group)
> >      return self._UnlockedGetGroupDiskParams(nodegroup)
> >  
> > +  def _UnlockedGetInstanceNodes(self, inst_uuid):
> > +    """Get all disk-related nodes for an instance.
> > +
> > +    For non-DRBD, this will be empty, for DRBD it will contain both
> > +    the primary and the secondaries.
> > +
> > +    @type inst_uuid: string
> > +    @param inst_uuid: The UUID of the instance we want to get nodes for
> > +    @return: A list of names for all the nodes of the instance
> > +
> > +    """
> 
> Missing @rtype in this method and others too.

ACK.

The @rtype is missing for the methods '_UnlockedGetInstanceNodes',
'_UnlockedGetInstanceSecondaryNodes' and
'_UnlockedGetInstanceLVsByNode'. I will send an interdiff for this patch
and the next one (that contains the '_UnlockedGetInstanceLVsByNode'
method).

> > +    instance = self._UnlockedGetInstanceInfo(inst_uuid)
> > +    if instance is None:
> > +      raise errors.ConfigurationError("Unknown instance '%s'" % inst_uuid)
> > +
> > +    instance_disks = instance.disks
> > +    all_nodes = []
> > +    for disk in instance_disks:
> > +      all_nodes.extend(disk.all_nodes)
> > +    all_nodes = set(all_nodes)
> > +    # ensure that primary node is always the first
> > +    all_nodes.discard(instance.primary_node)
> > +    return (instance.primary_node, ) + tuple(all_nodes)
> > +
> > +  @_ConfigSync(shared=1)
> > +  def GetInstanceNodes(self, inst_uuid):
> > +    """Get all disk-related nodes for an instance.
> > +
> > +    This is just a wrapper over L{_UnlockedGetInstanceNodes}
> > +
> > +    """
> > +    return self._UnlockedGetInstanceNodes(inst_uuid)
> > +
> > +  def _UnlockedGetInstanceSecondaryNodes(self, inst_uuid):
> > +    """Get the list of secondary nodes.
> > +
> > +    @type inst_uuid: string
> > +    @param inst_uuid: The UUID of the instance we want to get nodes for
> > +    @return: A list of names for all the secondary nodes of the instance
> > +
> > +    """
> > +    instance = self._UnlockedGetInstanceInfo(inst_uuid)
> > +    if instance is None:
> > +      raise errors.ConfigurationError("Unknown instance '%s'" % inst_uuid)
> > +
> > +    all_nodes = set(self._UnlockedGetInstanceNodes(inst_uuid))
> > +    all_nodes.discard(instance.primary_node)
> 
> In this logic, we are basically adding the primary node twice and
> removing it twice from the set.  We can definitely improve this.

This is actually the algorithm that is currently used for computing the
secondary nodes ('_ComputeSecondaryNodes' method in Instance object).
How do you suggest to improve this?

Thanks,
Ilias

> Rest LGTM.
> 
> Thanks,
> Jose
> 
> > +    return tuple(all_nodes)
> > +
> > +  @_ConfigSync(shared=1)
> > +  def GetInstanceSecondaryNodes(self, inst_uuid):
> > +    """Get the list of secondary nodes.
> > +
> > +    This is a simple wrapper over L{_UnlockedGetInstanceSecondaryNodes}.
> > +
> > +    """
> > +    return self._UnlockedGetInstanceSecondaryNodes(inst_uuid)
> > +
> >    @_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