On Sat, Jun 15, 2019 at 1:02 AM Nir Soffer <nsoffer@redhat.com> wrote:

On Sat, Jun 15, 2019 at 12:55 AM Vojtech Juranek <vjuranek@redhat.com> wrote:

Introduce helper function for reading sanlock magic numbers. This
helper function seeks to specified offset in given file and reads
unsigned int from this position.
---
 tests/python_test.py | 25 +++++++++----------------
 tests/util.py        | 11 +++++++++++
 2 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/tests/python_test.py b/tests/python_test.py
index 3242f57..623a13c 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -86,12 +86,9 @@ def test_write_lockspace(tmpdir, sanlock_daemon, filename, encoding, size, offse
         b"ls_name", 1, path, offset=offset, wait=False)
     assert acquired is False

-    with io.open(path, "rb") as f:
-        f.seek(offset)
-        magic, = struct.unpack("< I", f.read(4))
-        assert magic == constants.DELTA_DISK_MAGIC
-
-        # TODO: check more stuff here...
+    magic = util.read_uint(path, offset)
+    assert magic == constants.DELTA_DISK_MAGIC
+    # TODO: check more stuff here...

     util.check_guard(path, size)

@@ -117,9 +114,8 @@ def test_write_lockspace_4k(user_4k_path, sanlock_daemon, align):
     assert acquired is False

     # Verify that lockspace was written.
-    with io.open(user_4k_path, "rb") as f:
-        magic, = struct.unpack("< I", f.read(4))
-        assert magic == constants.DELTA_DISK_MAGIC
+    magic = util.read_uint(user_4k_path)
+    assert magic == constants.DELTA_DISK_MAGIC

     # Check that sanlock did not write beyond the lockspace area.
     util.check_guard(user_4k_path, align)
@@ -179,10 +175,8 @@ def test_write_resource(tmpdir, sanlock_daemon, filename, encoding, size, offset
     owners = sanlock.read_resource_owners(b"ls_name", b"res_name", disks)
     assert owners == []

-    with io.open(path, "rb") as f:
-        f.seek(offset)
-        magic, = struct.unpack("< I", f.read(4))
-        assert magic == constants.PAXOS_DISK_MAGIC
+    magic = util.read_uint(path, offset)
+    assert magic == constants.PAXOS_DISK_MAGIC

         # TODO: check more stuff here...

@@ -216,9 +210,8 @@ def test_write_resource_4k(sanlock_daemon, user_4k_path, align):
     assert owners == []

     # Verify that resource was written.
-    with io.open(user_4k_path, "rb") as f:
-        magic, = struct.unpack("< I", f.read(4))
-        assert magic == constants.PAXOS_DISK_MAGIC
+    magic = util.read_uint(user_4k_path)
+    assert magic == constants.PAXOS_DISK_MAGIC

     # Check that sanlock did not write beyond the lockspace area.
     util.check_guard(user_4k_path, align)
diff --git a/tests/util.py b/tests/util.py
index 08db236..32b34ec 100644
--- a/tests/util.py
+++ b/tests/util.py
@@ -177,3 +177,14 @@ def sanlock_is_running():
         if e.errno != errno.ENOENT:
             raise
     return False
+
+
+def read_uint(path, offset=0):
+    """
+    Read and return unsigned int from specified file at given offset.
+    Typical usage is to read sanlock magic number.
+    """
+    with io.open(path, "rb") as f:
+        f.seek(offset)
+        uint, = struct.unpack("< I", f.read(4))
+        return uint
--
2.20.1

Looks good, but maybe call it read_magic(path, offset)?

Looks good.

Regarding the Nir's comment, suggest "read_magic_int".