On Mon, May 20, 2019 at 1:34 PM Vojtech Juranek <vjuranek@redhat.com> wrote:
Looks good to me

Thanks, pushed now.

> Previously you had to setup the environment manually to enable the 4k
> tests, and pass environment variable to tests. The tests used single
> path, so you had to run the tests twice to test both 4k block device and
> filesystem backed by a 4k block device.
>
> Add storage helper script to make it easier to setup storage for the
> tests.
>
> To setup 4k storage for the tests, run:
>
>    $ python tests/storage.py setup
>
> This script creates both a 4k block device and filesystem backed by a 4k
> device. The user_4k_path() fixture enables the tests if the device and
> mount exist, and runs the test twice, once with the loop device, and
> once with the filesystem.
>
> To clean up the environment, run:
>
>    $ python tests/storage.py teardown
>
> Signed-off-by: Nir Soffer <nsoffer@redhat.com>
> ---
>
> Changes since v1:
> - Fix device checks in teardown (Amit)
> - Fix typos (Pavel)
> - Merge setup and teardown to single script (Pavel)
> - Rewrite helper script in python
>
> v1 was here:
> https://lists.fedorahosted.org/archives/list/sanlock-devel@lists.fedorahoste
> d.org/thread/QMGOQBIVL5C6XM7WNJXY7GDLXWDKAHIQ/

>  README.dev        |  44 ++++----------
>  tests/conftest.py |  38 ++++---------
>  tests/storage.py  | 142 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 165 insertions(+), 59 deletions(-)
>  create mode 100644 tests/storage.py
>
> diff --git a/README.dev b/README.dev
> index bfb170c..439b7b4 100644
> --- a/README.dev
> +++ b/README.dev
> @@ -27,44 +27,22 @@ To run only tests matching the substring "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.
> +To enable the 4k tests, you need to setup 4k stroage for the tests:

> -Common setup
> -------------
> +    $ python tests/storage.py setup

> -1. Create a backing file:
> +This creates two loop devices with 4k sector size; one for testing sanlock
> with
 +4k block device, and the other for testing with a filesystem backed
> by a 4k +block device.

> -    $ truncate -s 1G /var/tmp/backing
> +To teardown the storage:

> -2. Create a loop device with 4k sector size:
> +    $ python tests/storage.py teardown

> -    $ sudo losetup -f /var/tmp/backing --show --sector-size=4096
> -    /dev/loop2
> +The script unmounts the filesystem and detaches the loop devices.

> -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
> +The storage helper script uses sudo to perform privileged operations. The
> best
 +way to use it is to setup the environment once at the start of the
> session, and +teardown when you finish.
> diff --git a/tests/conftest.py b/tests/conftest.py
> index bb95d45..5212679 100644
> --- a/tests/conftest.py
> +++ b/tests/conftest.py
> @@ -5,10 +5,11 @@ Fixtures for sanlock testing.
>  import os
>  import stat

>  import pytest

> +from . import storage
>  from . import util


>  @pytest.fixture
>  def sanlock_daemon():
> @@ -24,36 +25,21 @@ def sanlock_daemon():
>          # which takes about 3 seconds, slowing down the tests.
>          p.kill()
>          p.wait()


> -@pytest.fixture
> -def user_4k_path():
> +@pytest.fixture(params=[
> +    pytest.param(storage.BLOCK, id="block"),
> +    pytest.param(storage.FILE, id="file"),
> +])
> +def user_4k_path(request):
>      """
>      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 specified file.
> +    If storage is not available, skip the tests.
>      """
> -    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
> +    if not os.path.exists(request.param):
> +        pytest.skip(
> +            "user storage available - run 'python tests/strorage.py setup'
> "
 +            "to enable 4k storage tests")
