[PATCH 0/3] lvb improvements

[Benny Zlotnik]

The following patchset addresses comment in[1]

- Improve documentation for get/set_lvb
- Add size support to get_lvb
- Add and mprove lvb tests

[1] https://lists.fedorahosted.org/archives/list/[e-mail]...
Github branch: https://github.com/bennyz/sanlock/tree/lvb-fixes

Benny Zlotnik (3):
  python: improve get/set_lvb documentation
  python: support size in get_lvb
  python: improve lvb tests

python/sanlock.c     | 23 ++++++----
 tests/python_test.py | 99 +++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 103 insertions(+), 19 deletions(-)

[PATCH 1/3] python: improve get/set_lvb documentation

[Benny Zlotnik]

- Specify lvb=True explicitly instead of mentioning the sanlock flag
- Document when data is written and visible

Signed-off-by: Benny Zlotnik 
---
 python/sanlock.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/python/sanlock.c b/python/sanlock.c
index fbcabb1..567d86c 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -1607,7 +1607,9 @@ Arguments\n\
 \n\
 Notes\n\
 \n\
-The resource must be acquired with the SANLK_ACQUIRE_LVB flag\n");
+The resource must be acquired with lvb=true.\n\
+The size of data is limited by the sector size (512/4K).\n\
+The new data is visible after the resource is released\n");
 static PyObject *
 py_set_lvb(PyObject *self __unused, PyObject *args, PyObject *keywds)
 {
@@ -1662,7 +1664,7 @@ Returns\n\
   data              data written with set_lvb\n\
 Notes\n\
 \n\
-The resource must be acquired with the SANLK_ACQUIRE_LVB flag\n");
+The resource must be acquired with lvb=True\n");
 static PyObject *
 py_get_lvb(PyObject *self __unused, PyObject *args, PyObject *keywds)
 {

[PATCH 1/3] python: improve get/set_lvb documentation

[Nir Soffer]

On Wed, Dec 9, 2020 at 7:43 PM Benny Zlotnik  wrote:
> Specify lvb=True explicitly instead of mentioning the sanlock flag
> Document when data is written and visible
> Signed-off-by: Benny Zlotnik
>
> python/sanlock.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/python/sanlock.c b/python/sanlock.c
> index fbcabb1..567d86c 100644
> --- a/python/sanlock.c
> +++ b/python/sanlock.c
> @@ -1607,7 +1607,9 @@ Arguments\n\
>  \n\
>  Notes\n\
>  \n\
> -The resource must be acquired with the SANLK_ACQUIRE_LVB flag\n");
> +The resource must be acquired with lvb=true.\n\
> +The size of data is limited by the sector size (512/4K).\n\
> +The new data is visible after the resource is released\n");
>  static PyObject *
>  py_set_lvb(PyObject *self __unused, PyObject *args, PyObject *keywds)
>  {
> @@ -1662,7 +1664,7 @@ Returns\n\
>    data              data written with set_lvb\n\

Missing \n
> Notes\n\
>  \n\
> -The resource must be acquired with the SANLK_ACQUIRE_LVB flag\n");
> +The resource must be acquired with lvb=True\n");

In a later patch you added

size is required and has to be > 0.

Which should be part of this patch. But we should also mention that the size
must be equal or less then the resource sector size.
> static PyObject *
>  py_get_lvb(PyObject *self __unused, PyObject *args, PyObject *keywds)
>  {
> --
> 2.28.0

[PATCH 1/3] python: improve get/set_lvb documentation

[Nir Soffer]

On Wed, Dec 9, 2020 at 10:16 PM Nir Soffer  wrote:
> On Wed, Dec 9, 2020 at 7:43 PM Benny Zlotnik  wrote:
> > Specify lvb=True explicitly instead of mentioning the sanlock flag
> > Document when data is written and visible
> > Signed-off-by: Benny Zlotnik
> >
> > python/sanlock.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/python/sanlock.c b/python/sanlock.c
> > index fbcabb1..567d86c 100644
> > --- a/python/sanlock.c
> > +++ b/python/sanlock.c
> > @@ -1607,7 +1607,9 @@ Arguments\n\
> >  \n\
> >  Notes\n\
> >  \n\
> > -The resource must be acquired with the SANLK_ACQUIRE_LVB flag\n");
> > +The resource must be acquired with lvb=true.\n\
> > +The size of data is limited by the sector size (512/4K).\n\
> > +The new data is visible after the resource is released\n");
> >  static PyObject *
> >  py_set_lvb(PyObject *self __unused, PyObject *args, PyObject *keywds)
> >  {
> > @@ -1662,7 +1664,7 @@ Returns\n\
> >    data              data written with set_lvb\n\
>
> Missing \n

You can check that text using:

$ PYTHONPATH=build/lib.linux-x86_64-3.8 python -c 'import
sanlock;help(sanlock.get_lvb)'
Help on built-in function get_lvb in module sanlock:

get_lvb(...)
    get_lvb(lockspace, resource, disks) -> bytes
    Read Lock Value Block for a given resource

Arguments
      lockspace         lockspace name (str)
      resource          resource name (int)
      disks             path and offset (tuple)
      size              Amount of data to read (int)

Returns
      data              data written with set_lvb
    Notes

The resource must be acquired with lvb=True
    size is required and has to be > 0.
> > Notes\n\
> >  \n\

Newline not needed here
> > -The resource must be acquired with the SANLK_ACQUIRE_LVB flag\n");
> > +The resource must be acquired with lvb=True\n");
>
> In a later patch you added
> size is required and has to be > 0.
>
> Which should be part of this patch.

size is added only in the next patch so this is fine.
> But we should also mention that the size
> must be equal or less then the resource sector size.
> > static PyObject *
> >  py_get_lvb(PyObject *self __unused, PyObject *args, PyObject *keywds)
> >  {
> > --
> > 2.28.0

[PATCH 2/3] python: support size in get_lvb

[Benny Zlotnik]

Add a size parameter to allow the user choose how much data to get back,
the API will now look like this:

sanlock.get_lvb(lockspace, resource, disks, size)

Signed-off-by: Benny Zlotnik 
---
 python/sanlock.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/python/sanlock.c b/python/sanlock.c
index 567d86c..41bbb43 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -1659,26 +1659,28 @@ Arguments\n\
   lockspace         lockspace name (str)\n\
   resource          resource name (int)\n\
   disks             path and offset (tuple)\n\
+  size              amount of data to read (int)\n\
 \n\
 Returns\n\
   data              data written with set_lvb\n\
 Notes\n\
 \n\
-The resource must be acquired with lvb=True\n");
+The resource must be acquired with lvb=True\n\
+size is required and has to be > 0.\n");
 static PyObject *
 py_get_lvb(PyObject *self __unused, PyObject *args, PyObject *keywds)
 {
-    uint32_t flags = 0;
+    uint32_t flags = 0, lvb_size = 0;
     int rv = -1;
     struct sanlk_resource *res = NULL;
     PyObject *lockspace = NULL, *resource = NULL;
     PyObject *disks;
-    char data[512];
+    char *lvb_data = NULL;

-    static char *kwlist[] = {"lockspace", "resource", "disks", NULL};
-    if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&O&O!", kwlist,
+    static char *kwlist[] = {"lockspace", "resource", "disks", "size", NULL};
+    if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&O&O!K", kwlist,
         convert_to_pybytes, &lockspace, convert_to_pybytes, &resource,
-        &PyList_Type, &disks)) {
+        &PyList_Type, &disks, &lvb_size)) {
         goto finally;
     }

@@ -1688,9 +1690,10 @@ py_get_lvb(PyObject *self __unused, PyObject *args, PyObject *keywds)

strncpy(res->lockspace_name, PyBytes_AsString(lockspace), SANLK_NAME_LEN);
     strncpy(res->name, PyBytes_AsString(resource), SANLK_NAME_LEN);
+    lvb_data = calloc(lvb_size, sizeof(char));

Py_BEGIN_ALLOW_THREADS
-    rv = sanlock_get_lvb(flags, res, data, sizeof(data));
+    rv = sanlock_get_lvb(flags, res, lvb_data, lvb_size);
     Py_END_ALLOW_THREADS

if (rv < 0) {
@@ -1705,7 +1708,7 @@ finally:
     if (rv < 0)
         return NULL;

-    return Py_BuildValue("y", data);
+    return Py_BuildValue("y#", lvb_data, lvb_size);
 }

static PyMethodDef
--
2.28.0

[PATCH 2/3] python: support size in get_lvb

[Nir Soffer]

On Wed, Dec 9, 2020 at 7:43 PM Benny Zlotnik  wrote:
> Add a size parameter to allow the user choose how much data to get back,
> the API will now look like this:
> sanlock.get_lvb(lockspace, resource, disks, size)
>
> Signed-off-by: Benny Zlotnik
>
> python/sanlock.c | 19 +++++++++++--------
>  1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/python/sanlock.c b/python/sanlock.c
> index 567d86c..41bbb43 100644
> --- a/python/sanlock.c
> +++ b/python/sanlock.c
> @@ -1659,26 +1659,28 @@ Arguments\n\
>    lockspace         lockspace name (str)\n\
>    resource          resource name (int)\n\
>    disks             path and offset (tuple)\n\
> size              amount of data to read (int)\n\
>
> \n\
>  Returns\n\
>    data              data written with set_lvb\n\

Missing \n
> Notes\n\
>  \n\

Remove \n
> -The resource must be acquired with lvb=True\n");
> +The resource must be acquired with lvb=True\n\
> +size is required and has to be > 0.\n");

Add the upper limit, see comment  I posted by mistake on the previous patch.
> static PyObject *
>  py_get_lvb(PyObject *self __unused, PyObject *args, PyObject *keywds)
>  {
> uint32_t flags = 0;
> uint32_t flags = 0, lvb_size = 0;

Lets use one variable per line in new code.
>  int rv = -1;
>  struct sanlk_resource *res = NULL;
>  PyObject *lockspace = NULL, *resource = NULL;
>  PyObject *disks;
> char data[512];
> char *lvb_data = NULL;
> static char *kwlist[] = {"lockspace", "resource", "disks", NULL};
> if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&O&O!", kwlist,
> static char *kwlist[] = {"lockspace", "resource", "disks", "size", NULL};
> if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&O&O!K", kwlist,

K is unsigned long, we can use I (capital i) for int.
>      convert_to_pybytes, &lockspace, convert_to_pybytes, &resource,
>    &PyList_Type, &disks)) {
>    &PyList_Type, &disks, &lvb_size)) {
>    goto finally;
> }

We need to validate lvb_len. If it is out of range (0 < lvb_len
<4096), we want to
raise a ValueError right here.
> @@ -1688,9 +1690,10 @@ py_get_lvb(PyObject *self __unused, PyObject *args, PyObject *keywds)
>  strncpy(res->lockspace_name, PyBytes_AsString(lockspace), SANLK_NAME_LEN);
>  strncpy(res->name, PyBytes_AsString(resource), SANLK_NAME_LEN);
> lvb_data = calloc(lvb_size, sizeof(char));

calloc(1, lv_len)

(see line 195)
>  Py_BEGIN_ALLOW_THREADS
> rv = sanlock_get_lvb(flags, res, data, sizeof(data));
>
> rv = sanlock_get_lvb(flags, res, lvb_data, lvb_size);
> Py_END_ALLOW_THREADS
>
> if (rv < 0) {
>
> @@ -1705,7 +1708,7 @@ finally:
>      if (rv < 0)
>          return NULL;

You leak lvb_data here...
> return Py_BuildValue("y", data);
> return Py_BuildValue("y#", lvb_data, lvb_size);

and here.

The nicest way would be to create a bytes object from lvb_data,
so it owns the data so we don't have to copy it twice. Python does
not support this, but we can do:

PyObject *result = NULL;
    ...
    result = PyBytes_FromStringAndSize(NULL, lvb_len);
    memset(PyBytes_AS_STRING(result), 0, lvb_len);

And pass the internal buffer to sanlock in the same way.

If the call fail we will clear the value, which decref the object and set
the pointer to NULL;

finally:
    Py_XDECREF(lockspace);
    Py_XDECREF(resource);
    free(res);
    if (rv < 0)
        Py_CLEAR(result);

return result;
> }
> static PyMethodDef
>
> 2.28.0

[PATCH 2/3] python: support size in get_lvb

[Benny Zlotnik]

> Remove \n

ACK
> Add the upper limit, see comment  I posted by mistake on the previous patch.

ACK
> Lets use one variable per line in new code.

ACK
> K is unsigned long, we can use I (capital i) for int.

ACK
> We need to validate lvb_len. If it is out of range (0 < lvb_len
> <4096), we want to
> raise a ValueError right here.

ACK
> calloc(1, lv_len)

ACK
> The nicest way would be to create a bytes object from lvb_data,
> so it owns the data so we don't have to copy it twice. Python does
> not support this, but we can do:
>
> PyObject *result = NULL;
>    ...
>    result = PyBytes_FromStringAndSize(NULL, lvb_len);
>    memset(PyBytes_AS_STRING(result), 0, lvb_len);
>
> And pass the internal buffer to sanlock in the same way.
>
> If the call fail we will clear the value, which decref the object and set
> the pointer to NULL;
>
> finally:
>    Py_XDECREF(lockspace);
>    Py_XDECREF(resource);
>    free(res);
>    if (rv < 0)
>        Py_CLEAR(result);
>
> return result;

ACK

[PATCH 3/3] python: improve lvb tests

[Benny Zlotnik]

- Remove null byte test, it is now tested in the basic test
- Add 4k tests for E2BIG failure
- Add test for writing entire 4k sector
- Add test for reading less than sector size

Signed-off-by: Benny Zlotnik 
---
 tests/python_test.py | 99 +++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 89 insertions(+), 10 deletions(-)

diff --git a/tests/python_test.py b/tests/python_test.py
index 28fd27f..b2e0198 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -780,17 +780,21 @@ def test_lvb(tmpdir, sanlock_daemon):
     sanlock.write_resource(b"ls_name", b"res_name", disks)

fd = sanlock.register()
+    lvb_data = b"first\0second".ljust(512, b"\0")

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}")
+    sanlock.set_lvb(b"ls_name", b"res_name", disks, lvb_data)
+    sanlock.release(b"ls_name", b"res_name", disks, slkfd=fd)
+
+    sanlock.acquire(b"ls_name", b"res_name", disks, slkfd=fd, lvb=True)
+    result = sanlock.get_lvb(b"ls_name", b"res_name", disks, size=512)

-    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}"
+    assert result == lvb_data

