[PATCH v3 0/4] New patch for updating userstorage package

[Albert Esteve]

Sanlock is using an old version of userstorage in its tests. The package it's been
installable through package manager for a while now, and it has been recently updated to
its version 0.5.1.

So I have created a patch to use the latest version that can be installed from PyPI, and
remove the old embedded package. The structure of the test storage folder remains the
same.

Also added a couple commits to add latest python versions, and a prerrequisite to the
documentation that I needed to install to be able to compile. But I can remove these
unrelated commits.

Albert Esteve (4):
  README.dev: update prerequisites
  tox.ini: add py310 to environments
  userstorage: replace by PyPI userstorage
  README.dev: update userstorage doc

README.dev        |  22 +++++--
 requirements.txt  |   1 +
 tests/conftest.py |  22 ++++---
 tests/storage.py  | 161 +++++++---------------------------------------
 tox.ini           |   5 +-
 5 files changed, 55 insertions(+), 156 deletions(-)
 create mode 100644 requirements.txt

[PATCH v3 1/4] README.dev: update prerequisites

[Albert Esteve]

Add uuid to the listed prerequisites to
avoid the build error due to a missing include:

fatal error: uuid/uuid.h: No such file or directory
 #include <uuid/uuid.h>

Signed-off-by: Albert Esteve 
---
 README.dev | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/README.dev b/README.dev
index 7f3df2c..1bf1d17 100644
--- a/README.dev
+++ b/README.dev
@@ -3,11 +3,11 @@ Prerequisites

For Fedora/CentOS install the following packages:

-   $ sudo yum install -y gcc make libaio-devel libblkid-devel
+   $ sudo yum install -y gcc make libaio-devel libblkid-devel libuuid-devel

For Ubuntu/Debian install the following packages:

-   $ sudo apt install -y gcc make libaio-dev libblkid-dev
+   $ sudo apt install -y gcc make libaio-dev libblkid-dev uuid-dev

How to test sanlock

[PATCH v3 2/4] tox.ini: add py310 to environments

[Albert Esteve]

Add py310 to the list of tox environments,
so that we can tests newer versions too.

Signed-off-by: Albert Esteve 
---
 tox.ini | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tox.ini b/tox.ini
index ee871ef..8847b34 100644
--- a/tox.ini
+++ b/tox.ini
@@ -4,7 +4,7 @@
 # and then run "tox" from this directory.

[tox]
-envlist = py{36,37,38,39},flake8
+envlist = py{36,37,38,39,310},flake8
 skipsdist = True
 skip_missing_interpreters = True

@@ -18,7 +18,7 @@ whitelist_externals = make
 deps =
     pytest
 commands =
-    py{36,37,38}: make BUILDARGS="--build-lib={envsitepackagesdir}"
+    py{36,37,38,39,310}: make BUILDARGS="--build-lib={envsitepackagesdir}"
     pytest {posargs}

[testenv:flake8]

[PATCH v3 3/4] userstorage: replace by PyPI userstorage

[Albert Esteve]

Remove the outdated version of userstorage
that is stored locally at tests/storage.py

The userstorage library is required
to create the storage before running the
tests with tox, and also in the test
virtual environment. In order to satisfy
this, add a requirements.txt to the root
folder to obtain the latests userstorage
version from PyPI, and add it as a tox
dependency for tests.

Behaviour for missing storage and folder
structure remains as originally intended.

Fixture is adapted to new style so that
is uses setup and teardown before and
after every test, respectively.

Signed-off-by: Albert Esteve 
---
 requirements.txt  |   1 +
 tests/conftest.py |  22 ++++---
 tests/storage.py  | 161 +++++++---------------------------------------
 tox.ini           |   1 +
 4 files changed, 36 insertions(+), 149 deletions(-)
 create mode 100644 requirements.txt

diff --git a/requirements.txt b/requirements.txt
new file mode 100644
index 0000000..d4a4b9f
--- /dev/null
+++ b/requirements.txt
@@ -0,0 +1 @@
+userstorage>=0.5.1
\ No newline at end of file
diff --git a/tests/conftest.py b/tests/conftest.py
index b40852b..79c514b 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -9,13 +9,17 @@ Fixtures for sanlock testing.
 """
 from __future__ import absolute_import

-import os
-
 import pytest
+import userstorage

-from . import storage
 from . import util

+# Mark tests with skip if userstorage is missing
+userstorage.missing_handler = pytest.skip
+
+# Requires relative path from tox basedir
+BACKENDS = userstorage.load_config("./tests/storage.py").BACKENDS
+

class SanlockIsRunning(Exception):
     """ Raised if sanlock running when it should not """
@@ -38,8 +42,8 @@ def sanlock_daemon():

@pytest.fixture(params=[
-    pytest.param(storage.BLOCK, id="block"),
-    pytest.param(storage.FILE, id="file"),
+    BACKENDS["block"],
+    BACKENDS["file"],
 ])
 def user_4k_path(request):
     """
@@ -48,11 +52,9 @@ def user_4k_path(request):

