thr3ads.net - Libguestfs - [Libguestfs] [PATCH nbdkit 1/8] python: Use PyObject_CallFunction instead of constructing the tuple. [Nov 2019]

If this information is useful, please help other people find it:
Share via:

Richard W.M. Jones

2019-Nov-21 16:58 UTC

[Libguestfs] [PATCH nbdkit 0/8] Implement nbdkit API v2 for Python plugins.

And fill out most of the missing bits of the API.

Rich.

Richard W.M. Jones

2019-Nov-21 16:58 UTC

head link

[Libguestfs] [PATCH nbdkit 1/8] python: Use PyObject_CallFunction instead of constructing the tuple.

It is unclear why we were constructing this by hand, but using the
following tip we can use PyObject_CallFunction:
https://stackoverflow.com/a/21221335
---
 plugins/python/python.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/plugins/python/python.c b/plugins/python/python.c
index 148097f..d65ac45 100644
--- a/plugins/python/python.c
+++ b/plugins/python/python.c
@@ -557,26 +557,21 @@ py_zero (void *handle, uint32_t count, uint64_t offset,
int may_trim)
 {
   PyObject *obj = handle;
   PyObject *fn;
-  PyObject *args;
   PyObject *r;
 
   if (callback_defined ("zero", &fn)) {
     PyErr_Clear ();
 
     last_error = 0;
-    args = PyTuple_New (4);
-    Py_INCREF (obj); /* decremented by Py_DECREF (args) */
-    PyTuple_SetItem (args, 0, obj);
-    PyTuple_SetItem (args, 1, PyLong_FromUnsignedLongLong (count));
-    PyTuple_SetItem (args, 2, PyLong_FromUnsignedLongLong (offset));
-    PyTuple_SetItem (args, 3, PyBool_FromLong (may_trim));
-    r = PyObject_CallObject (fn, args);
+    r = PyObject_CallFunction (fn, "OiLO",
+                               obj, count, offset,
+                               may_trim ? Py_True : Py_False, NULL);
     Py_DECREF (fn);
-    Py_DECREF (args);
     if (last_error == EOPNOTSUPP || last_error == ENOTSUP) {
       /* When user requests this particular error, we want to
-         gracefully fall back, and to accomodate both a normal return
-         and an exception. */
+       * gracefully fall back, and to accomodate both a normal return
+       * and an exception.
+       */
       nbdkit_debug ("zero requested falling back to pwrite");
       Py_XDECREF (r);
       PyErr_Clear ();
-- 
2.23.0

Richard W.M. Jones

2019-Nov-21 16:58 UTC

head link

[Libguestfs] [PATCH nbdkit 2/8] python: Add various constants to the API.

These are accessible from the plugin by:

  import nbdkit

  if flags & nbdkit.FLAG_MAY_TRIM:
  &c.

Many (all?) of these are not yet useful for plugins, some will never
be useful, but they only consume a tiny bit of memory and it's nice to
have the complete set available for future use.
---
 plugins/python/python.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/plugins/python/python.c b/plugins/python/python.c
index d65ac45..69cf4e9 100644
--- a/plugins/python/python.c
+++ b/plugins/python/python.c
@@ -231,6 +231,33 @@ create_nbdkit_module (void)
     nbdkit_error ("could not create the nbdkit API module");
     exit (EXIT_FAILURE);
   }
+
+  /* Constants corresponding to various flags. */
+  PyModule_AddIntConstant (m, "THREAD_MODEL_SERIALIZE_CONNECTIONS",
+                           NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS);
+  PyModule_AddIntConstant (m, "THREAD_MODEL_SERIALIZE_ALL_REQUESTS",
+                           NBDKIT_THREAD_MODEL_SERIALIZE_ALL_REQUESTS);
+  PyModule_AddIntConstant (m, "THREAD_MODEL_SERIALIZE_REQUESTS",
+                           NBDKIT_THREAD_MODEL_SERIALIZE_REQUESTS);
+  PyModule_AddIntConstant (m, "THREAD_MODEL_PARALLEL",
+                           NBDKIT_THREAD_MODEL_PARALLEL);
+
+  PyModule_AddIntConstant (m, "FLAG_MAY_TRIM", 
NBDKIT_FLAG_MAY_TRIM);
+  PyModule_AddIntConstant (m, "FLAG_FUA",       NBDKIT_FLAG_FUA);
+  PyModule_AddIntConstant (m, "FLAG_REQ_ONE",   NBDKIT_FLAG_REQ_ONE);
+  PyModule_AddIntConstant (m, "FLAG_FAST_ZERO",
NBDKIT_FLAG_FAST_ZERO);
+
+  PyModule_AddIntConstant (m, "FUA_NONE",       NBDKIT_FUA_NONE);
+  PyModule_AddIntConstant (m, "FUA_EMULATE",    NBDKIT_FUA_EMULATE);
+  PyModule_AddIntConstant (m, "FUA_NATIVE",     NBDKIT_FUA_NATIVE);
+
+  PyModule_AddIntConstant (m, "CACHE_NONE",     NBDKIT_CACHE_NONE);
+  PyModule_AddIntConstant (m, "CACHE_EMULATE", 
NBDKIT_CACHE_EMULATE);
+  PyModule_AddIntConstant (m, "CACHE_NATIVE",   NBDKIT_CACHE_NATIVE);
+
+  PyModule_AddIntConstant (m, "EXTENT_HOLE",    NBDKIT_EXTENT_HOLE);
+  PyModule_AddIntConstant (m, "EXTENT_ZERO",    NBDKIT_EXTENT_ZERO);
+
   return m;
 }
 
-- 
2.23.0

Richard W.M. Jones

2019-Nov-21 16:58 UTC

head link

[Libguestfs] [PATCH nbdkit 3/8] 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:

  def api_version():
    return 2

(Plugins which do not do this are assumed to want API version 1).
---
 plugins/python/example.py               |  15 +++-
 plugins/python/nbdkit-python-plugin.pod |  61 ++++++++++-----
 plugins/python/python.c                 | 100 ++++++++++++++++++++----
 tests/test.py                           |  20 +++--
 4 files changed, 148 insertions(+), 48 deletions(-)

diff --git a/plugins/python/example.py b/plugins/python/example.py
index 60f9d7f..25a0049 100644
--- a/plugins/python/example.py
+++ b/plugins/python/example.py
@@ -34,6 +34,13 @@ 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.
+def api_version():
+    return 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 +61,20 @@ def get_size(h):
     return len(disk)
 
 
-def pread(h, count, offset):
+def pread(h, count, offset, flags):
     global disk
     return disk[offset:offset+count]
 
 
-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 6453474..882c0d8 100644
--- a/plugins/python/nbdkit-python-plugin.pod
+++ b/plugins/python/nbdkit-python-plugin.pod
@@ -82,6 +82,19 @@ 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 function in your
+module:
+
+ def api_version():
+   return 2
+
+(where 2 is the latest version at the time this documentation was
+written).  All newly written Python modules must have this function.
+
 =head2 Executable script
 
 If you want you can make the script executable and include a
"shebang"
@@ -120,6 +133,10 @@ nbdkit-plugin(3).
 
 =over 4
 
+=item C<api_version>
+
+There are no arguments.  It must return C<2>.
+
 =item C<dump_plugin>
 
 (Optional)
@@ -199,12 +216,12 @@ contents will be garbage collected.
 
 (Required)
 
- def pread(h, count, offset):
+ def pread(h, count, offset, flags):
    # construct a bytearray of length count bytes and return it
 
 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>.
+disk starting at C<offset>.  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
@@ -215,13 +232,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 C<buf> string
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
@@ -232,11 +249,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.
@@ -245,32 +263,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 69cf4e9..20e05e0 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;
 
@@ -356,6 +359,26 @@ py_config (const char *key, const char *value)
                     "nbdkit requires these callbacks.", script);
       return -1;
     }
