June 2019 - sanlock-devel - Fedora mailing-lists

[PATCH v3 0/2] tests for clearing resource
by Vojtech Juranek 20 Jun '19

20 Jun '19

Add tests for clearing the resource. Also extract reading sanlock magic number into helper function. Vojtech Juranek (2): tests: replace reading of magic number by helper function tests: add tests for clearing resource tests/constants.py | 4 +++ tests/python_test.py | 72 ++++++++++++++++++++++++++++++++++---------- tests/util.py | 11 +++++++ 3 files changed, 71 insertions(+), 16 deletions(-) -- 2.20.1

3 5

[PATCH 0/2] Streamline tests output
by Nir Soffer 17 Jun '19

17 Jun '19

Streamline tests output by using normal mode and not listing skipped tests in the short test summary. Here is example output from travis build: $ pytest ============================= test session starts ============================== platform linux -- Python 3.7.1, pytest-4.3.1, py-1.7.0, pluggy-0.8.0 rootdir: /home/travis/build/nirs/sanlock, inifile: tox.ini collected 210 items tests/daemon_test.py ........... [ 5%] tests/direct_test.py .. [ 6%] tests/python_test.py ........ssssssssssss........ssssssssssssss......... [ 30%] ...............ss.ss.ss.ss.ss.ss.ss.ss.ss.ss.ss.ss.ss.ss.ss.ss.ss.ss.ss. [ 64%] ss.ss.ss.ss.ss.ss.ss.ss.ss.ss.ss.ss.ss.ss.ss.ss.ss.ss.ss.ss.ss.......... [ 99%] .. [100%] ========================== slowest 10 test durations =========================== 6.02s call tests/python_test.py::test_add_rem_lockspace_async 6.02s call tests/python_test.py::test_add_rem_lockspace[1099511627776-1099510579200] 6.02s call tests/python_test.py::test_add_rem_lockspace[1048576-0] 4.06s call tests/python_test.py::test_acquire_release_resource[1048576-0] 4.05s call tests/python_test.py::test_acquire_release_resource[1099511627776-1099510579200] 3.21s call tests/daemon_test.py::test_create 3.12s call tests/daemon_test.py::test_delete 3.09s call tests/daemon_test.py::test_lookup 3.05s call tests/daemon_test.py::test_lookup_missing 0.42s call tests/daemon_test.py::test_start_after_kill =================== 104 passed, 106 skipped in 46.73 seconds =================== The command "pytest" exited with 0. 0.60s$ flake8 --statistics tests python The command "flake8 --statistics tests python" exited with 0. Based on commit edf1e08326daa39a2e453d936fa9a99dd0ec136c (python: Convert example.py to python 3) Nir Soffer (2): python: Don't report skipped tests python: Use pytest normal mode README.dev | 9 +++++++++ tox.ini | 5 ++--- 2 files changed, 11 insertions(+), 3 deletions(-) -- 2.17.2

3 4

[sanlock] 02/02: python: Use pytest normal mode
by pagure＠pagure.io 17 Jun '19

17 Jun '19

This is an automated email from the git hooks/post-receive script. nsoffer pushed a commit to branch master in repository sanlock. commit a4ded879ecbddbc2328a2e59298fdaeae430c16f Author: Nir Soffer <nsoffer(a)redhat.com> AuthorDate: Sun Jun 16 18:28:33 2019 +0300 python: Use pytest normal mode We have too many tests to use verbose mode. If verbose more is needed it can be enabled using: tox -e py36 tests/python_test.py -- -vv Or using environment variable: export PYTEST_ADDOPTS=-vv Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> --- README.dev | 9 +++++++++ tox.ini | 3 +-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/README.dev b/README.dev index 8ade5e6..7f3df2c 100644 --- a/README.dev +++ b/README.dev @@ -41,6 +41,15 @@ To run basic lint and style check: $ tox -e flake8 +Sometimes when debugging failing tests verbose mode is useful. You can +enable it using: + + $ tox -e py36 tests/python_test.py -- -vv + +Or using environment variable: + + export PYTEST_ADDOPTS=-vv + Testing 4K support ================== diff --git a/tox.ini b/tox.ini index 0cadfb5..4a28561 100644 --- a/tox.ini +++ b/tox.ini @@ -29,10 +29,9 @@ commands = flake8 --statistics tests python [pytest] # Notes: # --basetemp: we must use /var/tmp as sanlock uses direct I/O. -# -vv: increasing verbosify twice shows more detailed failures tracebacks. # -rxX show extra test summary: (x)failed, (X)passed, # --durations: show slowest test duration -addopts = -rxX -vv --basetemp=/var/tmp/sanlock --durations=10 +addopts = -rxX --basetemp=/var/tmp/sanlock --durations=10 [flake8] show_source = True -- To stop receiving notification emails like this one, please contact the administrator of this repository.

1 0

[sanlock] 01/02: python: Don't report skipped tests
by pagure＠pagure.io 17 Jun '19

17 Jun '19

This is an automated email from the git hooks/post-receive script. nsoffer pushed a commit to branch master in repository sanlock. commit 1a6283528075469cdb7f75859bb3333c560e7a55 Author: Nir Soffer <nsoffer(a)redhat.com> AuthorDate: Sun Jun 16 18:27:20 2019 +0300 python: Don't report skipped tests In the short test summary, report only (x)failed and (X)passed tests. Skipped tests are normal, used when a test cannot run in the current environment. (x)failed or (X)passed tests must be fixed and should be listed. Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> --- tox.ini | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tox.ini b/tox.ini index 0e337ce..0cadfb5 100644 --- a/tox.ini +++ b/tox.ini @@ -30,9 +30,9 @@ commands = flake8 --statistics tests python # Notes: # --basetemp: we must use /var/tmp as sanlock uses direct I/O. # -vv: increasing verbosify twice shows more detailed failures tracebacks. -# -rxs: show extra test summary: (s)skipped, (x)failed +# -rxX show extra test summary: (x)failed, (X)passed, # --durations: show slowest test duration -addopts = -rxs -vv --basetemp=/var/tmp/sanlock --durations=10 +addopts = -rxX -vv --basetemp=/var/tmp/sanlock --durations=10 [flake8] show_source = True -- To stop receiving notification emails like this one, please contact the administrator of this repository.

1 0

Re: [PATCH v2 4/8] python: Extract create_resource() helper
by Nir Soffer 17 Jun '19

17 Jun '19

On Mon, Jun 17, 2019 at 1:04 PM Pavel Bar <pbar(a)redhat.com> wrote: > > > On Sun, Jun 16, 2019 at 11:50 PM Nir Soffer <nirsof(a)gmail.com> wrote: > >> Replace duplicate code for allocating and initializing sanlk_resource >> struct with a helper. >> >> Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> >> --- >> python/sanlock.c | 46 +++++++++++++++++++++++----------------------- >> 1 file changed, 23 insertions(+), 23 deletions(-) >> >> diff --git a/python/sanlock.c b/python/sanlock.c >> index 5c2fd1a..6824e01 100644 >> --- a/python/sanlock.c >> +++ b/python/sanlock.c >> @@ -170,29 +170,38 @@ parse_single_disk(PyObject* disk, struct >> sanlk_disk* res_disk) >> finally: >> Py_XDECREF(path); >> return rv; >> } >> >> +static struct sanlk_resource * >> +create_resource(int num_disks) >> +{ >> + size_t size = sizeof(struct sanlk_resource) + >> + sizeof(struct sanlk_disk) * num_disks; >> + >> + struct sanlk_resource *res = calloc(1, size); >> + if (res == NULL) { >> + PyErr_NoMemory(); >> + return NULL; >> + } >> + >> + res->num_disks = num_disks; >> + >> + return res; >> +} >> + >> static int >> parse_disks(PyObject *obj, struct sanlk_resource **res_ret) >> { >> - int num_disks, res_len; >> + int num_disks; >> struct sanlk_resource *res; >> > > What about declaring the local variables when first used, thus immediately > initializing them? I think C99 provides that. > I.e., > int num_disks = PyList_Size(obj); > struct sanlk_resource *res = create_resource(num_disks); > Moreover, in this case they will be at the beginning of the block anyway. > This is not needed to fix the problem I'm trying to solve here. We can do this work later if needed. >> num_disks = PyList_Size(obj); >> >> - res_len = sizeof(struct sanlk_resource) + >> - (sizeof(struct sanlk_disk) * num_disks); >> - res = malloc(res_len); >> - >> - if (res == NULL) { >> - PyErr_NoMemory(); >> + res = create_resource(num_disks); >> + if (res == NULL) >> return -1; >> - } >> - >> - memset(res, 0, res_len); >> - res->num_disks = num_disks; >> >> for (int i = 0; i < num_disks; i++) { >> PyObject *disk = PyList_GetItem(obj,i); >> >> if (!parse_single_disk(disk, &(res->disks[i]))) { >> @@ -637,30 +646,21 @@ Align can be one of (1048576, 2097152, 4194304, >> 8388608).\n\ >> Sector can be one of (512, 4096)."); >> >> static PyObject * >> py_read_resource(PyObject *self __unused, PyObject *args, PyObject >> *keywds) >> { >> - int rv = -1, res_len, sector = SECTOR_SIZE_512; >> + int rv = -1, sector = SECTOR_SIZE_512; >> long align = ALIGNMENT_1M; >> PyObject *path = NULL; >> struct sanlk_resource *res; >> PyObject *res_info = NULL; >> >> static char *kwlist[] = {"path", "offset", "align", "sector", NULL}; >> >> - /* allocate the needed memory for the resource and one disk */ >> - res_len = sizeof(struct sanlk_resource) + sizeof(struct sanlk_disk); >> - res = malloc(res_len); >> - >> - if (res == NULL) { >> - PyErr_NoMemory(); >> + res = create_resource(1 /* num_disks */); >> > > Not sure the comment is needed. > > >> + if (res == NULL) >> return NULL; >> - } >> - >> - /* initialize resource and disk structures */ >> - memset(res, 0, res_len); >> - res->num_disks = 1; >> >> /* parse python tuple */ >> if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&|kli", kwlist, >> pypath_converter, &path, &(res->disks[0].offset), &align, >> &sector)) { >> goto finally; >> -- >> 2.17.2 >> >>

2 1

Re: [PATCH v2 0/4] Fix python style issues
by Vojtech Juranek 17 Jun '19

17 Jun '19

> The whole patch looks good! +1 > On Sun, Jun 16, 2019 at 7:12 PM Nir Soffer <nirsof(a)gmail.com> wrote: > > Fix python style issues and add flake8 style check to the build to keep > > the code in good shape. > > > > Changes since v1: > > - Add flake8 to travis build > > - Replace star imports with explicit imports to allow flake8 detection > > > > of undefined names (Pavel) > > > > - Convert example.py to python3 - breaks flake8 on python 3 builds. > > > > v1 was here: > > > > https://lists.fedorahosted.org/archives/list/sanlock-devel@lists.fedorahos > > ted.org/thread/ZH4XT326TS25PCZH2ESGVME5XPSUZPUR/> > > Nir Soffer (4): > > python: Add flak8 env > > python: Avoid star imports > > python: Fix style issues > > python: Convert example.py to python 3 > > > > .travis.yml | 4 ++++ > > README.dev | 4 ++++ > > python/example.py | 56 +++++++++++++++++++++++++++----------------- > > tests/conftest.py | 8 +++---- > > tests/daemon_test.py | 15 ++++++++---- > > tests/direct_test.py | 2 +- > > tests/python_test.py | 48 +++++++++++++++++++++++++------------ > > tests/util.py | 2 +- > > tox.ini | 9 ++++++- > > 9 files changed, 101 insertions(+), 47 deletions(-) > > > > -- > > 2.17.2

2 1

[sanlock] 04/04: python: Convert example.py to python 3
by pagure＠pagure.io 17 Jun '19

17 Jun '19

This is an automated email from the git hooks/post-receive script. nsoffer pushed a commit to branch master in repository sanlock. commit 8bf78b8468d21d9adca78235e6e29a2235589e08 Author: Nir Soffer <nsoffer(a)redhat.com> AuthorDate: Sun Jun 16 17:47:52 2019 +0300 python: Convert example.py to python 3 Use print function instead of print statement to make the example compatible with both python 2 and 3. Tested only by flake8, may not actually work with python 3. Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> --- python/example.py | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/python/example.py b/python/example.py index 1be7e66..35d39f5 100644 --- a/python/example.py +++ b/python/example.py @@ -1,3 +1,5 @@ +from __future__ import print_function + import pwd import grp import os @@ -12,13 +14,13 @@ RESOURCE_NAME = "resource1" def sigTermHandler(): - print "SIGTERM signal received" + print("SIGTERM signal received") def main(): signal.signal(signal.SIGTERM, sigTermHandler) - print "Creating the sanlock disk" + print("Creating the sanlock disk") fd, disk = tempfile.mkstemp() os.close(fd) @@ -28,33 +30,33 @@ def main(): SNLK_DISKS = [(disk, offset)] - print "Registering to sanlock" + print("Registering to sanlock") fd = sanlock.register() - print "Initializing '%s'" % (LOCKSPACE_NAME,) + print("Initializing '%s'" % (LOCKSPACE_NAME,)) sanlock.write_lockspace(LOCKSPACE_NAME, disk, align=1048576, sector=512) - print "Initializing '%s' on '%s'" % (RESOURCE_NAME, LOCKSPACE_NAME) + print("Initializing '%s' on '%s'" % (RESOURCE_NAME, LOCKSPACE_NAME)) sanlock.write_resource( LOCKSPACE_NAME, RESOURCE_NAME, SNLK_DISKS, align=1048576, sector=512) - print "Acquiring the id '%i' on '%s'" % (HOST_ID, LOCKSPACE_NAME) + print("Acquiring the id '%i' on '%s'" % (HOST_ID, LOCKSPACE_NAME)) sanlock.add_lockspace(LOCKSPACE_NAME, HOST_ID, disk) try: - print "Acquiring '%s' on '%s'" % (RESOURCE_NAME, LOCKSPACE_NAME) + print("Acquiring '%s' on '%s'" % (RESOURCE_NAME, LOCKSPACE_NAME)) sanlock.acquire( LOCKSPACE_NAME, RESOURCE_NAME, SNLK_DISKS, slkfd=fd, version=0) while True: - print "Trying to get lockspace '%s' hosts" % LOCKSPACE_NAME + print("Trying to get lockspace '%s' hosts" % LOCKSPACE_NAME) try: hosts_list = sanlock.get_hosts(LOCKSPACE_NAME) except sanlock.SanlockException as e: if e.errno != os.errno.EAGAIN: raise else: - print "Lockspace '%s' hosts: " % LOCKSPACE_NAME, hosts_list + print("Lockspace '%s' hosts: " % LOCKSPACE_NAME, hosts_list) break time.sleep(5) @@ -64,17 +66,17 @@ def main(): SNLK_DISKS, align=1048576, sector=512) - print "Resource '%s' owners: %s" % (RESOURCE_NAME, owners) + print("Resource '%s' owners: %s" % (RESOURCE_NAME, owners)) - print "Releasing '%s' on '%s'" % (RESOURCE_NAME, LOCKSPACE_NAME) + print("Releasing '%s' on '%s'" % (RESOURCE_NAME, LOCKSPACE_NAME)) sanlock.release(LOCKSPACE_NAME, RESOURCE_NAME, SNLK_DISKS, slkfd=fd) except Exception as e: - print "Exception: ", e + print("Exception: ", e) finally: - print "Releasing the id '%i' on '%s'" % (HOST_ID, LOCKSPACE_NAME) + print("Releasing the id '%i' on '%s'" % (HOST_ID, LOCKSPACE_NAME)) sanlock.rem_lockspace(LOCKSPACE_NAME, HOST_ID, disk) - print "Removing the sanlock disk" + print("Removing the sanlock disk") os.remove(disk) -- To stop receiving notification emails like this one, please contact the administrator of this repository.

