From 49ef7e7d7c3602cc8e53d2052fce9d3a12840ea2 Mon Sep 17 00:00:00 2001

From: "Richard W.M. Jones" <rjones@redhat.com>

Date: Thu, 21 Nov 2019 15:44:39 +0000

Subject: [PATCH 05/19] python: Implement nbdkit API version 2.

To avoid breaking existing plugins, Python plugins wishing to use

version 2 of the API must opt in by declaring:

  API_VERSION = 2

(Plugins which do not do this are assumed to want API version 1).

For v2 API, we also avoid a copy by passing a buffer into pread.

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

https://www.redhat.com/archives/libguestfs/2019-November/thread.html#00220

(cherry picked from commit a9b2637cf4f00fb8a25ffaf31ee83be5fe019ae2)

---

 plugins/python/example.py               |  20 +++-

 plugins/python/nbdkit-python-plugin.pod |  69 +++++++-----

 plugins/python/python.c                 | 139 +++++++++++++++++++-----

 tests/python-exception.py               |   4 +-

 tests/shebang.py                        |   5 +-

 tests/test.py                           |  28 +++--

 6 files changed, 190 insertions(+), 75 deletions(-)

diff --git a/plugins/python/example.py b/plugins/python/example.py

index 60f9d7f0..c04b7e29 100644

--- a/plugins/python/example.py

+++ b/plugins/python/example.py

@@ -34,6 +34,12 @@ import errno

 disk = bytearray(1024 * 1024)

+# There are several variants of the API.  nbdkit will call this

+# function first to determine which one you want to use.  This is the

+# latest version at the time this example was written.

+API_VERSION = 2

+

+

 # This just prints the extra command line parameters, but real plugins

 # should parse them and reject any unknown parameters.

 def config(key, value):

@@ -54,20 +60,22 @@ def get_size(h):

     return len(disk)

-def pread(h, count, offset):

+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):

+def pwrite(h, buf, offset, flags):

     global disk

     end = offset + len(buf)

     disk[offset:end] = buf

-def zero(h, count, offset, may_trim):

+def zero(h, count, offset, flags):

     global disk

-    if may_trim:

+    if flags & nbdkit.FLAG_MAY_TRIM:

         disk[offset:offset+count] = bytearray(count)

     else:

         nbdkit.set_error(errno.EOPNOTSUPP)

diff --git a/plugins/python/nbdkit-python-plugin.pod b/plugins/python/nbdkit-python-plugin.pod

index 3680fd65..4923d9da 100644

--- a/plugins/python/nbdkit-python-plugin.pod

+++ b/plugins/python/nbdkit-python-plugin.pod

@@ -33,11 +33,12 @@ To write a Python nbdkit plugin, you create a Python file which

 contains at least the following required functions (in the top level

 C<__main__> module):

+ API_VERSION = 2

  def open(readonly):

    # see below

  def get_size(h):

    # see below

- def pread(h, count, offset):

+ def pread(h, buf, offset, flags):

    # see below

 Note that the subroutines must have those literal names (like C<open>),

@@ -82,6 +83,18 @@ I<--dump-plugin> option, eg:

  python_version=3.7.0

  python_pep_384_abi_version=3

+=head2 API versions

+

+The nbdkit API has evolved and new versions are released periodically.

+To ensure backwards compatibility plugins have to opt in to the new

+version.  From Python you do this by declaring a constant in your

+module:

+

+ API_VERSION = 2

+

+(where 2 is the latest version at the time this documentation was

+written).  All newly written Python modules must have this constant.

+

 =head2 Executable script

 If you want you can make the script executable and include a "shebang"

@@ -199,16 +212,12 @@ contents will be garbage collected.

 (Required)

- def pread(h, count, offset):

-   # 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>.

-

-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

@@ -219,13 +228,13 @@ C<nbdkit.set_error> first.

 (Optional)

- def pwrite(h, buf, offset):

