[PATCH 0/3] Test large offset

[Nir Soffer]

While working on python 3 port, we noticed that the python extension may
truncate large offset when converting python value to C value. Change
the tests to test also large offsets to make sure current code actually
works with offset > LONG_MAX.

Nir Soffer (3):
  tests: Allow testing large files
  tests: Remove unneeded str() conversion
  tests: Test reading and writing large offsets

 tests/python_test.py | 95 ++++++++++++++++++++++++++++++--------------
 tests/util.py        | 10 ++---
 2 files changed, 69 insertions(+), 36 deletions(-)

-- 
2.17.2

[PATCH 1/3] tests: Allow testing large files

[Nir Soffer]

util.create_file() was filling the file with "x". This works for small
files but prevent testing big files. Change to create sparse file so we
can test easily large files and offsets.

Tests that want to fill areas on storage can access the file directly.
We don't have such tests yet.

Signed-off-by: Nir Soffer <nsoffer(a)redhat.com&gt;
---
 tests/util.py | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/tests/util.py b/tests/util.py
index 2259473..a6b53e3 100644
--- a/tests/util.py
+++ b/tests/util.py
@@ -95,27 +95,23 @@ def wait_for_termination(p, timeout):
         if time.time() > deadline:
             raise TimeoutExpired
         time.sleep(0.05)
 
 
-def create_file(path, size, poison=b"x", guard=b"X",
guard_size=4096):
+def create_file(path, size, guard=b"X", guard_size=4096):
     """
-    Create file filled with poison byte.
-
-    To create an empty file, set poison to None.
+    Create sparse file of size bytes.
 
     If guard is set, add a guard area after the end of the file and fill it
     with guard bytes. This allows testing that the code under test do not write
     anything after the end of the file.
     """
     with io.open(path, "wb") as f:
         f.truncate(size)
-        if poison:
-            f.write(poison * size)
         if guard:
             f.seek(size)
