On Apr 17 15:14, Ilias Tsitsimpis wrote:
> 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?
A third method/function that is private and from which the list of
nodes actually comes from.
Thanks,
Jose
> 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
--
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