If storage is not available, skip the tests.
     """
-    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
+    backend = request.param
+    with backend:
+        yield backend.path

@pytest.fixture
diff --git a/tests/storage.py b/tests/storage.py
index 0fe6f56..0b12e47 100644
--- a/tests/storage.py
+++ b/tests/storage.py
@@ -8,145 +8,28 @@
 storage - provide storage for sanlock tests.
 """

-import argparse
-import errno
-import logging
-import os
-import subprocess
+from userstorage import File, Mount, LoopDevice

-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()
-
-    # Remove stale symlink.
-    if os.path.islink(link_path):
-        os.unlink(link_path)
-
-    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
+GiB = 1024**3

+BASE_DIR = "/var/tmp/sanlock-storage"

-if __name__ == "__main__":
-    main()
+BACKENDS = {
+
+    "file":
+        File(
+            Mount(
+                LoopDevice(
+                    base_dir=BASE_DIR,
+                    name="file",
+                    size=GiB,
+                    sector_size=4096))),
+
+    "block":
+        LoopDevice(
+            base_dir=BASE_DIR,
+            name="loop",
+            size=GiB,
+            sector_size=4096),
+
+}
diff --git a/tox.ini b/tox.ini
index 8847b34..c0cf5a1 100644
--- a/tox.ini
+++ b/tox.ini
@@ -17,6 +17,7 @@ setenv =
 whitelist_externals = make
 deps =
     pytest
+    userstorage>=0.5.1
 commands =
     py{36,37,38,39,310}: make BUILDARGS="--build-lib={envsitepackagesdir}"
     pytest {posargs}

[PATCH v3 3/4] userstorage: replace by PyPI userstorage

[Nir Soffer]

On Wed, Nov 9, 2022 at 3:51 PM Albert Esteve  wrote:
> Remove the outdated version of userstorage
> that is stored locally at tests/storage.py
>
> The userstorage library is required
> to create the storage before running the
> tests with tox, and also in the test
> virtual environment. In order to satisfy
> this, add a requirements.txt to the root
> folder to obtain the latests userstorage
> version from PyPI, and add it as a tox
> dependency for tests.
>
> Behaviour for missing storage and folder
> structure remains as originally intended.
>
> Fixture is adapted to new style so that
> is uses setup and teardown before and
> after every test, respectively.
> Signed-off-by: Albert Esteve
>
> requirements.txt  |   1 +
>  tests/conftest.py |  22 ++++---
>  tests/storage.py  | 161 +++++++---------------------------------------
>  tox.ini           |   1 +
>  4 files changed, 36 insertions(+), 149 deletions(-)
>  create mode 100644 requirements.txt
>
> diff --git a/requirements.txt b/requirements.txt
> new file mode 100644
> index 0000000..d4a4b9f
> --- /dev/null
> +++ b/requirements.txt
> @@ -0,0 +1 @@
> +userstorage>=0.5.1

I added tox here.
> \ No newline at end of file
> diff --git a/tests/conftest.py b/tests/conftest.py
> index b40852b..79c514b 100644
> --- a/tests/conftest.py
> +++ b/tests/conftest.py
> @@ -9,13 +9,17 @@ Fixtures for sanlock testing.
>  """
>  from __future__ import absolute_import
>
> -import os
>
> import pytest
> +import userstorage
>
> -from . import storage
>  from . import util
>
> +# Mark tests with skip if userstorage is missing
> +userstorage.missing_handler = pytest.skip
>
> +# Requires relative path from tox basedir
> +BACKENDS = userstorage.load_config("./tests/storage.py").BACKENDS
>
> class SanlockIsRunning(Exception):
>      """ Raised if sanlock running when it should not """
> @@ -38,8 +42,8 @@ def sanlock_daemon():
>
> @pytest.fixture(params=[
> pytest.param(storage.BLOCK, id="block"),
> pytest.param(storage.FILE, id="file"),
> BACKENDS["block"],
> BACKENDS["file"],
>
> ])
>  def user_4k_path(request):
>      """
> @@ -48,11 +52,9 @@ def user_4k_path(request):
>  If storage is not available, skip the tests.
>  """
> 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
> backend = request.param
> with backend:
>    yield backend.path
>
>
> @pytest.fixture
> diff --git a/tests/storage.py b/tests/storage.py
> index 0fe6f56..0b12e47 100644
> --- a/tests/storage.py
> +++ b/tests/storage.py
> @@ -8,145 +8,28 @@
>  storage - provide storage for sanlock tests.
>  """
>
> -import argparse
> -import errno
> -import logging
> -import os
> -import subprocess
> +from userstorage import File, Mount, LoopDevice
>
> -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()
> # Remove stale symlink.
> if os.path.islink(link_path):
>    os.unlink(link_path)
> 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
>
> +GiB = 1024**3
>
> +BASE_DIR = "/var/tmp/sanlock-storage"
>
> -if __name__ == "__main__":
> main()
>
> +BACKENDS = {
> "file":
>    File(
>        Mount(
>            LoopDevice(
>                base_dir=BASE_DIR,
>                name="file",
>                size=GiB,
>                sector_size=4096))),
> "block":
>    LoopDevice(
>        base_dir=BASE_DIR,
>        name="loop",
>        size=GiB,
>        sector_size=4096),
>
> +}
> diff --git a/tox.ini b/tox.ini
> index 8847b34..c0cf5a1 100644
> --- a/tox.ini
> +++ b/tox.ini
> @@ -17,6 +17,7 @@ setenv =
>  whitelist_externals = make
>  deps =
>      pytest
> userstorage>=0.5.1
>
> commands =
>      py{36,37,38,39,310}: make BUILDARGS="--build-lib={envsitepackagesdir}"
>      pytest {posargs}
> --
> 2.37.3