+
+    /* Get the API version. */
+    if (callback_defined ("api_version", &fn)) {
+      PyErr_Clear ();
+
+      r = PyObject_CallObject (fn, NULL);
+      Py_DECREF (fn);
+      if (check_python_failure ("api_version") == -1)
+        return -1;
+      py_api_version = (int) PyLong_AsLong (r);
+      Py_DECREF (r);
+      if (check_python_failure ("PyLong_AsLong") == -1)
+        return -1;
+      if (py_api_version < 1 || py_api_version > NBDKIT_API_VERSION) {
+        nbdkit_error ("%s: api_version() requested unknown version: %d. 
"
+                      "This plugin supports API versions between 1 and
%d.",
+                      script, py_api_version, NBDKIT_API_VERSION);
+        return -1;
+      }
+    }
   }
   else if (callback_defined ("config", &fn)) {
     /* Other parameters are passed to the Python .config callback. */
@@ -466,8 +489,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;
@@ -480,7 +503,15 @@ py_pread (void *handle, void *buf,
 
   PyErr_Clear ();
 
-  r = PyObject_CallFunction (fn, "OiL", obj, count, offset, NULL);
+  switch (py_api_version) {
+  case 1:
+    r = PyObject_CallFunction (fn, "OiL", obj, count, offset, NULL);
+    break;
+  case 2:
+    r = PyObject_CallFunction (fn, "OiLI", obj, count, offset, flags,
NULL);
+    break;
+  default: abort ();
+  }
   Py_DECREF (fn);
   if (check_python_failure ("pread") == -1)
     return -1;
@@ -505,8 +536,8 @@ py_pread (void *handle, void *buf,
 }
 
 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;
@@ -515,9 +546,19 @@ py_pwrite (void *handle, const void *buf,
   if (callback_defined ("pwrite", &fn)) {
     PyErr_Clear ();
 
-    r = PyObject_CallFunction (fn, "ONL", obj,
-                               PyByteArray_FromStringAndSize (buf, count),
-                               offset, NULL);
+    switch (py_api_version) {
+    case 1:
+      r = PyObject_CallFunction (fn, "ONL", obj,
+                                 PyByteArray_FromStringAndSize (buf, count),
+                                 offset, NULL);
+      break;
+    case 2:
+      r = PyObject_CallFunction (fn, "ONLI", obj,
+                                 PyByteArray_FromStringAndSize (buf, count),
+                                 offset, flags, NULL);
+      break;
+    default: abort ();
+    }
     Py_DECREF (fn);
     if (check_python_failure ("pwrite") == -1)
       return -1;
@@ -532,7 +573,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;
@@ -541,7 +582,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, NULL);
+      break;
+    default: abort ();
+    }
     Py_DECREF (fn);
     if (check_python_failure ("flush") == -1)
       return -1;
@@ -556,7 +605,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;
@@ -565,7 +614,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, NULL);
+    switch (py_api_version) {
+    case 1:
+      r = PyObject_CallFunction (fn, "OiL", obj, count, offset,
NULL);
+      break;
+    case 2:
+      r = PyObject_CallFunction (fn, "OiLI", obj, count, offset,
flags, NULL);
+      break;
+    default: abort ();
+    }
     Py_DECREF (fn);
     if (check_python_failure ("trim") == -1)
       return -1;
@@ -580,7 +637,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;
@@ -590,9 +647,20 @@ 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, NULL);
+    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, NULL);
+      break;
+    }
+    case 2:
+      r = PyObject_CallFunction (fn, "OiLI",
+                                 obj, count, offset, flags, NULL);
+      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/test.py b/tests/test.py
index 9a2e947..ac80d96 100644
--- a/tests/test.py
+++ b/tests/test.py
@@ -3,6 +3,10 @@ import nbdkit
 disk = bytearray(1024*1024)
 
 