-def test_lvb_value_too_long(tmpdir, sanlock_daemon):
+def test_lvb_value_too_long_512(tmpdir, sanlock_daemon):
     ls_path = str(tmpdir.join("ls_name"))
     util.create_file(ls_path, MiB)

@@ -805,15 +809,86 @@ def test_lvb_value_too_long(tmpdir, sanlock_daemon):

fd = sanlock.register()

-    long_val = b"a" * 513
+    long_val = b"a" * (SECTOR_SIZE_512 + 1)
+    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)
+
+
[e-mail]("align", sanlock.ALIGN_SIZE)
+def test_lvb_value_too_long_4k(user_4k_path, sanlock_daemon, align):
+
+    disks = [(user_4k_path, 0)]
+
+    # Poison resource 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(
+        b"ls_name",
+        user_4k_path,
+        iotimeout=1,
+        align=align,
+        sector=SECTOR_SIZE_4K)
+
+    sanlock.add_lockspace(b"ls_name", 1, user_4k_path, offset=0, iotimeout=1)
+
+    sanlock.write_resource(
+        b"ls_name", b"res_name", disks, align=align, sector=SECTOR_SIZE_4K)
+
+    fd = sanlock.register()
+
+    long_val = b"a" * (SECTOR_SIZE_4K + 1)
     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)

