[Libguestfs] [nbdkit PATCH 3/7] plugins: Move FUA fallback to plugins

Eric Blake eblake at redhat.com
Tue Jan 16 02:51:43 UTC 2018


As long as the plugins couldn't directly handle Forced Unit Access
(FUA), it made sense to have a common fallback in handle_request().
But upcoming patches will allow plugins to handle FUA, at which
point the fallback is easier to implement on a per-command basis,
via an extra parameter to the plugins wrapper code.

We also add an additional validation to commands sent by the client:
if we did not advertise FUA flag support (because the plugin does
not support flush), then the client should not be requesting FUA;
this in turn means that we can assert that the flush callback is
valid if the plugins layer gets a fua flag.

Signed-off-by: Eric Blake <eblake at redhat.com>
---
 src/connections.c | 21 +++++++++------------
 src/internal.h    |  8 ++++----
 src/plugins.c     | 42 +++++++++++++++++++++++++++++++-----------
 3 files changed, 44 insertions(+), 27 deletions(-)

diff --git a/src/connections.c b/src/connections.c
index 24df4b6..e4a0a82 100644
--- a/src/connections.c
+++ b/src/connections.c
@@ -814,6 +814,11 @@ validate_request (struct connection *conn,
     *error = EINVAL;
     return false;
   }
