[Libguestfs] [nbdkit PATCH] protocol: Trivially implement NBD_CMD_FLAG_DF

[Eric Blake]

The DF flag is only available to clients that negotiated structured
replies, and even then, the spec does not require that it be
implemented. However, since our current implementation can't fragment
NBD_CMD_READ replies, we trivially implement the flag (by ignoring
it); we don't even have to pass it on to the plugins.

Enhance some documentation about sparse reads while at it (when we
finally do allow plugins to implement sparse reads, we'll have to pass
on the DF flag, as well as perform reassembly back into one reply
ourselves if the plugin ignored the flag).

tests/test-eflags.sh can't really test this, as it requires the
negotiation of structured replies (which in turn requires newstyle,
not oldstyle...)

Signed-off-by: Eric Blake <eblake at redhat.com>
---
 docs/nbdkit-protocol.pod    | 13 +++++++++++++
 common/protocol/protocol.h  |  2 ++
 server/protocol-handshake.c |  3 +++
 server/protocol.c           | 16 +++++++++++++++-
 TODO                        | 10 +++++++---
 5 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/docs/nbdkit-protocol.pod b/docs/nbdkit-protocol.pod
index a9a3390..f706cfd 100644
--- a/docs/nbdkit-protocol.pod
+++ b/docs/nbdkit-protocol.pod
@@ -139,6 +139,19 @@ Supported in nbdkit E<ge> 1.11.10.
 Only C<base:allocation> (ie. querying which parts of an image are
 sparse) is supported.

+Sparse reads (using C<NBD_REPLY_TYPE_OFFSET_HOLE> are not directly
+supported, but a client can use block status to infer which portions
+of the export do not need to be read.
+
+=item C<NBD_FLAG_DF>
+
+Supported in nbdkit E<ge> 1.11.11.
+
+This protocol extension allows a client to force an all-or-none read
+when structured replies are in effect. However, the flag is a no-op
+until we extend the plugin API to allow a fragmented read in the first
+place.
+
 =item Resize Extension

 I<Not supported>.
diff --git a/common/protocol/protocol.h b/common/protocol/protocol.h
index a7de2f0..4334be4 100644
--- a/common/protocol/protocol.h
+++ b/common/protocol/protocol.h
@@ -94,6 +94,7 @@ extern const char *name_of_nbd_flag (int);
 #define NBD_FLAG_ROTATIONAL        (1 << 4)
 #define NBD_FLAG_SEND_TRIM         (1 << 5)
 #define NBD_FLAG_SEND_WRITE_ZEROES (1 << 6)
+#define NBD_FLAG_SEND_DF           (1 << 7)
 #define NBD_FLAG_CAN_MULTI_CONN    (1 << 8)

 /* NBD options (new style handshake only). */
@@ -217,6 +218,7 @@ extern const char *name_of_nbd_cmd (int);
 extern const char *name_of_nbd_cmd_flag (int);
 #define NBD_CMD_FLAG_FUA      (1<<0)
 #define NBD_CMD_FLAG_NO_HOLE  (1<<1)
+#define NBD_CMD_FLAG_DF       (1<<2)
 #define NBD_CMD_FLAG_REQ_ONE  (1<<3)

 /* Error codes (previously errno).
diff --git a/server/protocol-handshake.c b/server/protocol-handshake.c
index 9653210..f0e5a2c 100644
--- a/server/protocol-handshake.c
+++ b/server/protocol-handshake.c
@@ -121,6 +121,9 @@ protocol_compute_eflags (struct connection *conn, uint16_t *flags)
   if (fl)
     conn->can_extents = true;

+  if (conn->structured_replies)
+    eflags |= NBD_FLAG_SEND_DF;
+
   *flags = eflags;
   return 0;
 }
diff --git a/server/protocol.c b/server/protocol.c
index 383938f..d94cd19 100644
--- a/server/protocol.c
+++ b/server/protocol.c
@@ -110,7 +110,7 @@ validate_request (struct connection *conn,

   /* Validate flags */
   if (flags & ~(NBD_CMD_FLAG_FUA | NBD_CMD_FLAG_NO_HOLE |
-                NBD_CMD_FLAG_REQ_ONE)) {
+                NBD_CMD_FLAG_DF | NBD_CMD_FLAG_REQ_ONE)) {
     nbdkit_error ("invalid request: unknown flag (0x%x)", flags);
     *error = EINVAL;
     return false;
@@ -121,6 +121,20 @@ validate_request (struct connection *conn,
     *error = EINVAL;
     return false;
   }
+  if (flags & NBD_CMD_FLAG_DF) {
+    if (cmd != NBD_CMD_READ) {
+      nbdkit_error ("invalid request: DF flag needs READ request");
+      *error = EINVAL;
+      return false;
+    }
+    if (!conn->structured_replies) {
+      nbdkit_error ("invalid request: "
+                    "%s: structured replies was not negotiated",
+                    name_of_nbd_cmd (cmd));
+      *error = EINVAL;
+      return false;
+    }
+  }
   if ((flags & NBD_CMD_FLAG_REQ_ONE) &&
       cmd != NBD_CMD_BLOCK_STATUS) {
     nbdkit_error ("invalid request: REQ_ONE flag needs BLOCK_STATUS request");
diff --git a/TODO b/TODO
index 81d692b..2b944e9 100644
--- a/TODO
+++ b/TODO
@@ -24,8 +24,8 @@ General ideas for improvements
   to inform nbdkit when the response is ready:
   https://www.redhat.com/archives/libguestfs/2018-January/msg00149.html

-* More NBD protocol features.  The only currently missing feature is
-  online resize.
+* More NBD protocol features.  The only currently missing features are
+  sparse reads and online resize.

 * Add a callback to let plugins request minimum alignment for the
   buffer to pread/pwrite; useful for a plugin utilizing O_DIRECT or
@@ -216,7 +216,11 @@ using ‘#define NBDKIT_API_VERSION <version>’.

 * pread could be changed to allow it to support Structured Replies
   (SRs).  This could mean allowing it to return partial data, holes,
-  zeroes, etc.
+  zeroes, etc.  For a client that negotiates SR coupled with a plugin
+  that supports .extents, the v2 protocol would allow us to at least
+  synthesize NBD_REPLY_TYPE_OFFSET_HOLE for less network traffic, even
+  though the plugin will still have to fully populate the .pread
+  buffer; the v3 protocol should make sparse reads more direct.

 * Parameters should be systematized so that they aren't just (key,
   value) strings.  nbdkit should know the possible keys for the plugin
-- 
2.20.1