On Mon, May 13, 2019 at 3:48 PM Pavel Bar <pbar@redhat.com> wrote:


On Sun, May 12, 2019 at 6:54 PM Nir Soffer <nirsof@gmail.com> wrote:
We forgot to add the flags to this function, and it works with 4k
storage because sanlock implements a fallback mechanism. Since we
require align and sector arguments in read_resource(), we should also
require them for consistency in read_resource_owners().

Signed-off-by: Nir Soffer <nsoffer@redhat.com>
---
 python/example.py |  2 +-
 python/sanlock.c  | 24 +++++++++++++++++++-----
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/python/example.py b/python/example.py
index cb8f3e9..6ef9f7d 100644
--- a/python/example.py
+++ b/python/example.py
@@ -52,11 +52,11 @@ def main():
                 print "Lockspace '%s' hosts: " % LOCKSPACE_NAME, hosts_list
                 break
             time.sleep(5)
         print "Resource '%s' owners: " % RESOURCE_NAME, \
             sanlock.read_resource_owners(
-                LOCKSPACE_NAME, RESOURCE_NAME, SNLK_DISKS)
+                LOCKSPACE_NAME, RESOURCE_NAME, SNLK_DISKS, align=1048576, sector=512)

Thus is "example.py". Is it important to change it to "MiB"?

I don't think so, the MiB constants are implementation detail of the tests,
not part of sanlock API.
 
 
         print "Releasing '%s' on '%s'" % (RESOURCE_NAME, LOCKSPACE_NAME)
         sanlock.release(LOCKSPACE_NAME, RESOURCE_NAME, SNLK_DISKS, slkfd=fd)
     except Exception as e:
         print "Exception: ", e
     finally:
diff --git a/python/sanlock.c b/python/sanlock.c
index d6841e8..4be9be2 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -1195,30 +1195,36 @@ exit_fail:
     return NULL;
 }

 /* read_resource_owners */
 PyDoc_STRVAR(pydoc_read_resource_owners, "\
-read_resource_owners(lockspace, resource, disks) -> list\n\
+read_resource_owners(lockspace, resource, disks, align=1048576, sector=512) \
+-> list\n\
 Returns the list of hosts owning a resource, the list is not filtered and\n\
 it might contain hosts that are currently failing or dead. The hosts are\n\
 returned in the same format used by get_hosts.\n\
-The disks must be in the format: [(path, offset), ... ]");
+The disks must be in the format: [(path, offset), ... ].\n\
+Align can be one of (1048576, 2097152, 4194304, 8388608).\n\
+Sector can be one of (512, 4096).");

 static PyObject *
 py_read_resource_owners(PyObject *self __unused, PyObject *args, PyObject *keywds)
 {
     int rv, hss_count = 0;
+    int sector = SECTOR_SIZE_512;
+    long align = ALIGNMENT_1M;
     const char *lockspace, *resource;
     struct sanlk_resource *res = NULL;
     struct sanlk_host *hss = NULL;
     PyObject *disks, *ls_list = NULL;

-    static char *kwlist[] = {"lockspace", "resource", "disks", NULL};
+    static char *kwlist[] = {"lockspace", "resource", "disks", "align",
+                             "sector", NULL};

     /* parse python tuple */
-    if (!PyArg_ParseTupleAndKeywords(args, keywds, "ssO!", kwlist,
-        &lockspace, &resource, &PyList_Type, &disks)) {
+    if (!PyArg_ParseTupleAndKeywords(args, keywds, "ssO!|li", kwlist,
+        &lockspace, &resource, &PyList_Type, &disks, &align, &sector)) {
         return NULL;
     }

     /* parse and check sanlock resource */
     if (__parse_resource(disks, &res) < 0) {
@@ -1227,10 +1233,18 @@ py_read_resource_owners(PyObject *self __unused, PyObject *args, PyObject *keywd

     /* prepare sanlock names */
     strncpy(res->lockspace_name, lockspace, SANLK_NAME_LEN);
     strncpy(res->name, resource, SANLK_NAME_LEN);

+    /* set resource alignment/sector flags */

Suggestion: "alignment & sector".

I'll make it nicer before I push the patch.
 
 
+
+    if (add_align_flag(align, &res->flags) == -1)
+        goto exit_fail;
+
+    if (add_sector_flag(sector, &res->flags) == -1)
+        goto exit_fail;
+
     /* read resource owners (gil disabled) */
     Py_BEGIN_ALLOW_THREADS
     rv = sanlock_read_resource_owners(res, 0, &hss, &hss_count);
     Py_END_ALLOW_THREADS

--
2.17.2