sanlock-devel April 2019

sanlock-devel@lists.fedorahosted.org

4 participants
34 discussions

[PATCH v3 0/4] Values instead of flags in python API.
by Vojtech Juranek 01 May '19

01 May '19

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

2 7

[PATCH 1/4] tests: add sector and align parameters
by Vojtech Juranek 01 May '19

01 May '19

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

2 4

[PATCH v2 0/4] Values instead of flags in python API.
by Vojtech Juranek 26 Apr '19

26 Apr '19

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. Second version of this patch set fixes pointer dereffeence in 3th 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 | 161 ++++++++++++++++++++++++++++++++++--------- tests/python_test.py | 48 +++++++++++-- 3 files changed, 173 insertions(+), 40 deletions(-) -- 2.20.1

2 5

Re: [PATCH 3/4] Expose tuples with supported align and sector values
by Vojtech Juranek 26 Apr '19

26 Apr '19

Hi, > Hi Vojtech > > Please see some questions in-lined below. > > Thanks > Amit > > On Thu, Apr 25, 2019 at 2:57 PM Vojtech Juranek <vjuranek(a)redhat.com> 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 323f1bd..f367fbd 100644 > > --- a/python/sanlock.c > > +++ b/python/sanlock.c > > @@ -1739,13 +1739,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"); > > *shouldn't this be Py_DECREF(sector) ? AFAIK Py_DECREF should get a pointer > to PyObject * ops, sure, thanks for spotting it, same for align > > + > > + 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"); > > *shouldn't this be Py_DECREF(align) ? AFAIK Py_DECREF should get a pointer > to PyObject * > > > + > > > > } > > > > -- > > 2.20.1 > > *In What repo/branch are those patches available ? * official repo is [1], I pushed my changes into my fork on github [2], however, patches are handled over email on sanlock-devel ML [3], so you should CC also sanlock-devel(a)lists.fedorahosted.org (you have to subscribe to this ML first to be able to post there) [1] https://pagure.io/sanlock [2] https://github.com/vjuranek/sanlock/tree/4k_api_update [3] https://lists.fedorahosted.org/admin/lists/sanlock-devel.lists.fedorahosted…

1 0

[sanlock] 03/03: tests: Test reading and writing large offsets
by pagure＠pagure.io 22 Apr '19

22 Apr '19

