[Qemu-devel] RFC: Tweak the NBD_OPT_SET_META_CONTEXT layout

[Qemu-devel] RFC: Tweak the NBD_OPT_SET_META_CONTEXT layout

[Eric Blake]

As mentioned in earlier email threads, I experimented with an 
alternative layout to the structs sent across the wire during 
NBD_OPT_{LIST,SET}_META_CONTEXT, on the grounds that having the 
namespace and leaf-name combined into one string that requires further 
parsing on each end is not as nice as having two separate fields.  I'm 
replying to this mail with two RFC patches, one for the NBD spec, and 
one for the qemu implementation.

If we like the idea, this has to go into qemu 2.12, so we have less than 
a week.  Or, we can ignore the RFC, keep qemu the way it is currently 
coded, and when it is released, it will be time to promote the NBD 
extension-blockstatus to current as there will be an existing 
implementation to be interoperable with.

Note that I intentionally changed the value of NBD_OPT_SET_META_CONTEXT 
so that the existing Virtuozzo of OPT 10 will not get confused by the 
new layout (both new server and old client, as well as old server and 
new client, will merely fail to negotiate block status, and then be 
unable to use NBD_CMD_BLOCK_STATUS).  I didn't bother changing 
NBD_OPT_LIST_META_CONTEXT, as it looks like the existing Virtuozzo 
implementation did not use that option as a client.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

[Qemu-devel] [PATCH qemu] RFC: nbd: Separate namespace from leaf-name for metadata contexts

[Eric Blake]

During implementation of NBD_CMD_BLOCK_STATUS, we found that parsing
for the colon that separates namespaces from leaf-names can get
awkward.  Simplify things slightly by passing the two as separate
fields, and noting that since strings are capped to 4k, a 16-bit
rather than 32-bit length field is sufficient.  On the request
(NBD_OPT_{LIST,SET}_META_CONTEXT), the client can send more than
one query in a single wire packet, so must include the length of
the namespace and leaf per query.  On reply (NBD_REP_META_CONTEXT),
each context is set as a separate wire packet, and therefore the
header length is sufficient to reconstruct the leaf length without
having to redundantly send it.

Signed-off-by: Eric Blake <eblake@redhat.com>
---

As mentioned in the cover letter, this has to go in at the same
time as the corresponding NBD patch, and prior to the qemu 2.12
release, if we like it.  Or we can ignore it.

 include/block/nbd.h |  9 ++++---
 nbd/client.c        | 72 +++++++++++++++++++++++++++++++++++------------------
 nbd/server.c        | 71 +++++++++++++++++++++++++++++++++-------------------
 nbd/trace-events    |  7 +++---
 4 files changed, 103 insertions(+), 56 deletions(-)

