[PATCH v3 0/4] Values instead of flags in python API.

[Vojtech Juranek]

To have more consisten python API, value should be used for secor
size and alignment instead of flags. This patch set replaces flags
with values in public API, expose tuples with supported sector size
and alignment values and add some related tests.

The third version of this patch set fixes mostly typos, formatting,
use constants instead of raw values in tests and fixes other stuff
which doesn't change the logic of original patch.


Vojtech Juranek (4):
  tests: add sector and align parameters
  Accept values instead of flags for sector and aling parameters.
  Expose tuples with supported align and sector values
  tests: add tests for invalid align and sector values

 python/example.py    |   4 +-
 python/sanlock.c     | 168 +++++++++++++++++++++++++++++++++----------
 tests/python_test.py |  55 ++++++++++++--
 3 files changed, 182 insertions(+), 45 deletions(-)

-- 
2.20.1

[PATCH v3 1/4] tests: add sector and align parameters

[Vojtech Juranek]

Add sector and align parameters to read and write lockspace and
resource calls to test_write_lockspace and test_write_resource tests
to test that these parameters are accepted. Other tests still use
calls without these parameters to test the defaults.
---
 tests/python_test.py | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/tests/python_test.py b/tests/python_test.py
index 5029924..6393222 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -34,9 +34,12 @@ def test_write_lockspace(tmpdir, sanlock_daemon, size, offset):
     path = str(tmpdir.join("lockspace"))
     util.create_file(path, size)
 
-    sanlock.write_lockspace("name", path, offset=offset, iotimeout=1)
+    sanlock.write_lockspace(
+        "name", path, offset=offset, iotimeout=1, align=sanlock.ALIGN1M,
+        sector=sanlock.SECTOR512)
 
