[Libguestfs] [nbdkit PATCH 1/2] server: Assert sane error responses
Eric Blake
eblake at redhat.com
Tue Aug 13 22:03:43 UTC 2019
Our filters document that calling through next_ops should reliably set
*err on failure, and in turn that the filter must set *err if
returning -1. Since plugins.c wrapper ensures that *err is always set
regardless of the plugin, the only place where *err could be left
unset is in a broken filter. Our documentation also states that a
filter must never observe or return EOPNOTSUPP from a failed .zero,
since any fallback to .pwrite should have already happened (this
restriction will be changed in future patches when the new
NBD_CMD_FLAG_FAST_ZERO is added, but for now it is worth making sure
we obey). Since filters are always in-tree, it is appropriate to
assert that none of our backends are forgetting to set *err correctly.
Signed-off-by: Eric Blake <eblake at redhat.com>
---
server/filters.c | 56 +++++++++++++++++++++++++++++++++++++----------
server/protocol.c | 32 ++++++++++++++++++++-------
2 files changed, 69 insertions(+), 19 deletions(-)
diff --git a/server/filters.c b/server/filters.c
index 3d9e1efa..14ca0cc6 100644
--- a/server/filters.c
+++ b/server/filters.c
@@ -347,8 +347,13 @@ next_pread (void *nxdata, void *buf, uint32_t count, uint64_t offset,
uint32_t flags, int *err)
{
struct b_conn *b_conn = nxdata;
- return b_conn->b->pread (b_conn->b, b_conn->conn, buf, count, offset, flags,
- err);
+ int r;
+
+ r = b_conn->b->pread (b_conn->b, b_conn->conn, buf, count, offset, flags,
+ err);
+ if (r == -1)
+ assert (*err);
+ return 1;
}
static int
@@ -356,15 +361,25 @@ next_pwrite (void *nxdata, const void *buf, uint32_t count, uint64_t offset,
uint32_t flags, int *err)
{
struct b_conn *b_conn = nxdata;
- return b_conn->b->pwrite (b_conn->b, b_conn->conn, buf, count, offset, flags,
- err);
+ int r;
+
+ r = b_conn->b->pwrite (b_conn->b, b_conn->conn, buf, count, offset, flags,
+ err);
+ if (r == -1)
+ assert (*err);
+ return r;
}
static int
next_flush (void *nxdata, uint32_t flags, int *err)
{
struct b_conn *b_conn = nxdata;
- return b_conn->b->flush (b_conn->b, b_conn->conn, flags, err);
+ int r;
+
+ r = b_conn->b->flush (b_conn->b, b_conn->conn, flags, err);
+ if (r == -1)
+ assert (*err);
+ return r;
}
static int
@@ -372,7 +387,12 @@ next_trim (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags,
int *err)
{
struct b_conn *b_conn = nxdata;
- return b_conn->b->trim (b_conn->b, b_conn->conn, count, offset, flags, err);
+ int r;
+
+ r = b_conn->b->trim (b_conn->b, b_conn->conn, count, offset, flags, err);
+ if (r == -1)
+ assert (*err);
+ return r;
}
static int
@@ -380,7 +400,12 @@ next_zero (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags,
int *err)
{
struct b_conn *b_conn = nxdata;
- return b_conn->b->zero (b_conn->b, b_conn->conn, count, offset, flags, err);
+ int r;
+
+ r = b_conn->b->zero (b_conn->b, b_conn->conn, count, offset, flags, err);
+ if (r == -1)
+ assert (*err && *err != ENOTSUP && *err != EOPNOTSUPP);
+ return r;
}
static int
@@ -388,8 +413,13 @@ next_extents (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags,
struct nbdkit_extents *extents, int *err)
{
struct b_conn *b_conn = nxdata;
- return b_conn->b->extents (b_conn->b, b_conn->conn, count, offset, flags,
- extents, err);
+ int r;
+
+ r = b_conn->b->extents (b_conn->b, b_conn->conn, count, offset, flags,
+ extents, err);
+ if (r == -1)
+ assert (*err);
+ return r;
}
static int
@@ -397,8 +427,12 @@ next_cache (void *nxdata, uint32_t count, uint64_t offset,
uint32_t flags, int *err)
{
struct b_conn *b_conn = nxdata;
- return b_conn->b->cache (b_conn->b, b_conn->conn, count, offset, flags,
- err);
+ int r;
+
+ r = b_conn->b->cache (b_conn->b, b_conn->conn, count, offset, flags, err);
+ if (r == -1)
+ assert (*err);
+ return r;
}
static struct nbdkit_next_ops next_ops = {
diff --git a/server/protocol.c b/server/protocol.c
index 59676224..72f6f2a8 100644
--- a/server/protocol.c
+++ b/server/protocol.c
@@ -240,27 +240,35 @@ handle_request (struct connection *conn,
switch (cmd) {
case NBD_CMD_READ:
- if (backend->pread (backend, conn, buf, count, offset, 0, &err) == -1)
+ if (backend->pread (backend, conn, buf, count, offset, 0, &err) == -1) {
+ assert (err);
return err;
+ }
break;
case NBD_CMD_WRITE:
if (fua)
f |= NBDKIT_FLAG_FUA;
- if (backend->pwrite (backend, conn, buf, count, offset, f, &err) == -1)
+ if (backend->pwrite (backend, conn, buf, count, offset, f, &err) == -1) {
+ assert (err);
return err;
+ }
break;
case NBD_CMD_FLUSH:
- if (backend->flush (backend, conn, 0, &err) == -1)
+ if (backend->flush (backend, conn, 0, &err) == -1) {
+ assert (err);
return err;
+ }
break;
case NBD_CMD_TRIM:
if (fua)
f |= NBDKIT_FLAG_FUA;
- if (backend->trim (backend, conn, count, offset, f, &err) == -1)
+ if (backend->trim (backend, conn, count, offset, f, &err) == -1) {
+ assert (err);
return err;
+ }
break;
case NBD_CMD_CACHE:
@@ -271,13 +279,17 @@ handle_request (struct connection *conn,
while (count) {
limit = MIN (count, sizeof buf);
if (backend->pread (backend, conn, buf, limit, offset, flags,
- &err) == -1)
+ &err) == -1) {
+ assert (err);
return err;
+ }
count -= limit;
}
}
- else if (backend->cache (backend, conn, count, offset, 0, &err) == -1)
+ else if (backend->cache (backend, conn, count, offset, 0, &err) == -1) {
+ assert (err);
return err;
+ }
break;
case NBD_CMD_WRITE_ZEROES:
@@ -285,8 +297,10 @@ handle_request (struct connection *conn,
f |= NBDKIT_FLAG_MAY_TRIM;
if (fua)
f |= NBDKIT_FLAG_FUA;
- if (backend->zero (backend, conn, count, offset, f, &err) == -1)
+ if (backend->zero (backend, conn, count, offset, f, &err) == -1) {
+ assert (err && err != ENOTSUP && err != EOPNOTSUPP);
return err;
+ }
break;
case NBD_CMD_BLOCK_STATUS:
@@ -299,8 +313,10 @@ handle_request (struct connection *conn,
if (flags & NBD_CMD_FLAG_REQ_ONE)
f |= NBDKIT_FLAG_REQ_ONE;
if (backend->extents (backend, conn, count, offset, f,
- extents, &err) == -1)
+ extents, &err) == -1) {
+ assert (err);
return err;
+ }
}
else {
int r;
--
2.20.1
More information about the Libguestfs
mailing list