+ def pwrite(h, buf, offset, flags):

    length = len (buf)

    # no return value

 The body of your C<pwrite> function should write the buffer C<buf> to

 the disk.  You should write C<count> bytes to the disk starting at

-C<offset>.

+C<offset>.  C<flags> may contain C<nbdkit.FLAG_FUA>.

 NBD only supports whole writes, so your function should try to

 write the whole region (perhaps requiring a loop).  If the write

@@ -236,11 +245,12 @@ fails or is partial, your function should throw an exception,

 (Optional)

- def flush(h):

+ def flush(h, flags):

    # no return value

 The body of your C<flush> function should do a L<sync(2)> or

 L<fdatasync(2)> or equivalent on the backing store.

+C<flags> is always 0.

 If the flush fails, your function should throw an exception, optionally

 using C<nbdkit.set_error> first.

@@ -249,32 +259,35 @@ using C<nbdkit.set_error> first.

 (Optional)

- def trim(h, count, offset):

+ def trim(h, count, offset, flags):

    # no return value

-The body of your C<trim> function should "punch a hole" in the

-backing store.  If the trim fails, your function should throw an

-exception, optionally using C<nbdkit.set_error> first.

+The body of your C<trim> function should "punch a hole" in the backing

+store.  C<flags> may contain C<nbdkit.FLAG_FUA>.  If the trim fails,

+your function should throw an exception, optionally using

+C<nbdkit.set_error> first.

 =item C<zero>

 (Optional)

- def zero(h, count, offset, may_trim):

+ def zero(h, count, offset, flags):

    # no return value

-The body of your C<zero> function should ensure that C<count> bytes

-of the disk, starting at C<offset>, will read back as zero.  If

-C<may_trim> is true, the operation may be optimized as a trim as long

-as subsequent reads see zeroes.

+The body of your C<zero> function should ensure that C<count> bytes of

+the disk, starting at C<offset>, will read back as zero.  C<flags> is

+a bitmask which may include C<nbdkit.FLAG_MAY_TRIM>,

+C<nbdkit.FLAG_FUA>, C<nbdkit.FLAG_FAST_ZERO>.

 NBD only supports whole writes, so your function should try to

-write the whole region (perhaps requiring a loop).  If the write

-fails or is partial, your function should throw an exception,

-optionally using C<nbdkit.set_error> first.  In particular, if

-you would like to automatically fall back to C<pwrite> (perhaps

-because there is nothing to optimize if C<may_trim> is false),

-use C<nbdkit.set_error(errno.EOPNOTSUPP)>.

+write the whole region (perhaps requiring a loop).

+

+If the write fails or is partial, your function should throw an

+exception, optionally using C<nbdkit.set_error> first.  In particular,

+if you would like to automatically fall back to C<pwrite> (perhaps

+because there is nothing to optimize if

+S<C<flags & nbdkit.FLAG_MAY_TRIM>> is false), use

+S<C<nbdkit.set_error (errno.EOPNOTSUPP)>>.

 =back

diff --git a/plugins/python/python.c b/plugins/python/python.c

index 47da0838..0f28595f 100644

--- a/plugins/python/python.c

+++ b/plugins/python/python.c

@@ -46,6 +46,8 @@

 #define PY_SSIZE_T_CLEAN 1

 #include <Python.h>

+#define NBDKIT_API_VERSION 2

+

 #include <nbdkit-plugin.h>

 #include "cleanup.h"

@@ -60,6 +62,7 @@

  */

 static const char *script;

 static PyObject *module;

+static int py_api_version = 1;

 static int last_error;

@@ -285,9 +288,14 @@ py_dump_plugin (void)

   PyObject *fn;

   PyObject *r;

+  /* Python version and ABI. */

   printf ("python_version=%s\n", PY_VERSION);

   printf ("python_pep_384_abi_version=%d\n", PYTHON_ABI_VERSION);

+  /* Maximum nbdkit API version supported. */

+  printf ("nbdkit_python_maximum_api_version=%d\n", NBDKIT_API_VERSION);

