[Libguestfs] [nbdkit PATCH v2 4/5] python: Expose FUA support

Eric Blake
2018-04-11T05:03:41Z

Expose support for the FUA flag to pwrite, zero, and trim, as
well as a can_fua callback, for use in python plugins.  There
are some slight semantic differences: the C plugin had to
switch to a new API with a single uint32_t flags argument (so
we don't have to keep adding new ABI when new flags are added),
but in so doing, there is no way to probe whether a C plugin
supports FUA flags, so the C .can_fua has to return a tri-state.
But for python plugins, thanks to the fact that python functions
have named and optional parameters, it's a lot cleaner to
provide new flags as a bool option apiece, instead of requiring
the python program to parse out bits from an int, and easy to
introspect if FUA support is present, so can_fua only has to
return a bool on whether to advertise FUA support to the
client.

One other thing that makes for some interesting C glue code:
if at least one of the three callbacks supports FUA, then the
C can_fua function claims native support, but in so doing,
the python plugin must gracefully handle the fallback to flush
for the remaining callbacks that did not support FUA, since
nbdkit plugins.c won't do it.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 plugins/python/nbdkit-python-plugin.pod |  45 +++++++++-
 plugins/python/python.c                 | 153 +++++++++++++++++++++++++++++---
 plugins/python/example.py               |   6 +-
 3 files changed, 189 insertions(+), 15 deletions(-)

diff --git a/plugins/python/nbdkit-python-plugin.pod
b/plugins/python/nbdkit-python-plugin.pod
index 29250ce..a480a25 100644
--- a/plugins/python/nbdkit-python-plugin.pod
+++ b/plugins/python/nbdkit-python-plugin.pod
@@ -190,6 +190,22 @@ contents will be garbage collected.
  def can_zero(h):
    # return a boolean

+=item C<can_fua>
+
+(Optional)
+
+ def can_fua(h):
+   # return a boolean
+
+Unlike the C counterpart, the Python callback does not need a
+tri-state return value, because Python introspection is sufficient to
+learn whether callbacks support FUA.  Thus, this function only returns
+a bool, which merely controls whether Forced Unit Access (FUA) support
+is advertised to the client on a per-connection basis.  If omitted or
+this returns true, FUA support is advertised as native if any of the
+C<pwrite>, C<zero>, or C<trim> callbacks support an optional
parameter
+C<fua>; or as emulated if there is a C<flush> callback.
+
 =item C<pread>

 (Required)
@@ -210,7 +226,7 @@ C<nbdkit.set_error> first.

 (Optional)

- def pwrite(h, buf, offset):
+ def pwrite(h, buf, offset, fua=False):
    length = len (buf)
    # no return value

@@ -218,6 +234,12 @@ 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>.

+Your function may support an optional C<fua> parameter; if it is
+present and the caller sets it to True, then your callback must not
+return success until the write has landed in persistent storage.  If
+it is absent but C<can_fua> returned True, then nbdkit emulates a
+client request for FUA by calling C<flush>.
+
 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,
@@ -233,6 +255,11 @@ fails or is partial, your function should throw an
exception,
 The body of your C<flush> function should do a L<sync(2)> or
 L<fdatasync(2)> or equivalent on the backing store.

+This function will not be called directly by the client if
+C<can_flush> returned false; however, it may still be called by nbdkit
+if C<can_fua> returned True but C<pwrite>, C<zero>, or
C<trim> lacked
+a C<fua> parameter.
+
 If the flush fails, your function should throw an exception, optionally
 using C<nbdkit.set_error> first.

@@ -240,18 +267,24 @@ using C<nbdkit.set_error> first.

 (Optional)

- def trim(h, count, offset):
+ def trim(h, count, offset, fua=False):
    # 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.

+Your function may support an optional C<fua> parameter; if it is
+present and the caller sets it to True, then your callback must not
+return success until the trim has landed in persistent storage.  If it
+is absent but C<can_fua> returned True, then nbdkit emulates a client
+request for FUA by calling C<flush>.
+
 =item C<zero>

 (Optional)

- def zero(h, count, offset, may_trim=False):
+ def zero(h, count, offset, may_trim=False, may_fua=False):
    # no return value

 The body of your C<zero> function should ensure that C<count> bytes
@@ -263,6 +296,12 @@ optimize the operation by using a trim, as long as
subsequent reads
 see zeroes.  If you omit the optional parameter, or if the caller sets
 it to False, writing zeroes should not punch holes.

+Your function may support an optional C<fua> parameter; if it is
+present and the caller sets it to True, then your callback must not
+return success until the write has landed in persistent storage.  If
+it is absent but C<can_fua> returned True, then nbdkit emulates a
+client request for FUA by calling C<flush>.
+
 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,
diff --git a/plugins/python/python.c b/plugins/python/python.c
index a50bf85..ad79c80 100644
--- a/plugins/python/python.c
+++ b/plugins/python/python.c
@@ -64,6 +64,9 @@ static const char *script;
 static PyObject *module;

 static int last_error;
+static int pwrite_has_fua;
+static int zero_has_fua;
+static int trim_has_fua;

 static PyObject *
 set_error (PyObject *self, PyObject *args)
@@ -320,6 +323,32 @@ py_config (const char *key, const char *value)
       nbdkit_error ("%s: one of the required callbacks 'open',
'get_size' or 'pread' is not defined by this Python script.
nbdkit requires these callbacks.", script);
       return -1;
     }
+
+    /* One-time setup to learn which callbacks have a fua parameter */
+    if (callback_defined ("pwrite", &fn)) {
+      pwrite_has_fua = callback_has_parameter (fn, "fua");
+      Py_DECREF (fn);
+      if (pwrite_has_fua < 0) {
+        check_python_failure ("config");
+        return -1;
+      }
+    }
+    if (callback_defined ("zero", &fn)) {
+      zero_has_fua = callback_has_parameter (fn, "fua");
+      Py_DECREF (fn);
+      if (zero_has_fua < 0) {
+        check_python_failure ("config");
+        return -1;
+      }
+    }
+    if (callback_defined ("trim", &fn)) {
+      trim_has_fua = callback_has_parameter (fn, "fua");
+      Py_DECREF (fn);
+      if (trim_has_fua < 0) {
+        check_python_failure ("config");
+        return -1;
+      }
+    }
   }
   else if (callback_defined ("config", &fn)) {
     /* Other parameters are passed to the Python .config callback. */
@@ -469,22 +498,52 @@ py_pread (void *handle, void *buf,
   return 0;
 }

+static int py_flush (void *handle, uint32_t flags);
+
 static int
 py_pwrite (void *handle, const void *buf,
            uint32_t count, uint64_t offset, uint32_t flags)
 {
   PyObject *obj = handle;
   PyObject *fn;
+  PyObject *args;
+  PyObject *kwargs;
   PyObject *r;
+  int fua = (flags & NBDKIT_FLAG_FUA) != 0;
+  int need_flush = fua && !pwrite_has_fua;

-  assert (!flags);
+  assert (!(flags & ~NBDKIT_FLAG_FUA));
   if (callback_defined ("pwrite", &fn)) {
     PyErr_Clear ();

-    r = PyObject_CallFunction (fn, "ONL", obj,
-                               PyByteArray_FromStringAndSize (buf, count),
-                               offset, NULL);
+    args = Py_BuildValue ("ONL", obj,
+                          PyByteArray_FromStringAndSize (buf, count),
+                          offset);
+    if (!args) {
+      check_python_failure ("pwrite");
+      Py_DECREF (fn);
+      return -1;
+    }
+    kwargs = PyDict_New ();
+    if (!kwargs) {
+      check_python_failure ("pwrite");
+      Py_DECREF (args);
+      Py_DECREF (fn);
+      return -1;
+    }
+    if (pwrite_has_fua &&
+        PyDict_SetItemString (kwargs, "fua",
+                              fua ? Py_True : Py_False) == -1) {
+      check_python_failure ("pwrite");
+      Py_DECREF (kwargs);
+      Py_DECREF (args);
+      Py_DECREF (fn);
+      return -1;
+    }
+    r = PyObject_Call (fn, args, kwargs);
     Py_DECREF (fn);
+    Py_DECREF (args);
+    Py_DECREF (kwargs);
     if (check_python_failure ("pwrite") == -1)
       return -1;
     Py_DECREF (r);
@@ -494,7 +553,7 @@ py_pwrite (void *handle, const void *buf,
     return -1;
   }

-  return 0;
+  return need_flush ? py_flush (handle, 0) : 0;
 }

 static int
@@ -527,14 +586,42 @@ py_trim (void *handle, uint32_t count, uint64_t offset,
uint32_t flags)
 {
   PyObject *obj = handle;
   PyObject *fn;
+  PyObject *args;
+  PyObject *kwargs;
   PyObject *r;
+  int fua = (flags & NBDKIT_FLAG_FUA) != 0;
+  int need_flush = fua && !trim_has_fua;

-  assert (!flags);
+  assert (!(flags & ~NBDKIT_FLAG_FUA));
   if (callback_defined ("trim", &fn)) {
     PyErr_Clear ();

-    r = PyObject_CallFunction (fn, "OiL", obj, count, offset, NULL);
+    args = Py_BuildValue ("OiL", obj, count, offset);
+    if (!args) {
+      check_python_failure ("trim");
+      Py_DECREF (fn);
+      return -1;
+    }
+    kwargs = PyDict_New ();
+    if (!kwargs) {
+      check_python_failure ("trim");
+      Py_DECREF (args);
+      Py_DECREF (fn);
+      return -1;
+    }
+    if (trim_has_fua &&
+        PyDict_SetItemString (kwargs, "fua",
+                              fua ? Py_True : Py_False) == -1) {
+      check_python_failure ("trim");
+      Py_DECREF (kwargs);
+      Py_DECREF (args);
+      Py_DECREF (fn);
+      return -1;
+    }
+    r = PyObject_Call (fn, args, kwargs);
     Py_DECREF (fn);
+    Py_DECREF (args);
+    Py_DECREF (kwargs);
     if (check_python_failure ("trim") == -1)
       return -1;
     Py_DECREF (r);
@@ -544,7 +631,7 @@ py_trim (void *handle, uint32_t count, uint64_t offset,
uint32_t flags)
     return -1;
   }

-  return 0;
+  return need_flush ? py_flush (handle, 0) : 0;
 }

 static int
@@ -556,8 +643,10 @@ py_zero (void *handle, uint32_t count, uint64_t offset,
uint32_t flags)
   PyObject *kwargs;
   PyObject *r;
   int may_trim = (flags & NBDKIT_FLAG_MAY_TRIM) != 0;
+  int fua = (flags & NBDKIT_FLAG_FUA) != 0;
+  int need_flush = fua && !zero_has_fua;

-  assert (!(flags & ~NBDKIT_FLAG_MAY_TRIM));
+  assert (!(flags & ~(NBDKIT_FLAG_MAY_TRIM | NBDKIT_FLAG_FUA)));
   if (callback_defined ("zero", &fn)) {
     static int zero_may_trim = -1;

@@ -593,6 +682,15 @@ py_zero (void *handle, uint32_t count, uint64_t offset,
uint32_t flags)
       Py_DECREF (fn);
       return -1;
     }
+    if (zero_has_fua &&
+        PyDict_SetItemString (kwargs, "fua",
+                              fua ? Py_True : Py_False) == -1) {
+      check_python_failure ("zero");
+      Py_DECREF (kwargs);
+      Py_DECREF (args);
+      Py_DECREF (fn);
+      return -1;
+    }
     r = PyObject_Call (fn, args, kwargs);
     Py_DECREF (fn);
     Py_DECREF (args);
@@ -609,7 +707,7 @@ py_zero (void *handle, uint32_t count, uint64_t offset,
uint32_t flags)
     if (check_python_failure ("zero") == -1)
       return -1;
     Py_DECREF (r);
-    return 0;
+    return need_flush ? py_flush (handle, 0) : 0;
   }

   nbdkit_debug ("zero missing, falling back to pwrite");
@@ -732,6 +830,40 @@ py_can_zero (void *handle)
   return callback_defined ("zero", NULL);
 }

