On Fri, Dec 4, 2020 at 12:45 PM Benny Zlotnik <bzlotnik(a)redhat.com> wrote:
Add a binding to the get_lvb function to allow reading data from LVB.
Signed-off-by: Benny Zlotnik <bzlotnik(a)redhat.com>
---
Changes in v2:
- Fix parameter list
- Add return documentation
- Fix resource initialization
- Fix indentation
python/sanlock.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 59 insertions(+)
diff --git a/python/sanlock.c b/python/sanlock.c
index 9647c03..fbcabb1 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -1649,6 +1649,63 @@ finally:
Py_RETURN_NONE;
}
+PyDoc_STRVAR(pydoc_get_lvb, "\
+get_lvb(lockspace, resource, disks) -> bytes\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\
We can make this little more efficient if we add the size of the lvb here.
Maybe keep the same semantics as sanlock:
size=None
read one sector (512 or 4k)
size=n
read exactly n bytes
This is similar to os.read() and os.read(n).
+\n\
+Returns\n\
+ data data written with set_lvb\n\
+Notes\n\
+\n\
+The resource must be acquired with the SANLK_ACQUIRE_LVB flag\n");
lvb=True, see comment in patch 2
+static PyObject *
+py_get_lvb(PyObject *self __unused, PyObject *args, PyObject *keywds)
+{
+ uint32_t flags = 0;
+ int rv = -1;
+ struct sanlk_resource *res = NULL;
+ PyObject *lockspace = NULL, *resource = NULL;
+ PyObject *disks;
+ char data[512];
With 4k storage we can read up to 4k lvb.
For vdsm using 512 bytes should be good enough, but other users may like
to use bigger lvbs, and if we hard code the value here the python binding will
limit the size while the actual sanlock API does not.
I think the best way would be to let the user specify the size, and if
not, allocate
4k bytes using malloc here.
+
+ 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));
Looking at sanlock source, if you specify the size, sanlock will copy
what you asked for.
But if you specify 0 sanlock will copy entire sector:
753 if (!len)
754 len = r->leader.sector_size;
Since we don't have a good way to get the sector size from sanlock, I think we
should pass the size the user specified, and if the user did not
specify, allocate
and zero (calloc) a 4k buffer and pass sanlock lvblen=0, and let sanlock copy
an entire sector.
Looks like sanlock does not keep the size of the lvb passed in
set_lvb, so it cannot
return this value in. It also does not return the size it read when
using lvblen=0, so
we will always get one sector on the client side.
The python binding tries to keep the same semantics of sanlock API.
Looks the best
way to expose this to python is to allow writing and reading binary
blobs, and let the
user manage the data inside the blob.
If we use json on the python side to keep some text about the job, it
should not have
null bytes, since:
>> json.dumps({"null": "\0"})
'{"null": "\\u0000"}'
>> json.dumps({"null": "\0"},
ensure_ascii=False)
'{"null": "\\u0000"}'
So we can do:
lvb = json.dumps(info).encode("utf-8").ljust(512, b"\0")
set_lvb(..., lvb)
lvb = get_lvb(..., size=512).rstrip(b"\0")
json.loads(lvb)
So I think we should not have any issue with this API in vdsm.
+ 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);
This is the reason get_lvb() truncates data on the first null byte,
which may break
users of this API.
We need to use "y#" format, using the size of the lvb we got from sanlock, or
specified by the user.
+}
+
static PyMethodDef
sanlock_methods[] = {
{"register", py_register, METH_NOARGS, pydoc_register},
@@ -1688,6 +1745,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}
};
--
2.28.0