From patchwork Tue Feb 21 02:42:47 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 9583827 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 19151600C1 for ; Tue, 21 Feb 2017 02:52:57 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id EA90A28918 for ; Tue, 21 Feb 2017 02:52:56 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id DD8782891F; Tue, 21 Feb 2017 02:52:56 +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=-6.9 required=2.0 tests=BAYES_00,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 7422328918 for ; Tue, 21 Feb 2017 02:52:56 +0000 (UTC) Received: from localhost ([::1]:41928 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cg0ZT-0004lD-Jn for patchwork-qemu-devel@patchwork.kernel.org; Mon, 20 Feb 2017 21:52:55 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40800) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cg0Q1-0005ZV-Qp for qemu-devel@nongnu.org; Mon, 20 Feb 2017 21:43:12 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cg0Q0-0003WT-OO for qemu-devel@nongnu.org; Mon, 20 Feb 2017 21:43:09 -0500 Received: from mx1.redhat.com ([209.132.183.28]:43004) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cg0Px-0003Sh-IG; Mon, 20 Feb 2017 21:43:05 -0500 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 95BF7C04BD40; Tue, 21 Feb 2017 02:43:05 +0000 (UTC) Received: from red.redhat.com (ovpn-123-67.rdu2.redhat.com [10.10.123.67] (may be forged)) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v1L2gnuG023090; Mon, 20 Feb 2017 21:43:04 -0500 From: Eric Blake To: qemu-devel@nongnu.org Date: Mon, 20 Feb 2017 20:42:47 -0600 Message-Id: <20170221024248.11027-8-eblake@redhat.com> In-Reply-To: <20170221024248.11027-1-eblake@redhat.com> References: <20170221024248.11027-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Tue, 21 Feb 2017 02:43:05 +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 v4 7/8] nbd: Implement NBD_INFO_BLOCK_SIZE on server 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: pbonzini@redhat.com, vsementsov@virtuozzo.com, den@virtuozzo.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 The upstream NBD Protocol has defined a new extension to allow the server to advertise block sizes to the client, as well as a way for the client to inform the server that it intends to obey block sizes. Thanks to a recent fix (commit df7b97ff), our real minimum transfer size is always 1 (the block layer takes care of read-modify-write on our behalf), but we're still more efficient if we advertise 512 when the client supports it, as follows: - OPT_INFO, but no NBD_INFO_BLOCK_SIZE: advertise 512, then fail with NBD_REP_ERR_BLOCK_SIZE_REQD; client is free to try something else since we don't disconnect - OPT_INFO with NBD_INFO_BLOCK_SIZE: advertise 512 - OPT_GO, but no NBD_INFO_BLOCK_SIZE: advertise 1 - OPT_GO with NBD_INFO_BLOCK_SIZE: advertise 512 We can also advertise the optimum block size (presumably the cluster size, when exporting a qcow2 file), and our absolute maximum transfer size of 32M, to help newer clients avoid EINVAL failures or abrupt disconnects on oversize requests. We do not reject clients for using the older NBD_OPT_EXPORT_NAME; we are no worse off for those clients than we used to be. Signed-off-by: Eric Blake --- v4: new patch --- nbd/server.c | 47 +++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 43 insertions(+), 4 deletions(-) diff --git a/nbd/server.c b/nbd/server.c index 3b1a4a5..d63e5d3 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -409,6 +409,8 @@ static int nbd_negotiate_handle_info(NBDClient *client, uint32_t length, uint16_t request; uint32_t namelen; bool sendname = false; + bool blocksize = false; + uint32_t sizes[3]; char buf[sizeof(uint64_t) + sizeof(uint16_t)]; const char *msg; @@ -444,11 +446,16 @@ static int nbd_negotiate_handle_info(NBDClient *client, uint32_t length, length -= sizeof(request); TRACE("Client requested info %d (%s)", request, nbd_info_lookup(request)); - /* For now, we only care about NBD_INFO_NAME; everything else - * is either a request we don't know or something we send - * regardless of request. */ - if (request == NBD_INFO_NAME) { + /* We care about NBD_INFO_NAME and NBD_INFO_BLOCK_SIZE; + * everything else is either a request we don't know or + * something we send regardless of request */ + switch (request) { + case NBD_INFO_NAME: sendname = true; + break; + case NBD_INFO_BLOCK_SIZE: + blocksize = true; + break; } } @@ -499,6 +506,27 @@ static int nbd_negotiate_handle_info(NBDClient *client, uint32_t length, } } + /* Send NBD_INFO_BLOCK_SIZE always, but tweak the minimum size + * according to whether the client requested it, and according to + * whether this is OPT_INFO or OPT_GO. */ + /* minimum - 1 for back-compat, or 512 if client is new enough. + * TODO: consult blk_bs(blk)->request_align? */ + sizes[0] = (opt == NBD_OPT_INFO || blocksize) ? BDRV_SECTOR_SIZE : 1; + /* preferred - At least 4096, but larger as appropriate. */ + sizes[1] = MAX(blk_get_opt_transfer(exp->blk), 4096); + /* maximum - At most 32M, but smaller as appropriate. */ + sizes[2] = MIN(blk_get_max_transfer(exp->blk), NBD_MAX_BUFFER_SIZE); + TRACE("advertising minimum 0x%" PRIx32 ", preferred 0x%" PRIx32 + ", maximum 0x%" PRIx32, sizes[0], sizes[1], sizes[2]); + cpu_to_be32s(&sizes[0]); + cpu_to_be32s(&sizes[1]); + cpu_to_be32s(&sizes[2]); + rc = nbd_negotiate_send_info(client, opt, NBD_INFO_BLOCK_SIZE, + sizeof(sizes), sizes); + if (rc < 0) { + return rc; + } + /* Send NBD_INFO_EXPORT always */ TRACE("advertising size %" PRIu64 " and flags %" PRIx16, exp->size, exp->nbdflags | myflags); @@ -510,6 +538,17 @@ static int nbd_negotiate_handle_info(NBDClient *client, uint32_t length, return rc; } + /* If the client is just asking for NBD_OPT_INFO, but forgot to + * request block sizes, return an error. + * TODO: consult blk_bs(blk)->request_align, and only error if it + * is not 1? */ + if (opt == NBD_OPT_INFO && !blocksize) { + return nbd_negotiate_send_rep_err(client->ioc, + NBD_REP_ERR_BLOCK_SIZE_REQD, opt, + "request NBD_INFO_BLOCK_SIZE to " + "use this export"); + } + /* Final reply */ rc = nbd_negotiate_send_rep(client->ioc, NBD_REP_ACK, opt); if (rc < 0) {