Refactor volume initialization code

* Validate inputs during initialization of Volume object.
* Move glfs_new() and glfs_init() into mount() method.
* Provide user consumable unmount() method.
* Provide "mounted" property to Volume object. Users can now check state
  whether a volume is virtual mounted or not.

Change-Id: Idc53ee7f9386ed995315bd5fb492a7d36f44875f
Signed-off-by: Prashanth Pai <ppai@redhat.com>

4 files changed, 328 insertions, 30 deletions

diff --git a/gluster/exceptions.py b/gluster/exceptions.py
new file mode 100644
index 0000000..2bb4732
--- /dev/null
+++ b/gluster/exceptions.py
@@ -0,0 +1,18 @@
+# Copyright (c) 2012-2015 Red Hat, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+# implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+
+class LibgfapiException(Exception):
+    pass
diff --git a/gluster/gfapi.py b/gluster/gfapi.py
index 3554657..8a02831 100755
--- a/gluster/gfapi.py
+++ b/gluster/gfapi.py
@@ -14,6 +14,7 @@ import os
 import stat
 import errno
 from gluster import api
+from gluster.exceptions import LibgfapiException
 
 
 class File(object):
@@ -201,22 +202,132 @@ class Dir(object):
 
 class Volume(object):
 
-    def __init__(self, host, volid, proto="tcp", port=24007):
+    def __init__(self, host, volname,
+                 proto="tcp", port=24007, log_file=None, log_level=7):
+        """
+        Create a Volume object instance.
+
+        :param host: Host with glusterd management daemon running.
+        :param volname: Name of GlusterFS volume to be mounted and used.
+        :param proto: Transport protocol to be used to connect to management
+                      daemon. Permitted values are "tcp" and "rdma".
+        :param port: Port number where gluster management daemon is listening.
+        :param log_file: Path to log file. When this is set to None, a new
+                         logfile will be created in default log directory
+                         i.e /var/log/glusterfs
+        :param log_level: Integer specifying the degree of verbosity.
+                          Higher the value, more verbose the logging.
+
+        TODO: Provide an interface where user can specify volfile directly
+        instead of providing host and other details. This is helpful in cases
+        where user wants to load some non default xlator on client side. For
+        example, aux-gfid-mount or mount volume as read-only.
+        """
         # Add a reference so the module-level variable "api" doesn't
         # get yanked out from under us (see comment above File def'n).
         self._api = api
-        self.fs = api.glfs_new(volid)
-        api.glfs_set_volfile_server(self.fs, proto, host, port)
 
-    def __del__(self):
-        self._api.glfs_fini(self.fs)
-        self._api = None
+        self._mounted = False
+        self.fs = None
+        self.log_file = log_file
+        self.log_level = log_level
 
-    def set_logging(self, path, level):
-        api.glfs_set_logging(self.fs, path, level)
+        if None in (volname, host):
+            # TODO: Validate host based on regex for IP/FQDN.
+            raise LibgfapiException("Host and Volume name should not be None.")
+        if proto not in ('tcp', 'rdma'):
+            raise LibgfapiException("Invalid protocol specified.")
+        if not isinstance(port, (int, long)):
+            raise LibgfapiException("Invalid port specified.")
+
+        self.host = host
+        self.volname = volname
+        self.protocol = proto
+        self.port = port
+
+    @property
+    def mounted(self):
+        return self._mounted
 
     def mount(self):
