From patchwork Mon Oct 31 14:37:38 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Bonzini X-Patchwork-Id: 689396 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3t6yjs6mBLz9t2N for ; Tue, 1 Nov 2016 02:20:49 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b=XPegxc0l; dkim-atps=neutral Received: from localhost ([::1]:36276 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c1EOF-0004P5-0l for incoming@patchwork.ozlabs.org; Mon, 31 Oct 2016 11:20:47 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38270) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c1Dj7-0005wW-OD for qemu-devel@nongnu.org; Mon, 31 Oct 2016 10:38:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c1Dj6-0003wi-Md for qemu-devel@nongnu.org; Mon, 31 Oct 2016 10:38:17 -0400 Received: from mail-wm0-x243.google.com ([2a00:1450:400c:c09::243]:36148) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1c1Dj6-0003wH-Bj for qemu-devel@nongnu.org; Mon, 31 Oct 2016 10:38:16 -0400 Received: by mail-wm0-x243.google.com with SMTP id c17so18714549wmc.3 for ; Mon, 31 Oct 2016 07:38:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:cc:subject:date:message-id:in-reply-to:references; bh=un8PqeZJzVU5ubNgTpFfM8Zcr+b+N/8jbGXt+V5zVDs=; b=XPegxc0lkg5Cr5YLZjS3UuxdpZWs95fsoPVM2xHK1EeqoNQqyV9MZEdywIhXA7J4BO jQdFCvS/cWc79P+LO1NJK7RIBUE+HmL+qIaofHr20PICTQevG6xwxfg5CPqh6rAubZO1 twisGB8AWxhxPXgSXwAjpdYP2Uy/aZ2tVeebbmWSzmlW/PdFaORWdSSDLsHxsAED5imP TtXdZ7TKZwO0S1tGsQBOuK5tEUx1FMXtuTILpDzf6/uEbInjmYDUlN/lSQl7blWDZq4B Lg96by4C6uDPLWn5rqK1FDynrC4JniG0v5TT/02HKOv+TrzeR6vCfOf4CsHAkww5Y92q 5EkA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:from:to:cc:subject:date:message-id :in-reply-to:references; bh=un8PqeZJzVU5ubNgTpFfM8Zcr+b+N/8jbGXt+V5zVDs=; b=Bw9fr98xdBoJNLhswPeJixPwx6/tteM44JqPhgDJHezTYyYeZ3vkCYg7I/pSAEbEmo xD+nUe64Pzwr8jQv9O6CTVkgEEf9ze7Ca0TAdrP4nVgUD9cNTKUoZ7lx/slOBYRXkt5u G3K29ZLLeUvkNmij+1yw9Fp+/Ou1xJS0+ejmSzSeF5aH/hVm3r3Ke1qxm9+9GW/R7ZX+ opsE4FWP8HH6FDgtVexuxTKGGvaNeaB8d2gx7SM6LTiMiJya4IlIMPaqB2uDi8WrHfLu 4C2JY+lNCeDfwe5OSOmPVHFRDQdfIj6D9Qwx4+Bly5jgdq41vHuoxySH/2NQDRXzSlc3 gHOA== X-Gm-Message-State: ABUngvdP+FhrN90KOx1lN6DEedrlo5jrzfvUppZW0Sl1r9oMak42ZciISLUCYMyF0KuTUg== X-Received: by 10.28.135.84 with SMTP id j81mr11551050wmd.127.1477924695126; Mon, 31 Oct 2016 07:38:15 -0700 (PDT) Received: from donizetti.lan (dynamic-adsl-78-12-246-178.clienti.tiscali.it. [78.12.246.178]) by smtp.gmail.com with ESMTPSA id r72sm25166766wmd.21.2016.10.31.07.38.13 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 31 Oct 2016 07:38:14 -0700 (PDT) From: Paolo Bonzini To: qemu-devel@nongnu.org Date: Mon, 31 Oct 2016 15:37:38 +0100 Message-Id: <1477924663-30950-23-git-send-email-pbonzini@redhat.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1477924663-30950-1-git-send-email-pbonzini@redhat.com> References: <1477924663-30950-1-git-send-email-pbonzini@redhat.com> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2a00:1450:400c:c09::243 Subject: [Qemu-devel] [PULL 22/27] nbd: Improve server 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: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" From: Eric Blake 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 - but that's a bigger task for another day). Signed-off-by: Eric Blake Message-Id: <1476469998-28592-15-git-send-email-eblake@redhat.com> [Move dummy ESHUTDOWN to include/qemu/osdep.h. - Paolo] Signed-off-by: Paolo Bonzini --- include/block/nbd.h | 13 +++++++++---- include/qemu/osdep.h | 3 +++ nbd/client.c | 18 ++++++++++++++++++ nbd/nbd-internal.h | 1 + nbd/server.c | 12 ++++++++++++ 5 files changed, 43 insertions(+), 4 deletions(-) diff --git a/include/block/nbd.h b/include/block/nbd.h index d326308..eea7ef0 100644 --- a/include/block/nbd.h +++ b/include/block/nbd.h @@ -83,12 +83,17 @@ typedef struct NBDReply NBDReply; #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/include/qemu/osdep.h b/include/qemu/osdep.h index 0e3c330..689f253 100644 --- a/include/qemu/osdep.h +++ b/include/qemu/osdep.h @@ -128,6 +128,9 @@ extern int daemon(int, int); #if !defined(EMEDIUMTYPE) #define EMEDIUMTYPE 4098 #endif +#if !defined(ESHUTDOWN) +#define ESHUTDOWN 4099 +#endif #ifndef TIME_MAX #define TIME_MAX LONG_MAX #endif diff --git a/nbd/client.c b/nbd/client.c index 7bdce53..7db4301 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -40,6 +40,9 @@ static int nbd_errno_to_system_errno(int err) case NBD_ENOSPC: ret = ENOSPC; break; + case NBD_ESHUTDOWN: + ret = ESHUTDOWN; + break; default: TRACE("Squashing unexpected error %d to EINVAL", err); /* fallthrough */ @@ -239,11 +242,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); @@ -785,6 +798,11 @@ ssize_t nbd_receive_reply(QIOChannel *ioc, NBDReply *reply) reply->error = nbd_errno_to_system_errno(reply->error); + if (reply->error == ESHUTDOWN) { + /* This works even on mingw which lacks a native ESHUTDOWN */ + LOG("server shutting down"); + return -EINVAL; + } TRACE("Got reply: { magic = 0x%" PRIx32 ", .error = % " PRId32 ", handle = %" PRIu64" }", magic, reply->error, reply->handle); diff --git a/nbd/nbd-internal.h b/nbd/nbd-internal.h index dd57e18..eee20ab 100644 --- a/nbd/nbd-internal.h +++ b/nbd/nbd-internal.h @@ -92,6 +92,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/server.c b/nbd/server.c index afc1ec4..12e2631 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -39,6 +39,10 @@ static int system_errno_to_nbd_errno(int err) case EFBIG: case ENOSPC: return NBD_ENOSPC; +#ifdef ESHUTDOWN + case ESHUTDOWN: + return NBD_ESHUTDOWN; +#endif case EINVAL: default: return NBD_EINVAL; @@ -527,6 +531,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) { @@ -539,6 +547,10 @@ static int nbd_negotiate_options(NBDClient *client) break; case NBD_OPT_ABORT: + /* NBD spec says we must try to reply before + * disconnecting, but that we must also tolerate + * guests that don't wait for our reply. */ + nbd_negotiate_send_rep(client->ioc, NBD_REP_ACK, clientflags); return -EINVAL; case NBD_OPT_EXPORT_NAME: