[Qemu-devel] [PATCH] Amend NBD_OPT_SELECT and NBD_OPT_GO documentation

[Alex Bligh <alex@alex.org.uk>]

Amend the NBD_OPT_SELECT and NBD_OPT_GO documentation as
follows:

* Allow a name to be specified on NBD_OPT_GO

* Make clear the rules for default device selection

* Remove the provision concerning TLS resetting device selection

* Remove NBD_REP_ERR_INVALID as a reply to NBD_OPT_GO as there
  is now no necessity for a prior NBD_OPT_SELECT

* Make it clear NBD_OPT_GO is in effect a better alternative
  for NBD_OPT_EXPORT_NAME

* Make it clear the NBD_OPT_SELECT and NBD_OPT_GO are in
  essence the same command, save that NBD_OPT_GO puts you
  into transmission mode if successful.

* Clarify permitted option returns outside TLS to prevent
  export enumeration.

* Controversial: remove 'length' 32 bit quantity from
  NBD_OPT_SELECT (and don't copy it into NBD_OPT_GO) so it
  looks exactly like NBD_OPT_EXPORT_NAME bar the reply.
  This length is unnecessary as it's in the option header
  anyway.

Signed-off-by: Alex Bligh <alex@alex.org.uk>
---
 doc/proto.md | 123 ++++++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 83 insertions(+), 40 deletions(-)

diff --git a/doc/proto.md b/doc/proto.md
index 35a3266..06601fb 100644
--- a/doc/proto.md
+++ b/doc/proto.md
@@ -659,6 +659,8 @@ To remedy this, a `SELECT` extension is envisioned. This extension adds
 two option requests and one error reply type, and extends one existing
 option reply type.
 
+Both options have identical formats for requests and replies.
+
 * `NBD_OPT_SELECT`
 
     The client wishes to select the export with the given name for use
@@ -667,8 +669,12 @@ option reply type.
 
     Data:
 
-    - 32 bits, length of name (unsigned)
-    - Name of the export
+    - Name of the export (as with `NBD_OPT_EXPORT_NAME`, the length
+      comes from the option header).
+
+    If no name is specified (i.e. a zero length string is provided),
+    the default export (if any) is specified, as with
+    `NBD_OPT_EXPORT_NAME`.
 
     The server replies with one of the following:
 
@@ -676,21 +682,36 @@ option reply type.
       server.
     - `NBD_REP_ERR_TLS_REQD`: The server does not wish to export this
       block device unless the client negotiates TLS first.
-    - `NBD_REP_SERVER`: The server accepts the chosen export. In this
-      case, the `NBD_REP_SERVER` message's data takes on a different
+    - `NBD_REP_SERVER`: The server accepts the chosen export.
+
+      Additionally, the server MAY reply `NBD_REP_ERR_TLS_REQD` to
+      requests for exports that are unknown if TLS has not been
+      negotiated. This is so that clients cannot without TLS
+      enumerate exports.
+
+      In the case of `NBD_REP_SERVER`, the message's data takes on a different
       interpretation than the default (so as to provide additional
       binary information normally sent in reply to
       `NBD_OPT_EXPORT_NAME`, in place of the default UTF-8 free-form
       string); this layout is shared with the successful response to
-      `NBD_OPT_GO`.  The option reply length MUST be *length of
-      name* + 14, and the option data has the following layout:
-
-      - 32 bits, length of name (unsigned)
-      - Name of the export. This name MAY be different from the one
-	given in the `NBD_OPT_SELECT` option in case the server has
-	multiple alternate names for a single export.
-      - 64 bits, size of the export in bytes (unsigned)
-      - 16 bits, transmission flags
+      `NBD_OPT_GO`. The option reply length MUST be
+      *length of name* + 14, and the option data has the following layout:
+
+        - 32 bits, length of name (unsigned)
+        - Name of the export. This name MAY be different from the one
+	  given in the `NBD_OPT_SELECT` option in case the server has
+          multiple alternate names for a single export, or a default
+          export was specified.
+        - 64 bits, size of the export in bytes (unsigned)
+        - 16 bits, transmission flags. The values of the transmission flags
+          MAY differ from what was sent earlier in response to
+          an earlier `NBD_OPT_SELECT` (if any), based on other options
+          that were negotiated in the meantime.
+
+      The values of the transmission flags
+      MAY differ from what was sent earlier in response to
+      `NBD_OPT_SELECT`, based on other options that were negotiated in
+      the meantime.
 
     - For backwards compatibility, clients should be prepared to also
       handle `NBD_REP_ERR_UNSUP`. In this case, they should fall back to
@@ -699,34 +720,56 @@ option reply type.
 * `NBD_OPT_GO`
 
     The client wishes to terminate the negotiation phase and progress to
-    the transmission phase. Possible replies from the server include:
+    the transmission phase. This client MAY issue this command after
+    an `NBD_OPT_SELECT`, or MAY issue it without a previous
+    `NBD_OPT_SELECT`.
 
-    - `NBD_REP_SERVER`: The server acknowledges, using the same format
-      for the reply as in `NBD_OPT_SELECT` (thus allowing the client
-      to receive the same transmission flags as would have been sent
-      by `NBD_OPT_EXPORT_NAME`).  The values of the transmission flags
-      MAY differ from what was sent earlier in response to
-      `NBD_OPT_SELECT`, based on other options that were negotiated in
-      the meantime.  The server MUST NOT send any zero padding bytes
-      after the `NBD_REP_SERVER` data, whether or not the client
-      negotiated the `NBD_FLAG_C_NO_ZEROES` flag.  After receiving
-      this reply, the client and the server have both moved to the
-      transmission phase; therefore, the server MUST NOT send this
-      particular reply until all other pending option requests have
-      had their final reply.
-    - `NBD_REP_ERR_INVALID`: No `NBD_OPT_SELECT` command was
-      previously issued during this negotiation (there is no default);
-      or, the most recent `NBD_OPT_SELECT` command resulted in an error
-      reply (selecting a different export clears the currently selected
-      export, even if the new export does not exist on the server); or,
-      the most recent `NBD_OPT_SELECT` command issued during this
-      negotiation occurred before TLS was successfully negotiated
-      (negotiating TLS clears the selected export).
-    - Servers MAY also choose to send `NBD_REP_ERR_TLS_REQD` rather than
-      `NBD_REP_ERR_INVALID` if no non-TLS exports are supported.
-    - Servers MUST NOT send `NBD_REP_ERR_UNSUP` as a reply to this
-      message if they do not also send it as a reply to the
-      `NBD_OPT_SELECT` message.
+    Data:
+
+    - Name of the export (as with `NBD_OPT_EXPORT_NAME`, the length
+      comes from the option header).
+
+    If no name is specified (i.e. a zero length string is provided)
+    then the export selected with the immediately previous valid
+    `NBD_OPT_SELECT` will be selected (if any), or if no previous
+    `NBD_OPT_SELECT` valid has been issued, the default export will be
+    selected.
+
+    `NBD_OPT_GO` can thus be used as a substitute for `NBD_OPT_EXPORT_NAME`
+    that returns errors.
+
+    The server replies with one of the following:
+
+    - `NBD_REP_ERR_UNKNOWN`: The chosen export does not exist on this
+      server.
+    - `NBD_REP_ERR_TLS_REQD`: The server does not wish to export this
+      block device unless the client negotiates TLS first.
+    - `NBD_REP_SERVER`: The server accepts the chosen export.
+    - For backwards compatibility, clients should be prepared to also
+      handle `NBD_REP_ERR_UNSUP`. In this case, they should fall back to
+      using `NBD_OPT_EXPORT_NAME`.
+
+    Additionally, the server MAY reply `NBD_REP_ERR_TLS_REQD` to
+    requests for exports that are unknown if TLS has not been
+    negotiated. This is so that clients cannot without TLS
+    enumerate exports.
+
+    The format of `NBD_REP_SERVER` is identical to the reply
+    for `NBD_OPT_SELECT`, except for the fact that both client
+    and server subseequently enter the transmission phase. By using this
+    reply the server acknowledges the connection, using the same
+    format for the reply as in `NBD_OPT_SELECT` (thus allowing the client
+    to receive the same transmission flags as would have been sent
+    by `NBD_OPT_EXPORT_NAME`); as per the note therein these
+    transmission flags MAY differ from those returned by any
+    previous `NBD_OPT_SELECT`. The server MUST NOT send any zero
+    padding bytes after the `NBD_REP_SERVER` data, whether or not the
+    client negotiated the `NBD_FLAG_C_NO_ZEROES` flag. After sending
+    this reply the server MUST immediately move to the transmission
+    phase, and after receiving this reply, the MUST immediately move
+    to the transmission phase; therefore, the server MUST NOT send this
+    particular reply until all other pending option requests have
+    had their final reply.
 
 ### `WRITE_ZEROES` extension
 
-- 
1.9.1