diff --git a/include/block/nbd.h b/include/block/nbd.h
index fcdcd545023..2060c0c666d 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -42,9 +42,11 @@ struct NBDOptionReply {
 typedef struct NBDOptionReply NBDOptionReply;

 typedef struct NBDOptionReplyMetaContext {
-    NBDOptionReply h; /* h.type = NBD_REP_META_CONTEXT, h.length > 4 */
+    NBDOptionReply h; /* h.type = NBD_REP_META_CONTEXT, h.length > 7 */
     uint32_t context_id;
-    /* meta context name follows */
+    uint16_t namespace_len; /* Must be nonzero */
+    /* namespace follows, length given by namespace_len */
+    /* leaf-name follows, length computed by h.length - 6 - namespace_len */
 } QEMU_PACKED NBDOptionReplyMetaContext;

 /* Transmission phase structs
@@ -156,7 +158,8 @@ typedef struct NBDExtent {
 #define NBD_OPT_GO                (7)
 #define NBD_OPT_STRUCTURED_REPLY  (8)
 #define NBD_OPT_LIST_META_CONTEXT (9)
-#define NBD_OPT_SET_META_CONTEXT  (10)
+/* #define NBD_OPT_OLD_META_CONTEXT (10) not in use */
+#define NBD_OPT_SET_META_CONTEXT  (11)

 /* Option reply types. */
 #define NBD_REP_ERR(value) ((UINT32_C(1) << 31) | (value))
diff --git a/nbd/client.c b/nbd/client.c
index 478b6085df7..1858289ad4f 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -599,15 +599,16 @@ static QIOChannel *nbd_receive_starttls(QIOChannel *ioc,
  * Set one meta context. Simple means that reply must contain zero (not
  * negotiated) or one (negotiated) contexts. More contexts would be considered
  * as a protocol error. It's also implied that meta-data query equals queried
- * context name, so, if server replies with something different than @context,
- * it is considered an error too.
+ * context name, so, if server replies with something different than
+ * the same @namespace and @leaf, it is considered an error too.
  * return 1 for successful negotiation, context_id is set
  *        0 if operation is unsupported,
  *        -1 with errp set for any other error
  */
 static int nbd_negotiate_simple_meta_context(QIOChannel *ioc,
                                              const char *export,
-                                             const char *context,
+                                             const char *namespace,
+                                             const char *leaf,
                                              uint32_t *context_id,
                                              Error **errp)
 {
@@ -616,19 +617,23 @@ static int nbd_negotiate_simple_meta_context(QIOChannel *ioc,
     uint32_t received_id;
     bool received;
     uint32_t export_len = strlen(export);
-    uint32_t context_len = strlen(context);
+    uint16_t namespace_len = strlen(namespace);
+    uint16_t leaf_len = strlen(leaf);
     uint32_t data_len = sizeof(export_len) + export_len +
                         sizeof(uint32_t) + /* number of queries */
-                        sizeof(context_len) + context_len;
+                        sizeof(namespace_len) + namespace_len +
+                        sizeof(leaf_len) + leaf_len;
     char *data = g_malloc(data_len);
     char *p = data;

-    trace_nbd_opt_meta_request(context, export);
+    trace_nbd_opt_meta_request(namespace, leaf, export);
     stl_be_p(p, export_len);
     memcpy(p += sizeof(export_len), export, export_len);
     stl_be_p(p += export_len, 1);
-    stl_be_p(p += sizeof(uint32_t), context_len);
-    memcpy(p += sizeof(context_len), context, context_len);
+    stw_be_p(p += sizeof(uint32_t), namespace_len);
+    memcpy(p += sizeof(namespace_len), namespace, namespace_len);
+    stw_be_p(p += namespace_len, leaf_len);
+    memcpy(p += sizeof(leaf_len), leaf, leaf_len);

     ret = nbd_send_option_request(ioc, NBD_OPT_SET_META_CONTEXT, data_len, data,
                                   errp);
@@ -650,14 +655,16 @@ static int nbd_negotiate_simple_meta_context(QIOChannel *ioc,

     if (reply.type == NBD_REP_META_CONTEXT) {
         char *name;
-        size_t len;
+        uint16_t len;

-        if (reply.length != sizeof(received_id) + context_len) {
-            error_setg(errp, "Failed to negotiate meta context '%s', server "
-                       "answered with unexpected length %u", context,
-                       reply.length);
-            nbd_send_opt_abort(ioc);
-            return -1;
+        if (reply.length != sizeof(received_id) + sizeof(len) +
+            namespace_len + leaf_len) {
+            /* Name can't match, silently ignore this reply */
+            if (nbd_drop(ioc, reply.length, errp) < 0) {
+                return -1;
+            }
+            trace_nbd_opt_meta_skip("size mismatch compared to header");
+            goto next;
         }

         if (nbd_read(ioc, &received_id, sizeof(received_id), errp) < 0) {
@@ -665,16 +672,32 @@ static int nbd_negotiate_simple_meta_context(QIOChannel *ioc,
         }
         be32_to_cpus(&received_id);

-        len = reply.length - sizeof(received_id);
-        name = g_malloc(len + 1);
-        if (nbd_read(ioc, name, len, errp) < 0) {
+        if (nbd_read(ioc, &len, sizeof(len), errp) < 0) {
+            return -1;
+        }
+        be16_to_cpus(&len);
+        if (len != namespace_len) {
+            /* Name can't match, silently ignore this reply */
+            if (nbd_drop(ioc, reply.length - sizeof(received_id) - sizeof(len),
+                         errp) < 0) {
+                return -1;
+            }
+            trace_nbd_opt_meta_skip("size mismatch on namespace");
+            goto next;
+        }
+
+        name = g_malloc(namespace_len + leaf_len + 1);
+        if (nbd_read(ioc, name,
+                     reply.length - sizeof(received_id) - sizeof(len),
+                     errp) < 0) {
             g_free(name);
             return -1;
         }
-        name[len] = '\0';
-        if (strcmp(context, name)) {
-            error_setg(errp, "Failed to negotiate meta context '%s', server "
-                       "answered with different context '%s'", context,
+        name[namespace_len + leaf_len] = '\0';
+        if (strncmp(namespace, name, namespace_len) ||
+            strcmp(leaf, name + namespace_len)) {
+            error_setg(errp, "Failed to negotiate meta context '%s:%s', server "
+                       "answered with different context '%s'", namespace, leaf,
                        name);
             g_free(name);
             nbd_send_opt_abort(ioc);
@@ -682,9 +705,10 @@ static int nbd_negotiate_simple_meta_context(QIOChannel *ioc,
         }
         g_free(name);

-        trace_nbd_opt_meta_reply(context, received_id);
+        trace_nbd_opt_meta_reply(namespace, leaf, received_id);
         received = true;

+    next:
         /* receive NBD_REP_ACK */
         if (nbd_receive_option_reply(ioc, NBD_OPT_SET_META_CONTEXT, &reply,
                                      errp) < 0)
@@ -826,7 +850,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name,

             if (info->structured_reply && base_allocation) {
                 result = nbd_negotiate_simple_meta_context(
-                        ioc, name, "base:allocation",
+                        ioc, name, "base", "allocation",
                         &info->meta_base_allocation_id, errp);
                 if (result < 0) {
                     goto fail;
diff --git a/nbd/server.c b/nbd/server.c
index 9e1f2271784..21614ab89c0 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -712,42 +712,52 @@ static QIOChannel *nbd_negotiate_handle_starttls(NBDClient *client,
  * For NBD_OPT_LIST_META_CONTEXT @context_id is ignored, 0 is used instead.
  */
 static int nbd_negotiate_send_meta_context(NBDClient *client,
-                                           const char *context,
+                                           const char *namespace,
+                                           const char *leaf,
                                            uint32_t context_id,
                                            Error **errp)
 {
     NBDOptionReplyMetaContext opt;
     struct iovec iov[] = {
         {.iov_base = &opt, .iov_len = sizeof(opt)},
-        {.iov_base = (void *)context, .iov_len = strlen(context)}
+        {.iov_base = (void *)namespace, .iov_len = strlen(namespace)},
+        {.iov_base = (void *)leaf, .iov_len = strlen(leaf)},
     };

     if (client->opt == NBD_OPT_LIST_META_CONTEXT) {
         context_id = 0;
     }

-    trace_nbd_negotiate_meta_query_reply(context, context_id);
+    trace_nbd_negotiate_meta_query_reply(namespace, leaf, context_id);
     set_be_option_rep(&opt.h, client->opt, NBD_REP_META_CONTEXT,
-                      sizeof(opt) - sizeof(opt.h) + iov[1].iov_len);
+                      sizeof(opt) - sizeof(opt.h) + iov[1].iov_len +
+                      iov[2].iov_len);
     stl_be_p(&opt.context_id, context_id);
+    stw_be_p(&opt.namespace_len, iov[1].iov_len);

-    return qio_channel_writev_all(client->ioc, iov, 2, errp) < 0 ? -EIO : 0;
+    return qio_channel_writev_all(client->ioc, iov, 3, errp) < 0 ? -EIO : 0;
 }

 /* nbd_meta_base_query
  *
  * Handle query to 'base' namespace. For now, only base:allocation context is
- * available in it.  'len' is the amount of text remaining to be read from
- * the current name, after the 'base:' portion has been stripped.
+ * available in it.
  *
  * Return -errno on I/O error, 0 if option was completely handled by
  * sending a reply about inconsistent lengths, or 1 on success. */
 static int nbd_meta_base_query(NBDClient *client, NBDExportMetaContexts *meta,
-                               uint32_t len, Error **errp)
+                               Error **errp)
 {
     int ret;
-    char query[sizeof("allocation") - 1];
-    size_t alen = strlen("allocation");
+    char query[] = "allocation";
+    size_t alen = strlen(query);
+    uint16_t len;
+
+    ret = nbd_opt_read(client, &len, sizeof(len), errp);
+    if (ret <= 0) {
+        return ret;
+    }
+    cpu_to_be16s(&len);

     if (len == 0) {
         if (client->opt == NBD_OPT_LIST_META_CONTEXT) {
@@ -767,7 +777,7 @@ static int nbd_meta_base_query(NBDClient *client, NBDExportMetaContexts *meta,
         return ret;
     }

-    if (strncmp(query, "allocation", alen) == 0) {
+    if (!strcmp(query, "allocation")) {
         meta->base_allocation = true;
     }

@@ -791,34 +801,43 @@ static int nbd_negotiate_meta_query(NBDClient *client,
                                     NBDExportMetaContexts *meta, Error **errp)
 {
     int ret;
-    char query[sizeof("base:") - 1];
-    size_t baselen = strlen("base:");
-    uint32_t len;
+    char query[] = "base";
+    size_t baselen = strlen(query);
+    uint16_t len;

     ret = nbd_opt_read(client, &len, sizeof(len), errp);
     if (ret <= 0) {
         return ret;
     }
-    cpu_to_be32s(&len);
+    cpu_to_be16s(&len);

-    /* The only supported namespace for now is 'base'. So query should start
-     * with 'base:'. Otherwise, we can ignore it and skip the remainder. */
-    if (len < baselen) {
-        trace_nbd_negotiate_meta_query_skip("length too short");
-        return nbd_opt_skip(client, len, errp);
+    /* The only supported namespace for now is 'base'. Ignore any other
+     * namespace */
+    if (len != baselen) {
+        trace_nbd_negotiate_meta_query_skip("not for base: namespace");
+        ret = nbd_opt_skip(client, len, errp);
+        if (ret <= 0) {
+            return ret;
+        }
+        goto skip_leaf;
     }

-    len -= baselen;
-    ret = nbd_opt_read(client, query, baselen, errp);
+    ret = nbd_opt_read(client, query, len, errp);
     if (ret <= 0) {
         return ret;
     }
-    if (strncmp(query, "base:", baselen) != 0) {
+    if (!strcmp(query, "base")) {
         trace_nbd_negotiate_meta_query_skip("not for base: namespace");
-        return nbd_opt_skip(client, len, errp);
+        return nbd_meta_base_query(client, meta, errp);
     }

-    return nbd_meta_base_query(client, meta, len, errp);
+ skip_leaf:
+    ret = nbd_opt_read(client, &len, sizeof(len), errp);
+    if (ret <= 0) {
+        return ret;
+    }
+    cpu_to_be16s(&len);
+    return nbd_opt_skip(client, len, errp);
 }

 /* nbd_negotiate_meta_queries
@@ -880,7 +899,7 @@ static int nbd_negotiate_meta_queries(NBDClient *client,
     }

     if (meta->base_allocation) {
-        ret = nbd_negotiate_send_meta_context(client, "base:allocation",
+        ret = nbd_negotiate_send_meta_context(client, "base", "allocation",
                                               NBD_META_ID_BASE_ALLOCATION,
                                               errp);
         if (ret < 0) {
diff --git a/nbd/trace-events b/nbd/trace-events
index cfdbe04b447..585ca7e809d 100644
--- a/nbd/trace-events
+++ b/nbd/trace-events
@@ -10,8 +10,9 @@ nbd_receive_query_exports_start(const char *wantname) "Querying export list for
 nbd_receive_query_exports_success(const char *wantname) "Found desired export name '%s'"
 nbd_receive_starttls_new_client(void) "Setting up TLS"
 nbd_receive_starttls_tls_handshake(void) "Starting TLS handshake"
-nbd_opt_meta_request(const char *context, const char *export) "Requesting to set meta context %s for export %s"
-nbd_opt_meta_reply(const char *context, int id) "Received mapping of context %s to id %d"
+nbd_opt_meta_request(const char *namespace, const char *leaf, const char *export) "Requesting to set meta context %s:%s for export %s"
+nbd_opt_meta_skip(const char *reason) "Could not parse server reply: %s"
+nbd_opt_meta_reply(const char *namespace, const char *leaf, int id) "Received mapping of context %s:%s to id %d"
 nbd_receive_negotiate(void *tlscreds, const char *hostname) "Receiving negotiation tlscreds=%p hostname=%s"
 nbd_receive_negotiate_magic(uint64_t magic) "Magic is 0x%" PRIx64
 nbd_receive_negotiate_server_flags(uint32_t globalflags) "Global flags are 0x%" PRIx32
@@ -49,7 +50,7 @@ nbd_negotiate_handle_starttls_handshake(void) "Starting TLS handshake"
 nbd_negotiate_meta_context(const char *optname, const char *export, int queries) "Client requested %s for export %s, with %d queries"
 nbd_negotiate_meta_query_skip(const char *reason) "Skipping meta query: %s"
 nbd_negotiate_meta_query_parse(const char *query) "Parsed meta query '%s'"
-nbd_negotiate_meta_query_reply(const char *context, unsigned int id) "Replying with meta context '%s' id %d"
+nbd_negotiate_meta_query_reply(const char *namespace, const char *leaf, unsigned int id) "Replying with meta context '%s:%s' id %d"
 nbd_negotiate_options_flags(uint32_t flags) "Received client flags 0x%" PRIx32
 nbd_negotiate_options_check_magic(uint64_t magic) "Checking opts magic 0x%" PRIx64
 nbd_negotiate_options_check_option(uint32_t option, const char *name) "Checking option %" PRIu32 " (%s)"
-- 
2.14.3

[Qemu-devel] [PATCH nbd] RFC: doc: Separate namespace from leaf-name for metadata contexts

[Eric Blake]

During implementation of NBD_CMD_BLOCK_STATUS in qemu, it was
discovered that parsing for the colon that separates the namespace
from the leaf-name can get awkward; a slight tweak to the wire
layout produces something a little friendlier to manipulate.

Since NBD strings are capped at 4k each, 16 bits is sufficient
for string lengths rather than 32.  On the request (in
NBD_OPT_LIST_META_CONTEXT and NBD_OPT_SET_META_CONTEXT), the
client must send the length of both namespace and leaf-name, as
the client is bundling together one or more queries into a
single wire transaction.  On reply (NBD_REP_META_CONTEXT), each
context is sent as a separate wire transaction, and therefore
the header length is sufficient to reconstruct the leaf-name
length without having to redundantly send that.

Since there is a known implementation of a prior description
of experimental block status in the wild (Virtuozzo, which used
SET but not LIST), this spec change also bumps the value of
NBD_OPT_SET_META_CONTEXT; that way, a mismatch between
experimental build and the layout documented here will fail to
negotiate meta contexts due to a difference in values, rather
than confusing one another with different layout expectations.

Signed-off-by: Eric Blake <eblake@redhat.com>
---

As mentioned in the cover letter, this has to go in at the same
time as the corresponding qemu patch, and prior to the qemu 2.12
release, if we like it.  Or we can ignore it.

 doc/proto.md | 78 +++++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 46 insertions(+), 32 deletions(-)

diff --git a/doc/proto.md b/doc/proto.md
index 3abcf67..0e00d74 100644
--- a/doc/proto.md
+++ b/doc/proto.md
@@ -876,12 +876,13 @@ The reply to the `NBD_CMD_BLOCK_STATUS` request MUST be sent as a
 structured reply; this implies that in order to use metadata querying,
 structured replies MUST be negotiated first.

-Metadata contexts are identified by their names. The name MUST
-consist of a namespace, followed by a colon, followed by a leaf-name.
+Metadata contexts are identified by a namespace combined with a leaf-name.
 The namespace must consist entirely of printable non-whitespace
-UTF-8 characters other than colons, and be non-empty. The entire name
-(namespace, colon, and leaf-name) MUST follow the restrictions for
-strings as laid out earlier in this document.
+UTF-8 characters other than colons, and be non-empty. Both the namespace
+and leaf-name MUST follow the restrictions for
+strings as laid out earlier in this document.  For convenience, this
+document describes `namespace:leaf-name` as the name of a metadata
+context, even though the colon is not transmitted over the protocol.

 Namespaces MUST be consist of one of the following:
 - `base`, for metadata contexts defined by this document;
@@ -898,11 +899,11 @@ request to the mailing-list. The following additional third-party namespaces
 are currently registered:
 * (none)

-Save in respect of the `base:` namespace described below, this specification
+Save in respect of the `base` namespace described below, this specification
 requires no specific semantics of metadata contexts, except that all the
 information they provide MUST be representable within the flags field as
 defined for `NBD_REPLY_TYPE_BLOCK_STATUS`. Likewise, save in respect of
-the `base:` namespace, the syntax of query strings is not specified by this
+the `base` namespace, the syntax of query strings is not specified by this
 document, other than the recommendation that the empty leaf-name makes
 sense as a wildcard for a client query during `NBD_OPT_LIST_META_CONTEXT`,
 but SHOULD NOT select any contexts during `NBD_OPT_SET_META_CONTEXT`.
@@ -911,22 +912,22 @@ Server implementations SHOULD ensure the syntax for query strings they
 support and semantics for resulting metadata context is documented similarly
 to this document.

-### The `base:` metadata namespace
+### The `base` metadata namespace

-This standard defines exactly one metadata context; it is called
-`base:allocation`, and it provides information on the basic allocation
+This standard defines exactly one namespace, `base`, currently containing
+only one metadata context, `base:allocation`.  It provides information on the basic allocation
 status of extents (that is, whether they are allocated at all in a
 sparse file context).

-The query string within the `base:` metadata context can take
+The leaf-name portion of a query against the `base` namespace can take
 one of two forms:
-* `base:` - the server MUST ignore this form during
+* length 0 - the server MUST ignore this form during
   `NBD_OPT_SET_META_CONTEXT`, and MUST support this as a wildcard during
   `NBD_OPT_LIST_META_CONTEXT`, in which case the server's reply will
   contain a response for each supported metadata context within the
-  `base:` namespace (currently just `base:allocation`, although a
+  `base` namespace (currently just `base:allocation`, although a
   future revision of the standard might return multiple contexts); or
-* `base:[leaf-name]` to select `[leaf-name]` as a context leaf-name
+* non-empty string - select a context leaf-name
   that might exist within the `base` namespace.  If a `[leaf-name]`
   requested by the client is not recognized, the server MUST ignore
   it rather than report an error.
@@ -1297,15 +1298,15 @@ of the newstyle negotiation.
       contexts.
     - 32 bits, number of queries
     - Zero or more queries, each being:
-       - 32 bits, length of query.
-       - String, query to list a subset of the available metadata
-         contexts.  The syntax of this query is
-         implementation-defined, except that it MUST start with a
-         namespace and a colon.
+       - 16 bits, length of namespace (MUST be nonzero).
+       - String, namespace to select from.
+       - 16 bits, length of leaf-name (MAY be zero).
+       - String, leaf-name query. The syntax of this query is
+         implementation-defined according to the namespace.

-    For details on the query string, see the "Metadata querying"
+    For details on the namespace and query strings, see the "Metadata querying"
     section; note that a namespace may document that a different set
-    of queries are valid for `NBD_OPT_LIST_META_CONTEXT` than for
+    of leaf-name queries are valid for `NBD_OPT_LIST_META_CONTEXT` than for
     `NBD_OPT_SET_META_CONTEXT`, such as when using an empty
     leaf-name for wildcarding.

@@ -1313,8 +1314,7 @@ of the newstyle negotiation.
     length that would require reading beyond the original length given
     in the option header), the server MUST fail the request with
     `NBD_REP_ERR_INVALID`.  For requests that are semantically invalid
-    (such as lacking the required colon that delimits the namespace,
-    or using a leaf name that is invalid for a known namespace), the
+    (such as using a leaf name that is invalid for a known namespace), the
     server MAY fail the request with `NBD_REP_ERR_INVALID`.  However,
     the server MUST ignore query strings belonging to an unknown
     namespace.  If none of the query strings find any metadata
@@ -1359,16 +1359,15 @@ of the newstyle negotiation.
     'x-FooBar:') or with a range of values (e.g.
     'x-ModifiedDate:20160310-20161214'). The semantics of
     such a reply are a matter for the definition of the
-    namespace. However each namespace returned MUST begin
-    with the relevant namespace, followed by a colon, and then
-    other UTF-8 characters, with the entire string following the
+    namespace. However each returned context must include the
+    namespace, and the leaf-name must match
     restrictions for strings set out earlier in this
     document.

     The metadata context ID in these replies is reserved and SHOULD be
     set to zero; clients MUST disregard it.

-* `NBD_OPT_SET_META_CONTEXT` (10)
+* `NBD_OPT_SET_META_CONTEXT` (11)

     Change the set of active metadata contexts. Issuing this command
     replaces all previously-set metadata contexts (including when this
@@ -1393,10 +1392,11 @@ of the newstyle negotiation.
       contexts.
     - 32 bits, number of queries
     - Zero or more queries, each being:
-       - 32 bits, length of query
-       - String, query to select metadata contexts. The syntax of this
-         query is implementation-defined, except that it MUST start with a
-         namespace and a colon.
+       - 16 bits, length of namespace (MUST be nonzero).
+       - String, namespace to select from.
+       - 16 bits, length of leaf-name (SHOULD be nonzero).
+       - String, leaf-name query. The syntax of this query is
+         implementation-defined according to the namespace.

     If zero queries are sent, the server MUST select no metadata contexts.

@@ -1415,6 +1415,17 @@ of the newstyle negotiation.
     context, provided the client then does not attempt to issue
     `NBD_CMD_BLOCK_STATUS` commands.

+* Other requests
+
+    Some third-party implementations have experimented with additional
+    options not described in this document. In the interest of
+    interoperability, authors of such implementations SHOULD contact the
+    maintainer of this document, so that these messages can be listed here
+    to avoid conflicting implementations.
+
+    Currently one such message is known: `NBD_OPT_OLD_META_CONTEXT`, with
+    a value of 10, implemented by Virtuozzo.
+
 #### Option reply types

 These values are used in the "reply type" field, sent by the server
@@ -1584,7 +1595,10 @@ during option haggling in the fixed newstyle negotiation.
     A description of a metadata context. Data:

     - 32 bits, NBD metadata context ID.
-    - String, name of the metadata context. This is not required to be
+    - 16 bits, namespace length (MUST be nonzero)
+    - String, the namespace of the metadata context.
+    - String (length computed from length given in header minus 6 minus
+      the namespace length), the selected metadata leaf-name. This is not required to be
       a human-readable string, but it MUST be valid UTF-8 data.

 There are a number of error reply types, all of which are denoted by
-- 
2.14.3

Re: [Qemu-devel] Withdrawn: Tweak the NBD_OPT_SET_META_CONTEXT layout

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1280 bytes --]

On 03/31/2018 07:24 AM, Eric Blake wrote:
> As mentioned in earlier email threads, I experimented with an
> alternative layout to the structs sent across the wire during
> NBD_OPT_{LIST,SET}_META_CONTEXT, on the grounds that having the
> namespace and leaf-name combined into one string that requires further
> parsing on each end is not as nice as having two separate fields.  I'm
> replying to this mail with two RFC patches, one for the NBD spec, and
> one for the qemu implementation.
> 
> If we like the idea, this has to go into qemu 2.12, so we have less than
> a week.  Or, we can ignore the RFC, keep qemu the way it is currently
> coded, and when it is released, it will be time to promote the NBD
> extension-blockstatus to current as there will be an existing
> implementation to be interoperable with.

As I got no comments, and as qemu is now frozen hard at 2.12-rc3, it is
officially too late to change the format sent over the wire; qemu's
implementation will be the reference implementation, and for nbd.git,
I'll be promoting extension-blockstatus to the master branch once qemu
2.12 is officially released.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]