On Wed, May 19, 2021 at 8:29 PM Nir Soffer <nsoffer(a)redhat.com> wrote:
On Tue, May 18, 2021 at 4:29 PM Nir Soffer <nirsof(a)gmail.com> wrote:
>
> Use sanlock_inquire() to query the resource held by the current process
> (using the slkfd= argument) or held by another program (using the pid=
> argument).
>
> When using the slkfd= argument, we communicate with sanlock daemon using
> slkfd, ensuring that the current process is connected to sanlock. If the
> current process is not connected, sanlock assumes that the process is
> dead, and release all the leases acquired by the process.
>
> When using the pid= argument, the function opens a new socket to sanlock
> daemon and query the status of resources owned by specified pid.
>
> In both cases the information comes from sanlock daemon, without
> accessing storage. To verify storage content, the caller should use
> read_resource() and read_resource_owners().
>
> The call returns list of resources dicts that can be used for verifying
> that sanlock state matches the program state.
>
> sanlock_inquire() reports the SANLOCK_RES_LVER or sanlock.RES_SHARED
> flags in the resource flags field. Add the field to the returned dict
> and add sanlock constants for the flag.
>
> The resource flags are needed if you want to restore a lease after it
> was released, ensuring that nobody else acquired the lease after it was
> released. This flow is used by libvirt using libsanlock. With this
> change we can implement the same flow using the python binding.
>
> Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
> ---
> python/sanlock.c | 173 +++++++++++++++++++++++++++++++++++++++++++
> tests/python_test.py | 84 +++++++++++++++++++++
> 2 files changed, 257 insertions(+)
>
> diff --git a/python/sanlock.c b/python/sanlock.c
> index 67d34fc..238d172 100644
> --- a/python/sanlock.c
> +++ b/python/sanlock.c
> @@ -323,6 +323,88 @@ exit_fail:
> return NULL;
> }
>
> +/* Convert disks array to list of tuples. */
> +static PyObject *
> +disks_to_list(struct sanlk_disk *disks, uint32_t disks_count)
> +{
> + PyObject *result = NULL;
> + PyObject *disk = NULL;
> +
> + result = PyList_New(disks_count);
> + if (result == NULL)
> + return NULL;
> +
> + for (uint32_t i = 0; i < disks_count; i++) {
> + disk = Py_BuildValue(
> + "(s,K)",
> + disks[i].path,
> + disks[i].offset);
> + if (disk == NULL)
> + goto exit_fail;
> +
> + /* Steals reference to disk. */
> + if (PyList_SetItem(result, i, disk) != 0)
> + goto exit_fail;
> +
> + disk = NULL;
> + }
> +
> + return result;
> +
> +exit_fail:
> + Py_XDECREF(result);
> + Py_XDECREF(disk);
> +
> + return NULL;
> +}
> +
> +/* Convert resources array returned from sanlock_inquire() to list of resource
> + * dicts. */
> +static PyObject *
> +resources_to_list(struct sanlk_resource **res, int res_count)
> +{
> + PyObject *result = NULL;
> + PyObject *info = NULL;
> + PyObject *disks = NULL;
> +
> + if ((result = PyList_New(res_count)) == NULL)
> + return NULL;
> +
> + for (int i = 0; i < res_count; i++) {
> + disks = disks_to_list(res[i]->disks, res[i]->num_disks);
> + if (disks == NULL)
> + goto exit_fail;
> +
> + /* Steals reference to disks. */
> + info = Py_BuildValue(
> + "{s:y,s:y,s:k,s:K,s:N}",
> + "lockspace", res[i]->lockspace_name,
> + "resource", res[i]->name,
> + "flags", res[i]->flags,
> + "version", res[i]->lver,
> + "disks", disks);
> + if (info == NULL)
> + goto exit_fail;
> +
> + disks = NULL;
> +
> + /* Steals reference to info. */
> + if (PyList_SetItem(result, i, info) != 0)
> + goto exit_fail;
> +
> + info = NULL;
> + }
> +
> + return result;
> +
> +exit_fail:
> + Py_XDECREF(result);
> + Py_XDECREF(info);
> + Py_XDECREF(disks);
> +
> + return NULL;
> +}
> +
> /* register */
> PyDoc_STRVAR(pydoc_register, "\
> register() -> int\n\
> @@ -1062,6 +1144,89 @@ finally:
> Py_RETURN_NONE;
> }
>
> +/* inquire */
> +PyDoc_STRVAR(pydoc_inquire, "\
> +inquire(slkfd=fd, pid=owner)\n\
I copied this from acquire and release, but this is wrong and confusing.
Should be:
inquire(slkfd=-1, pid=-1)
I pushed the patch with this change.
I'll send v2 later.
> +Return list of resources held by current process (using the slkfd \n\
> +argument to specify the sanlock file descriptor) or for another \n\
> +process (using the pid argument).\n\
> +\n\
> +Does not access storage. To learn about resource state on storage,\n\
> +use sanlock.read_resource() and sanlock.read_resource_owners().\n\
> +\n\
> +Arguments\n\
> + slkfd (int): The file descriptor returned from sanlock.register().\n\
> + pid (int): The program pid to query.\n\
> +\n\
> +Returns\n\
> + List of resource dicts with the following keys:\n\
> + lockspace (bytes): lockspace name\n\
> + resource (bytes): resource name\n\
> + flags (int): resource flags (sanlock.RES_*)\n\
> + version (int): resource version\n\
> + disks (list): list of disk tuples (path, offset)\n\
> +");
> +
> +static PyObject *
> +py_inquire(PyObject *self __unused, PyObject *args, PyObject *keywds)
> +{
> + int sanlockfd = -1;
> + int pid = -1;
> + char *kwlist[] = {"slkfd", "pid", NULL};
> + int rv = -1;
> +
> + /* sanlock_inquire() return values. */
> + int res_count = 0;
> + char *res_state = NULL;
> +
> + /* Array of resoruces parsed from res_state. */
> + struct sanlk_resource **res_arr = NULL;
> +
> + /* List of resource dicts. */
> + PyObject *result = NULL;
> +
> + if (!PyArg_ParseTupleAndKeywords(
> + args, keywds, "|ii", kwlist, &sanlockfd, &pid)) {
> + return NULL;
> + }
> +
> + /* Check if any of the slkfd or pid parameters was given. */
> + if (sanlockfd == -1 && pid == -1) {
> + set_sanlock_error(-EINVAL, "Invalid slkfd and pid values");
> + return NULL;
> + }
> +
> + /* Inquire sanlock (gil disabled) */
> + Py_BEGIN_ALLOW_THREADS
> + rv = sanlock_inquire(sanlockfd, pid, 0, &res_count, &res_state);
> + Py_END_ALLOW_THREADS
> +
> + if (rv != 0) {
> + set_sanlock_error(rv, "Inquire error");
> + return NULL;
> + }
> +
> + if (res_count > 0) {
> + rv = sanlock_state_to_args(res_state, &res_count, &res_arr);
> + if (rv != 0) {
> + /* TODO: Include res_state in the error. */
> + set_sanlock_error(rv, "Error parsing inquire state string");
> + goto finally;
> + }
> + }
> +
> + result = resources_to_list(res_arr, res_count);
> +
> +finally:
> + free(res_state);
> +
> + for (int i = 0; i < res_count; i++)
> + free(res_arr[i]);
> + free(res_arr);
> +
> + return result;
> +}
> +
> /* release */
> PyDoc_STRVAR(pydoc_release, "\
> release(lockspace, resource, disks [, slkfd=fd, pid=owner])\n\
> @@ -1752,6 +1917,8 @@ sanlock_methods[] = {
> METH_VARARGS|METH_KEYWORDS, pydoc_read_resource_owners},
> {"acquire", (PyCFunction) py_acquire,
> METH_VARARGS|METH_KEYWORDS, pydoc_acquire},
> + {"inquire", (PyCFunction) py_inquire,
> + METH_VARARGS|METH_KEYWORDS, pydoc_inquire},
> {"release", (PyCFunction) py_release,
> METH_VARARGS|METH_KEYWORDS, pydoc_release},
> {"request", (PyCFunction) py_request,
> @@ -1850,6 +2017,12 @@ module_init(PyObject* m)
> if (PyModule_AddIntConstant(m, "SETEV_ALL_HOSTS",
SANLK_SETEV_ALL_HOSTS))
> return -1;
>
> + /* sanlock_inquire() result resource flags */
> + if (PyModule_AddIntConstant(m, "RES_LVER", SANLK_RES_LVER))
> + return -1;
> + if (PyModule_AddIntConstant(m, "RES_SHARED", SANLK_RES_SHARED))
> + return -1;
> +
> /* Tuples with supported sector size and alignment values */
>
> PyObject *sector = Py_BuildValue("ii", SECTOR_SIZE_512,
SECTOR_SIZE_4K);
> diff --git a/tests/python_test.py b/tests/python_test.py
> index 58a22c7..caf2f3e 100644
> --- a/tests/python_test.py
> +++ b/tests/python_test.py
> @@ -479,6 +479,90 @@ def test_acquire_release_resource(tmpdir, sanlock_daemon, size,
offset):
> assert owners == []
>
>
> +(a)pytest.mark.parametrize("res_name", [
> + "ascii",
> + "\u05d0", # Hebrew Alef
> +])
> +def test_inquire(tmpdir, sanlock_daemon, res_name):
> + ls_path = str(tmpdir.join("ls_name"))
> + util.create_file(ls_path, MiB)
> +
> + res_path = str(tmpdir.join(res_name))
> + util.create_file(res_path, 10 * MiB)
> +
> + fd = sanlock.register()
> +
> + # No lockspace yet.
> + assert sanlock.inquire(slkfd=fd) == []
> +
> + sanlock.write_lockspace(b"ls_name", ls_path, offset=0, iotimeout=1)
> + sanlock.add_lockspace(b"ls_name", 1, ls_path, offset=0, iotimeout=1)
> +
> + # No resources created yet.
> + assert sanlock.inquire(slkfd=fd) == []
> +
> + resources = [
> + # name, offset, acquire
> + (b"res-0", 0 * MiB, True),
> + (b"res-1", 1 * MiB, False),
> + (b"res-2", 2 * MiB, True),
> + (b"res-8", 8 * MiB, False),
> + (b"res-9", 9 * MiB, True),
> + ]
> +
> + for res_name, res_offset, acquire in resources:
> + sanlock.write_resource(b"ls_name", res_name, [(res_path,
res_offset)])
> +
> + # No resource acquired yet.
> + assert sanlock.inquire(slkfd=fd) == []
> +
> + # Acquire resources.
> + for res_name, res_offset, acquire in resources:
> + if acquire:
> + sanlock.acquire(
> + b"ls_name", res_name, [(res_path, res_offset)],
slkfd=fd)
> +
> + time.sleep(1)
> +
> + expected = [
> + {
> + "lockspace": b"ls_name",
> + "resource": b"res-0",
> + "flags": sanlock.RES_LVER,
> + "version": 1,
> + "disks": [(res_path, 0 * MiB)],
> + },
> + {
> + "lockspace": b"ls_name",
> + "resource": b"res-2",
> + "flags": sanlock.RES_LVER,
> + "version": 1,
> + "disks": [(res_path, 2 * MiB)],
> + },
> + {
> + "lockspace": b"ls_name",
> + "resource": b"res-9",
> + "flags": sanlock.RES_LVER,
> + "version": 1,
> + "disks": [(res_path, 9 * MiB)],
> + },
> + ]
> +
> + # Check acquired resources using snlkfd.
> + assert sanlock.inquire(slkfd=fd) == expected
> +
> + # Check acquired resources using pid.
> + assert sanlock.inquire(pid=os.getpid()) == expected
> +
> + for res_name, res_offset, acquire in resources:
> + if acquire:
> + sanlock.release(
> + b"ls_name", res_name, [(res_path, res_offset)],
slkfd=fd)
> +
> + # All resource released.
> + assert sanlock.inquire(slkfd=fd) == []
> +
> +
> @pytest.mark.parametrize("align, sector", [
> # Invalid alignment
> (KiB, sanlock.SECTOR_SIZE[0]),
> --
> 2.26.3
>