This is an automated email from the git hooks/post-receive script. teigland pushed a commit to branch master in repository sanlock. commit d04760e01d22b8b06b017b3826a07e35f060da03 Author: Nir Soffer <nirsof(a)gmail.com> AuthorDate: Fri Apr 19 18:24:38 2019 +0300 tests: Test reading and writing large offsets The python module uses "k" format string[1] to convert lockspace offset and PyInt_AsLong[2] to convert disk offset. Both return long, while sanlock offset use uint64_t. This is not correct on all systems but I'm not sure we support any system when sizeof(long) < 8. Add tests for large offset exceeding LONG_MAX to check that current code actually works. Vdsm does not use such offset yet, but when using 4k sector size and 2 MiB alignment, vdsm will access such offset. Theoretically we can test up to (16 TiB - 1 MiB) offset, but testing show that practical maximum file size is about 15 TiB. To make the test less likely to fail on developer machine or CI slave, use 1 TiB file for testing large offset. [1] https://docs.python.org/2.7/c-api/arg.html (see k (integer) [unsigned long]) [2] https://docs.python.org/2.7/c-api/int.html#c.PyInt_AsLong Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> --- tests/python_test.py | 91 ++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 64 insertions(+), 27 deletions(-) diff --git a/tests/python_test.py b/tests/python_test.py index be1e3ab..5029924 100644 --- a/tests/python_test.py +++ b/tests/python_test.py @@ -14,21 +14,37 @@ import sanlock from . import constants from . import util - -def test_write_lockspace(tmpdir, sanlock_daemon): +# 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 + +LOCKSPACE_SIZE = 1024**2 +MIN_RES_SIZE = 1024**2 + + +(a)pytest.mark.parametrize("size,offset", [ + # Smallest offset. + (LOCKSPACE_SIZE, 0), + # Large offset. + (LARGE_FILE_SIZE, LARGE_FILE_SIZE - LOCKSPACE_SIZE), +]) +def test_write_lockspace(tmpdir, sanlock_daemon, size, offset): path = str(tmpdir.join("lockspace")) - size = 1024**2 util.create_file(path, size) - sanlock.write_lockspace("name", path, offset=0, iotimeout=1) + sanlock.write_lockspace("name", path, offset=offset, iotimeout=1) - ls = sanlock.read_lockspace(path, offset=0) + ls = sanlock.read_lockspace(path, offset=offset) assert ls == {"iotimeout": 1, "lockspace": "name"} - acquired = sanlock.inq_lockspace("name", 1, path, wait=False) + acquired = sanlock.inq_lockspace( + "name", 1, path, offset=offset, wait=False) assert acquired is False with io.open(path, "rb") as f: + f.seek(offset) magic, = struct.unpack("< I", f.read(4)) assert magic == constants.DELTA_DISK_MAGIC @@ -37,15 +53,20 @@ def test_write_lockspace(tmpdir, sanlock_daemon): util.check_guard(path, size) -def test_write_resource(tmpdir, sanlock_daemon): +(a)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")) - size = 1024**2 util.create_file(path, size) - disks = [(path, 0)] + disks = [(path, offset)] sanlock.write_resource("ls_name", "res_name", disks) - res = sanlock.read_resource(path, 0) + res = sanlock.read_resource(path, offset=offset) assert res == { "lockspace": "ls_name", "resource": "res_name", @@ -56,6 +77,7 @@ def test_write_resource(tmpdir, sanlock_daemon): assert owners == [] with io.open(path, "rb") as f: + f.seek(offset) magic, = struct.unpack("< I", f.read(4)) assert magic == constants.PAXOS_DISK_MAGIC @@ -64,26 +86,35 @@ def test_write_resource(tmpdir, sanlock_daemon): util.check_guard(path, size) -def test_add_rem_lockspace(tmpdir, sanlock_daemon): +(a)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, 1024**2) + util.create_file(path, size) - sanlock.write_lockspace("ls_name", path, iotimeout=1) + sanlock.write_lockspace("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, wait=False) + acquired = sanlock.inq_lockspace( + "ls_name", 1, path, offset=offset, wait=False) assert acquired is False - sanlock.add_lockspace("ls_name", 1, path, iotimeout=1) + sanlock.add_lockspace("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, wait=False) + acquired = sanlock.inq_lockspace( + "ls_name", 1, path, offset=offset, wait=False) assert acquired is True - sanlock.rem_lockspace("ls_name", 1, path) + sanlock.rem_lockspace("ls_name", 1, path, offset=offset) # Once the lockspace is released, we exepect to get False. - acquired = sanlock.inq_lockspace("ls_name", 1, path, wait=False) + acquired = sanlock.inq_lockspace( + "ls_name", 1, path, offset=offset, wait=False) assert acquired is False @@ -123,15 +154,21 @@ def test_add_rem_lockspace_async(tmpdir, sanlock_daemon): assert acquired is False -def test_acquire_release_resource(tmpdir, sanlock_daemon): +(a)pytest.mark.parametrize("size,offset", [ + # Smallest offset. + (MIN_RES_SIZE, 0), + # Large offset. + (LARGE_FILE_SIZE, LARGE_FILE_SIZE - MIN_RES_SIZE), +]) +def test_acquire_release_resource(tmpdir, sanlock_daemon, size, offset): ls_path = str(tmpdir.join("ls_name")) - util.create_file(ls_path, 1024**2) + util.create_file(ls_path, size) res_path = str(tmpdir.join("res_name")) - util.create_file(res_path, 1024**2) + util.create_file(res_path, size) - sanlock.write_lockspace("ls_name", ls_path, iotimeout=1) - sanlock.add_lockspace("ls_name", 1, ls_path, iotimeout=1) + sanlock.write_lockspace("ls_name", ls_path, offset=offset, iotimeout=1) + sanlock.add_lockspace("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: @@ -142,10 +179,10 @@ def test_acquire_release_resource(tmpdir, sanlock_daemon): host = sanlock.get_hosts("ls_name", 1)[0] assert host["flags"] == sanlock.HOST_LIVE - disks = [(res_path, 0)] + disks = [(res_path, offset)] sanlock.write_resource("ls_name", "res_name", disks) - res = sanlock.read_resource(res_path, 0) + res = sanlock.read_resource(res_path, offset=offset) assert res == { "lockspace": "ls_name", "resource": "res_name", @@ -158,7 +195,7 @@ def test_acquire_release_resource(tmpdir, sanlock_daemon): fd = sanlock.register() sanlock.acquire("ls_name", "res_name", disks, slkfd=fd) - res = sanlock.read_resource(res_path, 0) + res = sanlock.read_resource(res_path, offset=offset) assert res == { "lockspace": "ls_name", "resource": "res_name", @@ -179,7 +216,7 @@ def test_acquire_release_resource(tmpdir, sanlock_daemon): sanlock.release("ls_name", "res_name", disks, slkfd=fd) - res = sanlock.read_resource(res_path, 0) + res = sanlock.read_resource(res_path, offset=offset) assert res == { "lockspace": "ls_name", "resource": "res_name", -- To stop receiving notification emails like this one, please contact the administrator of this repository.

1 0

[sanlock] 02/03: tests: Remove unneeded str() conversion
by pagure＠pagure.io 22 Apr '19

22 Apr '19

This is an automated email from the git hooks/post-receive script. teigland pushed a commit to branch master in repository sanlock. commit f3382c495680ab1b27c5f7efbef2d6576d4d0ac3 Author: Nir Soffer <nirsof(a)gmail.com> AuthorDate: Fri Apr 19 18:24:37 2019 +0300 tests: Remove unneeded str() conversion Two instances were forgottern in the last cleanup, removing them. Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> --- tests/python_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/python_test.py b/tests/python_test.py index 2dd4e17..be1e3ab 100644 --- a/tests/python_test.py +++ b/tests/python_test.py @@ -28,13 +28,13 @@ def test_write_lockspace(tmpdir, sanlock_daemon): acquired = sanlock.inq_lockspace("name", 1, path, wait=False) assert acquired is False - with io.open(str(path), "rb") as f: + with io.open(path, "rb") as f: magic, = struct.unpack("< I", f.read(4)) assert magic == constants.DELTA_DISK_MAGIC # TODO: check more stuff here... - util.check_guard(str(path), size) + util.check_guard(path, size) def test_write_resource(tmpdir, sanlock_daemon): -- To stop receiving notification emails like this one, please contact the administrator of this repository.