-            f.write(guard * 4096)
+            f.write(guard * guard_size)
 
 
 def check_guard(path, size, guard=b"X", guard_size=4096):
     """
     Assert that a file ends with a guard area filled with guard bytes.
-- 
2.17.2

[PATCH 2/3] tests: Remove unneeded str() conversion

[Nir Soffer]

Two instances were forgottern in the last cleanup, removing them.

Signed-off-by: Nir Soffer <nsoffer(a)redhat.com&gt;
---
 tests/python_test.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/python_test.py b/tests/python_test.py
index 2dd4e17..be1e3ab 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -26,17 +26,17 @@ def test_write_lockspace(tmpdir, sanlock_daemon):
     assert ls == {"iotimeout": 1, "lockspace": "name"}
 
     acquired = sanlock.inq_lockspace("name", 1, path, wait=False)
     assert acquired is False
 
-    with io.open(str(path), "rb") as f:
+    with io.open(path, "rb") as f:
         magic, = struct.unpack("< I", f.read(4))
         assert magic == constants.DELTA_DISK_MAGIC
 
         # TODO: check more stuff here...
 
-    util.check_guard(str(path), size)
+    util.check_guard(path, size)
 
 
 def test_write_resource(tmpdir, sanlock_daemon):
     path = str(tmpdir.join("resources"))
     size = 1024**2
-- 
2.17.2

[PATCH 3/3] tests: Test reading and writing large offsets

[Nir Soffer]

The python module uses "k" format string[1] to convert lockspace offset
and PyInt_AsLong[2] to convert disk offset.  Both return long, while
sanlock offset use uint64_t.  This is not correct on all systems but I'm
not sure we support any system when sizeof(long) < 8.

Add tests for large offset exceeding LONG_MAX to check that current code
actually works. Vdsm does not use such offset yet, but when using 4k
sector size and 2 MiB alignment, vdsm will access such offset.

Theoretically we can test up to (16 TiB - 1 MiB) offset, but testing
show that practical maximum file size is about 15 TiB.  To make the test
less likely to fail on developer machine or CI slave, use 1 TiB file for
testing large offset.

[1] https://docs.python.org/2.7/c-api/arg.html
    (see k (integer) [unsigned long])
[2] https://docs.python.org/2.7/c-api/int.html#c.PyInt_AsLong

Signed-off-by: Nir Soffer <nsoffer(a)redhat.com&gt;
---
 tests/python_test.py | 91 +++++++++++++++++++++++++++++++-------------
 1 file changed, 64 insertions(+), 27 deletions(-)

diff --git a/tests/python_test.py b/tests/python_test.py
index be1e3ab..5029924 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -12,80 +12,111 @@ import pytest
 import sanlock
 
 from . import constants
 from . import util
 
-
-def test_write_lockspace(tmpdir, sanlock_daemon):
+# Largest file size on ext4 is 16TiB, and on xfs 500 TiB. Use 1 TiB as it is
+# large enough to test large offsets, and less likely to fail on developer
+# machine or CI slave.
+# See https://access.redhat.com/articles/rhel-limits
+LARGE_FILE_SIZE = 1024**4
+
+LOCKSPACE_SIZE = 1024**2
+MIN_RES_SIZE = 1024**2
+
+
+(a)pytest.mark.parametrize(&quot;size,offset&quot;, [
+    # Smallest offset.
+    (LOCKSPACE_SIZE, 0),
+    # Large offset.
+    (LARGE_FILE_SIZE, LARGE_FILE_SIZE - LOCKSPACE_SIZE),
+])
+def test_write_lockspace(tmpdir, sanlock_daemon, size, offset):
     path = str(tmpdir.join("lockspace"))
-    size = 1024**2
     util.create_file(path, size)
 
-    sanlock.write_lockspace("name", path, offset=0, iotimeout=1)
+    sanlock.write_lockspace("name", path, offset=offset, iotimeout=1)
 
-    ls = sanlock.read_lockspace(path, offset=0)
+    ls = sanlock.read_lockspace(path, offset=offset)
     assert ls == {"iotimeout": 1, "lockspace": "name"}
 
-    acquired = sanlock.inq_lockspace("name", 1, path, wait=False)
+    acquired = sanlock.inq_lockspace(
+        "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...
 
     util.check_guard(path, size)
 
 
-def test_write_resource(tmpdir, sanlock_daemon):
+(a)pytest.mark.parametrize(&quot;size,offset&quot;, [
+    # Smallest offset.
+    (MIN_RES_SIZE, 0),
+    # Large offset.
+    (LARGE_FILE_SIZE, LARGE_FILE_SIZE - MIN_RES_SIZE),
+])
+def test_write_resource(tmpdir, sanlock_daemon, size, offset):
     path = str(tmpdir.join("resources"))
-    size = 1024**2
     util.create_file(path, size)
-    disks = [(path, 0)]
+    disks = [(path, offset)]
 
     sanlock.write_resource("ls_name", "res_name", disks)
 
-    res = sanlock.read_resource(path, 0)
+    res = sanlock.read_resource(path, offset=offset)
     assert res == {
         "lockspace": "ls_name",
         "resource": "res_name",
         "version": 0
     }
 
     owners = sanlock.read_resource_owners("ls_name", "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
 
         # TODO: check more stuff here...
 
     util.check_guard(path, size)
 
 
-def test_add_rem_lockspace(tmpdir, sanlock_daemon):
+(a)pytest.mark.parametrize(&quot;size,offset&quot;, [
+    # Smallest offset.
+    (MIN_RES_SIZE, 0),
+    # Large offset.
+    (LARGE_FILE_SIZE, LARGE_FILE_SIZE - MIN_RES_SIZE),
+])
+def test_add_rem_lockspace(tmpdir, sanlock_daemon, size, offset):
     path = str(tmpdir.join("ls_name"))
-    util.create_file(path, 1024**2)
+    util.create_file(path, size)
 
-    sanlock.write_lockspace("ls_name", path, iotimeout=1)
+    sanlock.write_lockspace("ls_name", path, offset=offset, iotimeout=1)
 
     # Since the lockspace is not acquired, we exepect to get False.
-    acquired = sanlock.inq_lockspace("ls_name", 1, path, wait=False)
+    acquired = sanlock.inq_lockspace(
+        "ls_name", 1, path, offset=offset, wait=False)
     assert acquired is False
 
-    sanlock.add_lockspace("ls_name", 1, path, iotimeout=1)
+    sanlock.add_lockspace("ls_name", 1, path, offset=offset, iotimeout=1)
 
     # Once the lockspace is acquired, we exepect to get True.
-    acquired = sanlock.inq_lockspace("ls_name", 1, path, wait=False)
+    acquired = sanlock.inq_lockspace(
+        "ls_name", 1, path, offset=offset, wait=False)
     assert acquired is True
 
-    sanlock.rem_lockspace("ls_name", 1, path)
+    sanlock.rem_lockspace("ls_name", 1, path, offset=offset)
 
     # Once the lockspace is released, we exepect to get False.
-    acquired = sanlock.inq_lockspace("ls_name", 1, path, wait=False)
+    acquired = sanlock.inq_lockspace(
+        "ls_name", 1, path, offset=offset, wait=False)
     assert acquired is False
 
 
 def test_add_rem_lockspace_async(tmpdir, sanlock_daemon):
     path = str(tmpdir.join("ls_name"))
@@ -121,33 +152,39 @@ def test_add_rem_lockspace_async(tmpdir, sanlock_daemon):
     # Once the lockspace was released, we expect to get False.
     acquired = sanlock.inq_lockspace("ls_name", 1, path, wait=True)
     assert acquired is False
 
 
-def test_acquire_release_resource(tmpdir, sanlock_daemon):
+(a)pytest.mark.parametrize(&quot;size,offset&quot;, [
+    # Smallest offset.
+    (MIN_RES_SIZE, 0),
+    # Large offset.
+    (LARGE_FILE_SIZE, LARGE_FILE_SIZE - MIN_RES_SIZE),
+])
+def test_acquire_release_resource(tmpdir, sanlock_daemon, size, offset):
     ls_path = str(tmpdir.join("ls_name"))
-    util.create_file(ls_path, 1024**2)
+    util.create_file(ls_path, size)
 
     res_path = str(tmpdir.join("res_name"))
-    util.create_file(res_path, 1024**2)
+    util.create_file(res_path, size)
 
-    sanlock.write_lockspace("ls_name", ls_path, iotimeout=1)
-    sanlock.add_lockspace("ls_name", 1, ls_path, iotimeout=1)
+    sanlock.write_lockspace("ls_name", ls_path, offset=offset, iotimeout=1)
+    sanlock.add_lockspace("ls_name", 1, ls_path, offset=offset, iotimeout=1)
 
     # Host status is not available until the first renewal.
     with pytest.raises(sanlock.SanlockException) as e:
         sanlock.get_hosts("ls_name", 1)
     assert e.value.errno == errno.EAGAIN
 
     time.sleep(1)
     host = sanlock.get_hosts("ls_name", 1)[0]
     assert host["flags"] == sanlock.HOST_LIVE
 
-    disks = [(res_path, 0)]
+    disks = [(res_path, offset)]
     sanlock.write_resource("ls_name", "res_name", disks)
 
-    res = sanlock.read_resource(res_path, 0)
+    res = sanlock.read_resource(res_path, offset=offset)
     assert res == {
         "lockspace": "ls_name",
         "resource": "res_name",
         "version": 0
     }
@@ -156,11 +193,11 @@ def test_acquire_release_resource(tmpdir, sanlock_daemon):
     assert owners == []
 
     fd = sanlock.register()
     sanlock.acquire("ls_name", "res_name", disks, slkfd=fd)
 
-    res = sanlock.read_resource(res_path, 0)
+    res = sanlock.read_resource(res_path, offset=offset)
     assert res == {
         "lockspace": "ls_name",
         "resource": "res_name",
         "version": 1
     }
@@ -177,11 +214,11 @@ def test_acquire_release_resource(tmpdir, sanlock_daemon):
     assert host["flags"] == sanlock.HOST_LIVE
     assert host["generation"] == owner["generation"]
 
     sanlock.release("ls_name", "res_name", disks, slkfd=fd)
 
-    res = sanlock.read_resource(res_path, 0)
+    res = sanlock.read_resource(res_path, offset=offset)
     assert res == {
         "lockspace": "ls_name",
         "resource": "res_name",
         "version": 1
     }
-- 
2.17.2