[Libguestfs] [libnbd PATCH v2 5/8] python: Make py_aio_buffer a private struct
Eric Blake
eblake at redhat.com
Tue Jun 7 02:08:30 UTC 2022
Instead of having methods.c poking into the internals of a struct that
we defined, have it use PyMemoryView* and Py_buffer*; this separation
makes it easier to separate how we store the persistent data (for now,
in a PyCapsule) from how we access the data. Eventually, the use of
PyMemoryView* will make it easier for future patches to get rid of
some layers of copying. nbd_internal_py_init_aio_buffer() is new; for
now it doesn't fail, but it can in a future patch.
For now, we still have to hang on to a python reference to the
PyCapsule object holding our malloc'd C pointer, and since the
MemoryView we just created is not backed by a Python object, we must
ensure we don't leak it to Python code. But this will change in an
upcoming patch.
Generated code changes as follows:
| --- python/methods.h.bak 2022-06-06 16:26:40.363250044 -0500
| +++ python/methods.h 2022-06-06 16:37:13.625328491 -0500
| @@ -28,17 +28,12 @@
|
| #include <assert.h>
|
| -struct py_aio_buffer {
| - Py_ssize_t len;
| - void *data;
| - bool initialized;
| -};
| -
| extern char **nbd_internal_py_get_string_list (PyObject *);
| extern void nbd_internal_py_free_string_list (char **);
| extern int nbd_internal_py_get_sockaddr (PyObject *,
| struct sockaddr_storage *, socklen_t *);
| -extern struct py_aio_buffer *nbd_internal_py_get_aio_buffer (PyObject *);
| +extern PyObject *nbd_internal_py_get_aio_view (PyObject *, bool);
| +extern int nbd_internal_py_init_aio_buffer (PyObject *);
|
| static inline struct nbd_handle *
| get_handle (PyObject *obj)
| --- python/methods.c.bak 2022-06-06 16:26:40.360250039 -0500
| +++ python/methods.c 2022-06-06 16:37:13.640328513 -0500
| @@ -3080,7 +3080,8 @@ nbd_internal_py_aio_pread (PyObject *sel
| int64_t ret;
| PyObject *py_ret = NULL;
| PyObject *buf; /* PyCapsule pointing to struct py_aio_buffer */
| - struct py_aio_buffer *buf_buf;
| + PyObject *buf_view = NULL; /* PyMemoryView of buf */
| + Py_buffer *py_buf; /* buffer of buf_view */
| uint64_t offset_u64;
| unsigned long long offset; /* really uint64_t */
| struct user_data *completion_user_data = NULL;
| @@ -3110,16 +3111,17 @@ nbd_internal_py_aio_pread (PyObject *sel
| else
| completion.callback = NULL; /* we're not going to call it */
| flags_u32 = flags;
| - buf_buf = nbd_internal_py_get_aio_buffer (buf);
| - if (!buf_buf) goto out;
| + buf_view = nbd_internal_py_get_aio_view (buf, false);
| + if (!buf_view) goto out;
| + py_buf = PyMemoryView_GET_BUFFER (buf_view);
| /* Increment refcount since buffer may be saved by libnbd. */
| Py_INCREF (buf);
| completion_user_data->buf = buf;
| offset_u64 = offset;
|
| - buf_buf->initialized = true;
| - ret = nbd_aio_pread (h, buf_buf->data, buf_buf->len, offset_u64,
| - completion, flags_u32);
| + if (nbd_internal_py_init_aio_buffer (buf) < 0) goto out;
| + ret = nbd_aio_pread (h, py_buf->buf, py_buf->len, offset_u64, completion,
| + flags_u32);
| completion_user_data = NULL;
| if (ret == -1) {
| raise_exception ();
| @@ -3128,6 +3130,7 @@ nbd_internal_py_aio_pread (PyObject *sel
| py_ret = PyLong_FromLongLong (ret);
|
| out:
| + Py_XDECREF (buf_view);
| free_user_data (completion_user_data);
| return py_ret;
| }
| @@ -3140,7 +3143,8 @@ nbd_internal_py_aio_pread_structured (Py
| int64_t ret;
| PyObject *py_ret = NULL;
| PyObject *buf; /* PyCapsule pointing to struct py_aio_buffer */
| - struct py_aio_buffer *buf_buf;
| + PyObject *buf_view = NULL; /* PyMemoryView of buf */
| + Py_buffer *py_buf; /* buffer of buf_view */
| uint64_t offset_u64;
| unsigned long long offset; /* really uint64_t */
| struct user_data *chunk_user_data = NULL;
| @@ -3175,8 +3179,9 @@ nbd_internal_py_aio_pread_structured (Py
| else
| completion.callback = NULL; /* we're not going to call it */
| flags_u32 = flags;
| - buf_buf = nbd_internal_py_get_aio_buffer (buf);
| - if (!buf_buf) goto out;
| + buf_view = nbd_internal_py_get_aio_view (buf, false);
| + if (!buf_view) goto out;
| + py_buf = PyMemoryView_GET_BUFFER (buf_view);
| /* Increment refcount since buffer may be saved by libnbd. */
| Py_INCREF (buf);
| completion_user_data->buf = buf;
| @@ -3192,9 +3197,9 @@ nbd_internal_py_aio_pread_structured (Py
| Py_INCREF (py_chunk_fn);
| chunk_user_data->fn = py_chunk_fn;
|
| - buf_buf->initialized = true;
| - ret = nbd_aio_pread_structured (h, buf_buf->data, buf_buf->len,
| - offset_u64, chunk, completion, flags_u32);
| + if (nbd_internal_py_init_aio_buffer (buf) < 0) goto out;
| + ret = nbd_aio_pread_structured (h, py_buf->buf, py_buf->len, offset_u64,
| + chunk, completion, flags_u32);
| chunk_user_data = NULL;
| completion_user_data = NULL;
| if (ret == -1) {
| @@ -3204,6 +3209,7 @@ nbd_internal_py_aio_pread_structured (Py
| py_ret = PyLong_FromLongLong (ret);
|
| out:
| + Py_XDECREF (buf_view);
| free_user_data (chunk_user_data);
| free_user_data (completion_user_data);
| return py_ret;
| @@ -3217,7 +3223,8 @@ nbd_internal_py_aio_pwrite (PyObject *se
| int64_t ret;
| PyObject *py_ret = NULL;
| PyObject *buf; /* PyCapsule pointing to struct py_aio_buffer */
| - struct py_aio_buffer *buf_buf;
| + PyObject *buf_view = NULL; /* PyMemoryView of buf */
| + Py_buffer *py_buf; /* buffer of buf_view */
| uint64_t offset_u64;
| unsigned long long offset; /* really uint64_t */
| struct user_data *completion_user_data = NULL;
| @@ -3247,19 +3254,16 @@ nbd_internal_py_aio_pwrite (PyObject *se
| else
| completion.callback = NULL; /* we're not going to call it */
| flags_u32 = flags;
| - buf_buf = nbd_internal_py_get_aio_buffer (buf);
| - if (!buf_buf) goto out;
| + buf_view = nbd_internal_py_get_aio_view (buf, true);
| + if (!buf_view) goto out;
| + py_buf = PyMemoryView_GET_BUFFER (buf_view);
| /* Increment refcount since buffer may be saved by libnbd. */
| Py_INCREF (buf);
| completion_user_data->buf = buf;
| offset_u64 = offset;
|
| - if (!buf_buf->initialized) {
| - memset (buf_buf->data, 0, buf_buf->len);
| - buf_buf->initialized = true;
| - }
| - ret = nbd_aio_pwrite (h, buf_buf->data, buf_buf->len, offset_u64,
| - completion, flags_u32);
| + ret = nbd_aio_pwrite (h, py_buf->buf, py_buf->len, offset_u64, completion,
| + flags_u32);
| completion_user_data = NULL;
| if (ret == -1) {
| raise_exception ();
| @@ -3268,6 +3272,7 @@ nbd_internal_py_aio_pwrite (PyObject *se
| py_ret = PyLong_FromLongLong (ret);
|
| out:
| + Py_XDECREF (buf_view);
| free_user_data (completion_user_data);
| return py_ret;
| }
---
generator/Python.ml | 38 ++++++++++++++++----------------
python/handle.c | 53 ++++++++++++++++++++++++++++++++++++---------
2 files changed, 62 insertions(+), 29 deletions(-)
diff --git a/generator/Python.ml b/generator/Python.ml
index 6244c8e..1afe0cf 100644
--- a/generator/Python.ml
+++ b/generator/Python.ml
@@ -34,17 +34,12 @@ let
pr "#include <assert.h>\n";
pr "\n";
pr "\
-struct py_aio_buffer {
- Py_ssize_t len;
- void *data;
- bool initialized;
-};
-
extern char **nbd_internal_py_get_string_list (PyObject *);
extern void nbd_internal_py_free_string_list (char **);
extern int nbd_internal_py_get_sockaddr (PyObject *,
struct sockaddr_storage *, socklen_t *);
-extern struct py_aio_buffer *nbd_internal_py_get_aio_buffer (PyObject *);
+extern PyObject *nbd_internal_py_get_aio_view (PyObject *, bool);
+extern int nbd_internal_py_init_aio_buffer (PyObject *);
static inline struct nbd_handle *
get_handle (PyObject *obj)
@@ -306,7 +301,8 @@ let
| BytesPersistOut (n, _) ->
pr " PyObject *%s; /* PyCapsule pointing to struct py_aio_buffer */\n"
n;
- pr " struct py_aio_buffer *%s_buf;\n" n
+ pr " PyObject *%s_view = NULL; /* PyMemoryView of %s */\n" n n;
+ pr " Py_buffer *py_%s; /* buffer of %s_view */\n" n n
| Closure { cbname } ->
pr " struct user_data *%s_user_data = NULL;\n" cbname;
pr " PyObject *py_%s_fn;\n" cbname;
@@ -366,7 +362,7 @@ let
"n", sprintf "&%s" count,
sprintf "PyByteArray_AS_STRING (%s), %s" n count
| BytesPersistIn (n, _) | BytesPersistOut (n, _) ->
- "O", sprintf "&%s" n, sprintf "%s_buf->data, %s_buf->len" n n
+ "O", sprintf "&%s" n, sprintf "py_%s->buf, py_%s->len" n n
| Closure { cbname } -> "O", sprintf "&py_%s_fn" cbname, cbname
| Enum (n, _) -> "i", sprintf "&%s" n, n
| Flags (n, _) -> "I", sprintf "&%s" n, sprintf "%s_u32" n
@@ -435,9 +431,17 @@ let
| BytesOut (n, count) ->
pr " %s = PyByteArray_FromStringAndSize (NULL, %s);\n" n count;
pr " if (%s == NULL) goto out;\n" n
- | BytesPersistIn (n, _) | BytesPersistOut (n, _) ->
- pr " %s_buf = nbd_internal_py_get_aio_buffer (%s);\n" n n;
- pr " if (!%s_buf) goto out;\n" n;
+ | BytesPersistIn (n, _) ->
+ pr " %s_view = nbd_internal_py_get_aio_view (%s, true);\n" n n;
+ pr " if (!%s_view) goto out;\n" n;
+ pr " py_%s = PyMemoryView_GET_BUFFER (%s_view);\n" n n;
+ pr " /* Increment refcount since buffer may be saved by libnbd. */\n";
+ pr " Py_INCREF (%s);\n" n;
+ pr " completion_user_data->buf = %s;\n" n
+ | BytesPersistOut (n, _) ->
+ pr " %s_view = nbd_internal_py_get_aio_view (%s, false);\n" n n;
+ pr " if (!%s_view) goto out;\n" n;
+ pr " py_%s = PyMemoryView_GET_BUFFER (%s_view);\n" n n;
pr " /* Increment refcount since buffer may be saved by libnbd. */\n";
pr " Py_INCREF (%s);\n" n;
pr " completion_user_data->buf = %s;\n" n
@@ -477,13 +481,8 @@ let
(* Second pass, and call the underlying C function. *)
List.iter (
function
- | BytesPersistIn (n, _) ->
- pr " if (!%s_buf->initialized) {\n" n;
- pr " memset (%s_buf->data, 0, %s_buf->len);\n" n n;
- pr " %s_buf->initialized = true;\n" n;
- pr " }\n"
| BytesPersistOut (n, _) ->
- pr " %s_buf->initialized = true;\n" n
+ pr " if (nbd_internal_py_init_aio_buffer (%s) < 0) goto out;\n" n
| _ -> ()
) args;
pr " ret = nbd_%s (" name;
@@ -550,7 +549,8 @@ let
pr " if (%s.obj)\n" n;
pr " PyBuffer_Release (&%s);\n" n
| BytesOut (n, _) -> pr " Py_XDECREF (%s);\n" n
- | BytesPersistIn _ | BytesPersistOut _ -> ()
+ | BytesPersistIn (n, _) | BytesPersistOut (n, _) ->
+ pr " Py_XDECREF (%s_view);\n" n
| Closure { cbname } ->
pr " free_user_data (%s_user_data);\n" cbname
| Enum _ -> ()
diff --git a/python/handle.c b/python/handle.c
index d42a563..61dd736 100644
--- a/python/handle.c
+++ b/python/handle.c
@@ -40,6 +40,12 @@
#include "methods.h"
+struct py_aio_buffer {
+ Py_ssize_t len;
+ void *data;
+ bool initialized;
+};
+
static inline PyObject *
put_handle (struct nbd_handle *h)
{
@@ -98,12 +104,41 @@ nbd_internal_py_display_version (PyObject *self, PyObject *args)
static const char aio_buffer_name[] = "nbd.Buffer";
-struct py_aio_buffer *
+/* Return internal buffer pointer of nbd.Buffer */
+static struct py_aio_buffer *
nbd_internal_py_get_aio_buffer (PyObject *capsule)
{
return PyCapsule_GetPointer (capsule, aio_buffer_name);
}
+/* Return new reference to MemoryView wrapping aio_buffer contents */
+PyObject *
+nbd_internal_py_get_aio_view (PyObject *capsule, bool require_init)
+{
+ struct py_aio_buffer *buf = nbd_internal_py_get_aio_buffer (capsule);
+
+ if (!buf)
+ return NULL;
+
+ if (require_init && !buf->initialized) {
+ memset (buf->data, 0, buf->len);
+ buf->initialized = true;
+ }
+
+ return PyMemoryView_FromMemory (buf->data, buf->len,
+ require_init ? PyBUF_READ : PyBUF_WRITE);
+}
+
+int
+nbd_internal_py_init_aio_buffer (PyObject *capsule)
+{
+ struct py_aio_buffer *buf = nbd_internal_py_get_aio_buffer (capsule);
+
+ assert (buf);
+ buf->initialized = true;
+ return 0;
+}
+
static void
free_aio_buffer (PyObject *capsule)
{
@@ -219,23 +254,21 @@ PyObject *
nbd_internal_py_aio_buffer_to_bytearray (PyObject *self, PyObject *args)
{
PyObject *obj;
- struct py_aio_buffer *buf;
+ PyObject *view;
+ PyObject *ret;
if (!PyArg_ParseTuple (args,
"O:nbd_internal_py_aio_buffer_to_bytearray",
&obj))
return NULL;
- buf = nbd_internal_py_get_aio_buffer (obj);
- if (buf == NULL)
+ view = nbd_internal_py_get_aio_view (obj, true);
+ if (view == NULL)
return NULL;
- if (!buf->initialized) {
- memset (buf->data, 0, buf->len);
- buf->initialized = true;
- }
-
- return PyByteArray_FromStringAndSize (buf->data, buf->len);
+ ret = PyByteArray_FromObject (view);
+ Py_DECREF (view);
+ return ret;
}
PyObject *
--
2.36.1
More information about the Libguestfs
mailing list