[Libguestfs] [libnbd PATCH] python: Allow control over copy/share of nbd.Buffer

[Richard W.M. Jones]

On Tue, Jun 14, 2022 at 07:58:08AM -0500, Eric Blake wrote:
> Add new methods nbd.Buffer.{to,from}_buffer that avoid the copying
> present in the existing nbd.Buffer.{to,from}_bytearray.  The reduction
> in copies possible by this approach is no longer quite as necessary,
> now that aio_p{read,write} have been converted to take buffers
> directly.  However, there is still one (marginal) benefit: if you use
> h.set_pread_initialize(False), and create a new buffer for every I/O
> (rather than reusing a buffer pool), nbd.Buffer(n) handed to
> h.aio_pread then buf.to_buffer() is slightly faster than handing a
> bytearray(n) directly to h.aio_pread, because we are able to skip the
> step of pre-zeroing the buffer.
> ---
> 
> Instead of adding a copy=True/False parameter to the existing API as I
> had previously suggested, I decided that a new API made more sense.
> 
>  generator/Python.ml              | 39 ++++++++++++++++++++++----
>  python/t/585-aio-buffer-share.py | 47 ++++++++++++++++++++++++++++++++
>  2 files changed, 80 insertions(+), 6 deletions(-)
>  create mode 100644 python/t/585-aio-buffer-share.py
> 
> diff --git a/generator/Python.ml b/generator/Python.ml
> index 03a7e6b..cb89ccd 100644
> --- a/generator/Python.ml
> +++ b/generator/Python.ml
> @@ -735,6 +735,21 @@ let
>          '''Allocate an uninitialized AIO buffer used for nbd.aio_pread.'''
>          self._o = libnbdmod.alloc_aio_buffer(len)
> 
> +    @classmethod
> +    def from_buffer(cls, buf):
> +        '''Create an AIO buffer that shares an existing buffer-like object.
> +
> +        Because the buffer is shared, changes to the original are visible
> +        to nbd.aio_pwrite, and changes in nbd.aio_pread are visible to the
> +        original.
> +        '''
> +        self = cls(0)
> +        # Ensure that buf is already buffer-like
> +        with memoryview(buf):
> +            self._o = buf
> +        self._init = True
> +        return self
> +
>      @classmethod
>      def from_bytearray(cls, ba):
>          '''Create an AIO buffer from a bytearray or other buffer-like object.
> @@ -743,16 +758,28 @@ let
>          bytearray constructor.  Otherwise, ba is copied.  Either way, the
>          resulting AIO buffer is independent from the original.
>          '''
> -        self = cls(0)
> -        self._o = bytearray(ba)
> -        self._init = True
> -        return self
> +        return cls.from_buffer(bytearray(ba))
> 
> -    def to_bytearray(self):
> -        '''Copy an AIO buffer into a bytearray.'''
> +    def to_buffer(self):
> +        '''Return a shared view of the AIO buffer contents.
> +
> +        This exposes the underlying buffer; changes to the buffer are
> +        visible to nbd.aio_pwrite, and changes from nbd.aio_pread are
> +        visible in the buffer.
> +        '''
>          if not hasattr(self, '_init'):
>              self._o = bytearray(len(self._o))
>              self._init = True
> +        return self._o
> +
> +    def to_bytearray(self):
> +        '''Copy an AIO buffer into a bytearray.
> +
> +        This copies the contents of an AIO buffer to a new bytearray, which
> +        remains independent from the original.
> +        '''
> +        if not hasattr(self, '_init'):
> +            return bytearray(len(self._o))
>          return bytearray(self._o)
> 
>      def size(self):
> diff --git a/python/t/585-aio-buffer-share.py b/python/t/585-aio-buffer-share.py
> new file mode 100644
> index 0000000..21752b7
> --- /dev/null
> +++ b/python/t/585-aio-buffer-share.py
> @@ -0,0 +1,47 @@
> +# libnbd Python bindings
> +# Copyright (C) 2010-2022 Red Hat Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> +
> +# This tests the nbd.Buffer is_zero method.  We can do this entirely
> +# without using the NBD protocol.
> +
> +import nbd
> +
> +# Use of to/from_bytearray always creates copies
> +ba = bytearray(512)
> +buf = nbd.Buffer.from_bytearray(ba)
> +ba.append(1)
> +assert len(ba) == 513
> +assert len(buf) == 512
> +assert buf.is_zero()
> +assert buf.to_bytearray() is not ba
> +
> +# Use of to/from_buffer shares the same buffer
> +buf = nbd.Buffer.from_buffer(ba)
> +assert not buf.is_zero()
> +assert len(buf) == 513
> +ba.pop()
> +assert buf.is_zero()
> +assert len(buf) == 512
> +assert buf.to_buffer() is ba
> +
> +# Even though nbd.Buffer(n) start uninitialized, we sanitize before exporting.
> +# This test cheats and examines the private member ._init
> +buf = nbd.Buffer(512)
> +assert buf.is_zero()
> +assert not hasattr(buf, '_init')
> +assert buf.to_buffer() == bytearray(512)
> +assert hasattr(buf, '_init')
> -- 
> 2.36.1

Acked-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
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