Fix loading of cache in pyocf

Flush/load metadata paths are heavily dependend on Data behaving
correctly in terms of seeks/position and that needed to be fixed.

Signed-off-by: Jan Musial <jan.musial@intel.com>

tests/functional/pyocf/types/cache.py

@@ -191,11 +191,10 @@ class Cache:
             self.cache_handle, cache_mode
         )
 
-        if status:
         self.put_and_write_unlock()
-            raise OcfError("Error changing cache mode", status)
 
-        self.put_and_write_unlock()
+        if status:
+            raise OcfError("Error changing cache mode", status)
 
     def set_cleaning_policy(self, cleaning_policy: CleaningPolicy):
         self.get_and_write_lock()
@@ -203,12 +202,12 @@ class Cache:
         status = self.owner.lib.ocf_mngt_cache_cleaning_set_policy(
             self.cache_handle, cleaning_policy
         )
-        if status:
-            self.put_and_write_unlock()
-            raise OcfError("Error changing cleaning policy", status)
 
         self.put_and_write_unlock()
 
+        if status:
+            raise OcfError("Error changing cleaning policy", status)
+
     def set_cleaning_policy_param(
         self, cleaning_policy: CleaningPolicy, param_id, param_value
     ):
@@ -217,26 +216,26 @@ class Cache:
         status = self.owner.lib.ocf_mngt_cache_cleaning_set_param(
             self.cache_handle, cleaning_policy, param_id, param_value
         )
-        if status:
-            self.put_and_write_unlock()
-            raise OcfError("Error setting cleaning policy param", status)
 
         self.put_and_write_unlock()
 
+        if status:
+            raise OcfError("Error setting cleaning policy param", status)
+
     def set_seq_cut_off_policy(self, policy: SeqCutOffPolicy):
         self.get_and_write_lock()
 
         status = self.owner.lib.ocf_mngt_core_set_seq_cutoff_policy_all(
             self.cache_handle, policy
         )
-        if status:
+
         self.put_and_write_unlock()
+
+        if status:
             raise OcfError("Error setting cache seq cut off policy", status)
 
-        self.put_and_write_unlock()
-
     def configure_device(
-        self, device, force=False, perform_test=False, cache_line_size=None
+        self, device, force=False, perform_test=True, cache_line_size=None
     ):
         self.device = device
         self.device_name = device.uuid
@@ -273,11 +272,10 @@ class Cache:
         )
 
         c.wait()
-        if c.results["error"]:
         self.put_and_write_unlock()
-            raise OcfError("Attaching cache device failed", c.results["error"])
 
-        self.put_and_write_unlock()
+        if c.results["error"]:
+            raise OcfError("Attaching cache device failed", c.results["error"])
 
     def load_cache(self, device):
         self.configure_device(device)
@@ -297,7 +295,12 @@ class Cache:
         c = cls(name=name, owner=device.owner)
 
         c.start_cache()
+        try:
             c.load_cache(device)
+        except:
+            c.stop()
+            raise
+
         return c
 
     @classmethod
@@ -388,14 +391,13 @@ class Cache:
         self.owner.lib.ocf_mngt_cache_remove_core(core.handle, c, None)
 
         c.wait()
-        if c.results["error"]:
         self.put_and_write_unlock()
+
+        if c.results["error"]:
             raise OcfError("Failed removing core", c.results["error"])
 
         self.cores.remove(core)
 
-        self.put_and_write_unlock()
-
     def get_stats(self):
         cache_info = CacheInfo()
         usage = UsageStats()
@@ -472,6 +474,22 @@ class Cache:
 
         return self.io_queues[0]
 
+    def save(self):
+        if not self.started:
+            raise Exception("Not started!")
+
+        self.get_and_write_lock()
+        c = OcfCompletion(
+            [("cache", c_void_p), ("priv", c_void_p), ("error", c_int)]
+        )
+        self.owner.lib.ocf_mngt_cache_save(self.cache_handle, c, None)
+
+        c.wait()
+        self.put_and_write_unlock()
+
+        if c.results["error"]:
+            raise OcfError("Failed saving cache", c.results["error"])
+
     def stop(self):
         if not self.started:
             raise Exception("Already stopped!")
