10:48 a.m.
Currently appropriate sanlock flags are accepted for sector size and
alignment parameters. This can be confusing for users, which may try
to pass values instead of flags and this approach also exposes sanlock
internal implementation details.
In order to have consistent and more intuitive API, values should be
accepted for sector size and alignment parameters. This patch replaces
flags by values in calls which accept sector size and alignment.
This patch also adds validation of provided values. If the value is
not the supported one, error with error code EINVAL is thrown.
The supported values are:
sector: 512, 4096
align: 1048576, 2097152, 4194304, 8388608
The patch also adds 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 for invalid align and sector values are also added.
---
python/example.py | 4 +-
python/sanlock.c | 119 +++++++++++++++++++++++++++++++++----------
tests/python_test.py | 59 +++++++++++++++++++++
3 files changed, 152 insertions(+), 30 deletions(-)
diff --git a/python/example.py b/python/example.py
index 4fe845d..cb8f3e9 100644
--- a/python/example.py
+++ b/python/example.py
@@ -29,10 +29,10 @@ def main():
fd = sanlock.register()
print "Initializing '%s'" % (LOCKSPACE_NAME,)
- sanlock.write_lockspace(LOCKSPACE_NAME, disk, max_hosts=0, iotimeout=0,
align=sanlock.ALIGN1M, sector=sanlock.SECTOR512)
+ sanlock.write_lockspace(LOCKSPACE_NAME, disk, max_hosts=0, iotimeout=0,
align=1048576, sector=512)
print "Initializing '%s' on '%s'" % (RESOURCE_NAME,
LOCKSPACE_NAME)
- sanlock.write_resource(LOCKSPACE_NAME, RESOURCE_NAME, SNLK_DISKS,
align=sanlock.ALIGN1M, sector=sanlock.SECTOR512)
+ sanlock.write_resource(LOCKSPACE_NAME, RESOURCE_NAME, SNLK_DISKS, align=1048576,
sector=512)
print "Acquiring the id '%i' on '%s'" % (HOST_ID,
LOCKSPACE_NAME)
sanlock.add_lockspace(LOCKSPACE_NAME, HOST_ID, disk)
diff --git a/python/sanlock.c b/python/sanlock.c
index a9c4cf6..3cd91f2 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -135,6 +135,49 @@ exit_fail:
return -1;
}
+enum {SECTOR_SIZE_512 = 512, SECTOR_SIZE_4K = 4096};
+
+static int
+add_sector_flag(int sector, uint32_t *flags)
+{
+ switch (sector) {
+ case SECTOR_SIZE_512:
+ *flags |= SANLK_LSF_SECTOR512;
+ break;
+ case SECTOR_SIZE_4K:
+ *flags |= SANLK_LSF_SECTOR4K;
+ break;
+ default:
+ PyErr_Format(PyExc_ValueError, "Invalid sector value: %d", sector);
+ return -1;
+ }
+ return 0;
+}
+
+enum {ALIGNMENT_1M = 1048576, ALIGNMENT_2M = 2097152, ALIGNMENT_4M = 4194304,
ALIGNMENT_8M = 8388608};
+
+static int
+add_align_flag(long align, uint32_t *flags)
+{
+ switch (align) {
+ case ALIGNMENT_1M:
+ *flags |= SANLK_RES_ALIGN1M;
+ break;
+ case ALIGNMENT_2M:
+ *flags |= SANLK_RES_ALIGN2M;
+ break;
+ case ALIGNMENT_4M:
+ *flags |= SANLK_RES_ALIGN4M;
+ break;
+ case ALIGNMENT_8M:
+ *flags |= SANLK_RES_ALIGN8M;
+ default:
+ PyErr_Format(PyExc_ValueError, "Invalid align value: %ld", align);
+ return -1;
+ }
+ return 0;
+}
+
static PyObject *
__hosts_to_list(struct sanlk_host *hss, int hss_count)
{
@@ -361,15 +404,17 @@ exit_fail:
/* write_lockspace */
PyDoc_STRVAR(pydoc_write_lockspace, "\
write_lockspace(lockspace, path, offset=0, max_hosts=0, iotimeout=0, \
-align=ALIGN1M, sector=SECTOR512)\n\
-Initialize or update a device to be used as sanlock lockspace.");
+align=1048576, sector=512)\n\
+Initialize or update a device to be used as sanlock lockspace.\n\
+Align can be one of (1048576, 2097152, 4194304, 8388608).\n\
+Sector can be one of (512, 4096).");
static PyObject *
py_write_lockspace(PyObject *self __unused, PyObject *args, PyObject *keywds)
{
- int rv, max_hosts = 0;
+ int rv, max_hosts = 0, sector = SECTOR_SIZE_512;
+ long align = ALIGNMENT_1M;
uint32_t io_timeout = 0;
- uint32_t align=SANLK_LSF_ALIGN1M, sector=SANLK_LSF_SECTOR512;
const char *lockspace, *path;
struct sanlk_lockspace ls;
@@ -380,7 +425,7 @@ py_write_lockspace(PyObject *self __unused, PyObject *args, PyObject
*keywds)
memset(&ls, 0, sizeof(struct sanlk_lockspace));
/* parse python tuple */
- if (!PyArg_ParseTupleAndKeywords(args, keywds, "ss|kiIII", kwlist,
+ if (!PyArg_ParseTupleAndKeywords(args, keywds, "ss|kiIli", kwlist,
&lockspace, &path, &ls.host_id_disk.offset, &max_hosts,
&io_timeout, &align, §or)) {
return NULL;
@@ -391,9 +436,12 @@ py_write_lockspace(PyObject *self __unused, PyObject *args, PyObject
*keywds)
strncpy(ls.host_id_disk.path, path, SANLK_PATH_LEN - 1);
/* set alignment/sector flags */
- ls.flags |= align;
- ls.flags |= sector;
+ if (add_align_flag(align, &ls.flags) == -1)
+ return NULL;
+ if (add_sector_flag(sector, &ls.flags) == -1)
+ return NULL;
+
/* write sanlock lockspace (gil disabled) */
Py_BEGIN_ALLOW_THREADS
rv = sanlock_write_lockspace(&ls, max_hosts, 0, io_timeout);
@@ -409,15 +457,17 @@ py_write_lockspace(PyObject *self __unused, PyObject *args, PyObject
*keywds)
/* read_lockspace */
PyDoc_STRVAR(pydoc_read_lockspace, "\
-read_lockspace(path, offset=0, align=ALIGN1M, sector=SECTOR512)\n -> dict\n\
-Read the lockspace information from a device at a specific offset.");
+read_lockspace(path, offset=0, align=1048576, sector=512)\n -> dict\n\
+Read the lockspace information from a device at a specific offset.\n\
+Align can be one of (1048576, 2097152, 4194304, 8388608).\n\
+Sector can be one of (512, 4096).");
static PyObject *
py_read_lockspace(PyObject *self __unused, PyObject *args, PyObject *keywds)
{
- int rv;
+ int rv, sector = SECTOR_SIZE_512;
+ long align = ALIGNMENT_1M;
uint32_t io_timeout = 0;
- uint32_t align=SANLK_LSF_ALIGN1M, sector=SANLK_LSF_SECTOR512;
const char *path;
struct sanlk_lockspace ls;
PyObject *ls_info = NULL, *ls_entry = NULL;
@@ -428,7 +478,7 @@ py_read_lockspace(PyObject *self __unused, PyObject *args, PyObject
*keywds)
memset(&ls, 0, sizeof(struct sanlk_lockspace));
/* parse python tuple */
- if (!PyArg_ParseTupleAndKeywords(args, keywds, "s|kII", kwlist,
+ if (!PyArg_ParseTupleAndKeywords(args, keywds, "s|kli", kwlist,
&path, &ls.host_id_disk.offset, &align, §or)) {
return NULL;
}
@@ -437,8 +487,11 @@ py_read_lockspace(PyObject *self __unused, PyObject *args, PyObject
*keywds)
strncpy(ls.host_id_disk.path, path, SANLK_PATH_LEN - 1);
/* set alignment/sector flags */
- ls.flags |= align;
- ls.flags |= sector;
+ if (add_align_flag(align, &ls.flags) == -1)
+ return NULL;
+
+ if (add_sector_flag(sector, &ls.flags) == -1)
+ return NULL;
/* read sanlock lockspace (gil disabled) */
Py_BEGIN_ALLOW_THREADS
@@ -481,14 +534,16 @@ exit_fail:
/* read_resource */
PyDoc_STRVAR(pydoc_read_resource, "\
-read_resource(path, offset=0, align=ALIGN1M, sector=SECTOR512) -> dict\n\
-Read the resource information from a device at a specific offset.");
+read_resource(path, offset=0, align=1048576, sector=512) -> dict\n\
+Read the resource information from a device at a specific offset.\n\
+Align can be one of (1048576, 2097152, 4194304, 8388608).\n\
+Sector can be one of (512, 4096).");
static PyObject *
py_read_resource(PyObject *self __unused, PyObject *args, PyObject *keywds)
{
- int rv, rs_len;
- uint32_t align=SANLK_RES_ALIGN1M, sector=SANLK_RES_SECTOR512;
+ int rv, rs_len, sector = SECTOR_SIZE_512;
+ long align = ALIGNMENT_1M;
const char *path;
struct sanlk_resource *rs;
PyObject *rs_info = NULL, *rs_entry = NULL;
@@ -509,7 +564,7 @@ py_read_resource(PyObject *self __unused, PyObject *args, PyObject
*keywds)
rs->num_disks = 1;
/* parse python tuple */
- if (!PyArg_ParseTupleAndKeywords(args, keywds, "s|kII", kwlist,
+ if (!PyArg_ParseTupleAndKeywords(args, keywds, "s|kli", kwlist,
&path, &(rs->disks[0].offset), &align, §or)) {
goto exit_fail;
}
@@ -518,8 +573,11 @@ py_read_resource(PyObject *self __unused, PyObject *args, PyObject
*keywds)
strncpy(rs->disks[0].path, path, SANLK_PATH_LEN - 1);
/* set alignment/sector flags */
- rs->flags |= align;
- rs->flags |= sector;
+ if (add_align_flag(align, &rs->flags) == -1)
+ goto exit_fail;
+
+ if (add_sector_flag(sector, &rs->flags) == -1)
+ goto exit_fail;
/* read sanlock resource (gil disabled) */
Py_BEGIN_ALLOW_THREADS
@@ -573,17 +631,19 @@ exit_fail:
/* write_resource */
PyDoc_STRVAR(pydoc_write_resource, "\
write_resource(lockspace, resource, disks, max_hosts=0, num_hosts=0, \
-clear=False, align=ALIGN1M, sector=SECTOR512)\n\
+clear=False, align=1048576, sector=512)\n\
Initialize a device to be used as sanlock resource.\n\
The disks must be in the format: [(path, offset), ... ].\n\
If clear is True, the resource is cleared so subsequent read will\n\
-return an error.");
+return an error.\n\
+Align can be one of (1048576, 2097152, 4194304, 8388608).\n\
+Sector can be one of (512, 4096).");
static PyObject *
py_write_resource(PyObject *self __unused, PyObject *args, PyObject *keywds)
{
- int rv, max_hosts = 0, num_hosts = 0, clear = 0;
- uint32_t align=SANLK_RES_ALIGN1M, sector=SANLK_RES_SECTOR512;
+ int rv, max_hosts = 0, num_hosts = 0, clear = 0, sector = SECTOR_SIZE_512;
+ long align = ALIGNMENT_1M;
const char *lockspace, *resource;
struct sanlk_resource *rs;
PyObject *disks;
@@ -593,7 +653,7 @@ py_write_resource(PyObject *self __unused, PyObject *args, PyObject
*keywds)
"num_hosts", "clear",
"align", "sector", NULL};
/* parse python tuple */
- if (!PyArg_ParseTupleAndKeywords(args, keywds, "ssO!|iiiII",
+ if (!PyArg_ParseTupleAndKeywords(args, keywds, "ssO!|iiili",
kwlist, &lockspace, &resource, &PyList_Type, &disks,
&max_hosts,
&num_hosts, &clear, &align, §or)) {
return NULL;
@@ -609,8 +669,11 @@ py_write_resource(PyObject *self __unused, PyObject *args, PyObject
*keywds)
strncpy(rs->name, resource, SANLK_NAME_LEN);
/* set alignment/sector flags */
- rs->flags |= align;
- rs->flags |= sector;
+ if (add_align_flag(align, &rs->flags) == -1)
+ goto exit_fail;
+
+ if (add_sector_flag(sector, &rs->flags) == -1)
+ goto exit_fail;
if (clear) {
flags |= SANLK_WRITE_CLEAR;
diff --git a/tests/python_test.py b/tests/python_test.py
index 5029924..5b1bb43 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -23,6 +23,9 @@ LARGE_FILE_SIZE = 1024**4
LOCKSPACE_SIZE = 1024**2
MIN_RES_SIZE = 1024**2
+ALIGNMENT_1M = 1024**2
+SECTOR_SIZE_512 = 512
+
@pytest.mark.parametrize("size,offset", [
# Smallest offset.
@@ -34,11 +37,21 @@ def test_write_lockspace(tmpdir, sanlock_daemon, size, offset):
path = str(tmpdir.join("lockspace"))
util.create_file(path, size)
+ # test read and write with default alignment and sector size values
sanlock.write_lockspace("name", path, offset=offset, iotimeout=1)
ls = sanlock.read_lockspace(path, offset=offset)
assert ls == {"iotimeout": 1, "lockspace": "name"}
+ # test read and write with explicit alignment and sector size values
+ sanlock.write_lockspace(
+ "name", path, offset=offset, iotimeout=1, align=ALIGNMENT_1M,
+ sector=SECTOR_SIZE_512)
+
+ ls = sanlock.read_lockspace(
+ path, offset=offset, align=ALIGNMENT_1M, sector=SECTOR_SIZE_512)
+ assert ls == {"iotimeout": 1, "lockspace": "name"}
+
acquired = sanlock.inq_lockspace(
"name", 1, path, offset=offset, wait=False)
assert acquired is False
@@ -64,6 +77,7 @@ def test_write_resource(tmpdir, sanlock_daemon, size, offset):
util.create_file(path, size)
disks = [(path, offset)]
+ # test read and write with default alignment and sector size values
sanlock.write_resource("ls_name", "res_name", disks)
res = sanlock.read_resource(path, offset=offset)
@@ -73,6 +87,19 @@ def test_write_resource(tmpdir, sanlock_daemon, size, offset):
"version": 0
}
+ # test read and write with explicit alignment and sector size values
+ sanlock.write_resource(
+ "ls_name", "res_name", disks, align=ALIGNMENT_1M,
+ sector=SECTOR_SIZE_512)
+
+ res = sanlock.read_resource(
+ path, offset=offset, align=ALIGNMENT_1M, sector=SECTOR_SIZE_512)
+ assert res == {
+ "lockspace": "ls_name",
+ "resource": "res_name",
+ "version": 0
+ }
+
owners = sanlock.read_resource_owners("ls_name", "res_name",
disks)
assert owners == []
@@ -225,3 +252,35 @@ def test_acquire_release_resource(tmpdir, sanlock_daemon, size,
offset):
owners = sanlock.read_resource_owners("ls_name", "res_name",
disks)
assert owners == []
+
+
+(a)pytest.mark.parametrize("align, sector", [
+ # Invalid alignment
+ (1024, 512),
+ # Invalid sector size
+ (1048576, 8192),
+])
+def test_write_lockspace_invalid_align_sector(
+ tmpdir, sanlock_daemon, align, sector):
+ path = str(tmpdir.join("lockspace"))
+ util.create_file(path, LOCKSPACE_SIZE)
+
+ with pytest.raises(ValueError):
+ sanlock.write_lockspace("name", path, align=align, sector=sector)
+
+
+(a)pytest.mark.parametrize("align, sector", [
+ # Invalid alignment
+ (1024, 512),
+ # Invalid sector size
+ (1048576, 8192),
+])
+def test_write_resource_invalid_align_sector(
+ tmpdir, sanlock_daemon, align, sector):
+ path = str(tmpdir.join("resources"))
+ util.create_file(path, MIN_RES_SIZE)
+ disks = [(path, 0)]
+
+ with pytest.raises(ValueError):
+ sanlock.write_resource(
+ "ls_name", "res_name", disks, align=align,
sector=sector)
--
2.20.1