1 0

[sanlock] 03/04: python: Fix style issues
by pagure＠pagure.io 17 Jun '19

17 Jun '19

This is an automated email from the git hooks/post-receive script. nsoffer pushed a commit to branch master in repository sanlock. commit 1b230a8da337cb2ff386f02007b668a4e7fb467c Author: Nir Soffer <nsoffer(a)redhat.com> AuthorDate: Fri Jun 14 20:04:54 2019 +0300 python: Fix style issues Fix style issues: - 2 blank lines around functions and classes - classes at the top of the module - reformat to avoid long hard to read lines - whitespace before ',' - whitespace after # - unused imports - unexpected indenation - remove blank line at end of file Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> --- python/example.py | 28 ++++++++++++++++++++-------- tests/conftest.py | 8 ++++---- tests/python_test.py | 50 ++++++++++++++++++++++++++++++++------------------ 3 files changed, 56 insertions(+), 30 deletions(-) diff --git a/python/example.py b/python/example.py index 6ef9f7d..1be7e66 100644 --- a/python/example.py +++ b/python/example.py @@ -10,9 +10,11 @@ HOST_ID = 1 LOCKSPACE_NAME = "lockspace1" RESOURCE_NAME = "resource1" + def sigTermHandler(): print "SIGTERM signal received" + def main(): signal.signal(signal.SIGTERM, sigTermHandler) @@ -20,7 +22,8 @@ def main(): fd, disk = tempfile.mkstemp() os.close(fd) - os.chown(disk, pwd.getpwnam("sanlock").pw_uid, grp.getgrnam("sanlock").gr_gid) + os.chown( + disk, pwd.getpwnam("sanlock").pw_uid, grp.getgrnam("sanlock").gr_gid) offset = sanlock.get_alignment(disk) SNLK_DISKS = [(disk, offset)] @@ -29,18 +32,20 @@ def main(): fd = sanlock.register() print "Initializing '%s'" % (LOCKSPACE_NAME,) - sanlock.write_lockspace(LOCKSPACE_NAME, disk, max_hosts=0, iotimeout=0, align=1048576, sector=512) + sanlock.write_lockspace(LOCKSPACE_NAME, disk, align=1048576, sector=512) print "Initializing '%s' on '%s'" % (RESOURCE_NAME, LOCKSPACE_NAME) - sanlock.write_resource(LOCKSPACE_NAME, RESOURCE_NAME, SNLK_DISKS, align=1048576, sector=512) + sanlock.write_resource( + LOCKSPACE_NAME, RESOURCE_NAME, SNLK_DISKS, align=1048576, sector=512) print "Acquiring the id '%i' on '%s'" % (HOST_ID, LOCKSPACE_NAME) sanlock.add_lockspace(LOCKSPACE_NAME, HOST_ID, disk) try: print "Acquiring '%s' on '%s'" % (RESOURCE_NAME, LOCKSPACE_NAME) - sanlock.acquire(LOCKSPACE_NAME, RESOURCE_NAME, SNLK_DISKS, slkfd=fd, - version=0) + sanlock.acquire( + LOCKSPACE_NAME, RESOURCE_NAME, SNLK_DISKS, slkfd=fd, version=0) + while True: print "Trying to get lockspace '%s' hosts" % LOCKSPACE_NAME try: @@ -52,9 +57,15 @@ def main(): print "Lockspace '%s' hosts: " % LOCKSPACE_NAME, hosts_list break time.sleep(5) - print "Resource '%s' owners: " % RESOURCE_NAME, \ - sanlock.read_resource_owners( - LOCKSPACE_NAME, RESOURCE_NAME, SNLK_DISKS, align=1048576, sector=512) + + owners = sanlock.read_resource_owners( + LOCKSPACE_NAME, + RESOURCE_NAME, + SNLK_DISKS, + align=1048576, + sector=512) + print "Resource '%s' owners: %s" % (RESOURCE_NAME, owners) + print "Releasing '%s' on '%s'" % (RESOURCE_NAME, LOCKSPACE_NAME) sanlock.release(LOCKSPACE_NAME, RESOURCE_NAME, SNLK_DISKS, slkfd=fd) except Exception as e: @@ -66,5 +77,6 @@ def main(): print "Removing the sanlock disk" os.remove(disk) + if __name__ == '__main__': main() diff --git a/tests/conftest.py b/tests/conftest.py index 70e83d4..868ae46 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -4,7 +4,6 @@ Fixtures for sanlock testing. from __future__ import absolute_import import os -import stat import pytest @@ -12,6 +11,10 @@ from . import storage from . import util +class SanlockIsRunning(Exception): + """ Raised if sanlock running when it should not """ + + @pytest.fixture def sanlock_daemon(): """ @@ -50,6 +53,3 @@ def user_4k_path(request): def no_sanlock_daemon(): if util.sanlock_is_running(): raise SanlockIsRunning - -class SanlockIsRunning(Exception): - pass diff --git a/tests/python_test.py b/tests/python_test.py index a032951..331e901 100644 --- a/tests/python_test.py +++ b/tests/python_test.py @@ -5,7 +5,6 @@ from __future__ import absolute_import import errno import io -import struct import time from contextlib import contextmanager @@ -36,7 +35,7 @@ SECTOR_SIZE_4K = 4 * KiB FILE_NAMES = [ - #name, encoding + # name, encoding ("ascii", None), (u"ascii", None), (u"\u05d0", None), @@ -49,21 +48,27 @@ LOCKSPACE_OR_RESOURCE_NAMES = [ # Python 2 also supports str. pytest.param( "\xd7\x90", - marks=pytest.mark.skipif(six.PY3, reason="python 3 supports only bytes")), + marks=pytest.mark.skipif( + six.PY3, + reason="python 3 supports only bytes")), # Python 2 also supports unicode with ascii content. pytest.param( u"ascii", - marks=pytest.mark.skipif(six.PY3, reason="python 3 supports only bytes")), + marks=pytest.mark.skipif( + six.PY3, + reason="python 3 supports only bytes")), ] -(a)pytest.mark.parametrize("filename, encoding" , FILE_NAMES) -(a)pytest.mark.parametrize("size,offset", [ + +(a)pytest.mark.parametrize("filename, encoding", FILE_NAMES) +(a)pytest.mark.parametrize("size, offset", [ # Smallest offset. (LOCKSPACE_SIZE, 0), # Large offset. (LARGE_FILE_SIZE, LARGE_FILE_SIZE - LOCKSPACE_SIZE), ]) -def test_write_lockspace(tmpdir, sanlock_daemon, filename, encoding, size, offset): +def test_write_lockspace( + tmpdir, sanlock_daemon, filename, encoding, size, offset): path = util.generate_path(tmpdir, filename, encoding) util.create_file(path, size) @@ -103,7 +108,11 @@ def test_write_lockspace_4k(user_4k_path, sanlock_daemon, align): util.write_guard(user_4k_path, align) sanlock.write_lockspace( - b"ls_name", user_4k_path, iotimeout=1, align=align, sector=SECTOR_SIZE_4K) + b"ls_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) @@ -144,7 +153,8 @@ def test_read_lockspace_4k_invalid_sector_size(sanlock_daemon, user_4k_path): # Large offset. (LARGE_FILE_SIZE, LARGE_FILE_SIZE - MIN_RES_SIZE), ]) -def test_write_resource(tmpdir, sanlock_daemon, filename, encoding, size, offset): +def test_write_resource( + tmpdir, sanlock_daemon, filename, encoding, size, offset): path = util.generate_path(tmpdir, filename, encoding) util.create_file(path, size) disks = [(path, offset)] @@ -177,9 +187,6 @@ def test_write_resource(tmpdir, sanlock_daemon, filename, encoding, size, offset magic = util.read_magic(path, offset) assert magic == constants.PAXOS_DISK_MAGIC - - # TODO: check more stuff here... - util.check_guard(path, size) @@ -567,7 +574,8 @@ def test_add_lockspace_parse_args(no_sanlock_daemon, name, filename, encoding): @pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES) @pytest.mark.parametrize("filename,encoding", FILE_NAMES) -def test_write_lockspace_parse_args(no_sanlock_daemon, name, filename, encoding): +def test_write_lockspace_parse_args( + no_sanlock_daemon, name, filename, encoding): path = util.generate_path("/tmp/", filename, encoding) with raises_sanlock_errno(): sanlock.write_lockspace(name, path) @@ -575,7 +583,8 @@ def test_write_lockspace_parse_args(no_sanlock_daemon, name, filename, encoding) @pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES) @pytest.mark.parametrize("filename,encoding", FILE_NAMES) -def test_write_resource_parse_args(no_sanlock_daemon, name, filename, encoding): +def test_write_resource_parse_args( + no_sanlock_daemon, name, filename, encoding): path = util.generate_path("/tmp/", filename, encoding) disks = [(path, 0)] with raises_sanlock_errno(): @@ -587,7 +596,8 @@ def test_write_resource_parse_args(no_sanlock_daemon, name, filename, encoding): @pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES) @pytest.mark.parametrize("filename,encoding", FILE_NAMES) -def test_release_resource_parse_args(no_sanlock_daemon, name, filename, encoding): +def test_release_resource_parse_args( + no_sanlock_daemon, name, filename, encoding): path = util.generate_path("/tmp/", filename, encoding) disks = [(path, 0)] with raises_sanlock_errno(): @@ -599,7 +609,8 @@ def test_release_resource_parse_args(no_sanlock_daemon, name, filename, encoding @pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES) @pytest.mark.parametrize("filename,encoding", FILE_NAMES) -def test_read_resource_owners_parse_args(no_sanlock_daemon, name, filename, encoding): +def test_read_resource_owners_parse_args( + no_sanlock_daemon, name, filename, encoding): path = util.generate_path("/tmp/", filename, encoding) disks = [(path, 0)] with raises_sanlock_errno(): @@ -643,7 +654,8 @@ def test_set_event_parse_args(no_sanlock_daemon, name): @pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES) @pytest.mark.parametrize("filename,encoding", FILE_NAMES) -def test_init_lockspace_parse_args(no_sanlock_daemon, name, filename, encoding): +def test_init_lockspace_parse_args( + no_sanlock_daemon, name, filename, encoding): path = util.generate_path("/tmp/", filename, encoding) with raises_sanlock_errno(errno.ENODEV): sanlock.init_lockspace(name, path) @@ -659,21 +671,23 @@ def test_init_resource_parse_args(no_sanlock_daemon, name, filename, encoding): with raises_sanlock_errno(errno.ENOENT): sanlock.init_resource(name, b"res_name", disks) + @pytest.mark.parametrize("filename,encoding", FILE_NAMES) def test_get_alignment_parse_args(no_sanlock_daemon, filename, encoding): path = util.generate_path("/tmp/", filename, encoding) with raises_sanlock_errno(errno.ENOENT): sanlock.get_alignment(path) + @pytest.mark.parametrize("filename,encoding", FILE_NAMES) def test_read_lockspace_parse_args(no_sanlock_daemon, filename, encoding): path = util.generate_path("/tmp/", filename, encoding) with raises_sanlock_errno(): sanlock.read_lockspace(path) + @pytest.mark.parametrize("filename,encoding", FILE_NAMES) def test_read_resource_parse_args(no_sanlock_daemon, filename, encoding): path = util.generate_path("/tmp/", filename, encoding) with raises_sanlock_errno(): sanlock.read_resource(path) - -- To stop receiving notification emails like this one, please contact the administrator of this repository.

