From patchwork Sat Dec 15 13:53:16 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 10732207 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id C7E6513BF for ; Sat, 15 Dec 2018 14:03:57 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B1F282A13E for ; Sat, 15 Dec 2018 14:03:57 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 9FE702A21D; Sat, 15 Dec 2018 14:03:57 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id E06882A13E for ; Sat, 15 Dec 2018 14:03:56 +0000 (UTC) Received: from localhost ([::1]:39168 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gYAXr-0003Y2-Mg for patchwork-qemu-devel@patchwork.kernel.org; Sat, 15 Dec 2018 09:03:55 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34547) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gYAOM-0004ya-4k for qemu-devel@nongnu.org; Sat, 15 Dec 2018 08:54:07 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gYAOK-0008Hr-Ss for qemu-devel@nongnu.org; Sat, 15 Dec 2018 08:54:06 -0500 Received: from mx1.redhat.com ([209.132.183.28]:36536) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gYAOF-0008ES-Sh; Sat, 15 Dec 2018 08:54:00 -0500 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 377CBC0495BA; Sat, 15 Dec 2018 13:53:59 +0000 (UTC) Received: from red.redhat.com (ovpn-122-76.rdu2.redhat.com [10.10.122.76]) by smtp.corp.redhat.com (Postfix) with ESMTP id C86905C26A; Sat, 15 Dec 2018 13:53:57 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Date: Sat, 15 Dec 2018 07:53:16 -0600 Message-Id: <20181215135324.152629-15-eblake@redhat.com> In-Reply-To: <20181215135324.152629-1-eblake@redhat.com> References: <20181215135324.152629-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Sat, 15 Dec 2018 13:53:59 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v2 14/22] nbd/client: Split out nbd_receive_one_meta_context() 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: nsoffer@redhat.com, vsementsov@virtuozzo.com, jsnow@redhat.com, rjones@redhat.com, qemu-block@nongnu.org Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP Refactor nbd_negotiate_simple_meta_context() to more closely resemble the pattern of nbd_receive_list(), separating the argument validation for one pass from the caller making a loop over passes. No major semantic change (although one error message loses the original query). The diff may be a bit hard to follow due to indentation changes and handling ACK first rather than last. Signed-off-by: Eric Blake Reviewed-by: Richard W.M. Jones --- v2: split patch into easier-to-review pieces [Rich, Vladimir] --- nbd/client.c | 144 +++++++++++++++++++++++++++-------------------- nbd/trace-events | 2 +- 2 files changed, 84 insertions(+), 62 deletions(-) diff --git a/nbd/client.c b/nbd/client.c index 5b6b9964097..0e5a9d59dbd 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -669,6 +669,83 @@ static int nbd_send_one_meta_context(QIOChannel *ioc, return ret; } +/* nbd_receive_one_meta_context: + * Called in a loop to receive and trace one set/list meta context reply. + * Pass non-NULL @name or @id to collect results back to the caller, which + * must eventually call g_free(). + * return 1 if more contexts are expected, + * 0 if operation is complete, + * -1 with errp set for any error + */ +static int nbd_receive_one_meta_context(QIOChannel *ioc, + uint32_t opt, + char **name, + uint32_t *id, + Error **errp) +{ + int ret; + NBDOptionReply reply; + char *local_name = NULL; + uint32_t local_id; + + if (nbd_receive_option_reply(ioc, opt, &reply, errp) < 0) { + return -1; + } + + ret = nbd_handle_reply_err(ioc, &reply, errp); + if (ret <= 0) { + return ret; + } + + if (reply.type == NBD_REP_ACK) { + if (reply.length != 0) { + error_setg(errp, "Unexpected length to ACK response"); + nbd_send_opt_abort(ioc); + return -1; + } + return 0; + } else if (reply.type != NBD_REP_META_CONTEXT) { + error_setg(errp, "Unexpected reply type %u (%s), expected %u (%s)", + reply.type, nbd_rep_lookup(reply.type), + NBD_REP_META_CONTEXT, nbd_rep_lookup(NBD_REP_META_CONTEXT)); + nbd_send_opt_abort(ioc); + return -1; + } + + if (reply.length <= sizeof(local_id) || + reply.length > NBD_MAX_BUFFER_SIZE) { + error_setg(errp, "Failed to negotiate meta context, server " + "answered with unexpected length %" PRIu32, + reply.length); + nbd_send_opt_abort(ioc); + return -1; + } + + if (nbd_read(ioc, &local_id, sizeof(local_id), errp) < 0) { + return -1; + } + local_id = be32_to_cpu(local_id); + + reply.length -= sizeof(local_id); + local_name = g_malloc(reply.length + 1); + if (nbd_read(ioc, local_name, reply.length, errp) < 0) { + g_free(local_name); + return -1; + } + local_name[reply.length] = '\0'; + trace_nbd_opt_meta_reply(nbd_opt_lookup(opt), local_name, local_id); + + if (name) { + *name = local_name; + } else { + g_free(local_name); + } + if (id) { + *id = local_id; + } + return 1; +} + /* nbd_negotiate_simple_meta_context: * Request the server to set the meta context for export @info->name * using @info->x_dirty_bitmap with a fallback to "base:allocation", @@ -690,7 +767,6 @@ static int nbd_negotiate_simple_meta_context(QIOChannel *ioc, * function should lose the term _simple. */ int ret; - NBDOptionReply reply; const char *context = info->x_dirty_bitmap ?: "base:allocation"; bool received = false; @@ -699,44 +775,17 @@ static int nbd_negotiate_simple_meta_context(QIOChannel *ioc, return -1; } - if (nbd_receive_option_reply(ioc, NBD_OPT_SET_META_CONTEXT, &reply, - errp) < 0) - { - return -1; - } - - ret = nbd_handle_reply_err(ioc, &reply, errp); - if (ret <= 0) { - return ret; - } - - while (reply.type == NBD_REP_META_CONTEXT) { + while (1) { char *name; - if (reply.length <= sizeof(info->context_id) || - reply.length > NBD_MAX_BUFFER_SIZE) { - error_setg(errp, "Failed to negotiate meta context '%s', server " - "answered with unexpected length %" PRIu32, context, - reply.length); - nbd_send_opt_abort(ioc); + ret = nbd_receive_one_meta_context(ioc, NBD_OPT_SET_META_CONTEXT, + &name, &info->context_id, errp); + if (ret < 0) { return -1; + } else if (ret == 0) { + return received; } - if (nbd_read(ioc, &info->context_id, sizeof(info->context_id), - errp) < 0) { - return -1; - } - info->context_id = be32_to_cpu(info->context_id); - - reply.length -= sizeof(info->context_id); - name = g_malloc(reply.length + 1); - if (nbd_read(ioc, name, reply.length, errp) < 0) { - g_free(name); - return -1; - } - name[reply.length] = '\0'; - trace_nbd_opt_meta_reply(name, info->context_id); - if (received) { error_setg(errp, "Server replied with more than one context"); g_free(name); @@ -754,34 +803,7 @@ static int nbd_negotiate_simple_meta_context(QIOChannel *ioc, } g_free(name); received = true; - - /* receive NBD_REP_ACK */ - if (nbd_receive_option_reply(ioc, NBD_OPT_SET_META_CONTEXT, &reply, - errp) < 0) - { - return -1; - } - - ret = nbd_handle_reply_err(ioc, &reply, errp); - if (ret <= 0) { - return ret; - } } - - if (reply.type != NBD_REP_ACK) { - error_setg(errp, "Unexpected reply type %u (%s), expected %u (%s)", - reply.type, nbd_rep_lookup(reply.type), - NBD_REP_ACK, nbd_rep_lookup(NBD_REP_ACK)); - nbd_send_opt_abort(ioc); - return -1; - } - if (reply.length) { - error_setg(errp, "Unexpected length to ACK response"); - nbd_send_opt_abort(ioc); - return -1; - } - - return received; } int nbd_receive_negotiate(QIOChannel *ioc, QCryptoTLSCreds *tlscreds, diff --git a/nbd/trace-events b/nbd/trace-events index 00872a6f9d4..02956c96042 100644 --- a/nbd/trace-events +++ b/nbd/trace-events @@ -12,7 +12,7 @@ nbd_receive_query_exports_success(const char *wantname) "Found desired export na nbd_receive_starttls_new_client(void) "Setting up TLS" nbd_receive_starttls_tls_handshake(void) "Starting TLS handshake" nbd_opt_meta_request(const char *optname, const char *context, const char *export) "Requesting %s %s for export %s" -nbd_opt_meta_reply(const char *context, uint32_t id) "Received mapping of context %s to id %" PRIu32 +nbd_opt_meta_reply(const char *optname, const char *context, uint32_t id) "Received %s mapping of %s to id %" PRIu32 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