[PATCH v3 4/4] README.dev: update userstorage doc

[Albert Esteve]

Update documentation regarding userstorage
setup and teardown.

Signed-off-by: Albert Esteve 
---
 README.dev | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/README.dev b/README.dev
index 1bf1d17..70e4a7c 100644
--- a/README.dev
+++ b/README.dev
@@ -54,9 +54,21 @@ Or using environment variable:
 Testing 4K support
 ==================

-To enable the 4k tests, you need to setup 4k stroage for the tests:
+To enable the 4k tests, you need to have userstorage installed.
+First, you need to create a virtual environment
+(https://docs.python.org/3/library/venv.html), and then install
+the userstorage library from the requirements.txt.
+This only needs to be done once:

-    $ python tests/storage.py setup
+	$ python3 -m venv ~/.venv/sanlock
+    $ source ~/.venv/vdsm/bin/activate
+    $ pip install --upgrade pip
+    $ pip install -r requirements.txt
+    $ deactivate
+
+Then, you just need to setup 4k storage for the tests:
+
+    $ userstorage create tests/storage.py

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
@@ -64,7 +76,7 @@ block device.

To teardown the storage:

-    $ python tests/storage.py teardown
+    $ userstorage delete tests/storage.py

The script unmounts the filesystem and detaches the loop devices.

[PATCH v3 4/4] README.dev: update userstorage doc

[Nir Soffer]

On Wed, Nov 9, 2022 at 3:51 PM Albert Esteve  wrote:
> Update documentation regarding userstorage
> setup and teardown.
> Signed-off-by: Albert Esteve
>
> README.dev | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/README.dev b/README.dev
> index 1bf1d17..70e4a7c 100644
> --- a/README.dev
> +++ b/README.dev
> @@ -54,9 +54,21 @@ Or using environment variable:
>  Testing 4K support
>  ==================
>
> -To enable the 4k tests, you need to setup 4k stroage for the tests:
> +To enable the 4k tests, you need to have userstorage installed.
> +First, you need to create a virtual environment
> +(https://docs.python.org/3/library/venv.html), and then install
> +the userstorage library from the requirements.txt.
> +This only needs to be done once:
> $ python tests/storage.py setup
>   $ python3 -m venv ~/.venv/sanlock
> $ source ~/.venv/vdsm/bin/activate
> $ pip install --upgrade pip
> $ pip install -r requirements.txt
> $ deactivate
>
> +Then, you just need to setup 4k storage for the tests:
> $ userstorage create tests/storage.py

This will fail if you follow the instructions, since we just deactivated
the virtual
environment. I amended to explain how to enter and exit the environment.
> 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
> @@ -64,7 +76,7 @@ block device.
>
> To teardown the storage:
> $ python tests/storage.py teardown
> $ userstorage delete tests/storage.py
>
> The script unmounts the filesystem and detaches the loop devices.
>
> --
> 2.37.3

[Nir Soffer]

On Wed, Nov 9, 2022 at 3:51 PM Albert Esteve  wrote:
> Sanlock is using an old version of userstorage in its tests. The package
> it's been
> installable through package manager for a while now, and it has been
> recently updated to
> its version 0.5.1.
>
> So I have created a patch to use the latest version that can be installed
> from PyPI, and
> remove the old embedded package. The structure of the test storage folder
> remains the
> same.
>
> Also added a couple commits to add latest python versions, and a
> prerrequisite to the
> documentation that I needed to install to be able to compile. But I can
> remove these
> unrelated commits.
>
> Albert Esteve (4):
>   README.dev: update prerequisites
>   tox.ini: add py310 to environments
>   userstorage: replace by PyPI userstorage
>   README.dev: update userstorage doc
>
> README.dev        |  22 +++++--
>  requirements.txt  |   1 +
>  tests/conftest.py |  22 ++++---
>  tests/storage.py  | 161 +++++++---------------------------------------
>  tox.ini           |   5 +-
>  5 files changed, 55 insertions(+), 156 deletions(-)
>  create mode 100644 requirements.txt

Thanks, pushed as:
1b306f0 README.dev: update userstorage doc
38ff685 userstorage: replace by PyPI userstorage
7de7658 tox.ini: add py310 to environments
0cb0978 README.dev: update prerequisites