1 0

[sanlock] 02/04: python: Avoid star imports
by pagure＠pagure.io 17 Jun '19

17 Jun '19

This is an automated email from the git hooks/post-receive script. nsoffer pushed a commit to branch master in repository sanlock. commit 88940740848b189843d3541a71ec8cf5fc063020 Author: Nir Soffer <nsoffer(a)redhat.com> AuthorDate: Sun Jun 16 16:58:07 2019 +0300 python: Avoid star imports Star Imports (from module import *) are very convenient but flake8 is not smart enough to tell which names are imported. Replace the star imports with specific imports. This changes fixes these flake8 errors: 5 F403 'from .constants import *' used; unable to detect undefined names 44 F405 'MiB' may be undefined, or defined from star imports: .constants, .units Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> --- tests/daemon_test.py | 15 +++++++++++---- tests/direct_test.py | 2 +- tests/python_test.py | 2 +- tests/util.py | 2 +- 4 files changed, 14 insertions(+), 7 deletions(-) diff --git a/tests/daemon_test.py b/tests/daemon_test.py index 36648e3..413ce95 100644 --- a/tests/daemon_test.py +++ b/tests/daemon_test.py @@ -9,9 +9,17 @@ import struct import pytest -from . constants import * +from . constants import ( + DELTA_DISK_MAGIC, + PAXOS_DISK_MAGIC, + PAXOS_DISK_CLEAR, + RINDEX_DISK_MAGIC, + RINDEX_ENTRIES_SECTORS, + RINDEX_ENTRY_SIZE +) + from . import util -from . units import * +from . units import MiB def test_single_instance(sanlock_daemon): @@ -126,8 +134,7 @@ def test_create(tmpdir, sanlock_daemon): # New entry should be created at the first slot # The first rindex sector is used by the rindex header. f.seek(MiB + 512) - util.check_rindex_entry(f.read(RINDEX_ENTRY_SIZE), - b"res", 3 * MiB, 0) + util.check_rindex_entry(f.read(RINDEX_ENTRY_SIZE), b"res", 3 * MiB, 0) # The rest of the entries should not be modified. rest = 512 * RINDEX_ENTRIES_SECTORS - RINDEX_ENTRY_SIZE diff --git a/tests/direct_test.py b/tests/direct_test.py index a157bc4..339ef51 100644 --- a/tests/direct_test.py +++ b/tests/direct_test.py @@ -8,7 +8,7 @@ import struct from . import constants from . import util -from . units import * +from . units import MiB def test_init_lockspace(tmpdir): diff --git a/tests/python_test.py b/tests/python_test.py index 8822521..a032951 100644 --- a/tests/python_test.py +++ b/tests/python_test.py @@ -17,7 +17,7 @@ import sanlock from . import constants from . import util -from . units import * +from . units import KiB, MiB, GiB, TiB # Largest file size on ext4 is 16TiB, and on xfs 500 TiB. Use 1 TiB as it is diff --git a/tests/util.py b/tests/util.py index 243b1d3..6a648c4 100644 --- a/tests/util.py +++ b/tests/util.py @@ -11,7 +11,7 @@ import struct import subprocess import time -from . units import * +from . units import KiB TESTDIR = os.path.dirname(__file__) SANLOCK = os.path.join(TESTDIR, os.pardir, "src", "sanlock") -- To stop receiving notification emails like this one, please contact the administrator of this repository.

1 0

[sanlock] 01/04: python: Add flak8 env
by pagure＠pagure.io 17 Jun '19

17 Jun '19

This is an automated email from the git hooks/post-receive script. nsoffer pushed a commit to branch master in repository sanlock. commit aa7d0bf5b0f196a9340163f0aff89f6dbdf349f0 Author: Nir Soffer <nsoffer(a)redhat.com> AuthorDate: Fri Jun 14 19:53:17 2019 +0300 python: Add flak8 env Flake8 provides basic linting and style checks for python code, helping to keep the source in good shape. Running tox will run flake8 automatically, showing errors in the source and statistics for current issues. To run only flake8 tests use: $ tox -e flake8 Running tox will fail now with these errors: 1 E203 whitespace before ',' 1 E265 block comment should start with '# ' 7 E302 expected 2 blank lines, found 1 1 E305 expected 2 blank lines after class or function definition, found 1 14 E501 line too long (82 > 79 characters) 1 F401 'stat' imported but unused 5 F403 'from .constants import *' used; unable to detect undefined names 44 F405 'MiB' may be undefined, or defined from star imports: .constants, .units 1 W391 blank line at end of file Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> --- .travis.yml | 4 ++++ README.dev | 4 ++++ tox.ini | 9 ++++++++- 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 7b3fbe5..89fb52a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -16,7 +16,11 @@ addons: - libblkid-dev - make +install: + - pip install flake8 + script: - make BUILDARGS="--build-lib=." - source tests/env.sh - pytest + - flake8 --statistics tests python diff --git a/README.dev b/README.dev index cc2c613..8ade5e6 100644 --- a/README.dev +++ b/README.dev @@ -37,6 +37,10 @@ To run only tests matching the substring "foo": $ tox -- -k foo +To run basic lint and style check: + + $ tox -e flake8 + Testing 4K support ================== diff --git a/tox.ini b/tox.ini index 5a48b23..0e337ce 100644 --- a/tox.ini +++ b/tox.ini @@ -4,7 +4,7 @@ # and then run "tox" from this directory. [tox] -envlist = py27,py36 +envlist = py27,py36,flake8 skipsdist = True skip_missing_interpreters = True @@ -22,6 +22,10 @@ commands = py36: make PY_VERSION=3.6 BUILDARGS="--build-lib={envsitepackagesdir}" pytest {posargs} +[testenv:flake8] +deps = flake8 +commands = flake8 --statistics tests python + [pytest] # Notes: # --basetemp: we must use /var/tmp as sanlock uses direct I/O. @@ -29,3 +33,6 @@ commands = # -rxs: show extra test summary: (s)skipped, (x)failed # --durations: show slowest test duration addopts = -rxs -vv --basetemp=/var/tmp/sanlock --durations=10 + +[flake8] +show_source = True -- To stop receiving notification emails like this one, please contact the administrator of this repository.

1 0

Re: [PATCH v2 3/8] python: Streamline initexception
by Nir Soffer 17 Jun '19

17 Jun '19