+    util.check_guard(user_4k_path, align)
+

-def test_lvb_null_bytes(tmpdir, sanlock_daemon):
[e-mail]("align", sanlock.ALIGN_SIZE)
+def test_lvb_use_entire_sector_4k(user_4k_path, sanlock_daemon, align):
+    disks = [(user_4k_path, 0)]
+
+    # Poison resource 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(
+        b"ls_name",
+        user_4k_path,
+        iotimeout=1,
+        align=align,
+        sector=SECTOR_SIZE_4K)
+
+    sanlock.add_lockspace(b"ls_name", 1, user_4k_path, offset=0, iotimeout=1)
+
+    sanlock.write_resource(
+        b"ls_name", b"res_name", disks, align=align, sector=SECTOR_SIZE_4K)
+
+    fd = sanlock.register()
+
+    long_val = b"a" * SECTOR_SIZE_4K
+    sanlock.acquire(b"ls_name", b"res_name", disks, slkfd=fd, lvb=True)
+    sanlock.set_lvb(b"ls_name", b"res_name", disks, long_val)
+    sanlock.release(b"ls_name", b"res_name", disks, slkfd=fd)
+
+    sanlock.acquire(b"ls_name", b"res_name", disks, slkfd=fd, lvb=True)
+    result = sanlock.get_lvb(b"ls_name", b"res_name", disks,
+                             size=SECTOR_SIZE_4K)
+    sanlock.release(b"ls_name", b"res_name", disks, slkfd=fd)
+    assert result == long_val
+
+    util.check_guard(user_4k_path, align)
+
+
+def test_lvb_read_less_than_cluster_size(tmpdir, sanlock_daemon):
     ls_path = str(tmpdir.join("ls_name"))
     util.create_file(ls_path, MiB)