+

+  /* If the script has a dump_plugin function, call it. */

   if (script && callback_defined ("dump_plugin", &fn)) {

     PyErr_Clear ();

@@ -297,6 +305,30 @@ py_dump_plugin (void)

   }

 }

+static int

+get_py_api_version (void)

+{

+  PyObject *obj;

+  long value;

+

+  obj = PyObject_GetAttrString (module, "API_VERSION");

+  if (obj == NULL)

+    return 1;                   /* Default to API version 1. */

+

+  value = PyLong_AsLong (obj);

+  Py_DECREF (obj);

+

+  if (value < 1 || value > NBDKIT_API_VERSION) {

+    nbdkit_error ("%s: API_VERSION requested unknown version: %ld.  "

+                  "This plugin supports API versions between 1 and %d.",

+                  script, value, NBDKIT_API_VERSION);

+    return -1;

+  }

+

+  nbdkit_debug ("module requested API_VERSION %ld", value);

+  return (int) value;

+}

+

 static int

 py_config (const char *key, const char *value)

 {

@@ -359,6 +391,11 @@ py_config (const char *key, const char *value)

                     "nbdkit requires these callbacks.", script);

       return -1;

     }

+

+    /* Get the API version. */

+    py_api_version = get_py_api_version ();

+    if (py_api_version == -1)

+      return -1;

   }

   else if (callback_defined ("config", &fn)) {

     /* Other parameters are passed to the Python .config callback. */

@@ -469,8 +506,8 @@ py_get_size (void *handle)

 }

 static int

-py_pread (void *handle, void *buf,

-          uint32_t count, uint64_t offset)