On Mon, Jun 17, 2019 at 12:47 PM Pavel Bar <pbar(a)redhat.com> wrote: > Just something to consider: *maybe* adding "finally" label at the end and > freeing all the local variables using "Py_CLEAR" in the reverse order of > their initialization and always returning "excp" (initialized to NULL at > the beginning) could be more readable. Just a thought. > The point of this patch is to remove the cleanup. Why keep the local variable when we can get rid of them quickly? On Sun, Jun 16, 2019 at 11:50 PM Nir Soffer <nirsof(a)gmail.com> wrote: > >> When we create the sanlock expcetion we create a function, create a >> > > expcetion --> exception > > >> method from the function, create a dict from the method, and finally >> create an exception from the dict. This simple flow does not require >> goto for cleanup. >> >> Creating a dict is simpler and less error prone using Py_BuildValue(). >> >> When we finish with unused pointers, it is safer to set them to NULL >> using Py_CLEAR(), ensuring that we don't access freed memory. >> >> Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> >> --- >> python/sanlock.c | 31 ++++++++++++------------------- >> 1 file changed, 12 insertions(+), 19 deletions(-) >> >> diff --git a/python/sanlock.c b/python/sanlock.c >> index 21a7a76..5c2fd1a 100644 >> --- a/python/sanlock.c >> +++ b/python/sanlock.c >> @@ -1741,33 +1741,26 @@ sanlock_exception = { >> }; >> >> static PyObject * >> initexception(void) >> { >> - int rv; >> - PyObject *dict, *func, *meth, *excp = NULL; >> - >> - if ((dict = PyDict_New()) == NULL) >> - goto exit_fail; >> - >> - if ((func = PyCFunction_New(&sanlock_exception, NULL)) == NULL) >> - goto exit_fail; >> + PyObject *func = PyCFunction_New(&sanlock_exception, NULL); >> + if (func == NULL) >> + return NULL; >> >> - meth = PyObject_CallFunction((PyObject *) &PyProperty_Type, "O", >> func); >> - Py_DECREF(func); >> + PyObject *meth = PyObject_CallFunction((PyObject *) >> &PyProperty_Type, "O", func); >> + Py_CLEAR(func); >> if (meth == NULL) >> - goto exit_fail; >> - >> - rv = PyDict_SetItemString(dict, sanlock_exception.ml_name, meth); >> - Py_DECREF(meth); >> - if (rv < 0) >> - goto exit_fail; >> + return NULL; >> >> - excp = PyErr_NewException("sanlock.SanlockException", NULL, dict); >> + PyObject *dict = Py_BuildValue("{s:O}", sanlock_exception.ml_name, >> meth); >> + Py_CLEAR(meth); >> + if (dict == NULL) >> + return NULL; >> >> -exit_fail: >> - Py_XDECREF(dict); >> + PyObject *excp = PyErr_NewException("sanlock.SanlockException", >> NULL, dict); >> + Py_CLEAR(dict); >> >> return excp; >> } >> >> static int >> -- >> 2.17.2 >> >>

1 0

[sanlock] 02/02: tests: add tests for clearing resource
by pagure＠pagure.io 17 Jun '19

17 Jun '19

This is an automated email from the git hooks/post-receive script. nsoffer pushed a commit to branch master in repository sanlock. commit bd061bf4586d62223c8ebe61c487c1e83194de2b Author: Vojtech Juranek <vjuranek(a)redhat.com> AuthorDate: Mon Jun 17 12:39:07 2019 +0200 tests: add tests for clearing resource Add tests for write_resource() with parameter 'clear' set to True, which should result into clearing the resource (overwriting PAXOS_DISK_MAGIC with PAXOS_DISK_CLEAR). Also add tests for corner cases like calling clear on empty storage or with empty lockspace and resource name. --- tests/constants.py | 4 ++++ tests/python_test.py | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/tests/constants.py b/tests/constants.py index a7eb16b..21089b8 100644 --- a/tests/constants.py +++ b/tests/constants.py @@ -17,3 +17,7 @@ RINDEX_DISK_MAGIC = 0x01042018 RINDEX_ENTRY_SIZE = 64 RINDEX_ENTRIES_SECTORS = 2000 + +# src/sanlock_rv.h + +SANLK_LEADER_MAGIC = -223 diff --git a/tests/python_test.py b/tests/python_test.py index 58d94c9..8822521 100644 --- a/tests/python_test.py +++ b/tests/python_test.py @@ -227,6 +227,53 @@ def test_write_resource_4k_invalid_sector_size(sanlock_daemon, user_4k_path): assert e.value.errno == errno.EINVAL +def test_clear_resource(tmpdir, sanlock_daemon): + path = util.generate_path(tmpdir, "clear_test") + util.create_file(path, MiB) + disks = [(path, 0)] + + sanlock.write_resource(b"ls_name", b"res_name", disks) + sanlock.write_resource(b"ls_name", b"res_name", disks, clear=True) + + with pytest.raises(sanlock.SanlockException) as e: + sanlock.read_resource(path) + assert e.value.errno == constants.SANLK_LEADER_MAGIC + + magic = util.read_magic(path) + assert magic == constants.PAXOS_DISK_CLEAR + + util.check_guard(path, MiB) + + # run clear on already cleared resource + sanlock.write_resource(b"ls_name", b"res_name", disks, clear=True) + magic = util.read_magic(path) + assert magic == constants.PAXOS_DISK_CLEAR + + +def test_clear_empty_lockspace_resource(tmpdir, sanlock_daemon): + path = util.generate_path(tmpdir, "clear_test") + util.create_file(path, MiB) + disks = [(path, 0)] + + sanlock.write_resource(b"ls_name", b"res_name", disks) + + # Clear with empty lockspace and resource - should succeed + sanlock.write_resource(b"", b"", disks, clear=True) + magic = util.read_magic(path) + assert magic == constants.PAXOS_DISK_CLEAR + + +def test_clear_empty_storage(tmpdir, sanlock_daemon): + path = util.generate_path(tmpdir, "clear_test") + util.create_file(path, MiB) + disks = [(path, 0)] + + # Clear area without any resource written - should succeed + sanlock.write_resource(b"ls_name", b"inval_res_name", disks, clear=True) + magic = util.read_magic(path) + assert magic == constants.PAXOS_DISK_CLEAR + + def test_read_resource_4k_invalid_sector_size(sanlock_daemon, user_4k_path): disks = [(user_4k_path, 0)] -- To stop receiving notification emails like this one, please contact the administrator of this repository.

1 0

[sanlock] 01/02: tests: replace reading of magic number by helper function
by pagure＠pagure.io 17 Jun '19

17 Jun '19

