[Libguestfs] [PATCH nbdkit v2 3/4] sh: Switch nbdkit-sh-plugin to use API version 2.

[Richard W.M. Jones]

This allows us to add the following callbacks:

 - can_zero     returns boolean
 - can_fua      should print "none", "emulate" or "native"

Furthermore the following callbacks are modified in a backwards
compatible manner:

 - pwrite       adding flags parameter
 - trim         adding flags parameter

This change is not backwards compatible:

 - zero         may_trim parameter replaced by flags parameter
---
 plugins/sh/nbdkit-sh-plugin.pod |  43 +++++++---
 plugins/sh/sh.c                 | 139 +++++++++++++++++++++++++++-----
 2 files changed, 154 insertions(+), 28 deletions(-)

diff --git a/plugins/sh/nbdkit-sh-plugin.pod b/plugins/sh/nbdkit-sh-plugin.pod
index 14fe7b8..4cea076 100644
--- a/plugins/sh/nbdkit-sh-plugin.pod
+++ b/plugins/sh/nbdkit-sh-plugin.pod
@@ -199,17 +199,21 @@ This method is required.
 
 =item C<can_trim>
 
+=item C<can_zero>
+
 Unlike in other languages, you B<must> provide the C<can_*> methods
 otherwise they are assumed to all return false and your C<pwrite>,
-C<flush> and C<trim> methods will never be called.  The reason for
-this is obscure: In other languages we can detect if (eg) a C<pwrite>
-method is defined and synthesize an appropriate response if no actual
-C<can_write> method is defined.  However detecting if methods are
-present without running them is not possible with this plugin.
+C<flush>, C<trim> and C<zero> methods will never be called.  The
+reason for this is obscure: In other languages we can detect if (eg) a
+C<pwrite> method is defined and synthesize an appropriate response if
+no actual C<can_write> method is defined.  However detecting if
+methods are present without running them is not possible with this
+plugin.
 
  /path/to/script can_write <handle>
  /path/to/script can_flush <handle>
  /path/to/script can_trim <handle>
+ /path/to/script can_zero <handle>
 
 The script should exit with code C<0> for true or code C<3> for false.
 
@@ -219,6 +223,16 @@ The script should exit with code C<0> for true or code C<3> for false.
 
 The script should exit with code C<0> for true or code C<3> for false.
 
+=item C<can_fua>
+
+ /path/to/script can_fua <handle>
+
+This controls Forced Unit Access (FUA) behaviour of the core server.
+
+Unlike the other C<can_*> callbacks, this one is I<not> a boolean.  It
+must print either "none", "emulate" or "native" to stdout.  The
+meaning of these is described in L<nbdkit-plugin(3)>.
+
 =item C<pread>
 
  /path/to/script pread <handle> <count> <offset>
@@ -230,10 +244,13 @@ This method is required.
 
 =item C<pwrite>
 
- /path/to/script pwrite <handle> <count> <offset>
+ /path/to/script pwrite <handle> <count> <offset> <flags>
 
 The script should read the binary data to be written from stdin.
 
+The C<flags> parameter can be an empty string or C<"fua">.  In the
+future, a comma-separated list of flags may be present.
+
 Unlike in other languages, if you provide a C<pwrite> method you
 B<must> also provide a C<can_write> method which exits with code C<0>
 (true).
@@ -248,16 +265,24 @@ B<must> also provide a C<can_flush> method which exits with code C<0>
 
 =item C<trim>
 
- /path/to/script trim <handle> <count> <offset>
+ /path/to/script trim <handle> <count> <offset> <flags>
+
+The C<flags> parameter can be an empty string or C<"fua">.  In the
+future, a comma-separated list of flags may be present.
 
 Unlike in other languages, if you provide a C<trim> method you B<must>
 also provide a C<can_trim> method which exits with code C<0> (true).
 
 =item C<zero>
 
