From patchwork Tue Jan 19 13:09:19 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?q?Daniel_P=2E_Berrang=C3=A9?= X-Patchwork-Id: 8061881 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 6681C9F818 for ; Tue, 19 Jan 2016 13:19:22 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 6BD5E203C1 for ; Tue, 19 Jan 2016 13:19:21 +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 4E1D220386 for ; Tue, 19 Jan 2016 13:19:20 +0000 (UTC) Received: from localhost ([::1]:36968 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aLWBr-0004TC-Le for patchwork-qemu-devel@patchwork.kernel.org; Tue, 19 Jan 2016 08:19:19 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39855) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aLW2g-0007Wz-Bw for qemu-devel@nongnu.org; Tue, 19 Jan 2016 08:09:51 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aLW2a-0005xb-B1 for qemu-devel@nongnu.org; Tue, 19 Jan 2016 08:09:50 -0500 Received: from mx1.redhat.com ([209.132.183.28]:36766) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aLW2W-0005vg-J5; Tue, 19 Jan 2016 08:09:40 -0500 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id 493441C5F47; Tue, 19 Jan 2016 13:09:40 +0000 (UTC) Received: from t530wlan.home.berrange.com.com (vpn1-6-80.ams2.redhat.com [10.36.6.80]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u0JD9Qb3012785; Tue, 19 Jan 2016 08:09:39 -0500 From: "Daniel P. Berrange" To: qemu-devel@nongnu.org Date: Tue, 19 Jan 2016 13:09:19 +0000 Message-Id: <1453208963-16834-10-git-send-email-berrange@redhat.com> In-Reply-To: <1453208963-16834-1-git-send-email-berrange@redhat.com> References: <1453208963-16834-1-git-send-email-berrange@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: Paolo Bonzini , qemu-block@nongnu.org Subject: [Qemu-devel] [PATCH v3 09/13] nbd: pick first exported volume if no export name is requested 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.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 The NBD client is currently only capable of using the new style protocol negotiation if an explicit export name is provided. This is a problem, because TLS support will require use of the new style protocol in all cases, and we wish to keep the export name as an optional request for backwards compatibility. The trivial way to deal with this is to use the NBD protocol option for listing exports and then pick the first returned export as the one to use. This makes the client "do the right thing" in the normal case where the qemu-nbd server only exports a single volume. Furthermore, in order to get proper error reporting of unknown options with fixed new style protocol, we must not send the NBD_OPT_EXPORT_NAME as the first thing. Thus, even if an export name is provided we still send NBD_OPT_LIST to enumerate server exports. This also gives us clearer error reporting in the case that the requested export name does not exist. If NBD_OPT_LIST is not supported, we still fallback to using the specified export name, so as not to break compatibility with old QEMU NBD server impls that predated NBD_OPT_LIST Signed-off-by: Daniel P. Berrange --- nbd/client.c | 201 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- nbd/server.c | 2 + 2 files changed, 198 insertions(+), 5 deletions(-) diff --git a/nbd/client.c b/nbd/client.c index 22d69ff..3c245a5 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -70,12 +70,187 @@ static QTAILQ_HEAD(, NBDExport) exports = QTAILQ_HEAD_INITIALIZER(exports); */ + +static int nbd_handle_reply_err(uint32_t opt, uint32_t type, Error **errp) +{ + if (!(type & (1 << 31))) { + return 0; + } + + switch (type) { + case NBD_REP_ERR_UNSUP: + error_setg(errp, "Unsupported option type %x", opt); + break; + + case NBD_REP_ERR_INVALID: + error_setg(errp, "Invalid data length for option %x", opt); + break; + + default: + error_setg(errp, "Unknown error code when asking for option %x", opt); + break; + } + + return -1; +} + +static int nbd_receive_list(QIOChannel *ioc, char **name, Error **errp) +{ + uint64_t magic; + uint32_t opt; + uint32_t type; + uint32_t len; + uint32_t namelen; + + *name = NULL; + if (read_sync(ioc, &magic, sizeof(magic)) != sizeof(magic)) { + error_setg(errp, "failed to read list option magic"); + 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 %x 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); + if (type == NBD_REP_ERR_UNSUP) { + return 0; + } + if (nbd_handle_reply_err(opt, type, errp) < 0) { + 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 (type == NBD_REP_ACK) { + if (len != 0) { + error_setg(errp, "length too long for option end"); + return -1; + } + } else if (type == NBD_REP_SERVER) { + if (read_sync(ioc, &namelen, sizeof(namelen)) != sizeof(namelen)) { + error_setg(errp, "failed to read option name length"); + return -1; + } + namelen = be32_to_cpu(namelen); + if (len != (namelen + sizeof(namelen))) { + error_setg(errp, "incorrect option mame length"); + return -1; + } + if (namelen > 255) { + error_setg(errp, "export name length too long %d", namelen); + return -1; + } + + *name = g_new0(char, namelen + 1); + if (read_sync(ioc, *name, namelen) != namelen) { + error_setg(errp, "failed to read export name"); + g_free(*name); + *name = NULL; + return -1; + } + (*name)[namelen] = '\0'; + } else { + error_setg(errp, "Unexpected reply type %x expected %x", + type, NBD_REP_SERVER); + return -1; + } + return 1; +} + + +static char *nbd_receive_pick_export(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; + char *chosenname = NULL; + + TRACE("Picking export name"); + if (write_sync(ioc, &magic, sizeof(magic)) != sizeof(magic)) { + error_setg(errp, "Failed to send list option magic"); + goto fail; + } + + if (write_sync(ioc, &opt, sizeof(opt)) != sizeof(opt)) { + error_setg(errp, "Failed to send list option number"); + goto fail; + } + + if (write_sync(ioc, &length, sizeof(length)) != sizeof(length)) { + error_setg(errp, "Failed to send list option length"); + goto fail; + } + + TRACE("Reading available export names"); + while (1) { + char *name = NULL; + int ret = nbd_receive_list(ioc, &name, errp); + + if (ret < 0) { + g_free(name); + name = NULL; + goto fail; + } + if (ret == 0) { /* Server doesn't support export listing */ + if (wantname == NULL) { + error_setg(errp, "Export name is required"); + goto fail; + } + chosenname = g_strdup(wantname); + break; + } + if (name == NULL) { + TRACE("End of export name list"); + break; + } + if (!chosenname && (!wantname || g_str_equal(name, wantname))) { + chosenname = name; + TRACE("Picked export name %s", name); + } else { + TRACE("Ignored export name %s", name); + g_free(name); + } + } + + if (wantname && !chosenname) { + error_setg(errp, "No export with name %s available", wantname); + goto fail; + } + + fail: + TRACE("Chosen export name %s", chosenname ? chosenname : ""); + return chosenname; +} + int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint32_t *flags, off_t *size, Error **errp) { char buf[256]; uint64_t magic, s; int rc; + char *autoname = NULL; TRACE("Receiving negotiation."); @@ -120,27 +295,40 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint32_t *flags, uint32_t namesize; uint16_t globalflags; uint16_t exportflags; + bool fixedNewStyle = false; if (read_sync(ioc, &globalflags, sizeof(globalflags)) != sizeof(globalflags)) { error_setg(errp, "Failed to read server flags"); goto fail; } - *flags = be16_to_cpu(globalflags) << 16; + globalflags = be16_to_cpu(globalflags); + *flags = globalflags << 16; + TRACE("Global flags are %x", globalflags); if (globalflags & NBD_FLAG_FIXED_NEWSTYLE) { + fixedNewStyle = true; TRACE("Server supports fixed new style"); clientflags |= NBD_FLAG_C_FIXED_NEWSTYLE; } /* client requested flags */ + clientflags = cpu_to_be32(clientflags); if (write_sync(ioc, &clientflags, sizeof(clientflags)) != sizeof(clientflags)) { error_setg(errp, "Failed to send clientflags field"); goto fail; } /* write the export name */ - if (!name) { - error_setg(errp, "Server requires an export name"); - goto fail; + if (fixedNewStyle) { + autoname = nbd_receive_pick_export(ioc, name, errp); + if (!autoname) { + goto fail; + } + name = autoname; + } else { + if (!name) { + error_setg(errp, "Server requires an export name"); + goto fail; + } } magic = cpu_to_be64(magic); if (write_sync(ioc, &magic, sizeof(magic)) != sizeof(magic)) { @@ -175,7 +363,9 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint32_t *flags, error_setg(errp, "Failed to read export flags"); goto fail; } - *flags |= be16_to_cpu(exportflags); + exportflags = be16_to_cpu(exportflags); + *flags |= exportflags; + TRACE("Export flags are %x", exportflags); } else if (magic == NBD_CLIENT_MAGIC) { if (name) { error_setg(errp, "Server does not support export names"); @@ -206,6 +396,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint32_t *flags, rc = 0; fail: + free(autoname); return rc; } diff --git a/nbd/server.c b/nbd/server.c index 8343a09..5296808 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -291,6 +291,8 @@ static int nbd_negotiate_handle_export_name(NBDClient *client, uint32_t length) } name[length] = '\0'; + TRACE("Client requested export '%s'", name); + client->exp = nbd_export_find(name); if (!client->exp) { LOG("export not found");