Eric Blake
2022-May-31 14:15 UTC
[Libguestfs] [libnbd PATCH 0/2] Optimize h.pread_structured
Add some more safety against a non-compliant server with structured reads, and kill off another useless copy operation for a noticeable speedup when using h.pread_structured. However, I'm still trying to play with code to see if PyObject_GetItem(memoryview, PySlice_New(start, end, NULL)) can get me the same zero-copy python object but in a way that can survive as long as the original object lives, rather than forcing .release() at the end of the callback. Maybe I'll have a patch 3/2 later today. Eric Blake (2): api: Tighter checking of structured read replies python: Optimize away copy during pread_structured lib/internal.h | 2 +- generator/Python.ml | 20 ++++++++++++++------ generator/states-reply-simple.c | 4 ++-- generator/states-reply-structured.c | 6 ++++-- lib/aio.c | 7 +++++-- 5 files changed, 26 insertions(+), 13 deletions(-) -- 2.36.1
Eric Blake
2022-May-31 14:15 UTC
[Libguestfs] [libnbd PATCH 1/2] api: Tighter checking of structured read replies
Now that we allow clients to bypass buffer pre-initialization, it
becomes more important to detect when a buggy server using structured
replies does not send us enough bytes to cover the requested read
size. Our check is not perfect (a server that duplicates reply chunks
for byte 0 and omits byte 1 gets past our check), but this is a
tighter sanity check so that we are less likely to report a successful
read containing uninitialized memory on a buggy server.
Because we have a maximum read buffer size of 64M, and first check
that the server's chunk fits bounds, we don't have to worry about
overflowing a uint32_t, even if a server sends enough duplicate
responses that an actual sum would overflow.
---
lib/internal.h | 2 +-
generator/states-reply-simple.c | 4 ++--
generator/states-reply-structured.c | 6 ++++--
lib/aio.c | 7 +++++--
4 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/lib/internal.h b/lib/internal.h
index 885cee1..4121a5c 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -352,8 +352,8 @@ struct command {
void *data; /* Buffer for read/write */
struct command_cb cb;
enum state state; /* State to resume with on next POLLIN */
- bool data_seen; /* For read, true if at least one data chunk seen */
bool initialized; /* For read, true if getting a hole may skip memset */
+ uint32_t data_seen; /* For read, cumulative size of data chunks seen */
uint32_t error; /* Local errno value */
};
diff --git a/generator/states-reply-simple.c b/generator/states-reply-simple.c
index 7dc26fd..2a7b9a9 100644
--- a/generator/states-reply-simple.c
+++ b/generator/states-reply-simple.c
@@ -1,5 +1,5 @@
/* nbd client library in userspace: state machine
- * Copyright (C) 2013-2019 Red Hat Inc.
+ * Copyright (C) 2013-2022 Red Hat Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -40,7 +40,7 @@ STATE_MACHINE {
if (cmd->error == 0 && cmd->type == NBD_CMD_READ) {
h->rbuf = cmd->data;
h->rlen = cmd->count;
- cmd->data_seen = true;
+ cmd->data_seen = cmd->count;
SET_NEXT_STATE (%RECV_READ_PAYLOAD);
}
else {
diff --git a/generator/states-reply-structured.c
b/generator/states-reply-structured.c
index 12c24f5..cabd543 100644
--- a/generator/states-reply-structured.c
+++ b/generator/states-reply-structured.c
@@ -354,7 +354,6 @@ STATE_MACHINE {
assert (cmd); /* guaranteed by CHECK */
assert (cmd->data && cmd->type == NBD_CMD_READ);
- cmd->data_seen = true;
/* Length of the data following. */
length -= 8;
@@ -364,6 +363,8 @@ STATE_MACHINE {
SET_NEXT_STATE (%.DEAD);
return 0;
}
+ if (cmd->data_seen <= cmd->count)
+ cmd->data_seen += length;
/* Now this is the byte offset in the read buffer. */
offset -= cmd->offset;
@@ -422,13 +423,14 @@ STATE_MACHINE {
assert (cmd); /* guaranteed by CHECK */
assert (cmd->data && cmd->type == NBD_CMD_READ);
- cmd->data_seen = true;
/* Is the data within bounds? */
if (! structured_reply_in_bounds (offset, length, cmd)) {
SET_NEXT_STATE (%.DEAD);
return 0;
}
+ if (cmd->data_seen <= cmd->count)
+ cmd->data_seen += length;
/* Now this is the byte offset in the read buffer. */
offset -= cmd->offset;
diff --git a/lib/aio.c b/lib/aio.c
index 9744840..dc01f90 100644
--- a/lib/aio.c
+++ b/lib/aio.c
@@ -1,5 +1,5 @@
/* NBD client library in userspace
- * Copyright (C) 2013-2019 Red Hat Inc.
+ * Copyright (C) 2013-2022 Red Hat Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -91,8 +91,11 @@ nbd_unlocked_aio_command_completed (struct nbd_handle *h,
assert (cmd->type != NBD_CMD_DISC);
/* The spec states that a 0-length read request is unspecified; but
* it is easy enough to treat it as successful as an extension.
+ * Conversely, make sure a server sending structured replies sent
+ * enough data chunks to cover the overall count (although we do not
+ * detect if it duplicated some bytes while omitting others).
*/
- if (type == NBD_CMD_READ && !cmd->data_seen &&
cmd->count && !error)
+ if (type == NBD_CMD_READ && cmd->data_seen != cmd->count
&& !error)
error = EIO;
/* Retire it from the list and free it. */
--
2.36.1
Eric Blake
2022-May-31 14:15 UTC
[Libguestfs] [libnbd PATCH 2/2] python: Optimize away copy during pread_structured
The Py_BuildValue "y#" format copies data; this is because Python
wants to allow our C memory to go out of scope without worrying about
whether the user's python callback has stashed off a longer-living
reference to its incoming parameter. But it is inefficient; we can do
better by utilizing Python's memoryview for a zero-copy exposure to
the callback's C buffer, as well as a .release method that we can
utilize just before our C memory goes out of scope. Now, if the user
stashes away a reference, they will get a clean Python error if they
try to access the memory after the fact. This IS an API change (code
previously expecting a stashed copy to be long-lived will break), but
we never promised Python API stability, and anyone writing a callback
that saves off data was already risky (neither libnbd nor nbdkit's
testsuite had such a case). For a demonstration of the new error,
where the old code succeeded:
$ ./run nbdsh
nbd> h.connect_command(["nbdkit", "-s",
"memory", "10"])
nbd> copy=None
nbd> def f(b,o,s,e):
... global copy
... copy = b
... print(b[0])
...
nbd> print(copy)
None
nbd> h.pread_structured(10,0,f)
0
bytearray(b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00')
nbd> copy
<released memory at 0x7ff97410de40>
nbd> copy[0]
Traceback (most recent call last):
File "/usr/lib64/python3.10/code.py", line 90, in runcode
exec(code, self.locals)
File "<console>", line 1, in <module>
ValueError: operation forbidden on released memoryview object
To demonstrate the speedup, I tested:
$ export script='
def f(b,o,s,e):
pass
m=1024*1024
size=h.get_size()
for i in range(size // m):
buf = h.pread_structured(m, m*i, f)
'
$ time ./run nbdkit -U - memory 10G --run 'nbdsh -u "$uri" -c
"$script"'
On my machine, this took 9.1s pre-patch, and 3.0s post-patch, for an
approximate 65% speedup.
The corresponding diff to the generated code is:
| --- python/methods.c.bak 2022-05-31 07:57:25.256293091 -0500
| +++ python/methods.c 2022-05-31 08:14:09.570567858 -0500
| @@ -73,8 +73,12 @@ chunk_wrapper (void *user_data, const vo
|
| PyGILState_STATE py_save = PyGILState_UNLOCKED;
| PyObject *py_args, *py_ret;
| + PyObject *py_subbuf = NULL;
| PyObject *py_error = NULL;
|
| + /* Cast away const */
| + py_subbuf = PyMemoryView_FromMemory ((char *) subbuf, count, PyBUF_READ);
| + if (!py_subbuf) { PyErr_PrintEx (0); goto out; }
| PyObject *py_error_modname = PyUnicode_FromString ("ctypes");
| if (!py_error_modname) { PyErr_PrintEx (0); goto out; }
| PyObject *py_error_mod = PyImport_Import (py_error_modname);
| @@ -84,7 +88,7 @@
| Py_DECREF (py_error_mod);
| if (!py_error) { PyErr_PrintEx (0); goto out; }
|
| - py_args = Py_BuildValue ("(" "y#" "K"
"I" "O" ")", subbuf, (int) count, offset, status,
py_error);
| + py_args = Py_BuildValue ("(" "O" "K"
"I" "O" ")", py_subbuf, offset, status, py_error);
| if (!py_args) { PyErr_PrintEx (0); goto out; }
|
| py_save = PyGILState_Ensure ();
| @@ -111,6 +115,11 @@ chunk_wrapper (void *user_data, const vo
| };
|
| out:
| + if (py_subbuf) {
| + PyObject *tmp = PyObject_CallMethod(py_subbuf, "release",
NULL);
| + Py_XDECREF (tmp);
| + Py_DECREF (py_subbuf);
| + }
| if (py_error) {
| PyObject *py_error_ret = PyObject_GetAttrString (py_error,
"value");
| *error = PyLong_AsLong (py_error_ret);
---
generator/Python.ml | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/generator/Python.ml b/generator/Python.ml
index 1c4446e..0191f79 100644
--- a/generator/Python.ml
+++ b/generator/Python.ml
@@ -169,8 +169,7 @@ let
pr " PyObject *py_args, *py_ret;\n";
List.iter (
function
- | CBArrayAndLen (UInt32 n, _) ->
- pr " PyObject *py_%s = NULL;\n" n
+ | CBArrayAndLen (UInt32 n, _) | CBBytesIn (n, _)
| CBMutable (Int n) ->
pr " PyObject *py_%s = NULL;\n" n
| _ -> ()
@@ -187,7 +186,10 @@ let
pr " if (!py_e_%s) { PyErr_PrintEx (0); goto out; }\n" n;
pr " PyList_SET_ITEM (py_%s, i_%s, py_e_%s);\n" n n n;
pr " }\n"
- | CBBytesIn _
+ | CBBytesIn (n, len) ->
+ pr " /* Cast away const */\n";
+ pr " py_%s = PyMemoryView_FromMemory ((char *) %s, %s,
PyBUF_READ);\n" n n len;
+ pr " if (!py_%s) { PyErr_PrintEx (0); goto out; }\n" n
| CBInt _
| CBInt64 _ -> ()
| CBMutable (Int n) ->
@@ -210,7 +212,7 @@ let
List.iter (
function
| CBArrayAndLen (UInt32 n, len) -> pr " \"O\""
- | CBBytesIn (n, len) -> pr " \"y#\""
+ | CBBytesIn (n, _) -> pr " \"O\""
| CBInt n -> pr " \"i\""
| CBInt64 n -> pr " \"L\""
| CBMutable (Int n) -> pr " \"O\""
@@ -223,7 +225,7 @@ let
List.iter (
function
| CBArrayAndLen (UInt32 n, _) -> pr ", py_%s" n
- | CBBytesIn (n, len) -> pr ", %s, (int) %s" n len
+ | CBBytesIn (n, _) -> pr ", py_%s" n
| CBMutable (Int n) -> pr ", py_%s" n
| CBInt n | CBInt64 n
| CBString n
@@ -268,7 +270,13 @@ let
pr " Py_DECREF (py_%s_ret);\n" n;
pr " Py_DECREF (py_%s);\n" n;
pr " }\n"
- | CBBytesIn _
+ | CBBytesIn (n, _) ->
+ (*
https://stackoverflow.com/questions/58870010/how-to-call-release-on-a-memoryview-in-python-c-api
*)
+ pr " if (py_%s) {\n" n;
+ pr " PyObject *tmp = PyObject_CallMethod(py_%s,
\"release\", NULL);\n" n;
+ pr " Py_XDECREF (tmp);\n";
+ pr " Py_DECREF (py_%s);\n" n;
+ pr " }\n"
| CBInt _ | CBInt64 _
| CBString _
| CBUInt _ | CBUInt64 _ -> ()
--
2.36.1
Eric Blake
2022-May-31 15:49 UTC
[Libguestfs] [libnbd PATCH 3/2] python: Slice pread_structured buffer from original
This patch fixes the corner-case regression introduced by the previous
patch where the pread_structured callback buffer lifetime ends as soon
as the callback (that is, where accessing a stashed callback parameter
can result in ValueError instead of modifying a harmless copy). With
careful effort, we can create a memoryview of the Python object that
we read into, then slice that memoryview as part of the callback; now
the slice will be valid as long as the original object is also valid.
Note that this does not tackle the case of aio_pread_structured; for
that, we either need to make nbd.Buffer honor the Python buffer
protocol, or else change our handling of BytesPersistOut to utilize
the buffer protocol directly instead of requiring the user to
round-trip through nbd.Buffer.
The runtime of this patch is still about the same 3.0s as in the test
of the previous patch, but now you can do:
$ nbdsh
nbd>
h.connect_command(["nbdkit","-s","memory","10"])
nbd> copy=None
nbd> def f(b,o,s,e):
... global copy
... copy = b
... print(b[0])
...
nbd> print(copy)
None
nbd> buf = h.pread_structured(10,0,f)
0
nbd> copy
<memory at 0x7fecef4e07c0>
nbd> copy[0]=0x31
nbd> print(buf)
bytearray(b'1\x00\x00\x00\x00\x00\x00\x00\x00\x00')
That is, because we passed in a memoryview slice of the original
buffer instead of a temporary slice of C memory, we can now stash that
slice into a global, modify it, and see those modifications reflected
back to the overall bytearray object returned by pread_structured.
The generated diff is a bit more verbose, including some no-op lines
added into pread, aio_pread, and aio_pread_structured. In practice,
only pread_structured is actually impacted.
| --- python/methods.c.bak 2022-05-31 09:51:30.406757934 -0500
| +++ python/methods.c 2022-05-31 10:26:01.797636894 -0500
| @@ -27,6 +27,7 @@
| #include <stdlib.h>
| #include <stdint.h>
| #include <stdbool.h>
| +#include <stddef.h>
|
| #include <libnbd.h>
|
| @@ -76,8 +77,24 @@ chunk_wrapper (void *user_data, const vo
| PyObject *py_subbuf = NULL;
| PyObject *py_error = NULL;
|
| - /* Cast away const */
| - py_subbuf = PyMemoryView_FromMemory ((char *) subbuf, count, PyBUF_READ);
| + if (data->buf) {
| + PyObject *start, *end, *slice;
| + assert (PyMemoryView_Check (data->buf));
| + ptrdiff_t offs = subbuf - PyMemoryView_GET_BUFFER (data->buf)->buf;
| + start = PyLong_FromLong (offs);
| + if (!start) { PyErr_PrintEx (0); goto out; }
| + end = PyLong_FromLong (offs + count);
| + if (!end) { Py_DECREF (start); PyErr_PrintEx (0); goto out; }
| + slice = PySlice_New (start, end, NULL);
| + Py_DECREF (start);
| + Py_DECREF (end);
| + if (!slice) { PyErr_PrintEx (0); goto out; }
| + py_subbuf = PyObject_GetItem (data->buf, slice);
| + }
| + else {
| + /* Cast away const */
| + py_subbuf = PyMemoryView_FromMemory ((char *) subbuf, count, PyBUF_READ);
| + }
| if (!py_subbuf) { PyErr_PrintEx (0); goto out; }
| PyObject *py_error_modname = PyUnicode_FromString ("ctypes");
| if (!py_error_modname) { PyErr_PrintEx (0); goto out; }
| @@ -115,11 +132,11 @@ chunk_wrapper (void *user_data, const vo
| };
|
| out:
| - if (py_subbuf) {
| + if (py_subbuf && !data->buf) {
| PyObject *tmp = PyObject_CallMethod(py_subbuf, "release",
NULL);
| Py_XDECREF (tmp);
| - Py_DECREF (py_subbuf);
| }
| + Py_XDECREF (py_subbuf);
| if (py_error) {
| PyObject *py_error_ret = PyObject_GetAttrString (py_error,
"value");
| *error = PyLong_AsLong (py_error_ret);
| @@ -2304,7 +2321,7 @@ nbd_internal_py_pread (PyObject *self, P
| struct nbd_handle *h;
| int ret;
| PyObject *py_ret = NULL;
| - PyObject *buf = NULL;
| + PyObject *py_buf = NULL;
| Py_ssize_t count;
| uint64_t offset_u64;
| unsigned long long offset; /* really uint64_t */
| @@ -2318,20 +2335,20 @@ nbd_internal_py_pread (PyObject *self, P
| h = get_handle (py_h);
| if (!h) goto out;
| flags_u32 = flags;
| - buf = PyByteArray_FromStringAndSize (NULL, count);
| - if (buf == NULL) goto out;
| + py_buf = PyByteArray_FromStringAndSize (NULL, count);
| + if (py_buf == NULL) goto out;
| offset_u64 = offset;
|
| - ret = nbd_pread (h, PyByteArray_AS_STRING (buf), count, offset_u64,
flags_u32);
| + ret = nbd_pread (h, PyByteArray_AS_STRING (py_buf), count, offset_u64,
flags_u32);
| if (ret == -1) {
| raise_exception ();
| goto out;
| }
| - py_ret = buf;
| - buf = NULL;
| + py_ret = py_buf;
| + py_buf = NULL;
|
| out:
| - Py_XDECREF (buf);
| + Py_XDECREF (py_buf);
| return py_ret;
| }
|
| @@ -2342,7 +2359,7 @@ nbd_internal_py_pread_structured (PyObje
| struct nbd_handle *h;
| int ret;
| PyObject *py_ret = NULL;
| - PyObject *buf = NULL;
| + PyObject *py_buf = NULL;
| Py_ssize_t count;
| uint64_t offset_u64;
| unsigned long long offset; /* really uint64_t */
| @@ -2360,8 +2377,8 @@ nbd_internal_py_pread_structured (PyObje
| h = get_handle (py_h);
| if (!h) goto out;
| flags_u32 = flags;
| - buf = PyByteArray_FromStringAndSize (NULL, count);
| - if (buf == NULL) goto out;
| + py_buf = PyByteArray_FromStringAndSize (NULL, count);
| + if (py_buf == NULL) goto out;
| offset_u64 = offset;
| chunk.user_data = chunk_user_data = alloc_user_data ();
| if (chunk_user_data == NULL) goto out;
| @@ -2373,18 +2390,20 @@ nbd_internal_py_pread_structured (PyObje
| /* Increment refcount since pointer may be saved by libnbd. */
| Py_INCREF (py_chunk_fn);
| chunk_user_data->fn = py_chunk_fn;
| + if (py_buf)
| + chunk_user_data->buf = PyMemoryView_FromObject (py_buf);
|
| - ret = nbd_pread_structured (h, PyByteArray_AS_STRING (buf), count,
offset_u64, chunk, flags_u32);
| + ret = nbd_pread_structured (h, PyByteArray_AS_STRING (py_buf), count,
offset_u64, chunk, flags_u32);
| chunk_user_data = NULL;
| if (ret == -1) {
| raise_exception ();
| goto out;
| }
| - py_ret = buf;
| - buf = NULL;
| + py_ret = py_buf;
| + py_buf = NULL;
|
| out:
| - Py_XDECREF (buf);
| + Py_XDECREF (py_buf);
| free_user_data (chunk_user_data);
| return py_ret;
| }
| @@ -3171,6 +3190,7 @@ nbd_internal_py_aio_pread (PyObject *sel
| struct nbd_handle *h;
| int64_t ret;
| PyObject *py_ret = NULL;
| + PyObject *py_buf = NULL;
| PyObject *buf; /* PyCapsule pointing to struct py_aio_buffer */
| struct py_aio_buffer *buf_buf;
| uint64_t offset_u64;
| @@ -3230,6 +3250,7 @@ nbd_internal_py_aio_pread_structured (Py
| struct nbd_handle *h;
| int64_t ret;
| PyObject *py_ret = NULL;
| + PyObject *py_buf = NULL;
| PyObject *buf; /* PyCapsule pointing to struct py_aio_buffer */
| struct py_aio_buffer *buf_buf;
| uint64_t offset_u64;
| @@ -3282,6 +3303,8 @@ nbd_internal_py_aio_pread_structured (Py
| /* Increment refcount since pointer may be saved by libnbd. */
| Py_INCREF (py_chunk_fn);
| chunk_user_data->fn = py_chunk_fn;
| + if (py_buf)
| + chunk_user_data->buf = PyMemoryView_FromObject (py_buf);
|
| ret = nbd_aio_pread_structured (h, buf_buf->data, buf_buf->len,
offset_u64, chunk, completion, flags_u32);
| chunk_user_data = NULL;
---
generator/Python.ml | 60 +++++++++++++++++++++++++++++++++------------
1 file changed, 44 insertions(+), 16 deletions(-)
diff --git a/generator/Python.ml b/generator/Python.ml
index 0191f79..4160759 100644
--- a/generator/Python.ml
+++ b/generator/Python.ml
@@ -187,8 +187,24 @@ let
pr " PyList_SET_ITEM (py_%s, i_%s, py_e_%s);\n" n n n;
pr " }\n"
| CBBytesIn (n, len) ->
- pr " /* Cast away const */\n";
- pr " py_%s = PyMemoryView_FromMemory ((char *) %s, %s,
PyBUF_READ);\n" n n len;
+ pr " if (data->buf) {\n";
+ pr " PyObject *start, *end, *slice;\n";
+ pr " assert (PyMemoryView_Check (data->buf));\n";
+ pr " ptrdiff_t offs = %s - PyMemoryView_GET_BUFFER
(data->buf)->buf;\n" n;
+ pr " start = PyLong_FromLong (offs);\n";
+ pr " if (!start) { PyErr_PrintEx (0); goto out; }\n";
+ pr " end = PyLong_FromLong (offs + %s);\n" len;
+ pr " if (!end) { Py_DECREF (start); PyErr_PrintEx (0); goto out;
}\n";
+ pr " slice = PySlice_New (start, end, NULL);\n";
+ pr " Py_DECREF (start);\n";
+ pr " Py_DECREF (end);\n";
+ pr " if (!slice) { PyErr_PrintEx (0); goto out; }\n";
+ pr " py_%s = PyObject_GetItem (data->buf, slice);\n" n;
+ pr " }\n";
+ pr " else {\n";
+ pr " /* Cast away const */\n";
+ pr " py_%s = PyMemoryView_FromMemory ((char *) %s, %s,
PyBUF_READ);\n" n n len;
+ pr " }\n";
pr " if (!py_%s) { PyErr_PrintEx (0); goto out; }\n" n
| CBInt _
| CBInt64 _ -> ()
@@ -272,11 +288,11 @@ let
pr " }\n"
| CBBytesIn (n, _) ->
(*
https://stackoverflow.com/questions/58870010/how-to-call-release-on-a-memoryview-in-python-c-api
*)
- pr " if (py_%s) {\n" n;
+ pr " if (py_%s && !data->buf) {\n" n;
pr " PyObject *tmp = PyObject_CallMethod(py_%s,
\"release\", NULL);\n" n;
pr " Py_XDECREF (tmp);\n";
- pr " Py_DECREF (py_%s);\n" n;
- pr " }\n"
+ pr " }\n";
+ pr " Py_XDECREF (py_%s);\n" n
| CBInt _ | CBInt64 _
| CBString _
| CBUInt _ | CBUInt64 _ -> ()
@@ -288,6 +304,7 @@ let
(* Generate the Python binding. *)
let print_python_binding name { args; optargs; ret; may_set_error } + let
store_mem = ref false in
pr "PyObject *\n";
pr "nbd_internal_py_%s (PyObject *self, PyObject *args)\n" name;
pr "{\n";
@@ -301,13 +318,19 @@ let
| BytesIn (n, _) ->
pr " Py_buffer %s = { .obj = NULL };\n" n
| BytesOut (n, count) ->
- pr " PyObject *%s = NULL;\n" n;
- pr " Py_ssize_t %s;\n" count
- | BytesPersistIn (n, _)
- | BytesPersistOut (n, _) ->
+ pr " PyObject *py_%s = NULL;\n" n;
+ pr " Py_ssize_t %s;\n" count;
+ store_mem := true
+ | BytesPersistIn (n, _) ->
pr " PyObject *%s; /* PyCapsule pointing to struct py_aio_buffer
*/\n"
n;
pr " struct py_aio_buffer *%s_buf;\n" n
+ | BytesPersistOut (n, _) ->
+ pr " PyObject *py_%s = NULL;\n" n;
+ pr " PyObject *%s; /* PyCapsule pointing to struct py_aio_buffer
*/\n"
+ n;
+ pr " struct py_aio_buffer *%s_buf;\n" n;
+ store_mem := true
| Closure { cbname } ->
pr " struct user_data *%s_user_data = NULL;\n" cbname;
pr " PyObject *py_%s_fn;\n" cbname;
@@ -440,8 +463,8 @@ let
| Bool _ -> ()
| BytesIn _ -> ()
| BytesOut (n, count) ->
- pr " %s = PyByteArray_FromStringAndSize (NULL, %s);\n" n
count;
- pr " if (%s == NULL) goto out;\n" n
+ pr " py_%s = PyByteArray_FromStringAndSize (NULL, %s);\n" n
count;
+ pr " if (py_%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;
@@ -458,7 +481,11 @@ let
pr " }\n";
pr " /* Increment refcount since pointer may be saved by libnbd.
*/\n";
pr " Py_INCREF (py_%s_fn);\n" cbname;
- pr " %s_user_data->fn = py_%s_fn;\n" cbname cbname
+ pr " %s_user_data->fn = py_%s_fn;\n" cbname cbname;
+ if !store_mem then (
+ pr " if (py_buf)\n";
+ pr " %s_user_data->buf = PyMemoryView_FromObject
(py_buf);\n" cbname
+ )
| Enum _ -> ()
| Flags (n, _) -> pr " %s_u32 = %s;\n" n n
| Fd _ | Int _ -> ()
@@ -487,7 +514,7 @@ let
function
| Bool n -> pr ", %s" n
| BytesIn (n, _) -> pr ", %s.buf, %s.len" n n
- | BytesOut (n, count) -> pr ", PyByteArray_AS_STRING (%s), %s"
n count
+ | BytesOut (n, count) -> pr ", PyByteArray_AS_STRING (py_%s),
%s" n count
| BytesPersistIn (n, _)
| BytesPersistOut (n, _) -> pr ", %s_buf->data,
%s_buf->len" n n
| Closure { cbname } -> pr ", %s" cbname
@@ -533,8 +560,8 @@ let
List.iter (
function
| BytesOut (n, _) ->
- pr " py_ret = %s;\n" n;
- pr " %s = NULL;\n" n;
+ pr " py_ret = py_%s;\n" n;
+ pr " py_%s = NULL;\n" n;
use_ret := false
| Bool _
| BytesIn _
@@ -581,7 +608,7 @@ let
| BytesIn (n, _) ->
pr " if (%s.obj)\n" n;
pr " PyBuffer_Release (&%s);\n" n
- | BytesOut (n, _) -> pr " Py_XDECREF (%s);\n" n
+ | BytesOut (n, _) -> pr " Py_XDECREF (py_%s);\n" n
| BytesPersistIn _ | BytesPersistOut _ -> ()
| Closure { cbname } ->
pr " free_user_data (%s_user_data);\n" cbname
@@ -620,6 +647,7 @@ let
pr "#include <stdlib.h>\n";
pr "#include <stdint.h>\n";
pr "#include <stdbool.h>\n";
+ pr "#include <stddef.h>\n";
pr "\n";
pr "#include <libnbd.h>\n";
pr "\n";
--
2.36.1
Richard W.M. Jones
2022-Jun-01 08:52 UTC
[Libguestfs] [libnbd PATCH 3/2] python: Slice pread_structured buffer from original
I agree with Nir's feedback and R-b's and don't have anything else to add. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v