@@ -505,11 +523,11 @@ class Cache:
         )
         self.owner.lib.ocf_mngt_cache_flush(self.cache_handle, c, None)
         c.wait()
-        if c.results["error"]:
         self.put_and_write_unlock()
+
+        if c.results["error"]:
             raise OcfError("Couldn't flush cache", c.results["error"])
 
-        self.put_and_write_unlock()
 
     def get_name(self):
         self.get_and_read_lock()

tests/functional/pyocf/types/data.py

@@ -56,6 +56,7 @@ class DataOps(Structure):
 
 
 class Data:
+    DATA_POISON=0xA5
     PAGE_SIZE = 4096
 
     _instances_ = {}
@@ -66,7 +67,8 @@ class Data:
         self.position = 0
         self.buffer = create_string_buffer(int(self.size))
         self.handle = cast(byref(self.buffer), c_void_p)
-        memset(self.handle, 0, self.size)
+
+        memset(self.handle, self.DATA_POISON, self.size)
         type(self)._instances_[self.handle.value] = weakref.ref(self)
         self._as_parameter_ = self.handle
 
@@ -105,13 +107,6 @@ class Data:
     def from_string(cls, source: str, encoding: str = "ascii"):
         return cls.from_bytes(bytes(source, encoding))
 
-    def set_data(self, contents):
-        if len(contents) > self.size:
-            raise Exception("Data too big to fit into allocated buffer")
-
-        memmove(self.handle, cast(contents, c_void_p), len(contents))
-        self.position = 0
-
     @staticmethod
     @DataOps.ALLOC
     def _alloc(pages):
@@ -170,11 +165,15 @@ class Data:
     def read(self, dst, size):
         to_read = min(self.size - self.position, size)
         memmove(dst, self.handle.value + self.position, to_read)
+
+        self.position += to_read
         return to_read
 
     def write(self, src, size):
         to_write = min(self.size - self.position, size)
         memmove(self.handle.value + self.position, src, to_write)
+
+        self.position += to_write
         return to_write
 
     def mlock(self):
@@ -186,6 +185,8 @@ class Data:
     def zero(self, size):
         to_zero = min(self.size - self.position, size)
         memset(self.handle.value + self.position, 0, to_zero)
+
+        self.position += to_zero
         return to_zero
 
     def seek(self, seek, size):
@@ -207,8 +208,8 @@ class Data:
     def secure_erase(self):
         pass
 
-    def dump(self):
+    def dump(self, ignore=DATA_POISON, **kwargs):
-        print_buffer(self.buffer, self.size)
+        print_buffer(self.buffer, self.size, ignore=ignore, **kwargs)
 
     def md5(self):
         m = md5()

tests/functional/pyocf/types/volume.py

@@ -74,6 +74,8 @@ class VolumeIoPriv(Structure):
 
 
 class Volume(Structure):
+    VOLUME_POISON=0x13
+
     _fields_ = [("_storage", c_void_p)]
     _instances_ = {}
     _uuid_ = {}
@@ -95,6 +97,7 @@ class Volume(Structure):
         type(self)._uuid_[self.uuid] = weakref.ref(self)
 
         self.data = create_string_buffer(int(self.size))
+        memset(self.data, self.VOLUME_POISON, self.size)
         self._storage = cast(self.data, c_void_p)
 
         self.reset_stats()
@@ -249,7 +252,7 @@ class Volume(Structure):
     def submit_discard(self, discard):
         try:
             dst = self._storage + discard.contents._addr
-            memset(dst, discard.contents._bytes)
+            memset(dst, 0, discard.contents._bytes)
 
             discard.contents._end(discard, 0)
         except:
@@ -280,14 +283,15 @@ class Volume(Structure):
         except:
             io.contents._end(io, -5)
 
-    def dump_contents(self, stop_after_zeros=0, offset=0, size=0):
+    def dump(self, offset=0, size=0, ignore=VOLUME_POISON, **kwargs):
         if size == 0:
             size = int(self.size) - int(offset)
