On Mon, Jun 06, 2022 at 09:08:31PM -0500, Eric Blake wrote: > 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; > +}
Reviewed-by: Richard W.M. Jones <rjo...@redhat.com> Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com nbdkit - Flexible, fast NBD server with plugins https://gitlab.com/nbdkit/nbdkit _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs