[PATCH] python: Fix issues introduced in latest 4k API changes

[Nir Soffer]

Fix few issues in the latests 4k API changes:
- Fix ValueError when using align=8388608 - a break was missing.
- Remove the unneeded ALIGNX and SECTORX constants - seems that David
  merged an older version of the patch which did not remove them.
- Fix capitalization and punctuation in new comments.
- Remove trailing whitespace.
- Wrap long enum difinition.

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

diff --git a/python/sanlock.c b/python/sanlock.c
index a1215bd..d6841e8 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -152,11 +152,16 @@ add_sector_flag(int sector, uint32_t *flags)
 	return -1;
     }
     return 0;
 }
 
-enum {ALIGNMENT_1M = 1048576, ALIGNMENT_2M = 2097152, ALIGNMENT_4M = 4194304,
ALIGNMENT_8M = 8388608};
+enum {
+    ALIGNMENT_1M = 1048576,
+    ALIGNMENT_2M = 2097152,
+    ALIGNMENT_4M = 4194304,
+    ALIGNMENT_8M = 8388608
+};
 
 static int
 add_align_flag(long align, uint32_t *flags)
 {
     switch (align) {
@@ -169,10 +174,11 @@ add_align_flag(long align, uint32_t *flags)
     case ALIGNMENT_4M:
         *flags |= SANLK_RES_ALIGN4M;
 	break;
     case ALIGNMENT_8M:
         *flags |= SANLK_RES_ALIGN8M;
+	break;
     default:
 	PyErr_Format(PyExc_ValueError, "Invalid align value: %ld", align);
 	return -1;
     }
     return 0;
@@ -439,11 +445,11 @@ py_write_lockspace(PyObject *self __unused, PyObject *args, PyObject
*keywds)
     if (add_align_flag(align, &ls.flags) == -1)
         return NULL;
 
     if (add_sector_flag(sector, &ls.flags) == -1)
         return NULL;
-    
+
     /* write sanlock lockspace (gil disabled) */
     Py_BEGIN_ALLOW_THREADS
     rv = sanlock_write_lockspace(&ls, max_hosts, 0, io_timeout);
     Py_END_ALLOW_THREADS
 
@@ -1718,29 +1724,21 @@ initsanlock(void)
     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");
 
-    /* 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 = Py_BuildValue("ii", SECTOR_SIZE_512, SECTOR_SIZE_4K);
     if (!sector)
         return;
     if (PyModule_AddObject(py_module, "SECTOR_SIZE", sector)) {
         Py_DECREF(sector);
         return;
     }
-      
+
     PyObject *align = Py_BuildValue("llll", ALIGNMENT_1M, ALIGNMENT_2M,
ALIGNMENT_4M, ALIGNMENT_8M);
     if (!align)
       return;
     if (PyModule_AddObject(py_module, "ALIGN_SIZE", align)) {
         Py_DECREF(align);
diff --git a/tests/python_test.py b/tests/python_test.py
index a0474f1..1a6bbbd 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -35,17 +35,17 @@ SECTOR_SIZE_512 = 512
 ])
 def test_write_lockspace(tmpdir, sanlock_daemon, size, offset):
     path = str(tmpdir.join("lockspace"))
     util.create_file(path, size)
 
-    # test read and write with default alignment and sector size values
+    # 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"}
 
-    # test read and write with explicit alignment and sector size values
+    # 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)
 
     ls = sanlock.read_lockspace(
@@ -75,21 +75,21 @@ def test_write_lockspace(tmpdir, sanlock_daemon, size, offset):
 def test_write_resource(tmpdir, sanlock_daemon, size, offset):
     path = str(tmpdir.join("resources"))
     util.create_file(path, size)
     disks = [(path, offset)]
 
-    # test read and write with default alignment and sector size values
+    # 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",
         "version": 0
     }
 
-    # test read and write with explicit alignment and sector size values
+    # Test read and write with explicit alignment and sector size values.
     sanlock.write_resource(
         "ls_name", "res_name", disks, align=ALIGNMENT_1M,
         sector=SECTOR_SIZE_512)
 
     res = sanlock.read_resource(
-- 
2.17.2

[Vojtech Juranek]

On neděle 5. května 2019 14:33:58 CEST Nir Soffer wrote:
>  Fix few issues in the latests 4k API changes:
>  - Fix ValueError when using align=8388608 - a break was missing.
>  - Remove the unneeded ALIGNX and SECTORX constants - seems that David
>    merged an older version of the patch which did not remove them.
>  - Fix capitalization and punctuation in new comments.
>  - Remove trailing whitespace.
>  - Wrap long enum difinition. 

thanks for fixing my issues!

>  Signed-off-by: Nir Soffer <nsoffer(a)redhat.com&gt;
>  ---
>   python/sanlock.c     | 20 +++++++++-----------
>   tests/python_test.py |  8 ++++----
>   2 files changed, 13 insertions(+), 15 deletions(-)
>  
>  diff --git a/python/sanlock.c b/python/sanlock.c
>  index a1215bd..d6841e8 100644
>  --- a/python/sanlock.c
>  +++ b/python/sanlock.c
>  @@ -152,11 +152,16 @@ add_sector_flag(int sector, uint32_t *flags)
>   	return -1;
>       }
>       return 0;
>   }
>   
>  -enum {ALIGNMENT_1M = 1048576, ALIGNMENT_2M = 2097152, ALIGNMENT_4M =
>  4194304, ALIGNMENT_8M = 8388608}; 
 +enum {
>  +    ALIGNMENT_1M = 1048576,
>  +    ALIGNMENT_2M = 2097152,
>  +    ALIGNMENT_4M = 4194304,
>  +    ALIGNMENT_8M = 8388608
>  +};
>   
>   static int
>   add_align_flag(long align, uint32_t *flags)
>   {
>       switch (align) {
>  @@ -169,10 +174,11 @@ add_align_flag(long align, uint32_t *flags)
>       case ALIGNMENT_4M:
>           *flags |= SANLK_RES_ALIGN4M;
>   	break;
>       case ALIGNMENT_8M:
>           *flags |= SANLK_RES_ALIGN8M;
>  +	break;
>       default:
>   	PyErr_Format(PyExc_ValueError, "Invalid align value: %ld", align);
>   	return -1;
>       }
>       return 0;
>  @@ -439,11 +445,11 @@ py_write_lockspace(PyObject *self __unused, PyObject
>  *args, PyObject *keywds) 
 if (add_align_flag(align, &ls.flags) == -1)
>           return NULL;
>   
>       if (add_sector_flag(sector, &ls.flags) == -1)
>           return NULL;
>  -    
>  +
>       /* write sanlock lockspace (gil disabled) */
>       Py_BEGIN_ALLOW_THREADS
>       rv = sanlock_write_lockspace(&ls, max_hosts, 0, io_timeout);
>       Py_END_ALLOW_THREADS
>   
>  @@ -1718,29 +1724,21 @@ initsanlock(void)
>       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"); 
>  -    /* 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 = Py_BuildValue("ii", SECTOR_SIZE_512,
>  SECTOR_SIZE_4K); 
 if (!sector)
>           return;
>       if (PyModule_AddObject(py_module, "SECTOR_SIZE", sector)) {
>           Py_DECREF(sector);
>           return;
>       }
>  -      
>  +
>       PyObject *align = Py_BuildValue("llll", ALIGNMENT_1M, ALIGNMENT_2M,
>  ALIGNMENT_4M, ALIGNMENT_8M); 
 if (!align)
>         return;
>       if (PyModule_AddObject(py_module, "ALIGN_SIZE", align)) {
>           Py_DECREF(align);
>  diff --git a/tests/python_test.py b/tests/python_test.py
>  index a0474f1..1a6bbbd 100644
>  --- a/tests/python_test.py
>  +++ b/tests/python_test.py
>  @@ -35,17 +35,17 @@ SECTOR_SIZE_512 = 512
>   ])
>   def test_write_lockspace(tmpdir, sanlock_daemon, size, offset):
>       path = str(tmpdir.join("lockspace"))
>       util.create_file(path, size)
>   
>  -    # test read and write with default alignment and sector size values
>  +    # 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"}
>   
>  -    # test read and write with explicit alignment and sector size values
>  +    # 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)
>   
>       ls = sanlock.read_lockspace(
>  @@ -75,21 +75,21 @@ def test_write_lockspace(tmpdir, sanlock_daemon, size,
>  offset): 
 def test_write_resource(tmpdir, sanlock_daemon, size, offset):
>       path = str(tmpdir.join("resources"))
>       util.create_file(path, size)
>       disks = [(path, offset)]
>   
>  -    # test read and write with default alignment and sector size values
>  +    # 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",
>           "version": 0
>       }
>   
>  -    # test read and write with explicit alignment and sector size values
>  +    # Test read and write with explicit alignment and sector size values.
>       sanlock.write_resource(
>           "ls_name", "res_name", disks, align=ALIGNMENT_1M,
>           sector=SECTOR_SIZE_512)
>   
>       res = sanlock.read_resource(
>  -- 
>  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