+  if (!conn->can_flush && (flags & NBD_CMD_FLAG_FUA)) {
+    nbdkit_error ("invalid request: FUA flag not supported");
+    *error = EINVAL;
+    return false;
+  }

   /* Refuse over-large read and write requests. */
   if ((cmd == NBD_CMD_WRITE || cmd == NBD_CMD_READ) &&
@@ -868,10 +873,7 @@ handle_request (struct connection *conn,
                 uint16_t cmd, uint16_t flags, uint64_t offset, uint32_t count,
                 void *buf)
 {
-  bool flush_after_command;
-
-  /* Flush after command performed? */
-  flush_after_command = conn->can_flush && (flags & NBD_CMD_FLAG_FUA);
+  bool fua = conn->can_flush && (flags & NBD_CMD_FLAG_FUA);

   /* The plugin should call nbdkit_set_error() to request a particular
      error, otherwise we fallback to errno or EIO. */
@@ -879,30 +881,28 @@ handle_request (struct connection *conn,

   switch (cmd) {
   case NBD_CMD_READ:
-    flush_after_command = false;
     if (plugin_pread (conn, buf, count, offset) == -1)
       return get_error (conn);
     break;

   case NBD_CMD_WRITE:
-    if (plugin_pwrite (conn, buf, count, offset) == -1)
+    if (plugin_pwrite (conn, buf, count, offset, fua) == -1)
       return get_error (conn);
     break;

   case NBD_CMD_FLUSH:
-    flush_after_command = false;
     if (plugin_flush (conn) == -1)
       return get_error (conn);
     break;

   case NBD_CMD_TRIM:
-    if (plugin_trim (conn, count, offset) == -1)
+    if (plugin_trim (conn, count, offset, fua) == -1)
       return get_error (conn);
     break;

   case NBD_CMD_WRITE_ZEROES:
     if (plugin_zero (conn, count, offset,
-                     !(flags & NBD_CMD_FLAG_NO_HOLE)) == -1)
+                     !(flags & NBD_CMD_FLAG_NO_HOLE), fua) == -1)
       return get_error (conn);
     break;

@@ -910,9 +910,6 @@ handle_request (struct connection *conn,
     abort ();
   }

-  if (flush_after_command && plugin_flush (conn) == -1)
-    return get_error (conn);
-
   return 0;
 }

diff --git a/src/internal.h b/src/internal.h
index 73bc09e..3ab08d3 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -1,5 +1,5 @@
 /* nbdkit
- * Copyright (C) 2013-2017 Red Hat Inc.
+ * Copyright (C) 2013-2018 Red Hat Inc.
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -164,10 +164,10 @@ extern int plugin_can_flush (struct connection *conn);
 extern int plugin_is_rotational (struct connection *conn);
 extern int plugin_can_trim (struct connection *conn);
 extern int plugin_pread (struct connection *conn, void *buf, uint32_t count, uint64_t offset);
-extern int plugin_pwrite (struct connection *conn, void *buf, uint32_t count, uint64_t offset);
+extern int plugin_pwrite (struct connection *conn, void *buf, uint32_t count, uint64_t offset, int fua);
 extern int plugin_flush (struct connection *conn);
-extern int plugin_trim (struct connection *conn, uint32_t count, uint64_t offset);
-extern int plugin_zero (struct connection *conn, uint32_t count, uint64_t offset, int may_trim);
+extern int plugin_trim (struct connection *conn, uint32_t count, uint64_t offset, int fua);
+extern int plugin_zero (struct connection *conn, uint32_t count, uint64_t offset, int may_trim, int fua);

 /* sockets.c */
 extern int *bind_unix_socket (size_t *);
diff --git a/src/plugins.c b/src/plugins.c
index 9b5d2d5..61f0cc8 100644
--- a/src/plugins.c
+++ b/src/plugins.c
@@ -1,5 +1,5 @@
 /* nbdkit
- * Copyright (C) 2013 Red Hat Inc.
+ * Copyright (C) 2013-2018 Red Hat Inc.
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -497,19 +497,26 @@ plugin_pread (struct connection *conn,

 int
 plugin_pwrite (struct connection *conn,
-               void *buf, uint32_t count, uint64_t offset)
+               void *buf, uint32_t count, uint64_t offset, int fua)
 {
+  int r;
   assert (dl);
   assert (connection_get_handle (conn));

-  debug ("pwrite count=%" PRIu32 " offset=%" PRIu64, count, offset);
+  debug ("pwrite count=%" PRIu32 " offset=%" PRIu64 " fua=%d", count, offset,
+         fua);

   if (plugin.pwrite != NULL)
-    return plugin.pwrite (connection_get_handle (conn), buf, count, offset);
+    r = plugin.pwrite (connection_get_handle (conn), buf, count, offset);
   else {
     errno = EROFS;
     return -1;
   }
+  if (r == 0 && fua) {
+    assert (plugin.flush);
+    r = plugin_flush (conn);
+  }
+  return r;
 }

 int
@@ -529,24 +536,31 @@ plugin_flush (struct connection *conn)
 }

 int
-plugin_trim (struct connection *conn, uint32_t count, uint64_t offset)
+plugin_trim (struct connection *conn, uint32_t count, uint64_t offset, int fua)
 {
+  int r;
   assert (dl);
   assert (connection_get_handle (conn));

-  debug ("trim count=%" PRIu32 " offset=%" PRIu64, count, offset);
+  debug ("trim count=%" PRIu32 " offset=%" PRIu64 " fua=%d", count, offset,
+         fua);

   if (plugin.trim != NULL)
-    return plugin.trim (connection_get_handle (conn), count, offset);
+    r = plugin.trim (connection_get_handle (conn), count, offset);
   else {
     errno = EINVAL;
     return -1;
   }
+  if (r == 0 && fua) {
+    assert (plugin.flush);
+    r = plugin_flush (conn);
+  }
+  return r;
 }

 int
 plugin_zero (struct connection *conn,
-             uint32_t count, uint64_t offset, int may_trim)
+             uint32_t count, uint64_t offset, int may_trim, int fua)
 {
   assert (dl);
   assert (connection_get_handle (conn));
@@ -555,8 +569,8 @@ plugin_zero (struct connection *conn,
   int result;
   int err = 0;

-  debug ("zero count=%" PRIu32 " offset=%" PRIu64 " may_trim=%d",
-         count, offset, may_trim);
+  debug ("zero count=%" PRIu32 " offset=%" PRIu64 " may_trim=%d fua=%d",
+         count, offset, may_trim, fua);

   if (!count)
     return 0;
@@ -569,7 +583,7 @@ plugin_zero (struct connection *conn,
         err = errno;
     }
     if (result == 0 || err != EOPNOTSUPP)
-      return result;
+      goto done;
   }

   assert (plugin.pwrite);
@@ -593,5 +607,11 @@ plugin_zero (struct connection *conn,
   err = errno;
   free (buf);
   errno = err;
+
+ done:
+  if (!result && fua) {
+    assert (plugin.flush);
+    result = plugin_flush (conn);
+  }
   return result;
 }
-- 
2.14.3




More information about the Libguestfs mailing list