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 :)

> 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",
               }

     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

Reply via email to