> +    return request.param
> diff --git a/tests/storage.py b/tests/storage.py
> new file mode 100644
> index 0000000..3258ed4
> --- /dev/null
> +++ b/tests/storage.py
> @@ -0,0 +1,142 @@
> +"""
> +storage - provide storage for sanlock tests.
> +"""
> +
> +import argparse
> +import errno
> +import logging
> +import os
> +import subprocess
> +
> +BASE_DIR = "/var/tmp/sanlock-storage"
> +MOUNTPOINT = os.path.join(BASE_DIR, "mnt")
> +
> +# For testing sanlock with 4k block device.
> +LOOP1 = os.path.join(BASE_DIR, "loop1")
> +BACKING1 = os.path.join(BASE_DIR, "backing1")
> +
> +# For testing sanlock with a filesystem backed by 4k block device.
> +LOOP2 = os.path.join(BASE_DIR, "loop2")
> +BACKING2 = os.path.join(BASE_DIR, "backing2")
> +
> +# Test paths.
> +BLOCK = LOOP1
> +FILE = os.path.join(MOUNTPOINT, "file")
> +
> +log = logging.getLogger("storage")
> +
> +
> +def main():
> +    parser = argparse.ArgumentParser(
> +        description='Storage helper for sanlock tests')
> +    parser.add_argument("command", choices=["setup", "teardown"])
> +    args = parser.parse_args()
> +
> +    logging.basicConfig(level=logging.INFO, format="storage: %(message)s")
> +
> +    if args.command == "setup":
> +        setup()
> +    elif args.command == "teardown":
> +        teardown()
> +
> +
> +def setup():
> +    create_dir(BASE_DIR)
> +
> +    if not os.path.exists(LOOP1):
> +        create_loop_device(LOOP1, BACKING1)
> +
> +    if not os.path.exists(LOOP2):
> +        create_loop_device(LOOP2, BACKING2)
> +        create_dir(MOUNTPOINT)
> +        create_filesystem(LOOP2, MOUNTPOINT)
> +
> +        # Sanlock allocates spaces as needed.
> +        with open(FILE, "wb") as f:
> +            f.truncate(0)
> +
> +
> +def teardown():
> +    if is_mounted(MOUNTPOINT):
> +        remove_filesystem(MOUNTPOINT)
> +
> +    if os.path.exists(LOOP2):
> +        remove_loop_device(LOOP2, BACKING2)
> +
> +    if os.path.exists(LOOP1):
> +        remove_loop_device(LOOP1, BACKING1)
> +
> +
> +def create_loop_device(link_path, backing_file, size=1024**3,
> +                       sector_size=4096):
> +    log.info("Creating loop device %s", link_path)
> +
> +    with open(backing_file, "wb") as f:
> +        f.truncate(size)
> +
> +    out = subprocess.check_output([
> +        "sudo",
> +        "losetup",
> +        "-f", backing_file,
> +        "--sector-size", str(sector_size),
> +        "--show",
> +    ])
> +
> +    device = out.decode("utf-8").strip()
> +
> +    os.symlink(device, link_path)
> +    chown(link_path)
> +
> +
> +def remove_loop_device(link_path, backing_file):
> +    log.info("Removing loop device %s", link_path)
> +
> +    subprocess.check_call(["sudo", "losetup", "-d", link_path])
> +    remove_file(link_path)
> +    remove_file(backing_file)
> +
> +
> +def create_filesystem(device, mountpoint):
> +    log.info("Creating filesystem %s", mountpoint)
> +
> +    subprocess.check_call(["sudo", "mkfs.xfs", "-q", device])
> +    subprocess.check_call(["sudo", "mount", device, mountpoint])
> +    chown(mountpoint)
> +
> +
> +def remove_filesystem(mountpoint):
> +    log.info("Removing filesystem %s", mountpoint)
> +    subprocess.check_call(["sudo", "umount", mountpoint])
> +
> +
> +def is_mounted(mountpoint):
> +    with open("/proc/self/mounts") as f:
> +        for line in f:
> +            if mountpoint in line:
> +                return True
> +    return False
> +
> +
> +def chown(path):
> +    user_group = "%(USER)s:%(USER)s" % os.environ
> +    subprocess.check_call(["sudo", "chown", user_group, path])
> +
> +
> +def create_dir(path):
> +    try:
> +        os.makedirs(path)
> +    except EnvironmentError as e:
> +        if e.errno != errno.EEXIST:
> +            raise
> +
> +
> +def remove_file(path):
> +    try:
> +        os.remove(path)
> +    except EnvironmentError as e:
> +        if e.errno != errno.ENOENT:
> +            raise
> +
> +
> +if __name__ == "__main__":
> +    main()
> --
> 2.17.2
> _______________________________________________
> sanlock-devel mailing list -- sanlock-devel@lists.fedorahosted.org
> To unsubscribe send an email to sanlock-devel-leave@lists.fedorahosted.org
> Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
> List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
> List Archives:
> https://lists.fedorahosted.org/archives/list/sanlock-devel@lists.fedorahost
> ed.org

_______________________________________________
sanlock-devel mailing list -- sanlock-devel@lists.fedorahosted.org
To unsubscribe send an email to sanlock-devel-leave@lists.fedorahosted.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: https://lists.fedorahosted.org/archives/list/sanlock-devel@lists.fedorahosted.org