- /path/to/script zero <handle> <count> <offset> <may_trim>
+ /path/to/script zero <handle> <count> <offset> <flags>
 
-C<may_trim> will be either C<true> or C<false>.
+The C<flags> parameter can be an empty string or a comma-separated
+list of the flags: C<"fua"> and C<"may_trim"> (eg. C<"">, C<"fua">,
+C<"fua,may_trim"> are all possible values).
+
+Unlike in other languages, if you provide a C<zero> method you B<must>
+also provide a C<can_zero> method which exits with code C<0> (true).
 
 =back
 
diff --git a/plugins/sh/sh.c b/plugins/sh/sh.c
index 4ca394f..159052e 100644
--- a/plugins/sh/sh.c
+++ b/plugins/sh/sh.c
@@ -35,12 +35,15 @@
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <stdbool.h>
 #include <inttypes.h>
 #include <string.h>
 #include <unistd.h>
 #include <errno.h>
 #include <sys/stat.h>
 
+#define NBDKIT_API_VERSION 2
+
 #include <nbdkit-plugin.h>
 
 #include "call.h"
@@ -360,7 +363,8 @@ sh_get_size (void *handle)
 }
 
 static int
-sh_pread (void *handle, void *buf, uint32_t count, uint64_t offset)
+sh_pread (void *handle, void *buf, uint32_t count, uint64_t offset,
+          uint32_t flags)
 {
   char *h = handle;
   char cbuf[32], obuf[32];
@@ -403,16 +407,53 @@ sh_pread (void *handle, void *buf, uint32_t count, uint64_t offset)
   }
 }
 
+/* Convert NBDKIT_FLAG_* to flags string. */
+static void flag_append (const char *str, bool *comma, char **buf, size_t *len);
+
+static void
+flags_string (uint32_t flags, char *buf, size_t len)
+{
+  bool comma = false;
+
+  if (flags & NBDKIT_FLAG_FUA)
+    flag_append ("fua", &comma, &buf, &len);
+
+  if (flags & NBDKIT_FLAG_MAY_TRIM)
+    flag_append ("may_trim", &comma, &buf, &len);
+}
+
+static void
+flag_append (const char *str, bool *comma, char **buf, size_t *len)
+{
+  size_t slen = strlen (str);
+
+  if (*comma) {
+    /* Too short flags buffer is an internal error so abort. */
+    if (*len <= 1) abort ();
+    strcpy (*buf, ",");
+    (*buf)++;
+    (*len)--;
+  }
+
+  if (*len <= slen) abort ();
+  strcpy (*buf, str);
+  (*buf) += slen;
+  (*len) -= slen;
+
+  *comma = true;
+}
+
 static int
