On reflection I have pushed this patch because it's an improvement to the current Python bindings and pretty much a change we would always want to make no matter how we deal with the buffer lifetimes issue. The rest of the series however, same as I wrote in the cover letter. On Sat, Aug 10, 2019 at 06:02:56PM +0100, Richard W.M. Jones wrote:
Create a class for AIO buffers. This is a mostly neutral code refactoring, but we add a convenient function for getting the size of the buffer (same as previous commit for OCaml). --- generator/generator | 111 +++++++++++++++++------------ python/handle.c | 26 +++++-- python/t/500-aio-pread.py | 4 +- python/t/505-aio-pread-callback.py | 10 +-- python/t/510-aio-pwrite.py | 6 +- 5 files changed, 96 insertions(+), 61 deletions(-)
diff --git a/generator/generator b/generator/generator index 26ab365..0107724 100755 --- a/generator/generator +++ b/generator/generator @@ -3981,8 +3981,10 @@ raise_exception () pr "extern PyObject *nbd_internal_py_%s (PyObject *self, PyObject *args);\n" name; ) ([ "create"; "close"; - "alloc_aio_buffer"; "aio_buffer_from_bytearray"; - "aio_buffer_to_bytearray" ] @ List.map fst handle_calls); + "alloc_aio_buffer"; + "aio_buffer_from_bytearray"; + "aio_buffer_to_bytearray"; + "aio_buffer_size" ] @ List.map fst handle_calls);
pr "\n"; pr "#endif /* LIBNBD_METHODS_H */\n" @@ -4009,8 +4011,10 @@ let generate_python_libnbdmod_c () = pr " { (char *) \"%s\", nbd_internal_py_%s, METH_VARARGS, NULL },\n" name name; ) ([ "create"; "close"; - "alloc_aio_buffer"; "aio_buffer_from_bytearray"; - "aio_buffer_to_bytearray" ] @ List.map fst handle_calls); + "alloc_aio_buffer"; + "aio_buffer_from_bytearray"; + "aio_buffer_to_bytearray"; + "aio_buffer_size" ] @ List.map fst handle_calls); pr " { NULL, NULL, 0, NULL }\n"; pr "};\n"; pr "\n"; @@ -4502,17 +4506,28 @@ Error.__str__ = _str
pr "\
-# AIO buffer functions. -def aio_buffer (len): - '''allocate an AIO buffer used for nbd.aio_pread and nbd.aio_pwrite''' - return libnbdmod.alloc_aio_buffer (len) - -def aio_buffer_from_bytearray (ba): - '''create an AIO buffer from a bytearray''' - return libnbdmod.aio_buffer_from_bytearray (ba) -def aio_buffer_to_bytearray (buf): - '''copy an AIO buffer into a bytearray''' - return libnbdmod.aio_buffer_to_bytearray (buf) +class Buffer (object): + '''Asynchronous I/O persistent buffer''' + + def __init__ (self, len): + '''allocate an uninitialized AIO buffer used for nbd.aio_pread''' + self._o = libnbdmod.alloc_aio_buffer (len) + + @classmethod + def from_bytearray (cls, ba): + '''create an AIO buffer from a bytearray''' + o = libnbdmod.aio_buffer_from_bytearray (ba) + self = cls (0) + self._o = o + return self + + def to_bytearray (self): + '''copy an AIO buffer into a bytearray''' + return libnbdmod.aio_buffer_to_bytearray (self._o) + + def size (self): + '''return the size of an AIO buffer''' + return libnbdmod.aio_buffer_size (self._o)
class NBD (object): '''NBD handle''' @@ -4532,42 +4547,46 @@ class NBD (object): let args = List.map ( function - | Bool n -> n - | BytesIn (n, _) | BytesPersistIn (n, _) -> n - | BytesPersistOut (n, _) -> n - | BytesOut (_, count) -> count - | Closure { cbname } -> cbname - | Int n -> n - | Int64 n -> n - | Path n -> n - | SockAddrAndLen (n, _) -> n - | String n -> n - | StringList n -> n - | UInt n -> n - | UInt32 n -> n - | UInt64 n -> n + | Bool n -> n, None, None + | BytesIn (n, _) -> n, None, None + | BytesOut (_, count) -> count, None, None + | BytesPersistIn (n, _) -> n, None, Some (sprintf "%s._o" n) + | BytesPersistOut (n, _) -> n, None, Some (sprintf "%s._o" n) + | Closure { cbname } -> cbname, None, None + | Int n -> n, None, None + | Int64 n -> n, None, None + | Path n -> n, None, None + | SockAddrAndLen (n, _) -> n, None, None + | String n -> n, None, None + | StringList n -> n, None, None + | UInt n -> n, None, None + | UInt32 n -> n, None, None + | UInt64 n -> n, None, None ) args in let optargs = List.map ( function - | OFlags n -> n, "0" + | OFlags n -> n, Some "0", None ) optargs in - let () = - let params = args @ List.map (fun (n, def) -> n ^ "=" ^ def) optargs in - let params = List.map ((^) ", ") params in - let params = String.concat "" params in - pr " def %s (self%s):\n" name params in - let () = - let longdesc = Str.global_replace py_fn_rex "C<nbd." longdesc in - let longdesc = Str.global_replace py_const_rex "C<" longdesc in - let longdesc = pod2text longdesc in - pr " '''▶ %s\n\n%s'''\n" - shortdesc (String.concat "\n" longdesc) in - let () = - let vars = args @ List.map fst optargs in - let vars = List.map ((^) ", ") vars in - let vars = String.concat "" vars in - pr " return libnbdmod.%s (self._o%s)\n" name vars in + let args = args @ optargs in + pr " def %s (self" name; + List.iter ( + function + | n, None, _ -> pr ", %s" n + | n, Some default, _ -> pr ", %s=%s" n default + ) args; + pr "):\n"; + let longdesc = Str.global_replace py_fn_rex "C<nbd." longdesc in + let longdesc = Str.global_replace py_const_rex "C<" longdesc in + let longdesc = pod2text longdesc in + pr " '''▶ %s\n\n%s'''\n" shortdesc (String.concat "\n" longdesc); + pr " return libnbdmod.%s (self._o" name; + List.iter ( + function + | _, _, Some getter -> pr ", %s" getter + | n, _, None -> pr ", %s" n + ) args; + pr ")\n"; pr "\n" ) handle_calls;
diff --git a/python/handle.c b/python/handle.c index 7cff41a..4b510ad 100644 --- a/python/handle.c +++ b/python/handle.c @@ -72,7 +72,7 @@ nbd_internal_py_close (PyObject *self, PyObject *args) return Py_None; }
-static char aio_buffer_name[] = "struct py_aio_buffer"; +static const char aio_buffer_name[] = "nbd.Buffer";
struct py_aio_buffer * nbd_internal_py_get_aio_buffer (PyObject *capsule) @@ -89,9 +89,7 @@ free_aio_buffer (PyObject *capsule) free (buf); }
-/* Allocate a persistent buffer used for nbd_aio_pread and - * nbd_aio_pwrite. - */ +/* Allocate a persistent buffer used for nbd_aio_pread. */ PyObject * nbd_internal_py_alloc_aio_buffer (PyObject *self, PyObject *args) { @@ -115,7 +113,7 @@ nbd_internal_py_alloc_aio_buffer (PyObject *self, PyObject *args) free (buf); return NULL; } - buf->data = calloc (1, buf->len); + buf->data = malloc (buf->len); if (buf->data == NULL) { PyErr_NoMemory (); free (buf); @@ -193,3 +191,21 @@ nbd_internal_py_aio_buffer_to_bytearray (PyObject *self, PyObject *args)
return PyByteArray_FromStringAndSize (buf->data, buf->len); } + +PyObject * +nbd_internal_py_aio_buffer_size (PyObject *self, PyObject *args) +{ + PyObject *obj; + struct py_aio_buffer *buf; + + if (!PyArg_ParseTuple (args, + (char *) "O:nbd_internal_py_aio_buffer_size", + &obj)) + return NULL; + + buf = nbd_internal_py_get_aio_buffer (obj); + if (buf == NULL) + return NULL; + + return PyLong_FromSsize_t (buf->len); +} diff --git a/python/t/500-aio-pread.py b/python/t/500-aio-pread.py index 6ff06fd..0c5a07a 100644 --- a/python/t/500-aio-pread.py +++ b/python/t/500-aio-pread.py @@ -20,12 +20,12 @@ import nbd h = nbd.NBD () h.connect_command (["nbdkit", "-s", "--exit-with-parent", "-v", "pattern", "size=512"]) -buf = nbd.aio_buffer (512) +buf = nbd.Buffer (512) cookie = h.aio_pread (buf, 0) while not (h.aio_command_completed (cookie)): h.poll (-1)
-buf = nbd.aio_buffer_to_bytearray (buf) +buf = buf.to_bytearray ()
print ("%r" % buf)
diff --git a/python/t/505-aio-pread-callback.py b/python/t/505-aio-pread-callback.py index 9246616..e552db8 100644 --- a/python/t/505-aio-pread-callback.py +++ b/python/t/505-aio-pread-callback.py @@ -43,21 +43,21 @@ def callback (user_data, err): assert user_data == 42
# First try: succeed in both callbacks -buf = nbd.aio_buffer (512) +buf = nbd.Buffer (512) cookie = h.aio_pread_structured_callback (buf, 0, lambda *args: chunk (42, *args), lambda *args: callback (42, *args)) while not (h.aio_command_completed (cookie)): h.poll (-1)
-buf = nbd.aio_buffer_to_bytearray (buf) +buf = buf.to_bytearray ()
print ("%r" % buf)
assert buf == expected
# Second try: fail only during callback -buf = nbd.aio_buffer (512) +buf = nbd.Buffer (512) cookie = h.aio_pread_structured_callback (buf, 0, lambda *args: chunk (42, *args), lambda *args: callback (43, *args)) @@ -69,7 +69,7 @@ except nbd.Error as ex: assert ex.errnum == errno.ENOMEM
# Third try: fail during both -buf = nbd.aio_buffer (512) +buf = nbd.Buffer (512) cookie = h.aio_pread_structured_callback (buf, 0, lambda *args: chunk (43, *args), lambda *args: callback (44, *args)) @@ -81,7 +81,7 @@ except nbd.Error as ex: assert ex.errnum == errno.ENOMEM
# Fourth try: fail only during chunk -buf = nbd.aio_buffer (512) +buf = nbd.Buffer (512) cookie = h.aio_pread_structured_callback (buf, 0, lambda *args: chunk (43, *args), lambda *args: callback (42, *args)) diff --git a/python/t/510-aio-pwrite.py b/python/t/510-aio-pwrite.py index 4bbc494..71aa9ba 100644 --- a/python/t/510-aio-pwrite.py +++ b/python/t/510-aio-pwrite.py @@ -33,17 +33,17 @@ h = nbd.NBD () h.connect_command (["nbdkit", "-s", "--exit-with-parent", "-v", "file", datafile])
-buf1 = nbd.aio_buffer_from_bytearray (buf) +buf1 = nbd.Buffer.from_bytearray (buf) cookie = h.aio_pwrite (buf1, 0, nbd.CMD_FLAG_FUA) while not (h.aio_command_completed (cookie)): h.poll (-1)
-buf2 = nbd.aio_buffer (512) +buf2 = nbd.Buffer (512) cookie = h.aio_pread (buf2, 0) while not (h.aio_command_completed (cookie)): h.poll (-1)
-assert buf == nbd.aio_buffer_to_bytearray (buf2) +assert buf == buf2.to_bytearray ()
with open (datafile, "rb") as f: content = f.read ()
Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top