From: "Richard W.M. Jones" <rjones(a)redhat.com>
This patch may be worth squashing?
---
docs/nbdkit-filter.pod | 65 +++++++++++++++++++++++++++++++++++++++++++------
include/nbdkit-filter.h | 29 ++++++++++++----------
src/filters.c | 55 ++++++++++++++++++++---------------------
3 files changed, 101 insertions(+), 48 deletions(-)
diff --git a/docs/nbdkit-filter.pod b/docs/nbdkit-filter.pod
index 75157ef..a050a1d 100644
--- a/docs/nbdkit-filter.pod
+++ b/docs/nbdkit-filter.pod
@@ -89,7 +89,12 @@ by the plugin.
To see example filters, take a look at the source of nbdkit, in the
C<filters> directory.
-Filters must be written in C and must be fully thread safe.
+Filters must be written in C, must be fully thread safe, and have
+tighter rules regarding what callbacks may do. While there is a
+guarantee that plugins written against an older version of nbdkit will
+still work with newer versions, filters do not have the same stability
+guarantee, and nbdkit may refuse to use a filter that was compiled
+against a different version rather than risk misbehavior.
=head1 C<nbdkit-filter.h>
@@ -320,11 +325,15 @@ error message and return C<-1>.
=head2 C<.pread>
int (*pread) (struct nbdkit_next_ops *next_ops, void *nxdata,
- void *handle, void *buf, uint32_t count, uint64_t offset);
+ void *handle, void *buf, uint32_t count, uint64_t offset,
+ uint32_t flags);
This intercepts the plugin C<.pread> method and can be used to read or
modify data read by the plugin.
+At this time, flags will be 0 on input, and the filter should not pass
+any flags to C<next_ops->pread>.
+
If there is an error (including a short read which couldn't be
recovered from), C<.pread> should call C<nbdkit_error> with an error
message B<and> set C<errno>, then return C<-1>.
@@ -333,11 +342,21 @@ message B<and> set C<errno>, then return C<-1>.
int (*pwrite) (struct nbdkit_next_ops *next_ops, void *nxdata,
void *handle,
- const void *buf, uint32_t count, uint64_t offset);
+ const void *buf, uint32_t count, uint64_t offset,
+ uint32_t flags);
This intercepts the plugin C<.pwrite> method and can be used to modify
data written by the plugin.
+At this time, flags may include C<NBDKIT_FLAG_FUA> on input based on
+the result of C<.can_flush>. In turn, the filter may only pass
+C<NBDKIT_FLAG_FUA> on to C<next_ops->pwrite> if
C<next_ops->can_flush>
+returned true.
+
+This function will not be called if C<.can_write> returned false; in
+turn, the filter should not call C<next_ops->pwrite> if
C<next_ops->can_write>
+did not return true.
+
If there is an error (including a short write which couldn't be
recovered from), C<.pwrite> should call C<nbdkit_error> with an error
message B<and> set C<errno>, then return C<-1>.
@@ -345,35 +364,67 @@ message B<and> set C<errno>, then return C<-1>.
=head2 C<.flush>
int (*flush) (struct nbdkit_next_ops *next_ops, void *nxdata,
- void *handle);
+ void *handle, uint32_t flags);
This intercepts the plugin C<.flush> method and can be used to modify
flush requests.
+At this time, flags will be 0 on input, and the filter should not pass
+any flags to C<next_ops->flush>.
+
+This function will not be called if C<.can_flush> returned false; in
+turn, the filter should not call C<next_ops->flush> if
C<next_ops->can_flush>
+did not return true.
+
If there is an error, C<.flush> should call C<nbdkit_error> with an
error message B<and> set C<errno>, then return C<-1>.
=head2 C<.trim>
int (*trim) (struct nbdkit_next_ops *next_ops, void *nxdata,
- void *handle, uint32_t count, uint64_t offset);
+ void *handle, uint32_t count, uint64_t offset, uint32_t flags);
This intercepts the plugin C<.trim> method and can be used to modify
trim requests.
+At this time, flags may include C<NBDKIT_FLAG_FUA> on input based on
+the result of C<.can_flush>. In turn, the filter may only pass
+C<NBDKIT_FLAG_FUA> on to C<next_ops->trim> if
C<next_ops->can_flush>
+returned true.
+
+This function will not be called if C<.can_trim> returned false; in
+turn, the filter should not call C<next_ops->trim> if
C<next_ops->can_trim>
+did not return true.
+
If there is an error, C<.trim> should call C<nbdkit_error> with an
error message B<and> set C<errno>, then return C<-1>.
=head2 C<.zero>
int (*zero) (struct nbdkit_next_ops *next_ops, void *nxdata,
- void *handle, uint32_t count, uint64_t offset, int may_trim);
+ void *handle, uint32_t count, uint64_t offset,
+ uint32_t flags);
This intercepts the plugin C<.zero> method and can be used to modify
zero requests.
+At this time, flags may include C<NBDKIT_FLAG_MAY_TRIM>
+unconditionally, and C<NBDKIT_FLAG_FUA> based on the result of
+C<.can_flush>. In turn, when calling C<next_ops->zero>, the filter may
+pass C<NBDKIT_FLAG_MAY_TRIM> unconditionally, but may only pass
+C<NBDKIT_FLAG_FUA> if C<next_ops->can_flush> returned true.
+
+This function will not be called if C<.can_write> returned false; in
+turn, the filter should not call C<next_ops->zero> if
C<next_ops->can_write>
+did not return true.
+
If there is an error, C<.zero> should call C<nbdkit_error> with an
-error message B<and> set C<errno>, then return C<-1>.
+error message B<and> set C<errno>, then return C<-1>; however,
+unlike plugins, this function must not return the C<EOPNOTSUPP>
+error (the code guarantees that C<next_ops->zero> will have already
+done a fallback to C<next_ops->write> rather than fail with that
+particular error, and the fallback must not be performed more
+than once).
=head1 THREADS
diff --git a/include/nbdkit-filter.h b/include/nbdkit-filter.h
index af79e33..c8b3c8c 100644
--- a/include/nbdkit-filter.h
+++ b/include/nbdkit-filter.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
@@ -59,17 +59,18 @@ struct nbdkit_next_ops {
int (*is_rotational) (void *nxdata);
int (*can_trim) (void *nxdata);
- int (*pread) (void *nxdata, void *buf, uint32_t count, uint64_t offset);
- int (*pwrite) (void *nxdata,
- const void *buf, uint32_t count, uint64_t offset);
- int (*flush) (void *nxdata);
- int (*trim) (void *nxdata, uint32_t count, uint64_t offset);
- int (*zero) (void *nxdata, uint32_t count, uint64_t offset, int may_trim);
+ int (*pread) (void *nxdata, void *buf, uint32_t count, uint64_t offset,
+ uint32_t flags);
+ int (*pwrite) (void *nxdata, const void *buf, uint32_t count,
+ uint64_t offset, uint32_t flags);
+ int (*flush) (void *nxdata, uint32_t flags);
+ int (*trim) (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags);
+ int (*zero) (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags);
};
struct nbdkit_filter {
/* Do not set these fields directly; use NBDKIT_REGISTER_FILTER.
- * They exist so that we can support filters compiled against
+ * They exist so that we can recognize filters compiled against
* one version of the header with a runtime compiled against a
* different version with more (or fewer) fields.
*/
@@ -112,16 +113,18 @@ struct nbdkit_filter {
void *handle);
int (*pread) (struct nbdkit_next_ops *next_ops, void *nxdata,
- void *handle, void *buf, uint32_t count, uint64_t offset);
+ void *handle, void *buf, uint32_t count, uint64_t offset,
+ uint32_t flags);
int (*pwrite) (struct nbdkit_next_ops *next_ops, void *nxdata,
void *handle,
- const void *buf, uint32_t count, uint64_t offset);
+ const void *buf, uint32_t count, uint64_t offset,
+ uint32_t flags);
int (*flush) (struct nbdkit_next_ops *next_ops, void *nxdata,
- void *handle);
+ void *handle, uint32_t flags);
int (*trim) (struct nbdkit_next_ops *next_ops, void *nxdata,
- void *handle, uint32_t count, uint64_t offset);
+ void *handle, uint32_t count, uint64_t offset, uint32_t flags);
int (*zero) (struct nbdkit_next_ops *next_ops, void *nxdata,
- void *handle, uint32_t count, uint64_t offset, int may_trim);
+ void *handle, uint32_t count, uint64_t offset, uint32_t flags);
};
#ifndef NBDKIT_CXX_LANG_C
diff --git a/src/filters.c b/src/filters.c
index 9a2022c..9768f20 100644
--- a/src/filters.c
+++ b/src/filters.c
@@ -280,43 +280,40 @@ next_can_trim (void *nxdata)
}
static int
-next_pread (void *nxdata, void *buf, uint32_t count, uint64_t offset)
+next_pread (void *nxdata, void *buf, uint32_t count, uint64_t offset,
+ uint32_t flags)
{
struct b_conn *b_conn = nxdata;
- return b_conn->b->pread (b_conn->b, b_conn->conn, buf, count, offset, 0);
+ return b_conn->b->pread (b_conn->b, b_conn->conn, buf, count, offset,
flags);
}
static int
-next_pwrite (void *nxdata, const void *buf, uint32_t count, uint64_t offset)
+next_pwrite (void *nxdata, const void *buf, uint32_t count, uint64_t offset,
+ uint32_t flags)
{
struct b_conn *b_conn = nxdata;
- return b_conn->b->pwrite (b_conn->b, b_conn->conn, buf, count, offset, 0);
+ return b_conn->b->pwrite (b_conn->b, b_conn->conn, buf, count, offset,
flags);
}
static int
-next_flush (void *nxdata)
+next_flush (void *nxdata, uint32_t flags)
{
struct b_conn *b_conn = nxdata;
- return b_conn->b->flush (b_conn->b, b_conn->conn, 0);
+ return b_conn->b->flush (b_conn->b, b_conn->conn, flags);
}
static int
-next_trim (void *nxdata, uint32_t count, uint64_t offset)
+next_trim (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags)
{
struct b_conn *b_conn = nxdata;
- return b_conn->b->trim (b_conn->b, b_conn->conn, count, offset, 0);
+ return b_conn->b->trim (b_conn->b, b_conn->conn, count, offset, flags);
}
static int
-next_zero (void *nxdata, uint32_t count, uint64_t offset, int may_trim)
+next_zero (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags)
{
struct b_conn *b_conn = nxdata;
- uint32_t f = 0;
-
- if (may_trim)
- f |= NBDKIT_FLAG_MAY_TRIM;
-
- return b_conn->b->zero (b_conn->b, b_conn->conn, count, offset, f);
+ return b_conn->b->zero (b_conn->b, b_conn->conn, count, offset, flags);
}
static struct nbdkit_next_ops next_ops = {
@@ -448,11 +445,12 @@ filter_pread (struct backend *b, struct connection *conn,
assert (flags == 0);
- debug ("pread count=%" PRIu32 " offset=%" PRIu64, count, offset);
+ debug ("pread count=%" PRIu32 " offset=%" PRIu64 "
flags=0x%" PRIx32,
+ count, offset, flags);
if (f->filter.pread)
return f->filter.pread (&next_ops, &nxdata, handle,
- buf, count, offset);
+ buf, count, offset, flags);
else
return f->backend.next->pread (f->backend.next, conn,
buf, count, offset, flags);
@@ -470,12 +468,12 @@ filter_pwrite (struct backend *b, struct connection *conn,
assert (!(flags & ~NBDKIT_FLAG_FUA));
- debug ("pwrite count=%" PRIu32 " offset=%" PRIu64 "
fua=%d",
- count, offset, fua);
+ debug ("pwrite count=%" PRIu32 " offset=%" PRIu64 "
flags=0x%" PRIx32,
+ count, offset, flags);
if (f->filter.pwrite)
return f->filter.pwrite (&next_ops, &nxdata, handle,
- buf, count, offset);
+ buf, count, offset, flags);
else
return f->backend.next->pwrite (f->backend.next, conn,
buf, count, offset, flags);
@@ -490,10 +488,10 @@ filter_flush (struct backend *b, struct connection *conn, uint32_t
flags)
assert (flags == 0);
- debug ("flush");
+ debug ("flush flags=0x%" PRIx32, flags);
if (f->filter.flush)
- return f->filter.flush (&next_ops, &nxdata, handle);
+ return f->filter.flush (&next_ops, &nxdata, handle, flags);
else
return f->backend.next->flush (f->backend.next, conn, flags);
}
@@ -509,10 +507,12 @@ filter_trim (struct backend *b, struct connection *conn,
assert (flags == 0);
- debug ("trim count=%" PRIu32 " offset=%" PRIu64, count, offset);
+ debug ("trim count=%" PRIu32 " offset=%" PRIu64 "
flags=0x%" PRIx32,
+ count, offset, flags);
if (f->filter.trim)
- return f->filter.trim (&next_ops, &nxdata, handle, count, offset);
+ return f->filter.trim (&next_ops, &nxdata, handle, count, offset,
+ flags);
else
return f->backend.next->trim (f->backend.next, conn, count, offset, flags);
}
@@ -524,16 +524,15 @@ filter_zero (struct backend *b, struct connection *conn,
struct backend_filter *f = container_of (b, struct backend_filter, backend);
void *handle = connection_get_handle (conn, f->backend.i);
struct b_conn nxdata = { .b = f->backend.next, .conn = conn };
- int may_trim = (flags & NBDKIT_FLAG_MAY_TRIM) != 0;
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 "
flags=0x%" PRIx32,
+ count, offset, flags);
if (f->filter.zero)
return f->filter.zero (&next_ops, &nxdata, handle,
- count, offset, may_trim);
+ count, offset, flags);
else
return f->backend.next->zero (f->backend.next, conn,
count, offset, flags);
--
2.14.3