+def api_version():
+    return 2
+
+
 def config_complete():
     print ("set_error = %r" % nbdkit.set_error)
 
@@ -32,25 +36,25 @@ def can_trim(h):
     return True
 
 
-def pread(h, count, offset):
+def pread(h, count, offset, flags):
     global disk
     return disk[offset:offset+count]
 
 
-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):
-    global disk
-    disk[offset:offset+count] = bytearray(count)
+def flush(h, flags):
+    pass
 
 
-def flush(h):
+def trim(h, count, offset, flags):
     pass
 
 
-def trim(h, count, offset):
-    pass
+def zero(h, count, offset, flags):
+    global disk
+    disk[offset:offset+count] = bytearray(count)
-- 
2.23.0

Richard W.M. Jones

2019-Nov-21 16:58 UTC

head link

[Libguestfs] [PATCH nbdkit 4/8] python: Document definitive list of the currently missing callbacks.

---
 plugins/python/nbdkit-python-plugin.pod | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/plugins/python/nbdkit-python-plugin.pod
b/plugins/python/nbdkit-python-plugin.pod
index 882c0d8..51e0f57 100644
--- a/plugins/python/nbdkit-python-plugin.pod
+++ b/plugins/python/nbdkit-python-plugin.pod
@@ -304,8 +304,25 @@ S<C<nbdkit.set_error (errno.EOPNOTSUPP)>>.
 These are not needed because you can just use ordinary Python
 constructs.
 
