On Sun, May 12, 2019 at 6:54 PM Nir Soffer <nirsof@gmail.com> wrote:

We added API for using 4k sector size and various alignments, but there
are no tests for the new APIs with disks using 4k sector size. Testing
this is complicated since creating a block device and mounting file
systems requires root, and we like to run the tests as a regular user.

Add new tests for writing and reading lockspace and resource using 4k
storage provided by the user. If a path is not specified the tests are
skipped.

To run the tests, you need to create a block device using 4k sector size
and optionally create a file system and mount it. The process is
explained in README.dev.

Then pass the path of the block device or file on the new file system to
the tests:

    USER_4K_PATH=/dev/loop2 tox -e py27

Adding and removing lockspace and acquiring resources is not tested yet
with 4k storage.

Signed-off-by: Nir Soffer <nsoffer@redhat.com>
---
 README.dev           | 44 ++++++++++++++++++++++++++++++
 tests/conftest.py    | 36 ++++++++++++++++++++++++
 tests/python_test.py | 65 ++++++++++++++++++++++++++++++++++++++++++++
 tests/util.py        | 34 +++++++++++++++--------
 tox.ini              |  2 +-
 5 files changed, 169 insertions(+), 12 deletions(-)

diff --git a/README.dev b/README.dev
index 3519993..bfb170c 100644
--- a/README.dev
+++ b/README.dev
@@ -22,5 +22,49 @@ To run only test from some modules:
     $ tox tests/daemon_test.py

 To run only tests matching the substring "foo":

     $ tox -- -k foo
+
+
+Testing 4K support
+==================
+
+This requires manual setup for creating a block device using 4k sector size,
+optionally creating and mounting a file system, and passing the test path to
+tests. The setup must be done as root, but the tests can run later as the
+current user.
+
+Common setup
+------------
+
+1. Create a backing file:
+
+    $ truncate -s 1G /var/tmp/backing
+
+2. Create a loop device with 4k sector size:
+
+    $ sudo losetup -f /var/tmp/backing --show --sector-size=4096
+    /dev/loop2
+
+3. Change the device (or mountpoint) owner to current user
+
+    $ sudo chown $USER:$USER /dev/loop2
+
+Testing 4k block device
+-----------------------
+
+Run the tests with USER_4K_PATH environment variable:
+
+    $ USER_4K_PATH=/dev/loop2 tox -e py27
+
+Testing 4k file system
+----------------------
+
+To test file system on top of 4k block device, create a file system on the
+device, mount it, and create a file for testing lockspace or resources:
+
+    $ sudo mkfs.xfs /dev/loop2
+    $ sudo mount /dev/loop2 /tmp/sanlock-4k
+    $ sudo chown $USER:$USER /tmp/sanlock-4k
+    $ truncate -s 128m /tmp/sanlock-4k/disk
+    $ USER_4K_PATH=/tmp/sanlock-4k/disk tox -e py27
diff --git a/tests/conftest.py b/tests/conftest.py
index 6b8b0e8..721ee41 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -1,9 +1,12 @@
 """
 Fixtures for sanlock testing.
 """

+import os
+import stat
+
 import pytest

 from . import util


@@ -19,5 +22,38 @@ def sanlock_daemon():
     finally:
         # Killing sanlock allows terminating without reomving the lockspace,
         # which takes about 3 seconds, slowing down the tests.
         p.kill()
         p.wait()
+
+
+@pytest.fixture
+def user_4k_path():
+    """
+    A path to block device or file on file system on top of 4k block device,
+    provided by the user.
+
+    The user must create the block device or the file system before running the
+    tests, and specify the path to the file in the USER_4K_PATH environment
+    variable.
+
+    If USER_4K_PATH was not specified, tests using this fixture will be skipped.
+    If USER_4K_PATH was specified but does not exist, or is not a file or block
+    device, RuntimeError is raised.
+
+    Return path to the user specifed file.

Typo: "specifed" --> "specified".

 

+    """
+    path = os.environ.get("USER_4K_PATH")
+    if path is None:
+        pytest.skip("USER_4K_PATH pointing to a 4k block device or file was "
+                    "not specified")
+
+    if not os.path.exists(path):
+        raise RuntimeError("USER_4K_PATH {!r} does not exist".format(path))
+
+    mode = os.stat(path).st_mode
+    if not (stat.S_ISBLK(mode) or stat.S_ISREG(mode)):
+        raise RuntimeError(
+            "USER_4K_PATH {!r} is not a block device or regular file"
+            .format(path))
+
+    return path
diff --git a/tests/python_test.py b/tests/python_test.py
index 1a6bbbd..0681570 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -23,10 +23,11 @@ LARGE_FILE_SIZE = 1024**4
 LOCKSPACE_SIZE = 1024**2
 MIN_RES_SIZE = 1024**2

 ALIGNMENT_1M = 1024**2
 SECTOR_SIZE_512 = 512
+SECTOR_SIZE_4K = 4096

