[Libguestfs] [PATCH nbdkit 1/2] python: For v2 API, avoid copy by passing a buffer to pread.

[Richard W.M. Jones]

It's more efficient if we pass the C buffer directly to Python code.
In some cases the Python code will be able to write directly into the
C buffer using functions like file.readinto and socket.recv_into.
This avoids an extra copy.

Thanks: Nir Soffer
---
 plugins/python/example.py               |  8 ++++---
 plugins/python/nbdkit-python-plugin.pod | 16 +++++--------
 plugins/python/python.c                 | 32 +++++++++++++++----------
 tests/python-exception.py               |  4 ++--
 tests/shebang.py                        |  5 ++--
 5 files changed, 36 insertions(+), 29 deletions(-)

diff --git a/plugins/python/example.py b/plugins/python/example.py
index c85d2f8..c04b7e2 100644
--- a/plugins/python/example.py
+++ b/plugins/python/example.py
@@ -60,10 +60,12 @@ def get_size(h):
     return len(disk)
 
 
-def pread(h, count, offset, flags):
+def pread(h, buf, offset, flags):
     global disk
-    return disk[offset:offset+count]
-
+    end = offset + len(buf)
+    buf[:] = disk[offset:end]
+    # or if reading from a file you can use:
+    #f.readinto(buf)
 
 def pwrite(h, buf, offset, flags):
     global disk
diff --git a/plugins/python/nbdkit-python-plugin.pod b/plugins/python/nbdkit-python-plugin.pod
index 0b31ebc..4065ec7 100644
--- a/plugins/python/nbdkit-python-plugin.pod
+++ b/plugins/python/nbdkit-python-plugin.pod
@@ -38,7 +38,7 @@ C<__main__> module):
    # see below
  def get_size(h):
    # see below
- def pread(h, count, offset, flags):
+ def pread(h, buf, offset, flags):
    # see below
 
 Note that the subroutines must have those literal names (like C<open>),
@@ -249,16 +249,12 @@ contents will be garbage collected.
 
 (Required)
 
- def pread(h, count, offset, flags):
-   # construct a buffer of length count bytes and return it
+ def pread(h, buf, offset, flags):
+   # read into the buffer
 
-The body of your C<pread> function should construct a buffer of length
-(at least) C<count> bytes.  You should read C<count> bytes from the
-disk starting at C<offset>.  C<flags> is always 0.
-
-The returned buffer can be any type compatible with the Python 3
-buffer protocol, such as bytearray, bytes or memoryview
-(L<https://docs.python.org/3/c-api/buffer.html>)
+The body of your C<pread> function should read exactly C<len(buf)>
+bytes of data starting at disk C<offset> and write it into the buffer
+C<buf>.  C<flags> is always 0.
 
 NBD only supports whole reads, so your function should try to read
 the whole region (perhaps requiring a loop).  If the read fails or
diff --git a/plugins/python/python.c b/plugins/python/python.c
index 252ca37..79766df 100644
--- a/plugins/python/python.c
+++ b/plugins/python/python.c
@@ -527,7 +527,9 @@ py_pread (void *handle, void *buf, uint32_t count, uint64_t offset,
     r = PyObject_CallFunction (fn, "OiL", obj, count, offset, NULL);
     break;
   case 2:
-    r = PyObject_CallFunction (fn, "OiLI", obj, count, offset, flags, NULL);
+    r = PyObject_CallFunction (fn, "ONLI", obj,
+          PyMemoryView_FromMemory ((char *)buf, count, PyBUF_WRITE),
+          offset, flags, NULL);
     break;
   default: abort ();
   }
@@ -535,19 +537,25 @@ py_pread (void *handle, void *buf, uint32_t count, uint64_t offset,
   if (check_python_failure ("pread") == -1)
     return ret;
 
-  if (PyObject_GetBuffer (r, &view, PyBUF_SIMPLE) == -1) {
-    nbdkit_error ("%s: value returned from pread does not support the "
-                  "buffer protocol",
-                  script);
-    goto out;
-  }
+  if (py_api_version == 1) {
+    /* In API v1 the Python pread function had to return a buffer
+     * protocol compatible function.  In API v2+ it writes directly to
+     * the C buffer so this code is not used.
+     */
+    if (PyObject_GetBuffer (r, &view, PyBUF_SIMPLE) == -1) {
+      nbdkit_error ("%s: value returned from pread does not support the "
+                    "buffer protocol",
+                    script);
+      goto out;
+    }
 
-  if (view.len < count) {
-    nbdkit_error ("%s: buffer returned from pread is too small", script);
-    goto out;
-  }
+    if (view.len < count) {
+      nbdkit_error ("%s: buffer returned from pread is too small", script);
+      goto out;
+    }
 
-  memcpy (buf, view.buf, count);
+    memcpy (buf, view.buf, count);
+  }
   ret = 0;
 
 out:
diff --git a/tests/python-exception.py b/tests/python-exception.py
index d0c79bb..ee4a3f3 100644
--- a/tests/python-exception.py
+++ b/tests/python-exception.py
@@ -62,5 +62,5 @@ def get_size(h):
     return 0
 
 
-def pread(h, count, offset):
-    return ""
+def pread(h, buf, offset):
+    buf[:] = bytearray(len(buf))
diff --git a/tests/shebang.py b/tests/shebang.py
index 6f33623..0634589 100755
--- a/tests/shebang.py
+++ b/tests/shebang.py
@@ -13,6 +13,7 @@ def get_size(h):
     return len(disk)
 
 
-def pread(h, count, offset):
+def pread(h, buf, offset):
     global disk
-    return disk[offset:offset+count]
+    end = offset + len(buf)
+    buf[:] = disk[offset:end]
-- 
2.23.0