[prev in list] [next in list] [prev in thread] [next in thread] 

List:       sanlock-devel
Subject:    Re: [PATCH 3/4] Expose tuples with supported align and sector values
From:       Vojtech Juranek <vjuranek () redhat ! com>
Date:       2019-04-26 6:52:16
Message-ID: 2206430.ciRM3ehC8a () localhost ! localdomain
[Download RAW message or body]

[Attachment #2 (multipart/signed)]


Hi,

> Hi Vojtech
> 
> Please see some questions in-lined below.
> 
> Thanks
> Amit
> 
> On Thu, Apr 25, 2019 at 2:57 PM Vojtech Juranek <vjuranek@redhat.com> wrote:
> > Previous patch switched from accepting align and sector flags to values.
> > Exposing internal sanlock flags doesn't make sense now. Tuples with
> > supported sector size and alignment values should be exposed instead.
> > ---
> > 
> >  python/sanlock.c | 24 ++++++++++++++++--------
> >  1 file changed, 16 insertions(+), 8 deletions(-)
> > 
> > diff --git a/python/sanlock.c b/python/sanlock.c
> > index 323f1bd..f367fbd 100644
> > --- a/python/sanlock.c
> > +++ b/python/sanlock.c
> > @@ -1739,13 +1739,21 @@ initsanlock(void)
> > 
> >      PYSNLK_INIT_ADD_CONSTANT(SANLK_SETEV_REPLACE_EVENT,
> > 
> > "SETEV_REPLACE_EVENT");
> > 
> >      PYSNLK_INIT_ADD_CONSTANT(SANLK_SETEV_ALL_HOSTS,
> > 
> > "SETEV_ALL_HOSTS");
> > 
> > -    /* Sector and align size flags */
> > -    PYSNLK_INIT_ADD_CONSTANT(SANLK_RES_SECTOR512, "SECTOR512");
> > -    PYSNLK_INIT_ADD_CONSTANT(SANLK_RES_SECTOR4K, "SECTOR4K");
> > -    PYSNLK_INIT_ADD_CONSTANT(SANLK_RES_ALIGN1M, "ALIGN1M");
> > -    PYSNLK_INIT_ADD_CONSTANT(SANLK_RES_ALIGN2M, "ALIGN2M");
> > -    PYSNLK_INIT_ADD_CONSTANT(SANLK_RES_ALIGN4M, "ALIGN4M");
> > -    PYSNLK_INIT_ADD_CONSTANT(SANLK_RES_ALIGN8M, "ALIGN8M");
> > -
> > 
> >  #undef PYSNLK_INIT_ADD_CONSTANT
> > 
> > +
> > +    /* Tuples with supported sector size and alignment values */
> > +    PyObject *sector = PyTuple_New(2);
> > +    PyTuple_SetItem(sector, 0, PyInt_FromLong(SECTOR512));
> > +    PyTuple_SetItem(sector, 1, PyInt_FromLong(SECTOR4K));
> > +    if (PyModule_AddObject(py_module, "SECTOR", sector))
> > +        Py_DECREF("SECTOR");
> 
> *shouldn't this be Py_DECREF(sector) ? AFAIK Py_DECREF should get a pointer
> to PyObject *

ops, sure, thanks for spotting it, same for align

> > +
> > +    PyObject *align = PyTuple_New(4);
> > +    PyTuple_SetItem(align, 0, PyInt_FromLong(ALIGN1M));
> > +    PyTuple_SetItem(align, 1, PyInt_FromLong(ALIGN2M));
> > +    PyTuple_SetItem(align, 2, PyInt_FromLong(ALIGN4M));
> > +    PyTuple_SetItem(align, 3, PyInt_FromLong(ALIGN8M));
> > +    if (PyModule_AddObject(py_module, "ALIGN", align))
> > +        Py_DECREF("ALIGN");
> 
> *shouldn't this be Py_DECREF(align) ? AFAIK Py_DECREF should get a pointer
> to PyObject *
> 
> > +
> > 
> >  }
> > 
> > --
> > 2.20.1
> 
> *In What repo/branch are those patches available ? *

official repo is [1], I pushed my changes into my fork on github [2], however, 
patches are handled over email on sanlock-devel ML [3], so you should CC also 
sanlock-devel@lists.fedorahosted.org (you have to subscribe to this ML first 
to be able to post there)

[1] https://pagure.io/sanlock
[2] https://github.com/vjuranek/sanlock/tree/4k_api_update
[3] https://lists.fedorahosted.org/admin/lists/sanlock-devel.lists.fedorahosted.org/
["signature.asc" (application/pgp-signature)]
[Attachment #6 (text/plain)]

_______________________________________________
sanlock-devel mailing list -- sanlock-devel@lists.fedorahosted.org
To unsubscribe send an email to sanlock-devel-leave@lists.fedorahosted.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: https://lists.fedorahosted.org/archives/list/sanlock-devel@lists.fedorahosted.org


[prev in list] [next in list] [prev in thread] [next in thread]