-=item Missing: C<name>, C<version>, C<longname>,
C<description>,
-C<config_help>, C<can_fua>, C<can_cache>, C<cache>
+=item Missing: C<thread_model>
+
+Probably not implementable while Python has a global interpreter lock.
+
+=item Missing:
+C<name>,
+C<version>,
+C<longname>,
+C<description>,
+C<config_help>,
+C<magic_config_key>,
+C<can_fua>,
+C<can_cache>,
+C<can_zero>,
+C<can_fast_zero>,
+C<can_extents>,
+C<can_multi_conn>,
+C<cache>,
+C<extents>.
 
 These are not yet supported.
 
-- 
2.23.0

Richard W.M. Jones

2019-Nov-21 16:58 UTC

head link

[Libguestfs] [PATCH nbdkit 5/8] python: Share common code in boolean callbacks.

This change is pure refactoring and should have no effect.
---
 plugins/python/nbdkit-python-plugin.pod | 12 ++--
 plugins/python/python.c                 | 90 +++++--------------------
 2 files changed, 24 insertions(+), 78 deletions(-)

diff --git a/plugins/python/nbdkit-python-plugin.pod
b/plugins/python/nbdkit-python-plugin.pod
index 51e0f57..0fd4dcb 100644
--- a/plugins/python/nbdkit-python-plugin.pod
+++ b/plugins/python/nbdkit-python-plugin.pod
@@ -184,25 +184,25 @@ contents will be garbage collected.
  def get_size(h):
    # return the size of the disk
 
-=item C<can_write>
+=item C<is_rotational>
 
 (Optional)
 
- def can_write(h):
+ def is_rotational(h):
    # return a boolean
 
-=item C<can_flush>
+=item C<can_write>
 
 (Optional)
 
- def can_flush(h):
+ def can_write(h):
    # return a boolean
 
-=item C<is_rotational>
+=item C<can_flush>
 
 (Optional)
 
- def is_rotational(h):
+ def can_flush(h):
    # return a boolean
 
 =item C<can_trim>
diff --git a/plugins/python/python.c b/plugins/python/python.c
index 20e05e0..9445343 100644
--- a/plugins/python/python.c
+++ b/plugins/python/python.c
@@ -684,110 +684,56 @@ py_zero (void *handle, uint32_t count, uint64_t offset,
uint32_t flags)
 }
 
 static int
