[PATCH v2 00/23] Initial python 3 support

[Nir Soffer]

The first phase of python 3 porting is done:

All the tests pass now in python 2.7, 3.6, 3.7 and 3.8.

I plan to push this once we cut python 3.7.3 for EL 7.7.

More work is needed:
- accept only bytes for resource and lockspaces names on python 3
- support both unicode and bytes for paths on python 3, same as posix module
- add missing tests (request, killpath, xxx_event)
- spec for python 3

The patches are also available in this branch:
https://github.com/nirs/sanlock/commits/py3
commit: 3c03d8672d8589219def0133b188a1ac31d41615

Amit Bawer (18):
  readme: Add prerequisites section
  tox: Add python version as make argument
  tests: Use python3 and python2 compatible socket exception type
  tests: Lockspace and resource names should be read as bytes
  tests: Enforce absolute import behavior for python2 tests
  sanlock: Resource API should only support disks as a tuple
  tests: Test parsing a resource which is not a list of tuples
  sanlock: Provide internal common module init function
  tests: Add util.generate_path for generating file paths by encoding
  tests: Add parameterized tests for file paths encodings
  sanlock: Add module init for Python 3
  python: Add pystring_as_cstring function
  python: Provide helper for Py{Int,Long}_AsUnsignedLongLongMask
  python: Refactor build of dictionary entries for hosts_to_list
  python: Refactor build of dictionary entries for py_get_lockspaces
  python: Refactor build of dictionary entries for py_read_lockspace
  python: Refactor build of dictionary entries for py_read_resource
  python: Use Py_BuildValue("i", ...) to create py integers

Nir Soffer (3):
  tests: Update lockspace and resource tests
  python: Simplify parse_resource() with PyArg_ParseTuple
  travis: Add python 3 builds

Pavel Bar (2):
  Fix typos in debug logs and comments.
  Adding constants for Kibibyte, Mebibyte, Gibibyte, Tebibyte & Pebibyte
    and using them everywhere in tests.

 .travis.yml          |   8 +-
 README.dev           |  12 ++
 python/Makefile      |   6 +-
 python/sanlock.c     | 409 +++++++++++++++++++++----------------------
 sanlock.spec.in      |   3 +-
 src/direct.c         |   2 +-
 src/resource.c       |   2 +-
 tests/conftest.py    |   1 +
 tests/daemon_test.py |  44 ++---
 tests/direct_test.py |   6 +-
 tests/python_test.py | 111 ++++++++----
 tests/units.py       |   9 +
 tests/util.py        |  21 ++-
 tox.ini              |   3 +-
 14 files changed, 361 insertions(+), 276 deletions(-)
 create mode 100644 tests/units.py

-- 
2.17.2

[PATCH v2 01/23] readme: Add prerequisites section

[Nir Soffer]

From: Amit Bawer <abawer.com(a)gmail.com&gt;

Signed-off-by: Amit Bawer <abawer(a)redhat.com&gt;
---
 README.dev | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/README.dev b/README.dev
index bfb170c..25e6dbf 100644
--- a/README.dev
+++ b/README.dev
@@ -1,5 +1,17 @@
+Prerequisites
+=============
+
+For Fedora/CentOS install the following packages:
+
+   $ sudo yum install -y gcc make libaio-devel libblkid-devel
+
+For Ubuntu/Debian install the following packages:
+
+   $ sudo apt install -y gcc make libaio-dev libblkid-dev
+
+
 How to test sanlock
 ===================
 
 To run the python based tests, you need tox. The best way to install a
 recent version is to use pip:
-- 
2.17.2

[PATCH v2 02/23] tox: Add python version as make argument

[Nir Soffer]

From: Amit Bawer <abawer.com(a)gmail.com&gt;

tox -e py27 will invoke
make PY_VERSION=2.7

tox -e py36 will invoke
make PY_VERSION=3.6

At this point rpm spec invokes
make PY_VERSION=2.7

co-authored with Pavel Bar <pbar(a)redhat.com&gt;

Signed-off-by: Amit Bawer <abawer(a)redhat.com&gt;
---
 python/Makefile | 6 ++++--
 sanlock.spec.in | 3 ++-
 tox.ini         | 3 ++-
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/python/Makefile b/python/Makefile
index c97e45a..2caf6a8 100644
--- a/python/Makefile
+++ b/python/Makefile
@@ -2,14 +2,16 @@
 #
 # This copyrighted material is made available to anyone wishing to use,
 # modify, copy, or redistribute it subject to the terms and conditions
 # of the GNU General Public License v.2.
 
+PYTHON := python$(PY_VERSION)
+
 all:
-	python2 setup.py build $(BUILDARGS)
+	$(PYTHON) setup.py build $(BUILDARGS)
 
 install:
-	python2 setup.py install --root=$(DESTDIR)
+	$(PYTHON) setup.py install --root=$(DESTDIR)
 
 clean:
 	rm -rf build
 
diff --git a/sanlock.spec.in b/sanlock.spec.in
index c31599d..6485c61 100644
--- a/sanlock.spec.in
+++ b/sanlock.spec.in
@@ -59,11 +59,12 @@ make -C src \
 make -C wdmd \
         install LIBDIR=%{_libdir} \
         DESTDIR=$RPM_BUILD_ROOT
 make -C python \
         install LIBDIR=%{_libdir} \
-        DESTDIR=$RPM_BUILD_ROOT
+        DESTDIR=$RPM_BUILD_ROOT \
+        PY_VERSION=2.7 # TODO: fix PY_VERSION to 3.6 when 2020 will arrive
 make -C reset \
         install LIBDIR=%{_libdir} \
         DESTDIR=$RPM_BUILD_ROOT
 
 
diff --git a/tox.ini b/tox.ini
index d21f2f3..5a48b23 100644
--- a/tox.ini
+++ b/tox.ini
@@ -16,11 +16,12 @@ setenv =
     SANLOCK_RUN_DIR=/tmp/sanlock
 whitelist_externals = make
 deps =
     pytest==4.0
 commands =
-    make BUILDARGS="--build-lib={envsitepackagesdir}"
+    py27: make PY_VERSION=2.7 BUILDARGS="--build-lib={envsitepackagesdir}"
+    py36: make PY_VERSION=3.6 BUILDARGS="--build-lib={envsitepackagesdir}"
     pytest {posargs}
 
 [pytest]
 # Notes:
 # --basetemp: we must use /var/tmp as sanlock uses direct I/O.
-- 
2.17.2

[PATCH v2 03/23] tests: Use python3 and python2 compatible socket exception type

[Nir Soffer]

From: Amit Bawer <abawer.com(a)gmail.com&gt;

Replace socket.error with EnvironmentError exception class

co-authored with Nir Soffer <nsoffer(a)redhat.com&gt;

Signed-off-by: Amit Bawer <abawer(a)redhat.com&gt;
---
 tests/util.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/util.py b/tests/util.py
index 0695d8c..8bb4f9f 100644
--- a/tests/util.py
+++ b/tests/util.py
@@ -61,12 +61,12 @@ def wait_for_daemon(timeout):
     try:
         while True:
             try:
                 s.connect(path)
                 return
-            except socket.error as e:
-                if e[0] not in (errno.ECONNREFUSED, errno.ENOENT):
+            except EnvironmentError as e:
+                if e.errno not in (errno.ECONNREFUSED, errno.ENOENT):
                     raise  # Unexpected error
             if time.time() > deadline:
                 raise TimeoutExpired
             time.sleep(0.05)
     finally:
-- 
2.17.2

[PATCH v2 04/23] tests: Lockspace and resource names should be read as bytes

[Nir Soffer]

From: Amit Bawer <abawer.com(a)gmail.com&gt;

This patch is preparation for python 3 porting. No regression for tests
under python 2 which is agnostic to strings/bytes.

Signed-off-by: Amit Bawer <abawer(a)redhat.com&gt;

Signed-off-by: Amit Bawer <abawer.com(a)gmail.com&gt;
---
 tests/daemon_test.py | 10 +++++-----
 tests/python_test.py | 18 +++++++++---------
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/tests/daemon_test.py b/tests/daemon_test.py
index 8896fb2..fe02ef6 100644
--- a/tests/daemon_test.py
+++ b/tests/daemon_test.py
@@ -188,11 +188,11 @@ def test_lookup(tmpdir, sanlock_daemon):
 
     util.sanlock("client", "add_lockspace", "-s",
lockspace, "-o", "1")
     util.sanlock("client", "create", "-x", rindex,
"-e", "res")
     lookup = util.sanlock("client", "lookup", "-x", rindex,
"-e", "res")
 
-    assert lookup == "lookup done 0\nname res offset 3145728\n"
+    assert lookup == b"lookup done 0\nname res offset 3145728\n"
 
 
 def test_lookup_uninitialized(tmpdir, sanlock_daemon):
     path = tmpdir.join("rindex")
     util.create_file(str(path), 1024**2)
@@ -200,12 +200,12 @@ def test_lookup_uninitialized(tmpdir, sanlock_daemon):
 
     with pytest.raises(util.CommandError) as e:
         util.sanlock("client", "lookup", "-x", rindex,
"-e", "res")
 
     assert e.value.returncode == 1
-    assert e.value.stdout == "lookup done -2\n"
-    assert e.value.stderr == ""
+    assert e.value.stdout == b"lookup done -2\n"
+    assert e.value.stderr == b""
 
 
 def test_lookup_missing(tmpdir, sanlock_daemon):
     path = tmpdir.join("rindex")
     # Slots: lockspace rindex master-lease user-lease-1 ... user-lease-7
@@ -222,7 +222,7 @@ def test_lookup_missing(tmpdir, sanlock_daemon):
     util.sanlock("client", "add_lockspace", "-s",
lockspace, "-o", "1")
     with pytest.raises(util.CommandError) as e:
         util.sanlock("client", "lookup", "-x", rindex,
"-e", "res")
 
     assert e.value.returncode == 1
-    assert e.value.stdout == "lookup done -2\n"
-    assert e.value.stderr == ""
+    assert e.value.stdout == b"lookup done -2\n"
+    assert e.value.stderr == b""
diff --git a/tests/python_test.py b/tests/python_test.py
index 12358c7..7e3910b 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -41,11 +41,11 @@ def test_write_lockspace(tmpdir, sanlock_daemon, size, offset):
 
     # Test read and write with default alignment and sector size values.
     sanlock.write_lockspace("name", path, offset=offset, iotimeout=1)
 
     ls = sanlock.read_lockspace(path, offset=offset)
-    assert ls == {"iotimeout": 1, "lockspace": "name"}
+    assert ls == {"iotimeout": 1, "lockspace": b"name"}
 
     # Test read and write with explicit alignment and sector size values.
     sanlock.write_lockspace(
         "name", path, offset=offset, iotimeout=1, align=ALIGNMENT_1M,
         sector=SECTOR_SIZE_512)
