On Thu, Dec 5, 2013 at 11:14 AM, Santi Raffa <[email protected]> wrote:
> On Thu, Dec 5, 2013 at 10:04 AM, Thomas Thrainer <[email protected]> > wrote: > > On Wed, Dec 4, 2013 at 2:50 PM, Santi Raffa <[email protected]> wrote: > >> + def GetKVMMountString(self, path): > > > > Please don't include hypervisor names in method names. It's better to > look > > at BlockDev.GetUserspaceAccessUri(...) and follow the naming scheme and > > logic from there. > > This is not part of the GlusterStorage class, but of GlusterVolume. > GlusterVolume checks if the hypervisor is KVM and then uses this > method if it is. I thus think it's fine for this method to just return > the string used by KVM, and if later we want to add support for other > hypervisors GlusterVolume can get new methods for > GetUserspaceAccessUri to use. > > I think that it's more consistent to have many small methods giving > one representation each of the volume, than having many small methods > giving one representation each of the volume except for the userspace > method, which checks it's the right hypervisor before giving its > representation of the volume :) > The point is that most probably multiple hypervisors will expect the same URI, as they are pretty standard. With one method it's easy to just return the same URI for all of them. > > > Please use documentation IP addresses ( > http://tools.ietf.org/html/rfc5737) > > throughout this tests. > > I have: 203.0.113.42 is in TEST-NET-3 (203.0.113.0/24) and > 001:DB8::74:65:28:6:69 is... er... not in 2001:DB8::/32 because of a > typo. > > > > Current interdiff: > > diff --git a/lib/storage/gluster.py b/lib/storage/gluster.py > index fa1532b..9ce76c3 100644 > --- a/lib/storage/gluster.py > +++ b/lib/storage/gluster.py > @@ -102,7 +102,7 @@ class GlusterVolume(object): > .format(ip=self.server_ip, port=self.port, > volume=self.volume) > > def __hash__(self): > - return self.__repr__().__hash__() > + return (self.server_ip, self.port, self.volume).__hash__() > > def _IsMounted(self): > """Checks if we are mounted or not. > diff --git a/test/py/ganeti.storage.gluster_unittest.py > b/test/py/ganeti.storage.gluster_unittest.py > index 55a189f..0c47e13 100644 > --- a/test/py/ganeti.storage.gluster_unittest.py > +++ b/test/py/ganeti.storage.gluster_unittest.py > @@ -41,7 +41,7 @@ class TestGlusterVolume(testutils.GanetiTestCase): > vol_name="pinky"): > > address = {4: "203.0.113.42", > - 6: "001:DB8::74:65:28:6:69", > + 6: "2001:DB8::74:65:28:6:69", > What's about the other IP's further down? > } > > return gluster.GlusterVolume(address[ipv] if not addr else addr, > > > -- > Raffa Santi > 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 > -- Thomas Thrainer | Software Engineer | [email protected] | 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
