This patch series adds python bindings to writing (set_lvb) and reading (get_lvb) data to and from the Lock Value Block of a sanlock resource.
Benny Zlotnik (4): python: add lvb flag to sanlock.acquire python: expose set_lvb python: expose get_lvb tests: add a test for LVB operation
python/sanlock.c | 140 ++++++++++++++++++++++++++++++++++++++++--- tests/python_test.py | 23 +++++++ 2 files changed, 156 insertions(+), 7 deletions(-)
The flag will allow to acquire a resource with the SANLK_ACQUIRE_LVB flag, allowing the user to write or read data from LVB space.
Signed-off-by: Benny Zlotnik bzlotnik@redhat.com --- python/sanlock.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/python/sanlock.c b/python/sanlock.c index d616008..c1c3e8b 100644 --- a/python/sanlock.c +++ b/python/sanlock.c @@ -981,28 +981,31 @@ finally: /* acquire */ PyDoc_STRVAR(pydoc_acquire, "\ acquire(lockspace, resource, disks \ -[, slkfd=fd, pid=owner, shared=False, version=None])\n\ +[, slkfd=fd, pid=owner, shared=False, version=None, lvb=False])\n\ Acquire a resource lease for the current process (using the slkfd argument\n\ to specify the sanlock file descriptor) or for an other process (using the\n\ pid argument). If shared is True the resource will be acquired in the shared\n\ 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"); +The disks must be in the format: [(path, offset), ... ]\n\ +If lvb is True the resource will be acquired with the LVB flag enabled\n\ +to allow access to LVB data.\n");
static PyObject * py_acquire(PyObject *self __unused, PyObject *args, PyObject *keywds) { - int rv = -1, sanlockfd = -1, pid = -1, shared = 0; + int rv = -1, sanlockfd = -1, pid = -1, shared = 0, lvb = 0; + uint32_t flags = 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}; + "pid", "shared", "lvb", "version", NULL};
/* parse python tuple */ - if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&O&O!|iiiO", kwlist, + if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&O&O!|iiiiO", kwlist, convert_to_pybytes, &lockspace, convert_to_pybytes, &resource, - &PyList_Type, &disks, &sanlockfd, &pid, &shared, &version)) { + &PyList_Type, &disks, &sanlockfd, &pid, &shared, &lvb, &version)) { goto finally; }
@@ -1026,6 +1029,10 @@ py_acquire(PyObject *self __unused, PyObject *args, PyObject *keywds) res->flags |= SANLK_RES_SHARED; }
+ if (lvb) { + flags |= SANLK_ACQUIRE_LVB; + } + /* prepare the resource version */ if (version != Py_None) { res->flags |= SANLK_RES_LVER; @@ -1038,7 +1045,7 @@ py_acquire(PyObject *self __unused, PyObject *args, PyObject *keywds)
/* acquire sanlock resource (gil disabled) */ Py_BEGIN_ALLOW_THREADS - rv = sanlock_acquire(sanlockfd, pid, 0, 1, &res, 0); + rv = sanlock_acquire(sanlockfd, pid, flags, 1, &res, 0); Py_END_ALLOW_THREADS
if (rv != 0) {
Add a binding to the set_lvb function to allow writing data to LVB.
Signed-off-by: Benny Zlotnik bzlotnik@redhat.com --- python/sanlock.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+)
diff --git a/python/sanlock.c b/python/sanlock.c index c1c3e8b..b2dc624 100644 --- a/python/sanlock.c +++ b/python/sanlock.c @@ -1594,6 +1594,63 @@ finally: Py_RETURN_NONE; }
+/* set_lvb */ +PyDoc_STRVAR(pydoc_set_lvb, "\ +set_lvb(lockspace, resource, disks, event, data)\n\ +Set Lock Value Block for a given resource\n\ +\n\ +Arguments\n\ + lockspace lockspace name (str)\n\ + resource resource name (int)\n\ + disks path and offset (tuple)\n\ + data data to write (str)\n\ +\n\ +Notes\n\ +\n\ +The resource must be acquired with the SANLK_ACQUIRE_LVB flag\n"); +static PyObject * +py_set_lvb(PyObject *self __unused, PyObject *args, PyObject *keywds) +{ + uint32_t flags = 0; + int rv = -1; + struct sanlk_resource *res; + PyObject *lockspace = NULL, *resource = NULL, *data = NULL; + PyObject *disks; + + static char *kwlist[] = {"lockspace", "resource", "disks", "data", NULL}; + if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&O&O!O&", kwlist, + convert_to_pybytes, &lockspace, convert_to_pybytes, &resource, &PyList_Type, &disks, convert_to_pybytes, &data)) + { + goto finally; + } + + if (parse_disks(disks, &res )< 0) + { + goto finally; + } + + strncpy(res->lockspace_name, PyBytes_AsString(lockspace), SANLK_NAME_LEN); + strncpy(res->name, PyBytes_AsString(resource), SANLK_NAME_LEN); + + Py_BEGIN_ALLOW_THREADS + rv = sanlock_set_lvb(flags, res, PyBytes_AS_STRING(data), PyBytes_GET_SIZE(data)); + Py_END_ALLOW_THREADS + + if (rv < 0) + { + set_sanlock_error(rv, "Unable to set lvb"); + goto finally; + } + +finally: + Py_XDECREF(lockspace); + Py_XDECREF(resource); + free(res); + if (rv < 0) + return NULL; + Py_RETURN_NONE; +} + static PyMethodDef sanlock_methods[] = { {"register", py_register, METH_NOARGS, pydoc_register}, @@ -1631,6 +1688,9 @@ sanlock_methods[] = { {"end_event", (PyCFunction) py_end_event, METH_VARARGS, pydoc_end_event}, {"set_event", (PyCFunction) py_set_event, METH_VARARGS|METH_KEYWORDS, pydoc_set_event}, + {"set_lvb", (PyCFunction) py_set_lvb, + METH_VARARGS|METH_KEYWORDS, pydoc_set_lvb}, + {NULL, NULL, 0, NULL} };
Add a binding to the get_lvb function to allow reading data from LVB.
Signed-off-by: Benny Zlotnik bzlotnik@redhat.com --- python/sanlock.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+)
diff --git a/python/sanlock.c b/python/sanlock.c index b2dc624..4803b1f 100644 --- a/python/sanlock.c +++ b/python/sanlock.c @@ -1651,6 +1651,63 @@ finally: Py_RETURN_NONE; }
+PyDoc_STRVAR(pydoc_get_lvb, "\ +get_lvb(lockspace, resource, disks, event, data)\n\ +Read Lock Value Block for a given resource\n\ +\n\ +Arguments\n\ + lockspace lockspace name (str)\n\ + resource resource name (int)\n\ + disks path and offset (tuple)\n\ +\n\ +Notes\n\ +\n\ +The resource must be acquired with the SANLK_ACQUIRE_LVB flag\n"); +static PyObject * +py_get_lvb(PyObject *self __unused, PyObject *args, PyObject *keywds) +{ + uint32_t flags = 0; + int rv = -1; + struct sanlk_resource *res; + PyObject *lockspace = NULL, *resource = NULL; + PyObject *disks; + char data[512]; + + static char *kwlist[] = {"lockspace", "resource", "disks", NULL}; + if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&O&O!", kwlist, + convert_to_pybytes, &lockspace, convert_to_pybytes, &resource, &PyList_Type, &disks)) + { + goto finally; + } + + if (parse_disks(disks, &res )< 0) + { + goto finally; + } + + strncpy(res->lockspace_name, PyBytes_AsString(lockspace), SANLK_NAME_LEN); + strncpy(res->name, PyBytes_AsString(resource), SANLK_NAME_LEN); + + Py_BEGIN_ALLOW_THREADS + rv = sanlock_get_lvb(flags, res, &data, sizeof(data)); + Py_END_ALLOW_THREADS + + if (rv < 0) + { + set_sanlock_error(rv, "Unable to get lvb"); + goto finally; + } + +finally: + Py_XDECREF(lockspace); + Py_XDECREF(resource); + free(res); + if (rv < 0) + return NULL; + + return Py_BuildValue("s", data); +} + static PyMethodDef sanlock_methods[] = { {"register", py_register, METH_NOARGS, pydoc_register}, @@ -1690,6 +1747,8 @@ sanlock_methods[] = { METH_VARARGS|METH_KEYWORDS, pydoc_set_event}, {"set_lvb", (PyCFunction) py_set_lvb, METH_VARARGS|METH_KEYWORDS, pydoc_set_lvb}, + {"get_lvb", (PyCFunction) py_get_lvb, + METH_VARARGS|METH_KEYWORDS, pydoc_get_lvb},
{NULL, NULL, 0, NULL} };
Add a binding to the get_lvb function to allow reading data from LVB.
Signed-off-by: Benny Zlotnik bzlotnik@redhat.com --- Changes in v2: - use 'y' instead of 's' in Py_BuildValue
python/sanlock.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+)
diff --git a/python/sanlock.c b/python/sanlock.c index b2dc624..ee9e500 100644 --- a/python/sanlock.c +++ b/python/sanlock.c @@ -1651,6 +1651,63 @@ finally: Py_RETURN_NONE; }
+PyDoc_STRVAR(pydoc_get_lvb, "\ +get_lvb(lockspace, resource, disks, event, data)\n\ +Read Lock Value Block for a given resource\n\ +\n\ +Arguments\n\ + lockspace lockspace name (str)\n\ + resource resource name (int)\n\ + disks path and offset (tuple)\n\ +\n\ +Notes\n\ +\n\ +The resource must be acquired with the SANLK_ACQUIRE_LVB flag\n"); +static PyObject * +py_get_lvb(PyObject *self __unused, PyObject *args, PyObject *keywds) +{ + uint32_t flags = 0; + int rv = -1; + struct sanlk_resource *res; + PyObject *lockspace = NULL, *resource = NULL; + PyObject *disks; + char data[512]; + + static char *kwlist[] = {"lockspace", "resource", "disks", NULL}; + if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&O&O!", kwlist, + convert_to_pybytes, &lockspace, convert_to_pybytes, &resource, &PyList_Type, &disks)) + { + goto finally; + } + + if (parse_disks(disks, &res )< 0) + { + goto finally; + } + + strncpy(res->lockspace_name, PyBytes_AsString(lockspace), SANLK_NAME_LEN); + strncpy(res->name, PyBytes_AsString(resource), SANLK_NAME_LEN); + + Py_BEGIN_ALLOW_THREADS + rv = sanlock_get_lvb(flags, res, &data, sizeof(data)); + Py_END_ALLOW_THREADS + + if (rv < 0) + { + set_sanlock_error(rv, "Unable to get lvb"); + goto finally; + } + +finally: + Py_XDECREF(lockspace); + Py_XDECREF(resource); + free(res); + if (rv < 0) + return NULL; + + return Py_BuildValue("y", data); +} + static PyMethodDef sanlock_methods[] = { {"register", py_register, METH_NOARGS, pydoc_register}, @@ -1690,6 +1747,8 @@ sanlock_methods[] = { METH_VARARGS|METH_KEYWORDS, pydoc_set_event}, {"set_lvb", (PyCFunction) py_set_lvb, METH_VARARGS|METH_KEYWORDS, pydoc_set_lvb}, + {"get_lvb", (PyCFunction) py_get_lvb, + METH_VARARGS|METH_KEYWORDS, pydoc_get_lvb},
{NULL, NULL, 0, NULL} };
This patch tests writing and reading to LVB via python bindings.
Signed-off-by: Benny Zlotnik bzlotnik@redhat.com --- tests/python_test.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)
diff --git a/tests/python_test.py b/tests/python_test.py index abd459e..6316790 100644 --- a/tests/python_test.py +++ b/tests/python_test.py @@ -764,3 +764,26 @@ def test_acquire_path_length(no_sanlock_daemon): path = "x" * (constants.SANLK_PATH_LEN - 1) with raises_sanlock_errno(): sanlock.acquire(b"ls_name", b"res_name", [(path, 0)], pid=os.getpid()) + +def test_lvb(tmpdir, sanlock_daemon): + ls_path = str(tmpdir.join("ls_name")) + util.create_file(ls_path, MiB) + + res_path = str(tmpdir.join("res_name")) + util.create_file(res_path, MiB) + + sanlock.write_lockspace(b"ls_name", ls_path, offset=0, iotimeout=1) + sanlock.add_lockspace(b"ls_name", 1, ls_path, offset=0, iotimeout=1) + + disks = [(res_path, 0)] + sanlock.write_resource(b"ls_name", b"res_name", disks) + + fd = sanlock.register() + + sanlock.acquire(b"ls_name", b"res_name", disks, slkfd=fd, lvb=True) + sanlock.set_lvb(b"ls_name", b"res_name", disks, b"{gen:0}") + + result = sanlock.get_lvb(b"ls_name", b"res_name", disks) + sanlock.release(b"ls_name", b"res_name", disks, slkfd=fd) + + assert result == "{gen:0}"
On Mon, Oct 5, 2020 at 1:45 PM Benny Zlotnik bzlotnik@redhat.com wrote:
This patch tests writing and reading to LVB via python bindings.
Signed-off-by: Benny Zlotnik bzlotnik@redhat.com
tests/python_test.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)
diff --git a/tests/python_test.py b/tests/python_test.py index abd459e..6316790 100644 --- a/tests/python_test.py +++ b/tests/python_test.py @@ -764,3 +764,26 @@ def test_acquire_path_length(no_sanlock_daemon): path = "x" * (constants.SANLK_PATH_LEN - 1) with raises_sanlock_errno(): sanlock.acquire(b"ls_name", b"res_name", [(path, 0)], pid=os.getpid())
+def test_lvb(tmpdir, sanlock_daemon):
- ls_path = str(tmpdir.join("ls_name"))
- util.create_file(ls_path, MiB)
- res_path = str(tmpdir.join("res_name"))
- util.create_file(res_path, MiB)
- sanlock.write_lockspace(b"ls_name", ls_path, offset=0, iotimeout=1)
- sanlock.add_lockspace(b"ls_name", 1, ls_path, offset=0, iotimeout=1)
- disks = [(res_path, 0)]
- sanlock.write_resource(b"ls_name", b"res_name", disks)
- fd = sanlock.register()
- sanlock.acquire(b"ls_name", b"res_name", disks, slkfd=fd, lvb=True)
- sanlock.set_lvb(b"ls_name", b"res_name", disks, b"{gen:0}")
Accessing lvm works only when the resource is acquired?
- result = sanlock.get_lvb(b"ls_name", b"res_name", disks)
- sanlock.release(b"ls_name", b"res_name", disks, slkfd=fd)
- assert result == "{gen:0}"
Nice test, but it shows that get_lvb is *decoding* the data on output, which is wrong. The API should accept and return the same type.
In the current form, if I try to set_lvb() with unicode value encoded in another encoding, like:
"\u05d0".encode("cp1255")
b'\xe0'
When reading the python bindings will try to decode using "utf-8" and:
b'\xe0'.decode("utf-8")
Traceback (most recent call last): File "<stdin>", line 1, in <module> UnicodeDecodeError: 'utf-8' codec can't decode byte 0xe0 in position 0: unexpected end of data
The issue is probably using the wrong format string when building the return value.
Other issues that should be tested: - Handling of too long input - the input should not be truncated silently but but fail the call. - Handling of null bytes in the lvm block - is this supported by sanlock or it treats lvm as a null terminated string?
I did not have time yet to review the other patches. The user facing API makes sense except the decoding issue.
Nir
Accessing lvm works only when the resource is acquired?
Yes, needs to be acquired with SANLK_ACQUIRE_LVB flag (added lvb=True for this in patch 1)
Nice test, but it shows that get_lvb is *decoding* the data on output, which is wrong. The API should accept and return the same type.
In the current form, if I try to set_lvb() with unicode value encoded in another encoding, like:
"\u05d0".encode("cp1255")
b'\xe0'
When reading the python bindings will try to decode using "utf-8" and:
b'\xe0'.decode("utf-8")
Traceback (most recent call last): File "<stdin>", line 1, in <module> UnicodeDecodeError: 'utf-8' codec can't decode byte 0xe0 in position 0: unexpected end of data
The issue is probably using the wrong format string when building the return value.
Thanks, it was indeed wrong, used 's' instead of 'y'
Other issues that should be tested:
- Handling of too long input - the input should not be truncated silently but but fail the call.
ack, sanlock returns E2BIG when it's larger than the sector size, will add a test
- Handling of null bytes in the lvm block - is this supported by sanlock or it treats lvm as a null terminated string?
ack, it terminates the string, will add a test
I did not have time yet to review the other patches. The user facing API makes sense except the decoding issue.
Nir
This patch add tests for: - Writing and reading the value to and from LVB - Proper handling of a value that is too long - Null-byte string termination
Signed-off-by: Benny Zlotnik bzlotnik@redhat.com --- Changes in v2: - Adjusted test_lvb to receive bytes in get_lvb as were sent in set_lvb - Added test_lvb_value_too_long - Added test_lvb_null_bytes
tests/python_test.py | 72 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+)
diff --git a/tests/python_test.py b/tests/python_test.py index abd459e..28fd27f 100644 --- a/tests/python_test.py +++ b/tests/python_test.py @@ -764,3 +764,75 @@ def test_acquire_path_length(no_sanlock_daemon): path = "x" * (constants.SANLK_PATH_LEN - 1) with raises_sanlock_errno(): sanlock.acquire(b"ls_name", b"res_name", [(path, 0)], pid=os.getpid()) + + +def test_lvb(tmpdir, sanlock_daemon): + ls_path = str(tmpdir.join("ls_name")) + util.create_file(ls_path, MiB) + + res_path = str(tmpdir.join("res_name")) + util.create_file(res_path, MiB) + + sanlock.write_lockspace(b"ls_name", ls_path, offset=0, iotimeout=1) + sanlock.add_lockspace(b"ls_name", 1, ls_path, offset=0, iotimeout=1) + + disks = [(res_path, 0)] + sanlock.write_resource(b"ls_name", b"res_name", disks) + + fd = sanlock.register() + + sanlock.acquire(b"ls_name", b"res_name", disks, slkfd=fd, lvb=True) + sanlock.set_lvb(b"ls_name", b"res_name", disks, b"{gen:0}") + + result = sanlock.get_lvb(b"ls_name", b"res_name", disks) + sanlock.release(b"ls_name", b"res_name", disks, slkfd=fd) + + assert result == b"{gen:0}" + + +def test_lvb_value_too_long(tmpdir, sanlock_daemon): + ls_path = str(tmpdir.join("ls_name")) + util.create_file(ls_path, MiB) + + res_path = str(tmpdir.join("res_name")) + util.create_file(res_path, MiB) + + sanlock.write_lockspace(b"ls_name", ls_path, offset=0, iotimeout=1) + sanlock.add_lockspace(b"ls_name", 1, ls_path, offset=0, iotimeout=1) + + disks = [(res_path, 0)] + sanlock.write_resource(b"ls_name", b"res_name", disks) + + fd = sanlock.register() + + long_val = b"a" * 513 + sanlock.acquire(b"ls_name", b"res_name", disks, slkfd=fd, lvb=True) + with raises_sanlock_errno(errno.E2BIG): + sanlock.set_lvb(b"ls_name", b"res_name", disks, long_val) + + sanlock.release(b"ls_name", b"res_name", disks, slkfd=fd) + + +def test_lvb_null_bytes(tmpdir, sanlock_daemon): + ls_path = str(tmpdir.join("ls_name")) + util.create_file(ls_path, MiB) + + res_path = str(tmpdir.join("res_name")) + util.create_file(res_path, MiB) + + sanlock.write_lockspace(b"ls_name", ls_path, offset=0, iotimeout=1) + sanlock.add_lockspace(b"ls_name", 1, ls_path, offset=0, iotimeout=1) + + disks = [(res_path, 0)] + sanlock.write_resource(b"ls_name", b"res_name", disks) + + fd = sanlock.register() + + sanlock.acquire(b"ls_name", b"res_name", disks, slkfd=fd, lvb=True) + sanlock.set_lvb(b"ls_name", b"res_name", disks, b"{ge\x00:0}") + + result = sanlock.get_lvb(b"ls_name", b"res_name", disks) + sanlock.release(b"ls_name", b"res_name", disks, slkfd=fd) + + # Check that the string we passed is terminated by the null-byte + assert result == b"{ge"
sanlock-devel@lists.fedorahosted.org