@@ -127,12 +127,12 @@ def test_write_resource(tmpdir, sanlock_daemon, size, offset):
     # Test read and write with default alignment and sector size values.
     sanlock.write_resource("ls_name", "res_name", disks)
 
     res = sanlock.read_resource(path, offset=offset)
     assert res == {
-        "lockspace": "ls_name",
-        "resource": "res_name",
+        "lockspace": b"ls_name",
+        "resource": b"res_name",
         "version": 0
     }
 
     # Test read and write with explicit alignment and sector size values.
     sanlock.write_resource(
@@ -355,12 +355,12 @@ def test_acquire_release_resource(tmpdir, sanlock_daemon, size,
offset):
     disks = [(res_path, offset)]
     sanlock.write_resource("ls_name", "res_name", disks)
 
     res = sanlock.read_resource(res_path, offset=offset)
     assert res == {
-        "lockspace": "ls_name",
-        "resource": "res_name",
+        "lockspace": b"ls_name",
+        "resource": b"res_name",
         "version": 0
     }
 
     owners = sanlock.read_resource_owners("ls_name", "res_name",
disks)
     assert owners == []
@@ -368,12 +368,12 @@ def test_acquire_release_resource(tmpdir, sanlock_daemon, size,
offset):
     fd = sanlock.register()
     sanlock.acquire("ls_name", "res_name", disks, slkfd=fd)
 
     res = sanlock.read_resource(res_path, offset=offset)
     assert res == {
-        "lockspace": "ls_name",
-        "resource": "res_name",
+        "lockspace": b"ls_name",
+        "resource": b"res_name",
         "version": 1
     }
 
     owner = sanlock.read_resource_owners("ls_name", "res_name",
disks)[0]
 
@@ -389,12 +389,12 @@ def test_acquire_release_resource(tmpdir, sanlock_daemon, size,
offset):
 
     sanlock.release("ls_name", "res_name", disks, slkfd=fd)
 
     res = sanlock.read_resource(res_path, offset=offset)
     assert res == {
-        "lockspace": "ls_name",
-        "resource": "res_name",
+        "lockspace": b"ls_name",
+        "resource": b"res_name",
         "version": 1
     }
 
     owners = sanlock.read_resource_owners("ls_name", "res_name",
disks)
     assert owners == []
-- 
2.17.2

[PATCH v2 05/23] tests: Update lockspace and resource tests

[Nir Soffer]

We introduced new assertions about lockspace and resource names in the
4k branch. Update the assertions to check for b"name" instead of
"name".

Signed-off-by: Nir Soffer <nsoffer(a)redhat.com&gt;
---
 tests/python_test.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tests/python_test.py b/tests/python_test.py
index 7e3910b..b2f903b 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -50,11 +50,11 @@ def test_write_lockspace(tmpdir, sanlock_daemon, size, offset):
         "name", path, offset=offset, iotimeout=1, align=ALIGNMENT_1M,
         sector=SECTOR_SIZE_512)
 
     ls = sanlock.read_lockspace(
         path, offset=offset, align=ALIGNMENT_1M, sector=SECTOR_SIZE_512)
-    assert ls == {"iotimeout": 1, "lockspace": "name"}
+    assert ls == {"iotimeout": 1, "lockspace": b"name"}
 
     acquired = sanlock.inq_lockspace(
         "name", 1, path, offset=offset, wait=False)
     assert acquired is False
 
@@ -81,11 +81,11 @@ def test_write_lockspace_4k(user_4k_path, sanlock_daemon, align):
         "name", user_4k_path, iotimeout=1, align=align, sector=SECTOR_SIZE_4K)
 
     ls = sanlock.read_lockspace(
         user_4k_path, align=align, sector=SECTOR_SIZE_4K)
 
-    assert ls == {"iotimeout": 1, "lockspace": "name"}
+    assert ls == {"iotimeout": 1, "lockspace": b"name"}
 
     acquired = sanlock.inq_lockspace("name", 1, user_4k_path, wait=False)
     assert acquired is False
 
     # Verify that lockspace was written.
@@ -140,12 +140,12 @@ def test_write_resource(tmpdir, sanlock_daemon, size, offset):
         sector=SECTOR_SIZE_512)
 
     res = sanlock.read_resource(
         path, offset=offset, align=ALIGNMENT_1M, sector=SECTOR_SIZE_512)
     assert res == {
-        "lockspace": "ls_name",
-        "resource": "res_name",
+        "lockspace": b"ls_name",
+        "resource": b"res_name",
         "version": 0
     }
 
     owners = sanlock.read_resource_owners("ls_name", "res_name",
disks)
     assert owners == []
@@ -175,12 +175,12 @@ def test_write_resource_4k(sanlock_daemon, user_4k_path, align):
 
     res = sanlock.read_resource(
         user_4k_path, align=align, sector=SECTOR_SIZE_4K)
 
     assert res == {
-        "lockspace": "ls_name",
-        "resource": "res_name",
+        "lockspace": b"ls_name",
+        "resource": b"res_name",
         "version": 0
     }
 
     owners = sanlock.read_resource_owners(
         "ls_name", "res_name", disks, align=align,
sector=SECTOR_SIZE_4K)
-- 
2.17.2

[PATCH v2 06/23] tests: Enforce absolute import behavior for python2 tests

[Nir Soffer]

From: Amit Bawer <abawer.com(a)gmail.com&gt;

added from __future__ import absolute_import

at top of tests modules.

co-authored with Nir Soffer <nsoffer(a)redhat.com&gt;

Signed-off-by: Amit Bawer <abawer(a)redhat.com&gt;
---
 tests/conftest.py    | 1 +
 tests/daemon_test.py | 1 +
 tests/direct_test.py | 1 +
 tests/python_test.py | 1 +
 tests/util.py        | 1 +
 5 files changed, 5 insertions(+)

diff --git a/tests/conftest.py b/tests/conftest.py
index bb95d45..537750c 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -1,8 +1,9 @@
 """
 Fixtures for sanlock testing.
 """
+from __future__ import absolute_import
 
 import os
 import stat
 
 import pytest
diff --git a/tests/daemon_test.py b/tests/daemon_test.py
index fe02ef6..c7f9042 100644
--- a/tests/daemon_test.py
+++ b/tests/daemon_test.py
@@ -1,8 +1,9 @@
 """
 Test sanlock client operations.
 """
+from __future__ import absolute_import
 
 import io
 import signal
 import struct
 
diff --git a/tests/direct_test.py b/tests/direct_test.py
index d32ef52..4ef63bc 100644
--- a/tests/direct_test.py
+++ b/tests/direct_test.py
@@ -1,8 +1,9 @@
 """
 Test sanlock direct options.
 """
+from __future__ import absolute_import
 
 import io
 import struct
 
 from . import constants
diff --git a/tests/python_test.py b/tests/python_test.py
index b2f903b..d7ec0fb 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -1,8 +1,9 @@
 """
 Test sanlock python binding with sanlock daemon.
 """
+from __future__ import absolute_import
 
 import errno
 import io
 import struct
 import time
diff --git a/tests/util.py b/tests/util.py
index 8bb4f9f..e31190e 100644
--- a/tests/util.py
+++ b/tests/util.py
@@ -1,8 +1,9 @@
 """
 Testing utilities
 """
+from __future__ import absolute_import
 
 import errno
 import io
 import os
 import socket
-- 
2.17.2

[PATCH v2 07/23] sanlock: Resource API should only support disks as a tuple

[Nir Soffer]

From: Amit Bawer <abawer(a)redhat.com&gt;

As stated in sanlock module help:
The disks must be in the format: [(path, offset), ... ]

Signed-off-by: Amit Bawer <abawer(a)redhat.com&gt;
---
 python/sanlock.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/python/sanlock.c b/python/sanlock.c
index e710f89..446a4b5 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -31,10 +31,11 @@
 #endif
 
 /* Functions prototypes */
 static void __set_exception(int en, char *msg) __sets_exception;
 static int __parse_resource(PyObject *obj, struct sanlk_resource **res_ret)
__neg_sets_exception;
+static void set_value_error(const char* format, PyObject* obj);
 
 /* Sanlock module */
 PyDoc_STRVAR(pydoc_sanlock, "\
 Copyright (C) 2010-2011 Red Hat, Inc.  All rights reserved.\n\
 This copyrighted material is made available to anyone wishing to use,\n\
@@ -107,12 +108,14 @@ __parse_resource(PyObject *obj, struct sanlk_resource **res_ret)
 
             if (!PyInt_Check(offset)) {
                 __set_exception(EINVAL, "Invalid resource offset");
                 goto exit_fail;
             }
-        } else if (PyString_Check(tuple)) {
-            p = PyString_AsString(tuple);
+        } else {
+            /* handle invalid non-tuple resource input */
+            set_value_error("invalid disk value: %s", tuple);
+            goto exit_fail;
         }
 
         if (p == NULL) {
             __set_exception(EINVAL, "Invalid resource path");
             goto exit_fail;
@@ -182,10 +185,21 @@ add_align_flag(long align, uint32_t *flags)
 	return -1;
     }
     return 0;
 }
 