This is an automated email from the git hooks/post-receive script. nsoffer pushed a commit to branch master in repository sanlock. commit 272221e6fcadda71b95487931286666da810ceda Author: Vojtech Juranek <vjuranek(a)redhat.com> AuthorDate: Mon Jun 17 12:39:06 2019 +0200 tests: replace reading of magic number by helper function Introduce helper function for reading sanlock magic numbers. This helper function seeks to specified offset in given file and reads unsigned int from this position. --- tests/python_test.py | 25 +++++++++---------------- tests/util.py | 11 +++++++++++ 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/tests/python_test.py b/tests/python_test.py index 3242f57..58d94c9 100644 --- a/tests/python_test.py +++ b/tests/python_test.py @@ -86,12 +86,9 @@ def test_write_lockspace(tmpdir, sanlock_daemon, filename, encoding, size, offse b"ls_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... + magic = util.read_magic(path, offset) + assert magic == constants.DELTA_DISK_MAGIC + # TODO: check more stuff here... util.check_guard(path, size) @@ -117,9 +114,8 @@ def test_write_lockspace_4k(user_4k_path, sanlock_daemon, align): 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 + magic = util.read_magic(user_4k_path) + assert magic == constants.DELTA_DISK_MAGIC # Check that sanlock did not write beyond the lockspace area. util.check_guard(user_4k_path, align) @@ -179,10 +175,8 @@ def test_write_resource(tmpdir, sanlock_daemon, filename, encoding, size, offset owners = sanlock.read_resource_owners(b"ls_name", b"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 + magic = util.read_magic(path, offset) + assert magic == constants.PAXOS_DISK_MAGIC # TODO: check more stuff here... @@ -216,9 +210,8 @@ def test_write_resource_4k(sanlock_daemon, user_4k_path, align): 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 + magic = util.read_magic(user_4k_path) + assert magic == constants.PAXOS_DISK_MAGIC # Check that sanlock did not write beyond the lockspace area. util.check_guard(user_4k_path, align) diff --git a/tests/util.py b/tests/util.py index 08db236..243b1d3 100644 --- a/tests/util.py +++ b/tests/util.py @@ -177,3 +177,14 @@ def sanlock_is_running(): if e.errno != errno.ENOENT: raise return False + + +def read_magic(path, offset=0): + """ + Read and return unsigned int from specified file at given offset. + Typical usage is to read sanlock magic number. + """ + with io.open(path, "rb") as f: + f.seek(offset) + uint, = struct.unpack("< I", f.read(4)) + return uint -- To stop receiving notification emails like this one, please contact the administrator of this repository.

1 0

Re: [PATCH v2 4/8] python: Extract create_resource() helper
by Vojtech Juranek 17 Jun '19

17 Jun '19

On pondělí 17. června 2019 12:04:35 CEST Pavel Bar wrote: > On Sun, Jun 16, 2019 at 11:50 PM Nir Soffer <nirsof(a)gmail.com> wrote: > > Replace duplicate code for allocating and initializing sanlk_resource > > struct with a helper. > > > > Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> > > --- > > > > python/sanlock.c | 46 +++++++++++++++++++++++----------------------- > > 1 file changed, 23 insertions(+), 23 deletions(-) > > > > diff --git a/python/sanlock.c b/python/sanlock.c > > index 5c2fd1a..6824e01 100644 > > --- a/python/sanlock.c > > +++ b/python/sanlock.c > > @@ -170,29 +170,38 @@ parse_single_disk(PyObject* disk, struct sanlk_disk* > > res_disk) > > > > finally: > > Py_XDECREF(path); > > return rv; > > > > } > > > > +static struct sanlk_resource * > > +create_resource(int num_disks) > > +{ > > + size_t size = sizeof(struct sanlk_resource) + > > + sizeof(struct sanlk_disk) * num_disks; > > + > > + struct sanlk_resource *res = calloc(1, size); > > + if (res == NULL) { > > + PyErr_NoMemory(); > > + return NULL; > > + } > > + > > + res->num_disks = num_disks; > > + > > + return res; > > +} > > + > > > > static int > > parse_disks(PyObject *obj, struct sanlk_resource **res_ret) > > { > > > > - int num_disks, res_len; > > + int num_disks; > > > > struct sanlk_resource *res; > > What about declaring the local variables when first used, thus immediately > initializing them? I think C99 provides that. > I.e., > int num_disks = PyList_Size(obj); > struct sanlk_resource *res = create_resource(num_disks); > Moreover, in this case they will be at the beginning of the block anyway. agree, IMHO good idea to initialize variables once they are declared > > num_disks = PyList_Size(obj); > > > > - res_len = sizeof(struct sanlk_resource) + > > - (sizeof(struct sanlk_disk) * num_disks); > > - res = malloc(res_len); > > - > > - if (res == NULL) { > > - PyErr_NoMemory(); > > + res = create_resource(num_disks); > > + if (res == NULL) > > > > return -1; > > > > - } > > - > > - memset(res, 0, res_len); > > - res->num_disks = num_disks; > > > > for (int i = 0; i < num_disks; i++) { > > > > PyObject *disk = PyList_GetItem(obj,i); > > > > if (!parse_single_disk(disk, &(res->disks[i]))) { > > > > @@ -637,30 +646,21 @@ Align can be one of (1048576, 2097152, 4194304, > > 8388608).\n\ > > > > Sector can be one of (512, 4096)."); > > > > static PyObject * > > py_read_resource(PyObject *self __unused, PyObject *args, PyObject > > > > *keywds) > > > > { > > > > - int rv = -1, res_len, sector = SECTOR_SIZE_512; > > + int rv = -1, sector = SECTOR_SIZE_512; > > > > long align = ALIGNMENT_1M; > > PyObject *path = NULL; > > struct sanlk_resource *res; > > PyObject *res_info = NULL; > > > > static char *kwlist[] = {"path", "offset", "align", "sector", NULL}; > > > > - /* allocate the needed memory for the resource and one disk */ > > - res_len = sizeof(struct sanlk_resource) + sizeof(struct sanlk_disk); > > - res = malloc(res_len); > > - > > - if (res == NULL) { > > - PyErr_NoMemory(); > > + res = create_resource(1 /* num_disks */); > > Not sure the comment is needed. > > > + if (res == NULL) > > > > return NULL; > > > > - } > > - > > - /* initialize resource and disk structures */ > > - memset(res, 0, res_len); > > - res->num_disks = 1; > > > > /* parse python tuple */ > > if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&|kli", kwlist, > > > > pypath_converter, &path, &(res->disks[0].offset), &align, > > > > &sector)) { > > > > goto finally; > > > > -- > > 2.17.2

1 0

Re: [PATCH v2 1/2] tests: replace reading of magic number by helper function
by Nir Soffer 16 Jun '19

16 Jun '19

On Sun, Jun 16, 2019 at 4:36 PM Pavel Bar <pbar(a)redhat.com> wrote: > > > On Sat, Jun 15, 2019 at 1:02 AM Nir Soffer <nsoffer(a)redhat.com> wrote: > >> On Sat, Jun 15, 2019 at 12:55 AM Vojtech Juranek <vjuranek(a)redhat.com> >> wrote: >> >>> Introduce helper function for reading sanlock magic numbers. This >>> helper function seeks to specified offset in given file and reads >>> unsigned int from this position. >>> --- >>> tests/python_test.py | 25 +++++++++---------------- >>> tests/util.py | 11 +++++++++++ >>> 2 files changed, 20 insertions(+), 16 deletions(-) >>> >>> diff --git a/tests/python_test.py b/tests/python_test.py >>> index 3242f57..623a13c 100644 >>> --- a/tests/python_test.py >>> +++ b/tests/python_test.py >>> @@ -86,12 +86,9 @@ def test_write_lockspace(tmpdir, sanlock_daemon, >>> filename, encoding, size, offse >>> b"ls_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... >>> + magic = util.read_uint(path, offset) >>> + assert magic == constants.DELTA_DISK_MAGIC >>> + # TODO: check more stuff here... >>> >>> util.check_guard(path, size) >>> >>> @@ -117,9 +114,8 @@ def test_write_lockspace_4k(user_4k_path, >>> sanlock_daemon, align): >>> 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 >>> + magic = util.read_uint(user_4k_path) >>> + assert magic == constants.DELTA_DISK_MAGIC >>> >>> # Check that sanlock did not write beyond the lockspace area. >>> util.check_guard(user_4k_path, align) >>> @@ -179,10 +175,8 @@ def test_write_resource(tmpdir, sanlock_daemon, >>> filename, encoding, size, offset >>> owners = sanlock.read_resource_owners(b"ls_name", b"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 >>> + magic = util.read_uint(path, offset) >>> + assert magic == constants.PAXOS_DISK_MAGIC >>> >>> # TODO: check more stuff here... >>> >>> @@ -216,9 +210,8 @@ def test_write_resource_4k(sanlock_daemon, >>> user_4k_path, align): >>> 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 >>> + magic = util.read_uint(user_4k_path) >>> + assert magic == constants.PAXOS_DISK_MAGIC >>> >>> # Check that sanlock did not write beyond the lockspace area. >>> util.check_guard(user_4k_path, align) >>> diff --git a/tests/util.py b/tests/util.py >>> index 08db236..32b34ec 100644 >>> --- a/tests/util.py >>> +++ b/tests/util.py >>> @@ -177,3 +177,14 @@ def sanlock_is_running(): >>> if e.errno != errno.ENOENT: >>> raise >>> return False >>> + >>> + >>> +def read_uint(path, offset=0): >>> + """ >>> + Read and return unsigned int from specified file at given offset. >>> + Typical usage is to read sanlock magic number. >>> + """ >>> + with io.open(path, "rb") as f: >>> + f.seek(offset) >>> + uint, = struct.unpack("< I", f.read(4)) >>> + return uint >>> -- >>> 2.20.1 >>> >>> >> Looks good, but maybe call it read_magic(path, offset)? >> > > Looks good. > Regarding the Nir's comment, suggest "read_magic_int". > read_magic_init does not not add anything useful. We will never have read_magic_float so there is no reason to include the type in the name.

1 0

[PATCH v2 0/4] Fix python style issues
by Nir Soffer 16 Jun '19

16 Jun '19

Fix python style issues and add flake8 style check to the build to keep the code in good shape. Changes since v1: - Add flake8 to travis build - Replace star imports with explicit imports to allow flake8 detection of undefined names (Pavel) - Convert example.py to python3 - breaks flake8 on python 3 builds. v1 was here: https://lists.fedorahosted.org/archives/list/sanlock-devel@lists.fedorahost… Nir Soffer (4): python: Add flak8 env python: Avoid star imports python: Fix style issues python: Convert example.py to python 3 .travis.yml | 4 ++++ README.dev | 4 ++++ python/example.py | 56 +++++++++++++++++++++++++++----------------- tests/conftest.py | 8 +++---- tests/daemon_test.py | 15 ++++++++---- tests/direct_test.py | 2 +- tests/python_test.py | 48 +++++++++++++++++++++++++------------ tests/util.py | 2 +- tox.ini | 9 ++++++- 9 files changed, 101 insertions(+), 47 deletions(-) -- 2.17.2

1 4

Re: [PATCH 10/14] python: Remove unneeded check for null
by Nir Soffer 16 Jun '19

16 Jun '19

On Sun, Jun 16, 2019 at 6:40 PM Pavel Bar <pbar(a)redhat.com> wrote: > I think there are more places like that in "sanlock.c". > git grep thinks differently: $ git grep -B1 'free(' python python/sanlock.c-exit_fail: python/sanlock.c: free(res); -- python/sanlock.c- Py_XDECREF(resource); python/sanlock.c: free(res); -- python/sanlock.c-finally: python/sanlock.c: free(rs); -- python/sanlock.c- Py_XDECREF(resource); python/sanlock.c: free(rs); -- python/sanlock.c- /* success */ python/sanlock.c: free(lss); -- python/sanlock.c-exit_fail: python/sanlock.c: if (lss) free(lss); -- python/sanlock.c- Py_XDECREF(lockspace); python/sanlock.c: free(hss); -- python/sanlock.c- Py_XDECREF(resource); python/sanlock.c: free(res); -- python/sanlock.c- Py_XDECREF(resource); python/sanlock.c: free(res); -- python/sanlock.c- Py_XDECREF(resource); python/sanlock.c: free(res); -- python/sanlock.c- Py_XDECREF(resource); python/sanlock.c: free(res); python/sanlock.c: free(hss); > > On Fri, Jun 14, 2019 at 11:54 PM Nir Soffer <nirsof(a)gmail.com> wrote: > >> When calling free() we don't need to check if a point is NULL. >> >> Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> >> --- >> python/sanlock.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/python/sanlock.c b/python/sanlock.c >> index ed9de0f..4c3e0a0 100644 >> --- a/python/sanlock.c >> +++ b/python/sanlock.c >> @@ -1025,11 +1025,11 @@ py_get_lockspaces(PyObject *self __unused, >> PyObject *args, PyObject *keywds) >> free(lss); >> return ls_list; >> >> /* failure */ >> exit_fail: >> - if (lss) free(lss); >> + free(lss); >> Py_XDECREF(ls_entry); >> Py_XDECREF(ls_list); >> return NULL; >> } >> >> -- >> 2.17.2 >> >>

1 0

Re: [PATCH 2/2] python: Fix style issues
by Nir Soffer 16 Jun '19

16 Jun '19

On Sun, Jun 16, 2019 at 4:11 PM Pavel Bar <pbar(a)redhat.com> wrote: > > > On Fri, Jun 14, 2019 at 8:15 PM Nir Soffer <nirsof(a)gmail.com> wrote: > >> Fix style issues: >> >> - 2 blank lines around functions and classes >> - classes at the top of the module >> - reformat to avoid long hard to read lines >> - whitespace before ',' >> - whitespace after # >> - unused imports >> - remove blank line at end of file >> >> Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> >> --- >> python/example.py | 28 +++++++++++++++++++-------- >> tests/conftest.py | 8 ++++---- >> tests/python_test.py | 46 ++++++++++++++++++++++++++++++-------------- >> 3 files changed, 56 insertions(+), 26 deletions(-) >> >> diff --git a/python/example.py b/python/example.py >> index 6ef9f7d..1be7e66 100644 >> --- a/python/example.py >> +++ b/python/example.py >> @@ -8,41 +8,46 @@ import sanlock >> >> HOST_ID = 1 >> LOCKSPACE_NAME = "lockspace1" >> RESOURCE_NAME = "resource1" >> >> + >> def sigTermHandler(): >> print "SIGTERM signal received" >> >> + >> def main(): >> signal.signal(signal.SIGTERM, sigTermHandler) >> >> print "Creating the sanlock disk" >> fd, disk = tempfile.mkstemp() >> os.close(fd) >> >> - os.chown(disk, pwd.getpwnam("sanlock").pw_uid, >> grp.getgrnam("sanlock").gr_gid) >> + os.chown( >> + disk, pwd.getpwnam("sanlock").pw_uid, >> grp.getgrnam("sanlock").gr_gid) >> > > Not sure that the 2nd style is better, IMHO, it's even worse. > The same long line with lot's of parameters with/without explicitly naming > the receiving parameter's name. > In my opinion if doing such a cleanup/improvement work, it should't be > just "to mark a V" that some tool is not complaining anymore. > In this case I believe it's better to split the line, so every parameter > will appear in a single line as it really adds to readability. That's what > you did below with "*owners = sanlock.read_resource_owners*..." > Using one argument per for all calls does not improve readability, so we need a simple rule when to more to one argument per line style. My general indentation rules are: 1. use one line if it fits in one line 2. or move arguments to next line if it fits in 2 lines 3. or use one line per argument This case matches rule 2. offset = sanlock.get_alignment(disk) >> >> SNLK_DISKS = [(disk, offset)] >> >> print "Registering to sanlock" >> fd = sanlock.register() >> >> print "Initializing '%s'" % (LOCKSPACE_NAME,) >> - sanlock.write_lockspace(LOCKSPACE_NAME, disk, max_hosts=0, >> iotimeout=0, align=1048576, sector=512) >> + sanlock.write_lockspace(LOCKSPACE_NAME, disk, align=1048576, >> sector=512) >> >> print "Initializing '%s' on '%s'" % (RESOURCE_NAME, LOCKSPACE_NAME) >> - sanlock.write_resource(LOCKSPACE_NAME, RESOURCE_NAME, SNLK_DISKS, >> align=1048576, sector=512) >> + sanlock.write_resource( >> + LOCKSPACE_NAME, RESOURCE_NAME, SNLK_DISKS, align=1048576, >> sector=512) >> > > Same note as above. > > >> >> print "Acquiring the id '%i' on '%s'" % (HOST_ID, LOCKSPACE_NAME) >> sanlock.add_lockspace(LOCKSPACE_NAME, HOST_ID, disk) >> >> try: >> print "Acquiring '%s' on '%s'" % (RESOURCE_NAME, LOCKSPACE_NAME) >> - sanlock.acquire(LOCKSPACE_NAME, RESOURCE_NAME, SNLK_DISKS, >> slkfd=fd, >> - version=0) >> + sanlock.acquire( >> + LOCKSPACE_NAME, RESOURCE_NAME, SNLK_DISKS, slkfd=fd, >> version=0) >> + >> while True: >> print "Trying to get lockspace '%s' hosts" % LOCKSPACE_NAME >> try: >> hosts_list = sanlock.get_hosts(LOCKSPACE_NAME) >> except sanlock.SanlockException as e: >> @@ -50,13 +55,19 @@ def main(): >> raise >> else: >> print "Lockspace '%s' hosts: " % LOCKSPACE_NAME, >> hosts_list >> break >> time.sleep(5) >> - print "Resource '%s' owners: " % RESOURCE_NAME, \ >> - sanlock.read_resource_owners( >> - LOCKSPACE_NAME, RESOURCE_NAME, SNLK_DISKS, >> align=1048576, sector=512) >> + >> + owners = sanlock.read_resource_owners( >> + LOCKSPACE_NAME, >> + RESOURCE_NAME, >> + SNLK_DISKS, >> + align=1048576, >> + sector=512) >> + print "Resource '%s' owners: %s" % (RESOURCE_NAME, owners) >> > > Doesn't the "print " require "()" in Python 3? Don't we want to be Python > 3 compatible (at least if we "touch" some code)? > Yes, the example code is not python 3 compatible yet. Converting it to python 3 require additional work, but it is not easy as this example is not easy to test. + >> print "Releasing '%s' on '%s'" % (RESOURCE_NAME, LOCKSPACE_NAME) >> sanlock.release(LOCKSPACE_NAME, RESOURCE_NAME, SNLK_DISKS, >> slkfd=fd) >> except Exception as e: >> print "Exception: ", e >> finally: >> @@ -64,7 +75,8 @@ def main(): >> sanlock.rem_lockspace(LOCKSPACE_NAME, HOST_ID, disk) >> >> print "Removing the sanlock disk" >> os.remove(disk) >> >> + >> if __name__ == '__main__': >> main() >> diff --git a/tests/conftest.py b/tests/conftest.py >> index 70e83d4..868ae46 100644 >> --- a/tests/conftest.py >> +++ b/tests/conftest.py >> @@ -2,18 +2,21 @@ >> Fixtures for sanlock testing. >> """ >> from __future__ import absolute_import >> >> import os >> -import stat >> >> import pytest >> >> from . import storage >> from . import util >> >> >> +class SanlockIsRunning(Exception): >> + """ Raised if sanlock running when it should not """ >> + >> + >> @pytest.fixture >> def sanlock_daemon(): >> """ >> Run sanlock daemon during a test. >> """ >> @@ -48,8 +51,5 @@ def user_4k_path(request): >> >> @pytest.fixture >> def no_sanlock_daemon(): >> if util.sanlock_is_running(): >> raise SanlockIsRunning >> - >> -class SanlockIsRunning(Exception): >> - pass >> diff --git a/tests/python_test.py b/tests/python_test.py >> index 3242f57..f37eab4 100644 >> --- a/tests/python_test.py >> +++ b/tests/python_test.py >> @@ -34,11 +34,11 @@ ALIGNMENT_2M = 2 * MiB >> SECTOR_SIZE_512 = 512 >> SECTOR_SIZE_4K = 4 * KiB >> >> >> FILE_NAMES = [ >> - #name, encoding >> + # name, encoding >> ("ascii", None), >> (u"ascii", None), >> (u"\u05d0", None), >> (u"\u05d0", "utf-8"), >> ] >> @@ -47,25 +47,31 @@ LOCKSPACE_OR_RESOURCE_NAMES = [ >> # Bytes are supported with python 2 and 3. >> pytest.param(b"\xd7\x90"), >> # Python 2 also supports str. >> pytest.param( >> "\xd7\x90", >> - marks=pytest.mark.skipif(six.PY3, reason="python 3 supports only >> bytes")), >> + marks=pytest.mark.skipif( >> + six.PY3, >> + reason="python 3 supports only bytes")), >> # Python 2 also supports unicode with ascii content. >> pytest.param( >> u"ascii", >> - marks=pytest.mark.skipif(six.PY3, reason="python 3 supports only >> bytes")), >> + marks=pytest.mark.skipif( >> + six.PY3, >> + reason="python 3 supports only bytes")), >> ] >> >> -(a)pytest.mark.parametrize("filename, encoding" , FILE_NAMES) >> -(a)pytest.mark.parametrize("size,offset", [ >> + >> +(a)pytest.mark.parametrize("filename, encoding", FILE_NAMES) >> +(a)pytest.mark.parametrize("size, offset", [ >> # Smallest offset. >> (LOCKSPACE_SIZE, 0), >> # Large offset. >> (LARGE_FILE_SIZE, LARGE_FILE_SIZE - LOCKSPACE_SIZE), >> ]) >> -def test_write_lockspace(tmpdir, sanlock_daemon, filename, encoding, >> size, offset): >> +def test_write_lockspace( >> + tmpdir, sanlock_daemon, filename, encoding, size, offset): >> path = util.generate_path(tmpdir, filename, encoding) >> util.create_file(path, size) >> >> # Test read and write with default alignment and sector size values. >> sanlock.write_lockspace(b"ls_name", path, offset=offset, iotimeout=1) >> @@ -104,11 +110,15 @@ def test_write_lockspace_4k(user_4k_path, >> sanlock_daemon, align): >> with io.open(user_4k_path, "rb+") as f: >> f.write(align * b"x") >> util.write_guard(user_4k_path, align) >> >> sanlock.write_lockspace( >> - b"ls_name", user_4k_path, iotimeout=1, align=align, >> sector=SECTOR_SIZE_4K) >> + b"ls_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": b"ls_name"} >> @@ -146,11 +156,12 @@ def >> test_read_lockspace_4k_invalid_sector_size(sanlock_daemon, user_4k_path): >> # Smallest offset. >> (MIN_RES_SIZE, 0), >> # Large offset. >> (LARGE_FILE_SIZE, LARGE_FILE_SIZE - MIN_RES_SIZE), >> ]) >> -def test_write_resource(tmpdir, sanlock_daemon, filename, encoding, >> size, offset): >> +def test_write_resource( >> + tmpdir, sanlock_daemon, filename, encoding, size, offset): >> path = util.generate_path(tmpdir, filename, encoding) >> util.create_file(path, size) >> disks = [(path, offset)] >> >> # Test read and write with default alignment and sector size values. >> @@ -525,19 +536,21 @@ def >> test_add_lockspace_parse_args(no_sanlock_daemon, name, filename, encoding): >> sanlock.add_lockspace(name, 1, path, 0, wait=False) >> >> >> @pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES) >> @pytest.mark.parametrize("filename,encoding", FILE_NAMES) >> -def test_write_lockspace_parse_args(no_sanlock_daemon, name, filename, >> encoding): >> +def test_write_lockspace_parse_args( >> + no_sanlock_daemon, name, filename, encoding): >> path = util.generate_path("/tmp/", filename, encoding) >> with raises_sanlock_errno(): >> sanlock.write_lockspace(name, path) >> >> >> @pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES) >> @pytest.mark.parametrize("filename,encoding", FILE_NAMES) >> -def test_write_resource_parse_args(no_sanlock_daemon, name, filename, >> encoding): >> +def test_write_resource_parse_args( >> + no_sanlock_daemon, name, filename, encoding): >> path = util.generate_path("/tmp/", filename, encoding) >> disks = [(path, 0)] >> with raises_sanlock_errno(): >> sanlock.write_resource(name, b"res_name", disks) >> >> @@ -545,11 +558,12 @@ def >> test_write_resource_parse_args(no_sanlock_daemon, name, filename, encoding): >> sanlock.write_resource(b"ls_name", name, disks) >> >> >> @pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES) >> @pytest.mark.parametrize("filename,encoding", FILE_NAMES) >> -def test_release_resource_parse_args(no_sanlock_daemon, name, filename, >> encoding): >> +def test_release_resource_parse_args( >> + no_sanlock_daemon, name, filename, encoding): >> path = util.generate_path("/tmp/", filename, encoding) >> disks = [(path, 0)] >> with raises_sanlock_errno(): >> sanlock.release(name, b"res_name", disks) >> >> @@ -557,11 +571,12 @@ def >> test_release_resource_parse_args(no_sanlock_daemon, name, filename, encoding >> sanlock.release(b"ls_name", name, disks) >> >> >> @pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES) >> @pytest.mark.parametrize("filename,encoding", FILE_NAMES) >> -def test_read_resource_owners_parse_args(no_sanlock_daemon, name, >> filename, encoding): >> +def test_read_resource_owners_parse_args( >> + no_sanlock_daemon, name, filename, encoding): >> path = util.generate_path("/tmp/", filename, encoding) >> disks = [(path, 0)] >> with raises_sanlock_errno(): >> sanlock.read_resource_owners(name, b"res_name", disks) >> >> @@ -601,11 +616,12 @@ def test_set_event_parse_args(no_sanlock_daemon, >> name): >> sanlock.set_event(name, 1, 1, 1) >> >> >> @pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES) >> @pytest.mark.parametrize("filename,encoding", FILE_NAMES) >> -def test_init_lockspace_parse_args(no_sanlock_daemon, name, filename, >> encoding): >> +def test_init_lockspace_parse_args( >> + no_sanlock_daemon, name, filename, encoding): >> path = util.generate_path("/tmp/", filename, encoding) >> with raises_sanlock_errno(errno.ENODEV): >> sanlock.init_lockspace(name, path) >> >> >> @@ -617,23 +633,25 @@ def >> test_init_resource_parse_args(no_sanlock_daemon, name, filename, encoding): >> with raises_sanlock_errno(errno.ENOENT): >> sanlock.init_resource(b"ls_name", name, disks) >> with raises_sanlock_errno(errno.ENOENT): >> sanlock.init_resource(name, b"res_name", disks) >> >> + >> @pytest.mark.parametrize("filename,encoding", FILE_NAMES) >> def test_get_alignment_parse_args(no_sanlock_daemon, filename, encoding): >> path = util.generate_path("/tmp/", filename, encoding) >> with raises_sanlock_errno(errno.ENOENT): >> sanlock.get_alignment(path) >> >> + >> @pytest.mark.parametrize("filename,encoding", FILE_NAMES) >> def test_read_lockspace_parse_args(no_sanlock_daemon, filename, >> encoding): >> path = util.generate_path("/tmp/", filename, encoding) >> with raises_sanlock_errno(): >> sanlock.read_lockspace(path) >> >> + >> @pytest.mark.parametrize("filename,encoding", FILE_NAMES) >> def test_read_resource_parse_args(no_sanlock_daemon, filename, encoding): >> path = util.generate_path("/tmp/", filename, encoding) >> with raises_sanlock_errno(): >> sanlock.read_resource(path) >> - >> -- >> 2.17.2 >> >>

1 0

Re: [PATCH 1/2] build: Add flak8 env
by Nir Soffer 16 Jun '19

16 Jun '19

On Sun, Jun 16, 2019 at 3:32 PM Pavel Bar <pbar(a)redhat.com> wrote: > > > On Fri, Jun 14, 2019 at 8:15 PM Nir Soffer <nirsof(a)gmail.com> wrote: > >> Flake8 provides basic linting and style checks for python code, helping >> to keep the source in good shape. >> >> Running tox will run flake8 automatically, showing errors in the source >> and statistics for current issues. To run only flake8 tests use: >> >> $ tox -e flake8 >> >> We explicitly ignore errors about star import (from . units import *), >> since these imports make the source much more readable. >> >> Running tox will fail now with these errors: >> >> 1 E203 whitespace before ',' >> 1 E265 block comment should start with '# ' >> 7 E302 expected 2 blank lines, found 1 >> 1 E305 expected 2 blank lines after class or function definition, >> found 1 >> 14 E501 line too long (82 > 79 characters) > > > What does it mean? What's the line limit? 79? > flake8 default max-line-length is 79. > > >> 1 F401 'stat' imported but unused >> 1 W391 blank line at end of file >> >> Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> >> --- >> README.dev | 4 ++++ >> tox.ini | 12 +++++++++++- >> 2 files changed, 15 insertions(+), 1 deletion(-) >> >> diff --git a/README.dev b/README.dev >> index cc2c613..8ade5e6 100644 >> --- a/README.dev >> +++ b/README.dev >> @@ -35,10 +35,14 @@ To run only test from some modules: >> >> To run only tests matching the substring "foo": >> >> $ tox -- -k foo >> >> +To run basic lint and style check: >> + >> + $ tox -e flake8 >> + >> >> Testing 4K support >> ================== >> >> To enable the 4k tests, you need to setup 4k stroage for the tests: >> > > Typo. "storage" > Not related to this patch. > > >> diff --git a/tox.ini b/tox.ini >> index 5a48b23..ca63f55 100644 >> --- a/tox.ini >> +++ b/tox.ini >> @@ -2,11 +2,11 @@ >> # in multiple virtualenvs. This configuration file will run the >> # test suite on all supported python versions. To use it, "pip install >> tox" >> # and then run "tox" from this directory. >> >> [tox] >> -envlist = py27,py36 >> +envlist = py27,py36,flake8 >> skipsdist = True >> skip_missing_interpreters = True >> >> [testenv] >> passenv = * >> @@ -20,12 +20,22 @@ deps = >> commands = >> py27: make PY_VERSION=2.7 >> BUILDARGS="--build-lib={envsitepackagesdir}" >> py36: make PY_VERSION=3.6 >> BUILDARGS="--build-lib={envsitepackagesdir}" >> pytest {posargs} >> >> +[testenv:flake8] >> +deps = flake8 >> +commands = flake8 --statistics tests python >> + >> [pytest] >> # Notes: >> # --basetemp: we must use /var/tmp as sanlock uses direct I/O. >> # -vv: increasing verbosify twice shows more detailed failures >> tracebacks. >> # -rxs: show extra test summary: (s)skipped, (x)failed >> # --durations: show slowest test duration >> addopts = -rxs -vv --basetemp=/var/tmp/sanlock --durations=10 >> + >> +[flake8] >> +# We like to use start imports units and constants. >> > > Typo. "star" > Thanks, will fix before pushing. > >> +# F403 'from .units import *' used; unable to detect undefined names >> +# F405 'KiB' may be undefined, or defined from star imports: .units >> > > Just want to understand: > 1) F403 - That means that all "import *" except "from .units" will fail > the build? If so, this is great, I don't like "import *" :) > 2) F405 - I understand that we explicitly ignore "KiB" error. Why? When > the error happens? What about other constants? > The error was copied from flake8 --statistics output, so it is confusing. F403 ignores errors about star imports, which is fine. F405 ignores errors about undefined variables, which is less useful. I will remove these ignores in v3, since we really like to have warnings about undefined variables. > > >> +ignore = F403, F405 >> -- >> 2.17.2 >> >>

1 0

Re: [PATCH] spec: Convert development spec to python 3
by Nir Soffer 16 Jun '19

16 Jun '19

On Sun, Jun 16, 2019 at 2:21 PM Pavel Bar <pbar(a)redhat.com> wrote: > Looks good. Just 2 points to consider, see below: > > > On Wed, Jun 12, 2019 at 10:04 PM Nir Soffer <nirsof(a)gmail.com> wrote: > >> The development spec creates now only python 3 packages. This will be >> the base for Fedora 30 and CentOS 8.1 spec. >> >> Also make rpmlint happy about mixing tabs and spaces by using only >> spaces. >> >> Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> >> --- >> sanlock.spec.in | 42 +++++++++++++++++------------------------- >> 1 file changed, 17 insertions(+), 25 deletions(-) >> >> diff --git a/sanlock.spec.in b/sanlock.spec.in >> index 9ad2985..087076c 100644 >> --- a/sanlock.spec.in >> +++ b/sanlock.spec.in >> @@ -16,30 +16,23 @@ URL: https://pagure.io/sanlock/ >> BuildRequires: gcc >> BuildRequires: libaio-devel >> BuildRequires: libblkid-devel >> BuildRequires: libuuid-devel >> BuildRequires: make >> -BuildRequires: python2 >> -BuildRequires: python2-devel >> +BuildRequires: python3 >> +BuildRequires: python3-devel >> BuildRequires: systemd-units >> Requires: %{name}-lib = %{version}-%{release} >> Requires(pre): /usr/sbin/groupadd >> Requires(pre): /usr/sbin/useradd >> Requires(post): systemd-units >> Requires(post): systemd-sysv >> Requires(preun): systemd-units >> Requires(postun): systemd-units >> Source0: >> https://releases.pagure.org/sanlock/%{name}-%{version}.tar.gz >> >> -# On Fedora we switched sanlock-python to python2-sanlock, as part of >> python 3 >> -# porting. This change was not done yet for RHEL. >> -# TODO: remove when RHEL switches to pythonN-sanlock. >> -%if 0%{?fedora} >> -%global python_package python2-%{name} >> -%else # centos/rhel >> -%global python_package %{name}-python >> -%endif >> +%global python_package python3-%{name} >> >> %description >> The sanlock daemon manages leases for applications on hosts using shared >> storage. >> >> %prep >> @@ -51,11 +44,11 @@ The sanlock daemon manages leases for applications on >> hosts using shared storage >> %endif >> # upstream does not require configure >> # upstream does not support _smp_mflags >> CFLAGS=$RPM_OPT_FLAGS make -C wdmd >> CFLAGS=$RPM_OPT_FLAGS make -C src >> -CFLAGS=$RPM_OPT_FLAGS make -C python >> +CFLAGS=$RPM_OPT_FLAGS make -C python PY_VERSION=3.6 >> CFLAGS=$RPM_OPT_FLAGS make -C reset >> >> %install >> rm -rf $RPM_BUILD_ROOT >> make -C src \ >> @@ -65,11 +58,11 @@ make -C wdmd \ >> install LIBDIR=%{_libdir} \ >> DESTDIR=$RPM_BUILD_ROOT >> make -C python \ >> install LIBDIR=%{_libdir} \ >> DESTDIR=$RPM_BUILD_ROOT \ >> - PY_VERSION=2.7 # TODO: fix PY_VERSION to 3.6 when 2020 will >> arrive >> + PY_VERSION=3.6 >> > > 2nd usage of the hardcoded "3.6" Python version. Consider declaring a > constant to avoid the situation that someone in the future might change > only one of the appearances. > Good idea, I wasted few minutes while preparing this patch when one call was using 3.6, and the other 2.7. > > >> make -C reset \ >> install LIBDIR=%{_libdir} \ >> DESTDIR=$RPM_BUILD_ROOT >> >> >> @@ -77,28 +70,28 @@ install -D -m 0644 init.d/sanlock.service.native >> $RPM_BUILD_ROOT/%{_unitdir}/san >> install -D -m 0755 init.d/wdmd >> $RPM_BUILD_ROOT/usr/lib/systemd/systemd-wdmd >> install -D -m 0644 init.d/wdmd.service.native >> $RPM_BUILD_ROOT/%{_unitdir}/wdmd.service >> install -D -m 0644 init.d/sanlk-resetd.service >> $RPM_BUILD_ROOT/%{_unitdir}/sanlk-resetd.service >> >> install -D -m 0644 src/logrotate.sanlock \ >> - $RPM_BUILD_ROOT/etc/logrotate.d/sanlock >> + $RPM_BUILD_ROOT/etc/logrotate.d/sanlock >> >> install -D -m 0644 src/sanlock.conf \ >> - $RPM_BUILD_ROOT/etc/sanlock/sanlock.conf >> + $RPM_BUILD_ROOT/etc/sanlock/sanlock.conf >> >> install -D -m 0644 init.d/wdmd.sysconfig \ >> - $RPM_BUILD_ROOT/etc/sysconfig/wdmd >> + $RPM_BUILD_ROOT/etc/sysconfig/wdmd >> >> install -Dd -m 0755 $RPM_BUILD_ROOT/etc/wdmd.d >> install -Dd -m 0775 $RPM_BUILD_ROOT/%{_rundir}/sanlock >> install -Dd -m 0775 $RPM_BUILD_ROOT/%{_rundir}/sanlk-resetd >> >> %pre >> getent group sanlock > /dev/null || /usr/sbin/groupadd \ >> - -g 179 sanlock >> + -g 179 sanlock >> getent passwd sanlock > /dev/null || /usr/sbin/useradd \ >> - -u 179 -c "sanlock" -s /sbin/nologin -r \ >> - -g 179 -d /run/sanlock sanlock >> + -u 179 -c "sanlock" -s /sbin/nologin -r \ >> + -g 179 -d /run/sanlock sanlock >> /usr/sbin/usermod -a -G disk sanlock >> >> %post >> %systemd_post wdmd.service sanlock.service >> >> @@ -144,25 +137,21 @@ access to the shared disks. >> >> %package -n %{python_package} >> Summary: Python bindings for the sanlock library >> Requires: %{name}-lib = %{version}-%{release} >> %if 0%{?fedora} >> -%{?python_provide:%python_provide python2-%{name}} >> -# Remove before F30 >> -Provides: %{name}-python = %{version}-%{release} >> -Provides: %{name}-python%{?_isa} = %{version}-%{release} >> -Obsoletes: %{name}-python < %{version}-%{release} >> +%{?python_provide:%python_provide %{python_package}} >> %endif # fedora >> >> %description -n %{python_package} >> The %{python_package} package contains a module that permits applications >> written in the Python programming language to use the interface >> supplied by the sanlock library. >> >> %files -n %{python_package} >> -%{python2_sitearch}/sanlock_python-*.egg-info >> -%{python2_sitearch}/sanlock.so >> +%{python3_sitearch}/sanlock_python-*.egg-info >> +%{python3_sitearch}/sanlock*.so >> >> %package devel >> Summary: Development files for %{name} >> Requires: %{name}-lib = %{version}-%{release} >> >> @@ -201,8 +190,11 @@ common sanlock lockspace. >> %dir %attr(-,root,root) %{_rundir}/sanlk-resetd >> %{_mandir}/man8/sanlk-reset* >> >> >> %changelog >> +* Wed Jun 12 2019 Nir Soffer <nsoffer(a)redhat.com> - 3.8.0-1 >> +- Convert spec to python 3 >> + >> > > Not sure it's important to add to the changelog section: mixing tabs and > spaces fix > This is internal change that does not affect the behavior so I don't think there is point to mention it in the changelog. > > >> * Mon Mar 25 2019 Nir Soffer <nsoffer(a)redhat.com> - 3.7.0-1 >> - Import spec from Fedora master branch (371e11a) >> - Fix spec to work on CentOS (20efe91) >> -- >> 2.17.2 >> >>

1 0

[PATCH v2 0/2] tests for clearing resource
by Vojtech Juranek 14 Jun '19

14 Jun '19

Add tests for clearing the resource. Also extract reading sanlock magic number into helper function. Vojtech Juranek (2): tests: replace reading of magic number by helper function tests: add tests for clearing resource tests/constants.py | 4 +++ tests/python_test.py | 67 +++++++++++++++++++++++++++++++++----------- tests/util.py | 11 ++++++++ 3 files changed, 66 insertions(+), 16 deletions(-) -- 2.20.1

2 4

[PATCH 0/2] Fix python style issues
by Nir Soffer 14 Jun '19

14 Jun '19

Fix python style issues and add flake8 style check to the build to keep the code in good shape. Nir Soffer (2): build: Add flak8 env python: Fix style issues README.dev | 4 ++++ python/example.py | 28 +++++++++++++++++++-------- tests/conftest.py | 8 ++++---- tests/python_test.py | 46 ++++++++++++++++++++++++++++++-------------- tox.ini | 12 +++++++++++- 5 files changed, 71 insertions(+), 27 deletions(-) -- 2.17.2

1 2

[PATCH] tests: add tests for clearing resource
by Vojtech Juranek 14 Jun '19

14 Jun '19

Add tests for write_resource() with parameter 'clear' set to True, which shoudl result into clearing the resource (overwritting PAXOS_DISK_MAGIC with PAXOS_DISK_CLEAR). --- tests/constants.py | 4 +++ tests/python_test.py | 71 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+) diff --git a/tests/constants.py b/tests/constants.py index a7eb16b..21089b8 100644 --- a/tests/constants.py +++ b/tests/constants.py @@ -17,3 +17,7 @@ RINDEX_DISK_MAGIC = 0x01042018 RINDEX_ENTRY_SIZE = 64 RINDEX_ENTRIES_SECTORS = 2000 + +# src/sanlock_rv.h + +SANLK_LEADER_MAGIC = -223 diff --git a/tests/python_test.py b/tests/python_test.py index 3242f57..185c3e1 100644 --- a/tests/python_test.py +++ b/tests/python_test.py @@ -234,6 +234,77 @@ def test_write_resource_4k_invalid_sector_size(sanlock_daemon, user_4k_path): assert e.value.errno == errno.EINVAL +(a)pytest.mark.parametrize("filename,encoding", FILE_NAMES) +(a)pytest.mark.parametrize("size,offset", [ + # Smallest offset. + (MIN_RES_SIZE, 0), + # Large offset. + (LARGE_FILE_SIZE, LARGE_FILE_SIZE - MIN_RES_SIZE), +]) +def test_clear_resource(tmpdir, sanlock_daemon, filename, encoding, size, offset): + path = util.generate_path(tmpdir, filename, encoding) + util.create_file(path, size) + disks = [(path, offset)] + + # Test write, clear and read with default alignment and sector size values. + sanlock.write_resource(b"ls_name", b"res_name", disks) + sanlock.write_resource(b"ls_name", b"res_name", disks, clear=True) + + with pytest.raises(sanlock.SanlockException) as e: + sanlock.read_resource(path, offset=offset) + assert e.value.errno == constants.SANLK_LEADER_MAGIC + + # Test write, clear and read with explicit alignment and sector size + # values. + sanlock.write_resource( + b"ls_name", b"res_name", disks, align=ALIGNMENT_1M, + sector=SECTOR_SIZE_512, clear=True) + + with pytest.raises(sanlock.SanlockException) as e: + sanlock.read_resource( + path, offset=offset, align=ALIGNMENT_1M, sector=SECTOR_SIZE_512) + assert e.value.errno == constants.SANLK_LEADER_MAGIC + + with io.open(path, "rb") as f: + f.seek(offset) + magic, = struct.unpack("< I", f.read(4)) + assert magic == constants.PAXOS_DISK_CLEAR + + util.check_guard(path, size) + + +(a)pytest.mark.parametrize("align", sanlock.ALIGN_SIZE) +def test_clear_resource_4k(sanlock_daemon, user_4k_path, align): + disks = [(user_4k_path, 0)] + + # Poison resource area, ensuring that previous tests will not break this + # 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_resource( + b"ls_name", b"res_name", disks, align=align, sector=SECTOR_SIZE_4K) + sanlock.write_resource( + b"ls_name", b"res_name", + disks, + align=align, + sector=SECTOR_SIZE_4K, + clear=True) + + with pytest.raises(sanlock.SanlockException) as e: + sanlock.read_resource(user_4k_path, align=align, sector=SECTOR_SIZE_4K) + assert e.value.errno == constants.SANLK_LEADER_MAGIC + + # Verify that resource was cleared. + with io.open(user_4k_path, "rb") as f: + magic, = struct.unpack("< I", f.read(4)) + assert magic == constants.PAXOS_DISK_CLEAR + + # Check that sanlock did not write beyond the lockspace area. + util.check_guard(user_4k_path, align) + + def test_read_resource_4k_invalid_sector_size(sanlock_daemon, user_4k_path): disks = [(user_4k_path, 0)] -- 2.20.1

