Re: [Libguestfs] [libnbd PATCH v2 6/8] python: Don't unwrap nbd.Buffer in nbd.py

Prior to this commit, we were unwrapping buf._o in nbd.py, which causes cryptic errors when the user passes in the wrong type: $ nbdkit -U - memory 10 --run \ 'nbdsh -u "$uri" -c "h.aio_pread(bytearray(10), 0)"' Traceback (most recent call last): ... File "/usr/lib64/python3.10/site-packages/nbd.py", line 2204, in aio_pread return libnbdmod.aio_pread(self._o, buf._o, offset, completion, AttributeError: 'bytearray' object has no attribute '_o' or worse, if the user passes in an unrelated type that does have an attribute "_o" but is not a PyCapsule wrapper: $ nbdkit -U - memory 10 --run 'nbdsh -u "$uri" -c " class foo(object): def __init__(self): self._o = 1 h.aio_pread(foo(), 0) "' Traceback (most recent call last): ... File "/usr/lib64/python3.10/site-packages/nbd.py", line 2204, in aio_pread return libnbdmod.aio_pread(self._o, buf._o, offset, completion, ValueError: PyCapsule_GetPointer called with invalid PyCapsule object Change things to instead do the unwrapping in our helper function. This will also make it easier for an upcoming patch to accept more types than just nbd.Buffer, with the goal of accepting any python buffer-like object. For now, passing in the wrong type still fails, but with a nicer message: $ ./run nbdkit -U - memory 10 --run \ 'nbdsh -u "$uri" -c "h.aio_pread(bytearray(10), 0)"' Traceback (most recent call last): ... File "/home/eblake/libnbd/python/nbd.py", line 2204, in aio_pread return libnbdmod.aio_pread(self._o, buf, offset, completion, flags) TypeError: aio_buffer: expecting nbd.Buffer instance The next patch will clean up the OCaml code, now that none of the parameter types need alternative text. The generated code changes as follows: | --- python/methods.h.bak 2022-06-06 16:55:27.549200523 -0500 | +++ python/methods.h 2022-06-06 16:55:34.097211464 -0500 | @@ -34,6 +34,7 @@ | struct sockaddr_storage *, socklen_t *); | extern PyObject *nbd_internal_py_get_aio_view (PyObject *, bool); | extern int nbd_internal_py_init_aio_buffer (PyObject *); | +extern PyObject *nbd_internal_py_get_nbd_buffer_type (void); | | static inline struct nbd_handle * | get_handle (PyObject *obj) | --- python/methods.c.bak 2022-06-06 16:55:27.543200513 -0500 | +++ python/methods.c 2022-06-06 16:55:34.114211492 -0500 | @@ -3079,7 +3079,7 @@ nbd_internal_py_aio_pread (PyObject *sel | struct nbd_handle *h; | int64_t ret; | PyObject *py_ret = NULL; | - PyObject *buf; /* PyCapsule pointing to struct py_aio_buffer */ | + PyObject *buf; /* Instance of nbd.Buffer */ | PyObject *buf_view = NULL; /* PyMemoryView of buf */ | Py_buffer *py_buf; /* buffer of buf_view */ | uint64_t offset_u64; | @@ -3142,7 +3142,7 @@ nbd_internal_py_aio_pread_structured (Py | struct nbd_handle *h; | int64_t ret; | PyObject *py_ret = NULL; | - PyObject *buf; /* PyCapsule pointing to struct py_aio_buffer */ | + PyObject *buf; /* Instance of nbd.Buffer */ | PyObject *buf_view = NULL; /* PyMemoryView of buf */ | Py_buffer *py_buf; /* buffer of buf_view */ | uint64_t offset_u64; | @@ -3222,7 +3222,7 @@ nbd_internal_py_aio_pwrite (PyObject *se | struct nbd_handle *h; | int64_t ret; | PyObject *py_ret = NULL; | - PyObject *buf; /* PyCapsule pointing to struct py_aio_buffer */ | + PyObject *buf; /* Instance of nbd.Buffer */ | PyObject *buf_view = NULL; /* PyMemoryView of buf */ | Py_buffer *py_buf; /* buffer of buf_view */ | uint64_t offset_u64; | --- python/nbd.py.bak 2022-06-06 16:55:27.552200528 -0500 | +++ python/nbd.py 2022-06-06 16:55:34.123211507 -0500 | @@ -141,7 +141,7 @@ class Buffer(object): | | def to_bytearray(self): | '''Copy an AIO buffer into a bytearray.''' | - return libnbdmod.aio_buffer_to_bytearray(self._o) | + return libnbdmod.aio_buffer_to_bytearray(self) | | def size(self): | '''Return the size of an AIO buffer.''' | @@ -161,7 +161,7 @@ class Buffer(object): | always returns true. If size > 0, we check the interval | [offset..offset+size-1]. | ''' | - return libnbdmod.aio_buffer_is_zero(self._o, offset, size) | + return libnbdmod.aio_buffer_is_zero(self, offset, size) | | | class NBD(object): | @@ -2208,8 +2208,7 @@ class NBD(object): | alter which scenarios should await a server reply rather | than failing fast. | ''' | - return libnbdmod.aio_pread(self._o, buf._o, offset, completion, | - flags) | + return libnbdmod.aio_pread(self._o, buf, offset, completion, flags) | | def aio_pread_structured(self, buf, offset, chunk, completion=None, | flags=0): | @@ -2243,8 +2242,8 @@ class NBD(object): | alter which scenarios should await a server reply rather | than failing fast. | ''' | - return libnbdmod.aio_pread_structured(self._o, buf._o, offset, | - chunk, completion, flags) | + return libnbdmod.aio_pread_structured(self._o, buf, offset, chunk, | + completion, flags) | | def aio_pwrite(self, buf, offset, completion=None, flags=0): | u'''▶ write to the NBD server | @@ -2267,8 +2266,7 @@ class NBD(object): | alter which scenarios should await a server reply rather | than failing fast. | ''' | - return libnbdmod.aio_pwrite(self._o, buf._o, offset, completion, | - flags) | + return libnbdmod.aio_pwrite(self._o, buf, offset, completion, flags) | | def aio_disconnect(self, flags=0): | u'''▶ disconnect from the NBD server --- generator/Python.ml | 14 +++++++------- python/handle.c | 20 ++++++++++++++------ python/utils.c | 20 +++++++++++++++++++- 3 files changed, 40 insertions(+), 14 deletions(-) diff --git a/generator/Python.ml b/generator/Python.ml index 1afe0cf..101f3e0 100644 --- a/generator/Python.ml +++ b/generator/Python.ml @@ -40,6 +40,7 @@ let struct sockaddr_storage *, socklen_t *); extern PyObject *nbd_internal_py_get_aio_view (PyObject *, bool); extern int nbd_internal_py_init_aio_buffer (PyObject *); +extern PyObject *nbd_internal_py_get_nbd_buffer_type (void); static inline struct nbd_handle * get_handle (PyObject *obj) @@ -299,8 +300,7 @@ let pr " Py_ssize_t %s;\n" count | BytesPersistIn (n, _) | BytesPersistOut (n, _) -> - pr " PyObject *%s; /* PyCapsule pointing to struct py_aio_buffer */\n" - n; + pr " PyObject *%s; /* Instance of nbd.Buffer */\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 } -> @@ -755,11 +755,11 @@ let def to_bytearray(self): '''Copy an AIO buffer into a bytearray.''' - return libnbdmod.aio_buffer_to_bytearray(self._o) + return libnbdmod.aio_buffer_to_bytearray(self) def size(self): '''Return the size of an AIO buffer.''' - return libnbdmod.aio_buffer_size(self._o) + return libnbdmod.aio_buffer_size(self) def is_zero(self, offset=0, size=-1): '''Returns true if and only if all bytes in the buffer are zeroes. @@ -775,7 +775,7 @@ let always returns true. If size > 0, we check the interval [offset..offset+size-1]. ''' - return libnbdmod.aio_buffer_is_zero(self._o, offset, size) + return libnbdmod.aio_buffer_is_zero(self, offset, size) class NBD(object): @@ -800,8 +800,8 @@ let | 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) + | BytesPersistIn (n, _) -> n, None, None + | BytesPersistOut (n, _) -> n, None, None | Closure { cbname } -> cbname, None, None | Enum (n, _) -> n, None, None | Flags (n, _) -> n, None, None diff --git a/python/handle.c b/python/handle.c index 61dd736..79b369d 100644 --- a/python/handle.c +++ b/python/handle.c @@ -106,16 +106,24 @@ static const char aio_buffer_name[] = "nbd.Buffer"; /* Return internal buffer pointer of nbd.Buffer */ static struct py_aio_buffer * -nbd_internal_py_get_aio_buffer (PyObject *capsule) +nbd_internal_py_get_aio_buffer (PyObject *object) { - return PyCapsule_GetPointer (capsule, aio_buffer_name); + if (PyObject_IsInstance (object, nbd_internal_py_get_nbd_buffer_type ())) { + PyObject *capsule = PyObject_GetAttrString(object, "_o"); + + return PyCapsule_GetPointer (capsule, aio_buffer_name); + } + + PyErr_SetString (PyExc_TypeError, + "aio_buffer: expecting nbd.Buffer instance"); + return NULL; } /* Return new reference to MemoryView wrapping aio_buffer contents */ PyObject * -nbd_internal_py_get_aio_view (PyObject *capsule, bool require_init) +nbd_internal_py_get_aio_view (PyObject *object, bool require_init) { - struct py_aio_buffer *buf = nbd_internal_py_get_aio_buffer (capsule); + struct py_aio_buffer *buf = nbd_internal_py_get_aio_buffer (object); if (!buf) return NULL; @@ -130,9 +138,9 @@ nbd_internal_py_get_aio_view (PyObject *capsule, bool require_init) } int -nbd_internal_py_init_aio_buffer (PyObject *capsule) +nbd_internal_py_init_aio_buffer (PyObject *object) { - struct py_aio_buffer *buf = nbd_internal_py_get_aio_buffer (capsule); + struct py_aio_buffer *buf = nbd_internal_py_get_aio_buffer (object); assert (buf); buf->initialized = true; diff --git a/python/utils.c b/python/utils.c index 37f0c55..e0df181 100644 --- a/python/utils.c +++ b/python/utils.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 @@ -155,3 +155,21 @@ nbd_internal_py_get_sockaddr (PyObject *addr, return -1; } } + +/* Obtain the type object for nbd.Buffer */ +PyObject * +nbd_internal_py_get_nbd_buffer_type (void) +{ + static PyObject *type; + + if (!type) { + PyObject *modname = PyUnicode_FromString ("nbd"); + PyObject *module = PyImport_Import (modname); + assert (module); + type = PyObject_GetAttrString (module, "Buffer"); + assert (type); + Py_DECREF (modname); + Py_DECREF (module); + } + return type; +}