-        return api.glfs_init(self.fs)
+        """
+        Mount a GlusterFS volume for use.
+        """
+        if self.fs and self._mounted:
+            # Already mounted
+            return
+
+        self.fs = api.glfs_new(self.volname)
+        if not self.fs:
+            raise LibgfapiException("glfs_new(%s) failed." % (self.volname))
+
+        ret = api.glfs_set_volfile_server(self.fs, self.protocol,
+                                          self.host, self.port)
+        if ret < 0:
+            # FIXME: For some reason, get_errno() is not able to capture
+            # proper errno. Until then..
+            # https://bugzilla.redhat.com/show_bug.cgi?id=1196161
+            raise LibgfapiException("glfs_set_volfile_server(%s, %s, %s, "
+                                    "%s) failed." % (self.fs, self.protocol,
+                                                     self.host, self.port))
+
+        self.set_logging(self.log_file, self.log_level)
+
+        if self.fs and not self._mounted:
+            ret = api.glfs_init(self.fs)
+            if ret < 0:
+                raise LibgfapiException("glfs_init(%s) failed." % (self.fs))
+            else:
+                self._mounted = True
+
+    def unmount(self):
+        """
+        Unmount a mounted GlusterFS volume.
+
+        Provides users a way to free resources instead of just waiting for
+        python garbage collector to call __del__() at some point later.
+        """
+        if self.fs:
+            ret = self._api.glfs_fini(self.fs)
+            if ret < 0:
+                raise LibgfapiException("glfs_fini(%s) failed." % (self.fs))
+            else:
+                # Succeeded. Protect against multiple unmount() calls.
+                self._mounted = False
+                self.fs = None
+
+    def __del__(self):
+        try:
+            self.unmount()
+        except LibgfapiException:
+            pass
+
+    def set_logging(self, log_file, log_level):
+        """
+        Set logging parameters. Can be invoked either before or after
+        invoking mount().
+
+        When invoked before mount(), the preferred log file and log level
+        choices are recorded and then later enforced internally as part of
+        mount()
+
+        When invoked at any point after mount(), the change in log file
+        and log level is instantaneous.
+
+        :param log_file: Path of log file.
+                         If set to "/dev/null", nothing will be logged.
+                         If set to None, a new logfile will be created in
+                         default log directory (/var/log/glusterfs)
+        :param log_level: Integer specifying the degree of verbosity.
+                          Higher the value, more verbose the logging.
+        """
+        if self.fs:
+            ret = api.glfs_set_logging(self.fs, self.log_file, self.log_level)
+            if ret < 0:
+                raise LibgfapiException("glfs_set_logging(%s, %s) failed." %
+                                        (self.log_file, self.log_level))
+        self.log_file = log_file
+        self.log_level = log_level
 
     def chmod(self, path, mode):
         """
diff --git a/test/functional/libgfapi-python-tests.py b/test/functional/libgfapi-python-tests.py
index 1519426..c0e760a 100644
--- a/test/functional/libgfapi-python-tests.py
+++ b/test/functional/libgfapi-python-tests.py
@@ -15,8 +15,10 @@ import types
 import errno
 
 from gluster import gfapi
+from gluster.exceptions import LibgfapiException
 from test import get_test_config
 from ConfigParser import NoSectionError, NoOptionError
+from uuid import uuid4
 
 config = get_test_config()
 if config:
@@ -42,14 +44,10 @@ class BinFileOpsTest(unittest.TestCase):
     @classmethod
     def setUpClass(cls):
         cls.vol = gfapi.Volume(HOST, VOLNAME)
-        cls.vol.set_logging("/dev/null", 7)
         ret = cls.vol.mount()
         if ret == 0:
             # Cleanup volume
             cls.vol.rmtree("/", ignore_errors=True)
-        else:
-            raise Exception("Initializing volume %s:%s failed." %
-                            (HOST, VOLNAME))
 
     @classmethod
     def tearDownClass(cls):
@@ -80,14 +78,10 @@ class FileOpsTest(unittest.TestCase):
     @classmethod
     def setUpClass(cls):
         cls.vol = gfapi.Volume(HOST, VOLNAME)
-        cls.vol.set_logging("/dev/null", 7)
         ret = cls.vol.mount()
         if ret == 0:
             # Cleanup volume
             cls.vol.rmtree("/", ignore_errors=True)
-        else:
-            raise Exception("Initializing volume %s:%s failed." %
-                            (HOST, VOLNAME))
 
     @classmethod
     def tearDownClass(cls):
@@ -253,15 +247,10 @@ class DirOpsTest(unittest.TestCase):
     @classmethod
     def setUpClass(cls):
         cls.vol = gfapi.Volume(HOST, VOLNAME)
-        cls.vol.set_logging("/dev/null", 7)
-        cls.vol.mount()
         ret = cls.vol.mount()
         if ret == 0:
             # Cleanup volume
             cls.vol.rmtree("/", ignore_errors=True)
-        else:
-            raise Exception("Initializing volume %s:%s failed." %
-                            (HOST, VOLNAME))
         cls.testfile = "testfile"
 
     @classmethod
@@ -323,3 +312,69 @@ class DirOpsTest(unittest.TestCase):
         self.vol.rmtree(self.dir_path, True)
         self.assertRaises(OSError, self.vol.lstat, f)
         self.assertRaises(OSError, self.vol.lstat, self.dir_path)
+
+
+class TestVolumeInit(unittest.TestCase):
+
+    def test_mount_unmount_default(self):
+        # Create volume object instance
+        vol = gfapi.Volume(HOST, VOLNAME)
+        # Check attribute init
+        self.assertEqual(vol.log_file, None)
+        self.assertEqual(vol.log_level, 7)
+        self.assertEqual(vol.host, HOST)
+        self.assertEqual(vol.volname, VOLNAME)
+        self.assertEqual(vol.port, 24007)
+        self.assertFalse(vol.mounted)
+        # Mount the volume
+        vol.mount()
+        # Check mounted property
+        self.assertTrue(vol.mounted)
+        # Unmount the volume
+        vol.unmount()
+        # Check mounted property again
+        self.assertFalse(vol.mounted)
+        # Do a double unmount - should not crash or raise exception
+        vol.unmount()
+        self.assertFalse(vol.mounted)
+        # Do a double mount - should not crash or raise exception
+        vol.mount()
+        vol.mount()
+        self.assertTrue(vol.mounted)
+        # Unmount the volume
+        vol.unmount()
+        self.assertFalse(vol.mounted)
+
+    def test_mount_err(self):
+        # Volume does not exist
+        fake_volname = str(uuid4().hex)[:10]
+        vol = gfapi.Volume(HOST, fake_volname)
+        self.assertRaises(LibgfapiException, vol.mount)
+        self.assertFalse(vol.mounted)
+
+        # Invalid host - glfs_set_volfile_server will fail
+        fake_hostname = str(uuid4().hex)[:10]
+        vol = gfapi.Volume(fake_hostname, VOLNAME)
+        self.assertRaises(LibgfapiException, vol.mount)
+        self.assertFalse(vol.mounted)
+
+    def test_set_logging(self):
+        # Create volume object instance
+        vol = gfapi.Volume(HOST, VOLNAME)
+        # Call set_logging before mount()
+        log_file = "/tmp/%s" % (uuid4().hex)
+        vol.set_logging(log_file, 7)
+        # Mount the volume
+        vol.mount()
+        self.assertTrue(vol.mounted)
+        self.assertEqual(vol.log_file, log_file)
+        self.assertEqual(vol.log_level, 7)
+        # Check that log has been created and exists
+        self.assertTrue(os.path.exists(log_file))
+        # Change the logging after mounting
+        log_file2 = "/tmp/%s" % (uuid4().hex)
+        vol.set_logging(log_file2, 7)
+        self.assertEqual(vol.log_file, log_file2)
+        # Unmount the volume
+        vol.unmount()
+        self.assertFalse(vol.mounted)
diff --git a/test/unit/gluster/test_gfapi.py b/test/unit/gluster/test_gfapi.py
index dc470a1..8880727 100644
--- a/test/unit/gluster/test_gfapi.py
+++ b/test/unit/gluster/test_gfapi.py
@@ -17,6 +17,7 @@ import errno
 
 from gluster import gfapi
 from gluster import api
+from gluster.exceptions import LibgfapiException
 from nose import SkipTest
 from mock import Mock, patch
 from contextlib import nested
@@ -31,15 +32,23 @@ def _mock_glfs_closedir(fd):
 
 
 def _mock_glfs_new(volid):
-    return 2
+    return 12345
+
+
+def _mock_glfs_init(fs):
+    return 0
 
 
 def _mock_glfs_set_volfile_server(fs, proto, host, port):
-    return
+    return 0
 
 
 def _mock_glfs_fini(fs):
-    return
+    return 0
+
+
+def _mock_glfs_set_logging(fs, log_file, log_level):
+    return 0
 
 
 class TestFile(unittest.TestCase):
@@ -183,10 +192,10 @@ class TestFile(unittest.TestCase):
             self.assertEqual(buflen, 12345)
             return buflen
 
-        for buflen in (-1,-2,-999):
+        for buflen in (-1, -2, -999):
             with patch("gluster.gfapi.api.glfs_read", _mock_glfs_read):
                 with patch("gluster.gfapi.File.fgetsize", _mock_fgetsize):
-                    b = self.fd.read(buflen)
+                    self.fd.read(buflen)
 
     def test_write_success(self):
         mock_glfs_write = Mock()
@@ -281,6 +290,9 @@ class TestVolume(unittest.TestCase):
         gluster.gfapi.api.glfs_set_volfile_server = \
             _mock_glfs_set_volfile_server
 
+        cls._saved_glfs_init = gluster.gfapi.api.glfs_init
+        gluster.gfapi.api.glfs_init = _mock_glfs_init
+
         cls._saved_glfs_fini = gluster.gfapi.api.glfs_fini
         gluster.gfapi.api.glfs_fini = _mock_glfs_fini
 
@@ -289,7 +301,13 @@ class TestVolume(unittest.TestCase):
 
         cls._saved_glfs_closedir = gluster.gfapi.api.glfs_closedir
         gluster.gfapi.api.glfs_closedir = _mock_glfs_closedir
+
+        cls._saved_glfs_set_logging = gluster.gfapi.api.glfs_set_logging
+        gluster.gfapi.api.glfs_set_logging = _mock_glfs_set_logging
+
         cls.vol = gfapi.Volume("mockhost", "test")
+        cls.vol.fs = 12345
+        cls.vol._mounted = True
 
     @classmethod
     def tearDownClass(cls):
@@ -301,6 +319,101 @@ class TestVolume(unittest.TestCase):
         gluster.gfapi.api.glfs_close = cls._saved_glfs_close
         gluster.gfapi.api.glfs_closedir = cls._saved_glfs_closedir
 
+    def test_initialization_error(self):
+        self.assertRaises(LibgfapiException, gfapi.Volume, "host", None)
+        self.assertRaises(LibgfapiException, gfapi.Volume, None, "vol")
+        self.assertRaises(LibgfapiException, gfapi.Volume, None, None)
+        self.assertRaises(LibgfapiException, gfapi.Volume, "host", "vol", "ZZ")
+        self.assertRaises(LibgfapiException, gfapi.Volume, "host", "vol",
+                          "tcp", "invalid_port")
+
+    def test_initialization_success(self):
+        v = gfapi.Volume("host", "vol", "tcp", 9876)
+        self.assertEqual(v.host, "host")
+        self.assertEqual(v.volname, "vol")
+        self.assertEqual(v.protocol, "tcp")
+        self.assertEqual(v.port, 9876)
+        self.assertFalse(v.mounted)
+
+    def test_mount_unmount_success(self):
+        v = gfapi.Volume("host", "vol")
+        v.mount()
+        self.assertTrue(v.mounted)
+        self.assertTrue(v.fs)
+        v.unmount()
+        self.assertFalse(v.mounted)
+        self.assertFalse(v.fs)
+
+    def test_mount_multiple(self):
+        _m_glfs_new = Mock()
+        v = gfapi.Volume("host", "vol")
+        with patch("gluster.gfapi.api.glfs_new", _m_glfs_new):
+            # Mounting for first time
+            v.mount()
+            _m_glfs_new.assert_called_once_with("vol")
+            _m_glfs_new.reset_mock()
+            for i in range(0, 5):
+                v.mount()
+                self.assertFalse(_m_glfs_new.called)
+                self.assertTrue(v.mounted)
+
+    def test_mount_error(self):
+        # glfs_new() failed
+        _m_glfs_new = Mock(return_value=None)
+        v = gfapi.Volume("host", "vol")
+        with patch("gluster.gfapi.api.glfs_new", _m_glfs_new):
+            self.assertRaises(LibgfapiException, v.mount)
+            self.assertFalse(v.fs)
+            self.assertFalse(v.mounted)
+            _m_glfs_new.assert_called_once_with("vol")
+
+        # glfs_set_volfile_server() failed
+        _m_set_vol = Mock(return_value=-1)
+        v = gfapi.Volume("host", "vol")
+        with patch("gluster.gfapi.api.glfs_set_volfile_server", _m_set_vol):
+            self.assertRaises(LibgfapiException, v.mount)
+            self.assertFalse(v.mounted)
+            _m_glfs_new.assert_called_once_with("vol")
+            _m_set_vol.assert_called_once_with(v.fs, v.protocol,
+                                               v.host, v.port)
+
+        # glfs_init() failed
+        _m_glfs_init = Mock(return_value=-1)
+        v = gfapi.Volume("host", "vol")
+        with patch("gluster.gfapi.api.glfs_init", _m_glfs_init):
+            self.assertRaises(LibgfapiException, v.mount)
+            self.assertFalse(v.mounted)
+            _m_glfs_init.assert_caled_once_with(v.fs)
+
+    def test_unmount_error(self):
+        v = gfapi.Volume("host", "vol")
+        v.mount()
+        _m_glfs_fini = Mock(return_value=-1)
+        with patch("gluster.gfapi.api.glfs_fini", _m_glfs_fini):
+            self.assertRaises(LibgfapiException, v.unmount)
+            _m_glfs_fini.assert_called_once_with(v.fs)
+            # Should still be mounted as unmount failed.
+            self.assertTrue(v.mounted)
+
+    def test_set_logging(self):
+        _m_set_logging = Mock()
+
+        # Called after mount()
+        v = gfapi.Volume("host", "vol")
+        with patch("gluster.gfapi.api.glfs_set_logging", _m_set_logging):
+            v.mount()
+            v.set_logging("/path/whatever", 7)
+            self.assertEqual(v.log_file, "/path/whatever")
+            self.assertEqual(v.log_level, 7)
+
+    def test_set_logging_err(self):
+        v = gfapi.Volume("host", "vol")
+        v.fs = 12345
+        _m_set_logging = Mock(return_value=-1)
+        with patch("gluster.gfapi.api.glfs_set_logging", _m_set_logging):
+            self.assertRaises(LibgfapiException, v.set_logging, "/dev/null", 7)
+            _m_set_logging.assert_called_once_with(v.fs, None, 7)
+
     def test_chmod_success(self):
         mock_glfs_chmod = Mock()
         mock_glfs_chmod.return_value = 0
@@ -339,7 +452,7 @@ class TestVolume(unittest.TestCase):
             with self.vol.open("file.txt", os.O_CREAT, 0644) as fd:
                 self.assertTrue(isinstance(fd, gfapi.File))
                 self.assertEqual(mock_glfs_creat.call_count, 1)
-                mock_glfs_creat.assert_called_once_with(2,
+                mock_glfs_creat.assert_called_once_with(12345,
                                                         "file.txt",
                                                         os.O_CREAT, 0644)
 
@@ -615,7 +728,7 @@ class TestVolume(unittest.TestCase):
             with self.vol.open("file.txt", os.O_WRONLY) as fd:
                 self.assertTrue(isinstance(fd, gfapi.File))
                 self.assertEqual(mock_glfs_open.call_count, 1)
-                mock_glfs_open.assert_called_once_with(2,
+                mock_glfs_open.assert_called_once_with(12345,
                                                        "file.txt", os.O_WRONLY)
 
     def test_open_with_statement_fail_exception(self):
@@ -637,7 +750,8 @@ class TestVolume(unittest.TestCase):
             fd = self.vol.open("file.txt", os.O_WRONLY)
             self.assertTrue(isinstance(fd, gfapi.File))
             self.assertEqual(mock_glfs_open.call_count, 1)
-            mock_glfs_open.assert_called_once_with(2, "file.txt", os.O_WRONLY)
+            mock_glfs_open.assert_called_once_with(12345, "file.txt",
+                                                   os.O_WRONLY)
 
     def test_open_direct_fail_exception(self):
         mock_glfs_open = Mock()