CA-379315 Use XE_SR_ERRORCODES in the LVM journaller

From: Tim Smith <tim.smith@citrix.com>

Rather than allow JournalerExceptions to propagate up and eventually
turn into "unhandled" error 1200s, turn all instances into the
appropriate type of LVM error code and reture the JournalerException
class, at least from the LVM journaller

Signed-off-by: Tim Smith <tim.smith@citrix.com>
---
 drivers/XE_SR_ERRORCODES.xml |    7 ++
 drivers/fjournaler.py        |    5 +-
 drivers/journaler.py         |   16 ++---
 tests/test_fjournaler.py     |  138 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 155 insertions(+), 11 deletions(-)
 create mode 100644 tests/test_fjournaler.py

diff --git a/drivers/XE_SR_ERRORCODES.xml b/drivers/XE_SR_ERRORCODES.xml
index 97236fe..de074ce 100755
--- a/drivers/XE_SR_ERRORCODES.xml
+++ b/drivers/XE_SR_ERRORCODES.xml
@@ -501,6 +501,13 @@
                 <value>116</value>
         </code>
 
+        <!-- An extra LVM error -->
+        <code>
+                <name>LVMRead</name>
+                <description>Logical Volume read error</description>
+                <value>117</value>
+        </code>
+
        <!-- Agent database query errors 150+ -->
         <code>
                 <name>APISession</name>
diff --git a/drivers/fjournaler.py b/drivers/fjournaler.py
index 7d06856..cb5ca34 100644
--- a/drivers/fjournaler.py
+++ b/drivers/fjournaler.py
@@ -21,10 +21,13 @@ import os
 import errno
 
 import util
-from journaler import JournalerException
 
 SEPARATOR = "_"
 
+class JournalerException(util.SMException):
+    pass
+
+
 class Journaler:
     """Simple file-based journaler. A journal is a id-value pair, and there
     can be only one journal for a given id."""
diff --git a/drivers/journaler.py b/drivers/journaler.py
index 9d40edf..c619c21 100644
--- a/drivers/journaler.py
+++ b/drivers/journaler.py
@@ -18,13 +18,12 @@
 # LVM-based journaling
 
 import util
+import xs_errors
 from srmetadata import open_file, close, get_min_blk_size_wrapper, \
     file_read_wrapper, file_write_wrapper
 
 LVM_MAX_NAME_LEN = 127
 
