[PATCH v3 0/4] Values instead of flags in python API.
by 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
4 years, 1 month
[PATCH 1/4] tests: add sector and align parameters
by 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
4 years, 1 month
[PATCH v2 0/4] Values instead of flags in python API.
by 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.
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
4 years, 1 month
Re: [PATCH 3/4] Expose tuples with supported align and sector values
by Vojtech Juranek
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.fedorahost...
4 years, 1 month
[sanlock] 03/03: tests: Test reading and writing large offsets
by pagure＠pagure.io
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.
4 years, 1 month
[sanlock] 02/03: tests: Remove unneeded str() conversion
by pagure＠pagure.io
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.
4 years, 1 month
[sanlock] 01/03: tests: Allow testing large files
by pagure＠pagure.io
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.
4 years, 1 month
[PATCH 0/3] Test large offset
by Nir Soffer
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
4 years, 1 month
[PATCH] python: improve rem_lockspace API doc wording
by Vojtech Juranek
---
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
4 years, 1 month
Re: [PATCH] python: improve rem_lockspace API doc wording
by Nir Soffer
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
>
>
>
4 years, 1 month
