From nobody Sun May 5 09:47:36 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (208.118.235.17 [208.118.235.17]) by mx.zohomail.com with SMTPS id 1522499319392283.23719480409795; Sat, 31 Mar 2018 05:28:39 -0700 (PDT) Received: from localhost ([::1]:45570 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1f2FcR-0006G5-Ma for importer@patchew.org; Sat, 31 Mar 2018 08:28:30 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51124) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1f2FbO-0005nD-2M for qemu-devel@nongnu.org; Sat, 31 Mar 2018 08:27:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1f2FbM-00010B-6k for qemu-devel@nongnu.org; Sat, 31 Mar 2018 08:27:22 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:55692 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1f2FbE-0000vj-SC; Sat, 31 Mar 2018 08:27:12 -0400 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 69133BD9E; Sat, 31 Mar 2018 12:27:12 +0000 (UTC) Received: from red.redhat.com (ovpn-120-139.rdu2.redhat.com [10.10.120.139]) by smtp.corp.redhat.com (Postfix) with ESMTP id 93EAAD7E13; Sat, 31 Mar 2018 12:27:11 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org, qemu-block@nongnu.org Date: Sat, 31 Mar 2018 07:27:08 -0500 Message-Id: <20180331122708.1954245-1-eblake@redhat.com> In-Reply-To: <815919ea-1087-9567-9b81-d445aeacb102@redhat.com> References: <815919ea-1087-9567-9b81-d445aeacb102@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.11.54.5 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Sat, 31 Mar 2018 12:27:12 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Sat, 31 Mar 2018 12:27:12 +0000 (UTC) for IP:'10.11.54.5' DOMAIN:'int-mx05.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'eblake@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH qemu] RFC: nbd: Separate namespace from leaf-name for metadata contexts X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , vsementsov@virtuozzo.com, Max Reitz , Paolo Bonzini , w@uter.be, nbd@other.debian.org Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" 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 --- 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 =3D NBD_REP_META_CONTEXT, h.length > 4 */ + NBDOptionReply h; /* h.type =3D 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 *i= oc, * Set one meta context. Simple means that reply must contain zero (not * negotiated) or one (negotiated) contexts. More contexts would be consid= ered * as a protocol error. It's also implied that meta-data query equals quer= ied - * context name, so, if server replies with something different than @cont= ext, - * 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(QIOChann= el *ioc, uint32_t received_id; bool received; uint32_t export_len =3D strlen(export); - uint32_t context_len =3D strlen(context); + uint16_t namespace_len =3D strlen(namespace); + uint16_t leaf_len =3D strlen(leaf); uint32_t data_len =3D 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 =3D g_malloc(data_len); char *p =3D data; - trace_nbd_opt_meta_request(context, export); + trace_nbd_opt_meta_request(namespace, leaf, export); stl_be_p(p, export_len); memcpy(p +=3D sizeof(export_len), export, export_len); stl_be_p(p +=3D export_len, 1); - stl_be_p(p +=3D sizeof(uint32_t), context_len); - memcpy(p +=3D sizeof(context_len), context, context_len); + stw_be_p(p +=3D sizeof(uint32_t), namespace_len); + memcpy(p +=3D sizeof(namespace_len), namespace, namespace_len); + stw_be_p(p +=3D namespace_len, leaf_len); + memcpy(p +=3D sizeof(leaf_len), leaf, leaf_len); ret =3D nbd_send_option_request(ioc, NBD_OPT_SET_META_CONTEXT, data_le= n, data, errp); @@ -650,14 +655,16 @@ static int nbd_negotiate_simple_meta_context(QIOChann= el *ioc, if (reply.type =3D=3D NBD_REP_META_CONTEXT) { char *name; - size_t len; + uint16_t len; - if (reply.length !=3D sizeof(received_id) + context_len) { - error_setg(errp, "Failed to negotiate meta context '%s', serve= r " - "answered with unexpected length %u", context, - reply.length); - nbd_send_opt_abort(ioc); - return -1; + if (reply.length !=3D 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(QIOChann= el *ioc, } be32_to_cpus(&received_id); - len =3D reply.length - sizeof(received_id); - name =3D 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 !=3D 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 =3D 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] =3D '\0'; - if (strcmp(context, name)) { - error_setg(errp, "Failed to negotiate meta context '%s', serve= r " - "answered with different context '%s'", context, + name[namespace_len + leaf_len] =3D '\0'; + if (strncmp(namespace, name, namespace_len) || + strcmp(leaf, name + namespace_len)) { + error_setg(errp, "Failed to negotiate meta context '%s:%s', se= rver " + "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(QIOChanne= l *ioc, } g_free(name); - trace_nbd_opt_meta_reply(context, received_id); + trace_nbd_opt_meta_reply(namespace, leaf, received_id); received =3D 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 =3D 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(NBDC= lient *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[] =3D { {.iov_base =3D &opt, .iov_len =3D sizeof(opt)}, - {.iov_base =3D (void *)context, .iov_len =3D strlen(context)} + {.iov_base =3D (void *)namespace, .iov_len =3D strlen(namespace)}, + {.iov_base =3D (void *)leaf, .iov_len =3D strlen(leaf)}, }; if (client->opt =3D=3D NBD_OPT_LIST_META_CONTEXT) { context_id =3D 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 *m= eta, - uint32_t len, Error **errp) + Error **errp) { int ret; - char query[sizeof("allocation") - 1]; - size_t alen =3D strlen("allocation"); + char query[] =3D "allocation"; + size_t alen =3D strlen(query); + uint16_t len; + + ret =3D nbd_opt_read(client, &len, sizeof(len), errp); + if (ret <=3D 0) { + return ret; + } + cpu_to_be16s(&len); if (len =3D=3D 0) { if (client->opt =3D=3D NBD_OPT_LIST_META_CONTEXT) { @@ -767,7 +777,7 @@ static int nbd_meta_base_query(NBDClient *client, NBDEx= portMetaContexts *meta, return ret; } - if (strncmp(query, "allocation", alen) =3D=3D 0) { + if (!strcmp(query, "allocation")) { meta->base_allocation =3D true; } @@ -791,34 +801,43 @@ static int nbd_negotiate_meta_query(NBDClient *client, NBDExportMetaContexts *meta, Error **e= rrp) { int ret; - char query[sizeof("base:") - 1]; - size_t baselen =3D strlen("base:"); - uint32_t len; + char query[] =3D "base"; + size_t baselen =3D strlen(query); + uint16_t len; ret =3D nbd_opt_read(client, &len, sizeof(len), errp); if (ret <=3D 0) { return ret; } - cpu_to_be32s(&len); + cpu_to_be16s(&len); - /* The only supported namespace for now is 'base'. So query should sta= rt - * 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 !=3D baselen) { + trace_nbd_negotiate_meta_query_skip("not for base: namespace"); + ret =3D nbd_opt_skip(client, len, errp); + if (ret <=3D 0) { + return ret; + } + goto skip_leaf; } - len -=3D baselen; - ret =3D nbd_opt_read(client, query, baselen, errp); + ret =3D nbd_opt_read(client, query, len, errp); if (ret <=3D 0) { return ret; } - if (strncmp(query, "base:", baselen) !=3D 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 =3D nbd_opt_read(client, &len, sizeof(len), errp); + if (ret <=3D 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 =3D nbd_negotiate_send_meta_context(client, "base:allocation", + ret =3D nbd_negotiate_send_meta_context(client, "base", "allocatio= n", 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) "Qu= erying export list for nbd_receive_query_exports_success(const char *wantname) "Found desired exp= ort 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 conte= xt %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) "Recei= ved mapping of context %s:%s to id %d" nbd_receive_negotiate(void *tlscreds, const char *hostname) "Receiving neg= otiation tlscreds=3D%p hostname=3D%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 T= LS handshake" nbd_negotiate_meta_context(const char *optname, const char *export, int qu= eries) "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) "Repl= ying with meta context '%s' id %d" +nbd_negotiate_meta_query_reply(const char *namespace, const char *leaf, un= signed int id) "Replying with meta context '%s:%s' id %d" nbd_negotiate_options_flags(uint32_t flags) "Received client flags 0x%" PR= Ix32 nbd_negotiate_options_check_magic(uint64_t magic) "Checking opts magic 0x%= " PRIx64 nbd_negotiate_options_check_option(uint32_t option, const char *name) "Che= cking option %" PRIu32 " (%s)" --=20 2.14.3