-py_can_write (void *handle)
+boolean_callback (void *handle, const char *can_fn, const char *plain_fn)
 {
   PyObject *obj = handle;
   PyObject *fn;
   PyObject *r;
   int ret;
 
-  if (callback_defined ("can_write", &fn)) {
+  if (callback_defined (can_fn, &fn)) {
     PyErr_Clear ();
 
     r = PyObject_CallFunctionObjArgs (fn, obj, NULL);
     Py_DECREF (fn);
-    if (check_python_failure ("can_write") == -1)
+    if (check_python_failure (can_fn) == -1)
       return -1;
     ret = r == Py_True;
     Py_DECREF (r);
     return ret;
   }
-  /* No Python can_write callback, but there's a Python pwrite callback
-   * defined, so return 1.  (In C modules, nbdkit would do this).
+  /* No Python ‘can_fn’ (eg. ‘can_write’), but if there's a Python
+   * ‘plain_fn’ (eg. ‘pwrite’) callback defined, return 1.  (In C
+   * modules, nbdkit would do this).
    */
-  else if (callback_defined ("pwrite", NULL))
+  else if (plain_fn && callback_defined (plain_fn, NULL))
     return 1;
   else
     return 0;
 }
 
 static int
-py_can_flush (void *handle)
+py_is_rotational (void *handle)
 {
-  PyObject *obj = handle;
-  PyObject *fn;
-  PyObject *r;
-  int ret;
-
-  if (callback_defined ("can_flush", &fn)) {
-    PyErr_Clear ();
-
-    r = PyObject_CallFunctionObjArgs (fn, obj, NULL);
-    Py_DECREF (fn);
-    if (check_python_failure ("can_flush") == -1)
-      return -1;
-    ret = r == Py_True;
-    Py_DECREF (r);
-    return ret;
-  }
-  /* No Python can_flush callback, but there's a Python flush callback
-   * defined, so return 1.  (In C modules, nbdkit would do this).
-   */
-  else if (callback_defined ("flush", NULL))
-    return 1;
-  else
-    return 0;
+  return boolean_callback (handle, "is_rotational", NULL);
 }
 
 static int
-py_is_rotational (void *handle)
+py_can_write (void *handle)
 {
-  PyObject *obj = handle;
-  PyObject *fn;
-  PyObject *r;
-  int ret;
-
-  if (callback_defined ("is_rotational", &fn)) {
-    PyErr_Clear ();
+  return boolean_callback (handle, "can_write", "pwrite");
+}
 
-    r = PyObject_CallFunctionObjArgs (fn, obj, NULL);
-    Py_DECREF (fn);
-    if (check_python_failure ("is_rotational") == -1)
-      return -1;
-    ret = r == Py_True;
-    Py_DECREF (r);
-    return ret;
-  }
-  else
-    return 0;
+static int
+py_can_flush (void *handle)
+{
+  return boolean_callback (handle, "can_flush", "flush");
 }
 
 static int
 py_can_trim (void *handle)
 {
-  PyObject *obj = handle;
-  PyObject *fn;
-  PyObject *r;
-  int ret;
-
-  if (callback_defined ("can_trim", &fn)) {
-    PyErr_Clear ();
-
-    r = PyObject_CallFunctionObjArgs (fn, obj, NULL);
-    Py_DECREF (fn);
-    if (check_python_failure ("can_trim") == -1)
-      return -1;
-    ret = r == Py_True;
-    Py_DECREF (r);
-    return ret;
-  }
-  /* No Python can_trim callback, but there's a Python trim callback
-   * defined, so return 1.  (In C modules, nbdkit would do this).
-   */
-  else if (callback_defined ("trim", NULL))
-    return 1;
-  else
-    return 0;
+  return boolean_callback (handle, "can_trim", "trim");
 }
 
 #define py_config_help \
@@ -812,9 +758,9 @@ static struct nbdkit_plugin plugin = {
   .close             = py_close,
 
   .get_size          = py_get_size,
+  .is_rotational     = py_is_rotational,
   .can_write         = py_can_write,
   .can_flush         = py_can_flush,
-  .is_rotational     = py_is_rotational,
   .can_trim          = py_can_trim,
 
   .pread             = py_pread,
-- 
2.23.0

Richard W.M. Jones

2019-Nov-21 16:58 UTC

head link

[Libguestfs] [PATCH nbdkit 6/8] python: Implement cache, can_cache.

---
 plugins/python/nbdkit-python-plugin.pod | 22 +++++++++++--
 plugins/python/python.c                 | 41 +++++++++++++++++++++++++
 2 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/plugins/python/nbdkit-python-plugin.pod
b/plugins/python/nbdkit-python-plugin.pod
index 0fd4dcb..2bc4722 100644
--- a/plugins/python/nbdkit-python-plugin.pod
+++ b/plugins/python/nbdkit-python-plugin.pod
@@ -212,6 +212,13 @@ contents will be garbage collected.
  def can_trim(h):
    # return a boolean
 
+=item C<can_cache>
+
+(Optional)
+
+ def can_cache(h):
+   # return a boolean
+
 =item C<pread>
 
 (Required)
@@ -293,6 +300,19 @@ because there is nothing to optimize if
 S<C<flags & nbdkit.FLAG_MAY_TRIM>> is false), use
 S<C<nbdkit.set_error (errno.EOPNOTSUPP)>>.
 
+=item C<cache>
+
+(Optional)
+
+ def cache(h, count, offset, flags):
+   # no return value
+
+The body of your C<cache> function should prefetch data in the
+indicated range.
+
+If the cache operation fails, your function should throw an exception,
+optionally using C<nbdkit.set_error> first.
+
 =back
 
 =head2 Missing callbacks
@@ -316,12 +336,10 @@ C<description>,
 C<config_help>,
 C<magic_config_key>,
 C<can_fua>,
-C<can_cache>,
 C<can_zero>,
 C<can_fast_zero>,
 C<can_extents>,
 C<can_multi_conn>,
-C<cache>,
 C<extents>.
 
 These are not yet supported.
diff --git a/plugins/python/python.c b/plugins/python/python.c
index 9445343..33fb6e4 100644
--- a/plugins/python/python.c
+++ b/plugins/python/python.c
@@ -683,6 +683,39 @@ py_zero (void *handle, uint32_t count, uint64_t offset,
uint32_t flags)
   return -1;
 }
 