1 0

[sanlock] 01/03: tests: Allow testing large files
by pagure＠pagure.io 22 Apr '19

22 Apr '19

This is an automated email from the git hooks/post-receive script. teigland pushed a commit to branch master in repository sanlock. commit 68b5d5cdb037299cd6d629bcf15867330f1d5093 Author: Nir Soffer <nirsof(a)gmail.com> AuthorDate: Fri Apr 19 18:24:36 2019 +0300 tests: Allow testing large files util.create_file() was filling the file with "x". This works for small files but prevent testing big files. Change to create sparse file so we can test easily large files and offsets. Tests that want to fill areas on storage can access the file directly. We don't have such tests yet. Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> --- tests/util.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/tests/util.py b/tests/util.py index 2259473..a6b53e3 100644 --- a/tests/util.py +++ b/tests/util.py @@ -97,11 +97,9 @@ def wait_for_termination(p, timeout): time.sleep(0.05) -def create_file(path, size, poison=b"x", guard=b"X", guard_size=4096): +def create_file(path, size, guard=b"X", guard_size=4096): """ - Create file filled with poison byte. - - To create an empty file, set poison to None. + Create sparse file of size bytes. If guard is set, add a guard area after the end of the file and fill it with guard bytes. This allows testing that the code under test do not write @@ -109,11 +107,9 @@ def create_file(path, size, poison=b"x", guard=b"X", guard_size=4096): """ with io.open(path, "wb") as f: f.truncate(size) - if poison: - f.write(poison * size) if guard: f.seek(size) - f.write(guard * 4096) + f.write(guard * guard_size) def check_guard(path, size, guard=b"X", guard_size=4096): -- To stop receiving notification emails like this one, please contact the administrator of this repository.

1 0

[PATCH 0/3] Test large offset
by Nir Soffer 19 Apr '19

19 Apr '19

While working on python 3 port, we noticed that the python extension may truncate large offset when converting python value to C value. Change the tests to test also large offsets to make sure current code actually works with offset > LONG_MAX. Nir Soffer (3): tests: Allow testing large files tests: Remove unneeded str() conversion tests: Test reading and writing large offsets tests/python_test.py | 95 ++++++++++++++++++++++++++++++-------------- tests/util.py | 10 ++--- 2 files changed, 69 insertions(+), 36 deletions(-) -- 2.17.2

1 3

[PATCH] python: improve rem_lockspace API doc wording
by Vojtech Juranek 16 Apr '19

16 Apr '19

--- python/sanlock.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/python/sanlock.c b/python/sanlock.c index b387307..3bfd1f2 100644 --- a/python/sanlock.c +++ b/python/sanlock.c @@ -746,8 +746,9 @@ rem_lockspace(lockspace, host_id, path, offset=0, async=False, unused=False)\n\ Remove a lockspace, releasing the acquired host_id. If async is True the\n\ function will return immediately and the status can be checked using\n\ inq_lockspace. If unused is True the command will fail (EBUSY) if there is\n\ -at least one acquired resource in the lockspace (instead of automatically\n\ -release it)."); +at least one acquired resource in the lockspace. Otherwise (the default)\n\ +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) -- 2.20.1

1 0

Re: [PATCH] python: improve rem_lockspace API doc wording
by Nir Soffer 16 Apr '19

16 Apr '19

On Tue, Apr 16, 2019 at 10:37 PM Vojtech Juranek <vjuranek(a)redhat.com> wrote: > --- > python/sanlock.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/python/sanlock.c b/python/sanlock.c > index b387307..3bfd1f2 100644 > --- a/python/sanlock.c > +++ b/python/sanlock.c > @@ -746,8 +746,9 @@ rem_lockspace(lockspace, host_id, path, offset=0, > async=False, unused=False)\n\ > Remove a lockspace, releasing the acquired host_id. If async is True > the\n\ > function will return immediately and the status can be checked using\n\ > inq_lockspace. If unused is True the command will fail (EBUSY) if there > is\n\ > -at least one acquired resource in the lockspace (instead of > automatically\n\ > -release it)."); > +at least one acquired resource in the lockspace. Otherwise (the > default)\n\ > +sanlock will try to terminate processes holding resource leases and > upon\n\ > +successful termination these leases will be released."); > Looks good. Adding a note about the time this may block can be helpful, but this is probably true for other functions and we can improve it later. Nir > > static PyObject * > py_rem_lockspace(PyObject *self __unused, PyObject *args, PyObject > *keywds) > -- > 2.20.1 > > >

1 0

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

sanlock-devel April 2019