[Libguestfs] [libnbd PATCH v2 6/8] python: Don't unwrap nbd.Buffer in nbd.py
Richard W.M. Jones
rjones at redhat.com
Tue Jun 7 13:18:11 UTC 2022
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 <rjones at 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
More information about the Libguestfs
mailing list