From patchwork Fri Apr 22 23:40:44 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 8916601 Return-Path: X-Original-To: patchwork-qemu-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 1EBD8BF29F for ; Sat, 23 Apr 2016 00:04:54 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 5B9F62020F for ; Sat, 23 Apr 2016 00:04:53 +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 64061201C7 for ; Sat, 23 Apr 2016 00:04:52 +0000 (UTC) Received: from localhost ([::1]:42029 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1atl47-0003Jb-Mo for patchwork-qemu-devel@patchwork.kernel.org; Fri, 22 Apr 2016 20:04:51 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45608) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1atkhq-0004JR-8q for qemu-devel@nongnu.org; Fri, 22 Apr 2016 19:41:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1atkhn-0005Yq-QI for qemu-devel@nongnu.org; Fri, 22 Apr 2016 19:41:50 -0400 Received: from mx1.redhat.com ([209.132.183.28]:52933) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1atkhk-0005W4-Lc; Fri, 22 Apr 2016 19:41:44 -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 4412D8AE72; Fri, 22 Apr 2016 23:41:44 +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 u3MNfHXP028475; Fri, 22 Apr 2016 19:41:42 -0400 From: Eric Blake To: qemu-devel@nongnu.org Date: Fri, 22 Apr 2016 17:40:44 -0600 Message-Id: <1461368452-10389-37-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 36/44] nbd: Improve handling of shutdown requests 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 NBD commit 6d34500b clarified how clients and servers are supposed to behave before closing a connection. It added NBD_REP_ERR_SHUTDOWN (for the server to announce it is about to go away during option haggling, so the client should quit sending NBD_OPT_* other than NBD_OPT_ABORT) and ESHUTDOWN (for the server to announce it is about to go away during transmission, so the client should quit sending NBD_CMD_* other than NBD_CMD_DISC). It also clarified that NBD_OPT_ABORT gets a reply, while NBD_CMD_DISC does not. This patch merely adds the missing reply to NBD_OPT_ABORT and teaches the client to recognize server errors. Actually teaching the server to send NBD_REP_ERR_SHUTDOWN or ESHUTDOWN would require knowing that the server has been requested to shut down soon (maybe we could do that by installing a SIGINT handler in qemu-nbd, which transitions from RUNNING to a new state that waits for the client to react, rather than just out-right quitting). Signed-off-by: Eric Blake --- include/block/nbd.h | 13 +++++++++---- nbd/nbd-internal.h | 1 + nbd/client.c | 16 ++++++++++++++++ nbd/server.c | 16 +++++++++++++++- 4 files changed, 41 insertions(+), 5 deletions(-) diff --git a/include/block/nbd.h b/include/block/nbd.h index d707761..2fd1a67 100644 --- a/include/block/nbd.h +++ b/include/block/nbd.h @@ -82,12 +82,17 @@ typedef struct nbd_reply nbd_reply; #define NBD_FLAG_C_NO_ZEROES (1 << 1) /* End handshake without zeroes. */ /* Reply types. */ +#define NBD_REP_ERR(value) ((UINT32_C(1) << 31) | (value)) + #define NBD_REP_ACK (1) /* Data sending finished. */ #define NBD_REP_SERVER (2) /* Export description. */ -#define NBD_REP_ERR_UNSUP ((UINT32_C(1) << 31) | 1) /* Unknown option. */ -#define NBD_REP_ERR_POLICY ((UINT32_C(1) << 31) | 2) /* Server denied */ -#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 */ + +#define NBD_REP_ERR_UNSUP NBD_REP_ERR(1) /* Unknown option. */ +#define NBD_REP_ERR_POLICY NBD_REP_ERR(2) /* Server denied */ +#define NBD_REP_ERR_INVALID NBD_REP_ERR(3) /* Invalid length */ +#define NBD_REP_ERR_PLATFORM NBD_REP_ERR(4) /* Not compiled in */ +#define NBD_REP_ERR_TLS_REQD NBD_REP_ERR(5) /* TLS required */ +#define NBD_REP_ERR_SHUTDOWN NBD_REP_ERR(7) /* Server shutting down */ /* Request flags, sent from client to server during transmission phase */ #define NBD_CMD_FLAG_FUA (1 << 0) diff --git a/nbd/nbd-internal.h b/nbd/nbd-internal.h index 95069db..0d40b1f 100644 --- a/nbd/nbd-internal.h +++ b/nbd/nbd-internal.h @@ -91,6 +91,7 @@ #define NBD_ENOMEM 12 #define NBD_EINVAL 22 #define NBD_ENOSPC 28 +#define NBD_ESHUTDOWN 108 static inline ssize_t read_sync(QIOChannel *ioc, void *buffer, size_t size) { diff --git a/nbd/client.c b/nbd/client.c index 68e9473..4140d13 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -34,6 +34,8 @@ static int nbd_errno_to_system_errno(int err) return ENOMEM; case NBD_ENOSPC: return ENOSPC; + case NBD_ESHUTDOWN: + return ESHUTDOWN; default: TRACE("Squashing unexpected error %d to EINVAL", err); /* fallthrough */ @@ -210,11 +212,21 @@ static int nbd_handle_reply_err(QIOChannel *ioc, nbd_opt_reply *reply, reply->option); break; + case NBD_REP_ERR_PLATFORM: + error_setg(errp, "Server lacks support for option %" PRIx32, + reply->option); + break; + case NBD_REP_ERR_TLS_REQD: error_setg(errp, "TLS negotiation required before option %" PRIx32, reply->option); break; + case NBD_REP_ERR_SHUTDOWN: + error_setg(errp, "Server shutting down before option %" PRIx32, + reply->option); + break; + default: error_setg(errp, "Unknown error code when asking for option %" PRIx32, reply->option); @@ -754,6 +766,10 @@ ssize_t nbd_receive_reply(QIOChannel *ioc, struct nbd_reply *reply) LOG("invalid magic (got 0x%" PRIx32 ")", magic); return -EINVAL; } + if (reply->error == ESHUTDOWN) { + LOG("server shutting down"); + return -EINVAL; + } return 0; } diff --git a/nbd/server.c b/nbd/server.c index dadc928..fa6a994 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -39,6 +39,8 @@ static int system_errno_to_nbd_errno(int err) case EFBIG: case ENOSPC: return NBD_ENOSPC; + case ESHUTDOWN: + return NBD_ESHUTDOWN; case EINVAL: default: return NBD_EINVAL; @@ -484,6 +486,10 @@ static int nbd_negotiate_options(NBDClient *client) if (ret < 0) { return ret; } + /* Let the client keep trying, unless they asked to quit */ + if (clientflags == NBD_OPT_ABORT) { + return -EINVAL; + } break; } } else if (fixedNewstyle) { @@ -496,7 +502,15 @@ static int nbd_negotiate_options(NBDClient *client) break; case NBD_OPT_ABORT: - return -EINVAL; + /* NBD spec says we must reply before disconnecting, + * but that we must also tolerate guests that don't + * wait for our reply. */ + ret = nbd_negotiate_send_rep(client->ioc, NBD_REP_ACK, + clientflags); + if (!ret) { + ret = -EINVAL; + } + return ret; case NBD_OPT_EXPORT_NAME: return nbd_negotiate_handle_export_name(client, length);