2 2

[sanlock] 02/02: release 3.8.0
by pagure＠pagure.io 12 Jun '19

12 Jun '19

This is an automated email from the git hooks/post-receive script. teigland pushed a commit to branch master in repository sanlock. commit 1b159cad7b11a570d6353df089ee48171a8dbc28 Author: David Teigland <teigland(a)redhat.com> AuthorDate: Wed Jun 12 14:07:38 2019 -0500 release 3.8.0 --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index c1e43e6..1981190 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -3.7.3 +3.8.0 -- To stop receiving notification emails like this one, please contact the administrator of this repository.

1 0

[sanlock] 01/02: spec: Convert development spec to python 3
by pagure＠pagure.io 12 Jun '19

12 Jun '19

This is an automated email from the git hooks/post-receive script. teigland pushed a commit to branch master in repository sanlock. commit 1fdae8ea67442df78d792f91ac029133ff7ded28 Author: Nir Soffer <nirsof(a)gmail.com> AuthorDate: Wed Jun 12 22:03:39 2019 +0300 spec: Convert development spec to python 3 The development spec creates now only python 3 packages. This will be the base for Fedora 30 and CentOS 8.1 spec. Also make rpmlint happy about mixing tabs and spaces by using only spaces. Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> --- sanlock.spec.in | 42 +++++++++++++++++------------------------- 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/sanlock.spec.in b/sanlock.spec.in index 9ad2985..087076c 100644 --- a/sanlock.spec.in +++ b/sanlock.spec.in @@ -18,8 +18,8 @@ BuildRequires: libaio-devel BuildRequires: libblkid-devel BuildRequires: libuuid-devel BuildRequires: make -BuildRequires: python2 -BuildRequires: python2-devel +BuildRequires: python3 +BuildRequires: python3-devel BuildRequires: systemd-units Requires: %{name}-lib = %{version}-%{release} Requires(pre): /usr/sbin/groupadd @@ -30,14 +30,7 @@ Requires(preun): systemd-units Requires(postun): systemd-units Source0: https://releases.pagure.org/sanlock/%{name}-%{version}.tar.gz -# On Fedora we switched sanlock-python to python2-sanlock, as part of python 3 -# porting. This change was not done yet for RHEL. -# TODO: remove when RHEL switches to pythonN-sanlock. -%if 0%{?fedora} -%global python_package python2-%{name} -%else # centos/rhel -%global python_package %{name}-python -%endif +%global python_package python3-%{name} %description The sanlock daemon manages leases for applications on hosts using shared storage. @@ -53,7 +46,7 @@ The sanlock daemon manages leases for applications on hosts using shared storage # upstream does not support _smp_mflags CFLAGS=$RPM_OPT_FLAGS make -C wdmd CFLAGS=$RPM_OPT_FLAGS make -C src -CFLAGS=$RPM_OPT_FLAGS make -C python +CFLAGS=$RPM_OPT_FLAGS make -C python PY_VERSION=3.6 CFLAGS=$RPM_OPT_FLAGS make -C reset %install @@ -67,7 +60,7 @@ make -C wdmd \ make -C python \ install LIBDIR=%{_libdir} \ DESTDIR=$RPM_BUILD_ROOT \ - PY_VERSION=2.7 # TODO: fix PY_VERSION to 3.6 when 2020 will arrive + PY_VERSION=3.6 make -C reset \ install LIBDIR=%{_libdir} \ DESTDIR=$RPM_BUILD_ROOT @@ -79,13 +72,13 @@ install -D -m 0644 init.d/wdmd.service.native $RPM_BUILD_ROOT/%{_unitdir}/wdmd.s install -D -m 0644 init.d/sanlk-resetd.service $RPM_BUILD_ROOT/%{_unitdir}/sanlk-resetd.service install -D -m 0644 src/logrotate.sanlock \ - $RPM_BUILD_ROOT/etc/logrotate.d/sanlock + $RPM_BUILD_ROOT/etc/logrotate.d/sanlock install -D -m 0644 src/sanlock.conf \ - $RPM_BUILD_ROOT/etc/sanlock/sanlock.conf + $RPM_BUILD_ROOT/etc/sanlock/sanlock.conf install -D -m 0644 init.d/wdmd.sysconfig \ - $RPM_BUILD_ROOT/etc/sysconfig/wdmd + $RPM_BUILD_ROOT/etc/sysconfig/wdmd install -Dd -m 0755 $RPM_BUILD_ROOT/etc/wdmd.d install -Dd -m 0775 $RPM_BUILD_ROOT/%{_rundir}/sanlock @@ -93,10 +86,10 @@ install -Dd -m 0775 $RPM_BUILD_ROOT/%{_rundir}/sanlk-resetd %pre getent group sanlock > /dev/null || /usr/sbin/groupadd \ - -g 179 sanlock + -g 179 sanlock getent passwd sanlock > /dev/null || /usr/sbin/useradd \ - -u 179 -c "sanlock" -s /sbin/nologin -r \ - -g 179 -d /run/sanlock sanlock + -u 179 -c "sanlock" -s /sbin/nologin -r \ + -g 179 -d /run/sanlock sanlock /usr/sbin/usermod -a -G disk sanlock %post @@ -146,11 +139,7 @@ access to the shared disks. Summary: Python bindings for the sanlock library Requires: %{name}-lib = %{version}-%{release} %if 0%{?fedora} -%{?python_provide:%python_provide python2-%{name}} -# Remove before F30 -Provides: %{name}-python = %{version}-%{release} -Provides: %{name}-python%{?_isa} = %{version}-%{release} -Obsoletes: %{name}-python < %{version}-%{release} +%{?python_provide:%python_provide %{python_package}} %endif # fedora %description -n %{python_package} @@ -159,8 +148,8 @@ written in the Python programming language to use the interface supplied by the sanlock library. %files -n %{python_package} -%{python2_sitearch}/sanlock_python-*.egg-info -%{python2_sitearch}/sanlock.so +%{python3_sitearch}/sanlock_python-*.egg-info +%{python3_sitearch}/sanlock*.so %package devel Summary: Development files for %{name} @@ -203,6 +192,9 @@ common sanlock lockspace. %changelog +* Wed Jun 12 2019 Nir Soffer <nsoffer(a)redhat.com> - 3.8.0-1 +- Convert spec to python 3 + * Mon Mar 25 2019 Nir Soffer <nsoffer(a)redhat.com> - 3.7.0-1 - Import spec from Fedora master branch (371e11a) - Fix spec to work on CentOS (20efe91) -- To stop receiving notification emails like this one, please contact the administrator of this repository.

1 0

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

sanlock-devel June 2019