From patchwork Fri Apr 22 23:40:40 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 8916571 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 7F34D9F1C1 for ; Sat, 23 Apr 2016 00:03:05 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 4DB472020F for ; Sat, 23 Apr 2016 00:03:04 +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 DB978201C7 for ; Sat, 23 Apr 2016 00:03:02 +0000 (UTC) Received: from localhost ([::1]:42015 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1atl2L-0007ef-Vu for patchwork-qemu-devel@patchwork.kernel.org; Fri, 22 Apr 2016 20:03:02 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45571) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1atkho-0004Eu-C6 for qemu-devel@nongnu.org; Fri, 22 Apr 2016 19:41:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1atkhm-0005XW-FY for qemu-devel@nongnu.org; Fri, 22 Apr 2016 19:41:48 -0400 Received: from mx1.redhat.com ([209.132.183.28]:51503) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1atkhh-0005Sw-6r; Fri, 22 Apr 2016 19:41:41 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (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 C44A8C049D58; Fri, 22 Apr 2016 23:41:40 +0000 (UTC) Received: from red.redhat.com (ovpn-113-21.phx2.redhat.com [10.3.113.21]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u3MNfHXL028475; Fri, 22 Apr 2016 19:41:40 -0400 From: Eric Blake To: qemu-devel@nongnu.org Date: Fri, 22 Apr 2016 17:40:40 -0600 Message-Id: <1461368452-10389-33-git-send-email-eblake@redhat.com> In-Reply-To: <1461368452-10389-1-git-send-email-eblake@redhat.com> References: <1461368452-10389-1-git-send-email-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v3 32/44] nbd: Share common option-sending code in client 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 , Paolo Bonzini , alex@alex.org.uk, qemu-block@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, UNPARSEABLE_RELAY autolearn=unavailable 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 Rather than open-coding each option request, it's easier to have common helper functions do the work. That in turn requires having convenient packed types for handling option requests and replies. Signed-off-by: Eric Blake Reviewed-by: Alex Bligh --- v3: rebase, tweak a debug message --- include/block/nbd.h | 29 +++++- nbd/nbd-internal.h | 2 +- nbd/client.c | 250 ++++++++++++++++++++++------------------------------ 3 files changed, 129 insertions(+), 152 deletions(-) diff --git a/include/block/nbd.h b/include/block/nbd.h index f4ae86c..5227ec6 100644 --- a/include/block/nbd.h +++ b/include/block/nbd.h @@ -26,20 +26,41 @@ #include "io/channel-socket.h" #include "crypto/tlscreds.h" +/* Handshake phase structs */ + +struct nbd_option { + uint64_t magic; /* NBD_OPTS_MAGIC */ + uint32_t option; /* NBD_OPT_* */ + uint32_t length; +} QEMU_PACKED; +typedef struct nbd_option nbd_option; + +struct nbd_opt_reply { + uint64_t magic; /* NBD_REP_MAGIC */ + uint32_t option; /* NBD_OPT_* */ + uint32_t type; /* NBD_REP_* */ + uint32_t length; +} QEMU_PACKED; +typedef struct nbd_opt_reply nbd_opt_reply; + +/* Transmission phase structs */ + struct nbd_request { - uint32_t magic; - uint16_t flags; - uint16_t type; + uint32_t magic; /* NBD_REQUEST_MAGIC */ + uint16_t flags; /* NBD_CMD_FLAG_* */ + uint16_t type; /* NBD_CMD_* */ uint64_t handle; uint64_t from; uint32_t len; } QEMU_PACKED; +typedef struct nbd_request nbd_request; struct nbd_reply { - uint32_t magic; + uint32_t magic; /* NBD_REPLY_MAGIC */ uint32_t error; uint64_t handle; } QEMU_PACKED; +typedef struct nbd_reply nbd_reply; /* Transmission (export) flags: sent from server to client during handshake, but describe what will happen during transmission */ diff --git a/nbd/nbd-internal.h b/nbd/nbd-internal.h index d0108a1..95069db 100644 --- a/nbd/nbd-internal.h +++ b/nbd/nbd-internal.h @@ -61,7 +61,7 @@ #define NBD_REPLY_MAGIC 0x67446698 #define NBD_OPTS_MAGIC 0x49484156454F5054LL #define NBD_CLIENT_MAGIC 0x0000420281861253LL -#define NBD_REP_MAGIC 0x3e889045565a9LL +#define NBD_REP_MAGIC 0x0003e889045565a9LL #define NBD_SET_SOCK _IO(0xab, 0) #define NBD_SET_BLKSIZE _IO(0xab, 1) diff --git a/nbd/client.c b/nbd/client.c index a9e173a..6626fa8 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -75,64 +75,123 @@ static QTAILQ_HEAD(, NBDExport) exports = QTAILQ_HEAD_INITIALIZER(exports); */ +/* Send an option request. Return 0 if successful, -1 with errp set if + * it is impossible to continue. */ +static int nbd_send_option_request(QIOChannel *ioc, uint32_t opt, + uint32_t len, const char *data, + Error **errp) +{ + nbd_option req; + QEMU_BUILD_BUG_ON(sizeof(req) != 16); -/* If type represents success, return 1 without further action. - * If type represents an error reply, consume the rest of the packet on ioc. - * Then return 0 for unsupported (so the client can fall back to - * other approaches), or -1 with errp set for other errors. + if (len == -1) { + req.length = len = strlen(data); + } + TRACE("Sending option request %"PRIu32", len %"PRIu32, opt, len); + + stq_be_p(&req.magic, NBD_OPTS_MAGIC); + stl_be_p(&req.option, opt); + stl_be_p(&req.length, len); + + if (write_sync(ioc, &req, sizeof(req)) != sizeof(req)) { + error_setg(errp, "Failed to send option request header"); + return -1; + } + + if (len && write_sync(ioc, (char *) data, len) != len) { + error_setg(errp, "Failed to send option request data"); + return -1; + } + + return 0; +} + +/* Receive the header of an option reply, which should match the given + * opt. Read through the length field, but NOT the length bytes of + * payload. Return 0 if successful, -1 with errp set if it is + * impossible to continue. */ +static int nbd_receive_option_reply(QIOChannel *ioc, uint32_t opt, + nbd_opt_reply *reply, Error **errp) +{ + QEMU_BUILD_BUG_ON(sizeof(*reply) != 20); + if (read_sync(ioc, reply, sizeof(*reply)) != sizeof(*reply)) { + error_setg(errp, "failed to read option reply"); + return -1; + } + be64_to_cpus(&reply->magic); + be32_to_cpus(&reply->option); + be32_to_cpus(&reply->type); + be32_to_cpus(&reply->length); + + TRACE("Received option reply %"PRIx32", type %"PRIx32", len %"PRIu32, + reply->option, reply->type, reply->length); + + if (reply->magic != NBD_REP_MAGIC) { + error_setg(errp, "Unexpected option reply magic"); + return -1; + } + if (reply->option != opt) { + error_setg(errp, "Unexpected option type %x expected %x", + reply->option, opt); + return -1; + } + return 0; +} + +/* If reply represents success, return 1 without further action. + * If reply represents an error, consume the optional payload of + * the packet on ioc. Then return 0 for unsupported (so the client + * can fall back to other approaches), or -1 with errp set for other + * errors. */ -static int nbd_handle_reply_err(QIOChannel *ioc, uint32_t opt, uint32_t type, +static int nbd_handle_reply_err(QIOChannel *ioc, nbd_opt_reply *reply, Error **errp) { - uint32_t len; char *msg = NULL; int result = -1; - if (!(type & (1 << 31))) { + if (!(reply->type & (1 << 31))) { return 1; } - if (read_sync(ioc, &len, sizeof(len)) != sizeof(len)) { - error_setg(errp, "failed to read option length"); - return -1; - } - len = be32_to_cpu(len); - if (len) { - if (len > NBD_MAX_BUFFER_SIZE) { + if (reply->length) { + if (reply->length > NBD_MAX_BUFFER_SIZE) { error_setg(errp, "server's error message is too long"); goto cleanup; } - msg = g_malloc(len + 1); - if (read_sync(ioc, msg, len) != len) { + msg = g_malloc(reply->length + 1); + if (read_sync(ioc, msg, reply->length) != reply->length) { error_setg(errp, "failed to read option error message"); goto cleanup; } - msg[len] = '\0'; + msg[reply->length] = '\0'; } - switch (type) { + switch (reply->type) { case NBD_REP_ERR_UNSUP: TRACE("server doesn't understand request %" PRIx32 - ", attempting fallback", opt); + ", attempting fallback", reply->option); result = 0; goto cleanup; case NBD_REP_ERR_POLICY: - error_setg(errp, "Denied by server for option %" PRIx32, opt); + error_setg(errp, "Denied by server for option %" PRIx32, + reply->option); break; case NBD_REP_ERR_INVALID: - error_setg(errp, "Invalid data length for option %" PRIx32, opt); + error_setg(errp, "Invalid data length for option %" PRIx32, + reply->option); break; case NBD_REP_ERR_TLS_REQD: error_setg(errp, "TLS negotiation required before option %" PRIx32, - opt); + reply->option); break; default: error_setg(errp, "Unknown error code when asking for option %" PRIx32, - opt); + reply->option); break; } @@ -147,58 +206,29 @@ static int nbd_handle_reply_err(QIOChannel *ioc, uint32_t opt, uint32_t type, static int nbd_receive_list(QIOChannel *ioc, char **name, Error **errp) { - uint64_t magic; - uint32_t opt; - uint32_t type; + nbd_opt_reply reply; uint32_t len; uint32_t namelen; int error; *name = NULL; - if (read_sync(ioc, &magic, sizeof(magic)) != sizeof(magic)) { - error_setg(errp, "failed to read list option magic"); + if (nbd_receive_option_reply(ioc, NBD_OPT_LIST, &reply, errp) < 0) { return -1; } - magic = be64_to_cpu(magic); - if (magic != NBD_REP_MAGIC) { - error_setg(errp, "Unexpected option list magic"); - return -1; - } - if (read_sync(ioc, &opt, sizeof(opt)) != sizeof(opt)) { - error_setg(errp, "failed to read list option"); - return -1; - } - opt = be32_to_cpu(opt); - if (opt != NBD_OPT_LIST) { - error_setg(errp, "Unexpected option type %" PRIx32 " expected %x", - opt, NBD_OPT_LIST); - return -1; - } - - if (read_sync(ioc, &type, sizeof(type)) != sizeof(type)) { - error_setg(errp, "failed to read list option type"); - return -1; - } - type = be32_to_cpu(type); - error = nbd_handle_reply_err(ioc, opt, type, errp); + error = nbd_handle_reply_err(ioc, &reply, errp); if (error <= 0) { return error; } + len = reply.length; - if (read_sync(ioc, &len, sizeof(len)) != sizeof(len)) { - error_setg(errp, "failed to read option length"); - return -1; - } - len = be32_to_cpu(len); - - if (type == NBD_REP_ACK) { + if (reply.type == NBD_REP_ACK) { if (len != 0) { error_setg(errp, "length too long for option end"); return -1; } - } else if (type == NBD_REP_SERVER) { + } else if (reply.type == NBD_REP_SERVER) { if (len < sizeof(namelen) || len > NBD_MAX_BUFFER_SIZE) { - error_setg(errp, "incorrect option length"); + error_setg(errp, "incorrect option length %"PRIu32, len); return -1; } if (read_sync(ioc, &namelen, sizeof(namelen)) != sizeof(namelen)) { @@ -240,7 +270,7 @@ static int nbd_receive_list(QIOChannel *ioc, char **name, Error **errp) } } else { error_setg(errp, "Unexpected reply type %" PRIx32 " expected %x", - type, NBD_REP_SERVER); + reply.type, NBD_REP_SERVER); return -1; } return 1; @@ -251,24 +281,10 @@ static int nbd_receive_query_exports(QIOChannel *ioc, const char *wantname, Error **errp) { - uint64_t magic = cpu_to_be64(NBD_OPTS_MAGIC); - uint32_t opt = cpu_to_be32(NBD_OPT_LIST); - uint32_t length = 0; bool foundExport = false; TRACE("Querying export list"); - if (write_sync(ioc, &magic, sizeof(magic)) != sizeof(magic)) { - error_setg(errp, "Failed to send list option magic"); - return -1; - } - - if (write_sync(ioc, &opt, sizeof(opt)) != sizeof(opt)) { - error_setg(errp, "Failed to send list option number"); - return -1; - } - - if (write_sync(ioc, &length, sizeof(length)) != sizeof(length)) { - error_setg(errp, "Failed to send list option length"); + if (nbd_send_option_request(ioc, NBD_OPT_LIST, 0, NULL, errp) < 0) { return -1; } @@ -314,72 +330,29 @@ static QIOChannel *nbd_receive_starttls(QIOChannel *ioc, QCryptoTLSCreds *tlscreds, const char *hostname, Error **errp) { - uint64_t magic = cpu_to_be64(NBD_OPTS_MAGIC); - uint32_t opt = cpu_to_be32(NBD_OPT_STARTTLS); - uint32_t length = 0; - uint32_t type; + nbd_opt_reply reply; QIOChannelTLS *tioc; struct NBDTLSHandshakeData data = { 0 }; TRACE("Requesting TLS from server"); - if (write_sync(ioc, &magic, sizeof(magic)) != sizeof(magic)) { - error_setg(errp, "Failed to send option magic"); - return NULL; - } - - if (write_sync(ioc, &opt, sizeof(opt)) != sizeof(opt)) { - error_setg(errp, "Failed to send option number"); - return NULL; - } - - if (write_sync(ioc, &length, sizeof(length)) != sizeof(length)) { - error_setg(errp, "Failed to send option length"); - return NULL; - } - - TRACE("Getting TLS reply from server1"); - if (read_sync(ioc, &magic, sizeof(magic)) != sizeof(magic)) { - error_setg(errp, "failed to read option magic"); - return NULL; - } - magic = be64_to_cpu(magic); - if (magic != NBD_REP_MAGIC) { - error_setg(errp, "Unexpected option magic"); - return NULL; - } - TRACE("Getting TLS reply from server2"); - if (read_sync(ioc, &opt, sizeof(opt)) != sizeof(opt)) { - error_setg(errp, "failed to read option"); - return NULL; - } - opt = be32_to_cpu(opt); - if (opt != NBD_OPT_STARTTLS) { - error_setg(errp, "Unexpected option type %" PRIx32 " expected %x", - opt, NBD_OPT_STARTTLS); + if (nbd_send_option_request(ioc, NBD_OPT_STARTTLS, 0, NULL, errp) < 0) { return NULL; } TRACE("Getting TLS reply from server"); - if (read_sync(ioc, &type, sizeof(type)) != sizeof(type)) { - error_setg(errp, "failed to read option type"); + if (nbd_receive_option_reply(ioc, NBD_OPT_STARTTLS, &reply, errp) < 0) { return NULL; } - type = be32_to_cpu(type); - if (type != NBD_REP_ACK) { + + if (reply.type != NBD_REP_ACK) { error_setg(errp, "Server rejected request to start TLS %" PRIx32, - type); + reply.type); return NULL; } - TRACE("Getting TLS reply from server"); - if (read_sync(ioc, &length, sizeof(length)) != sizeof(length)) { - error_setg(errp, "failed to read option length"); - return NULL; - } - length = be32_to_cpu(length); - if (length != 0) { + if (reply.length != 0) { error_setg(errp, "Start TLS response was not zero %" PRIu32, - length); + reply.length); return NULL; } @@ -466,8 +439,6 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags, if (magic == NBD_OPTS_MAGIC) { uint32_t clientflags = 0; - uint32_t opt; - uint32_t namesize; uint16_t globalflags; bool fixedNewStyle = false; @@ -517,28 +488,13 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags, goto fail; } } - /* write the export name */ - magic = cpu_to_be64(magic); - if (write_sync(ioc, &magic, sizeof(magic)) != sizeof(magic)) { - error_setg(errp, "Failed to send export name magic"); - goto fail; - } - opt = cpu_to_be32(NBD_OPT_EXPORT_NAME); - if (write_sync(ioc, &opt, sizeof(opt)) != sizeof(opt)) { - error_setg(errp, "Failed to send export name option number"); - goto fail; - } - namesize = cpu_to_be32(strlen(name)); - if (write_sync(ioc, &namesize, sizeof(namesize)) != - sizeof(namesize)) { - error_setg(errp, "Failed to send export name length"); - goto fail; - } - if (write_sync(ioc, (char *)name, strlen(name)) != strlen(name)) { - error_setg(errp, "Failed to send export name"); + /* write the export name request */ + if (nbd_send_option_request(ioc, NBD_OPT_EXPORT_NAME, -1, name, + errp) < 0) { goto fail; } + /* Read the response */ if (read_sync(ioc, &s, sizeof(s)) != sizeof(s)) { error_setg(errp, "Failed to read export length"); goto fail;