[Libguestfs] [nbdkit PATCH v2 5/5] RFC: python: Track and cache per-connection state in C struct
Richard W.M. Jones
rjones at redhat.com
Thu Apr 19 17:40:50 UTC 2018
On Wed, Apr 11, 2018 at 12:03:42AM -0500, Eric Blake wrote:
> Now that we have FUA support, the C code can call can_fua as
> frequently as on every write. If the python script has a
> can_fua, we can avoid doubling the calls into python by
> caching the per-connection results, done by wrapping the
> python handle in a C struct.
>
> This commit is marked RFC because it might be nicer if the C
> code implemented the caching for ALL plugins (TODO already
> mentions that).
It's not very clear to me (and this applies to all can_* functions) if
it's OK to call them only once per connection.
I can't think of any argument against that. Is there a potential
meaning for a plugin to "change its mind" during a connection?
Rich.
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
> plugins/python/python.c | 83 ++++++++++++++++++++++++++++++-------------------
> 1 file changed, 51 insertions(+), 32 deletions(-)
>
> diff --git a/plugins/python/python.c b/plugins/python/python.c
> index ad79c80..757a0e9 100644
> --- a/plugins/python/python.c
> +++ b/plugins/python/python.c
> @@ -389,51 +389,66 @@ py_config_complete (void)
> return 0;
> }
>
> +/* All per-connection state */
> +typedef struct ConnHandle {
> + PyObject *obj;
> + int fua;
> +} ConnHandle;
> +
> static void *
> py_open (int readonly)
> {
> PyObject *fn;
> - PyObject *handle;
> + ConnHandle *h = malloc (sizeof *h);
>
> + if (!h) {
> + nbdkit_error ("%s: %m", script);
> + return NULL;
> + }
> if (!callback_defined ("open", &fn)) {
> nbdkit_error ("%s: missing callback: %s", script, "open");
> + free (h);
> return NULL;
> }
>
> PyErr_Clear ();
>
> - handle = PyObject_CallFunctionObjArgs (fn, readonly ? Py_True : Py_False,
> + h->obj = PyObject_CallFunctionObjArgs (fn, readonly ? Py_True : Py_False,
> NULL);
> Py_DECREF (fn);
> - if (check_python_failure ("open") == -1)
> + if (check_python_failure ("open") == -1) {
> + free (h);
> return NULL;
> + }
>
> - return handle;
> + h->fua = -1;
> + return h;
> }
>
> static void
> py_close (void *handle)
> {
> - PyObject *obj = handle;
> + ConnHandle *h = handle;
> PyObject *fn;
> PyObject *r;
>
> if (callback_defined ("close", &fn)) {
> PyErr_Clear ();
>
> - r = PyObject_CallFunctionObjArgs (fn, obj, NULL);
> + r = PyObject_CallFunctionObjArgs (fn, h->obj, NULL);
> Py_DECREF (fn);
> check_python_failure ("close");
> Py_XDECREF (r);
> }
>
> - Py_DECREF (obj);
> + Py_DECREF (h->obj);
> + free (h);
> }
>
> static int64_t
> py_get_size (void *handle)
> {
> - PyObject *obj = handle;
> + ConnHandle *h = handle;
> PyObject *fn;
> PyObject *r;
> int64_t ret;
> @@ -445,7 +460,7 @@ py_get_size (void *handle)
>
> PyErr_Clear ();
>
> - r = PyObject_CallFunctionObjArgs (fn, obj, NULL);
> + r = PyObject_CallFunctionObjArgs (fn, h->obj, NULL);
> Py_DECREF (fn);
> if (check_python_failure ("get_size") == -1)
> return -1;
> @@ -462,7 +477,7 @@ static int
> py_pread (void *handle, void *buf,
> uint32_t count, uint64_t offset, uint32_t flags)
> {
> - PyObject *obj = handle;
> + ConnHandle *h = handle;
> PyObject *fn;
> PyObject *r;
>
> @@ -474,7 +489,7 @@ py_pread (void *handle, void *buf,
>
> PyErr_Clear ();
>
> - r = PyObject_CallFunction (fn, "OiL", obj, count, offset, NULL);
> + r = PyObject_CallFunction (fn, "OiL", h->obj, count, offset, NULL);
> Py_DECREF (fn);
> if (check_python_failure ("pread") == -1)
> return -1;
> @@ -504,7 +519,7 @@ static int
> py_pwrite (void *handle, const void *buf,
> uint32_t count, uint64_t offset, uint32_t flags)
> {
> - PyObject *obj = handle;
> + ConnHandle *h = handle;
> PyObject *fn;
> PyObject *args;
> PyObject *kwargs;
> @@ -516,7 +531,7 @@ py_pwrite (void *handle, const void *buf,
> if (callback_defined ("pwrite", &fn)) {
> PyErr_Clear ();
>
> - args = Py_BuildValue ("ONL", obj,
> + args = Py_BuildValue ("ONL", h->obj,
> PyByteArray_FromStringAndSize (buf, count),
> offset);
> if (!args) {
> @@ -559,7 +574,7 @@ py_pwrite (void *handle, const void *buf,
> static int
> py_flush (void *handle, uint32_t flags)
> {
> - PyObject *obj = handle;
> + ConnHandle *h = handle;
> PyObject *fn;
> PyObject *r;
>
> @@ -567,7 +582,7 @@ py_flush (void *handle, uint32_t flags)
> if (callback_defined ("flush", &fn)) {
> PyErr_Clear ();
>
> - r = PyObject_CallFunctionObjArgs (fn, obj, NULL);
> + r = PyObject_CallFunctionObjArgs (fn, h->obj, NULL);
> Py_DECREF (fn);
> if (check_python_failure ("flush") == -1)
> return -1;
> @@ -584,7 +599,7 @@ py_flush (void *handle, uint32_t flags)
> static int
> py_trim (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
> {
> - PyObject *obj = handle;
> + ConnHandle *h = handle;
> PyObject *fn;
> PyObject *args;
> PyObject *kwargs;
> @@ -596,7 +611,7 @@ py_trim (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
> if (callback_defined ("trim", &fn)) {
> PyErr_Clear ();
>
> - args = Py_BuildValue ("OiL", obj, count, offset);
> + args = Py_BuildValue ("OiL", h->obj, count, offset);
> if (!args) {
> check_python_failure ("trim");
> Py_DECREF (fn);
> @@ -637,7 +652,7 @@ py_trim (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
> static int
> py_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
> {
> - PyObject *obj = handle;
> + ConnHandle *h = handle;
> PyObject *fn;
> PyObject *args;
> PyObject *kwargs;
> @@ -660,7 +675,7 @@ py_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
> PyErr_Clear ();
>
> last_error = 0;
> - args = Py_BuildValue ("OiL", obj, count, offset);
> + args = Py_BuildValue ("OiL", h->obj, count, offset);
> if (!args) {
> check_python_failure ("zero");
> Py_DECREF (fn);
> @@ -718,7 +733,7 @@ py_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
> static int
> py_can_write (void *handle)
> {
> - PyObject *obj = handle;
> + ConnHandle *h = handle;
> PyObject *fn;
> PyObject *r;
> int ret;
> @@ -726,7 +741,7 @@ py_can_write (void *handle)
> if (callback_defined ("can_write", &fn)) {
> PyErr_Clear ();
>
> - r = PyObject_CallFunctionObjArgs (fn, obj, NULL);
> + r = PyObject_CallFunctionObjArgs (fn, h->obj, NULL);
> Py_DECREF (fn);
> if (check_python_failure ("can_write") == -1)
> return -1;
> @@ -741,7 +756,7 @@ py_can_write (void *handle)
> static int
> py_can_flush (void *handle)
> {
> - PyObject *obj = handle;
> + ConnHandle *h = handle;
> PyObject *fn;
> PyObject *r;
> int ret;
> @@ -749,7 +764,7 @@ py_can_flush (void *handle)
> if (callback_defined ("can_flush", &fn)) {
> PyErr_Clear ();
>
> - r = PyObject_CallFunctionObjArgs (fn, obj, NULL);
> + r = PyObject_CallFunctionObjArgs (fn, h->obj, NULL);
> Py_DECREF (fn);
> if (check_python_failure ("can_flush") == -1)
> return -1;
> @@ -764,7 +779,7 @@ py_can_flush (void *handle)
> static int
> py_is_rotational (void *handle)
> {
> - PyObject *obj = handle;
> + ConnHandle *h = handle;
> PyObject *fn;
> PyObject *r;
> int ret;
> @@ -772,7 +787,7 @@ py_is_rotational (void *handle)
> if (callback_defined ("is_rotational", &fn)) {
> PyErr_Clear ();
>
> - r = PyObject_CallFunctionObjArgs (fn, obj, NULL);
> + r = PyObject_CallFunctionObjArgs (fn, h->obj, NULL);
> Py_DECREF (fn);
> if (check_python_failure ("is_rotational") == -1)
> return -1;
> @@ -787,7 +802,7 @@ py_is_rotational (void *handle)
> static int
> py_can_trim (void *handle)
> {
> - PyObject *obj = handle;
> + ConnHandle *h = handle;
> PyObject *fn;
> PyObject *r;
> int ret;
> @@ -795,7 +810,7 @@ py_can_trim (void *handle)
> if (callback_defined ("can_trim", &fn)) {
> PyErr_Clear ();
>
> - r = PyObject_CallFunctionObjArgs (fn, obj, NULL);
> + r = PyObject_CallFunctionObjArgs (fn, h->obj, NULL);
> Py_DECREF (fn);
> if (check_python_failure ("can_trim") == -1)
> return -1;
> @@ -810,7 +825,7 @@ py_can_trim (void *handle)
> static int
> py_can_zero (void *handle)
> {
> - PyObject *obj = handle;
> + ConnHandle *h = handle;
> PyObject *fn;
> PyObject *r;
> int ret;
> @@ -818,7 +833,7 @@ py_can_zero (void *handle)
> if (callback_defined ("can_zero", &fn)) {
> PyErr_Clear ();
>
> - r = PyObject_CallFunctionObjArgs (fn, obj, NULL);
> + r = PyObject_CallFunctionObjArgs (fn, h->obj, NULL);
> Py_DECREF (fn);
> if (check_python_failure ("can_zero") == -1)
> return -1;
> @@ -833,16 +848,20 @@ py_can_zero (void *handle)
> static int
> py_can_fua (void *handle)
> {
> - PyObject *obj = handle;
> + ConnHandle *h = handle;
> PyObject *fn;
> PyObject *r;
> int fua;
>
> + /* Check the cache first */
> + if (h->fua != -1)
> + return h->fua;
> +
> /* We have to convert a python bool into the nbdkit tristate. */
> if (callback_defined ("can_fua", &fn)) {
> PyErr_Clear ();
>
> - r = PyObject_CallFunctionObjArgs (fn, obj, NULL);
> + r = PyObject_CallFunctionObjArgs (fn, h->obj, NULL);
> Py_DECREF (fn);
> if (check_python_failure ("can_fua") == -1)
> return -1;
> @@ -861,7 +880,7 @@ py_can_fua (void *handle)
> else
> fua = NBDKIT_FUA_NONE;
>
> - return fua;
> + return h->fua = fua;
> }
>
> #define py_config_help \
> --
> 2.14.3
>
> _______________________________________________
> Libguestfs mailing list
> Libguestfs at redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
More information about the Libguestfs
mailing list