Need to remember change to constants from "units.py".

 

 @pytest.mark.parametrize("size,offset", [
     # Smallest offset.
     (LOCKSPACE_SIZE, 0),
@@ -64,10 +65,39 @@ def test_write_lockspace(tmpdir, sanlock_daemon, size, offset):
         # TODO: check more stuff here...

     util.check_guard(path, size)


+@pytest.mark.parametrize("align", sanlock.ALIGN_SIZE)
+def test_write_lockspace_4k(user_4k_path, sanlock_daemon, align):
+
+    # Postion lockspace area, ensuring that previous tests will not break this

Typo: "Postion" --> "Position"

 

+    # test, and sanlock does not write beyond the lockspace area.
+    with io.open(user_4k_path, "rb+") as f:
+        f.write(align * b"x")
+    util.write_guard(user_4k_path, align)
+
+    sanlock.write_lockspace(
+        "name", user_4k_path, iotimeout=1, align=align, sector=SECTOR_SIZE_4K)
+
+    ls = sanlock.read_lockspace(
+        user_4k_path, align=align, sector=SECTOR_SIZE_4K)
+
+    assert ls == {"iotimeout": 1, "lockspace": "name"}
+
+    acquired = sanlock.inq_lockspace("name", 1, user_4k_path, wait=False)
+    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
+
+    # Check that sanlock did not write beyond the lockspace area.
+    util.check_guard(user_4k_path, align)
+
+
 @pytest.mark.parametrize("size,offset", [
     # Smallest offset.
     (MIN_RES_SIZE, 0),
     # Large offset.
     (LARGE_FILE_SIZE, LARGE_FILE_SIZE - MIN_RES_SIZE),
@@ -111,10 +141,45 @@ def test_write_resource(tmpdir, sanlock_daemon, size, offset):
         # TODO: check more stuff here...

     util.check_guard(path, size)


+@pytest.mark.parametrize("align", sanlock.ALIGN_SIZE)
+def test_write_resource_4k(sanlock_daemon, user_4k_path, align):
+    disks = [(user_4k_path, 0)]
+
+    # Postion resource area, ensuring that previous tests will not break this

Typo.

 

+    # test, and sanlock does not write beyond the lockspace area.
+    with io.open(user_4k_path, "rb+") as f:
+        f.write(align * b"x")

Is it possible to use "GUARD" from the "util.py"?

 

+    util.write_guard(user_4k_path, align)
+
+    sanlock.write_resource(
+        "ls_name", "res_name", disks, align=align, sector=SECTOR_SIZE_4K)
+
+    res = sanlock.read_resource(
+        user_4k_path, align=align, sector=SECTOR_SIZE_4K)
+
+    assert res == {
+        "lockspace": "ls_name",
+        "resource": "res_name",
+        "version": 0
+    }
+
+    owners = sanlock.read_resource_owners(
+        "ls_name", "res_name", disks, align=align, sector=SECTOR_SIZE_4K)
+    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
+
+    # Check that sanlock did not write beyond the lock space area.
+    util.check_guard(user_4k_path, align)
+
+
 @pytest.mark.parametrize("size,offset", [
     # Smallest offset.
     (MIN_RES_SIZE, 0),
     # Large offset.
     (LARGE_FILE_SIZE, LARGE_FILE_SIZE - MIN_RES_SIZE),
diff --git a/tests/util.py b/tests/util.py
index a6b53e3..0695d8c 100644
--- a/tests/util.py
+++ b/tests/util.py
@@ -11,10 +11,13 @@ import subprocess
 import time

 TESTDIR = os.path.dirname(__file__)
 SANLOCK = os.path.join(TESTDIR, os.pardir, "src", "sanlock")

+GUARD = b"X"
+GUARD_SIZE = 4096

Remember to change to constant from "units.py": 4096 --> 4 * KiB

 

+

 class TimeoutExpired(Exception):
     """ Raised when timeout expired """


@@ -95,32 +98,41 @@ def wait_for_termination(p, timeout):
         if time.time() > deadline:
             raise TimeoutExpired
         time.sleep(0.05)


-def create_file(path, size, guard=b"X", guard_size=4096):

Can use "GUARD" instead of b"X"?

Can use "GUARD_SIZE" instead of 4096?

 

+def create_file(path, size, guard=True):
     """
     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.
+    If guard is True, add a guard area beyond the end of the file.
     """
     with io.open(path, "wb") as f:
         f.truncate(size)
-        if guard:
-            f.seek(size)
-            f.write(guard * guard_size)
+
+    if guard:
+        write_guard(path, size)
+
+
+def write_guard(path, offset):
+    """
+    Write guard areas at offset and fill with guard byte.
+
+    Use check_guard() to verify that nothing was written to the guard area.
+    """
+    with io.open(path, "rb+") as f:
+        f.seek(offset)
+        f.write(GUARD * GUARD_SIZE)


-def check_guard(path, size, guard=b"X", guard_size=4096):
+def check_guard(path, offset):
     """
-    Assert that a file ends with a guard area filled with guard bytes.
+    Assert that guard area at offset was not modified.
     """
     with io.open(path, "rb") as f:
-        f.seek(size)
-        assert f.read() == guard * guard_size
+        f.seek(GUARD)
+        assert f.read(GUARD_SIZE) == GUARD * GUARD_SIZE


 def check_rindex_entry(entry, name, offset=None, flags=None):
     # See src/ondisk.c rindex_entry_in()
     e_offset, e_flags, e_unused, e_name = struct.unpack("<Q L L 48s", entry)
diff --git a/tox.ini b/tox.ini
index 1c2349e..d21f2f3 100644
--- a/tox.ini
+++ b/tox.ini
@@ -7,11 +7,11 @@
 envlist = py27,py36
 skipsdist = True
 skip_missing_interpreters = True

 [testenv]
-passenv = USER
+passenv = *
 setenv =
     LD_LIBRARY_PATH={env:PWD}/wdmd:{env:PWD}/src
     SANLOCK_PRIVILEGED=0
     SANLOCK_RUN_DIR=/tmp/sanlock
 whitelist_externals = make
--
2.17.2