+static int
+py_cache (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
+{
+  PyObject *obj = handle;
+  PyObject *fn;
+  PyObject *r;
+
+  if (callback_defined ("cache", &fn)) {
+    PyErr_Clear ();
+
+    switch (py_api_version) {
+    case 1:
+      nbdkit_error ("%s can only be called when using api_version >=
2",
+                    "cache");
+      return -1;
+    case 2:
+      r = PyObject_CallFunction (fn, "OiLI", obj, count, offset,
flags, NULL);
+      break;
+    default: abort ();
+    }
+    Py_DECREF (fn);
+    if (check_python_failure ("cache") == -1)
+      return -1;
+    Py_DECREF (r);
+  }
+  else {
+    nbdkit_error ("%s not implemented", "cache");
+    return -1;
+  }
+
+  return 0;
+}
+
 static int
 boolean_callback (void *handle, const char *can_fn, const char *plain_fn)
 {
@@ -736,6 +769,12 @@ py_can_trim (void *handle)
   return boolean_callback (handle, "can_trim", "trim");
 }
 
+static int
+py_can_cache (void *handle)
+{
+  return boolean_callback (handle, "can_cache", "cache");
+}
+
 #define py_config_help \
   "script=<FILENAME>     (required) The Python plugin to
run.\n" \
   "[other arguments may be used by the plugin that you load]"
@@ -762,12 +801,14 @@ static struct nbdkit_plugin plugin = {
   .can_write         = py_can_write,
   .can_flush         = py_can_flush,
   .can_trim          = py_can_trim,
+  .can_cache         = py_can_cache,
 
   .pread             = py_pread,
   .pwrite            = py_pwrite,
   .flush             = py_flush,
   .trim              = py_trim,
   .zero              = py_zero,
+  .cache             = py_cache,
 };
 
 NBDKIT_REGISTER_PLUGIN (plugin)
-- 
2.23.0

Richard W.M. Jones

2019-Nov-21 16:58 UTC

head link

[Libguestfs] [PATCH nbdkit 7/8] python: Implement can_zero, can_fast_zero.

---
 plugins/python/nbdkit-python-plugin.pod | 16 ++++++++++++++--
 plugins/python/python.c                 | 14 ++++++++++++++
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/plugins/python/nbdkit-python-plugin.pod
b/plugins/python/nbdkit-python-plugin.pod
index 2bc4722..96a83ef 100644
--- a/plugins/python/nbdkit-python-plugin.pod
+++ b/plugins/python/nbdkit-python-plugin.pod
@@ -212,6 +212,20 @@ contents will be garbage collected.
  def can_trim(h):
    # return a boolean
 
+=item C<can_zero>
+
+(Optional)
+
+ def can_zero(h):
+   # return a boolean
+
+=item C<can_fast_zero>
+
+(Optional)
+
+ def can_fast_zero(h):
+   # return a boolean
+
 =item C<can_cache>
 
 (Optional)
@@ -336,8 +350,6 @@ C<description>,
 C<config_help>,
 C<magic_config_key>,
 C<can_fua>,
-C<can_zero>,
-C<can_fast_zero>,
 C<can_extents>,
 C<can_multi_conn>,
 C<extents>.
diff --git a/plugins/python/python.c b/plugins/python/python.c
index 33fb6e4..468b1ec 100644
--- a/plugins/python/python.c
+++ b/plugins/python/python.c
@@ -769,6 +769,18 @@ py_can_trim (void *handle)
   return boolean_callback (handle, "can_trim", "trim");
 }
 
