[Libguestfs] [nbdkit PATCH v2 06/13] plugins: Move FUA fallback to plugins
Eric Blake
eblake at redhat.com
Fri Jan 19 13:40:22 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 the just-added flags parameter in backend functions.
Furthermore, the upcoming addition of filters means that we do not
want to call .flush at the wrong layer of the backend stack, so it
makes sense to have the fallback as low as possible.
The NBD spec says that we must tolerate a client sending
NBD_CMD_FLAG_FUA on any command (due to historical behavior
of at least qemu sending it on READ), but that it only has
to have defined semantics on commands that can cause write
actions. So, we only pass the FUA flag through on WRITE,
WRITE_ZEROES, and TRIM.
Note that validate_request() already ensured that that we are
not calling a write command if conn->readonly; so we no longer
have to check that. For now, FUA support is synonymous with
.can_flush support, because we are still implementing FUA via
the fallback to .flush; but future patches will split the
two conditions as part of wiring up further FUA support. It
also means that we can assert that if we did not advertise
FUA support to the client, then the client should not be
requesting FUA; and therefore if the plugins layer sees the
FUA flag, we must have a .flush callback.
Signed-off-by: Eric Blake <eblake at redhat.com>
---
src/connections.c | 25 +++++++++++--------
src/internal.h | 1 +
src/plugins.c | 75 ++++++++++++++++++++++++++++++++++++-------------------
3 files changed, 64 insertions(+), 37 deletions(-)
diff --git a/src/connections.c b/src/connections.c
index 8446691..55bfe9e 100644
--- a/src/connections.c
+++ b/src/connections.c
@@ -816,6 +816,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) &&
@@ -870,13 +875,8 @@ handle_request (struct connection *conn,
uint16_t cmd, uint16_t flags, uint64_t offset, uint32_t count,
void *buf)
{
- bool flush_after_command;
uint32_t f = 0;
-
- /* Flush after command performed? */
- flush_after_command = (flags & NBD_CMD_FLAG_FUA) != 0;
- if (!conn->can_flush || conn->readonly)
- flush_after_command = false;
+ 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. */
@@ -889,7 +889,9 @@ handle_request (struct connection *conn,
break;
case NBD_CMD_WRITE:
- if (backend->pwrite (backend, conn, buf, count, offset, 0) == -1)
+ if (fua)
+ f |= NBDKIT_FLAG_FUA;
+ if (backend->pwrite (backend, conn, buf, count, offset, f) == -1)
return get_error (conn);
break;
@@ -899,13 +901,17 @@ handle_request (struct connection *conn,
break;
case NBD_CMD_TRIM:
- if (backend->trim (backend, conn, count, offset, 0) == -1)
+ if (fua)
+ f |= NBDKIT_FLAG_FUA;
+ if (backend->trim (backend, conn, count, offset, f) == -1)
return get_error (conn);
break;
case NBD_CMD_WRITE_ZEROES:
if (!(flags & NBD_CMD_FLAG_NO_HOLE))
f |= NBDKIT_FLAG_MAY_TRIM;
+ if (fua)
+ f |= NBDKIT_FLAG_FUA;
if (backend->zero (backend, conn, count, offset, f) == -1)
return get_error (conn);
break;
@@ -914,9 +920,6 @@ handle_request (struct connection *conn,
abort ();
}
- if (flush_after_command && backend->flush (backend, conn, 0) == -1)
- return get_error (conn);
-
return 0;
}
diff --git a/src/internal.h b/src/internal.h
index be1a0ca..c69ec25 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -98,6 +98,7 @@
})
#define NBDKIT_FLAG_MAY_TRIM (1<<0) /* Maps to !NBD_CMD_FLAG_NO_HOLE */
+#define NBDKIT_FLAG_FUA (1<<1) /* Maps to NBD_CMD_FLAG_FUA */
/* main.c */
extern const char *exportname;
diff --git a/src/plugins.c b/src/plugins.c
index 4c6c3a5..eff25d3 100644
--- a/src/plugins.c
+++ b/src/plugins.c
@@ -354,25 +354,6 @@ plugin_pread (struct backend *b, struct connection *conn,
return p->plugin.pread (connection_get_handle (conn), buf, count, offset);
}
-static int
-plugin_pwrite (struct backend *b, struct connection *conn,
- const void *buf, uint32_t count, uint64_t offset, uint32_t flags)
-{
- struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
-
- assert (connection_get_handle (conn));
- assert (!flags);
-
- debug ("pwrite count=%" PRIu32 " offset=%" PRIu64, count, offset);
-
- if (p->plugin.pwrite != NULL)
- return p->plugin.pwrite (connection_get_handle (conn), buf, count, offset);
- else {
- errno = EROFS;
- return -1;
- }
-}
-
static int
plugin_flush (struct backend *b, struct connection *conn, uint32_t flags)
{
@@ -391,23 +372,58 @@ plugin_flush (struct backend *b, struct connection *conn, uint32_t flags)
}
}
+static int
+plugin_pwrite (struct backend *b, struct connection *conn,
+ const void *buf, uint32_t count, uint64_t offset, uint32_t flags)
+{
+ int r;
+ struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
+ bool fua = flags & NBDKIT_FLAG_FUA;
+
+ assert (connection_get_handle (conn));
+ assert (!(flags & ~NBDKIT_FLAG_FUA));
+
+ debug ("pwrite count=%" PRIu32 " offset=%" PRIu64 " fua=%d", count, offset,
+ fua);
+
+ if (p->plugin.pwrite != NULL)
+ r = p->plugin.pwrite (connection_get_handle (conn), buf, count, offset);
+ else {
+ errno = EROFS;
+ return -1;
+ }
+ if (r == 0 && fua) {
+ assert (p->plugin.flush);
+ r = plugin_flush (b, conn, 0);
+ }
+ return r;
+}
+
static int
plugin_trim (struct backend *b, struct connection *conn,
uint32_t count, uint64_t offset, uint32_t flags)
{
+ int r;
struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
+ bool fua = flags & NBDKIT_FLAG_FUA;
assert (connection_get_handle (conn));
- assert (!flags);
+ assert (!(flags & ~NBDKIT_FLAG_FUA));
- debug ("trim count=%" PRIu32 " offset=%" PRIu64, count, offset);
+ debug ("trim count=%" PRIu32 " offset=%" PRIu64 " fua=%d", count, offset,
+ fua);
if (p->plugin.trim != NULL)
- return p->plugin.trim (connection_get_handle (conn), count, offset);
+ r = p->plugin.trim (connection_get_handle (conn), count, offset);
else {
errno = EINVAL;
return -1;
}
+ if (r == 0 && fua) {
+ assert (p->plugin.flush);
+ r = plugin_flush (b, conn, 0);
+ }
+ return r;
}
static int
@@ -420,12 +436,13 @@ plugin_zero (struct backend *b, struct connection *conn,
int result;
int err = 0;
int may_trim = (flags & NBDKIT_FLAG_MAY_TRIM) != 0;
+ bool fua = flags & NBDKIT_FLAG_FUA;
assert (connection_get_handle (conn));
- assert (!(flags & ~NBDKIT_FLAG_MAY_TRIM));
+ assert (!(flags & ~(NBDKIT_FLAG_MAY_TRIM | NBDKIT_FLAG_FUA)));
- 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;
@@ -439,7 +456,7 @@ plugin_zero (struct backend *b, struct connection *conn,
err = errno;
}
if (result == 0 || err != EOPNOTSUPP)
- return result;
+ goto done;
}
assert (p->plugin.pwrite);
@@ -464,6 +481,12 @@ plugin_zero (struct backend *b, struct connection *conn,
err = errno;
free (buf);
errno = err;
+
+ done:
+ if (!result && fua) {
+ assert (p->plugin.flush);
+ result = plugin_flush (b, conn, 0);
+ }
return result;
}
--
2.14.3
More information about the Libguestfs
mailing list