-sh_pwrite (void *handle, const void *buf,
-                   uint32_t count, uint64_t offset)
+sh_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset,
+           uint32_t flags)
 {
   char *h = handle;
-  char cbuf[32], obuf[32];
-  const char *args[] = { script, "pwrite", h, cbuf, obuf, NULL };
+  char cbuf[32], obuf[32], fbuf[32];
+  const char *args[] = { script, "pwrite", h, cbuf, obuf, fbuf, NULL };
 
   snprintf (cbuf, sizeof cbuf, "%" PRIu32, count);
   snprintf (obuf, sizeof obuf, "%" PRIu64, offset);
+  flags_string (flags, fbuf, sizeof fbuf);
 
   switch (call_write (buf, count, args)) {
   case OK:
@@ -467,12 +508,6 @@ sh_can_flush (void *handle)
   return boolean_method (handle, "can_flush");
 }
 
-static int
-sh_can_trim (void *handle)
-{
-  return boolean_method (handle, "can_trim");
-}
-
 static int
 sh_is_rotational (void *handle)
 {
@@ -480,7 +515,70 @@ sh_is_rotational (void *handle)
 }
 
 static int
-sh_flush (void *handle)
+sh_can_trim (void *handle)
+{
+  return boolean_method (handle, "can_trim");
+}
+
+static int
+sh_can_zero (void *handle)
+{
+  return boolean_method (handle, "can_zero");
+}
+
+static int
+sh_can_fua (void *handle)
+{
+  char *h = handle;
+  const char *args[] = { script, "can_fua", h, NULL };
+  char *s = NULL;
+  size_t slen;
+  int r;
+
+  switch (call_read (&s, &slen, args)) {
+  case OK:
+    if (slen > 0 && s[slen-1] == '\n')
+      s[slen-1] = '\0';
+    if (strcmp (s, "none") == 0)
+      r = NBDKIT_FUA_NONE;
+    else if (strcmp (s, "emulate") == 0)
+      r = NBDKIT_FUA_EMULATE;
+    else if (strcmp (s, "native") == 0)
+      r = NBDKIT_FUA_NATIVE;
+    else {
+      nbdkit_error ("%s: could not parse output from can_fua method: %s",
+                    script, s);
+      free (s);
+      return -1;
+    }
+    free (s);
+    return r;
+
+  case MISSING:
+    free (s);
+    /* NBDKIT_FUA_EMULATE means that nbdkit will call .flush.  However
+     * we cannot know if that callback exists, so the safest default
+     * is to return NBDKIT_FUA_NONE.
+     */
+    return NBDKIT_FUA_NONE;
+
+  case ERROR:
+    free (s);
+    return -1;
+
+  case RET_FALSE:
+    free (s);
+    nbdkit_error ("%s: %s method returned unexpected code (3/false)",
+                  script, "can_fua");
+    errno = EIO;
+    return -1;
+
+  default: abort ();
+  }
+}
+
+static int
+sh_flush (void *handle, uint32_t flags)
 {
   char *h = handle;
   const char *args[] = { script, "flush", h, NULL };
@@ -507,14 +605,15 @@ sh_flush (void *handle)
 }
 
 static int
-sh_trim (void *handle, uint32_t count, uint64_t offset)
+sh_trim (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
 {
   char *h = handle;
-  char cbuf[32], obuf[32];
-  const char *args[] = { script, "trim", h, cbuf, obuf, NULL };
+  char cbuf[32], obuf[32], fbuf[32];
+  const char *args[] = { script, "trim", h, cbuf, obuf, fbuf, NULL };
 
   snprintf (cbuf, sizeof cbuf, "%" PRIu32, count);
   snprintf (obuf, sizeof obuf, "%" PRIu64, offset);
+  flags_string (flags, fbuf, sizeof fbuf);
 
   switch (call (args)) {
   case OK:
@@ -538,15 +637,15 @@ sh_trim (void *handle, uint32_t count, uint64_t offset)
 }
 
 static int
-sh_zero (void *handle, uint32_t count, uint64_t offset, int may_trim)
+sh_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
 {
   char *h = handle;
-  char cbuf[32], obuf[32];
-  const char *args[] = { script, "zero", h, cbuf, obuf,
-                         may_trim ? "true" : "false", NULL };
+  char cbuf[32], obuf[32], fbuf[32];
+  const char *args[] = { script, "zero", h, cbuf, obuf, fbuf, NULL };
 
   snprintf (cbuf, sizeof cbuf, "%" PRIu32, count);
   snprintf (obuf, sizeof obuf, "%" PRIu64, offset);
+  flags_string (flags, fbuf, sizeof fbuf);
 
   switch (call (args)) {
   case OK:
@@ -596,6 +695,8 @@ static struct nbdkit_plugin plugin = {
   .can_flush         = sh_can_flush,
   .is_rotational     = sh_is_rotational,
   .can_trim          = sh_can_trim,
+  .can_zero          = sh_can_zero,
+  .can_fua           = sh_can_fua,
 
   .pread             = sh_pread,
   .pwrite            = sh_pwrite,
-- 
2.19.2