From patchwork Sat Dec 15 13:53:14 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 10732191 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 417DE13BF for ; Sat, 15 Dec 2018 13:57:25 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2D35E1FFF9 for ; Sat, 15 Dec 2018 13:57:25 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 213AA2955D; Sat, 15 Dec 2018 13:57:25 +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 AAD751FFF9 for ; Sat, 15 Dec 2018 13:57:24 +0000 (UTC) Received: from localhost ([::1]:39124 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gYARX-0006S2-Lq for patchwork-qemu-devel@patchwork.kernel.org; Sat, 15 Dec 2018 08:57:23 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34487) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gYAOF-0004qf-Na for qemu-devel@nongnu.org; Sat, 15 Dec 2018 08:54:00 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gYAOE-0008E2-Lc for qemu-devel@nongnu.org; Sat, 15 Dec 2018 08:53:59 -0500 Received: from mx1.redhat.com ([209.132.183.28]:35838) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gYAOC-0008By-OY; Sat, 15 Dec 2018 08:53:56 -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 1414780F6C; Sat, 15 Dec 2018 13:53:56 +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 C349F5C1B2; Sat, 15 Dec 2018 13:53:54 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Date: Sat, 15 Dec 2018 07:53:14 -0600 Message-Id: <20181215135324.152629-13-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.27]); Sat, 15 Dec 2018 13:53:56 +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 12/22] nbd/client: Improve error handling in nbd_negotiate_simple_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 Always allocate space for the reply returned by the server and hoist the trace earlier, as it is more interesting to trace the server's reply (even if it is unexpected) than parroting our request only on success. After all, skipping the allocation for a wrong size was merely a micro-optimization that only benefitted a broken server, rather than the common case of a compliant server that meets our expectations. Then turn the reply handling into a loop (even though we still never iterate more than once), to make this code easier to use when later patches do support multiple server replies. This changes the error message for a server with two replies (a corner case we are unlikely to hit in practice) from: Unexpected reply type 4 (meta context), expected 0 (ack) to: Server replied with more than one context Signed-off-by: Eric Blake --- v2: split patch into easier-to-review pieces [Rich, Vladimir] --- nbd/client.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/nbd/client.c b/nbd/client.c index bcccd5f555e..b6a85fc3ef8 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -684,10 +684,11 @@ static int nbd_negotiate_simple_meta_context(QIOChannel *ioc, return ret; } - if (reply.type == NBD_REP_META_CONTEXT) { + while (reply.type == NBD_REP_META_CONTEXT) { char *name; - if (reply.length != sizeof(info->context_id) + context_len) { + 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); @@ -708,6 +709,15 @@ static int nbd_negotiate_simple_meta_context(QIOChannel *ioc, 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); + nbd_send_opt_abort(ioc); + return -1; + } + if (strcmp(context, name)) { error_setg(errp, "Failed to negotiate meta context '%s', server " "answered with different context '%s'", context, @@ -717,8 +727,6 @@ static int nbd_negotiate_simple_meta_context(QIOChannel *ioc, return -1; } g_free(name); - - trace_nbd_opt_meta_reply(context, info->context_id); received = true; /* receive NBD_REP_ACK */