+static int
+py_can_zero (void *handle)
+{
+  return boolean_callback (handle, "can_zero", "zero");
+}
+
+static int
+py_can_fast_zero (void *handle)
+{
+  return boolean_callback (handle, "can_fast_zero", NULL);
+}
+
 static int
 py_can_cache (void *handle)
 {
@@ -801,6 +813,8 @@ static struct nbdkit_plugin plugin = {
   .can_write         = py_can_write,
   .can_flush         = py_can_flush,
   .can_trim          = py_can_trim,
+  .can_zero          = py_can_zero,
+  .can_fast_zero     = py_can_fast_zero,
   .can_cache         = py_can_cache,
 
   .pread             = py_pread,
-- 
2.23.0

Richard W.M. Jones

2019-Nov-21 16:58 UTC

head link

[Libguestfs] [PATCH nbdkit 8/8] python: Implement can_multi_conn.

---
 plugins/python/nbdkit-python-plugin.pod | 8 +++++++-
 plugins/python/python.c                 | 7 +++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/plugins/python/nbdkit-python-plugin.pod
b/plugins/python/nbdkit-python-plugin.pod
index 96a83ef..ca1cc03 100644
--- a/plugins/python/nbdkit-python-plugin.pod
+++ b/plugins/python/nbdkit-python-plugin.pod
@@ -191,6 +191,13 @@ contents will be garbage collected.
  def is_rotational(h):
    # return a boolean
 
+=item C<can_multi_conn>
+
+(Optional)
+
+ def can_multi_conn(h):
+   # return a boolean
+
 =item C<can_write>
 
 (Optional)
@@ -351,7 +358,6 @@ C<config_help>,
 C<magic_config_key>,
 C<can_fua>,
 C<can_extents>,
-C<can_multi_conn>,
 C<extents>.
 
 These are not yet supported.
diff --git a/plugins/python/python.c b/plugins/python/python.c
index 468b1ec..ce9607f 100644
--- a/plugins/python/python.c
+++ b/plugins/python/python.c
@@ -751,6 +751,12 @@ py_is_rotational (void *handle)
   return boolean_callback (handle, "is_rotational", NULL);
 }
 
+static int
+py_can_multi_conn (void *handle)
+{
+  return boolean_callback (handle, "can_multi_conn", NULL);
+}
+
 static int
 py_can_write (void *handle)
 {
@@ -810,6 +816,7 @@ static struct nbdkit_plugin plugin = {
 
   .get_size          = py_get_size,
   .is_rotational     = py_is_rotational,
+  .can_multi_conn    = py_can_multi_conn,
   .can_write         = py_can_write,
   .can_flush         = py_can_flush,
   .can_trim          = py_can_trim,
-- 
2.23.0

Richard W.M. Jones

2019-Nov-22 19:43 UTC

head link

Re: [Libguestfs] [PATCH nbdkit 6/8] python: Implement cache, can_cache.

On Thu, Nov 21, 2019 at 04:58:12PM +0000, Richard W.M. Jones
wrote:> ---
>  plugins/python/nbdkit-python-plugin.pod | 22 +++++++++++--
>  plugins/python/python.c                 | 41 +++++++++++++++++++++++++
>  2 files changed, 61 insertions(+), 2 deletions(-)
> 
> diff --git a/plugins/python/nbdkit-python-plugin.pod
b/plugins/python/nbdkit-python-plugin.pod
> index 0fd4dcb..2bc4722 100644
> --- a/plugins/python/nbdkit-python-plugin.pod
> +++ b/plugins/python/nbdkit-python-plugin.pod
> @@ -212,6 +212,13 @@ contents will be garbage collected.
>   def can_trim(h):
>     # return a boolean
>  
> +=item C<can_cache>
> +
> +(Optional)
> +
> + def can_cache(h):
> +   # return a boolean
This is wrong because can_cache (like can_fua) returns NONE | EMULATE |
NATIVE.

I'm working on an updated patch series + test which fixes this.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

Eric Blake

2019-Nov-25 17:17 UTC

head link

Re: [Libguestfs] [PATCH nbdkit 1/8] python: Use PyObject_CallFunction instead of constructing the tuple.

On 11/21/19 10:58 AM, Richard W.M. Jones wrote:> It is unclear why we were constructing this by hand, but using the
> following tip we can use PyObject_CallFunction:
> https://stackoverflow.com/a/21221335
> ---
>   plugins/python/python.c | 17 ++++++-----------
>   1 file changed, 6 insertions(+), 11 deletions(-)
> 
> -    r = PyObject_CallObject (fn, args);
> +    r = PyObject_CallFunction (fn, "OiLO",
> +                               obj, count, offset,
> +                               may_trim ? Py_True : Py_False, NULL);
The use of trailing ', NULL' is appears to be copy-and-paste from my 
earlier commit 5ae45c00, where I used it on 3 of the 4 conversions made 
there.  It is not necessary, nor is it mentioned in the mentioned 
stackoverflow link.  If you have a format string, a trailing NULL is not 
necessary because the format string tells how many varargs to consume; 
but with PyObject_CallFunctionObjArgs, a trailing NULL is necessary 
because there is no format string and the varargs are parsed as a series 
of Python objects until a trailing NULL terminator is encountered.

I've pushed an obvious cleanup patch.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Reasonably Related Threads

Search for more possibly parallel threads

Libguestfs - Nov 2019 - [PATCH nbdkit 1/8] python: Use PyObject_CallFunction instead of constructing the tuple.

[Libguestfs] [PATCH nbdkit 0/8] Implement nbdkit API v2 for Python plugins.

[Libguestfs] [PATCH nbdkit 1/8] python: Use PyObject_CallFunction instead of constructing the tuple.

[Libguestfs] [PATCH nbdkit 2/8] python: Add various constants to the API.

[Libguestfs] [PATCH nbdkit 3/8] python: Implement nbdkit API version 2.

[Libguestfs] [PATCH nbdkit 4/8] python: Document definitive list of the currently missing callbacks.

[Libguestfs] [PATCH nbdkit 5/8] python: Share common code in boolean callbacks.

[Libguestfs] [PATCH nbdkit 6/8] python: Implement cache, can_cache.

[Libguestfs] [PATCH nbdkit 7/8] python: Implement can_zero, can_fast_zero.

[Libguestfs] [PATCH nbdkit 8/8] python: Implement can_multi_conn.

Re: [Libguestfs] [PATCH nbdkit 6/8] python: Implement cache, can_cache.

Re: [Libguestfs] [PATCH nbdkit 1/8] python: Use PyObject_CallFunction instead of constructing the tuple.

Reasonably Related Threads