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

Eric Blake eblake at redhat.com
Mon Nov 25 22:48:37 UTC 2019


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 at redhat.com>
---
 plugins/python/nbdkit-python-plugin.pod |  46 +++++++-
 plugins/python/python.c                 | 148 +++++++++++++++++++++---
 plugins/python/example.py               |   6 +-
 3 files changed, 179 insertions(+), 21 deletions(-)

diff --git a/plugins/python/nbdkit-python-plugin.pod b/plugins/python/nbdkit-python-plugin.pod
index ec126f5a..6940f4d9 100644
--- a/plugins/python/nbdkit-python-plugin.pod
+++ b/plugins/python/nbdkit-python-plugin.pod
@@ -215,6 +215,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)
@@ -239,7 +255,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

@@ -247,6 +263,12 @@ 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>.

+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,
@@ -262,6 +284,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.

@@ -269,18 +296,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, fua=False):
    # no return value

 The body of your C<zero> function should ensure that C<count> bytes
@@ -292,6 +325,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,
@@ -322,7 +361,6 @@ C<longname>,
 C<description>,
 C<config_help>,
 C<magic_config_key>,
-C<can_fua>,
 C<can_cache>,
 C<can_fast_zero>,
 C<can_extents>,
diff --git a/plugins/python/python.c b/plugins/python/python.c
index 1954881c..677420bd 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)
@@ -368,6 +371,32 @@ py_config (const char *key, const char *value)
                     "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. */
@@ -524,22 +553,52 @@ out:
   return ret;
 }

+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,
+    args = Py_BuildValue ("ONL", obj,
             PyMemoryView_FromMemory ((char *)buf, count, PyBUF_READ),
             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);
@@ -549,7 +608,7 @@ py_pwrite (void *handle, const void *buf,
     return -1;
   }

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

 static int
@@ -582,14 +641,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);
+    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);
@@ -599,7 +686,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
@@ -611,8 +698,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;

@@ -648,6 +737,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);
@@ -665,7 +763,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");
@@ -674,7 +772,8 @@ py_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
 }

 static int
-boolean_callback (void *handle, const char *can_fn, const char *plain_fn)
+boolean_callback (void *handle, const char *can_fn, const char *plain_fn,
+                  int missing)
 {
   PyObject *obj = handle;
   PyObject *fn;
@@ -699,37 +798,55 @@ boolean_callback (void *handle, const char *can_fn, const char *plain_fn)
   else if (plain_fn && callback_defined (plain_fn, NULL))
     return 1;
   else
-    return 0;
+    return missing;
 }

 static int
 py_is_rotational (void *handle)
 {
-  return boolean_callback (handle, "is_rotational", NULL);
+  return boolean_callback (handle, "is_rotational", NULL, 0);
 }

 static int
 py_can_write (void *handle)
 {
-  return boolean_callback (handle, "can_write", "pwrite");
+  return boolean_callback (handle, "can_write", "pwrite", 0);
 }

 static int
 py_can_flush (void *handle)
 {
-  return boolean_callback (handle, "can_flush", "flush");
+  return boolean_callback (handle, "can_flush", "flush", 0);
 }

 static int
 py_can_trim (void *handle)
 {
-  return boolean_callback (handle, "can_trim", "trim");
+  return boolean_callback (handle, "can_trim", "trim", 0);
 }

 static int
 py_can_zero (void *handle)
 {
-  return boolean_callback (handle, "can_zero", "zero");
+  return boolean_callback (handle, "can_zero", "zero", 0);
+}
+
+static int
+py_can_fua (void *handle)
+{
+  /* We have to convert a python bool into the nbdkit tristate. */
+  switch (boolean_callback (handle, "can_fua", NULL, 1)) {
+  case -1:
+    return -1;
+  case 0:
+    return NBDKIT_FUA_NONE;
+  default:
+    if (pwrite_has_fua || zero_has_fua || trim_has_fua)
+      return NBDKIT_FUA_NATIVE;
+    if (callback_defined ("flush", NULL))
+      return NBDKIT_FUA_EMULATE;
+    return NBDKIT_FUA_NONE;
+  }
 }

 #define py_config_help \
@@ -759,6 +876,7 @@ static struct nbdkit_plugin plugin = {
   .can_flush         = py_can_flush,
   .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 9205f107..ca7c2661 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.21.0




More information about the Libguestfs mailing list