Re: [PATCH v2 1/6] python: Add align and sector arguments to read_resource_owners()

[Nir Soffer]

On Mon, May 13, 2019 at 3:48 PM Pavel Bar <pbar(a)redhat.com&gt; wrote:

>  On Sun, May 12, 2019 at 6:54 PM Nir Soffer <nirsof(a)gmail.com&gt; 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(a)redhat.com&gt;
> > ---
> >  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
> >
> >