Re: [PATCH 14/18] python: Use PyBytes converter for
read_resource_owners API
by Amit Bawer
i noticed that while working on it, but its more readable to know we handle
the bad situation explicitly.
On Mon, Jun 10, 2019 at 7:07 PM Pavel Bar <pbar(a)redhat.com> wrote:
> If "rv != 0", "ls_list" is NULL, so returning "ls_list" is totally the
> same as returning NULL explicitly. That's what the old code did.
> But I'm totally for leaving the 2 lines that you added, because they do
> add for both readability, maintenance and stability for future changes
> (what if "ls_list" will be updated before we jump to "finally").
>
>
>
> On Mon, Jun 10, 2019 at 6:59 PM Amit Bawer <abawer(a)redhat.com> wrote:
>
>> we check for sanlock rv error and return NULL when rv != 0, this if is
>> not redundant at all.
>>
>> On Mon, Jun 10, 2019 at 6:55 PM Pavel Bar <pbar(a)redhat.com> wrote:
>>
>>> I was talking about other 2 lines:
>>>
>>>> + if (rv != 0)
>>>> + return NULL;
>>>>
>>>
>>>
>>> On Mon, Jun 10, 2019 at 6:53 PM Amit Bawer <abawer(a)redhat.com> wrote:
>>>
>>>> that's why they are called redundant. nobody needs them.
>>>> both pointer are initialized to NULL if not allocated, so call to free
>>>> is safe by ISO-C standards.
>>>>
>>>> On Mon, Jun 10, 2019 at 5:29 PM Pavel Bar <pbar(a)redhat.com> wrote:
>>>>
>>>>>
>>>>>
>>>>> On Mon, Jun 3, 2019 at 11:31 PM Nir Soffer <nirsof(a)gmail.com> wrote:
>>>>>
>>>>>> From: Amit Bawer <abawer(a)redhat.com>
>>>>>>
>>>>>> Implications:
>>>>>>
>>>>>> - Py2 : no difference
>>>>>> - Py3 : if used, only accept bytes for lockspace name.
>>>>>> ---
>>>>>> python/sanlock.c | 33 +++++++++++++++++++--------------
>>>>>> 1 file changed, 19 insertions(+), 14 deletions(-)
>>>>>>
>>>>>> diff --git a/python/sanlock.c b/python/sanlock.c
>>>>>> index 0964671..ed9b4f0 100644
>>>>>> --- a/python/sanlock.c
>>>>>> +++ b/python/sanlock.c
>>>>>> @@ -1226,59 +1226,64 @@ 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 rv = -1, hss_count = 0;
>>>>>> int sector = SECTOR_SIZE_512;
>>>>>> long align = ALIGNMENT_1M;
>>>>>> - const char *lockspace, *resource;
>>>>>> + PyObject *lockspace = NULL, *resource = NULL;
>>>>>> struct sanlk_resource *res = NULL;
>>>>>> struct sanlk_host *hss = NULL;
>>>>>> PyObject *disks, *ls_list = NULL;
>>>>>>
>>>>>> static char *kwlist[] = {"lockspace", "resource", "disks",
>>>>>> "align",
>>>>>> "sector", NULL};
>>>>>>
>>>>>> /* parse python tuple */
>>>>>> - if (!PyArg_ParseTupleAndKeywords(args, keywds, "ssO!|li", kwlist,
>>>>>> - &lockspace, &resource, &PyList_Type, &disks, &align,
>>>>>> §or)) {
>>>>>> - return NULL;
>>>>>> + if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&O&O!|li",
>>>>>> kwlist,
>>>>>> + convert_to_pybytes, &lockspace, convert_to_pybytes,
>>>>>> &resource,
>>>>>> + &PyList_Type, &disks, &align, §or)) {
>>>>>> + goto finally;
>>>>>> }
>>>>>>
>>>>>> /* parse and check sanlock resource */
>>>>>> if (__parse_resource(disks, &res) < 0) {
>>>>>> - return NULL;
>>>>>> + goto finally;
>>>>>> }
>>>>>>
>>>>>> /* prepare sanlock names */
>>>>>> - strncpy(res->lockspace_name, lockspace, SANLK_NAME_LEN);
>>>>>> - strncpy(res->name, resource, SANLK_NAME_LEN);
>>>>>> + strncpy(res->lockspace_name, PyBytes_AsString(lockspace),
>>>>>> SANLK_NAME_LEN);
>>>>>> + strncpy(res->name, PyBytes_AsString(resource), SANLK_NAME_LEN);
>>>>>>
>>>>>> /* set resource alignment and sector flags */
>>>>>>
>>>>>> if (add_align_flag(align, &res->flags) == -1)
>>>>>> - goto exit_fail;
>>>>>> + goto finally;
>>>>>>
>>>>>> if (add_sector_flag(sector, &res->flags) == -1)
>>>>>> - goto exit_fail;
>>>>>> + goto finally;
>>>>>>
>>>>>> /* read resource owners (gil disabled) */
>>>>>> Py_BEGIN_ALLOW_THREADS
>>>>>> rv = sanlock_read_resource_owners(res, 0, &hss, &hss_count);
>>>>>> Py_END_ALLOW_THREADS
>>>>>>
>>>>>> if (rv != 0) {
>>>>>> __set_exception(rv, "Unable to read resource owners");
>>>>>> - goto exit_fail;
>>>>>> + goto finally;
>>>>>> }
>>>>>>
>>>>>> ls_list = __hosts_to_list(hss, hss_count);
>>>>>>
>>>>>> -exit_fail:
>>>>>> - if (res) free(res);
>>>>>> - if (hss) free(hss);
>>>>>> +finally:
>>>>>> + Py_XDECREF(lockspace);
>>>>>> + Py_XDECREF(resource);
>>>>>> + free(res);
>>>>>> + free(hss);
>>>>>> + if (rv != 0)
>>>>>> + return NULL;
>>>>>>
>>>>>
>>>>> The above 2 lines (the "if" statement) are redundant, that's why they
>>>>> were not in the original code. Although they do add for readability, so
>>>>> better leave them.
>>>>>
>>>>>
>>>>>> return ls_list;
>>>>>> }
>>>>>>
>>>>>> /* killpath */
>>>>>> PyDoc_STRVAR(pydoc_killpath, "\
>>>>>> --
>>>>>> 2.17.2
>>>>>>
>>>>>>
4 years, 10 months
Re: [PATCH 14/18] python: Use PyBytes converter for
read_resource_owners API
by Amit Bawer
we check for sanlock rv error and return NULL when rv != 0, this if is not
redundant at all.
On Mon, Jun 10, 2019 at 6:55 PM Pavel Bar <pbar(a)redhat.com> wrote:
> I was talking about other 2 lines:
>
>> + if (rv != 0)
>> + return NULL;
>>
>
>
> On Mon, Jun 10, 2019 at 6:53 PM Amit Bawer <abawer(a)redhat.com> wrote:
>
>> that's why they are called redundant. nobody needs them.
>> both pointer are initialized to NULL if not allocated, so call to free is
>> safe by ISO-C standards.
>>
>> On Mon, Jun 10, 2019 at 5:29 PM Pavel Bar <pbar(a)redhat.com> wrote:
>>
>>>
>>>
>>> On Mon, Jun 3, 2019 at 11:31 PM Nir Soffer <nirsof(a)gmail.com> wrote:
>>>
>>>> From: Amit Bawer <abawer(a)redhat.com>
>>>>
>>>> Implications:
>>>>
>>>> - Py2 : no difference
>>>> - Py3 : if used, only accept bytes for lockspace name.
>>>> ---
>>>> python/sanlock.c | 33 +++++++++++++++++++--------------
>>>> 1 file changed, 19 insertions(+), 14 deletions(-)
>>>>
>>>> diff --git a/python/sanlock.c b/python/sanlock.c
>>>> index 0964671..ed9b4f0 100644
>>>> --- a/python/sanlock.c
>>>> +++ b/python/sanlock.c
>>>> @@ -1226,59 +1226,64 @@ 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 rv = -1, hss_count = 0;
>>>> int sector = SECTOR_SIZE_512;
>>>> long align = ALIGNMENT_1M;
>>>> - const char *lockspace, *resource;
>>>> + PyObject *lockspace = NULL, *resource = NULL;
>>>> struct sanlk_resource *res = NULL;
>>>> struct sanlk_host *hss = NULL;
>>>> PyObject *disks, *ls_list = NULL;
>>>>
>>>> static char *kwlist[] = {"lockspace", "resource", "disks", "align",
>>>> "sector", NULL};
>>>>
>>>> /* parse python tuple */
>>>> - if (!PyArg_ParseTupleAndKeywords(args, keywds, "ssO!|li", kwlist,
>>>> - &lockspace, &resource, &PyList_Type, &disks, &align, §or))
>>>> {
>>>> - return NULL;
>>>> + if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&O&O!|li", kwlist,
>>>> + convert_to_pybytes, &lockspace, convert_to_pybytes, &resource,
>>>> + &PyList_Type, &disks, &align, §or)) {
>>>> + goto finally;
>>>> }
>>>>
>>>> /* parse and check sanlock resource */
>>>> if (__parse_resource(disks, &res) < 0) {
>>>> - return NULL;
>>>> + goto finally;
>>>> }
>>>>
>>>> /* prepare sanlock names */
>>>> - strncpy(res->lockspace_name, lockspace, SANLK_NAME_LEN);
>>>> - strncpy(res->name, resource, SANLK_NAME_LEN);
>>>> + strncpy(res->lockspace_name, PyBytes_AsString(lockspace),
>>>> SANLK_NAME_LEN);
>>>> + strncpy(res->name, PyBytes_AsString(resource), SANLK_NAME_LEN);
>>>>
>>>> /* set resource alignment and sector flags */
>>>>
>>>> if (add_align_flag(align, &res->flags) == -1)
>>>> - goto exit_fail;
>>>> + goto finally;
>>>>
>>>> if (add_sector_flag(sector, &res->flags) == -1)
>>>> - goto exit_fail;
>>>> + goto finally;
>>>>
>>>> /* read resource owners (gil disabled) */
>>>> Py_BEGIN_ALLOW_THREADS
>>>> rv = sanlock_read_resource_owners(res, 0, &hss, &hss_count);
>>>> Py_END_ALLOW_THREADS
>>>>
>>>> if (rv != 0) {
>>>> __set_exception(rv, "Unable to read resource owners");
>>>> - goto exit_fail;
>>>> + goto finally;
>>>> }
>>>>
>>>> ls_list = __hosts_to_list(hss, hss_count);
>>>>
>>>> -exit_fail:
>>>> - if (res) free(res);
>>>> - if (hss) free(hss);
>>>> +finally:
>>>> + Py_XDECREF(lockspace);
>>>> + Py_XDECREF(resource);
>>>> + free(res);
>>>> + free(hss);
>>>> + if (rv != 0)
>>>> + return NULL;
>>>>
>>>
>>> The above 2 lines (the "if" statement) are redundant, that's why they
>>> were not in the original code. Although they do add for readability, so
>>> better leave them.
>>>
>>>
>>>> return ls_list;
>>>> }
>>>>
>>>> /* killpath */
>>>> PyDoc_STRVAR(pydoc_killpath, "\
>>>> --
>>>> 2.17.2
>>>>
>>>>
4 years, 10 months
Re: [PATCH 11/18] python: Use PyBytes converter for resource acquire API
by Amit Bawer
the original check here is for rv != 0. and cleanup code checks the same.
if we want to change the rv checks for sanlock calls we probably should to
that in a separate patch, not related to py3 changes anyway.
On Mon, Jun 10, 2019 at 3:41 PM Pavel Bar <pbar(a)redhat.com> wrote:
>
>
> On Mon, Jun 3, 2019 at 11:31 PM Nir Soffer <nirsof(a)gmail.com> wrote:
>
>> From: Amit Bawer <abawer(a)redhat.com>
>>
>> Implications:
>>
>> - Py2 : no difference
>> - Py3 : if used, only accept bytes for lockspace/resource name.
>> ---
>> python/sanlock.c | 35 ++++++++++++++++++-----------------
>> 1 file changed, 18 insertions(+), 17 deletions(-)
>>
>> diff --git a/python/sanlock.c b/python/sanlock.c
>> index c7518c6..7bdbd69 100644
>> --- a/python/sanlock.c
>> +++ b/python/sanlock.c
>> @@ -1028,39 +1028,39 @@ mode. The version is the version of the lease
>> that must be acquired or fail.\n\
>> The disks must be in the format: [(path, offset), ... ]\n");
>>
>> static PyObject *
>> py_acquire(PyObject *self __unused, PyObject *args, PyObject *keywds)
>> {
>> - int rv, sanlockfd = -1, pid = -1, shared = 0;
>> - const char *lockspace, *resource;
>> - struct sanlk_resource *res;
>> + int rv = -1, sanlockfd = -1, pid = -1, shared = 0;
>> + PyObject *lockspace = NULL, *resource = NULL;
>> + struct sanlk_resource *res = NULL;
>> PyObject *disks, *version = Py_None;
>>
>> static char *kwlist[] = {"lockspace", "resource", "disks", "slkfd",
>> "pid", "shared", "version", NULL};
>>
>> /* parse python tuple */
>> - if (!PyArg_ParseTupleAndKeywords(args, keywds, "ssO!|iiiO", kwlist,
>> - &lockspace, &resource, &PyList_Type, &disks, &sanlockfd, &pid,
>> - &shared, &version)) {
>> - return NULL;
>> + if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&O&O!|iiiO", kwlist,
>> + convert_to_pybytes, &lockspace, convert_to_pybytes, &resource,
>> + &PyList_Type, &disks, &sanlockfd, &pid, &shared, &version)) {
>> + goto finally;
>> }
>>
>> /* check if any of the slkfd or pid parameters was given */
>> if (sanlockfd == -1 && pid == -1) {
>> __set_exception(EINVAL, "Invalid slkfd and pid values");
>> - return NULL;
>> + goto finally;
>> }
>>
>> /* parse and check sanlock resource */
>> if (__parse_resource(disks, &res) < 0) {
>> - return NULL;
>> + goto finally;
>> }
>>
>> /* prepare sanlock names */
>> - strncpy(res->lockspace_name, lockspace, SANLK_NAME_LEN);
>> - strncpy(res->name, resource, SANLK_NAME_LEN);
>> + strncpy(res->lockspace_name, PyBytes_AsString(lockspace),
>> SANLK_NAME_LEN);
>> + strncpy(res->name, PyBytes_AsString(resource), SANLK_NAME_LEN);
>>
>> /* prepare sanlock flags */
>> if (shared) {
>> res->flags |= SANLK_RES_SHARED;
>> }
>> @@ -1069,30 +1069,31 @@ py_acquire(PyObject *self __unused, PyObject
>> *args, PyObject *keywds)
>> if (version != Py_None) {
>> res->flags |= SANLK_RES_LVER;
>> res->lver = pyinteger_as_unsigned_long_long_mask(version);
>> if (res->lver == (uint64_t)-1) {
>> __set_exception(EINVAL, "Unable to convert the version
>> value");
>> - goto exit_fail;
>> + goto finally;
>> }
>> }
>>
>> /* acquire sanlock resource (gil disabled) */
>> Py_BEGIN_ALLOW_THREADS
>> rv = sanlock_acquire(sanlockfd, pid, 0, 1, &res, 0);
>> Py_END_ALLOW_THREADS
>>
>> if (rv != 0) {
>> __set_exception(rv, "Sanlock resource not acquired");
>> - goto exit_fail;
>> + goto finally;
>> }
>>
>> +finally:
>> + Py_XDECREF(lockspace);
>> + Py_XDECREF(resource);
>> free(res);
>> + if (rv != 0)
>>
>
> Is it intentional "rv != 0" instead of "rv < 0"? I do see "rv != 0" in the
> existing code a few lines above in the method, but wasn't it a mistake? If
> the convention is that any non-negative is a success, then it should be
> changed.
>
>
>> + return NULL;
>> Py_RETURN_NONE;
>> -
>> -exit_fail:
>> - free(res);
>> - return NULL;
>> }
>>
>> /* release */
>> PyDoc_STRVAR(pydoc_release, "\
>> release(lockspace, resource, disks [, slkfd=fd, pid=owner])\n\
>> --
>> 2.17.2
>>
>>
4 years, 10 months
Re: [PATCH 18/18] tests: Remove xfail marks for unsupported
lockspace/res names as bytes
by Amit Bawer
guess we could omit the "for".
"specifically" is good enough.
On Mon, Jun 10, 2019 at 6:30 PM Pavel Bar <pbar(a)redhat.com> wrote:
>
>
> On Mon, Jun 3, 2019 at 11:31 PM Nir Soffer <nirsof(a)gmail.com> wrote:
>
>> From: Amit Bawer <abawer(a)redhat.com>
>>
>> From this point, all API tests for should PASS or xfail specifically.
>>
>
> Redundant "for". Or need to add something after "for".
> Suggestion if I understood you correctly: "specifically" --> "explicitly"
>
>
>> XPASS is only known for python 3 on read invalid 4k sector size test.
>> ---
>> tests/python_test.py | 21 +--------------------
>> 1 file changed, 1 insertion(+), 20 deletions(-)
>>
>> diff --git a/tests/python_test.py b/tests/python_test.py
>> index 7093aac..926a0f0 100644
>> --- a/tests/python_test.py
>> +++ b/tests/python_test.py
>> @@ -51,24 +51,21 @@ FILE_NAMES = [
>> reason="currently not supporting bytes paths")),
>> ]
>>
>> LOCKSPACE_OR_RESOURCE_NAMES = [
>> # Bytes are supported with python 2 and 3.
>> - pytest.param(
>> - b"\xd7\x90",
>> - marks=pytest.mark.xfail(six.PY3, reason="bytes support not
>> implemented yet")),
>> + pytest.param(b"\xd7\x90"),
>> # Python 2 also supports str.
>> pytest.param(
>> "\xd7\x90",
>> marks=pytest.mark.skipif(six.PY3, reason="python 3 supports only
>> bytes")),
>> # Python 2 also supports unicode with ascii content.
>> pytest.param(
>> u"ascii",
>> marks=pytest.mark.skipif(six.PY3, reason="python 3 supports only
>> bytes")),
>> ]
>>
>> -(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes
>> are unsupported yet")
>> @pytest.mark.parametrize("filename, encoding" , FILE_NAMES)
>> @pytest.mark.parametrize("size,offset", [
>> # Smallest offset.
>> (LOCKSPACE_SIZE, 0),
>> # Large offset.
>> @@ -105,11 +102,10 @@ def test_write_lockspace(tmpdir, sanlock_daemon,
>> filename, encoding, size, offse
>> # TODO: check more stuff here...
>>
>> util.check_guard(path, size)
>>
>>
>> -(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes
>> are unsupported yet")
>> @pytest.mark.parametrize("align", sanlock.ALIGN_SIZE)
>> def test_write_lockspace_4k(user_4k_path, sanlock_daemon, align):
>>
>> # Poison lockspace area, ensuring that previous tests will not break
>> this
>> # test, and sanlock does not write beyond the lockspace area.
>> @@ -135,11 +131,10 @@ def test_write_lockspace_4k(user_4k_path,
>> sanlock_daemon, align):
>>
>> # Check that sanlock did not write beyond the lockspace area.
>> util.check_guard(user_4k_path, align)
>>
>>
>> -(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes
>> are unsupported yet")
>> def test_write_lockspace_4k_invalid_sector_size(sanlock_daemon,
>> user_4k_path):
>> with pytest.raises(sanlock.SanlockException) as e:
>> sanlock.write_lockspace(
>> b"ls_name", user_4k_path, iotimeout=1,
>> sector=SECTOR_SIZE_512)
>> assert e.value.errno == errno.EINVAL
>> @@ -153,11 +148,10 @@ def
>> test_read_lockspace_4k_invalid_sector_size(sanlock_daemon, user_4k_path):
>> with pytest.raises(sanlock.SanlockException) as e:
>> sanlock.read_lockspace(user_4k_path, sector=SECTOR_SIZE_512)
>> assert e.value.errno == errno.EINVAL
>>
>>
>> -(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes
>> are unsupported yet")
>> @pytest.mark.parametrize("filename,encoding", FILE_NAMES)
>> @pytest.mark.parametrize("size,offset", [
>> # Smallest offset.
>> (MIN_RES_SIZE, 0),
>> # Large offset.
>> @@ -202,11 +196,10 @@ def test_write_resource(tmpdir, sanlock_daemon,
>> filename, encoding, size, offset
>> # TODO: check more stuff here...
>>
>> util.check_guard(path, size)
>>
>>
>> -(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes
>> are unsupported yet")
>> @pytest.mark.parametrize("align", sanlock.ALIGN_SIZE)
>> def test_write_resource_4k(sanlock_daemon, user_4k_path, align):
>> disks = [(user_4k_path, 0)]
>>
>> # Poison resource area, ensuring that previous tests will not break
>> this
>> @@ -248,11 +241,10 @@ def
>> test_write_resource_4k_invalid_sector_size(sanlock_daemon, user_4k_path):
>> sanlock.write_resource(
>> b"ls_name", b"res_name", disks, sector=SECTOR_SIZE_512)
>> assert e.value.errno == errno.EINVAL
>>
>>
>> -(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes
>> are unsupported yet")
>> def test_read_resource_4k_invalid_sector_size(sanlock_daemon,
>> user_4k_path):
>> disks = [(user_4k_path, 0)]
>>
>> sanlock.write_resource(
>> b"ls_name",
>> @@ -264,11 +256,10 @@ def
>> test_read_resource_4k_invalid_sector_size(sanlock_daemon, user_4k_path):
>> with pytest.raises(sanlock.SanlockException) as e:
>> sanlock.read_resource(user_4k_path, sector=SECTOR_SIZE_512)
>> assert e.value.errno == errno.EINVAL
>>
>>
>> -(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes
>> are unsupported yet")
>> def test_read_resource_owners_4k_invalid_sector_size(
>> sanlock_daemon, user_4k_path):
>> disks = [(user_4k_path, 0)]
>>
>> sanlock.write_resource(
>> @@ -282,11 +273,10 @@ def
>> test_read_resource_owners_4k_invalid_sector_size(
>> sanlock.read_resource_owners(
>> b"ls_name", b"res_name", disks, sector=SECTOR_SIZE_512)
>> assert e.value.errno == errno.EINVAL
>>
>>
>> -(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes
>> are unsupported yet")
>> def test_read_resource_owners_invalid_align_size(tmpdir, sanlock_daemon):
>> path = str(tmpdir.join("path"))
>> util.create_file(path, GiB)
>> disks = [(path, 0)]
>>
>> @@ -305,11 +295,10 @@ def
>> test_read_resource_owners_invalid_align_size(tmpdir, sanlock_daemon):
>> align=ALIGNMENT_2M,
>> sector=SECTOR_SIZE_512)
>> assert e.value.errno == errno.EINVAL
>>
>>
>> -(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes
>> are unsupported yet")
>> @pytest.mark.parametrize("size,offset", [
>> # Smallest offset.
>> (MIN_RES_SIZE, 0),
>> # Large offset.
>> (LARGE_FILE_SIZE, LARGE_FILE_SIZE - MIN_RES_SIZE),
>> @@ -350,11 +339,10 @@ def test_add_rem_lockspace(tmpdir, sanlock_daemon,
>> size, offset):
>>
>> lockspaces = sanlock.get_lockspaces()
>> assert lockspaces == []
>>
>>
>> -(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes
>> are unsupported yet")
>> def test_add_rem_lockspace_async(tmpdir, sanlock_daemon):
>> path = str(tmpdir.join("ls_name"))
>> util.create_file(path, MiB)
>>
>> sanlock.write_lockspace(b"ls_name", path, iotimeout=1)
>> @@ -387,11 +375,10 @@ def test_add_rem_lockspace_async(tmpdir,
>> sanlock_daemon):
>> # Once the lockspace was released, we expect to get False.
>> acquired = sanlock.inq_lockspace(b"ls_name", 1, path, wait=True)
>> assert acquired is False
>>
>>
>> -(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes
>> are unsupported yet")
>> @pytest.mark.parametrize("size,offset", [
>> # Smallest offset.
>> (MIN_RES_SIZE, 0),
>> # Large offset.
>> (LARGE_FILE_SIZE, LARGE_FILE_SIZE - MIN_RES_SIZE),
>> @@ -461,11 +448,10 @@ def test_acquire_release_resource(tmpdir,
>> sanlock_daemon, size, offset):
>>
>> owners = sanlock.read_resource_owners(b"ls_name", b"res_name", disks)
>> assert owners == []
>>
>>
>> -(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes
>> are unsupported yet")
>> @pytest.mark.parametrize("align, sector", [
>> # Invalid alignment
>> (KiB, sanlock.SECTOR_SIZE[0]),
>> # Invalid sector size
>> (sanlock.ALIGN_SIZE[0], 8 * KiB),
>> @@ -477,11 +463,10 @@ def test_write_lockspace_invalid_align_sector(
>>
>> with pytest.raises(ValueError):
>> sanlock.write_lockspace(b"ls_name", path, align=align,
>> sector=sector)
>>
>>
>> -(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes
>> are unsupported yet")
>> @pytest.mark.parametrize("align, sector", [
>> # Invalid alignment
>> (KiB, sanlock.SECTOR_SIZE[0]),
>> # Invalid sector size
>> (sanlock.ALIGN_SIZE[0], 8 * KiB),
>> @@ -495,11 +480,10 @@ def test_write_resource_invalid_align_sector(
>> with pytest.raises(ValueError):
>> sanlock.write_resource(
>> b"ls_name", b"res_name", disks, align=align, sector=sector)
>>
>>
>> -(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes
>> are unsupported yet")
>> @pytest.mark.parametrize("disk", [
>> # Not a tuple - unicode and bytes:
>> "not a tuple",
>> b"not a tuple",
>> u'\u05e9\u05dc\u05d5\u05dd',
>> @@ -550,31 +534,28 @@ def
>> test_add_lockspace_parse_args(no_sanlock_daemon, name):
>> def test_write_lockspace_parse_args(no_sanlock_daemon, name):
>> with raises_sanlock_errno():
>> sanlock.write_lockspace(name, "ls_path")
>>
>>
>> -(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes
>> are unsupported yet")
>> @pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
>> def test_write_resource_parse_args(no_sanlock_daemon, name):
>> with raises_sanlock_errno():
>> sanlock.write_resource(name, b"res_name", [("disk_path",0)])
>>
>> with raises_sanlock_errno():
>> sanlock.write_resource(b"ls_name", name, [("disk_path",0)])
>>
>>
>> -(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes
>> are unsupported yet")
>> @pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
>> def test_release_resource_parse_args(no_sanlock_daemon, name):
>> with raises_sanlock_errno():
>> sanlock.release(name, b"res_name", [("disk_path",0)])
>>
>> with raises_sanlock_errno():
>> sanlock.release(b"ls_name", name, [("disk_path",0)])
>>
>>
>> -(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes
>> are unsupported yet")
>> @pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
>> def test_read_resource_owners_parse_args(no_sanlock_daemon, name):
>> with raises_sanlock_errno():
>> sanlock.read_resource_owners(name, b"res_name",
>> [("disk_path",0)])
>>
>> --
>> 2.17.2
>>
>>
4 years, 10 months
Re: [PATCH 14/18] python: Use PyBytes converter for
read_resource_owners API
by Amit Bawer
that's why they are called redundant. nobody needs them.
both pointer are initialized to NULL if not allocated, so call to free is
safe by ISO-C standards.
On Mon, Jun 10, 2019 at 5:29 PM Pavel Bar <pbar(a)redhat.com> wrote:
>
>
> On Mon, Jun 3, 2019 at 11:31 PM Nir Soffer <nirsof(a)gmail.com> wrote:
>
>> From: Amit Bawer <abawer(a)redhat.com>
>>
>> Implications:
>>
>> - Py2 : no difference
>> - Py3 : if used, only accept bytes for lockspace name.
>> ---
>> python/sanlock.c | 33 +++++++++++++++++++--------------
>> 1 file changed, 19 insertions(+), 14 deletions(-)
>>
>> diff --git a/python/sanlock.c b/python/sanlock.c
>> index 0964671..ed9b4f0 100644
>> --- a/python/sanlock.c
>> +++ b/python/sanlock.c
>> @@ -1226,59 +1226,64 @@ 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 rv = -1, hss_count = 0;
>> int sector = SECTOR_SIZE_512;
>> long align = ALIGNMENT_1M;
>> - const char *lockspace, *resource;
>> + PyObject *lockspace = NULL, *resource = NULL;
>> struct sanlk_resource *res = NULL;
>> struct sanlk_host *hss = NULL;
>> PyObject *disks, *ls_list = NULL;
>>
>> static char *kwlist[] = {"lockspace", "resource", "disks", "align",
>> "sector", NULL};
>>
>> /* parse python tuple */
>> - if (!PyArg_ParseTupleAndKeywords(args, keywds, "ssO!|li", kwlist,
>> - &lockspace, &resource, &PyList_Type, &disks, &align, §or)) {
>> - return NULL;
>> + if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&O&O!|li", kwlist,
>> + convert_to_pybytes, &lockspace, convert_to_pybytes, &resource,
>> + &PyList_Type, &disks, &align, §or)) {
>> + goto finally;
>> }
>>
>> /* parse and check sanlock resource */
>> if (__parse_resource(disks, &res) < 0) {
>> - return NULL;
>> + goto finally;
>> }
>>
>> /* prepare sanlock names */
>> - strncpy(res->lockspace_name, lockspace, SANLK_NAME_LEN);
>> - strncpy(res->name, resource, SANLK_NAME_LEN);
>> + strncpy(res->lockspace_name, PyBytes_AsString(lockspace),
>> SANLK_NAME_LEN);
>> + strncpy(res->name, PyBytes_AsString(resource), SANLK_NAME_LEN);
>>
>> /* set resource alignment and sector flags */
>>
>> if (add_align_flag(align, &res->flags) == -1)
>> - goto exit_fail;
>> + goto finally;
>>
>> if (add_sector_flag(sector, &res->flags) == -1)
>> - goto exit_fail;
>> + goto finally;
>>
>> /* read resource owners (gil disabled) */
>> Py_BEGIN_ALLOW_THREADS
>> rv = sanlock_read_resource_owners(res, 0, &hss, &hss_count);
>> Py_END_ALLOW_THREADS
>>
>> if (rv != 0) {
>> __set_exception(rv, "Unable to read resource owners");
>> - goto exit_fail;
>> + goto finally;
>> }
>>
>> ls_list = __hosts_to_list(hss, hss_count);
>>
>> -exit_fail:
>> - if (res) free(res);
>> - if (hss) free(hss);
>> +finally:
>> + Py_XDECREF(lockspace);
>> + Py_XDECREF(resource);
>> + free(res);
>> + free(hss);
>> + if (rv != 0)
>> + return NULL;
>>
>
> The above 2 lines (the "if" statement) are redundant, that's why they were
> not in the original code. Although they do add for readability, so better
> leave them.
>
>
>> return ls_list;
>> }
>>
>> /* killpath */
>> PyDoc_STRVAR(pydoc_killpath, "\
>> --
>> 2.17.2
>>
>>
4 years, 10 months
[sanlock] 18/18: tests: Remove xfail marks for unsupported
lockspace/res names as bytes
by pagure＠pagure.io
This is an automated email from the git hooks/post-receive script.
nsoffer pushed a commit to branch master
in repository sanlock.
commit 2d8e8b35120d9369babbd48b99dc09ce96db6549
Author: Amit Bawer <abawer(a)redhat.com>
AuthorDate: Sun Jun 2 18:02:04 2019 +0300
tests: Remove xfail marks for unsupported lockspace/res names as bytes
From this point, all API tests for should PASS or xfail specifically.
XPASS is only known for python 3 on read invalid 4k sector size test.
---
tests/python_test.py | 21 +--------------------
1 file changed, 1 insertion(+), 20 deletions(-)
diff --git a/tests/python_test.py b/tests/python_test.py
index 7093aac..926a0f0 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -53,9 +53,7 @@ FILE_NAMES = [
LOCKSPACE_OR_RESOURCE_NAMES = [
# Bytes are supported with python 2 and 3.
- pytest.param(
- b"\xd7\x90",
- marks=pytest.mark.xfail(six.PY3, reason="bytes support not implemented yet")),
+ pytest.param(b"\xd7\x90"),
# Python 2 also supports str.
pytest.param(
"\xd7\x90",
@@ -66,7 +64,6 @@ LOCKSPACE_OR_RESOURCE_NAMES = [
marks=pytest.mark.skipif(six.PY3, reason="python 3 supports only bytes")),
]
-(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet")
@pytest.mark.parametrize("filename, encoding" , FILE_NAMES)
@pytest.mark.parametrize("size,offset", [
# Smallest offset.
@@ -107,7 +104,6 @@ def test_write_lockspace(tmpdir, sanlock_daemon, filename, encoding, size, offse
util.check_guard(path, size)
-(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet")
@pytest.mark.parametrize("align", sanlock.ALIGN_SIZE)
def test_write_lockspace_4k(user_4k_path, sanlock_daemon, align):
@@ -137,7 +133,6 @@ def test_write_lockspace_4k(user_4k_path, sanlock_daemon, align):
util.check_guard(user_4k_path, align)
-(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet")
def test_write_lockspace_4k_invalid_sector_size(sanlock_daemon, user_4k_path):
with pytest.raises(sanlock.SanlockException) as e:
sanlock.write_lockspace(
@@ -155,7 +150,6 @@ def test_read_lockspace_4k_invalid_sector_size(sanlock_daemon, user_4k_path):
assert e.value.errno == errno.EINVAL
-(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet")
@pytest.mark.parametrize("filename,encoding", FILE_NAMES)
@pytest.mark.parametrize("size,offset", [
# Smallest offset.
@@ -204,7 +198,6 @@ def test_write_resource(tmpdir, sanlock_daemon, filename, encoding, size, offset
util.check_guard(path, size)
-(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet")
@pytest.mark.parametrize("align", sanlock.ALIGN_SIZE)
def test_write_resource_4k(sanlock_daemon, user_4k_path, align):
disks = [(user_4k_path, 0)]
@@ -250,7 +243,6 @@ def test_write_resource_4k_invalid_sector_size(sanlock_daemon, user_4k_path):
assert e.value.errno == errno.EINVAL
-(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet")
def test_read_resource_4k_invalid_sector_size(sanlock_daemon, user_4k_path):
disks = [(user_4k_path, 0)]
@@ -266,7 +258,6 @@ def test_read_resource_4k_invalid_sector_size(sanlock_daemon, user_4k_path):
assert e.value.errno == errno.EINVAL
-(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet")
def test_read_resource_owners_4k_invalid_sector_size(
sanlock_daemon, user_4k_path):
disks = [(user_4k_path, 0)]
@@ -284,7 +275,6 @@ def test_read_resource_owners_4k_invalid_sector_size(
assert e.value.errno == errno.EINVAL
-(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet")
def test_read_resource_owners_invalid_align_size(tmpdir, sanlock_daemon):
path = str(tmpdir.join("path"))
util.create_file(path, GiB)
@@ -307,7 +297,6 @@ def test_read_resource_owners_invalid_align_size(tmpdir, sanlock_daemon):
assert e.value.errno == errno.EINVAL
-(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet")
@pytest.mark.parametrize("size,offset", [
# Smallest offset.
(MIN_RES_SIZE, 0),
@@ -352,7 +341,6 @@ def test_add_rem_lockspace(tmpdir, sanlock_daemon, size, offset):
assert lockspaces == []
-(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet")
def test_add_rem_lockspace_async(tmpdir, sanlock_daemon):
path = str(tmpdir.join("ls_name"))
util.create_file(path, MiB)
@@ -389,7 +377,6 @@ def test_add_rem_lockspace_async(tmpdir, sanlock_daemon):
assert acquired is False
-(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet")
@pytest.mark.parametrize("size,offset", [
# Smallest offset.
(MIN_RES_SIZE, 0),
@@ -463,7 +450,6 @@ def test_acquire_release_resource(tmpdir, sanlock_daemon, size, offset):
assert owners == []
-(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet")
@pytest.mark.parametrize("align, sector", [
# Invalid alignment
(KiB, sanlock.SECTOR_SIZE[0]),
@@ -479,7 +465,6 @@ def test_write_lockspace_invalid_align_sector(
sanlock.write_lockspace(b"ls_name", path, align=align, sector=sector)
-(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet")
@pytest.mark.parametrize("align, sector", [
# Invalid alignment
(KiB, sanlock.SECTOR_SIZE[0]),
@@ -497,7 +482,6 @@ def test_write_resource_invalid_align_sector(
b"ls_name", b"res_name", disks, align=align, sector=sector)
-(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet")
@pytest.mark.parametrize("disk", [
# Not a tuple - unicode and bytes:
"not a tuple",
@@ -552,7 +536,6 @@ def test_write_lockspace_parse_args(no_sanlock_daemon, name):
sanlock.write_lockspace(name, "ls_path")
-(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet")
@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
def test_write_resource_parse_args(no_sanlock_daemon, name):
with raises_sanlock_errno():
@@ -562,7 +545,6 @@ def test_write_resource_parse_args(no_sanlock_daemon, name):
sanlock.write_resource(b"ls_name", name, [("disk_path",0)])
-(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet")
@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
def test_release_resource_parse_args(no_sanlock_daemon, name):
with raises_sanlock_errno():
@@ -572,7 +554,6 @@ def test_release_resource_parse_args(no_sanlock_daemon, name):
sanlock.release(b"ls_name", name, [("disk_path",0)])
-(a)pytest.mark.xfail(six.PY3, reason="lockspace/resource names in bytes are unsupported yet")
@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
def test_read_resource_owners_parse_args(no_sanlock_daemon, name):
with raises_sanlock_errno():
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
4 years, 10 months
[sanlock] 17/18: python: Use PyBytes converter for set_event API
by pagure＠pagure.io
This is an automated email from the git hooks/post-receive script.
nsoffer pushed a commit to branch master
in repository sanlock.
commit 3a71eefe6688fbf3ddf09b698594d290153e392e
Author: Amit Bawer <abawer(a)redhat.com>
AuthorDate: Sun Jun 2 17:50:11 2019 +0300
python: Use PyBytes converter for set_event API
Also add a stub test for arguments parsing.
Implications:
- Py2 : no difference
- Py3 : if used, only accept bytes for lockspace name.
---
python/sanlock.c | 20 ++++++++++++--------
tests/python_test.py | 6 ++++++
2 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/python/sanlock.c b/python/sanlock.c
index 8f99227..39a9fb6 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -1617,29 +1617,33 @@ with -EBUSY error in this case.\n\
static PyObject *
py_set_event(PyObject *self __unused, PyObject *args, PyObject *keywds)
{
- const char *lockspace;
+ PyObject *lockspace = NULL;
struct sanlk_host_event he = {0};
uint32_t flags = 0;
- int rv;
+ int rv = -1;
static char *kwlist[] = {"lockspace", "host_id", "generation", "event",
"data", "flags", NULL};
- if (!PyArg_ParseTupleAndKeywords(args, keywds, "sKKK|KI", kwlist,
- &lockspace, &he.host_id, &he.generation, &he.event, &he.data,
- &flags)) {
- return NULL;
+ if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&KKK|KI", kwlist,
+ convert_to_pybytes, &lockspace, &he.host_id, &he.generation, &he.event,
+ &he.data, &flags)) {
+ goto finally;
}
Py_BEGIN_ALLOW_THREADS
- rv = sanlock_set_event(lockspace, &he, flags);
+ rv = sanlock_set_event(PyBytes_AsString(lockspace), &he, flags);
Py_END_ALLOW_THREADS
if (rv < 0) {
__set_exception(rv, "Unable to set event");
- return NULL;
+ goto finally;
}
+finally:
+ Py_XDECREF(lockspace);
+ if (rv < 0)
+ return NULL;
Py_RETURN_NONE;
}
diff --git a/tests/python_test.py b/tests/python_test.py
index 7406bcb..7093aac 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -607,6 +607,12 @@ def test_end_event_parse_args(no_sanlock_daemon, name):
@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
+def test_set_event_parse_args(no_sanlock_daemon, name):
+ with raises_sanlock_errno():
+ sanlock.set_event(name, 1, 1, 1)
+
+
+(a)pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
def test_init_lockspace_parse_args(no_sanlock_daemon, name):
with raises_sanlock_errno(errno.ENODEV):
sanlock.init_lockspace(name, "path")
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
4 years, 10 months
[sanlock] 16/18: python: Use PyBytes converter for end_event API
by pagure＠pagure.io
This is an automated email from the git hooks/post-receive script.
nsoffer pushed a commit to branch master
in repository sanlock.
commit d809ff35947de4040ccc85a36d945211cc22f6d3
Author: Amit Bawer <abawer(a)redhat.com>
AuthorDate: Sun Jun 2 17:44:05 2019 +0300
python: Use PyBytes converter for end_event API
Implications:
- Py2 : no difference
- Py3 : if used, only accept bytes for lockspace name.
---
python/sanlock.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/python/sanlock.c b/python/sanlock.c
index 1f2960e..8f99227 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -1539,21 +1539,26 @@ static PyObject *
py_end_event(PyObject *self __unused, PyObject *args)
{
int fd = -1;
- const char *lockspace = NULL;
- int rv;
+ PyObject *lockspace = NULL;
+ int rv = -1;
- if (!PyArg_ParseTuple(args, "is", &fd, &lockspace))
- return NULL;
+ if (!PyArg_ParseTuple(args, "iO&", &fd, convert_to_pybytes, &lockspace)) {
+ goto finally;
+ }
Py_BEGIN_ALLOW_THREADS
- rv = sanlock_end_event(fd, lockspace, 0 /* flags */);
+ rv = sanlock_end_event(fd, PyBytes_AsString(lockspace), 0 /* flags */);
Py_END_ALLOW_THREADS
if (rv < 0) {
__set_exception(rv, "Unable to unregister event fd");
- return NULL;
+ goto finally;
}
+finally:
+ Py_XDECREF(lockspace);
+ if (rv < 0)
+ return NULL;
Py_RETURN_NONE;
}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
4 years, 10 months
[sanlock] 15/18: python: Use PyBytes converter for reg_event API
by pagure＠pagure.io
This is an automated email from the git hooks/post-receive script.
nsoffer pushed a commit to branch master
in repository sanlock.
commit 50cabdcb8f52b84948263c62e06a4d6f19287444
Author: Amit Bawer <abawer(a)redhat.com>
AuthorDate: Sun Jun 2 17:39:26 2019 +0300
python: Use PyBytes converter for reg_event API
Implications:
- Py2 : no difference
- Py3 : if used, only accept bytes for lockspace name.
---
python/sanlock.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/python/sanlock.c b/python/sanlock.c
index ed9b4f0..1f2960e 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -1396,21 +1396,26 @@ unregister the event listener using end_event.");
static PyObject *
py_reg_event(PyObject *self __unused, PyObject *args)
{
- const char *lockspace = NULL;
+ PyObject *lockspace = NULL;
int fd = -1;
- if (!PyArg_ParseTuple(args, "s", &lockspace))
- return NULL;
+ if (!PyArg_ParseTuple(args, "O&", convert_to_pybytes, &lockspace)) {
+ goto finally;
+ }
Py_BEGIN_ALLOW_THREADS
- fd = sanlock_reg_event(lockspace, NULL /* event */, 0 /* flags */);
+ fd = sanlock_reg_event(PyBytes_AsString(lockspace), NULL /* event */, 0 /* flags */);
Py_END_ALLOW_THREADS
if (fd < 0) {
__set_exception(fd, "Unable to register event fd");
- return NULL;
+ goto finally;
}
+finally:
+ Py_XDECREF(lockspace);
+ if (fd < 0)
+ return NULL;
return Py_BuildValue("i", fd);
}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
4 years, 10 months
[sanlock] 14/18: python: Use PyBytes converter for
read_resource_owners API
by pagure＠pagure.io
This is an automated email from the git hooks/post-receive script.
nsoffer pushed a commit to branch master
in repository sanlock.
commit d7eaa34ac37d1c9ec5676c1432c74bb445d2bd51
Author: Amit Bawer <abawer(a)redhat.com>
AuthorDate: Sun Jun 2 17:33:05 2019 +0300
python: Use PyBytes converter for read_resource_owners API
Implications:
- Py2 : no difference
- Py3 : if used, only accept bytes for lockspace name.
---
python/sanlock.c | 33 +++++++++++++++++++--------------
1 file changed, 19 insertions(+), 14 deletions(-)
diff --git a/python/sanlock.c b/python/sanlock.c
index 0964671..ed9b4f0 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -1228,10 +1228,10 @@ 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 rv = -1, hss_count = 0;
int sector = SECTOR_SIZE_512;
long align = ALIGNMENT_1M;
- const char *lockspace, *resource;
+ PyObject *lockspace = NULL, *resource = NULL;
struct sanlk_resource *res = NULL;
struct sanlk_host *hss = NULL;
PyObject *disks, *ls_list = NULL;
@@ -1240,27 +1240,28 @@ py_read_resource_owners(PyObject *self __unused, PyObject *args, PyObject *keywd
"sector", NULL};
/* parse python tuple */
- if (!PyArg_ParseTupleAndKeywords(args, keywds, "ssO!|li", kwlist,
- &lockspace, &resource, &PyList_Type, &disks, &align, §or)) {
- return NULL;
+ if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&O&O!|li", kwlist,
+ convert_to_pybytes, &lockspace, convert_to_pybytes, &resource,
+ &PyList_Type, &disks, &align, §or)) {
+ goto finally;
}
/* parse and check sanlock resource */
if (__parse_resource(disks, &res) < 0) {
- return NULL;
+ goto finally;
}
/* prepare sanlock names */
- strncpy(res->lockspace_name, lockspace, SANLK_NAME_LEN);
- strncpy(res->name, resource, SANLK_NAME_LEN);
+ strncpy(res->lockspace_name, PyBytes_AsString(lockspace), SANLK_NAME_LEN);
+ strncpy(res->name, PyBytes_AsString(resource), SANLK_NAME_LEN);
/* set resource alignment and sector flags */
if (add_align_flag(align, &res->flags) == -1)
- goto exit_fail;
+ goto finally;
if (add_sector_flag(sector, &res->flags) == -1)
- goto exit_fail;
+ goto finally;
/* read resource owners (gil disabled) */
Py_BEGIN_ALLOW_THREADS
@@ -1269,14 +1270,18 @@ py_read_resource_owners(PyObject *self __unused, PyObject *args, PyObject *keywd
if (rv != 0) {
__set_exception(rv, "Unable to read resource owners");
- goto exit_fail;
+ goto finally;
}
ls_list = __hosts_to_list(hss, hss_count);
-exit_fail:
- if (res) free(res);
- if (hss) free(hss);
+finally:
+ Py_XDECREF(lockspace);
+ Py_XDECREF(resource);
+ free(res);
+ free(hss);
+ if (rv != 0)
+ return NULL;
return ls_list;
}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
4 years, 10 months