+
         print_buffer(
             self._storage,
-            int(size),
+            size,
-            offset=int(offset),
+            ignore=ignore,
-            stop_after_zeros=int(stop_after_zeros),
+            **kwargs
         )
 
     def md5(self):
@@ -315,6 +319,20 @@ class ErrorDevice(Volume):
         super().reset_stats()
         self.stats["errors"] = {IoDir.WRITE: 0, IoDir.READ: 0}
 
+class TraceDevice(Volume):
+    def __init__(self, size, trace_fcn=None, uuid=None):
+        super().__init__(size, uuid)
+        self.trace_fcn=trace_fcn
+
+    def submit_io(self, io):
+        submit = True
+
+        if self.trace_fcn:
+            submit = self.trace_fcn(self, io)
+
+        if submit:
+            super().submit_io(io)
+
 
 lib = OcfLib.getInstance()
 lib.ocf_io_get_priv.restype = POINTER(VolumeIoPriv)

tests/functional/pyocf/utils.py

@@ -6,31 +6,32 @@
 from ctypes import string_at
 
 
-def print_buffer(buf, length, offset=0, width=16, stop_after_zeros=0):
+def print_buffer(buf, length, offset=0, width=16, ignore=0, stop_after_count_ignored=0, print_fcn=print):
-    end = offset + length
+    end = int(offset) + int(length)
-    zero_lines = 0
+    offset = int(offset)
+    ignored_lines = 0
     buf = string_at(buf, length)
-    whole_buffer_empty = True
+    whole_buffer_ignored = True
-    stop_after_zeros = int(stop_after_zeros / width)
+    stop_after_count_ignored = int(stop_after_count_ignored / width)
 
     for addr in range(offset, end, width):
         cur_line = buf[addr : min(end, addr + width)]
         byteline = ""
         asciiline = ""
-        if not any(cur_line):
+        if not any(x != ignore for x in cur_line):
-            if stop_after_zeros and zero_lines > stop_after_zeros:
+            if stop_after_count_ignored and ignored_lines > stop_after_count_ignored:
-                print(
+                print_fcn(
-                    "<{} bytes of empty space encountered, stopping>".format(
+                        "<{} bytes of '0x{:02X}' encountered, stopping>".format(
-                        stop_after_zeros * width
+                        stop_after_count_ignored * width, ignore
                     )
                 )
                 return
-            zero_lines += 1
+            ignored_lines += 1
             continue
 
-        if zero_lines:
+        if ignored_lines:
-            print("<{} zero bytes omitted>".format(zero_lines * width))
+            print_fcn("<{} of '0x{:02X}' bytes omitted>".format(ignored_lines * width, ignore))
-            zero_lines = 0
+            ignored_lines = 0
 
         for byte in cur_line:
             byte = int(byte)
@@ -41,13 +42,13 @@ def print_buffer(buf, length, offset=0, width=16, stop_after_zeros=0):
                 char = "."
             asciiline += char
 
-        print("0x{:08X}\t{}\t{}".format(addr, byteline, asciiline))
+        print_fcn("0x{:08X}\t{}\t{}".format(addr, byteline, asciiline))
-        whole_buffer_empty = False
+        whole_buffer_ignored = False
 
-    if whole_buffer_empty:
+    if whole_buffer_ignored:
-        print("<whole buffer empty>")
+        print_fcn("<whole buffer ignored>")
-    elif zero_lines:
+    elif ignored_lines:
-        print("<zero until end>")
+        print_fcn("<'0x{:02X}' until end>".format(ignore))
 
 
 class Size:

tests/functional/tests/basic/test_pyocf.py

@@ -66,8 +66,6 @@ def test_load_cache_no_preexisting_data(pyocf_ctx):
         cache = Cache.load_from_device(cache_device)
 
 
-# TODO: Find out why this fails and fix
-@pytest.mark.xfail
 def test_load_cache(pyocf_ctx):
     cache_device = Volume(S.from_MiB(30))