-    ls = sanlock.read_lockspace(path, offset=offset)
+    ls = sanlock.read_lockspace(
+        path, offset=offset, align=sanlock.ALIGN1M, sector=sanlock.SECTOR512)
     assert ls == {"iotimeout": 1, "lockspace": "name"}
 
     acquired = sanlock.inq_lockspace(
@@ -64,9 +67,12 @@ def test_write_resource(tmpdir, sanlock_daemon, size, offset):
     util.create_file(path, size)
     disks = [(path, offset)]
 
-    sanlock.write_resource("ls_name", "res_name", disks)
+    sanlock.write_resource(
+        "ls_name", "res_name", disks, align=sanlock.ALIGN1M,
+        sector=sanlock.SECTOR512)
 
-    res = sanlock.read_resource(path, offset=offset)
+    res = sanlock.read_resource(
+        path, offset=offset, align=sanlock.ALIGN1M, sector=sanlock.SECTOR512)
     assert res == {
         "lockspace": "ls_name",
         "resource": "res_name",
-- 
2.20.1

[PATCH v3 2/4] Accept values instead of flags for sector and aling parameters.

[Vojtech Juranek]

Currently appropriate sanlock flags are accepted for sector size and
alignment parameters. This can be confusing for users, which may try
to pass values instead of flags and this approach also exposes sanlock
internal implementation details.

In order to have consistent and more intuitive API, values should be
accepted for sector size and alignment parameters. This patch replaces
flags by values in calls which accept sector size and alignment.

This patch also adds validation of provided values. If the value is
not the supported one, error with error code EINVAL is thrown.

The supported values are:
sector: 512, 4096
align: 1048576, 2097152, 4194304, 8388608
---
 python/example.py    |   4 +-
 python/sanlock.c     | 144 +++++++++++++++++++++++++++++++++----------
 tests/python_test.py |  17 ++---
 3 files changed, 125 insertions(+), 40 deletions(-)

diff --git a/python/example.py b/python/example.py
index 4fe845d..cb8f3e9 100644
--- a/python/example.py
+++ b/python/example.py
@@ -29,10 +29,10 @@ def main():
     fd = sanlock.register()
 
     print "Initializing '%s'" % (LOCKSPACE_NAME,)
-    sanlock.write_lockspace(LOCKSPACE_NAME, disk, max_hosts=0, iotimeout=0,
align=sanlock.ALIGN1M, sector=sanlock.SECTOR512)
+    sanlock.write_lockspace(LOCKSPACE_NAME, disk, max_hosts=0, iotimeout=0,
align=1048576, sector=512)
 
     print "Initializing '%s' on '%s'" % (RESOURCE_NAME,
LOCKSPACE_NAME)
-    sanlock.write_resource(LOCKSPACE_NAME, RESOURCE_NAME, SNLK_DISKS,
align=sanlock.ALIGN1M, sector=sanlock.SECTOR512)
+    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)
diff --git a/python/sanlock.c b/python/sanlock.c
index 2ad4497..b57b51d 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -53,6 +53,7 @@ __set_exception(int en, char *msg)
     if (en < 0 && en > -200) {
         en = -en;
         err_name = strerror(en);
+
     } else {
         /* Safe to call without releasing the GIL. */
         err_name = sanlock_strerror(en);
@@ -135,6 +136,54 @@ exit_fail:
     return -1;
 }
 
+enum sector_size {SECTOR512 = 512, SECTOR4K = 4096};
+
+static uint32_t
+__sector_to_flag(int sector)
+{
+    uint32_t flag = 0;
+  
+    switch (sector) {
+    case SECTOR512:
+        flag = SANLK_LSF_SECTOR512;
+        break;
+    case SECTOR4K:
+        flag = SANLK_LSF_SECTOR4K;
+        break;
+    default:
+        __set_exception(EINVAL, "Invalid sector size value");
+    }
+
+    return flag;
+}
+
+enum alignment {ALIGN1M = 1048576, ALIGN2M = 2097152, ALIGN4M = 4194304, ALIGN8M =
8388608};
+
+static uint32_t
+__align_to_flag(long align)
+{
+    uint32_t flag = 0;
+  
+    switch (align) {
+    case ALIGN1M:
+        flag = SANLK_RES_ALIGN1M;
+        break;
+    case ALIGN2M:
+        flag = SANLK_RES_ALIGN2M;
+        break;
+    case ALIGN4M:
+        flag = SANLK_RES_ALIGN4M;
+        break;
+    case ALIGN8M:
+        flag = SANLK_RES_ALIGN8M;
+        break;
+    default:
+        __set_exception(EINVAL, "Invalid alignment value");
+    }
+
+    return flag;
+}
+
 static PyObject *
 __hosts_to_list(struct sanlk_host *hss, int hss_count)
 {
@@ -361,15 +410,17 @@ exit_fail:
 /* write_lockspace */
 PyDoc_STRVAR(pydoc_write_lockspace, "\
 write_lockspace(lockspace, path, offset=0, max_hosts=0, iotimeout=0, \
-align=ALIGN1M, sector=SECTOR512)\n\
-Initialize or update a device to be used as sanlock lockspace.");
+align=1048576, sector=512)\n\
+Initialize or update a device to be used as sanlock lockspace.\n\
+Align can be one of (1048576, 2097152, 4194304, 8388608).\n\
+Sector can be one of (512, 4096).");
 
 static PyObject *
 py_write_lockspace(PyObject *self __unused, PyObject *args, PyObject *keywds)
 {
-    int rv, max_hosts = 0;
-    uint32_t io_timeout = 0;
-    uint32_t  align=SANLK_LSF_ALIGN1M, sector=SANLK_LSF_SECTOR512;
+    int rv, max_hosts = 0, sector = SECTOR512;
+    long align = ALIGN1M;
+    uint32_t io_timeout = 0, flag = 0;
     const char *lockspace, *path;
     struct sanlk_lockspace ls;
 
@@ -380,7 +431,7 @@ py_write_lockspace(PyObject *self __unused, PyObject *args, PyObject
*keywds)
     memset(&ls, 0, sizeof(struct sanlk_lockspace));
 
     /* parse python tuple */
-    if (!PyArg_ParseTupleAndKeywords(args, keywds, "ss|kiIII", kwlist,
+    if (!PyArg_ParseTupleAndKeywords(args, keywds, "ss|kiIli", kwlist,
         &lockspace, &path, &ls.host_id_disk.offset, &max_hosts,
         &io_timeout, &align, &sector)) {
         return NULL;
@@ -391,9 +442,14 @@ py_write_lockspace(PyObject *self __unused, PyObject *args, PyObject
*keywds)
     strncpy(ls.host_id_disk.path, path, SANLK_PATH_LEN - 1);
 
     /* set alignment/sector flags */
-    ls.flags |= align;
-    ls.flags |= sector;
+    if ((flag = __align_to_flag(align)) == 0)
+        return NULL;
+    ls.flags |= flag;
 
+    if ((flag = __sector_to_flag(sector)) == 0)
+        return NULL;
+    ls.flags |= flag;
+    
     /* write sanlock lockspace (gil disabled) */
     Py_BEGIN_ALLOW_THREADS
     rv = sanlock_write_lockspace(&ls, max_hosts, 0, io_timeout);
@@ -409,15 +465,17 @@ py_write_lockspace(PyObject *self __unused, PyObject *args, PyObject
*keywds)
 
 /* read_lockspace */
 PyDoc_STRVAR(pydoc_read_lockspace, "\
-read_lockspace(path, offset=0, align=ALIGN1M, sector=SECTOR512)\n -> dict\n\
-Read the lockspace information from a device at a specific offset.");
+read_lockspace(path, offset=0, align=1048576, sector=512)\n -> dict\n\
+Read the lockspace information from a device at a specific offset.\n\
+Align can be one of (1048576, 2097152, 4194304, 8388608).\n\
+Sector can be one of (512, 4096).");
 
 static PyObject *
 py_read_lockspace(PyObject *self __unused, PyObject *args, PyObject *keywds)
 {
-    int rv;
-    uint32_t io_timeout = 0;
-    uint32_t  align=SANLK_LSF_ALIGN1M, sector=SANLK_LSF_SECTOR512;
+    int rv, sector = SECTOR512;
+    long align = ALIGN1M;
+    uint32_t io_timeout = 0, flag = 0;
     const char *path;
     struct sanlk_lockspace ls;
     PyObject *ls_info = NULL, *ls_entry = NULL;
@@ -428,7 +486,7 @@ py_read_lockspace(PyObject *self __unused, PyObject *args, PyObject
*keywds)
     memset(&ls, 0, sizeof(struct sanlk_lockspace));
 
     /* parse python tuple */
-    if (!PyArg_ParseTupleAndKeywords(args, keywds, "s|kII", kwlist,
+    if (!PyArg_ParseTupleAndKeywords(args, keywds, "s|kli", kwlist,
         &path, &ls.host_id_disk.offset, &align, &sector)) {
         return NULL;
     }
@@ -437,8 +495,13 @@ py_read_lockspace(PyObject *self __unused, PyObject *args, PyObject
*keywds)
     strncpy(ls.host_id_disk.path, path, SANLK_PATH_LEN - 1);
 
     /* set alignment/sector flags */
-    ls.flags |= align;
-    ls.flags |= sector;
+    if ((flag = __align_to_flag(align)) == 0)
+        return NULL;
+    ls.flags |= flag;
+
+    if ((flag = __sector_to_flag(sector)) == 0)
+        return NULL;
+    ls.flags |= flag;
 
     /* read sanlock lockspace (gil disabled) */
     Py_BEGIN_ALLOW_THREADS
@@ -481,14 +544,17 @@ exit_fail:
 
 /* read_resource */
 PyDoc_STRVAR(pydoc_read_resource, "\
-read_resource(path, offset=0, align=ALIGN1M, sector=SECTOR512) -> dict\n\
-Read the resource information from a device at a specific offset.");
+read_resource(path, offset=0, align=1048576, sector=512) -> dict\n\
+Read the resource information from a device at a specific offset.\n\
+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, rs_len;
-    uint32_t  align=SANLK_RES_ALIGN1M, sector=SANLK_RES_SECTOR512;
+    int rv, rs_len, sector = SECTOR512;
+    long align = ALIGN1M;
+    uint32_t flag = 0;
     const char *path;
     struct sanlk_resource *rs;
     PyObject *rs_info = NULL, *rs_entry = NULL;
@@ -509,7 +575,7 @@ py_read_resource(PyObject *self __unused, PyObject *args, PyObject
*keywds)
     rs->num_disks = 1;
 
     /* parse python tuple */
-    if (!PyArg_ParseTupleAndKeywords(args, keywds, "s|kII", kwlist,
+    if (!PyArg_ParseTupleAndKeywords(args, keywds, "s|kli", kwlist,
         &path, &(rs->disks[0].offset), &align, &sector)) {
         goto exit_fail;
     }
@@ -518,8 +584,15 @@ py_read_resource(PyObject *self __unused, PyObject *args, PyObject
*keywds)
     strncpy(rs->disks[0].path, path, SANLK_PATH_LEN - 1);
 
     /* set alignment/sector flags */
-    rs->flags |= align;
-    rs->flags |= sector;
+    if ((flag = __align_to_flag(align)) != 0)
+        rs->flags |= flag;
+    else
+        goto exit_fail;
+
+    if ((flag = __sector_to_flag(sector)) != 0)
+        rs->flags |= flag;
+    else
+        goto exit_fail;
 
     /* read sanlock resource (gil disabled) */
     Py_BEGIN_ALLOW_THREADS
@@ -573,27 +646,29 @@ exit_fail:
 /* write_resource */
 PyDoc_STRVAR(pydoc_write_resource, "\
 write_resource(lockspace, resource, disks, max_hosts=0, num_hosts=0, \
-clear=False, align=ALIGN1M, sector=SECTOR512)\n\
+clear=False, align=1048576, sector=512)\n\
 Initialize a device to be used as sanlock resource.\n\
 The disks must be in the format: [(path, offset), ... ].\n\
 If clear is True, the resource is cleared so subsequent read will\n\
-return an error.");
+return an error.\n\
+Align can be one of (1048576, 2097152, 4194304, 8388608).\n\
+Sector can be one of (512, 4096).");
 
 static PyObject *
 py_write_resource(PyObject *self __unused, PyObject *args, PyObject *keywds)
 {
-    int rv, max_hosts = 0, num_hosts = 0, clear = 0;
-    uint32_t  align=SANLK_RES_ALIGN1M, sector=SANLK_RES_SECTOR512;
+    int rv, max_hosts = 0, num_hosts = 0, clear = 0, sector = SECTOR512;
+    long align = ALIGN1M;
     const char *lockspace, *resource;
     struct sanlk_resource *rs;
     PyObject *disks;
-    uint32_t flags = 0;
+    uint32_t flags = 0, flag = 0;
 
     static char *kwlist[] = {"lockspace", "resource",
"disks", "max_hosts",
                                 "num_hosts", "clear",
"align", "sector", NULL};
 
     /* parse python tuple */
-    if (!PyArg_ParseTupleAndKeywords(args, keywds, "ssO!|iiiII",
+    if (!PyArg_ParseTupleAndKeywords(args, keywds, "ssO!|iiili",
         kwlist, &lockspace, &resource, &PyList_Type, &disks,
&max_hosts,
         &num_hosts, &clear, &align, &sector)) {
         return NULL;
@@ -609,8 +684,15 @@ py_write_resource(PyObject *self __unused, PyObject *args, PyObject
*keywds)
     strncpy(rs->name, resource, SANLK_NAME_LEN);
 
     /* set alignment/sector flags */
-    rs->flags |= align;
-    rs->flags |= sector;
+    if ((flag = __align_to_flag(align)) != 0)
+        rs->flags |= flag;
+    else
+        goto exit_fail;
+
+    if ((flag = __sector_to_flag(sector)) != 0)
+        rs->flags |= flag;
+    else
+        goto exit_fail;
 
     if (clear) {
         flags |= SANLK_WRITE_CLEAR;
diff --git a/tests/python_test.py b/tests/python_test.py
index 6393222..1bb714a 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -23,6 +23,9 @@ LARGE_FILE_SIZE = 1024**4
 LOCKSPACE_SIZE = 1024**2
 MIN_RES_SIZE = 1024**2
 
+ALIGNMENT_1M = 1024**2
+SECTOR_SIZE_512 = 512
+
 
 @pytest.mark.parametrize("size,offset", [
     # Smallest offset.
@@ -35,11 +38,11 @@ def test_write_lockspace(tmpdir, sanlock_daemon, size, offset):
     util.create_file(path, size)
 
     sanlock.write_lockspace(
-        "name", path, offset=offset, iotimeout=1, align=sanlock.ALIGN1M,
-        sector=sanlock.SECTOR512)
+        "name", path, offset=offset, iotimeout=1, align=ALIGNMENT_1M,
+        sector=SECTOR_SIZE_512)
 
-    ls = sanlock.read_lockspace(
-        path, offset=offset, align=sanlock.ALIGN1M, sector=sanlock.SECTOR512)
+    ls = sanlock.read_lockspace(path, offset=offset, align=ALIGNMENT_1M,
+                                sector=SECTOR_SIZE_512)
     assert ls == {"iotimeout": 1, "lockspace": "name"}
 
     acquired = sanlock.inq_lockspace(
@@ -68,11 +71,11 @@ def test_write_resource(tmpdir, sanlock_daemon, size, offset):
     disks = [(path, offset)]
 
     sanlock.write_resource(
-        "ls_name", "res_name", disks, align=sanlock.ALIGN1M,
-        sector=sanlock.SECTOR512)
+        "ls_name", "res_name", disks, align=ALIGNMENT_1M,
+        sector=SECTOR_SIZE_512)
 
     res = sanlock.read_resource(
-        path, offset=offset, align=sanlock.ALIGN1M, sector=sanlock.SECTOR512)
+        path, offset=offset, align=ALIGNMENT_1M, sector=SECTOR_SIZE_512)
     assert res == {
         "lockspace": "ls_name",
         "resource": "res_name",
-- 
2.20.1

[PATCH v3 3/4] Expose tuples with supported align and sector values

[Vojtech Juranek]

Previous patch switched from accepting align and sector flags to values.
Exposing internal sanlock flags doesn't make sense now. Tuples with
supported sector size and alignment values should be exposed instead.
---
 python/sanlock.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/python/sanlock.c b/python/sanlock.c
index b57b51d..6319931 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -1736,13 +1736,21 @@ initsanlock(void)
     PYSNLK_INIT_ADD_CONSTANT(SANLK_SETEV_REPLACE_EVENT, 
"SETEV_REPLACE_EVENT");
     PYSNLK_INIT_ADD_CONSTANT(SANLK_SETEV_ALL_HOSTS,      "SETEV_ALL_HOSTS");
 
-    /* Sector and align size flags */
-    PYSNLK_INIT_ADD_CONSTANT(SANLK_RES_SECTOR512, "SECTOR512");
-    PYSNLK_INIT_ADD_CONSTANT(SANLK_RES_SECTOR4K, "SECTOR4K");
-    PYSNLK_INIT_ADD_CONSTANT(SANLK_RES_ALIGN1M, "ALIGN1M");
-    PYSNLK_INIT_ADD_CONSTANT(SANLK_RES_ALIGN2M, "ALIGN2M");
-    PYSNLK_INIT_ADD_CONSTANT(SANLK_RES_ALIGN4M, "ALIGN4M");
-    PYSNLK_INIT_ADD_CONSTANT(SANLK_RES_ALIGN8M, "ALIGN8M");
-
 #undef PYSNLK_INIT_ADD_CONSTANT
+
+    /* Tuples with supported sector size and alignment values */
+    PyObject *sector = PyTuple_New(2);
+    PyTuple_SetItem(sector, 0, PyInt_FromLong(SECTOR512));
+    PyTuple_SetItem(sector, 1, PyInt_FromLong(SECTOR4K));
+    if (PyModule_AddObject(py_module, "SECTOR", sector))
+        Py_DECREF(sector);
+
+    PyObject *align = PyTuple_New(4);
+    PyTuple_SetItem(align, 0, PyInt_FromLong(ALIGN1M));
+    PyTuple_SetItem(align, 1, PyInt_FromLong(ALIGN2M));
+    PyTuple_SetItem(align, 2, PyInt_FromLong(ALIGN4M));
+    PyTuple_SetItem(align, 3, PyInt_FromLong(ALIGN8M));
+    if (PyModule_AddObject(py_module, "ALIGN", align))
+        Py_DECREF(align);
+
 }
-- 
2.20.1

[PATCH v3 4/4] tests: add tests for invalid align and sector values

[Vojtech Juranek]

---
 tests/python_test.py | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/tests/python_test.py b/tests/python_test.py
index 1bb714a..f822c3e 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -234,3 +234,41 @@ def test_acquire_release_resource(tmpdir, sanlock_daemon, size,
offset):
 
     owners = sanlock.read_resource_owners("ls_name", "res_name",
disks)
     assert owners == []
+
+
+(a)pytest.mark.parametrize("align, sector", [
+    # Invalid alignment
+    (1024, sanlock.SECTOR[0]),
+    # Invalid sector size
+    (sanlock.ALIGN[0], 8192),
+    # Invalid both alignment and sector size
+    (3145728, 8192),
+])
+def test_write_lockspace_invalid_align_sector(
+        tmpdir, sanlock_daemon, align, sector):
+    path = str(tmpdir.join("lockspace"))
+    util.create_file(path, LOCKSPACE_SIZE)
+
+    with pytest.raises(sanlock.SanlockException) as e:
+        sanlock.write_lockspace("name", path, align=align, sector=sector)
+    assert e.value.errno == errno.EINVAL
+
+
+(a)pytest.mark.parametrize("align, sector", [
+    # Invalid alignment
+    (1024, sanlock.SECTOR[0]),
+    # Invalid sector size
+    (sanlock.ALIGN[0], 8192),
+    # Invalid both alignment and sector size
+    (3145728, 8192),
+])
+def test_write_resource_invalid_align_sector(
+        tmpdir, sanlock_daemon, align, sector):
+    path = str(tmpdir.join("resources"))
+    util.create_file(path, MIN_RES_SIZE)
+    disks = [(path, 0)]
+
+    with pytest.raises(sanlock.SanlockException) as e:
+        sanlock.write_resource(
+            "ls_name", "res_name", disks, align=align,
sector=sector)
+    assert e.value.errno == errno.EINVAL
-- 
2.20.1

[PATCH v3 1/4] tests: add sector and align parameters

[Nir Soffer]

On Tue, Apr 30, 2019 at 5:34 PM Vojtech Juranek <vjuranek(a)redhat.com&gt; wrote:

>  Add sector and align parameters to read and write lockspace and
>  resource calls to test_write_lockspace and test_write_resource tests
>  to test that these parameters are accepted. Other tests still use
>  calls without these parameters to test the defaults.
>  
There are few problems with these tests:
- we change them in the next patch to use values
- adding them now makes it harder to deliver this in 3.7.1, requiring
  adding another patch in all fedora branches

Lets add the tests with the patch changing the API.

Nir



>  ---
>   tests/python_test.py | 14 ++++++++++----
>   1 file changed, 10 insertions(+), 4 deletions(-)
>
>  diff --git a/tests/python_test.py b/tests/python_test.py
>  index 5029924..6393222 100644
>  --- a/tests/python_test.py
>  +++ b/tests/python_test.py
>  @@ -34,9 +34,12 @@ def test_write_lockspace(tmpdir, sanlock_daemon, size,
>  offset):
>       path = str(tmpdir.join("lockspace"))
>       util.create_file(path, size)
>
>  -    sanlock.write_lockspace("name", path, offset=offset, iotimeout=1)
>  +    sanlock.write_lockspace(
>  +        "name", path, offset=offset, iotimeout=1, align=sanlock.ALIGN1M,
>  +        sector=sanlock.SECTOR512)
>
>  -    ls = sanlock.read_lockspace(path, offset=offset)
>  +    ls = sanlock.read_lockspace(
>  +        path, offset=offset, align=sanlock.ALIGN1M,
>  sector=sanlock.SECTOR512)
>       assert ls == {"iotimeout": 1, "lockspace": "name"}
>
>       acquired = sanlock.inq_lockspace(
>  @@ -64,9 +67,12 @@ def test_write_resource(tmpdir, sanlock_daemon, size,
>  offset):
>       util.create_file(path, size)
>       disks = [(path, offset)]
>
>  -    sanlock.write_resource("ls_name", "res_name", disks)
>  +    sanlock.write_resource(
>  +        "ls_name", "res_name", disks, align=sanlock.ALIGN1M,
>  +        sector=sanlock.SECTOR512)
>
>  -    res = sanlock.read_resource(path, offset=offset)
>  +    res = sanlock.read_resource(
>  +        path, offset=offset, align=sanlock.ALIGN1M,
>  sector=sanlock.SECTOR512)
>       assert res == {
>           "lockspace": "ls_name",
>           "resource": "res_name",
>  --
>  2.20.1
>  _______________________________________________
>  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]...
>

[PATCH v3 3/4] Expose tuples with supported align and sector values

[Nir Soffer]

On Tue, Apr 30, 2019 at 5:34 PM Vojtech Juranek <vjuranek(a)redhat.com&gt; wrote:

>  Previous patch switched from accepting align and sector flags to
> values.
>  Exposing internal sanlock flags doesn't make sense now. Tuples with
>  supported sector size and alignment values should be exposed instead.
>  ---
>   python/sanlock.c | 24 ++++++++++++++++--------
>   1 file changed, 16 insertions(+), 8 deletions(-)
>
>  diff --git a/python/sanlock.c b/python/sanlock.c
>  index b57b51d..6319931 100644
>  --- a/python/sanlock.c
>  +++ b/python/sanlock.c
>  @@ -1736,13 +1736,21 @@ initsanlock(void)
>       PYSNLK_INIT_ADD_CONSTANT(SANLK_SETEV_REPLACE_EVENT,
>  "SETEV_REPLACE_EVENT");
>       PYSNLK_INIT_ADD_CONSTANT(SANLK_SETEV_ALL_HOSTS,
>  "SETEV_ALL_HOSTS");
>
>  -    /* Sector and align size flags */
>  -    PYSNLK_INIT_ADD_CONSTANT(SANLK_RES_SECTOR512, "SECTOR512");
>  -    PYSNLK_INIT_ADD_CONSTANT(SANLK_RES_SECTOR4K, "SECTOR4K");
>  -    PYSNLK_INIT_ADD_CONSTANT(SANLK_RES_ALIGN1M, "ALIGN1M");
>  -    PYSNLK_INIT_ADD_CONSTANT(SANLK_RES_ALIGN2M, "ALIGN2M");
>  -    PYSNLK_INIT_ADD_CONSTANT(SANLK_RES_ALIGN4M, "ALIGN4M");
>  -    PYSNLK_INIT_ADD_CONSTANT(SANLK_RES_ALIGN8M, "ALIGN8M");
>  -
>   #undef PYSNLK_INIT_ADD_CONSTANT
>  +
>  +    /* Tuples with supported sector size and alignment values */
>  +    PyObject *sector = PyTuple_New(2);
>  +    PyTuple_SetItem(sector, 0, PyInt_FromLong(SECTOR512));
>  +    PyTuple_SetItem(sector, 1, PyInt_FromLong(SECTOR4K));
>  
This works, but creates more work for guys working on python 3, since in
python
3 we need to use PyLong_FromLong().

The best way to avoid these issues and to simplify the code is to use:

    PyObject *sector = Py_BuildValue("ii", SECTOR_SIZE_512, SECTOR_SIZE_4K);

See https://docs.python.org/2.7/c-api/arg.html#c.Py_BuildValue

Same for the coder building the align tuple.



>  +    if (PyModule_AddObject(py_module, "SECTOR", sector))
>  +        Py_DECREF(sector);
>  
If we failed to add the object, we should return here. The python import
will
probably fail with MemoryError.



>  +
>  +    PyObject *align = PyTuple_New(4);
>  +    PyTuple_SetItem(align, 0, PyInt_FromLong(ALIGN1M));
>  +    PyTuple_SetItem(align, 1, PyInt_FromLong(ALIGN2M));
>  +    PyTuple_SetItem(align, 2, PyInt_FromLong(ALIGN4M));
>  +    PyTuple_SetItem(align, 3, PyInt_FromLong(ALIGN8M));
>  +    if (PyModule_AddObject(py_module, "ALIGN", align))
>  +        Py_DECREF(align);
>  +
>   }
>  --
>  2.20.1
>  _______________________________________________
>  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]...
>  s
>

[PATCH v3 3/4] Expose tuples with supported align and sector values

[Nir Soffer]

On Tue, Apr 30, 2019 at 5:34 PM Vojtech Juranek <vjuranek(a)redhat.com&gt; wrote:

>  Previous patch switched from accepting align and sector flags to
> values.
>  Exposing internal sanlock flags doesn't make sense now. Tuples with
>  supported sector size and alignment values should be exposed instead.
>  ---
>   python/sanlock.c | 24 ++++++++++++++++--------
>   1 file changed, 16 insertions(+), 8 deletions(-)
>
>  diff --git a/python/sanlock.c b/python/sanlock.c
>  index b57b51d..6319931 100644
>  --- a/python/sanlock.c
>  +++ b/python/sanlock.c
>  @@ -1736,13 +1736,21 @@ initsanlock(void)
>       PYSNLK_INIT_ADD_CONSTANT(SANLK_SETEV_REPLACE_EVENT,
>  "SETEV_REPLACE_EVENT");
>       PYSNLK_INIT_ADD_CONSTANT(SANLK_SETEV_ALL_HOSTS,
>  "SETEV_ALL_HOSTS");
>
>  -    /* Sector and align size flags */
>  -    PYSNLK_INIT_ADD_CONSTANT(SANLK_RES_SECTOR512, "SECTOR512");
>  -    PYSNLK_INIT_ADD_CONSTANT(SANLK_RES_SECTOR4K, "SECTOR4K");
>  -    PYSNLK_INIT_ADD_CONSTANT(SANLK_RES_ALIGN1M, "ALIGN1M");
>  -    PYSNLK_INIT_ADD_CONSTANT(SANLK_RES_ALIGN2M, "ALIGN2M");
>  -    PYSNLK_INIT_ADD_CONSTANT(SANLK_RES_ALIGN4M, "ALIGN4M");
>  -    PYSNLK_INIT_ADD_CONSTANT(SANLK_RES_ALIGN8M, "ALIGN8M");
>  -
>   #undef PYSNLK_INIT_ADD_CONSTANT
>  +
>  +    /* Tuples with supported sector size and alignment values */
>  +    PyObject *sector = PyTuple_New(2);
>  +    PyTuple_SetItem(sector, 0, PyInt_FromLong(SECTOR512));
>  +    PyTuple_SetItem(sector, 1, PyInt_FromLong(SECTOR4K));
>  +    if (PyModule_AddObject(py_module, "SECTOR", sector))
>  +        Py_DECREF(sector);
>  
Another issue I forgot to mention, maybe use sanlock.ALIGN_SIZE and
sanlock.SECTOR_SIZE for the tuples?

This makes it more clear that it is not related to SANLK_XXX_ALIGNXX
and SANLK_XXX_SECTORXX flags.



>  +
>  +    PyObject *align = PyTuple_New(4);
>  +    PyTuple_SetItem(align, 0, PyInt_FromLong(ALIGN1M));
>  +    PyTuple_SetItem(align, 1, PyInt_FromLong(ALIGN2M));
>  +    PyTuple_SetItem(align, 2, PyInt_FromLong(ALIGN4M));
>  +    PyTuple_SetItem(align, 3, PyInt_FromLong(ALIGN8M));
>  +    if (PyModule_AddObject(py_module, "ALIGN", align))
>  +        Py_DECREF(align);
>  +
>   }
>  --
>  2.20.1
>  _______________________________________________
>  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]...
>