From patchwork Mon Mar 25 19:01:03 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 10869979 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id A24B217E0 for ; Mon, 25 Mar 2019 19:04:52 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 8D1AF2933A for ; Mon, 25 Mar 2019 19:04:52 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 812BA29348; Mon, 25 Mar 2019 19:04:52 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 2AA7C2933A for ; Mon, 25 Mar 2019 19:04:52 +0000 (UTC) Received: from localhost ([127.0.0.1]:47098 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h8Utv-0003Yv-F3 for patchwork-qemu-devel@patchwork.kernel.org; Mon, 25 Mar 2019 15:04:51 -0400 Received: from eggs.gnu.org ([209.51.188.92]:43564) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h8UqT-00012K-PV for qemu-devel@nongnu.org; Mon, 25 Mar 2019 15:01:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1h8UqS-0005Oi-6b for qemu-devel@nongnu.org; Mon, 25 Mar 2019 15:01:17 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38008) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1h8UqP-0005Jm-DL; Mon, 25 Mar 2019 15:01:13 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 291223086272; Mon, 25 Mar 2019 19:01:10 +0000 (UTC) Received: from blue.redhat.com (ovpn-116-59.phx2.redhat.com [10.3.116.59]) by smtp.corp.redhat.com (Postfix) with ESMTP id 7E08A5C232; Mon, 25 Mar 2019 19:01:09 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Date: Mon, 25 Mar 2019 14:01:03 -0500 Message-Id: <20190325190104.30213-2-eblake@redhat.com> In-Reply-To: <20190325190104.30213-1-eblake@redhat.com> References: <20190325190104.30213-1-eblake@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.49]); Mon, 25 Mar 2019 19:01:10 +0000 (UTC) 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 for-4.0 v2 1/2] nbd: Don't lose server's error to NBD_CMD_BLOCK_STATUS 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 , vsementsov@virtuozzo.com, qemu-block@nongnu.org, rjones@redhat.com, Max Reitz , jsnow@redhat.com Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP When the server replies with a (structured [*]) error to NBD_CMD_BLOCK_STATUS, without any extent information sent first, the client code was blindly throwing away the server's error code and instead telling the caller that EIO occurred. This has been broken since its introduction in 78a33ab5 (v2.12, where we should have called: error_setg(&local_err, "Server did not reply with any status extents"); nbd_iter_error(&iter, false, -EIO, &local_err); to declare the situation as a non-fatal error if no earlier error had already been flagged, rather than just blindly slamming iter.err and iter.ret), although it is more noticeable since commit 7f86068d, which actually tries hard to preserve the server's code thanks to a separate iter.request_ret. [*] The spec is clear that the server is also permitted to reply with a simple error, but that's a separate fix. I was able to provoke this scenario with a hack to the server, then seeing whether ENOMEM makes it back to the caller: | diff --git a/nbd/server.c b/nbd/server.c | index fd013a2817a..29c7995de02 100644 | --- a/nbd/server.c | +++ b/nbd/server.c | @@ -2269,6 +2269,8 @@ static coroutine_fn int nbd_handle_request(NBDClient *client, | "discard failed", errp); | | case NBD_CMD_BLOCK_STATUS: | + return nbd_send_generic_reply(client, request->handle, -ENOMEM, | + "no status for you today", errp); | if (!request->len) { | return nbd_send_generic_reply(client, request->handle, -EINVAL, | "need non-zero length", errp); | -- Signed-off-by: Eric Blake Message-Id: <20190323142455.5301-1-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy --- block/nbd-client.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/block/nbd-client.c b/block/nbd-client.c index 8fe660b6093..b37a5963013 100644 --- a/block/nbd-client.c +++ b/block/nbd-client.c @@ -769,12 +769,9 @@ static int nbd_co_receive_blockstatus_reply(NBDClientSession *s, payload = NULL; } - if (!extent->length && !iter.err) { - error_setg(&iter.err, - "Server did not reply with any status extents"); - if (!iter.ret) { - iter.ret = -EIO; - } + if (!extent->length && !iter.request_ret) { + error_setg(&local_err, "Server did not reply with any status extents"); + nbd_iter_channel_error(&iter, -EIO, &local_err); } error_propagate(errp, iter.err); From patchwork Mon Mar 25 19:01:04 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 10869957 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 1E7A914DE for ; Mon, 25 Mar 2019 19:02:53 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0955B2933C for ; Mon, 25 Mar 2019 19:02:53 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id F196D2935A; Mon, 25 Mar 2019 19:02:52 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 9B3962933C for ; Mon, 25 Mar 2019 19:02:52 +0000 (UTC) Received: from localhost ([127.0.0.1]:47084 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h8Us0-0001vq-0a for patchwork-qemu-devel@patchwork.kernel.org; Mon, 25 Mar 2019 15:02:52 -0400 Received: from eggs.gnu.org ([209.51.188.92]:43562) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h8UqT-00012C-L1 for qemu-devel@nongnu.org; Mon, 25 Mar 2019 15:01:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1h8UqS-0005Oz-8K for qemu-devel@nongnu.org; Mon, 25 Mar 2019 15:01:17 -0400 Received: from mx1.redhat.com ([209.132.183.28]:57638) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1h8UqP-0005K1-IV; Mon, 25 Mar 2019 15:01:13 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 1211E307D95F; Mon, 25 Mar 2019 19:01:11 +0000 (UTC) Received: from blue.redhat.com (ovpn-116-59.phx2.redhat.com [10.3.116.59]) by smtp.corp.redhat.com (Postfix) with ESMTP id 53BBB66604; Mon, 25 Mar 2019 19:01:10 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Date: Mon, 25 Mar 2019 14:01:04 -0500 Message-Id: <20190325190104.30213-3-eblake@redhat.com> In-Reply-To: <20190325190104.30213-1-eblake@redhat.com> References: <20190325190104.30213-1-eblake@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.48]); Mon, 25 Mar 2019 19:01:11 +0000 (UTC) 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 for-4.0 v2 2/2] nbd: Permit simple error to NBD_CMD_BLOCK_STATUS 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 , vsementsov@virtuozzo.com, qemu-block@nongnu.org, rjones@redhat.com, Max Reitz , jsnow@redhat.com Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP The NBD spec is clear that when structured replies are active, a simple error reply is acceptable to any command except for NBD_CMD_READ. However, we were mistakenly requiring structured errors for NBD_CMD_BLOCK_STATUS, and hanging up on a server that gave a simple error (since qemu does not behave as such a server, we didn't notice the problem until now). Broken since its introduction in commit 78a33ab5 (v2.12). Noticed while debugging a separate failure reported by nbdkit while working out its initial implementation of BLOCK_STATUS, although it turns out that nbdkit also chose to send structured error replies for BLOCK_STATUS, so I had to manually provoke the situation by hacking qemu's server to send a simple error reply: | diff --git i/nbd/server.c w/nbd/server.c | index fd013a2817a..833288d7c45 100644 | 00--- i/nbd/server.c | +++ w/nbd/server.c | @@ -2269,6 +2269,8 @@ static coroutine_fn int nbd_handle_request(NBDClient *client, | "discard failed", errp); | | case NBD_CMD_BLOCK_STATUS: | + return nbd_co_send_simple_reply(client, request->handle, ENOMEM, | + NULL, 0, errp); | if (!request->len) { | return nbd_send_generic_reply(client, request->handle, -EINVAL, | "need non-zero length", errp); | Signed-off-by: Eric Blake Message-Id: <20190323142455.5301-1-eblake@redhat.com> Acked-by: Richard W.M. Jones Reviewed-by: Vladimir Sementsov-Ogievskiy --- block/nbd-client.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/block/nbd-client.c b/block/nbd-client.c index b37a5963013..a3b70d14004 100644 --- a/block/nbd-client.c +++ b/block/nbd-client.c @@ -729,9 +729,7 @@ static int nbd_co_receive_blockstatus_reply(NBDClientSession *s, bool received = false; assert(!extent->length); - NBD_FOREACH_REPLY_CHUNK(s, iter, handle, s->info.structured_reply, - NULL, &reply, &payload) - { + NBD_FOREACH_REPLY_CHUNK(s, iter, handle, false, NULL, &reply, &payload) { int ret; NBDStructuredReplyChunk *chunk = &reply.structured;