On Mon, May 13, 2019 at 4:28 PM Pavel Bar <pbar(a)redhat.com> wrote:
>
>
> On Sun, May 12, 2019 at 6:54 PM Nir Soffer <nirsof(a)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(a)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()
>> +
>> +
>> +(a)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".
>
Please update your PR once we merge this.
>
>
>>
>>
>> @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)
>>
>>
>> +(a)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"
>
Should be "poison" I'll fix this when I push.
>
>
>> + # 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)
>>
>>
>> +(a)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.
>
Yea, same.
>
>
>> + # 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"?
>
This is the poison byte "x", not the guard byte "X".
Maybe we should add later a constant or a helper like:
util.poison(path, offset, size)
>
>
>> + 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
>>
>>