@@ -827,12 +902,16 @@ def test_lvb_null_bytes(tmpdir, sanlock_daemon):
     sanlock.write_resource(b"ls_name", b"res_name", disks)

fd = sanlock.register()
+    lvb_data = b"first\0second"
+
+    sanlock.acquire(b"ls_name", b"res_name", disks, slkfd=fd, lvb=True)
+    sanlock.set_lvb(b"ls_name", b"res_name", disks, lvb_data.ljust(512, b"\0"))
+    sanlock.release(b"ls_name", b"res_name", disks, slkfd=fd)

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,
+                             size=len(lvb_data))

-    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"
+    assert result == lvb_data
--
2.28.0

[PATCH 3/3] python: improve lvb tests

[Nir Soffer]

On Wed, Dec 9, 2020 at 7:43 PM Benny Zlotnik  wrote:
> Remove null byte test, it is now tested in the basic test
> Add 4k tests for E2BIG failure
> Add test for writing entire 4k sector
> Add test for reading less than sector size
> Signed-off-by: Benny Zlotnik
>
> tests/python_test.py | 99 +++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 89 insertions(+), 10 deletions(-)
>
> diff --git a/tests/python_test.py b/tests/python_test.py
> index 28fd27f..b2e0198 100644
> --- a/tests/python_test.py
> +++ b/tests/python_test.py
> @@ -780,17 +780,21 @@ def test_lvb(tmpdir, sanlock_daemon):
>      sanlock.write_resource(b"ls_name", b"res_name", disks)
>  fd = sanlock.register()
>
> lvb_data = b"first\0second".ljust(512, b"\0")
>
> 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}")
> sanlock.set_lvb(b"ls_name", b"res_name", disks, lvb_data)
> sanlock.release(b"ls_name", b"res_name", disks, slkfd=fd)
> sanlock.acquire(b"ls_name", b"res_name", disks, slkfd=fd, lvb=True)
> result = sanlock.get_lvb(b"ls_name", b"res_name", disks, size=512)

