[Libguestfs] [nbdkit PATCH] Update filters to support FUA flags.

Eric Blake eblake at redhat.com
Fri Jan 19 16:23:32 UTC 2018


From: "Richard W.M. Jones" <rjones at 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




More information about the Libguestfs mailing list