+py_pread (void *handle, void *buf, uint32_t count, uint64_t offset,

+          uint32_t flags)

 {

   PyObject *obj = handle;

   PyObject *fn;

@@ -485,24 +522,40 @@ py_pread (void *handle, void *buf,

   PyErr_Clear ();

-  r = PyObject_CallFunction (fn, "OiL", obj, count, offset);

+  switch (py_api_version) {

+  case 1:

+    r = PyObject_CallFunction (fn, "OiL", obj, count, offset);

+    break;

+  case 2:

+    r = PyObject_CallFunction (fn, "ONLI", obj,

+          PyMemoryView_FromMemory ((char *)buf, count, PyBUF_WRITE),

+          offset, flags);

+    break;

+  default: abort ();

+  }

   Py_DECREF (fn);

   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:

@@ -515,8 +568,8 @@ out:

 }

 static int

-py_pwrite (void *handle, const void *buf,

-           uint32_t count, uint64_t offset)

+py_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset,

+           uint32_t flags)

 {

   PyObject *obj = handle;

   PyObject *fn;

@@ -525,9 +578,19 @@ py_pwrite (void *handle, const void *buf,

   if (callback_defined ("pwrite", &fn)) {

     PyErr_Clear ();

-    r = PyObject_CallFunction (fn, "ONL", obj,

+    switch (py_api_version) {

+    case 1:

+      r = PyObject_CallFunction (fn, "ONL", obj,

             PyMemoryView_FromMemory ((char *)buf, count, PyBUF_READ),

             offset);

+      break;

+    case 2:

+      r = PyObject_CallFunction (fn, "ONLI", obj,

+            PyMemoryView_FromMemory ((char *)buf, count, PyBUF_READ),

+            offset, flags);

+      break;

+    default: abort ();

+    }

     Py_DECREF (fn);

     if (check_python_failure ("pwrite") == -1)

       return -1;

@@ -542,7 +605,7 @@ py_pwrite (void *handle, const void *buf,

 }

 static int

-py_flush (void *handle)

+py_flush (void *handle, uint32_t flags)

 {

   PyObject *obj = handle;

   PyObject *fn;

@@ -551,7 +614,15 @@ py_flush (void *handle)

   if (callback_defined ("flush", &fn)) {

     PyErr_Clear ();

-    r = PyObject_CallFunctionObjArgs (fn, obj, NULL);

+    switch (py_api_version) {

+    case 1:

+      r = PyObject_CallFunctionObjArgs (fn, obj, NULL);

+      break;

+    case 2:

+      r = PyObject_CallFunction (fn, "OI", obj, flags);

+      break;

+    default: abort ();

+    }

     Py_DECREF (fn);

     if (check_python_failure ("flush") == -1)

       return -1;

@@ -566,7 +637,7 @@ py_flush (void *handle)

 }

 static int

-py_trim (void *handle, uint32_t count, uint64_t offset)

+py_trim (void *handle, uint32_t count, uint64_t offset, uint32_t flags)

 {

   PyObject *obj = handle;

   PyObject *fn;

@@ -575,7 +646,15 @@ py_trim (void *handle, uint32_t count, uint64_t offset)

   if (callback_defined ("trim", &fn)) {

     PyErr_Clear ();

-    r = PyObject_CallFunction (fn, "OiL", obj, count, offset);

+    switch (py_api_version) {

+    case 1:

+      r = PyObject_CallFunction (fn, "OiL", obj, count, offset);

+      break;

+    case 2:

+      r = PyObject_CallFunction (fn, "OiLI", obj, count, offset, flags);

+      break;

+    default: abort ();

+    }

     Py_DECREF (fn);

     if (check_python_failure ("trim") == -1)

       return -1;

@@ -590,7 +669,7 @@ py_trim (void *handle, uint32_t count, uint64_t offset)

 }

 static int

-py_zero (void *handle, uint32_t count, uint64_t offset, int may_trim)

+py_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags)

 {

   PyObject *obj = handle;

   PyObject *fn;

@@ -600,9 +679,19 @@ py_zero (void *handle, uint32_t count, uint64_t offset, int may_trim)

     PyErr_Clear ();

     last_error = 0;

-    r = PyObject_CallFunction (fn, "OiLO",

-                               obj, count, offset,

-                               may_trim ? Py_True : Py_False);

+    switch (py_api_version) {

+    case 1: {

+      int may_trim = flags & NBDKIT_FLAG_MAY_TRIM;

+      r = PyObject_CallFunction (fn, "OiLO",

+                                 obj, count, offset,

+                                 may_trim ? Py_True : Py_False);

+      break;

+    }

+    case 2:

+      r = PyObject_CallFunction (fn, "OiLI", obj, count, offset, flags);

+      break;

+    default: abort ();

+    }

     Py_DECREF (fn);

     if (last_error == EOPNOTSUPP || last_error == ENOTSUP) {

       /* When user requests this particular error, we want to

diff --git a/tests/python-exception.py b/tests/python-exception.py

index d0c79bb0..ee4a3f3a 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 6f336230..0634589a 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]

diff --git a/tests/test.py b/tests/test.py

index 9a2e947d..4db56623 100644

--- a/tests/test.py

+++ b/tests/test.py

@@ -3,6 +3,9 @@ import nbdkit

 disk = bytearray(1024*1024)

+API_VERSION = 2

+

+

 def config_complete():

     print ("set_error = %r" % nbdkit.set_error)

@@ -32,25 +35,26 @@ def can_trim(h):

     return True

-def pread(h, count, offset):

+def pread(h, buf, offset, flags):

     global disk

-    return disk[offset:offset+count]

+    end = offset + len(buf)

+    buf[:] = disk[offset:end]

-def pwrite(h, buf, offset):

+def pwrite(h, buf, offset, flags):

     global disk

     end = offset + len(buf)

     disk[offset:end] = buf

-def zero(h, count, offset, may_trim=False):

+def flush(h, flags):

+    pass

+

+

+def trim(h, count, offset, flags):

+    pass

+

+

+def zero(h, count, offset, flags):

     global disk

     disk[offset:offset+count] = bytearray(count)

-

-

-def flush(h):

-    pass

-

-

-def trim(h, count, offset):

-    pass

-- 

2.18.2