Remove this blank line
> 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}"
> assert result == lvb_data

Nice
> -def test_lvb_value_too_long(tmpdir, sanlock_daemon):
> +def test_lvb_value_too_long_512(tmpdir, sanlock_daemon):
>      ls_path = str(tmpdir.join("ls_name"))
>      util.create_file(ls_path, MiB)
>
> @@ -805,15 +809,86 @@ def test_lvb_value_too_long(tmpdir, sanlock_daemon):
>  fd = sanlock.register()
>
> long_val = b"a" * 513
> long_val = b"a" * (SECTOR_SIZE_512 + 1)
> 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)

Looks good
> sanlock.release(b"ls_name", b"res_name", disks, slkfd=fd)
>
> [e-mail]("align", sanlock.ALIGN_SIZE)
> +def test_lvb_value_too_long_4k(user_4k_path, sanlock_daemon, align):
> disks = [(user_4k_path, 0)]
> # Poison resource 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(
>    b"ls_name",
>    user_4k_path,
>    iotimeout=1,
>    align=align,
>    sector=SECTOR_SIZE_4K)
> sanlock.add_lockspace(b"ls_name", 1, user_4k_path, offset=0, iotimeout=1)
> sanlock.write_resource(
>    b"ls_name", b"res_name", disks, align=align, sector=SECTOR_SIZE_4K)
> fd = sanlock.register()
> long_val = b"a" * (SECTOR_SIZE_4K + 1)
> 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)