+static void
+set_value_error(const char* format, PyObject* obj)
+{
+    const char* str_rep = "";
+    PyObject* rep = PyObject_Repr(obj);
+    if (rep)
+        str_rep = PyString_AsString(rep);
+    PyErr_Format(PyExc_ValueError, format, str_rep);
+    Py_XDECREF(rep);
+}
+
 static PyObject *
 __hosts_to_list(struct sanlk_host *hss, int hss_count)
 {
     int i, rv;
     PyObject *ls_list = NULL, *ls_entry = NULL, *ls_value = NULL;
-- 
2.17.2

[PATCH v2 08/23] tests: Test parsing a resource which is not a list of tuples

[Nir Soffer]

From: Amit Bawer <abawer(a)redhat.com&gt;

Expecting to get ValueError for resource argument which
is not a list of tuples (disk, offset).

Signed-off-by: Amit Bawer <abawer(a)redhat.com&gt;
---
 tests/python_test.py | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/tests/python_test.py b/tests/python_test.py
index d7ec0fb..28827d6 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -429,5 +429,20 @@ def test_write_resource_invalid_align_sector(
     disks = [(path, 0)]
 
     with pytest.raises(ValueError):
         sanlock.write_resource(
             "ls_name", "res_name", disks, align=align,
sector=sector)
+
+
+(a)pytest.mark.parametrize(&quot;resource&quot;, [
+    "invalid resource tuple",
+    b"invalid resource tuple",
+    u'\u05e9\u05dc\u05d5\u05dd',
+    b"\xd7\x90"
+])
+def test_write_resource_invalid_path(tmpdir, sanlock_daemon, resource):
+    # Test parsing a resource which is not a list of tuples
+    disks = [resource]
+    with pytest.raises(ValueError) as e:
+        sanlock.write_resource("ls_name", "res_name", disks)
+    assert repr(resource) in str(e.value)
+
-- 
2.17.2

[PATCH v2 09/23] sanlock: Provide internal common module init function

[Nir Soffer]

From: Amit Bawer <abawer(a)redhat.com&gt;

This will serve python 3 porting in future patches to come.

Signed-off-by: Amit Bawer <abawer(a)redhat.com&gt;
---
 python/sanlock.c | 103 +++++++++++++++++++++++++++--------------------
 1 file changed, 60 insertions(+), 43 deletions(-)

diff --git a/python/sanlock.c b/python/sanlock.c
index 446a4b5..420b4ba 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -28,10 +28,12 @@
     __attribute__((cpychecker_negative_result_sets_exception))
 #else
 #define __neg_sets_exception
 #endif
 
+#define MODULE_NAME "sanlock"
+
 /* Functions prototypes */
 static void __set_exception(int en, char *msg) __sets_exception;
 static int __parse_resource(PyObject *obj, struct sanlk_resource **res_ret)
__neg_sets_exception;
 static void set_value_error(const char* format, PyObject* obj);
 
@@ -1700,79 +1702,94 @@ exit_fail:
     Py_XDECREF(dict);
 
     return excp;
 }
 
-PyMODINIT_FUNC
-initsanlock(void)
+static int
+module_init(PyObject* m)
 {
-    PyObject *py_module, *sk_constant;
-
-    py_module = Py_InitModule4("sanlock",
-                sanlock_methods, pydoc_sanlock, NULL, PYTHON_API_VERSION);
-
-    if (py_module == NULL)
-        return;
-
     if (py_exception == NULL) {
         py_exception = initexception();
         if (py_exception == NULL)
-            return;
+            return -1;
     }
 
     Py_INCREF(py_exception);
-    if (PyModule_AddObject(py_module, "SanlockException", py_exception)) {
+    if (PyModule_AddObject(m, "SanlockException", py_exception)) {
         Py_DECREF(py_exception);
-        return;
-    }
-
-#define PYSNLK_INIT_ADD_CONSTANT(x, y) \
-    if ((sk_constant = PyInt_FromLong(x)) != NULL) { \
-        if (PyModule_AddObject(py_module, y, sk_constant)) { \
-            Py_DECREF(sk_constant); \
-        } \
+        return -1;
     }
 
     /* lockspaces list flags */
-    PYSNLK_INIT_ADD_CONSTANT(SANLK_LSF_ADD, "LSFLAG_ADD");
-    PYSNLK_INIT_ADD_CONSTANT(SANLK_LSF_REM, "LSFLAG_REM");
+    if (PyModule_AddIntConstant(m, "LSFLAG_ADD", SANLK_LSF_ADD))
+        return -1;
+    if (PyModule_AddIntConstant(m, "LSFLAG_REM", SANLK_LSF_REM))
+        return -1;
 
     /* resource request flags */
-    PYSNLK_INIT_ADD_CONSTANT(SANLK_REQ_FORCE, "REQ_FORCE");
-    PYSNLK_INIT_ADD_CONSTANT(SANLK_REQ_GRACEFUL, "REQ_GRACEFUL");
+    if (PyModule_AddIntConstant(m, "REQ_FORCE", SANLK_REQ_FORCE))
+        return -1;
+    if (PyModule_AddIntConstant(m, "REQ_GRACEFUL", SANLK_REQ_GRACEFUL))
+        return -1;
 
     /* hosts list flags */
-    PYSNLK_INIT_ADD_CONSTANT(SANLK_HOST_FREE, "HOST_FREE");
-    PYSNLK_INIT_ADD_CONSTANT(SANLK_HOST_LIVE, "HOST_LIVE");
-    PYSNLK_INIT_ADD_CONSTANT(SANLK_HOST_FAIL, "HOST_FAIL");
-    PYSNLK_INIT_ADD_CONSTANT(SANLK_HOST_DEAD, "HOST_DEAD");
-    PYSNLK_INIT_ADD_CONSTANT(SANLK_HOST_UNKNOWN, "HOST_UNKNOWN");
+    if (PyModule_AddIntConstant(m, "HOST_FREE", SANLK_HOST_FREE))
+        return -1;
+    if (PyModule_AddIntConstant(m, "HOST_LIVE", SANLK_HOST_LIVE))
+        return -1;
+    if (PyModule_AddIntConstant(m, "HOST_FAIL", SANLK_HOST_FAIL))
+        return -1;
+    if (PyModule_AddIntConstant(m, "HOST_DEAD", SANLK_HOST_DEAD))
+        return -1;
+    if (PyModule_AddIntConstant(m, "HOST_UNKNOWN", SANLK_HOST_UNKNOWN))
+        return -1;
 
     /* set event flags */
-    PYSNLK_INIT_ADD_CONSTANT(SANLK_SETEV_CUR_GENERATION,
"SETEV_CUR_GENERATION");
-    PYSNLK_INIT_ADD_CONSTANT(SANLK_SETEV_CLEAR_HOSTID,  
"SETEV_CLEAR_HOSTID");
-    PYSNLK_INIT_ADD_CONSTANT(SANLK_SETEV_CLEAR_EVENT,    "SETEV_CLEAR_EVENT");
-    PYSNLK_INIT_ADD_CONSTANT(SANLK_SETEV_REPLACE_EVENT, 
"SETEV_REPLACE_EVENT");
-    PYSNLK_INIT_ADD_CONSTANT(SANLK_SETEV_ALL_HOSTS,      "SETEV_ALL_HOSTS");
-
-#undef PYSNLK_INIT_ADD_CONSTANT
+    if (PyModule_AddIntConstant(m, "SETEV_CUR_GENERATION",
SANLK_SETEV_CUR_GENERATION))
+        return -1;
+    if (PyModule_AddIntConstant(m, "SETEV_CLEAR_HOSTID",
SANLK_SETEV_CLEAR_HOSTID))
+        return -1;
+    if (PyModule_AddIntConstant(m, "SETEV_CLEAR_EVENT",
SANLK_SETEV_CLEAR_EVENT))
+        return -1;
+    if (PyModule_AddIntConstant(m, "SETEV_REPLACE_EVENT",
SANLK_SETEV_REPLACE_EVENT))
+        return -1;
+    if (PyModule_AddIntConstant(m, "SETEV_ALL_HOSTS", SANLK_SETEV_ALL_HOSTS))
+        return -1;
 
     /* Tuples with supported sector size and alignment values */
+
     PyObject *sector = Py_BuildValue("ii", SECTOR_SIZE_512, SECTOR_SIZE_4K);
     if (!sector)
-        return;
-    if (PyModule_AddObject(py_module, "SECTOR_SIZE", sector)) {
+        return -1;
+    if (PyModule_AddObject(m, "SECTOR_SIZE", sector)) {
         Py_DECREF(sector);
-        return;
+        return -1;
     }
 
-    PyObject *align = Py_BuildValue("llll", ALIGNMENT_1M, ALIGNMENT_2M,
ALIGNMENT_4M, ALIGNMENT_8M);
+    PyObject *align = Py_BuildValue(
+        "llll", ALIGNMENT_1M, ALIGNMENT_2M, ALIGNMENT_4M, ALIGNMENT_8M);
     if (!align)
-      return;
-    if (PyModule_AddObject(py_module, "ALIGN_SIZE", align)) {
+        return -1;
+    if (PyModule_AddObject(m, "ALIGN_SIZE", align)) {
         Py_DECREF(align);
-	return;
+        return -1;
     }
 
+    return 0;
+}
+
+PyMODINIT_FUNC
+initsanlock(void)
+{
+    PyObject *m = Py_InitModule3(
+        MODULE_NAME,
+        sanlock_methods,
+        pydoc_sanlock);
+
+    if (m == NULL)
+        return;
+
+    /* We don't have anything to do if module_init() fails. */
+    module_init(m);
 }
 
 /* vim: set expandtab shiftwidth=4 tabstop=4 : */
-- 
2.17.2

[PATCH v2 09/23] sanlock: Provide internal common module init function

[Vojtech Juranek]

On neděle 19. května 2019 20:49:34 CEST Nir Soffer wrote:
>  -PyMODINIT_FUNC
>  -initsanlock(void)
>  +static int
>  +module_init(PyObject* m) 
IMHO `module` instead of `m` would be better name. In general, I'd avoid one 
letter names (but probably fine when used as local variables in short 
functions like in `initsanlock()` where it's easy to find all occurrences by 
reading few lines of code)

[PATCH v2 09/23] sanlock: Provide internal common module init function

[Nir Soffer]

On Mon, May 20, 2019 at 2:42 PM Vojtech Juranek <vjuranek(a)redhat.com&gt; wrote:

>  On neděle 19. května 2019 20:49:34 CEST Nir Soffer wrote:
>  > -PyMODINIT_FUNC
>  > -initsanlock(void)
>  > +static int
>  > +module_init(PyObject* m)
>
>  IMHO `module` instead of `m` would be better name. In general, I'd avoid
>  one
>  letter names (but probably fine when used as local variables in short
>  functions like in `initsanlock()` where it's easy to find all occurrences
>  by
>  reading few lines of code)
>  
I agree with this approach, and it matches the way we use the module
in module_init() and in the init functions.

Nir

[PATCH v2 10/23] Fix typos in debug logs and comments.

[Nir Soffer]

From: Pavel Bar <pbar(a)redhat.com&gt;

Signed-off-by: Pavel Bar <pbar(a)redhat.com&gt;
---
 src/direct.c   | 2 +-
 src/resource.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/direct.c b/src/direct.c
index 344fe3d..6b9b78a 100644
--- a/src/direct.c
+++ b/src/direct.c
@@ -337,11 +337,11 @@ static int do_delta_action(int action,
 	if (!align_size && com.align_size)
 		align_size = com.align_size;
 
 	switch (action) {
 	case ACT_DIRECT_INIT:
-		/* delta_lease_init looks at ls->flags for sector/align sizses */
+		/* delta_lease_init looks at ls->flags for sector/align sizes */
 
 		rv = delta_lease_init(task, ls, io_timeout, &sd);
 		break;
 
 	case ACT_ACQUIRE_ID:
diff --git a/src/resource.c b/src/resource.c
index 76d506c..0235b53 100644
--- a/src/resource.c
+++ b/src/resource.c
@@ -226,11 +226,11 @@ int read_resource_owners(struct task *task, struct token *token,
 	 * If the caller did not specify sector size and our guess was wrong,
 	 * retry with the actual value
 	 */
 	if ((token->sector_size != leader.sector_size) ||
 	    (token->align_size != align_size)) {
-		log_debug("read_resource_owners rereading with correct sizses");
+		log_debug("read_resource_owners rereading with correct sizes");
 		token->sector_size = leader.sector_size;
 		token->align_size  = align_size;
 		free(lease_buf);
 		lease_buf = NULL;
 		goto retry;
-- 
2.17.2

[PATCH v2 11/23] Adding constants for Kibibyte, Mebibyte, Gibibyte, Tebibyte & Pebibyte and using them everywhere in tests.

[Nir Soffer]

From: Pavel Bar <pbar(a)redhat.com&gt;

Signed-off-by: Pavel Bar <pbar(a)redhat.com&gt;
---
 tests/daemon_test.py | 33 +++++++++++++++++----------------
 tests/direct_test.py |  5 +++--
 tests/python_test.py | 26 ++++++++++++++------------
 tests/units.py       |  9 +++++++++
 tests/util.py        |  4 +++-
 5 files changed, 46 insertions(+), 31 deletions(-)
 create mode 100644 tests/units.py

diff --git a/tests/daemon_test.py b/tests/daemon_test.py
index c7f9042..36648e3 100644
--- a/tests/daemon_test.py
+++ b/tests/daemon_test.py
@@ -9,10 +9,11 @@ import struct
 
 import pytest
 
 from . constants import *
 from . import util
+from . units import *
 
 
 def test_single_instance(sanlock_daemon):
     # Starting another instance while the daemon must fail.
     p = util.start_daemon()
@@ -43,11 +44,11 @@ def test_client_failure():
     assert e.value.returncode == 1
 
 
 def test_init_lockspace(tmpdir, sanlock_daemon):
     path = tmpdir.join("lockspace")
-    size = 1024**2
+    size = MiB
     util.create_file(str(path), size)
 
     lockspace = "name:1:%s:0" % path
     util.sanlock("client", "init", "-s", lockspace)
 
@@ -60,11 +61,11 @@ def test_init_lockspace(tmpdir, sanlock_daemon):
     util.check_guard(str(path), size)
 
 
 def test_init_resource(tmpdir, sanlock_daemon):
     path = tmpdir.join("resources")
-    size = 1024**2
+    size = MiB
     util.create_file(str(path), size)
 
     resource = "ls_name:res_name:%s:0" % path
     util.sanlock("client", "init", "-r", resource)
 
@@ -77,40 +78,40 @@ def test_init_resource(tmpdir, sanlock_daemon):
     util.check_guard(str(path), size)
 
 
 def test_format(tmpdir, sanlock_daemon):
     path = tmpdir.join("rindex")
-    size = 1024**2 * 3
+    size = 3 * MiB
     util.create_file(str(path), size)
 
     rindex = "ls_name:%s:1M" % path
     util.sanlock("client", "format", "-x", rindex)
 
     with io.open(str(path), "rb") as f:
         # The first slot should contain the rindex header sector.
-        f.seek(1024**2)
+        f.seek(MiB)
         magic, = struct.unpack("< I", f.read(4))
         assert magic == RINDEX_DISK_MAGIC
 
         # The rindex entries starts at the second rindex slot sector. All
         # entries should be zeroed.
-        f.seek(1024**2 + 512)
+        f.seek(MiB + 512)
         entries_size = 512 * RINDEX_ENTRIES_SECTORS
         assert f.read(entries_size) == b"\0" * entries_size
 
         # The next slot should contain the internal lease.
-        f.seek(1024**2 * 2)
+        f.seek(2 * MiB)
         magic, = struct.unpack("< I", f.read(4))
         assert magic == PAXOS_DISK_MAGIC
 
     util.check_guard(str(path), size)
 
 
 def test_create(tmpdir, sanlock_daemon):
     path = tmpdir.join("rindex")
     # Slots: lockspace rindex master-lease user-lease-1
-    size = 1024**2 * 4
+    size = 4 * MiB
     util.create_file(str(path), size)
 
     # Note: using 1 second io timeout (-o 1) for quicker tests.
     lockspace = "ls_name:1:%s:0" % path
     util.sanlock("client", "init", "-s", lockspace,
"-o", "1")
@@ -122,30 +123,30 @@ def test_create(tmpdir, sanlock_daemon):
     util.sanlock("client", "create", "-x", rindex,
"-e", "res")
 
     with io.open(str(path), "rb") as f:
         # New entry should be created at the first slot
         # The first rindex sector is used by the rindex header.
-        f.seek(1024**2 + 512)
+        f.seek(MiB + 512)
         util.check_rindex_entry(f.read(RINDEX_ENTRY_SIZE),
-                                b"res", 1024**2 * 3, 0)
+                                b"res", 3 * MiB, 0)
 
         # The rest of the entries should not be modified.
         rest = 512 * RINDEX_ENTRIES_SECTORS - RINDEX_ENTRY_SIZE
         assert f.read(rest) == b"\0" * rest
 
         # The next slot should contain the internal lease.
-        f.seek(1024**2 * 3)
+        f.seek(3 * MiB)
         magic, = struct.unpack("< I", f.read(4))
         assert magic == PAXOS_DISK_MAGIC
 
     util.check_guard(str(path), size)
 
 
 def test_delete(tmpdir, sanlock_daemon):
     path = tmpdir.join("rindex")
     # Slots: lockspace rindex master-lease user-lease-1
-    size = 1024**2 * 4
+    size = 4 * MiB
     util.create_file(str(path), size)
 
     # Note: using 1 second io timeout (-o 1) for quicker tests.
     lockspace = "ls_name:1:%s:0" % path
     util.sanlock("client", "init", "-s", lockspace,
"-o", "1")
@@ -157,29 +158,29 @@ def test_delete(tmpdir, sanlock_daemon):
     util.sanlock("client", "create", "-x", rindex,
"-e", "res")
     util.sanlock("client", "delete", "-x", rindex,
"-e", "res")
 
     with io.open(str(path), "rb") as f:
         # First entry should be cleared.
-        f.seek(1024**2 + 512)
+        f.seek(MiB + 512)
         util.check_rindex_entry(f.read(RINDEX_ENTRY_SIZE), b"", 0, 0)
 
         # Rest of entires should not be modified.
         rest = 512 * RINDEX_ENTRIES_SECTORS - RINDEX_ENTRY_SIZE
         assert f.read(rest) == b"\0" * rest
 
         # The next slot should contain a cleared lease.
-        f.seek(1024**2 * 3)
+        f.seek(3 * MiB)
         magic, = struct.unpack("< I", f.read(4))
         assert magic == PAXOS_DISK_CLEAR
 
     util.check_guard(str(path), size)
 
 
 def test_lookup(tmpdir, sanlock_daemon):
     path = tmpdir.join("rindex")
     # Slots: lockspace rindex master-lease user-lease-1 ... user-lease-7
-    size = 1024**2 * 10
+    size = 10 * MiB
     util.create_file(str(path), size)
 
     # Note: using 1 second io timeout (-o 1) for quicker tests.
     lockspace = "ls_name:1:%s:0" % path
     util.sanlock("client", "init", "-s", lockspace,
"-o", "1")
@@ -194,11 +195,11 @@ def test_lookup(tmpdir, sanlock_daemon):
     assert lookup == b"lookup done 0\nname res offset 3145728\n"
 
 
 def test_lookup_uninitialized(tmpdir, sanlock_daemon):
     path = tmpdir.join("rindex")
-    util.create_file(str(path), 1024**2)
+    util.create_file(str(path), MiB)
     rindex = "ls_name:%s:1M" % path
 
     with pytest.raises(util.CommandError) as e:
         util.sanlock("client", "lookup", "-x", rindex,
"-e", "res")
 
@@ -208,11 +209,11 @@ def test_lookup_uninitialized(tmpdir, sanlock_daemon):
 
 
 def test_lookup_missing(tmpdir, sanlock_daemon):
     path = tmpdir.join("rindex")
     # Slots: lockspace rindex master-lease user-lease-1 ... user-lease-7
-    size = 1024**2 * 10
+    size = 10 * MiB
     util.create_file(str(path), size)
 
     # Note: using 1 second io timeout (-o 1) for quicker tests.
     lockspace = "ls_name:1:%s:0" % path
     util.sanlock("client", "init", "-s", lockspace,
"-o", "1")
diff --git a/tests/direct_test.py b/tests/direct_test.py
index 4ef63bc..a157bc4 100644
--- a/tests/direct_test.py
+++ b/tests/direct_test.py
@@ -6,15 +6,16 @@ from __future__ import absolute_import
 import io
 import struct
 
 from . import constants
 from . import util
+from . units import *
 
 
 def test_init_lockspace(tmpdir):
     path = tmpdir.join("lockspace")
-    size = 1024**2
+    size = MiB
     util.create_file(str(path), size)
 
     lockspace = "name:1:%s:0" % path
     util.sanlock("direct", "init", "-s", lockspace)
 
@@ -27,11 +28,11 @@ def test_init_lockspace(tmpdir):
     util.check_guard(str(path), size)
 
 
 def test_init_resource(tmpdir, sanlock_daemon):
     path = tmpdir.join("resources")
-    size = 1024**2
+    size = MiB
     util.create_file(str(path), size)
 
     resource = "ls_name:res_name:%s:0" % path
     util.sanlock("direct", "init", "-r", resource)
 
diff --git a/tests/python_test.py b/tests/python_test.py
index 28827d6..e721fe8 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -12,24 +12,26 @@ import pytest
 
 import sanlock
 
 from . import constants
 from . import util
+from . units import *
+
 
 # Largest file size on ext4 is 16TiB, and on xfs 500 TiB. Use 1 TiB as it is
 # large enough to test large offsets, and less likely to fail on developer
 # machine or CI slave.
 # See https://access.redhat.com/articles/rhel-limits
-LARGE_FILE_SIZE = 1024**4
+LARGE_FILE_SIZE = TiB
 
-LOCKSPACE_SIZE = 1024**2
-MIN_RES_SIZE = 1024**2
+LOCKSPACE_SIZE = MiB
+MIN_RES_SIZE = MiB
 
-ALIGNMENT_1M = 1 * 1024**2
-ALIGNMENT_2M = 2 * 1024**2
+ALIGNMENT_1M = 1 * MiB
+ALIGNMENT_2M = 2 * MiB
 SECTOR_SIZE_512 = 512
-SECTOR_SIZE_4K = 4096
+SECTOR_SIZE_4K = 4 * KiB
 
 
 @pytest.mark.parametrize("size,offset", [
     # Smallest offset.
     (LOCKSPACE_SIZE, 0),
@@ -238,11 +240,11 @@ def test_read_resource_owners_4k_invalid_sector_size(
     assert e.value.errno == errno.EINVAL
 
 
 def test_read_resource_owners_invalid_align_size(tmpdir, sanlock_daemon):
     path = str(tmpdir.join("path"))
-    util.create_file(path, 1024**3)
+    util.create_file(path, GiB)
     disks = [(path, 0)]
 
     sanlock.write_resource(
         "ls_name",
         "res_name",
@@ -292,11 +294,11 @@ def test_add_rem_lockspace(tmpdir, sanlock_daemon, size, offset):
     assert acquired is False
 
 
 def test_add_rem_lockspace_async(tmpdir, sanlock_daemon):
     path = str(tmpdir.join("ls_name"))
-    util.create_file(path, 1024**2)
+    util.create_file(path, MiB)
 
     sanlock.write_lockspace("ls_name", path, iotimeout=1)
     acquired = sanlock.inq_lockspace("ls_name", 1, path, wait=False)
     assert acquired is False
 
@@ -401,13 +403,13 @@ def test_acquire_release_resource(tmpdir, sanlock_daemon, size,
offset):
     assert owners == []
 
 
 @pytest.mark.parametrize("align, sector", [
     # Invalid alignment
-    (1024, sanlock.SECTOR_SIZE[0]),
+    (KiB, sanlock.SECTOR_SIZE[0]),
     # Invalid sector size
-    (sanlock.ALIGN_SIZE[0], 8192),
+    (sanlock.ALIGN_SIZE[0], 8 * KiB),
 ])
 def test_write_lockspace_invalid_align_sector(
         tmpdir, sanlock_daemon, align, sector):
     path = str(tmpdir.join("lockspace"))
     util.create_file(path, LOCKSPACE_SIZE)
@@ -416,13 +418,13 @@ def test_write_lockspace_invalid_align_sector(
         sanlock.write_lockspace("name", path, align=align, sector=sector)
 
 
 @pytest.mark.parametrize("align, sector", [
     # Invalid alignment
-    (1024, sanlock.SECTOR_SIZE[0]),
+    (KiB, sanlock.SECTOR_SIZE[0]),
     # Invalid sector size
-    (sanlock.ALIGN_SIZE[0], 8192),
+    (sanlock.ALIGN_SIZE[0], 8 * KiB),
 ])
 def test_write_resource_invalid_align_sector(
         tmpdir, sanlock_daemon, align, sector):
     path = str(tmpdir.join("resources"))
     util.create_file(path, MIN_RES_SIZE)
diff --git a/tests/units.py b/tests/units.py
new file mode 100644
index 0000000..b47d668
--- /dev/null
+++ b/tests/units.py
@@ -0,0 +1,9 @@
+"""
+Constants for file/disk sizes.
+"""
+
+KiB = 1024
+MiB = 1024**2
+GiB = 1024**3
+TiB = 1024**4
+PiB = 1024**5
diff --git a/tests/util.py b/tests/util.py
index e31190e..86c6447 100644
--- a/tests/util.py
+++ b/tests/util.py
@@ -9,15 +9,17 @@ import os
 import socket
 import struct
 import subprocess
 import time
 
+from . units import *
+
 TESTDIR = os.path.dirname(__file__)
 SANLOCK = os.path.join(TESTDIR, os.pardir, "src", "sanlock")
 
 GUARD = b"X"
-GUARD_SIZE = 4096
+GUARD_SIZE = 4 * KiB
 
 
 class TimeoutExpired(Exception):
     """ Raised when timeout expired """
 
-- 
2.17.2

[PATCH v2 12/23] tests: Add util.generate_path for generating file paths by encoding

[Nir Soffer]

From: Amit Bawer <abawer(a)redhat.com&gt;

The path generation utility function will be used for testing API's
ability to handle the different encoded path strings/bytes in
upcoming changes related to path argument parsing and conversion.
---
 tests/util.py | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tests/util.py b/tests/util.py
index 86c6447..d944611 100644
--- a/tests/util.py
+++ b/tests/util.py
@@ -146,5 +146,17 @@ def check_rindex_entry(entry, name, offset=None, flags=None):
     if offset is not None:
         assert e_offset == offset
 
     if flags is not None:
         assert e_flags == flags
+
+
+def generate_path(dirname, filename, encoding=None):
+    """
+    Return path by joining dirname and filename.
+    The resulting path is either string if encoding is None,
+    or bytes encoded according to given encoding parameter.
+    """
+    path = os.path.join(str(dirname), filename)
+    if encoding:
+        path = path.encode(encoding)
+    return path
-- 
2.17.2

[PATCH v2 13/23] tests: Add parameterized tests for file paths encodings

[Nir Soffer]

From: Amit Bawer <abawer(a)redhat.com&gt;

Currently we have two tests expecting to fail, this holds until we add
the path converter funcationality.

- Py2: currenlty not supporing non-ascii paths
- Py3: currently not supporting bytes paths
---
 tests/python_test.py | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/tests/python_test.py b/tests/python_test.py
index e721fe8..a24990d 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -6,10 +6,11 @@ from __future__ import absolute_import
 import errno
 import io
 import struct
 import time
 
+import six
 import pytest
 
 import sanlock
 
 from . import constants
@@ -30,18 +31,35 @@ ALIGNMENT_1M = 1 * MiB
 ALIGNMENT_2M = 2 * MiB
 SECTOR_SIZE_512 = 512
 SECTOR_SIZE_4K = 4 * KiB
 
 
+FILE_NAMES = [
+    #name, encoding
+    ("ascii", None),
+    (u"ascii", None),
+    pytest.param(
+        u"\u05d0", None,
+        marks=pytest.mark.xfail(
+            six.PY2,
+            reason="currently not supporting non-ascii paths")),
+    pytest.param(
+        u"\u05d0", "utf-8",
+        marks=pytest.mark.xfail(
+            six.PY3,
+            reason="currently not supporting bytes paths")),
+]
+
+(a)pytest.mark.parametrize(&quot;filename, encoding" , FILE_NAMES)
 @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, size, offset):
-    path = str(tmpdir.join("lockspace"))
+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("name", path, offset=offset, iotimeout=1)
 
@@ -114,18 +132,19 @@ def test_read_lockspace_4k_invalid_sector_size(sanlock_daemon,
user_4k_path):
     with pytest.raises(sanlock.SanlockException) as e:
         sanlock.read_lockspace(user_4k_path, sector=SECTOR_SIZE_512)
     assert e.value.errno == errno.EINVAL
 
 
+(a)pytest.mark.parametrize(&quot;filename,encoding&quot;, FILE_NAMES)
 @pytest.mark.parametrize("size,offset", [
     # Smallest offset.
     (MIN_RES_SIZE, 0),
     # Large offset.
     (LARGE_FILE_SIZE, LARGE_FILE_SIZE - MIN_RES_SIZE),
 ])
-def test_write_resource(tmpdir, sanlock_daemon, size, offset):
-    path = str(tmpdir.join("resources"))
+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.
     sanlock.write_resource("ls_name", "res_name", disks)
-- 
2.17.2

[PATCH v2 14/23] sanlock: Add module init for Python 3

[Nir Soffer]

From: Amit Bawer <abawer(a)redhat.com&gt;

At this stage building the sanlock module for Py3 is
expected to fail as we have modifications to be done
in upcoming patches to fill in missing Py3 API calls.
---
 python/sanlock.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/python/sanlock.c b/python/sanlock.c
index 420b4ba..aa3b9a8 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -1775,10 +1775,38 @@ module_init(PyObject* m)
     }
 
     return 0;
 }
 
+#if PY_MAJOR_VERSION >= 3 /* Python 3 module init */
+
+static struct PyModuleDef moduledef = {
+    PyModuleDef_HEAD_INIT,
+    MODULE_NAME,
+    pydoc_sanlock,
+    -1,
+    sanlock_methods,
+};
+
+PyMODINIT_FUNC
+PyInit_sanlock(void)
+{
+    PyObject *m = PyModule_Create(&moduledef);
+
+    if (m == NULL)
+        return NULL;
+
+    if (module_init(m)) {
+        Py_DECREF(m);
+        return NULL;
+    }
+
+    return m;
+}
+
+#else /* Python 2 module init */
+
 PyMODINIT_FUNC
 initsanlock(void)
 {
     PyObject *m = Py_InitModule3(
         MODULE_NAME,
@@ -1790,6 +1818,8 @@ initsanlock(void)
 
     /* We don't have anything to do if module_init() fails. */
     module_init(m);
 }
 
+#endif
+
 /* vim: set expandtab shiftwidth=4 tabstop=4 : */
-- 
2.17.2

[PATCH v2 14/23] sanlock: Add module init for Python 3

[Vojtech Juranek]

>  +#if PY_MAJOR_VERSION >= 3 /* Python 3 module init */
>  + 
in later commit everywhere is used 

```
#if PY_MAJOR_VERSION == 2
    ...
#else
```

maybe we should use it here as well for consistency (but I haven't any strong 
opinion about it, keep it as it is if you like PY_MAJOR_VERSION >= 3  more)

[PATCH v2 14/23] sanlock: Add module init for Python 3

[Nir Soffer]

On Mon, May 20, 2019 at 3:22 PM Vojtech Juranek <vjuranek(a)redhat.com&gt; wrote:

>  > +#if PY_MAJOR_VERSION >= 3 /* Python 3 module init */
>  > +
>
>  in later commit everywhere is used
>
>  ```
>  #if PY_MAJOR_VERSION == 2
>      ...
>  #else
>  ```
>
>  maybe we should use it here as well for consistency (but I haven't any
>  strong
>  opinion about it, keep it as it is if you like PY_MAJOR_VERSION >= 3  more)
>  
I think == 3 is best, since we don't know if this will work on 4 as is, but
it is
not critical.

I don't think we except python 4 before we will drop python 2, and when we
do we will
simply drop the ifdef and leave only the python 3 version.

Nir

[PATCH v2 15/23] python: Add pystring_as_cstring function

[Nir Soffer]

From: Amit Bawer <abawer(a)redhat.com&gt;

This helper allows accessing the inner bytes of
PyObject according to python version:

- Py2: String object
- Py3: Unicode object

This method is mostly used for paths as a temp
solution until we add the path convertor.
---
 python/sanlock.c | 30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/python/sanlock.c b/python/sanlock.c
index aa3b9a8..108a7b8 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -69,10 +69,32 @@ __set_exception(int en, char *msg)
         PyErr_SetObject(py_exception, exc_tuple);
         Py_DECREF(exc_tuple);
     }
 }
 
+/*
+ * Returns NULL-terminated representation of the contents of obj.
+ *
+ * obj must be a string object (py2) or Unicode object (py3), otherwise returns NULL
+ * and raises TypeError.[1][2]
+ *
+ * The returned pointer refers to the internal buffer of string, not a copy. It must not
be
+ * deallocated, and the object must be kept alive as long as the retruned pointer is
used.
+ * [1] https://docs.python.org/2/c-api/string.html#c.PyString_AsString
+ * [2] https://docs.python.org/3/c-api/unicode.html#c.PyUnicode_AsUTF8
+ *
+ */
+static const char*
+pystring_as_cstring(PyObject *obj)
+{
+#if PY_MAJOR_VERSION == 2
+    return PyString_AsString(obj);
+#else
+    return PyUnicode_AsUTF8(obj);
+#endif
+}
+
 static int
 __parse_resource(PyObject *obj, struct sanlk_resource **res_ret)
 {
     int i, num_disks, res_len;
     struct sanlk_resource *res;
@@ -90,11 +112,11 @@ __parse_resource(PyObject *obj, struct sanlk_resource **res_ret)
 
     memset(res, 0, res_len);
     res->num_disks = num_disks;
 
     for (i = 0; i < num_disks; i++) {
-        char *p = NULL;
+        const char *p = NULL;
         PyObject *tuple, *path = NULL, *offset = NULL;
 
         tuple = PyList_GetItem(obj, i);
 
         if (PyTuple_Check(tuple)) {
@@ -104,11 +126,11 @@ __parse_resource(PyObject *obj, struct sanlk_resource **res_ret)
             }
 
             path = PyTuple_GetItem(tuple, 0);
             offset = PyTuple_GetItem(tuple, 1);
 
-            p = PyString_AsString(path);
+            p = pystring_as_cstring(path);
 
             if (!PyInt_Check(offset)) {
                 __set_exception(EINVAL, "Invalid resource offset");
                 goto exit_fail;
             }
@@ -193,11 +215,11 @@ static void
 set_value_error(const char* format, PyObject* obj)
 {
     const char* str_rep = "";
     PyObject* rep = PyObject_Repr(obj);
     if (rep)
-        str_rep = PyString_AsString(rep);
+        str_rep = pystring_as_cstring(rep);
     PyErr_Format(PyExc_ValueError, format, str_rep);
     Py_XDECREF(rep);
 }
 
 static PyObject *
@@ -1313,11 +1335,11 @@ py_killpath(PyObject *self __unused, PyObject *args, PyObject
*keywds)
     /* creating the arguments string from a python list */
     for (i = 0, n = 0; i < num_args; i++) {
         size_t arg_len;
 
         item = PyList_GetItem(argslist, i);
-        p = PyString_AsString(item);
+        p = pystring_as_cstring(item);
 
         if (p == NULL) {
             __set_exception(EINVAL, "Killpath argument not a string");
             return NULL;
         }
-- 
2.17.2

[PATCH v2 16/23] python: Provide helper for Py{Int,Long}_AsUnsignedLongLongMask

[Nir Soffer]

From: Amit Bawer <abawer(a)redhat.com&gt;

This is part of patches for making numerical API compatible
with Py3 and Py2.

All use-cases of this function assigns value to uint64_t vars
so we switch to respective AsUnsignedLongLongMask APIs.

Current status:
- Py2: build and testing should be Okay.
- Py3: build will fail for missing numerical APIs.

Signed-off-by: Amit Bawer <abawer(a)redhat.com&gt;
---
 python/sanlock.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/python/sanlock.c b/python/sanlock.c
index 108a7b8..9109032 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -69,10 +69,20 @@ __set_exception(int en, char *msg)
         PyErr_SetObject(py_exception, exc_tuple);
         Py_DECREF(exc_tuple);
     }
 }
 
+static uint64_t
+pyinteger_as_unsigned_long_long_mask(PyObject *obj)
+{
+#if PY_MAJOR_VERSION == 2
+    return PyInt_AsUnsignedLongLongMask(obj);
+#else
+    return PyLong_AsUnsignedLongLongMask(obj);
+#endif
+}
+
 /*
  * Returns NULL-terminated representation of the contents of obj.
  *
  * obj must be a string object (py2) or Unicode object (py3), otherwise returns NULL
  * and raises TypeError.[1][2]
@@ -1090,11 +1100,11 @@ py_acquire(PyObject *self __unused, PyObject *args, PyObject
*keywds)
     }
 
     /* prepare the resource version */
     if (version != Py_None) {
         res->flags |= SANLK_RES_LVER;
-        res->lver = PyInt_AsUnsignedLongMask(version);
+        res->lver = pyinteger_as_unsigned_long_long_mask(version);
         if (res->lver == -1) {
             __set_exception(EINVAL, "Unable to convert the version value");
             goto exit_fail;
         }
     }
@@ -1206,11 +1216,11 @@ py_request(PyObject *self __unused, PyObject *args, PyObject
*keywds)
     /* prepare the resource version */
     if (version == Py_None) {
         flags = SANLK_REQUEST_NEXT_LVER;
     } else {
         res->flags |= SANLK_RES_LVER;
-        res->lver = PyInt_AsUnsignedLongMask(version);
+        res->lver = pyinteger_as_unsigned_long_long_mask(version);
         if (res->lver == -1) {
             __set_exception(EINVAL, "Unable to convert the version value");
             goto exit_fail;
         }
     }
-- 
2.17.2

[PATCH v2 17/23] python: Refactor build of dictionary entries for hosts_to_list

[Nir Soffer]

From: Amit Bawer <abawer(a)redhat.com&gt;

Use Py_BuildValue("{...}",...) for creating each dictionary entry
in hosts list.

Current status:
- Py2: will build and pass tests.
- Py3: will fail to build for missing respective API calls.
---
 python/sanlock.c | 53 ++++++++++--------------------------------------
 1 file changed, 11 insertions(+), 42 deletions(-)

diff --git a/python/sanlock.c b/python/sanlock.c
index 9109032..b4533f2 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -233,59 +233,28 @@ set_value_error(const char* format, PyObject* obj)
 }
 
 static PyObject *
 __hosts_to_list(struct sanlk_host *hss, int hss_count)
 {
-    int i, rv;
-    PyObject *ls_list = NULL, *ls_entry = NULL, *ls_value = NULL;
+    int i;
+    PyObject *ls_list = NULL, *ls_entry = NULL;
 
     /* prepare the dictionary holding the information */
     if ((ls_list = PyList_New(0)) == NULL)
         goto exit_fail;
 
     for (i = 0; i < hss_count; i++) {
-        if ((ls_entry = PyDict_New()) == NULL)
-            goto exit_fail;
-
-        /* fill the dictionary information: host_id */
-        if ((ls_value = PyInt_FromLong(hss[i].host_id)) == NULL)
-            goto exit_fail;
-        rv = PyDict_SetItemString(ls_entry, "host_id", ls_value);
-        Py_DECREF(ls_value);
-        if (rv != 0)
-            goto exit_fail;
-
-        /* fill the dictionary information: generation */
-        if ((ls_value = PyInt_FromLong(hss[i].generation)) == NULL)
-            goto exit_fail;
-        rv = PyDict_SetItemString(ls_entry, "generation", ls_value);
-        Py_DECREF(ls_value);
-        if (rv != 0)
-            goto exit_fail;
 
-        /* fill the dictionary information: timestamp */
-        if ((ls_value = PyInt_FromLong(hss[i].timestamp)) == NULL)
-            goto exit_fail;
-        rv = PyDict_SetItemString(ls_entry, "timestamp", ls_value);
-        Py_DECREF(ls_value);
-        if (rv != 0)
-            goto exit_fail;
-
-        /* fill the dictionary information: io_timeout */
-        if ((ls_value = PyInt_FromLong(hss[i].io_timeout)) == NULL)
-            goto exit_fail;
-        rv = PyDict_SetItemString(ls_entry, "io_timeout", ls_value);
-        Py_DECREF(ls_value);
-        if (rv != 0)
-            goto exit_fail;
-
-        /* fill the dictionary information: flags */
-        if ((ls_value = PyInt_FromLong(hss[i].flags)) == NULL)
-            goto exit_fail;
-        rv = PyDict_SetItemString(ls_entry, "flags", ls_value);
-        Py_DECREF(ls_value);
-        if (rv != 0)
+        /* fill the dictionary information */
+        ls_entry = Py_BuildValue(
+                "{s:K,s:K,s:K,s:I,s:I}",
+                "host_id", hss[i].host_id,
+                "generation", hss[i].generation,
+                "timestamp", hss[i].timestamp,
+                "io_timeout", hss[i].io_timeout,
+                "flags", hss[i].flags);
+        if (ls_entry == NULL)
             goto exit_fail;
 
         if (PyList_Append(ls_list, ls_entry) != 0)
             goto exit_fail;
 
-- 
2.17.2

[PATCH v2 18/23] python: Simplify parse_resource() with PyArg_ParseTuple

[Nir Soffer]

Replace complex manual parsing code with se PyArg_ParseTuple, avoiding
the backward compatibility issue, and greatly simplifying the code.

The test for invalid resource was modifed to test more types and
combinations, including incorrect number of items, and invalid types for
the path and offset.

Current status:
    - Py2: will build and pass tests.
    - Py3: will fail to build for missing respective API calls.

Signed-off-by: Nir Soffer <nsoffer(a)redhat.com&gt;
---
 python/sanlock.c     | 42 ++++++++++++------------------------------
 tests/python_test.py | 24 ++++++++++++++++--------
 2 files changed, 28 insertions(+), 38 deletions(-)

diff --git a/python/sanlock.c b/python/sanlock.c
index b4533f2..5d45b6a 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -122,48 +122,30 @@ __parse_resource(PyObject *obj, struct sanlk_resource **res_ret)
 
     memset(res, 0, res_len);
     res->num_disks = num_disks;
 
     for (i = 0; i < num_disks; i++) {
-        const char *p = NULL;
-        PyObject *tuple, *path = NULL, *offset = NULL;
+        PyObject *disk;
+        const char *path;
+        uint64_t offset;
 
-        tuple = PyList_GetItem(obj, i);
+        disk = PyList_GetItem(obj, i);
 
-        if (PyTuple_Check(tuple)) {
-            if (PyTuple_Size(tuple) != 2) {
-                __set_exception(EINVAL, "Invalid resource tuple");
-                goto exit_fail;
-            }
-
-            path = PyTuple_GetItem(tuple, 0);
-            offset = PyTuple_GetItem(tuple, 1);
-
-            p = pystring_as_cstring(path);
-
-            if (!PyInt_Check(offset)) {
-                __set_exception(EINVAL, "Invalid resource offset");
-                goto exit_fail;
-            }
-        } else {
-            /* handle invalid non-tuple resource input */
-            set_value_error("invalid disk value: %s", tuple);
+        if (!PyTuple_Check(disk)) {
+            set_value_error("Invalid disk %s", disk);
             goto exit_fail;
+
         }
 
-        if (p == NULL) {
-            __set_exception(EINVAL, "Invalid resource path");
+        if (!PyArg_ParseTuple(disk, "sK", &path, &offset)) {
+            /* Override the error since it confusing in this context. */
+            set_value_error("Invalid disk %s", disk);
             goto exit_fail;
         }
 
-        strncpy(res->disks[i].path, p, SANLK_PATH_LEN - 1);
-
-        if (offset == NULL) {
-            res->disks[i].offset = 0;
-        } else {
-            res->disks[i].offset = PyInt_AsLong(offset);
-        }
+        strncpy(res->disks[i].path, path, SANLK_PATH_LEN - 1);
+        res->disks[i].offset = offset;
     }
 
     *res_ret = res;
     return 0;
 
diff --git a/tests/python_test.py b/tests/python_test.py
index a24990d..cfbdf9c 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -452,18 +452,26 @@ def test_write_resource_invalid_align_sector(
     with pytest.raises(ValueError):
         sanlock.write_resource(
             "ls_name", "res_name", disks, align=align,
sector=sector)
 
 
-(a)pytest.mark.parametrize(&quot;resource&quot;, [
-    "invalid resource tuple",
-    b"invalid resource tuple",
+(a)pytest.mark.parametrize(&quot;disk&quot;, [
+    # Not a tuple - unicode and bytes:
+    "not a tuple",
+    b"not a tuple",
     u'\u05e9\u05dc\u05d5\u05dd',
-    b"\xd7\x90"
+    b"\xd7\x90",
+    # Tuple with incorrect length:
+    (),
+    ("path",),
+    ("path", 0, "extra"),
+    # Tuple with invalid content:
+    (0, "path"),
+    ("path", "not an offset"),
 ])
-def test_write_resource_invalid_path(tmpdir, sanlock_daemon, resource):
-    # Test parsing a resource which is not a list of tuples
-    disks = [resource]
+def test_write_resource_invalid_disk(tmpdir, sanlock_daemon, disk):
+    # Test parsing disks list with invalid content.
+    disks = [disk]
     with pytest.raises(ValueError) as e:
         sanlock.write_resource("ls_name", "res_name", disks)
-    assert repr(resource) in str(e.value)
+    assert repr(disk) in str(e.value)
 
-- 
2.17.2

[PATCH v2 19/23] python: Refactor build of dictionary entries for py_get_lockspaces

[Nir Soffer]

From: Amit Bawer <abawer(a)redhat.com&gt;

Use Py_BuildValue("{...}",...) for creating each dictionary entry
in lockspaces list.

Using helper for different path formats:
- Py2: will treat path as PyString using the "s" format.
- Py3: will treat path as PyBytes using the "y" format.

Current status:
- Py2: will build and pass tests.
- Py3: will fail to build for missing respective API calls.
---
 python/sanlock.c | 55 ++++++++++++------------------------------------
 1 file changed, 14 insertions(+), 41 deletions(-)

diff --git a/python/sanlock.c b/python/sanlock.c
index 5d45b6a..e4b1407 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -876,11 +876,11 @@ released.\n");
 static PyObject *
 py_get_lockspaces(PyObject *self __unused, PyObject *args, PyObject *keywds)
 {
     int rv, i, lss_count;
     struct sanlk_lockspace *lss = NULL;
-    PyObject *ls_list = NULL, *ls_entry = NULL, *ls_value = NULL;
+    PyObject *ls_list = NULL, *ls_entry = NULL;
 
     /* get all the lockspaces (gil disabled) */
     Py_BEGIN_ALLOW_THREADS
     rv = sanlock_get_lockspaces(&lss, &lss_count, 0);
     Py_END_ALLOW_THREADS
@@ -893,51 +893,24 @@ py_get_lockspaces(PyObject *self __unused, PyObject *args, PyObject
*keywds)
     /* prepare the dictionary holding the information */
     if ((ls_list = PyList_New(0)) == NULL)
         goto exit_fail;
 
     for (i = 0; i < lss_count; i++) {
-        if ((ls_entry = PyDict_New()) == NULL)
-            goto exit_fail;
-
-        /* fill the dictionary information: lockspace */
-        if ((ls_value = PyString_FromString(lss[i].name)) == NULL)
-            goto exit_fail;
-        rv = PyDict_SetItemString(ls_entry, "lockspace", ls_value);
-        Py_DECREF(ls_value);
-        if (rv != 0)
-            goto exit_fail;
-
-        /* fill the dictionary information: host_id */
-        if ((ls_value = PyInt_FromLong(lss[i].host_id)) == NULL)
-            goto exit_fail;
-        rv = PyDict_SetItemString(ls_entry, "host_id", ls_value);
-        Py_DECREF(ls_value);
-        if (rv != 0)
-            goto exit_fail;
-
-        /* fill the dictionary information: path */
-        if ((ls_value = PyString_FromString(lss[i].host_id_disk.path)) == NULL)
-            goto exit_fail;
-        rv = PyDict_SetItemString(ls_entry, "path", ls_value);
-        Py_DECREF(ls_value);
-        if (rv != 0)
-            goto exit_fail;
 
-        /* fill the dictionary information: offset */
-        if ((ls_value = PyInt_FromLong(lss[i].host_id_disk.offset)) == NULL)
-            goto exit_fail;
-        rv = PyDict_SetItemString(ls_entry, "offset", ls_value);
-        Py_DECREF(ls_value);
-        if (rv != 0)
-            goto exit_fail;
-
-        /* fill the dictionary information: flags */
-        if ((ls_value = PyInt_FromLong(lss[i].flags)) == NULL)
-            goto exit_fail;
-        rv = PyDict_SetItemString(ls_entry, "flags", ls_value);
-        Py_DECREF(ls_value);
-        if (rv != 0)
+        /* fill the dictionary information */
+        ls_entry = Py_BuildValue(
+#if PY_MAJOR_VERSION == 2
+                "{s:s,s:K,s:s,s:K,s:I}",
+#else
+                "{s:y,s:K,s:s,s:K,s:I}",
+#endif
+                "lockspace", lss[i].name,
+                "host_id", lss[i].host_id,
+                "path", lss[i].host_id_disk.path,
+                "offset", lss[i].host_id_disk.offset,
+                "flags", lss[i].flags);
+        if (ls_entry == NULL)
             goto exit_fail;
 
         if (PyList_Append(ls_list, ls_entry) != 0)
             goto exit_fail;
 
-- 
2.17.2

[PATCH v2 20/23] python: Refactor build of dictionary entries for py_read_lockspace

[Nir Soffer]

From: Amit Bawer <abawer(a)redhat.com&gt;

Use Py_BuildValue("{...}",...) for creating dictionary information
for lockspace.

Using helper to determine the build value format:
- Py2: Handle lockspace name as PyString with "s" format.
- Py3: Handle lockspace name as PyBytes with "y" format.

Current status:
- Py2: will build and pass tests.
- Py3: will fail to build for missing respective API calls.
---
 python/sanlock.c | 37 ++++++++++++-------------------------
 1 file changed, 12 insertions(+), 25 deletions(-)

diff --git a/python/sanlock.c b/python/sanlock.c
index e4b1407..5870081 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -473,11 +473,11 @@ py_read_lockspace(PyObject *self __unused, PyObject *args, PyObject
*keywds)
     int rv, sector = SECTOR_SIZE_512;
     long align = ALIGNMENT_1M;
     uint32_t io_timeout = 0;
     const char *path;
     struct sanlk_lockspace ls;
-    PyObject *ls_info = NULL, *ls_entry = NULL;
+    PyObject *ls_info = NULL;
 
     static char *kwlist[] = {"path", "offset", "align",
"sector", NULL};
 
     /* initialize lockspace structure */
     memset(&ls, 0, sizeof(struct sanlk_lockspace));
@@ -506,37 +506,24 @@ py_read_lockspace(PyObject *self __unused, PyObject *args, PyObject
*keywds)
     if (rv != 0) {
         __set_exception(rv, "Sanlock lockspace read failure");
         return NULL;
     }
 
-    /* prepare the dictionary holding the information */
-    if ((ls_info = PyDict_New()) == NULL)
-        goto exit_fail;
-
-    /* fill the dictionary information: lockspace */
-    if ((ls_entry = PyString_FromString(ls.name)) == NULL)
-        goto exit_fail;
-    rv = PyDict_SetItemString(ls_info, "lockspace", ls_entry);
-    Py_DECREF(ls_entry);
-    if (rv != 0)
-        goto exit_fail;
-
-    /* fill the dictionary information: iotimeout */
-    if ((ls_entry = PyInt_FromLong(io_timeout)) == NULL)
-        goto exit_fail;
-    rv = PyDict_SetItemString(ls_info, "iotimeout", ls_entry);
-    Py_DECREF(ls_entry);
-    if (rv != 0)
-        goto exit_fail;
+    /* fill the information dictionary */
+    ls_info = Py_BuildValue(
+#if PY_MAJOR_VERSION == 2
+            "{s:s,s:I}",
+#else
+            "{s:y,s:I}",
+#endif
+            "lockspace", ls.name,
+            "iotimeout", io_timeout);
+    if (ls_info  == NULL)
+        return NULL;
 
     /* success */
     return ls_info;
-
-    /* failure */
-exit_fail:
-    Py_XDECREF(ls_info);
-    return NULL;
 }
 
 /* read_resource */
 PyDoc_STRVAR(pydoc_read_resource, "\
 read_resource(path, offset=0, align=1048576, sector=512) -> dict\n\
-- 
2.17.2

[PATCH v2 21/23] python: Refactor build of dictionary entries for py_read_resource

[Nir Soffer]

From: Amit Bawer <abawer(a)redhat.com&gt;

Use Py_BuildValue("{...}",...) for creating dictionary information
for resource.

Using helper to determine the build value format:
 - Py2: Handle lockspace and resource name as PyString with "s" format.
 - Py3: Handle lockspace and resource name as PyBytes with "y" format.

Current status:
- Py2: will build and pass tests.
- Py3: build will fail for missing respective API calls.
---
 python/sanlock.c | 37 +++++++++++--------------------------
 1 file changed, 11 insertions(+), 26 deletions(-)

diff --git a/python/sanlock.c b/python/sanlock.c
index 5870081..6477afe 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -536,11 +536,11 @@ py_read_resource(PyObject *self __unused, PyObject *args, PyObject
*keywds)
 {
     int rv, rs_len, sector = SECTOR_SIZE_512;
     long align = ALIGNMENT_1M;
     const char *path;
     struct sanlk_resource *rs;
-    PyObject *rs_info = NULL, *rs_entry = NULL;
+    PyObject *rs_info = NULL;
 
     static char *kwlist[] = {"path", "offset", "align",
"sector", NULL};
 
     /* allocate the needed memory for the resource and one disk */
     rs_len = sizeof(struct sanlk_resource) + sizeof(struct sanlk_disk);
@@ -580,35 +580,20 @@ py_read_resource(PyObject *self __unused, PyObject *args, PyObject
*keywds)
         __set_exception(rv, "Sanlock resource read failure");
         goto exit_fail;
     }
 
     /* prepare the dictionary holding the information */
-    if ((rs_info = PyDict_New()) == NULL)
-        goto exit_fail;
-
-    /* fill the dictionary information: lockspace */
-    if ((rs_entry = PyString_FromString(rs->lockspace_name)) == NULL)
-        goto exit_fail;
-    rv = PyDict_SetItemString(rs_info, "lockspace", rs_entry);
-    Py_DECREF(rs_entry);
-    if (rv != 0)
-        goto exit_fail;
-
-    /* fill the dictionary information: resource */
-    if ((rs_entry = PyString_FromString(rs->name)) == NULL)
-        goto exit_fail;
-    rv = PyDict_SetItemString(rs_info, "resource", rs_entry);
-    Py_DECREF(rs_entry);
-    if (rv != 0)
-        goto exit_fail;
-
-    /* fill the dictionary information: version */
-    if ((rs_entry = PyLong_FromUnsignedLong(rs->lver)) == NULL)
-        goto exit_fail;
-    rv = PyDict_SetItemString(rs_info, "version", rs_entry);
-    Py_DECREF(rs_entry);
-    if (rv != 0)
+    rs_info = Py_BuildValue(
+#if PY_MAJOR_VERSION == 2
+        "{s:s,s:s,s:K}",
+#else
+        "{s:y,s:y,s:K}",
+#endif
+        "lockspace", rs->lockspace_name,
+        "resource", rs->name,
+        "version", rs->lver);
+    if (rs_info  == NULL)
         goto exit_fail;
 
     /* success */
     free(rs);
     return rs_info;
-- 
2.17.2

[PATCH v2 22/23] python: Use Py_BuildValue("i", ...) to create py integers

[Nir Soffer]

From: Amit Bawer <abawer(a)redhat.com&gt;

Preserve compatibility for both Py3 and Py2.

- Py2: will construct new PyInt
- Py3: will construct new PyLong

Both Py versions will build and pass tests at this point.
---
 python/sanlock.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/python/sanlock.c b/python/sanlock.c
index 6477afe..b412b6a 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -271,11 +271,11 @@ py_register(PyObject *self __unused, PyObject *args)
     if (sanlockfd < 0) {
         __set_exception(sanlockfd, "Sanlock registration failed");
         return NULL;
     }
 
-    return PyInt_FromLong(sanlockfd);
+    return Py_BuildValue("i", sanlockfd);
 }
 
 /* get_alignment */
 PyDoc_STRVAR(pydoc_get_alignment, "\
 get_alignment(path) -> int\n\
@@ -304,11 +304,11 @@ py_get_alignment(PyObject *self __unused, PyObject *args)
     if (rv < 0) {
         __set_exception(rv, "Unable to get device alignment");
         return NULL;
     }
 
-    return PyInt_FromLong(rv);
+    return Py_BuildValue("i", rv);
 }
 
 /* init_lockspace */
 PyDoc_STRVAR(pydoc_init_lockspace, "\
 init_lockspace(lockspace, path, offset=0, max_hosts=0, num_hosts=0, \
-- 
2.17.2

[PATCH v2 23/23] travis: Add python 3 builds

[Nir Soffer]

All tests pass now with python 2 and 3, so we can start testing every
push with all python versions.

Signed-off-by: Nir Soffer <nsoffer(a)redhat.com&gt;
---
 .travis.yml | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index 7c1749c..7b3fbe5 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,8 +1,14 @@
 dist: xenial
+
 language: python
-python: 2.7
+
+python:
+  - "2.7"
+  - "3.6"
+  - "3.7"
+  - "3.8-dev"
 
 addons:
   apt:
     packages:
     - gcc
-- 
2.17.2

[Vojtech Juranek]

Overall looks good to me (2 minor issue, no objections if we keep the code as 
it is)

>  The first phase of python 3 porting is done:
>  
>  All the tests pass now in python 2.7, 3.6, 3.7 and 3.8.
>  
>  I plan to push this once we cut python 3.7.3 for EL 7.7.
>  
>  More work is needed:
>  - accept only bytes for resource and lockspaces names on python 3
>  - support both unicode and bytes for paths on python 3, same as posix
>  module 
 - add missing tests (request, killpath, xxx_event)
>  - spec for python 3
>  
>  The patches are also available in this branch:
>  https://github.com/nirs/sanlock/commits/py3
>  commit: 3c03d8672d8589219def0133b188a1ac31d41615
>  
>  Amit Bawer (18):
>    readme: Add prerequisites section
>    tox: Add python version as make argument
>    tests: Use python3 and python2 compatible socket exception type
>    tests: Lockspace and resource names should be read as bytes
>    tests: Enforce absolute import behavior for python2 tests
>    sanlock: Resource API should only support disks as a tuple
>    tests: Test parsing a resource which is not a list of tuples
>    sanlock: Provide internal common module init function
>    tests: Add util.generate_path for generating file paths by encoding
>    tests: Add parameterized tests for file paths encodings
>    sanlock: Add module init for Python 3
>    python: Add pystring_as_cstring function
>    python: Provide helper for Py{Int,Long}_AsUnsignedLongLongMask
>    python: Refactor build of dictionary entries for hosts_to_list
>    python: Refactor build of dictionary entries for py_get_lockspaces
>    python: Refactor build of dictionary entries for py_read_lockspace
>    python: Refactor build of dictionary entries for py_read_resource
>    python: Use Py_BuildValue("i", ...) to create py integers
>  
>  Nir Soffer (3):
>    tests: Update lockspace and resource tests
>    python: Simplify parse_resource() with PyArg_ParseTuple
>    travis: Add python 3 builds
>  
>  Pavel Bar (2):
>    Fix typos in debug logs and comments.
>    Adding constants for Kibibyte, Mebibyte, Gibibyte, Tebibyte & Pebibyte
>      and using them everywhere in tests.
>  
>   .travis.yml          |   8 +-
>   README.dev           |  12 ++
>   python/Makefile      |   6 +-
>   python/sanlock.c     | 409 +++++++++++++++++++++----------------------
>   sanlock.spec.in      |   3 +-
>   src/direct.c         |   2 +-
>   src/resource.c       |   2 +-
>   tests/conftest.py    |   1 +
>   tests/daemon_test.py |  44 ++---
>   tests/direct_test.py |   6 +-
>   tests/python_test.py | 111 ++++++++----
>   tests/units.py       |   9 +
>   tests/util.py        |  21 ++-
>   tox.ini              |   3 +-
>   14 files changed, 361 insertions(+), 276 deletions(-)
>   create mode 100644 tests/units.py
>  
>  -- 
>  2.17.2
>  _______________________________________________
>  sanlock-devel mailing list -- sanlock-devel(a)lists.fedorahosted.org
>  To unsubscribe send an email to sanlock-devel-leave(a)lists.fedorahosted.org
>  Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
>  List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
>  List Archives:
>  https://lists.fedorahosted.org/archives/list/[e-mail]
>  ed.org

[Nir Soffer]

On Mon, May 20, 2019 at 3:23 PM Vojtech Juranek <vjuranek(a)redhat.com&gt; wrote:

>  Overall looks good to me (2 minor issue, no objections if we keep the
> code
>  as
>  it is)
>
>  > The first phase of python 3 porting is done:
>  >
>  > All the tests pass now in python 2.7, 3.6, 3.7 and 3.8.
>  >
>  > I plan to push this once we cut python 3.7.3 for EL 7.7.
>  >
>  > More work is needed:
>  > - accept only bytes for resource and lockspaces names on python 3
>  > - support both unicode and bytes for paths on python 3, same as posix
>  > module
>   - add missing tests (request, killpath, xxx_event)
>  > - spec for python 3
>  
David, now that we tagged the 3.7.3, I think we can push this.


>  >
>  > The patches are also available in this branch:
>  > https://github.com/nirs/sanlock/commits/py3
>  > commit: 3c03d8672d8589219def0133b188a1ac31d41615
>  >
>  > Amit Bawer (18):
>  >   readme: Add prerequisites section
>  >   tox: Add python version as make argument
>  >   tests: Use python3 and python2 compatible socket exception type
>  >   tests: Lockspace and resource names should be read as bytes
>  >   tests: Enforce absolute import behavior for python2 tests
>  >   sanlock: Resource API should only support disks as a tuple
>  >   tests: Test parsing a resource which is not a list of tuples
>  >   sanlock: Provide internal common module init function
>  >   tests: Add util.generate_path for generating file paths by encoding
>  >   tests: Add parameterized tests for file paths encodings
>  >   sanlock: Add module init for Python 3
>  >   python: Add pystring_as_cstring function
>  >   python: Provide helper for Py{Int,Long}_AsUnsignedLongLongMask
>  >   python: Refactor build of dictionary entries for hosts_to_list
>  >   python: Refactor build of dictionary entries for py_get_lockspaces
>  >   python: Refactor build of dictionary entries for py_read_lockspace
>  >   python: Refactor build of dictionary entries for py_read_resource
>  >   python: Use Py_BuildValue("i", ...) to create py integers
>  >
>  > Nir Soffer (3):
>  >   tests: Update lockspace and resource tests
>  >   python: Simplify parse_resource() with PyArg_ParseTuple
>  >   travis: Add python 3 builds
>  >
>  > Pavel Bar (2):
>  >   Fix typos in debug logs and comments.
>  >   Adding constants for Kibibyte, Mebibyte, Gibibyte, Tebibyte & Pebibyte
>  >     and using them everywhere in tests.
>  >
>  >  .travis.yml          |   8 +-
>  >  README.dev           |  12 ++
>  >  python/Makefile      |   6 +-
>  >  python/sanlock.c     | 409 +++++++++++++++++++++----------------------
>  >  sanlock.spec.in      |   3 +-
>  >  src/direct.c         |   2 +-
>  >  src/resource.c       |   2 +-
>  >  tests/conftest.py    |   1 +
>  >  tests/daemon_test.py |  44 ++---
>  >  tests/direct_test.py |   6 +-
>  >  tests/python_test.py | 111 ++++++++----
>  >  tests/units.py       |   9 +
>  >  tests/util.py        |  21 ++-
>  >  tox.ini              |   3 +-
>  >  14 files changed, 361 insertions(+), 276 deletions(-)
>  >  create mode 100644 tests/units.py
>  >
>  > --
>  > 2.17.2
>  > _______________________________________________
>  > sanlock-devel mailing list -- sanlock-devel(a)lists.fedorahosted.org
>  > To unsubscribe send an email to
>  sanlock-devel-leave(a)lists.fedorahosted.org
>  > Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
>  > List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
>  > List Archives:
>  >
>  https://lists.fedorahosted.org/archives/list/[e-mail]
>  > ed.org
>
>  _______________________________________________
>  sanlock-devel mailing list -- sanlock-devel(a)lists.fedorahosted.org
>  To unsubscribe send an email to sanlock-devel-leave(a)lists.fedorahosted.org
>  Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
>  List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
>  List Archives:
>  https://lists.fedorahosted.org/archives/list/[e-mail]...
>