From: Amit Bawer <abawer(a)redhat.com&gt; This patch is a preparation for next changes in sanlock where lockspace and resource names will be parsed as bytes in both python versions and also as ascii string in python 2 for back compatibility. Implications: - Py2: no difference, ascii strings and bytes are parsed the same - Py3: marked as xfail as currently expecting only unicode string --- tests/python_test.py | 138 ++++++++++++++++++++++++------------------- 1 file changed, 78 insertions(+), 60 deletions(-) diff --git a/tests/python_test.py b/tests/python_test.py index db77862..f831c15 100644 --- a/tests/python_test.py +++ b/tests/python_test.py @@ -64,10 +64,11 @@ LOCKSPACE_OR_RESOURCE_NAMES = [ pytest.param( u"ascii", marks=pytest.mark.skipif(six.PY3, reason="python 3 supports only bytes")), ] +(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet") @pytest.mark.parametrize("filename, encoding" , FILE_NAMES) @pytest.mark.parametrize("size,offset", [ # Smallest offset. (LOCKSPACE_SIZE, 0), # Large offset. @@ -76,26 +77,26 @@ LOCKSPACE_OR_RESOURCE_NAMES = [ 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) + sanlock.write_lockspace(b"ls_name", path, offset=offset, iotimeout=1) ls = sanlock.read_lockspace(path, offset=offset) - assert ls == {"iotimeout": 1, "lockspace": b"name"} + assert ls == {"iotimeout": 1, "lockspace": b"ls_name"} # Test read and write with explicit alignment and sector size values. sanlock.write_lockspace( - "name", path, offset=offset, iotimeout=1, align=ALIGNMENT_1M, + b"ls_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": b"name"} + assert ls == {"iotimeout": 1, "lockspace": b"ls_name"} acquired = sanlock.inq_lockspace( - "name", 1, path, offset=offset, wait=False) + b"ls_name", 1, path, offset=offset, wait=False) assert acquired is False with io.open(path, "rb") as f: f.seek(offset) magic, = struct.unpack("< I", f.read(4)) @@ -104,28 +105,29 @@ def test_write_lockspace(tmpdir, sanlock_daemon, filename, encoding, size, offse # TODO: check more stuff here... util.check_guard(path, size) +(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet") @pytest.mark.parametrize("align", sanlock.ALIGN_SIZE) def test_write_lockspace_4k(user_4k_path, sanlock_daemon, align): # Poison lockspace area, ensuring that previous tests will not break this # test, and sanlock does not write beyond the lockspace area. with io.open(user_4k_path, "rb+") as f: f.write(align * b"x") util.write_guard(user_4k_path, align) sanlock.write_lockspace( - "name", user_4k_path, iotimeout=1, align=align, sector=SECTOR_SIZE_4K) + b"ls_name", user_4k_path, iotimeout=1, align=align, sector=SECTOR_SIZE_4K) ls = sanlock.read_lockspace( user_4k_path, align=align, sector=SECTOR_SIZE_4K) - assert ls == {"iotimeout": 1, "lockspace": b"name"} + assert ls == {"iotimeout": 1, "lockspace": b"ls_name"} - acquired = sanlock.inq_lockspace("name", 1, user_4k_path, wait=False) + acquired = sanlock.inq_lockspace(b"ls_name", 1, user_4k_path, wait=False) assert acquired is False # Verify that lockspace was written. with io.open(user_4k_path, "rb") as f: magic, = struct.unpack("< I", f.read(4)) @@ -133,26 +135,29 @@ def test_write_lockspace_4k(user_4k_path, sanlock_daemon, align): # Check that sanlock did not write beyond the lockspace area. util.check_guard(user_4k_path, align) +(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet") def test_write_lockspace_4k_invalid_sector_size(sanlock_daemon, user_4k_path): with pytest.raises(sanlock.SanlockException) as e: sanlock.write_lockspace( - "name", user_4k_path, iotimeout=1, sector=SECTOR_SIZE_512) + b"ls_name", user_4k_path, iotimeout=1, sector=SECTOR_SIZE_512) assert e.value.errno == errno.EINVAL +(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet") def test_read_lockspace_4k_invalid_sector_size(sanlock_daemon, user_4k_path): sanlock.write_lockspace( - "name", user_4k_path, iotimeout=1, sector=SECTOR_SIZE_4K) + b"ls_name", user_4k_path, iotimeout=1, sector=SECTOR_SIZE_4K) 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.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet") @pytest.mark.parametrize("filename,encoding", FILE_NAMES) @pytest.mark.parametrize("size,offset", [ # Smallest offset. (MIN_RES_SIZE, 0), # Large offset. @@ -162,33 +167,33 @@ 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) + sanlock.write_resource(b"ls_name", b"res_name", disks) res = sanlock.read_resource(path, offset=offset) assert res == { "lockspace": b"ls_name", "resource": b"res_name", "version": 0 } # Test read and write with explicit alignment and sector size values. sanlock.write_resource( - "ls_name", "res_name", disks, align=ALIGNMENT_1M, + b"ls_name", b"res_name", disks, align=ALIGNMENT_1M, sector=SECTOR_SIZE_512) res = sanlock.read_resource( path, offset=offset, align=ALIGNMENT_1M, sector=SECTOR_SIZE_512) assert res == { "lockspace": b"ls_name", "resource": b"res_name", "version": 0 } - owners = sanlock.read_resource_owners("ls_name", "res_name", disks) + owners = sanlock.read_resource_owners(b"ls_name", b"res_name", disks) assert owners == [] with io.open(path, "rb") as f: f.seek(offset) magic, = struct.unpack("< I", f.read(4)) @@ -197,10 +202,11 @@ def test_write_resource(tmpdir, sanlock_daemon, filename, encoding, size, offset # TODO: check more stuff here... util.check_guard(path, size) +(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet") @pytest.mark.parametrize("align", sanlock.ALIGN_SIZE) def test_write_resource_4k(sanlock_daemon, user_4k_path, align): disks = [(user_4k_path, 0)] # Poison resource area, ensuring that previous tests will not break this @@ -208,11 +214,11 @@ def test_write_resource_4k(sanlock_daemon, user_4k_path, align): with io.open(user_4k_path, "rb+") as f: f.write(align * b"x") util.write_guard(user_4k_path, align) sanlock.write_resource( - "ls_name", "res_name", disks, align=align, sector=SECTOR_SIZE_4K) + b"ls_name", b"res_name", disks, align=align, sector=SECTOR_SIZE_4K) res = sanlock.read_resource( user_4k_path, align=align, sector=SECTOR_SIZE_4K) assert res == { @@ -220,11 +226,11 @@ def test_write_resource_4k(sanlock_daemon, user_4k_path, align): "resource": b"res_name", "version": 0 } owners = sanlock.read_resource_owners( - "ls_name", "res_name", disks, align=align, sector=SECTOR_SIZE_4K) + b"ls_name", b"res_name", disks, align=align, sector=SECTOR_SIZE_4K) assert owners == [] # Verify that resource was written. with io.open(user_4k_path, "rb") as f: magic, = struct.unpack("< I", f.read(4)) @@ -238,90 +244,94 @@ def test_write_resource_4k(sanlock_daemon, user_4k_path, align): def test_write_resource_4k_invalid_sector_size(sanlock_daemon, user_4k_path): disks = [(user_4k_path, 0)] with pytest.raises(sanlock.SanlockException) as e: sanlock.write_resource( - "ls_name", "res_name", disks, sector=SECTOR_SIZE_512) + b"ls_name", b"res_name", disks, sector=SECTOR_SIZE_512) assert e.value.errno == errno.EINVAL +(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet") def test_read_resource_4k_invalid_sector_size(sanlock_daemon, user_4k_path): disks = [(user_4k_path, 0)] sanlock.write_resource( - "ls_name", - "res_name", + b"ls_name", + b"res_name", disks, align=ALIGNMENT_1M, sector=SECTOR_SIZE_4K) with pytest.raises(sanlock.SanlockException) as e: sanlock.read_resource(user_4k_path, sector=SECTOR_SIZE_512) assert e.value.errno == errno.EINVAL +(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet") def test_read_resource_owners_4k_invalid_sector_size( sanlock_daemon, user_4k_path): disks = [(user_4k_path, 0)] sanlock.write_resource( - "ls_name", - "res_name", + b"ls_name", + b"res_name", disks, align=ALIGNMENT_1M, sector=SECTOR_SIZE_4K) with pytest.raises(sanlock.SanlockException) as e: sanlock.read_resource_owners( - "ls_name", "res_name", disks, sector=SECTOR_SIZE_512) + b"ls_name", b"res_name", disks, sector=SECTOR_SIZE_512) assert e.value.errno == errno.EINVAL +(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet") def test_read_resource_owners_invalid_align_size(tmpdir, sanlock_daemon): path = str(tmpdir.join("path")) util.create_file(path, GiB) disks = [(path, 0)] sanlock.write_resource( - "ls_name", - "res_name", + b"ls_name", + b"res_name", disks, align=ALIGNMENT_1M, sector=SECTOR_SIZE_512) with pytest.raises(sanlock.SanlockException) as e: sanlock.read_resource_owners( - "ls_name", - "res_name", + b"ls_name", + b"res_name", disks, align=ALIGNMENT_2M, sector=SECTOR_SIZE_512) assert e.value.errno == errno.EINVAL +(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet") @pytest.mark.parametrize("size,offset", [ # Smallest offset. (MIN_RES_SIZE, 0), # Large offset. (LARGE_FILE_SIZE, LARGE_FILE_SIZE - MIN_RES_SIZE), ]) def test_add_rem_lockspace(tmpdir, sanlock_daemon, size, offset): path = str(tmpdir.join("ls_name")) util.create_file(path, size) - sanlock.write_lockspace("ls_name", path, offset=offset, iotimeout=1) + sanlock.write_lockspace(b"ls_name", path, offset=offset, iotimeout=1) # Since the lockspace is not acquired, we exepect to get False. acquired = sanlock.inq_lockspace( - "ls_name", 1, path, offset=offset, wait=False) + b"ls_name", 1, path, offset=offset, wait=False) assert acquired is False - sanlock.add_lockspace("ls_name", 1, path, offset=offset, iotimeout=1) + sanlock.add_lockspace(b"ls_name", 1, path, offset=offset, iotimeout=1) # Once the lockspace is acquired, we exepect to get True. acquired = sanlock.inq_lockspace( - "ls_name", 1, path, offset=offset, wait=False) + b"ls_name", 1, path, offset=offset, wait=False) assert acquired is True lockspaces = sanlock.get_lockspaces() assert lockspaces == [{ 'flags': 0, @@ -329,57 +339,59 @@ def test_add_rem_lockspace(tmpdir, sanlock_daemon, size, offset): 'lockspace': b'ls_name', 'offset': offset, 'path': path }] - sanlock.rem_lockspace("ls_name", 1, path, offset=offset) + sanlock.rem_lockspace(b"ls_name", 1, path, offset=offset) # Once the lockspace is released, we exepect to get False. acquired = sanlock.inq_lockspace( - "ls_name", 1, path, offset=offset, wait=False) + b"ls_name", 1, path, offset=offset, wait=False) assert acquired is False lockspaces = sanlock.get_lockspaces() assert lockspaces == [] +(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet") def test_add_rem_lockspace_async(tmpdir, sanlock_daemon): path = str(tmpdir.join("ls_name")) util.create_file(path, MiB) - sanlock.write_lockspace("ls_name", path, iotimeout=1) - acquired = sanlock.inq_lockspace("ls_name", 1, path, wait=False) + sanlock.write_lockspace(b"ls_name", path, iotimeout=1) + acquired = sanlock.inq_lockspace(b"ls_name", 1, path, wait=False) assert acquired is False # This will take 3 seconds. - sanlock.add_lockspace("ls_name", 1, path, iotimeout=1, **{"async": True}) + sanlock.add_lockspace(b"ls_name", 1, path, iotimeout=1, **{"async": True}) # While the lockspace is being aquired, we expect to get None. time.sleep(1) - acquired = sanlock.inq_lockspace("ls_name", 1, path, wait=False) + acquired = sanlock.inq_lockspace(b"ls_name", 1, path, wait=False) assert acquired is None # Once the lockspace is acquired, we exepect to get True. - acquired = sanlock.inq_lockspace("ls_name", 1, path, wait=True) + acquired = sanlock.inq_lockspace(b"ls_name", 1, path, wait=True) assert acquired is True # This will take about 3 seconds. - sanlock.rem_lockspace("ls_name", 1, path, **{"async": True}) + sanlock.rem_lockspace(b"ls_name", 1, path, **{"async": True}) # Wait until the lockspace change state from True to None. - while sanlock.inq_lockspace("ls_name", 1, path, wait=False): + while sanlock.inq_lockspace(b"ls_name", 1, path, wait=False): time.sleep(1) # While the lockspace is being released, we expect to get None. - acquired = sanlock.inq_lockspace("ls_name", 1, path, wait=False) + acquired = sanlock.inq_lockspace(b"ls_name", 1, path, wait=False) assert acquired is None # Once the lockspace was released, we expect to get False. - acquired = sanlock.inq_lockspace("ls_name", 1, path, wait=True) + acquired = sanlock.inq_lockspace(b"ls_name", 1, path, wait=True) assert acquired is False +(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet") @pytest.mark.parametrize("size,offset", [ # Smallest offset. (MIN_RES_SIZE, 0), # Large offset. (LARGE_FILE_SIZE, LARGE_FILE_SIZE - MIN_RES_SIZE), @@ -389,70 +401,71 @@ def test_acquire_release_resource(tmpdir, sanlock_daemon, size, offset): util.create_file(ls_path, size) res_path = str(tmpdir.join("res_name")) util.create_file(res_path, size) - sanlock.write_lockspace("ls_name", ls_path, offset=offset, iotimeout=1) - sanlock.add_lockspace("ls_name", 1, ls_path, offset=offset, iotimeout=1) + sanlock.write_lockspace(b"ls_name", ls_path, offset=offset, iotimeout=1) + sanlock.add_lockspace(b"ls_name", 1, ls_path, offset=offset, iotimeout=1) # Host status is not available until the first renewal. with pytest.raises(sanlock.SanlockException) as e: - sanlock.get_hosts("ls_name", 1) + sanlock.get_hosts(b"ls_name", 1) assert e.value.errno == errno.EAGAIN time.sleep(1) - host = sanlock.get_hosts("ls_name", 1)[0] + host = sanlock.get_hosts(b"ls_name", 1)[0] assert host["flags"] == sanlock.HOST_LIVE disks = [(res_path, offset)] - sanlock.write_resource("ls_name", "res_name", disks) + sanlock.write_resource(b"ls_name", b"res_name", disks) res = sanlock.read_resource(res_path, offset=offset) assert res == { "lockspace": b"ls_name", "resource": b"res_name", "version": 0 } - owners = sanlock.read_resource_owners("ls_name", "res_name", disks) + owners = sanlock.read_resource_owners(b"ls_name", b"res_name", disks) assert owners == [] fd = sanlock.register() - sanlock.acquire("ls_name", "res_name", disks, slkfd=fd) + sanlock.acquire(b"ls_name", b"res_name", disks, slkfd=fd) res = sanlock.read_resource(res_path, offset=offset) assert res == { "lockspace": b"ls_name", "resource": b"res_name", "version": 1 } - owner = sanlock.read_resource_owners("ls_name", "res_name", disks)[0] + owner = sanlock.read_resource_owners(b"ls_name", b"res_name", disks)[0] assert owner["host_id"] == 1 assert owner["flags"] == 0 assert owner["generation"] == 1 assert owner["io_timeout"] == 0 # Why 0? # TODO: check timestamp. - host = sanlock.get_hosts("ls_name", 1)[0] + host = sanlock.get_hosts(b"ls_name", 1)[0] assert host["flags"] == sanlock.HOST_LIVE assert host["generation"] == owner["generation"] - sanlock.release("ls_name", "res_name", disks, slkfd=fd) + sanlock.release(b"ls_name", b"res_name", disks, slkfd=fd) res = sanlock.read_resource(res_path, offset=offset) assert res == { "lockspace": b"ls_name", "resource": b"res_name", "version": 1 } - owners = sanlock.read_resource_owners("ls_name", "res_name", disks) + owners = sanlock.read_resource_owners(b"ls_name", b"res_name", disks) assert owners == [] +(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet") @pytest.mark.parametrize("align, sector", [ # Invalid alignment (KiB, sanlock.SECTOR_SIZE[0]), # Invalid sector size (sanlock.ALIGN_SIZE[0], 8 * KiB), @@ -461,13 +474,14 @@ 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(ValueError): - sanlock.write_lockspace("name", path, align=align, sector=sector) + sanlock.write_lockspace(b"ls_name", path, align=align, sector=sector) +(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet") @pytest.mark.parametrize("align, sector", [ # Invalid alignment (KiB, sanlock.SECTOR_SIZE[0]), # Invalid sector size (sanlock.ALIGN_SIZE[0], 8 * KiB), @@ -478,13 +492,14 @@ def test_write_resource_invalid_align_sector( util.create_file(path, MIN_RES_SIZE) disks = [(path, 0)] with pytest.raises(ValueError): sanlock.write_resource( - "ls_name", "res_name", disks, align=align, sector=sector) + b"ls_name", b"res_name", disks, align=align, sector=sector) +(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet") @pytest.mark.parametrize("disk", [ # Not a tuple - unicode and bytes: "not a tuple", b"not a tuple", u'\u05e9\u05dc\u05d5\u05dd', @@ -499,11 +514,11 @@ def test_write_resource_invalid_align_sector( ]) 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) + sanlock.write_resource(b"ls_name", b"res_name", disks) assert repr(disk) in str(e.value) @pytest.mark.parametrize("filename,encoding", FILE_NAMES) def test_killpath(tmpdir, sanlock_daemon, filename, encoding): @@ -535,35 +550,38 @@ def test_add_lockspace_parse_args(no_sanlock_daemon, name): def test_write_lockspace_parse_args(no_sanlock_daemon, name): with raises_sanlock_errno(): sanlock.write_lockspace(name, "ls_path") +(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet") @pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES) def test_write_resource_parse_args(no_sanlock_daemon, name): with raises_sanlock_errno(): - sanlock.write_resource(name, "res_name", [("disk_path",0)]) + sanlock.write_resource(name, b"res_name", [("disk_path",0)]) with raises_sanlock_errno(): - sanlock.write_resource("ls_name", name, [("disk_path",0)]) + sanlock.write_resource(b"ls_name", name, [("disk_path",0)]) +(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet") @pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES) def test_release_resource_parse_args(no_sanlock_daemon, name): with raises_sanlock_errno(): - sanlock.release(name, "res_name", [("disk_path",0)]) + sanlock.release(name, b"res_name", [("disk_path",0)]) with raises_sanlock_errno(): - sanlock.release("ls_name", name, [("disk_path",0)]) + sanlock.release(b"ls_name", name, [("disk_path",0)]) +(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet") @pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES) def test_read_resource_owners_parse_args(no_sanlock_daemon, name): with raises_sanlock_errno(): - sanlock.read_resource_owners(name, "res_name", [("disk_path",0)]) + sanlock.read_resource_owners(name, b"res_name", [("disk_path",0)]) with raises_sanlock_errno(): - sanlock.read_resource_owners("ls_name", name, [("disk_path",0)]) + sanlock.read_resource_owners(b"ls_name", name, [("disk_path",0)]) @pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES) def test_get_hosts_parse_args(no_sanlock_daemon, name): with raises_sanlock_errno(): -- 2.17.2

