From patchwork Thu Mar 31 21:20:20 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 8718681 Return-Path: X-Original-To: patchwork-qemu-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 257959F36E for ; Thu, 31 Mar 2016 21:24:16 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 0081F20303 for ; Thu, 31 Mar 2016 21:24:15 +0000 (UTC) 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.kernel.org (Postfix) with ESMTPS id B0735202F0 for ; Thu, 31 Mar 2016 21:24:13 +0000 (UTC) Received: from localhost ([::1]:34500 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1alk4b-0003tI-3x for patchwork-qemu-devel@patchwork.kernel.org; Thu, 31 Mar 2016 17:24:13 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40754) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1alk3C-0001kN-U9 for qemu-devel@nongnu.org; Thu, 31 Mar 2016 17:22:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1alk39-0000bI-Ky for qemu-devel@nongnu.org; Thu, 31 Mar 2016 17:22:46 -0400 Received: from resqmta-po-04v.sys.comcast.net ([96.114.154.163]:46227) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1alk39-0000b3-E3 for qemu-devel@nongnu.org; Thu, 31 Mar 2016 17:22:43 -0400 Received: from resomta-po-09v.sys.comcast.net ([96.114.154.233]) by comcast with SMTP id lk0qaMOS7IY3Mlk1Ca2VOd; Thu, 31 Mar 2016 21:20:42 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=comcast.net; s=q20140121; t=1459459242; bh=cmNO98wNoHUqlFLl+qlY57D7qGi7V2QAQ8gSIBkQkOA=; h=Received:Received:From:To:Subject:Date:Message-Id; b=NWjXBtlwmefa8mgihA1IZmSrlkZoKA41L6ACjQ/k2JjfceMZLNk/rvAUlcpernVsv q0FL/5GkjnVkgM/xxVQ7WLc/5hsx2YTm3w4GrLXxJXgg0DddRuQpDMVE8eT02Sx2Ij 2z0wSFWY3VhbH1qBhDurSFNQ8YSWnsgxq9OzB0QPTPaxSRXhqAMjjR65dCshhoiGnJ VKNGJLWGGpodc/T00Ssu9hFFiSfJjdf/agukNLw8xFkvXJP5/2+SEpQvHEQHjl/789 T0y2JHjAVP58sTXuPWch1FqwIGU9IY/tS+em1IvjsBlKYuzqJpdI9dNYQgwbn8JeZY YSMuzjSkE1vSA== Received: from red.redhat.com ([24.10.254.122]) by resomta-po-09v.sys.comcast.net with comcast id clLN1s00K2fD5rL01lLh0Z; Thu, 31 Mar 2016 21:20:42 +0000 From: Eric Blake To: qemu-devel@nongnu.org Date: Thu, 31 Mar 2016 15:20:20 -0600 Message-Id: <1459459222-8637-2-git-send-email-eblake@redhat.com> X-Mailer: git-send-email 2.5.5 In-Reply-To: <1459459222-8637-1-git-send-email-eblake@redhat.com> References: <1459459222-8637-1-git-send-email-eblake@redhat.com> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 96.114.154.163 Cc: Kevin Wolf , Paolo Bonzini , "open list:Block layer core" Subject: [Qemu-devel] [PATCH 1/3] nbd: Treat flags vs. command type as separate fields X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Current upstream NBD documents that requests have a 16-bit flags, followed by a 16-bit type integer; although older versions mentioned only a 32-bit field with masking to find flags. Since the protocol is in network order (big-endian over the wire), the ABI is unchanged; but dealing with the flags as a separate field rather than masking will make it easier to add support for upcoming NBD extensions that improve the efficiency of sparse file handling (since those extensions will increase the number of both flags and commands). Improve some comments in nbd.h based on the current upstream NBD protocol (https://github.com/yoe/nbd/blob/master/doc/proto.md), and touch some nearby code to keep checkpatch.pl happy. Signed-off-by: Eric Blake --- include/block/nbd.h | 17 +++++++++++------ block/nbd-client.c | 11 ++++------- nbd/client.c | 17 ++++++++++------- nbd/server.c | 28 ++++++++++++++++------------ 4 files changed, 41 insertions(+), 32 deletions(-) diff --git a/include/block/nbd.h b/include/block/nbd.h index b86a976..f018968 100644 --- a/include/block/nbd.h +++ b/include/block/nbd.h @@ -27,7 +27,8 @@ struct nbd_request { uint32_t magic; - uint32_t type; + uint16_t flags; + uint16_t type; uint64_t handle; uint64_t from; uint32_t len; @@ -39,6 +40,8 @@ struct nbd_reply { uint64_t handle; } QEMU_PACKED; +/* Transmission (export) flags: sent from server to client during handshake, + but describe what will happen during transmission */ #define NBD_FLAG_HAS_FLAGS (1 << 0) /* Flags are there */ #define NBD_FLAG_READ_ONLY (1 << 1) /* Device is read-only */ #define NBD_FLAG_SEND_FLUSH (1 << 2) /* Send FLUSH */ @@ -46,10 +49,12 @@ struct nbd_reply { #define NBD_FLAG_ROTATIONAL (1 << 4) /* Use elevator algorithm - rotational media */ #define NBD_FLAG_SEND_TRIM (1 << 5) /* Send TRIM (discard) */ -/* New-style global flags. */ +/* New-style handshake (global) flags, sent from server to client, and + control what will happen during handshake phase. */ #define NBD_FLAG_FIXED_NEWSTYLE (1 << 0) /* Fixed newstyle protocol. */ -/* New-style client flags. */ +/* New-style client flags, sent from client to server to control what happens + during handshake phase. */ #define NBD_FLAG_C_FIXED_NEWSTYLE (1 << 0) /* Fixed newstyle protocol. */ /* Reply types. */ @@ -60,10 +65,10 @@ struct nbd_reply { #define NBD_REP_ERR_INVALID ((UINT32_C(1) << 31) | 3) /* Invalid length. */ #define NBD_REP_ERR_TLS_REQD ((UINT32_C(1) << 31) | 5) /* TLS required */ +/* Request flags, sent from client to server during transmission phase */ +#define NBD_CMD_FLAG_FUA (1 << 0) -#define NBD_CMD_MASK_COMMAND 0x0000ffff -#define NBD_CMD_FLAG_FUA (1 << 16) - +/* Supported request types */ enum { NBD_CMD_READ = 0, NBD_CMD_WRITE = 1, diff --git a/block/nbd-client.c b/block/nbd-client.c index 021a88b..8645be4 100644 --- a/block/nbd-client.c +++ b/block/nbd-client.c @@ -252,7 +252,7 @@ static int nbd_co_writev_1(BlockDriverState *bs, int64_t sector_num, if ((*flags & BDRV_REQ_FUA) && (client->nbdflags & NBD_FLAG_SEND_FUA)) { *flags &= ~BDRV_REQ_FUA; - request.type |= NBD_CMD_FLAG_FUA; + request.flags |= NBD_CMD_FLAG_FUA; } request.from = sector_num * 512; @@ -319,8 +319,9 @@ int nbd_client_co_flush(BlockDriverState *bs) return 0; } + /* XXX: NBD Protocol only documents use of FUA with WRITE */ if (client->nbdflags & NBD_FLAG_SEND_FUA) { - request.type |= NBD_CMD_FLAG_FUA; + request.flags |= NBD_CMD_FLAG_FUA; } request.from = 0; @@ -380,11 +381,7 @@ void nbd_client_attach_aio_context(BlockDriverState *bs, void nbd_client_close(BlockDriverState *bs) { NbdClientSession *client = nbd_get_client_session(bs); - struct nbd_request request = { - .type = NBD_CMD_DISC, - .from = 0, - .len = 0 - }; + struct nbd_request request = { .type = NBD_CMD_DISC }; if (client->ioc == NULL) { return; diff --git a/nbd/client.c b/nbd/client.c index f89c0a1..5af9f26 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -628,15 +628,18 @@ ssize_t nbd_send_request(QIOChannel *ioc, struct nbd_request *request) uint8_t buf[NBD_REQUEST_SIZE]; ssize_t ret; - cpu_to_be32w((uint32_t*)buf, NBD_REQUEST_MAGIC); - cpu_to_be32w((uint32_t*)(buf + 4), request->type); - cpu_to_be64w((uint64_t*)(buf + 8), request->handle); - cpu_to_be64w((uint64_t*)(buf + 16), request->from); - cpu_to_be32w((uint32_t*)(buf + 24), request->len); + cpu_to_be32w((uint32_t *)buf, NBD_REQUEST_MAGIC); + cpu_to_be16w((uint16_t *)(buf + 4), request->flags); + cpu_to_be16w((uint16_t *)(buf + 6), request->type); + cpu_to_be64w((uint64_t *)(buf + 8), request->handle); + cpu_to_be64w((uint64_t *)(buf + 16), request->from); + cpu_to_be32w((uint32_t *)(buf + 24), request->len); TRACE("Sending request to client: " - "{ .from = %" PRIu64", .len = %u, .handle = %" PRIu64", .type=%i}", - request->from, request->len, request->handle, request->type); + "{ .from = %" PRIu64", .len = %u, .handle = %" PRIu64 + ", .flags=%x, .type=%i}", + request->from, request->len, request->handle, request->flags, + request->type); ret = write_sync(ioc, buf, sizeof(buf)); if (ret < 0) { diff --git a/nbd/server.c b/nbd/server.c index b95571b..a590773 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -625,21 +625,24 @@ static ssize_t nbd_receive_request(QIOChannel *ioc, struct nbd_request *request) /* Request [ 0 .. 3] magic (NBD_REQUEST_MAGIC) - [ 4 .. 7] type (0 == READ, 1 == WRITE) + [ 4 .. 5] flags (NBD_CMD_FLAG_FUA, ...) + [ 6 .. 7] type (NBD_CMD_READ, ...) [ 8 .. 15] handle [16 .. 23] from [24 .. 27] len */ - magic = be32_to_cpup((uint32_t*)buf); - request->type = be32_to_cpup((uint32_t*)(buf + 4)); - request->handle = be64_to_cpup((uint64_t*)(buf + 8)); - request->from = be64_to_cpup((uint64_t*)(buf + 16)); - request->len = be32_to_cpup((uint32_t*)(buf + 24)); + magic = be32_to_cpup((uint32_t *)buf); + request->flags = be16_to_cpup((uint16_t *)(buf + 4)); + request->type = be16_to_cpup((uint16_t *)(buf + 6)); + request->handle = be64_to_cpup((uint64_t *)(buf + 8)); + request->from = be64_to_cpup((uint64_t *)(buf + 16)); + request->len = be32_to_cpup((uint32_t *)(buf + 24)); TRACE("Got request: " - "{ magic = 0x%x, .type = %d, from = %" PRIu64" , len = %u }", - magic, request->type, request->from, request->len); + "{ magic = 0x%x, .flags = %x, .type = %d, from = %" PRIu64 + " , len = %u }", + magic, request->flags, request->type, request->from, request->len); if (magic != NBD_REQUEST_MAGIC) { LOG("invalid magic (got 0x%x)", magic); @@ -980,7 +983,7 @@ static ssize_t nbd_co_receive_request(NBDRequest *req, struct nbd_request *reque TRACE("Decoding type"); - command = request->type & NBD_CMD_MASK_COMMAND; + command = request->type; if (command == NBD_CMD_READ || command == NBD_CMD_WRITE) { if (request->len > NBD_MAX_BUFFER_SIZE) { LOG("len (%u) is larger than max len (%u)", @@ -1044,7 +1047,7 @@ static void nbd_trip(void *opaque) reply.error = -ret; goto error_reply; } - command = request.type & NBD_CMD_MASK_COMMAND; + command = request.type; if (command != NBD_CMD_DISC && (request.from + request.len) > exp->size) { LOG("From: %" PRIu64 ", Len: %u, Size: %" PRIu64 ", Offset: %" PRIu64 "\n", @@ -1066,7 +1069,8 @@ static void nbd_trip(void *opaque) case NBD_CMD_READ: TRACE("Request type is READ"); - if (request.type & NBD_CMD_FLAG_FUA) { + /* XXX: NBD Protocol only documents use of FUA with WRITE */ + if (request.flags & NBD_CMD_FLAG_FUA) { ret = blk_co_flush(exp->blk); if (ret < 0) { LOG("flush failed"); @@ -1108,7 +1112,7 @@ static void nbd_trip(void *opaque) goto error_reply; } - if (request.type & NBD_CMD_FLAG_FUA) { + if (request.flags & NBD_CMD_FLAG_FUA) { ret = blk_co_flush(exp->blk); if (ret < 0) { LOG("flush failed");