This goes to sanlock damon just to fail with invalid value that we can check
in the python side. I think it will be easier to debug issues if we validate the
size in the python binding and raise ValueError with clear error message.

We need another test for size=0. Since we have expensive setup code
for each test,
it will be useful to test all invalid values in the same test.
>  sanlock.release(b"ls_name", b"res_name", disks, slkfd=fd)
>
> util.check_guard(user_4k_path, align)
>
> -def test_lvb_null_bytes(tmpdir, sanlock_daemon):
> [e-mail]("align", sanlock.ALIGN_SIZE)
> +def test_lvb_use_entire_sector_4k(user_4k_path, sanlock_daemon, align):

I think a more consistent name would be test_lvb_4k
> disks = [(user_4k_path, 0)]
> # Poison resource 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(
>    b"ls_name",
>    user_4k_path,
>    iotimeout=1,
>    align=align,
>    sector=SECTOR_SIZE_4K)
> sanlock.add_lockspace(b"ls_name", 1, user_4k_path, offset=0, iotimeout=1)
> sanlock.write_resource(
>    b"ls_name", b"res_name", disks, align=align, sector=SECTOR_SIZE_4K)
> fd = sanlock.register()
> long_val = b"a" * SECTOR_SIZE_4K
> sanlock.acquire(b"ls_name", b"res_name", disks, slkfd=fd, lvb=True)
> sanlock.set_lvb(b"ls_name", b"res_name", disks, long_val)
> sanlock.release(b"ls_name", b"res_name", disks, slkfd=fd)
> sanlock.acquire(b"ls_name", b"res_name", disks, slkfd=fd, lvb=True)
> result = sanlock.get_lvb(b"ls_name", b"res_name", disks,
>                         size=SECTOR_SIZE_4K)
> sanlock.release(b"ls_name", b"res_name", disks, slkfd=fd)
> assert result == long_val
> util.check_guard(user_4k_path, align)

This mostly duplicates the 512 byte tests. We can avoid this using
pytest parameters,
but it require using userstorage module from pip (like ovirt-imageio).
To keep it simple
for now, lets have a check_xxx() helper accepting path and size and
have tiny tests
calling this:

def test_lvb_512():
     path = util.generate_path(...)
     check_lvb(path, 512)

def test_lvb_4k(user_4k_path):
     check_lvb(user_4k_path, 4096)
> +def test_lvb_read_less_than_cluster_size(tmpdir, sanlock_daemon):
>      ls_path = str(tmpdir.join("ls_name"))
>      util.create_file(ls_path, MiB)
>
> @@ -827,12 +902,16 @@ def test_lvb_null_bytes(tmpdir, sanlock_daemon):
>      sanlock.write_resource(b"ls_name", b"res_name", disks)
>  fd = sanlock.register()
>
> lvb_data = b"first\0second"
>
> sanlock.acquire(b"ls_name", b"res_name", disks, slkfd=fd, lvb=True)
>
> sanlock.set_lvb(b"ls_name", b"res_name", disks, lvb_data.ljust(512, b"\0"))
>
> sanlock.release(b"ls_name", b"res_name", disks, slkfd=fd)
>
> 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,
>                         size=len(lvb_data))
>
> 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"
> assert result == lvb_data
>
> --
> 2.28.0