From: Amit Bawer <abawer(a)redhat.com&gt; This API was deprecated by write_lockspace but we want the lockspace/resource name as bytes change to be uniform so we modify it as well for sake of old use cases. Also added stub test for argument parsing. Implications: - Py2 : no difference - Py3 : if used, only accept bytes for lockspace name. --- python/sanlock.c | 19 ++++++++++++------- tests/python_test.py | 7 +++++++ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/python/sanlock.c b/python/sanlock.c index 86aec94..f73c507 100644 --- a/python/sanlock.c +++ b/python/sanlock.c @@ -354,41 +354,46 @@ use_aio=True)\n\ Initialize a device to be used as sanlock lockspace."); static PyObject * py_init_lockspace(PyObject *self __unused, PyObject *args, PyObject *keywds) { - int rv, max_hosts = 0, num_hosts = 0, use_aio = 1; - const char *lockspace, *path; + int rv = -1, max_hosts = 0, num_hosts = 0, use_aio = 1; + PyObject *lockspace = NULL; + const char *path; struct sanlk_lockspace ls; static char *kwlist[] = {"lockspace", "path", "offset", "max_hosts", "num_hosts", "use_aio", NULL}; /* initialize lockspace structure */ memset(&ls, 0, sizeof(struct sanlk_lockspace)); /* parse python tuple */ - if (!PyArg_ParseTupleAndKeywords(args, keywds, "ss|kiii", kwlist, - &lockspace, &path, &ls.host_id_disk.offset, &max_hosts, + if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&s|kiii", kwlist, + convert_to_pybytes, &lockspace, &path, &ls.host_id_disk.offset, &max_hosts, &num_hosts, &use_aio)) { - return NULL; + goto finally; } /* prepare sanlock names */ - strncpy(ls.name, lockspace, SANLK_NAME_LEN); + strncpy(ls.name, PyBytes_AsString(lockspace), SANLK_NAME_LEN); strncpy(ls.host_id_disk.path, path, SANLK_PATH_LEN - 1); /* init sanlock lockspace (gil disabled) */ Py_BEGIN_ALLOW_THREADS rv = sanlock_direct_init(&ls, NULL, max_hosts, num_hosts, use_aio); Py_END_ALLOW_THREADS if (rv != 0) { __set_exception(rv, "Sanlock lockspace init failure"); - return NULL; + goto finally; } +finally: + Py_XDECREF(lockspace); + if (rv != 0) + return NULL; Py_RETURN_NONE; } /* init_resource */ PyDoc_STRVAR(pydoc_init_resource, "\ diff --git a/tests/python_test.py b/tests/python_test.py index f831c15..0d97cb9 100644 --- a/tests/python_test.py +++ b/tests/python_test.py @@ -597,9 +597,16 @@ def test_inq_lockspace_parse_args(no_sanlock_daemon, name): @pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES) def test_reg_event_parse_args(no_sanlock_daemon, name): with raises_sanlock_errno(): sanlock.reg_event(name) + @pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES) def test_end_event_parse_args(no_sanlock_daemon, name): with raises_sanlock_errno(errno.EALREADY): sanlock.end_event(-1, name) + + +(a)pytest.mark.parametrize(&quot;name&quot;, LOCKSPACE_OR_RESOURCE_NAMES) +def test_init_lockspace_parse_args(no_sanlock_daemon, name): + with raises_sanlock_errno(errno.ENODEV): + sanlock.init_lockspace(name, "path") -- 2.17.2

From: Amit Bawer <abawer(a)redhat.com&gt; Implications: - Py2 : no difference - Py3 : if used, only accept bytes for lockspace name. --- python/sanlock.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/python/sanlock.c b/python/sanlock.c index 3d0bfb0..493608c 100644 --- a/python/sanlock.c +++ b/python/sanlock.c @@ -458,50 +458,55 @@ 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, sector = SECTOR_SIZE_512; + int rv = -1, max_hosts = 0, sector = SECTOR_SIZE_512; long align = ALIGNMENT_1M; uint32_t io_timeout = 0; - const char *lockspace, *path; + PyObject *lockspace = NULL; + const char *path; struct sanlk_lockspace ls; static char *kwlist[] = {"lockspace", "path", "offset", "max_hosts", "iotimeout", "align", "sector", NULL}; /* initialize lockspace structure */ memset(&ls, 0, sizeof(struct sanlk_lockspace)); /* parse python tuple */ - if (!PyArg_ParseTupleAndKeywords(args, keywds, "ss|kiIli", kwlist, - &lockspace, &path, &ls.host_id_disk.offset, &max_hosts, - &io_timeout, &align, &sector)) { - return NULL; + if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&s|kiIli", kwlist, + convert_to_pybytes, &lockspace, &path, &ls.host_id_disk.offset, + &max_hosts, &io_timeout, &align, &sector)) { + goto finally; } /* prepare sanlock names */ - strncpy(ls.name, lockspace, SANLK_NAME_LEN); + strncpy(ls.name, PyBytes_AsString(lockspace), SANLK_NAME_LEN); strncpy(ls.host_id_disk.path, path, SANLK_PATH_LEN - 1); /* set alignment/sector flags */ if (add_align_flag(align, &ls.flags) == -1) - return NULL; + goto finally; if (add_sector_flag(sector, &ls.flags) == -1) - return NULL; + goto finally; /* write sanlock lockspace (gil disabled) */ Py_BEGIN_ALLOW_THREADS rv = sanlock_write_lockspace(&ls, max_hosts, 0, io_timeout); Py_END_ALLOW_THREADS if (rv != 0) { __set_exception(rv, "Sanlock lockspace write failure"); - return NULL; + goto finally; } +finally: + Py_XDECREF(lockspace); + if (rv != 0) + return NULL; Py_RETURN_NONE; } /* read_lockspace */ PyDoc_STRVAR(pydoc_read_lockspace, "\ -- 2.17.2

From: Amit Bawer <abawer(a)redhat.com&gt; Implications: - Py2 : no difference - Py3 : if used, only accept bytes for lockspace name. --- python/sanlock.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/python/sanlock.c b/python/sanlock.c index ad99436..fca9c80 100644 --- a/python/sanlock.c +++ b/python/sanlock.c @@ -732,47 +732,52 @@ The iotimeout option configures the io timeout for the specific lockspace,\n\ overriding the default value (see the sanlock daemon parameter -o)."); static PyObject * py_add_lockspace(PyObject *self __unused, PyObject *args, PyObject *keywds) { - int rv, async = 0, flags = 0; + int rv = -1, async = 0, flags = 0; uint32_t iotimeout = 0; - const char *lockspace, *path; + PyObject *lockspace = NULL; + const char *path; struct sanlk_lockspace ls; static char *kwlist[] = {"lockspace", "host_id", "path", "offset", "iotimeout", "async", NULL}; /* initialize lockspace structure */ memset(&ls, 0, sizeof(struct sanlk_lockspace)); /* parse python tuple */ - if (!PyArg_ParseTupleAndKeywords(args, keywds, "sks|kIi", kwlist, - &lockspace, &ls.host_id, &path, &ls.host_id_disk.offset, &iotimeout, - &async)) { - return NULL; + if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&ks|kIi", kwlist, + convert_to_pybytes, &lockspace, &ls.host_id, &path, &ls.host_id_disk.offset, + &iotimeout, &async)) { + goto finally; } /* prepare sanlock_add_lockspace flags */ if (async) { flags |= SANLK_ADD_ASYNC; } /* prepare sanlock names */ - strncpy(ls.name, lockspace, SANLK_NAME_LEN); + strncpy(ls.name, PyBytes_AsString(lockspace), SANLK_NAME_LEN); strncpy(ls.host_id_disk.path, path, SANLK_PATH_LEN - 1); /* add sanlock lockspace (gil disabled) */ Py_BEGIN_ALLOW_THREADS rv = sanlock_add_lockspace_timeout(&ls, flags, iotimeout); Py_END_ALLOW_THREADS if (rv != 0) { __set_exception(rv, "Sanlock lockspace add failure"); - return NULL; + goto finally; } +finally: + Py_XDECREF(lockspace); + if (rv != 0 ) + return NULL; Py_RETURN_NONE; } /* inq_lockspace */ PyDoc_STRVAR(pydoc_inq_lockspace, "\ -- 2.17.2

From: Amit Bawer <abawer(a)redhat.com&gt; Implications: - Py2 : no difference - Py3 : if used, only accept bytes for lockspace name. --- python/sanlock.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/python/sanlock.c b/python/sanlock.c index 1fdb143..28146df 100644 --- a/python/sanlock.c +++ b/python/sanlock.c @@ -853,29 +853,30 @@ sanlock will try to terminate processes holding resource leases and upon\n\ successful termination these leases will be released."); static PyObject * py_rem_lockspace(PyObject *self __unused, PyObject *args, PyObject *keywds) { - int rv, async = 0, unused = 0, flags = 0; - const char *lockspace, *path; + int rv = -1, async = 0, unused = 0, flags = 0; + PyObject *lockspace = NULL; + const char *path; struct sanlk_lockspace ls; static char *kwlist[] = {"lockspace", "host_id", "path", "offset", "async", "unused", NULL}; /* initialize lockspace structure */ memset(&ls, 0, sizeof(struct sanlk_lockspace)); /* parse python tuple */ - if (!PyArg_ParseTupleAndKeywords(args, keywds, "sks|kii", kwlist, - &lockspace, &ls.host_id, &path, &ls.host_id_disk.offset, &async, - &unused)) { - return NULL; + if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&ks|kii", kwlist, + convert_to_pybytes, &lockspace, &ls.host_id, &path, &ls.host_id_disk.offset, + &async, &unused)) { + goto finally; } /* prepare sanlock names */ - strncpy(ls.name, lockspace, SANLK_NAME_LEN); + strncpy(ls.name, PyBytes_AsString(lockspace), SANLK_NAME_LEN); strncpy(ls.host_id_disk.path, path, SANLK_PATH_LEN - 1); /* prepare sanlock_rem_lockspace flags */ if (async) { flags |= SANLK_REM_ASYNC; @@ -890,13 +891,17 @@ py_rem_lockspace(PyObject *self __unused, PyObject *args, PyObject *keywds) rv = sanlock_rem_lockspace(&ls, flags); Py_END_ALLOW_THREADS if (rv != 0) { __set_exception(rv, "Sanlock lockspace remove failure"); - return NULL; + goto finally; } +finally: + Py_XDECREF(lockspace); + if (rv != 0) + return NULL; Py_RETURN_NONE; } /* get_lockspaces */ PyDoc_STRVAR(pydoc_get_lockspaces, "\ -- 2.17.2

From: Amit Bawer <abawer(a)redhat.com&gt; Implications: - Py2 : no difference - Py3 : if used, only accept bytes for lockspace/resource name. --- python/sanlock.c | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/python/sanlock.c b/python/sanlock.c index c7518c6..7bdbd69 100644 --- a/python/sanlock.c +++ b/python/sanlock.c @@ -1028,39 +1028,39 @@ mode. The version is the version of the lease that must be acquired or fail.\n\ The disks must be in the format: [(path, offset), ... ]\n"); static PyObject * py_acquire(PyObject *self __unused, PyObject *args, PyObject *keywds) { - int rv, sanlockfd = -1, pid = -1, shared = 0; - const char *lockspace, *resource; - struct sanlk_resource *res; + int rv = -1, sanlockfd = -1, pid = -1, shared = 0; + PyObject *lockspace = NULL, *resource = NULL; + struct sanlk_resource *res = NULL; PyObject *disks, *version = Py_None; static char *kwlist[] = {"lockspace", "resource", "disks", "slkfd", "pid", "shared", "version", NULL}; /* parse python tuple */ - if (!PyArg_ParseTupleAndKeywords(args, keywds, "ssO!|iiiO", kwlist, - &lockspace, &resource, &PyList_Type, &disks, &sanlockfd, &pid, - &shared, &version)) { - return NULL; + if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&O&O!|iiiO", kwlist, + convert_to_pybytes, &lockspace, convert_to_pybytes, &resource, + &PyList_Type, &disks, &sanlockfd, &pid, &shared, &version)) { + goto finally; } /* check if any of the slkfd or pid parameters was given */ if (sanlockfd == -1 && pid == -1) { __set_exception(EINVAL, "Invalid slkfd and pid values"); - return NULL; + goto finally; } /* parse and check sanlock resource */ if (__parse_resource(disks, &res) < 0) { - return NULL; + goto finally; } /* prepare sanlock names */ - strncpy(res->lockspace_name, lockspace, SANLK_NAME_LEN); - strncpy(res->name, resource, SANLK_NAME_LEN); + strncpy(res->lockspace_name, PyBytes_AsString(lockspace), SANLK_NAME_LEN); + strncpy(res->name, PyBytes_AsString(resource), SANLK_NAME_LEN); /* prepare sanlock flags */ if (shared) { res->flags |= SANLK_RES_SHARED; } @@ -1069,30 +1069,31 @@ py_acquire(PyObject *self __unused, PyObject *args, PyObject *keywds) if (version != Py_None) { res->flags |= SANLK_RES_LVER; res->lver = pyinteger_as_unsigned_long_long_mask(version); if (res->lver == (uint64_t)-1) { __set_exception(EINVAL, "Unable to convert the version value"); - goto exit_fail; + goto finally; } } /* acquire sanlock resource (gil disabled) */ Py_BEGIN_ALLOW_THREADS rv = sanlock_acquire(sanlockfd, pid, 0, 1, &res, 0); Py_END_ALLOW_THREADS if (rv != 0) { __set_exception(rv, "Sanlock resource not acquired"); - goto exit_fail; + goto finally; } +finally: + Py_XDECREF(lockspace); + Py_XDECREF(resource); free(res); + if (rv != 0) + return NULL; Py_RETURN_NONE; - -exit_fail: - free(res); - return NULL; } /* release */ PyDoc_STRVAR(pydoc_release, "\ release(lockspace, resource, disks [, slkfd=fd, pid=owner])\n\ -- 2.17.2

From: Amit Bawer <abawer(a)redhat.com&gt; Implications: - Py2 : no difference - Py3 : if used, only accept bytes for lockspace/resource name. --- python/sanlock.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/python/sanlock.c b/python/sanlock.c index b43d635..0964671 100644 --- a/python/sanlock.c +++ b/python/sanlock.c @@ -1157,61 +1157,63 @@ to acquire or None (which automatically uses the next version).\n\ The disks must be in the format: [(path, offset), ... ]"); static PyObject * py_request(PyObject *self __unused, PyObject *args, PyObject *keywds) { - int rv, action = SANLK_REQ_GRACEFUL, flags = 0; - const char *lockspace, *resource; - struct sanlk_resource *res; + int rv = -1, action = SANLK_REQ_GRACEFUL, flags = 0; + PyObject *lockspace = NULL, *resource = NULL; + struct sanlk_resource *res = NULL; PyObject *disks, *version = Py_None; static char *kwlist[] = {"lockspace", "resource", "disks", "action", "version", NULL}; /* parse python tuple */ - if (!PyArg_ParseTupleAndKeywords(args, keywds, "ssO!|iO", kwlist, - &lockspace, &resource, &PyList_Type, &disks, &action, &version)) { - return NULL; + if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&O&O!|iO", kwlist, + convert_to_pybytes, &lockspace, convert_to_pybytes, &resource, + &PyList_Type, &disks, &action, &version)) { + goto finally; } /* parse and check sanlock resource */ if (__parse_resource(disks, &res) < 0) { - return NULL; + goto finally; } /* prepare sanlock names */ - strncpy(res->lockspace_name, lockspace, SANLK_NAME_LEN); - strncpy(res->name, resource, SANLK_NAME_LEN); + strncpy(res->lockspace_name, PyBytes_AsString(lockspace), SANLK_NAME_LEN); + strncpy(res->name, PyBytes_AsString(resource), SANLK_NAME_LEN); /* prepare the resource version */ if (version == Py_None) { flags = SANLK_REQUEST_NEXT_LVER; } else { res->flags |= SANLK_RES_LVER; res->lver = pyinteger_as_unsigned_long_long_mask(version); if (res->lver == (uint64_t)-1) { __set_exception(EINVAL, "Unable to convert the version value"); - goto exit_fail; + goto finally; } } /* request sanlock resource (gil disabled) */ Py_BEGIN_ALLOW_THREADS rv = sanlock_request(flags, action, res); Py_END_ALLOW_THREADS if (rv != 0) { __set_exception(rv, "Sanlock request not submitted"); - goto exit_fail; + goto finally; } +finally: + Py_XDECREF(lockspace); + Py_XDECREF(resource); free(res); + if (rv != 0) + return NULL; Py_RETURN_NONE; - -exit_fail: - free(res); - return NULL; } /* read_resource_owners */ PyDoc_STRVAR(pydoc_read_resource_owners, "\ read_resource_owners(lockspace, resource, disks, align=1048576, sector=512) \ -- 2.17.2

From: Amit Bawer <abawer(a)redhat.com&gt; Implications: - Py2 : no difference - Py3 : if used, only accept bytes for lockspace name. --- python/sanlock.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/python/sanlock.c b/python/sanlock.c index ed9b4f0..1f2960e 100644 --- a/python/sanlock.c +++ b/python/sanlock.c @@ -1394,25 +1394,30 @@ you can use get_event to get pending events. When you are done, you must\n\ unregister the event listener using end_event."); static PyObject * py_reg_event(PyObject *self __unused, PyObject *args) { - const char *lockspace = NULL; + PyObject *lockspace = NULL; int fd = -1; - if (!PyArg_ParseTuple(args, "s", &lockspace)) - return NULL; + if (!PyArg_ParseTuple(args, "O&", convert_to_pybytes, &lockspace)) { + goto finally; + } Py_BEGIN_ALLOW_THREADS - fd = sanlock_reg_event(lockspace, NULL /* event */, 0 /* flags */); + fd = sanlock_reg_event(PyBytes_AsString(lockspace), NULL /* event */, 0 /* flags */); Py_END_ALLOW_THREADS if (fd < 0) { __set_exception(fd, "Unable to register event fd"); - return NULL; + goto finally; } +finally: + Py_XDECREF(lockspace); + if (fd < 0) + return NULL; return Py_BuildValue("i", fd); } /* get_event */ PyDoc_STRVAR(pydoc_get_event, "\ -- 2.17.2

From: Amit Bawer <abawer(a)redhat.com&gt; Also add a stub test for arguments parsing. Implications: - Py2 : no difference - Py3 : if used, only accept bytes for lockspace name. --- python/sanlock.c | 20 ++++++++++++-------- tests/python_test.py | 6 ++++++ 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/python/sanlock.c b/python/sanlock.c index 8f99227..39a9fb6 100644 --- a/python/sanlock.c +++ b/python/sanlock.c @@ -1615,33 +1615,37 @@ with -EBUSY error in this case.\n\ "); static PyObject * py_set_event(PyObject *self __unused, PyObject *args, PyObject *keywds) { - const char *lockspace; + PyObject *lockspace = NULL; struct sanlk_host_event he = {0}; uint32_t flags = 0; - int rv; + int rv = -1; static char *kwlist[] = {"lockspace", "host_id", "generation", "event", "data", "flags", NULL}; - if (!PyArg_ParseTupleAndKeywords(args, keywds, "sKKK|KI", kwlist, - &lockspace, &he.host_id, &he.generation, &he.event, &he.data, - &flags)) { - return NULL; + if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&KKK|KI", kwlist, + convert_to_pybytes, &lockspace, &he.host_id, &he.generation, &he.event, + &he.data, &flags)) { + goto finally; } Py_BEGIN_ALLOW_THREADS - rv = sanlock_set_event(lockspace, &he, flags); + rv = sanlock_set_event(PyBytes_AsString(lockspace), &he, flags); Py_END_ALLOW_THREADS if (rv < 0) { __set_exception(rv, "Unable to set event"); - return NULL; + goto finally; } +finally: + Py_XDECREF(lockspace); + if (rv < 0) + return NULL; Py_RETURN_NONE; } static PyMethodDef sanlock_methods[] = { diff --git a/tests/python_test.py b/tests/python_test.py index 7406bcb..7093aac 100644 --- a/tests/python_test.py +++ b/tests/python_test.py @@ -604,10 +604,16 @@ def test_reg_event_parse_args(no_sanlock_daemon, name): def test_end_event_parse_args(no_sanlock_daemon, name): with raises_sanlock_errno(errno.EALREADY): sanlock.end_event(-1, name) +(a)pytest.mark.parametrize(&quot;name&quot;, LOCKSPACE_OR_RESOURCE_NAMES) +def test_set_event_parse_args(no_sanlock_daemon, name): + with raises_sanlock_errno(): + sanlock.set_event(name, 1, 1, 1) + + @pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES) def test_init_lockspace_parse_args(no_sanlock_daemon, name): with raises_sanlock_errno(errno.ENODEV): sanlock.init_lockspace(name, "path") -- 2.17.2