+static int
+py_can_fua (void *handle)
+{
+  PyObject *obj = handle;
+  PyObject *fn;
+  PyObject *r;
+  int fua;
+
+  /* We have to convert a python bool into the nbdkit tristate. */
+  if (callback_defined ("can_fua", &fn)) {
+    PyErr_Clear ();
+
+    r = PyObject_CallFunctionObjArgs (fn, obj, NULL);
+    Py_DECREF (fn);
+    if (check_python_failure ("can_fua") == -1)
+      return -1;
+    if (r == Py_False)
+      fua = NBDKIT_FUA_NONE;
+    else if (pwrite_has_fua || zero_has_fua || trim_has_fua)
+      fua = NBDKIT_FUA_NATIVE;
+    else
+      fua = NBDKIT_FUA_EMULATE;
+    Py_DECREF (r);
+  }
+  else if (pwrite_has_fua || zero_has_fua || trim_has_fua)
+    fua = NBDKIT_FUA_NATIVE;
+  else if (callback_defined ("flush", NULL))
+    fua = NBDKIT_FUA_EMULATE;
+  else
+    fua = NBDKIT_FUA_NONE;
+
+  return fua;
+}
+
 #define py_config_help \
   "script=<FILENAME>     (required) The Python plugin to
run.\n" \
   "[other arguments may be used by the plugin that you load]"
@@ -759,6 +891,7 @@ static struct nbdkit_plugin plugin = {
   .is_rotational     = py_is_rotational,
   .can_trim          = py_can_trim,
   .can_zero          = py_can_zero,
+  .can_fua           = py_can_fua,

   .pread             = py_pread,
   .pwrite            = py_pwrite,
diff --git a/plugins/python/example.py b/plugins/python/example.py
index 9205f10..ca7c266 100644
--- a/plugins/python/example.py
+++ b/plugins/python/example.py
@@ -59,13 +59,15 @@ def pread(h, count, offset):
     return disk[offset:offset+count]


-def pwrite(h, buf, offset):
+# Since everything is stored in memory, flush is a no-op; this example
+# does not provide a flush(), but honors the fua flag by doing nothing.
+def pwrite(h, buf, offset, fua=False):
     global disk
     end = offset + len(buf)
     disk[offset:end] = buf


-def zero(h, count, offset, may_trim=False):
+def zero(h, count, offset, may_trim=False, fua=False):
     global disk
     if may_trim:
         disk[offset:offset+count] = bytearray(count)
--
2.14.3