-class JournalerException(util.SMException):
-    pass
 
 class Journaler:
     """Simple journaler that uses LVM namespace for persistent "storage".
@@ -47,8 +46,7 @@ class Journaler:
         valExisting = self.get(type, id)
         writeData = False
         if valExisting:
-            raise JournalerException("Journal already exists for '%s:%s': %s" \
-                    % (type, id, valExisting))
+            raise xs_errors.XenError('LVMCreate', opterr="Journal already exists for '%s:%s': %s" % (type, id, valExisting))
         lvName = self._getNameLV(type, id, val)
 
         mapperDevice = self._getLVMapperName(lvName)
@@ -91,15 +89,14 @@ class Journaler:
                 except Exception, e:
                     util.SMlog('WARNING: failed to clean up failed journal ' \
                             ' creation: %s (error ignored)' % e)
-                raise JournalerException("Failed to write to journal %s" \
-                    % lvName)
+                raise xs_errors.XenError('LVMWrite', opterr="Failed to write to journal %s" % lvName)
 
     def remove(self, type, id):
         """Remove the entry of type "type" for "id". Error if the entry doesn't
         exist."""
         val = self.get(type, id)
         if not val:
-            raise JournalerException("No journal for '%s:%s'" % (type, id))
+            raise xs_errors.XenError('LVMNoVolume', opterr="No journal for '%s:%s'" % (type, id))
         lvName = self._getNameLV(type, id, val)
 
         mapperDevice = self._getLVMapperName(lvName)
@@ -140,7 +137,7 @@ class Journaler:
         for lvName in lvList:
             parts = lvName.split(self.SEPARATOR, 2)
             if len(parts) != 3:
-                raise JournalerException("Bad LV name: %s" % lvName)
+                raise xs_errors.XenError('LVMNoVolume', opterr="Bad LV name: %s" % lvName)
             type, id, val = parts
             if readFile:
                 # For clone and leaf journals, additional
@@ -157,8 +154,7 @@ class Journaler:
                             length, val = data.split(" ", 1)
                             val = val[:int(length)]
                         except:
-                            raise JournalerException("Failed to read from journal %s" \
-                                  % lvName)
+                            raise xs_errors.XenError('LVMRead', opterr="Failed to read from journal %s" % lvName)
                     finally:
                         close(fd)
                         self.lvmCache.deactivateNoRefcount(lvName)
diff --git a/tests/test_fjournaler.py b/tests/test_fjournaler.py
new file mode 100644
index 0000000..bfa8903
--- /dev/null
+++ b/tests/test_fjournaler.py
@@ -0,0 +1,138 @@
+from io import BytesIO as StringIO
+import os
+import unittest
+import mock
+
+import fjournaler
+
+TEST_DIR_PATH = '/var/lib/sometest'
+
+
+class FakeFile(object):
+
+    def __init__(self):
+        self.content = b''
+        self.data = None
+
+    def open(self):
+        print('Opening fake file, content = {}'.format(self.content))
+        self.data = StringIO(self.content)
+        return self
+
+    def close(self):
+        self.content = self.data.getvalue()
+        self.data.close()
+        self.data = None
+
+    def write(self, val):
+        self.data.write(str.encode(val))
+
+    def readline(self):
+        return self.data.readline()
+
+
+class TestFjournaler(unittest.TestCase):
+
+    def setUp(self):
+        self.files = {}
+        self.open_handlers = {TEST_DIR_PATH: self.__fake_open}
+
+        self.subject = fjournaler.Journaler(TEST_DIR_PATH)
+
+        self.real_open = open
+        open_patcher = mock.patch("__builtin__.open", autospec=True)
+        self.mock_open = open_patcher.start()
+        self.mock_open.side_effect = self.__open_selector
+
+        exists_patcher = mock.patch('fjournaler.util.pathexists',
+                                    autospec=True)
+        self.mock_exists = exists_patcher.start()
+        self.mock_exists.side_effect = self.__fake_exists
+
+        unlink_patcher = mock.patch('fjournaler.os.unlink', autospec=True)
+        self.mock_unlink = unlink_patcher.start()
+        self.mock_unlink.side_effect = self.__fake_unlink
+
+        listdir_patcher = mock.patch('fjournaler.os.listdir', autospec=True)
+        self.mock_listdir = listdir_patcher.start()
+        self.mock_listdir.side_effect = self.__fake_listdir
+
+        self.addCleanup(mock.patch.stopall)
+
+    def __fake_exists(self, path):
+        return path in self.files
+
+    def __open_selector(self, path, mode):
+        handler = self.open_handlers.get(os.path.dirname(path), self.real_open)
+        return handler(path, mode)
+
+    def __fake_open(self, path, mode):
+        if path not in self.files:
+            mock_file = FakeFile()
+            self.files[path] = mock_file
+        return self.files[path].open()
+
+    def __fake_unlink(self, path):
+        del self.files[path]
+
+    def __fake_listdir(self, path):
+        assert(path == TEST_DIR_PATH)
+        return [os.path.basename(x) for x in list(self.files.keys())]
+
+    def test_non_existing(self):
+        self.assertIsNone(self.subject.get('clone', '1'))
+
+    def test_create_and_exists(self):
+        self.subject.create('clone', '1', 'a')
+
+        val = self.subject.get('clone', '1')
+        self.assertEqual(b'a', val)
+
+        self.subject.remove('clone', '1')
+
+        self.assertIsNone(self.subject.get('clone', '1'))
+
+    def test_create_mulitple(self):
+        self.subject.create("modify", "X", "831_3")
+        self.subject.create("modify", "Z", "831_4")
+        self.subject.create("modify", "Y", "53_0")
+
+        self.assertEqual(b"831_3", self.subject.get("modify", "X"),
+                         msg="create underscore_val failed")
+        self.assertEqual(b"53_0", self.subject.get("modify", "Y"),
+                         msg="create multiple id's failed")
+
+        entries = self.subject.getAll('modify')
+        self.assertSetEqual({'X', 'Y', 'Z'}, set(entries.keys()))
+        self.assertEqual(b"831_3", entries['X'])
+        self.assertEqual(b"53_0", entries['Y'])
+
+        # Check no extra returned
+        self.subject.create('clone', '1', 'a')
+        entries = self.subject.getAll('modify')
+        self.assertSetEqual({'X', 'Y', 'Z'}, set(entries.keys()))
+
+        self.subject.create('modify2', '1', 'z')
+        entries = self.subject.getAll('modify')
+        self.assertSetEqual({'X', 'Y', 'Z'}, set(entries.keys()))
+
+        # getAll should not return empty value journals
+        self.subject.create('modify', 'N', '')
+        entries = self.subject.getAll('modify')
+        self.assertSetEqual({'X', 'Y', 'Z'}, set(entries.keys()))
+
+        # Remove X
+        self.subject.remove('modify', 'X')
+        entries = self.subject.getAll('modify')
+        self.assertSetEqual({'Y', 'Z'}, set(entries.keys()))
+        self.assertEqual(b"53_0", entries['Y'])
+
+    def test_create_existing_error(self):
+        self.subject.create('clone', '1', 'a')
+
+        with self.assertRaises(fjournaler.JournalerException):
+            self.subject.create('clone', '1', 'a')
+
+    def test_remove_non_existing_error(self):
+        with self.assertRaises(fjournaler.JournalerException):
+            self.subject.remove('clone', '1')
