sanlock-devel June 2019

sanlock-devel@lists.fedorahosted.org

5 participants
95 discussions

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 17 Jun '19

17 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

← Newer
1
2
3
4
5
6
7
8
9
10
Older →

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

sanlock-devel June 2019