From patchwork Mon Apr 1 14:08:50 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 1072852 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 44Xvct3MTYz9sPp for ; Tue, 2 Apr 2019 01:20:14 +1100 (AEDT) Received: from localhost ([127.0.0.1]:35928 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAxnI-0005P8-C8 for incoming@patchwork.ozlabs.org; Mon, 01 Apr 2019 10:20:12 -0400 Received: from eggs.gnu.org ([209.51.188.92]:45730) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAxcc-0001r1-Nj for qemu-devel@nongnu.org; Mon, 01 Apr 2019 10:09:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hAxcb-0004hh-Qo for qemu-devel@nongnu.org; Mon, 01 Apr 2019 10:09:10 -0400 Received: from mx1.redhat.com ([209.132.183.28]:3697) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hAxcZ-0004fq-BB; Mon, 01 Apr 2019 10:09:07 -0400 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A4B39307D981; Mon, 1 Apr 2019 14:09:06 +0000 (UTC) Received: from blue.redhat.com (ovpn-116-75.phx2.redhat.com [10.3.116.75]) by smtp.corp.redhat.com (Postfix) with ESMTP id F271B19C7C; Mon, 1 Apr 2019 14:09:05 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Date: Mon, 1 Apr 2019 09:08:50 -0500 Message-Id: <20190401140903.19186-2-eblake@redhat.com> In-Reply-To: <20190401140903.19186-1-eblake@redhat.com> References: <20190401140903.19186-1-eblake@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.48]); Mon, 01 Apr 2019 14:09:06 +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] [PULL 01/14] qemu-img: Report bdrv_block_status failures 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 , Vladimir Sementsov-Ogievskiy , "Richard W . M . Jones" , "open list:Block layer core" , Max Reitz Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" If bdrv_block_status_above() fails, we are aborting the convert process but failing to print an error message. Broken in commit 690c7301 (v2.4) when rewriting convert's logic. Discovered when teaching nbdkit to support NBD_CMD_BLOCK_STATUS, and accidentally violating the protocol by returning more than one extent in spite of qemu asking for NBD_CMD_FLAG_REQ_ONE. The qemu NBD code should probably handle the server's non-compliance more gracefully than failing with EINVAL, but qemu-img shouldn't be silently squelching any block status failures. It doesn't help that qemu 3.1 masks the qemu-img bug with extra noise that the nbd code is dumping to stderr (that noise was cleaned up in d8b4bad8). Reported-by: Richard W.M. Jones Signed-off-by: Eric Blake Message-Id: <20190323212639.579-2-eblake@redhat.com> Reviewed-by: Kevin Wolf Reviewed-by: Vladimir Sementsov-Ogievskiy --- qemu-img.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/qemu-img.c b/qemu-img.c index 8ee63daeaeb..03a9a10dec1 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1630,6 +1630,8 @@ static int convert_iteration_sectors(ImgConvertState *s, int64_t sector_num) count, &count, NULL, NULL); } if (ret < 0) { + error_report("error while reading block status of sector %" PRId64 + ": %s", sector_num, strerror(-ret)); return ret; } n = DIV_ROUND_UP(count, BDRV_SECTOR_SIZE); From patchwork Mon Apr 1 14:08:51 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 1072857 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 44XvhX3vLjz9sPv for ; Tue, 2 Apr 2019 01:23:24 +1100 (AEDT) Received: from localhost ([127.0.0.1]:36745 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAxqL-0008Dz-Ru for incoming@patchwork.ozlabs.org; Mon, 01 Apr 2019 10:23:21 -0400 Received: from eggs.gnu.org ([209.51.188.92]:45766) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAxce-0001tZ-Jn for qemu-devel@nongnu.org; Mon, 01 Apr 2019 10:09:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hAxcd-0004ih-6b for qemu-devel@nongnu.org; Mon, 01 Apr 2019 10:09:12 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48902) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hAxca-0004gQ-6W; Mon, 01 Apr 2019 10:09:08 -0400 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 859B03D37; Mon, 1 Apr 2019 14:09:07 +0000 (UTC) Received: from blue.redhat.com (ovpn-116-75.phx2.redhat.com [10.3.116.75]) by smtp.corp.redhat.com (Postfix) with ESMTP id D496419C70; Mon, 1 Apr 2019 14:09:06 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Date: Mon, 1 Apr 2019 09:08:51 -0500 Message-Id: <20190401140903.19186-3-eblake@redhat.com> In-Reply-To: <20190401140903.19186-1-eblake@redhat.com> References: <20190401140903.19186-1-eblake@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Mon, 01 Apr 2019 14:09:07 +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] [PULL 02/14] nbd: Tolerate some server non-compliance in 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 , Vladimir Sementsov-Ogievskiy , "Richard W . M . Jones" , "open list:Network Block Dev..." , Max Reitz Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" The NBD spec states that NBD_CMD_FLAG_REQ_ONE (which we currently always use) should not reply with an extent larger than our request, and that the server's response should be exactly one extent. Right now, that means that if a server sends more than one extent, we treat the server as broken, fail the block status request, and disconnect, which prevents all further use of the block device. But while good software should be strict in what it sends, it should be tolerant in what it receives. While trying to implement NBD_CMD_BLOCK_STATUS in nbdkit, we temporarily had a non-compliant server sending too many extents in spite of REQ_ONE. Oddly enough, 'qemu-img convert' with qemu 3.1 failed with a somewhat useful message: qemu-img: Protocol error: invalid payload for NBD_REPLY_TYPE_BLOCK_STATUS which then disappeared with commit d8b4bad8, on the grounds that an error message flagged only at the time of coroutine teardown is pointless, and instead we should rely on the actual failed API to report an error - in other words, the 3.1 behavior was masking the fact that qemu-img was not reporting an error. That has since been fixed in the previous patch, where qemu-img convert now fails with: qemu-img: error while reading block status of sector 0: Invalid argument But even that is harsh. Since we already partially relaxed things in commit acfd8f7a to tolerate a server that exceeds the cap (although that change was made prior to the NBD spec actually putting a cap on the extent length during REQ_ONE - in fact, the NBD spec change was BECAUSE of the qemu behavior prior to that commit), it's not that much harder to argue that we should also tolerate a server that sends too many extents. But at the same time, it's nice to trace when we are being tolerant of server non-compliance, in order to help server writers fix their implementations to be more portable (if they refer to our traces, rather than just stderr). Reported-by: Richard W.M. Jones Signed-off-by: Eric Blake Message-Id: <20190323212639.579-3-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy --- block/nbd-client.c | 21 ++++++++++++++++----- block/trace-events | 1 + 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/block/nbd-client.c b/block/nbd-client.c index bfbaf7ebe94..8fe660b6093 100644 --- a/block/nbd-client.c +++ b/block/nbd-client.c @@ -240,8 +240,8 @@ static int nbd_parse_offset_hole_payload(NBDStructuredReplyChunk *chunk, } /* nbd_parse_blockstatus_payload - * support only one extent in reply and only for - * base:allocation context + * Based on our request, we expect only one extent in reply, for the + * base:allocation context. */ static int nbd_parse_blockstatus_payload(NBDClientSession *client, NBDStructuredReplyChunk *chunk, @@ -250,7 +250,8 @@ static int nbd_parse_blockstatus_payload(NBDClientSession *client, { uint32_t context_id; - if (chunk->length != sizeof(context_id) + sizeof(*extent)) { + /* The server succeeded, so it must have sent [at least] one extent */ + if (chunk->length < sizeof(context_id) + sizeof(*extent)) { error_setg(errp, "Protocol error: invalid payload for " "NBD_REPLY_TYPE_BLOCK_STATUS"); return -EINVAL; @@ -276,10 +277,20 @@ static int nbd_parse_blockstatus_payload(NBDClientSession *client, return -EINVAL; } - /* The server is allowed to send us extra information on the final - * extent; just clamp it to the length we requested. */ + /* + * We used NBD_CMD_FLAG_REQ_ONE, so the server should not have + * sent us any more than one extent, nor should it have included + * status beyond our request in that extent. However, it's easy + * enough to ignore the server's noncompliance without killing the + * connection; just ignore trailing extents, and clamp things to + * the length of our request. + */ + if (chunk->length > sizeof(context_id) + sizeof(*extent)) { + trace_nbd_parse_blockstatus_compliance("more than one extent"); + } if (extent->length > orig_length) { extent->length = orig_length; + trace_nbd_parse_blockstatus_compliance("extent length too large"); } return 0; diff --git a/block/trace-events b/block/trace-events index e6bb5a8f05c..debb25c0ac8 100644 --- a/block/trace-events +++ b/block/trace-events @@ -157,6 +157,7 @@ nvme_cmd_map_qiov_iov(void *s, int i, void *page, int pages) "s %p iov[%d] %p pa iscsi_xcopy(void *src_lun, uint64_t src_off, void *dst_lun, uint64_t dst_off, uint64_t bytes, int ret) "src_lun %p offset %"PRIu64" dst_lun %p offset %"PRIu64" bytes %"PRIu64" ret %d" # nbd-client.c +nbd_parse_blockstatus_compliance(const char *err) "ignoring extra data from non-compliant server: %s" nbd_read_reply_entry_fail(int ret, const char *err) "ret = %d, err: %s" nbd_co_request_fail(uint64_t from, uint32_t len, uint64_t handle, uint16_t flags, uint16_t type, const char *name, int ret, const char *err) "Request failed { .from = %" PRIu64", .len = %" PRIu32 ", .handle = %" PRIu64 ", .flags = 0x%" PRIx16 ", .type = %" PRIu16 " (%s) } ret = %d, err: %s" From patchwork Mon Apr 1 14:08:52 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 1072848 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 44XvYl0D0mz9sPj for ; Tue, 2 Apr 2019 01:17:31 +1100 (AEDT) Received: from localhost ([127.0.0.1]:35264 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAxke-0002X1-Sp for incoming@patchwork.ozlabs.org; Mon, 01 Apr 2019 10:17:28 -0400 Received: from eggs.gnu.org ([209.51.188.92]:45770) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAxce-0001tj-QD for qemu-devel@nongnu.org; Mon, 01 Apr 2019 10:09:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hAxcd-0004iz-N7 for qemu-devel@nongnu.org; Mon, 01 Apr 2019 10:09:12 -0400 Received: from mx1.redhat.com ([209.132.183.28]:40232) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hAxcb-0004gr-0D; Mon, 01 Apr 2019 10:09:09 -0400 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 4E59D309707A; Mon, 1 Apr 2019 14:09:08 +0000 (UTC) Received: from blue.redhat.com (ovpn-116-75.phx2.redhat.com [10.3.116.75]) by smtp.corp.redhat.com (Postfix) with ESMTP id B18E227CB3; Mon, 1 Apr 2019 14:09:07 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Date: Mon, 1 Apr 2019 09:08:52 -0500 Message-Id: <20190401140903.19186-4-eblake@redhat.com> In-Reply-To: <20190401140903.19186-1-eblake@redhat.com> References: <20190401140903.19186-1-eblake@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.43]); Mon, 01 Apr 2019 14:09:08 +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] [PULL 03/14] 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 , Vladimir Sementsov-Ogievskiy , "open list:Network Block Dev..." , Max Reitz Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" 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: <20190325190104.30213-2-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 Apr 1 14:08:53 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 1072849 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 44XvZ30z3Nz9sPj for ; Tue, 2 Apr 2019 01:17:47 +1100 (AEDT) Received: from localhost ([127.0.0.1]:35339 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAxkv-0002in-3A for incoming@patchwork.ozlabs.org; Mon, 01 Apr 2019 10:17:45 -0400 Received: from eggs.gnu.org ([209.51.188.92]:45786) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAxcf-0001v4-Gb for qemu-devel@nongnu.org; Mon, 01 Apr 2019 10:09:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hAxce-0004jJ-Cv for qemu-devel@nongnu.org; Mon, 01 Apr 2019 10:09:13 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38338) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hAxcb-0004hP-T1; Mon, 01 Apr 2019 10:09:10 -0400 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2DD812D7E7; Mon, 1 Apr 2019 14:09:09 +0000 (UTC) Received: from blue.redhat.com (ovpn-116-75.phx2.redhat.com [10.3.116.75]) by smtp.corp.redhat.com (Postfix) with ESMTP id 7B1F219C70; Mon, 1 Apr 2019 14:09:08 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Date: Mon, 1 Apr 2019 09:08:53 -0500 Message-Id: <20190401140903.19186-5-eblake@redhat.com> In-Reply-To: <20190401140903.19186-1-eblake@redhat.com> References: <20190401140903.19186-1-eblake@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Mon, 01 Apr 2019 14:09:09 +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] [PULL 04/14] 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 , Vladimir Sementsov-Ogievskiy , "Richard W . M . Jones" , "open list:Network Block Dev..." , Max Reitz Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" 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 Acked-by: Richard W.M. Jones Message-Id: <20190325190104.30213-3-eblake@redhat.com> 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; From patchwork Mon Apr 1 14:08:54 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 1072856 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 44Xvh32Pltz9sPB for ; Tue, 2 Apr 2019 01:22:59 +1100 (AEDT) Received: from localhost ([127.0.0.1]:36629 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAxpx-0007s8-9j for incoming@patchwork.ozlabs.org; Mon, 01 Apr 2019 10:22:57 -0400 Received: from eggs.gnu.org ([209.51.188.92]:45851) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAxcj-0001zG-12 for qemu-devel@nongnu.org; Mon, 01 Apr 2019 10:09:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hAxci-0004mD-1J for qemu-devel@nongnu.org; Mon, 01 Apr 2019 10:09:16 -0400 Received: from mx1.redhat.com ([209.132.183.28]:18738) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hAxcf-0004jl-DH; Mon, 01 Apr 2019 10:09:13 -0400 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id B1AC23688E; Mon, 1 Apr 2019 14:09:12 +0000 (UTC) Received: from blue.redhat.com (ovpn-116-75.phx2.redhat.com [10.3.116.75]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5987119C70; Mon, 1 Apr 2019 14:09:09 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Date: Mon, 1 Apr 2019 09:08:54 -0500 Message-Id: <20190401140903.19186-6-eblake@redhat.com> In-Reply-To: <20190401140903.19186-1-eblake@redhat.com> References: <20190401140903.19186-1-eblake@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Mon, 01 Apr 2019 14:09:12 +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] [PULL 05/14] qemu-img: Gracefully shutdown when map can't finish 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 , John Snow , "open list:Block layer core" , Max Reitz Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Trying 'qemu-img map -f raw nbd://localhost:10809' causes the NBD server to output a scary message: qemu-nbd: Disconnect client, due to: Failed to read request: Unexpected end-of-file before all bytes were read This is because the NBD client, being remote, has no way to expose a human-readable map (the --output=json data is fine, however). But because we exit(1) right after the message, causing the client to bypass all block cleanup, the server sees the abrupt exit and warns, whereas it would be silent had the client had a chance to send NBD_CMD_DISC. Other protocols may have similar cleanup issues, where failure to blk_unref() could cause unintended effects. Signed-off-by: Eric Blake Message-Id: <20190326184043.7544-1-eblake@redhat.com> Reviewed-by: John Snow Reviewed-by: Kevin Wolf --- qemu-img.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/qemu-img.c b/qemu-img.c index 03a9a10dec1..76a961df824 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -2736,14 +2736,14 @@ static int img_info(int argc, char **argv) return 0; } -static void dump_map_entry(OutputFormat output_format, MapEntry *e, - MapEntry *next) +static int dump_map_entry(OutputFormat output_format, MapEntry *e, + MapEntry *next) { switch (output_format) { case OFORMAT_HUMAN: if (e->data && !e->has_offset) { error_report("File contains external, encrypted or compressed clusters."); - exit(1); + return -1; } if (e->data && !e->zero) { printf("%#-16"PRIx64"%#-16"PRIx64"%#-16"PRIx64"%s\n", @@ -2776,6 +2776,7 @@ static void dump_map_entry(OutputFormat output_format, MapEntry *e, } break; } + return 0; } static int get_block_status(BlockDriverState *bs, int64_t offset, @@ -2968,12 +2969,15 @@ static int img_map(int argc, char **argv) } if (curr.length > 0) { - dump_map_entry(output_format, &curr, &next); + ret = dump_map_entry(output_format, &curr, &next); + if (ret < 0) { + goto out; + } } curr = next; } - dump_map_entry(output_format, &curr, NULL); + ret = dump_map_entry(output_format, &curr, NULL); out: blk_unref(blk); From patchwork Mon Apr 1 14:08:55 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 1072854 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 44Xvf3720yz9sPp for ; Tue, 2 Apr 2019 01:21:15 +1100 (AEDT) Received: from localhost ([127.0.0.1]:36206 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAxoI-0006BA-0G for incoming@patchwork.ozlabs.org; Mon, 01 Apr 2019 10:21:14 -0400 Received: from eggs.gnu.org ([209.51.188.92]:45885) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAxck-00021Q-V4 for qemu-devel@nongnu.org; Mon, 01 Apr 2019 10:09:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hAxcj-0004o3-0i for qemu-devel@nongnu.org; Mon, 01 Apr 2019 10:09:18 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49020) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hAxcg-0004kF-5l; Mon, 01 Apr 2019 10:09:14 -0400 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 776B719D05E; Mon, 1 Apr 2019 14:09:13 +0000 (UTC) Received: from blue.redhat.com (ovpn-116-75.phx2.redhat.com [10.3.116.75]) by smtp.corp.redhat.com (Postfix) with ESMTP id DBEF819C70; Mon, 1 Apr 2019 14:09:12 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Date: Mon, 1 Apr 2019 09:08:55 -0500 Message-Id: <20190401140903.19186-7-eblake@redhat.com> In-Reply-To: <20190401140903.19186-1-eblake@redhat.com> References: <20190401140903.19186-1-eblake@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Mon, 01 Apr 2019 14:09:13 +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] [PULL 06/14] nbd-client: Work around server BLOCK_STATUS misalignment at EOF 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 , Vladimir Sementsov-Ogievskiy , "open list:Network Block Dev..." , Max Reitz Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" The NBD spec is clear that a server that advertises a minimum block size should reply to NBD_CMD_BLOCK_STATUS with extents aligned accordingly. However, we know that the qemu NBD server implementation has had a corner-case bug where it is not compliant with the spec, present since the introduction of NBD_CMD_BLOCK_STATUS in qemu 2.12 (and unlikely to be patched in time for 4.0). Namely, when qemu is serving a file that is not a multiple of 512 bytes, it rounds the size advertised over NBD up to the next sector boundary (someday, I'd like to fix that to be byte-accurate, but it's a much bigger audit not appropriate for this release); yet if the final sector contains data prior to EOF, lseek(SEEK_HOLE) will point to the implicit hole mid-sector which qemu then reported over NBD. We are well within our rights to hang up on a server that can't follow the spec, but it is more useful to try and keep the connection alive in spite of the problem. Do so by tracing a message about the problem, and then either truncating the request back to an aligned boundary (if it covered more than the final sector) or widening it out to the full boundary with a forced status of data (since truncating would result in 0 bytes, but we have to make progress, and valid since data is a default-safe answer). And in practice, since the problem only happens on a sector that starts with data and ends with a hole, we are going to want to read that full sector anyway (where qemu as the server fills in the tail beyond EOF with appropriate NUL bytes). Easy reproduction: $ printf %1000d 1 > file $ qemu-nbd -f raw -t file & pid=$! $ qemu-img map --output=json -f raw nbd://localhost:10809 qemu-img: Could not read file metadata: Invalid argument $ kill $pid where the patched version instead succeeds with: [{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true}] Signed-off-by: Eric Blake Message-Id: <20190326171317.4036-1-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy --- block/nbd-client.c | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/block/nbd-client.c b/block/nbd-client.c index a3b70d14004..150af9cc46f 100644 --- a/block/nbd-client.c +++ b/block/nbd-client.c @@ -269,14 +269,36 @@ static int nbd_parse_blockstatus_payload(NBDClientSession *client, extent->length = payload_advance32(&payload); extent->flags = payload_advance32(&payload); - if (extent->length == 0 || - (client->info.min_block && !QEMU_IS_ALIGNED(extent->length, - client->info.min_block))) { + if (extent->length == 0) { error_setg(errp, "Protocol error: server sent status chunk with " - "invalid length"); + "zero length"); return -EINVAL; } + /* + * A server sending unaligned block status is in violation of the + * protocol, but as qemu-nbd 3.1 is such a server (at least for + * POSIX files that are not a multiple of 512 bytes, since qemu + * rounds files up to 512-byte multiples but lseek(SEEK_HOLE) + * still sees an implicit hole beyond the real EOF), it's nicer to + * work around the misbehaving server. If the request included + * more than the final unaligned block, truncate it back to an + * aligned result; if the request was only the final block, round + * up to the full block and change the status to fully-allocated + * (always a safe status, even if it loses information). + */ + if (client->info.min_block && !QEMU_IS_ALIGNED(extent->length, + client->info.min_block)) { + trace_nbd_parse_blockstatus_compliance("extent length is unaligned"); + if (extent->length > client->info.min_block) { + extent->length = QEMU_ALIGN_DOWN(extent->length, + client->info.min_block); + } else { + extent->length = client->info.min_block; + extent->flags = 0; + } + } + /* * We used NBD_CMD_FLAG_REQ_ONE, so the server should not have * sent us any more than one extent, nor should it have included From patchwork Mon Apr 1 14:08:56 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 1072859 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 44XvjL1sBlz9sPB for ; Tue, 2 Apr 2019 01:24:06 +1100 (AEDT) Received: from localhost ([127.0.0.1]:36917 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAxr2-0000LP-77 for incoming@patchwork.ozlabs.org; Mon, 01 Apr 2019 10:24:04 -0400 Received: from eggs.gnu.org ([209.51.188.92]:46010) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAxcs-0002B8-2x for qemu-devel@nongnu.org; Mon, 01 Apr 2019 10:09:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hAxco-0004wP-JT for qemu-devel@nongnu.org; Mon, 01 Apr 2019 10:09:24 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48524) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hAxch-0004km-0N; Mon, 01 Apr 2019 10:09:15 -0400 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 4FB0530BA356; Mon, 1 Apr 2019 14:09:14 +0000 (UTC) Received: from blue.redhat.com (ovpn-116-75.phx2.redhat.com [10.3.116.75]) by smtp.corp.redhat.com (Postfix) with ESMTP id A8A1D19C70; Mon, 1 Apr 2019 14:09:13 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Date: Mon, 1 Apr 2019 09:08:56 -0500 Message-Id: <20190401140903.19186-8-eblake@redhat.com> In-Reply-To: <20190401140903.19186-1-eblake@redhat.com> References: <20190401140903.19186-1-eblake@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.49]); Mon, 01 Apr 2019 14:09:14 +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] [PULL 07/14] iotests: Add 241 to test NBD on unaligned images 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 , Vladimir Sementsov-Ogievskiy , "open list:Block layer core" , Max Reitz Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Add a test for the NBD client workaround in the previous patch. It's not really feasible for an iotest to assume a specific tracing engine, so we can't really probe trace_nbd_parse_blockstatus_compliance to see if the server was fixed vs. whether the client just worked around the server (other than by rearranging order between code patches and this test). But having a successful exchange sure beats the previous state of an error message. Since format probing can change alignment, we can use that as an easy way to test several configurations. Not tested yet, but worth adding to this test in future patches: an NBD server that can advertise a non-sector-aligned size (such as nbdkit) causes qemu as the NBD client to misbehave when it rounds the size up and accesses beyond the advertised size. Qemu as NBD server never advertises a non-sector-aligned size (since bdrv_getlength() currently rounds up to sector boundaries); until qemu can act as such a server, testing that flaw will have to rely on external binaries. Signed-off-by: Eric Blake Message-Id: <20190329042750.14704-2-eblake@redhat.com> Tested-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Vladimir Sementsov-Ogievskiy [eblake: add forced-512 alignment, and nbdkit reproducer comment] --- tests/qemu-iotests/241 | 100 +++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/241.out | 26 ++++++++++ tests/qemu-iotests/group | 1 + 3 files changed, 127 insertions(+) create mode 100755 tests/qemu-iotests/241 create mode 100644 tests/qemu-iotests/241.out diff --git a/tests/qemu-iotests/241 b/tests/qemu-iotests/241 new file mode 100755 index 00000000000..4b196857387 --- /dev/null +++ b/tests/qemu-iotests/241 @@ -0,0 +1,100 @@ +#!/bin/bash +# +# Test qemu-nbd vs. unaligned images +# +# Copyright (C) 2018-2019 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +seq="$(basename $0)" +echo "QA output created by $seq" + +status=1 # failure is the default! + +nbd_unix_socket=$TEST_DIR/test_qemu_nbd_socket + +_cleanup() +{ + _cleanup_test_img + nbd_server_stop +} +trap "_cleanup; exit \$status" 0 1 2 3 15 + +# get standard environment, filters and checks +. ./common.rc +. ./common.filter +. ./common.nbd + +_supported_fmt raw +_supported_proto nbd +_supported_os Linux +_require_command QEMU_NBD + +# can't use _make_test_img, because qemu-img rounds image size up, +# and because we want to use Unix socket rather than TCP port. Likewise, +# we have to redirect TEST_IMG to our server. +# This tests that we can deal with the hole at the end of an unaligned +# raw file (either because the server doesn't advertise alignment too +# large, or because the client ignores the server's noncompliance - even +# though we can't actually wire iotests into checking trace messages). +printf %01000d 0 > "$TEST_IMG_FILE" +TEST_IMG="nbd:unix:$nbd_unix_socket" + +echo +echo "=== Exporting unaligned raw image, natural alignment ===" +echo + +nbd_server_start_unix_socket -f $IMGFMT "$TEST_IMG_FILE" + +$QEMU_NBD_PROG --list -k $nbd_unix_socket | grep '\(size\|min\)' +$QEMU_IMG map -f raw --output=json "$TEST_IMG" | _filter_qemu_img_map +$QEMU_IO -f raw -c map "$TEST_IMG" +nbd_server_stop + +echo +echo "=== Exporting unaligned raw image, forced server sector alignment ===" +echo + +# Intentionally omit '-f' to force image probing, which in turn forces +# sector alignment, here at the server. +nbd_server_start_unix_socket "$TEST_IMG_FILE" + +$QEMU_NBD_PROG --list -k $nbd_unix_socket | grep '\(size\|min\)' +$QEMU_IMG map -f raw --output=json "$TEST_IMG" | _filter_qemu_img_map +$QEMU_IO -f raw -c map "$TEST_IMG" +nbd_server_stop + +echo +echo "=== Exporting unaligned raw image, forced client sector alignment ===" +echo + +# Now force sector alignment at the client. +nbd_server_start_unix_socket -f $IMGFMT "$TEST_IMG_FILE" + +$QEMU_NBD_PROG --list -k $nbd_unix_socket | grep '\(size\|min\)' +$QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map +$QEMU_IO -c map "$TEST_IMG" +nbd_server_stop + +# Not tested yet: we also want to ensure that qemu as NBD client does +# not access beyond the end of a server's advertised unaligned size: +# nbdkit -U - memory size=513 --run 'qemu-io -f raw -c "r 512 512" $nbd' +# However, since qemu as server always rounds up to a sector alignment, +# we would have to use nbdkit to provoke the current client failures. + +# success, all done +echo '*** done' +rm -f $seq.full +status=0 diff --git a/tests/qemu-iotests/241.out b/tests/qemu-iotests/241.out new file mode 100644 index 00000000000..b76a6234d72 --- /dev/null +++ b/tests/qemu-iotests/241.out @@ -0,0 +1,26 @@ +QA output created by 241 + +=== Exporting unaligned raw image, natural alignment === + + size: 1024 + min block: 512 +[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true}] +1 KiB (0x400) bytes allocated at offset 0 bytes (0x0) + +=== Exporting unaligned raw image, forced server sector alignment === + +WARNING: Image format was not specified for '/home/eblake/qemu/tests/qemu-iotests/scratch/t.raw' and probing guessed raw. + Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted. + Specify the 'raw' format explicitly to remove the restrictions. + size: 1024 + min block: 512 +[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true}] +1 KiB (0x400) bytes allocated at offset 0 bytes (0x0) + +=== Exporting unaligned raw image, forced client sector alignment === + + size: 1024 + min block: 512 +[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true}] +1 KiB (0x400) bytes allocated at offset 0 bytes (0x0) +*** done diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index 41da10c6cf5..bae77183809 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -240,6 +240,7 @@ 238 auto quick 239 rw auto quick 240 auto quick +241 rw auto quick 242 rw auto quick 243 rw auto quick 244 rw auto quick From patchwork Mon Apr 1 14:08:57 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 1072870 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 44Xvqq2WhVz9sPj for ; Tue, 2 Apr 2019 01:29:42 +1100 (AEDT) Received: from localhost ([127.0.0.1]:38490 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAxwS-0004y3-Tb for incoming@patchwork.ozlabs.org; Mon, 01 Apr 2019 10:29:40 -0400 Received: from eggs.gnu.org ([209.51.188.92]:45939) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAxco-000257-BU for qemu-devel@nongnu.org; Mon, 01 Apr 2019 10:09:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hAxcl-0004rP-4q for qemu-devel@nongnu.org; Mon, 01 Apr 2019 10:09:22 -0400 Received: from mx1.redhat.com ([209.132.183.28]:40326) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hAxch-0004l7-QL; Mon, 01 Apr 2019 10:09:16 -0400 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 25AD63097034; Mon, 1 Apr 2019 14:09:15 +0000 (UTC) Received: from blue.redhat.com (ovpn-116-75.phx2.redhat.com [10.3.116.75]) by smtp.corp.redhat.com (Postfix) with ESMTP id 7430E19C70; Mon, 1 Apr 2019 14:09:14 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Date: Mon, 1 Apr 2019 09:08:57 -0500 Message-Id: <20190401140903.19186-9-eblake@redhat.com> In-Reply-To: <20190401140903.19186-1-eblake@redhat.com> References: <20190401140903.19186-1-eblake@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.43]); Mon, 01 Apr 2019 14:09:15 +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] [PULL 08/14] nbd/client: Lower min_block for block-status, unaligned size 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 , Vladimir Sementsov-Ogievskiy , "Richard W . M . Jones" , "open list:Network Block Dev..." , Max Reitz Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" We have a latent bug in our NBD client code, tickled by the brand new nbdkit 1.11.10 block status support: $ nbdkit --filter=log --filter=truncate -U - \ data data="1" size=511 truncate=64K logfile=/dev/stdout \ --run 'qemu-img convert $nbd /var/tmp/out' ... qemu-img: block/io.c:2122: bdrv_co_block_status: Assertion `*pnum && QEMU_IS_ALIGNED(*pnum, align) && align > offset - aligned_offset' failed. The culprit? Our implementation of .bdrv_co_block_status can return unaligned block status for any server that operates with a lower actual alignment than what we tell the block layer in request_alignment, in violation of the block layer's constraints. To date, we've been unable to trip the bug, because qemu as NBD server always advertises block sizing (at which point it is a server bug if the server sends unaligned status - although qemu 3.1 is such a server and I've sent separate patches for 4.0 both to get the server to obey the spec, and to let the client to tolerate server oddities at EOF). But nbdkit does not (yet) advertise block sizing, and therefore is not in violation of the spec for returning block status at whatever boundaries it wants, and those unaligned results can occur anywhere rather than just at EOF. While we are still wise to avoid sending sub-sector read/write requests to a server of unknown origin, we MUST consider that a server telling us block status without an advertised block size is correct. So, we either have to munge unaligned answers from the server into aligned ones that we hand back to the block layer, or we have to tell the block layer about a smaller alignment. Similarly, if the server advertises an image size that is not sector-aligned, we might as well assume that the server intends to let us access those tail bytes, and therefore supports a minimum block size of 1, regardless of whether the server supports block status (although we still need more patches to fix the problem that with an unaligned image, we can send read or block status requests that exceed EOF to the server). Again, qemu as server cannot trip this problem (because it rounds images to sector alignment), but nbdkit advertised unaligned size even before it gained block status support. Solve both alignment problems at once by using better heuristics on what alignment to report to the block layer when the server did not give us something to work with. Note that very few NBD servers implement block status (to date, only qemu and nbdkit are known to do so); and as the NBD spec mentioned block sizing constraints prior to documenting block status, it can be assumed that any future implementations of block status are aware that they must advertise block size if they want a minimum size other than 1. We've had a long history of struggles with picking the right alignment to use in the block layer, as evidenced by the commit message of fd8d372d (v2.12) that introduced the current choice of forced 512-byte alignment. There is no iotest coverage for this fix, because qemu can't provoke it, and I didn't want to make test 241 dependent on nbdkit. Fixes: fd8d372d Reported-by: Richard W.M. Jones Signed-off-by: Eric Blake Message-Id: <20190329042750.14704-3-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy Tested-by: Richard W.M. Jones --- block/nbd.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/block/nbd.c b/block/nbd.c index 2e72df528ac..208be596027 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -437,7 +437,24 @@ static void nbd_refresh_limits(BlockDriverState *bs, Error **errp) uint32_t min = s->info.min_block; uint32_t max = MIN_NON_ZERO(NBD_MAX_BUFFER_SIZE, s->info.max_block); - bs->bl.request_alignment = min ? min : BDRV_SECTOR_SIZE; + /* + * If the server did not advertise an alignment: + * - a size that is not sector-aligned implies that an alignment + * of 1 can be used to access those tail bytes + * - advertisement of block status requires an alignment of 1, so + * that we don't violate block layer constraints that block + * status is always aligned (as we can't control whether the + * server will report sub-sector extents, such as a hole at EOF + * on an unaligned POSIX file) + * - otherwise, assume the server is so old that we are safer avoiding + * sub-sector requests + */ + if (!min) { + min = (!QEMU_IS_ALIGNED(s->info.size, BDRV_SECTOR_SIZE) || + s->info.base_allocation) ? 1 : BDRV_SECTOR_SIZE; + } + + bs->bl.request_alignment = min; bs->bl.max_pdiscard = max; bs->bl.max_pwrite_zeroes = max; bs->bl.max_transfer = max; From patchwork Mon Apr 1 14:08:58 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 1072881 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 44Xvzb3F9lz9sQt for ; Tue, 2 Apr 2019 01:36:27 +1100 (AEDT) Received: from localhost ([127.0.0.1]:40148 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAy2z-0001pV-E3 for incoming@patchwork.ozlabs.org; Mon, 01 Apr 2019 10:36:25 -0400 Received: from eggs.gnu.org ([209.51.188.92]:45984) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAxcq-00027u-7Y for qemu-devel@nongnu.org; Mon, 01 Apr 2019 10:09:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hAxco-0004vU-BP for qemu-devel@nongnu.org; Mon, 01 Apr 2019 10:09:24 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38448) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hAxci-0004mi-NN; Mon, 01 Apr 2019 10:09:16 -0400 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 09A5C356F3; Mon, 1 Apr 2019 14:09:16 +0000 (UTC) Received: from blue.redhat.com (ovpn-116-75.phx2.redhat.com [10.3.116.75]) by smtp.corp.redhat.com (Postfix) with ESMTP id 57D5519C70; Mon, 1 Apr 2019 14:09:15 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Date: Mon, 1 Apr 2019 09:08:58 -0500 Message-Id: <20190401140903.19186-10-eblake@redhat.com> In-Reply-To: <20190401140903.19186-1-eblake@redhat.com> References: <20190401140903.19186-1-eblake@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Mon, 01 Apr 2019 14:09:16 +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] [PULL 09/14] nbd/client: Report offsets in bdrv_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 , Vladimir Sementsov-Ogievskiy , "Richard W . M . Jones" , "open list:Network Block Dev..." , Max Reitz Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" It is desirable for 'qemu-img map' to have the same output for a file whether it is served over file or nbd protocols. However, ever since we implemented block status for NBD (2.12), the NBD protocol forgot to inform the block layer that as the final layer in the chain, the offset is valid; without an offset, the human-readable form of qemu-img map gives up with the unhelpful: $ nbdkit -U - data data="1" size=512 --run 'qemu-img map $nbd' Offset Length Mapped to File qemu-img: File contains external, encrypted or compressed clusters. The --output=json form always works, because it is reporting the lower-level bdrv_block_status results directly rather than trying to filter out sparse ranges for human consumption - but now it also shows the offset member. With this patch, the human output changes to: Offset Length Mapped to File 0 0x200 0 nbd+unix://?socket=/tmp/nbdkitOxeoLa/socket This change is observable to several iotests. Fixes: 78a33ab5 Reported-by: Richard W.M. Jones Signed-off-by: Eric Blake Message-Id: <20190329042750.14704-4-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy --- block/nbd-client.c | 9 +++++++-- tests/qemu-iotests/209.out | 4 ++-- tests/qemu-iotests/223.out | 18 +++++++++--------- tests/qemu-iotests/241.out | 6 +++--- 4 files changed, 21 insertions(+), 16 deletions(-) diff --git a/block/nbd-client.c b/block/nbd-client.c index 150af9cc46f..3edb508f668 100644 --- a/block/nbd-client.c +++ b/block/nbd-client.c @@ -972,7 +972,9 @@ int coroutine_fn nbd_client_co_block_status(BlockDriverState *bs, if (!client->info.base_allocation) { *pnum = bytes; - return BDRV_BLOCK_DATA; + *map = offset; + *file = bs; + return BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID; } ret = nbd_co_send_request(bs, &request, NULL); @@ -995,8 +997,11 @@ int coroutine_fn nbd_client_co_block_status(BlockDriverState *bs, assert(extent.length); *pnum = extent.length; + *map = offset; + *file = bs; return (extent.flags & NBD_STATE_HOLE ? 0 : BDRV_BLOCK_DATA) | - (extent.flags & NBD_STATE_ZERO ? BDRV_BLOCK_ZERO : 0); + (extent.flags & NBD_STATE_ZERO ? BDRV_BLOCK_ZERO : 0) | + BDRV_BLOCK_OFFSET_VALID; } void nbd_client_detach_aio_context(BlockDriverState *bs) diff --git a/tests/qemu-iotests/209.out b/tests/qemu-iotests/209.out index 0d29724e84a..214e27bfcec 100644 --- a/tests/qemu-iotests/209.out +++ b/tests/qemu-iotests/209.out @@ -1,2 +1,2 @@ -[{ "start": 0, "length": 524288, "depth": 0, "zero": false, "data": true}, -{ "start": 524288, "length": 524288, "depth": 0, "zero": true, "data": false}] +[{ "start": 0, "length": 524288, "depth": 0, "zero": false, "data": true, "offset": 0}, +{ "start": 524288, "length": 524288, "depth": 0, "zero": true, "data": false, "offset": 524288}] diff --git a/tests/qemu-iotests/223.out b/tests/qemu-iotests/223.out index 95c40a17ad7..7828a447392 100644 --- a/tests/qemu-iotests/223.out +++ b/tests/qemu-iotests/223.out @@ -67,18 +67,18 @@ read 1048576/1048576 bytes at offset 1048576 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) read 2097152/2097152 bytes at offset 2097152 2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true}, -{ "start": 4096, "length": 1044480, "depth": 0, "zero": true, "data": false}, -{ "start": 1048576, "length": 3145728, "depth": 0, "zero": false, "data": true}] +[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": OFFSET}, +{ "start": 4096, "length": 1044480, "depth": 0, "zero": true, "data": false, "offset": OFFSET}, +{ "start": 1048576, "length": 3145728, "depth": 0, "zero": false, "data": true, "offset": OFFSET}] [{ "start": 0, "length": 65536, "depth": 0, "zero": false, "data": false}, -{ "start": 65536, "length": 2031616, "depth": 0, "zero": false, "data": true}, +{ "start": 65536, "length": 2031616, "depth": 0, "zero": false, "data": true, "offset": OFFSET}, { "start": 2097152, "length": 2097152, "depth": 0, "zero": false, "data": false}] === Contrast to small granularity dirty-bitmap === -[{ "start": 0, "length": 512, "depth": 0, "zero": false, "data": true}, +[{ "start": 0, "length": 512, "depth": 0, "zero": false, "data": true, "offset": OFFSET}, { "start": 512, "length": 512, "depth": 0, "zero": false, "data": false}, -{ "start": 1024, "length": 2096128, "depth": 0, "zero": false, "data": true}, +{ "start": 1024, "length": 2096128, "depth": 0, "zero": false, "data": true, "offset": OFFSET}, { "start": 2097152, "length": 2097152, "depth": 0, "zero": false, "data": false}] === End qemu NBD server === @@ -94,10 +94,10 @@ read 2097152/2097152 bytes at offset 2097152 === Use qemu-nbd as server === [{ "start": 0, "length": 65536, "depth": 0, "zero": false, "data": false}, -{ "start": 65536, "length": 2031616, "depth": 0, "zero": false, "data": true}, +{ "start": 65536, "length": 2031616, "depth": 0, "zero": false, "data": true, "offset": OFFSET}, { "start": 2097152, "length": 2097152, "depth": 0, "zero": false, "data": false}] -[{ "start": 0, "length": 512, "depth": 0, "zero": false, "data": true}, +[{ "start": 0, "length": 512, "depth": 0, "zero": false, "data": true, "offset": OFFSET}, { "start": 512, "length": 512, "depth": 0, "zero": false, "data": false}, -{ "start": 1024, "length": 2096128, "depth": 0, "zero": false, "data": true}, +{ "start": 1024, "length": 2096128, "depth": 0, "zero": false, "data": true, "offset": OFFSET}, { "start": 2097152, "length": 2097152, "depth": 0, "zero": false, "data": false}] *** done diff --git a/tests/qemu-iotests/241.out b/tests/qemu-iotests/241.out index b76a6234d72..f22eabbf324 100644 --- a/tests/qemu-iotests/241.out +++ b/tests/qemu-iotests/241.out @@ -4,7 +4,7 @@ QA output created by 241 size: 1024 min block: 512 -[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true}] +[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true, "offset": OFFSET}] 1 KiB (0x400) bytes allocated at offset 0 bytes (0x0) === Exporting unaligned raw image, forced server sector alignment === @@ -14,13 +14,13 @@ WARNING: Image format was not specified for '/home/eblake/qemu/tests/qemu-iotest Specify the 'raw' format explicitly to remove the restrictions. size: 1024 min block: 512 -[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true}] +[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true, "offset": OFFSET}] 1 KiB (0x400) bytes allocated at offset 0 bytes (0x0) === Exporting unaligned raw image, forced client sector alignment === size: 1024 min block: 512 -[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true}] +[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true, "offset": OFFSET}] 1 KiB (0x400) bytes allocated at offset 0 bytes (0x0) *** done From patchwork Mon Apr 1 14:08:59 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 1072860 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 44XvmV45wnz9sPv for ; Tue, 2 Apr 2019 01:26:47 +1100 (AEDT) Received: from localhost ([127.0.0.1]:37660 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAxtc-0002WN-Io for incoming@patchwork.ozlabs.org; Mon, 01 Apr 2019 10:26:44 -0400 Received: from eggs.gnu.org ([209.51.188.92]:45942) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAxco-00025N-H0 for qemu-devel@nongnu.org; Mon, 01 Apr 2019 10:09:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hAxcm-0004sT-CJ for qemu-devel@nongnu.org; Mon, 01 Apr 2019 10:09:22 -0400 Received: from mx1.redhat.com ([209.132.183.28]:40336) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hAxcj-0004nt-9j; Mon, 01 Apr 2019 10:09:17 -0400 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 9D21F309267E; Mon, 1 Apr 2019 14:09:16 +0000 (UTC) Received: from blue.redhat.com (ovpn-116-75.phx2.redhat.com [10.3.116.75]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3478A19C70; Mon, 1 Apr 2019 14:09:16 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Date: Mon, 1 Apr 2019 09:08:59 -0500 Message-Id: <20190401140903.19186-11-eblake@redhat.com> In-Reply-To: <20190401140903.19186-1-eblake@redhat.com> References: <20190401140903.19186-1-eblake@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.43]); Mon, 01 Apr 2019 14:09:16 +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] [PULL 10/14] nbd/client: Reject inaccessible tail of inconsistent server 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: Vladimir Sementsov-Ogievskiy , "open list:Network Block Dev..." Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" The NBD spec suggests that a server should never advertise a size inconsistent with its minimum block alignment, as that tail is effectively inaccessible to a compliant client obeying those block constraints. Since we have a habit of rounding up rather than truncating, to avoid losing the last few bytes of user input, and we cannot access the tail when the server advertises bogus block sizing, abort the connection to alert the server to fix their bug. And rejecting such servers matches what we already did for a min_block that was not a power of 2 or which was larger than max_block. Does not impact either qemu (which always sends properly aligned sizes) or nbdkit (which does not send minimum block requirements yet); so this is mostly aimed at new NBD server implementations, and ensures that the rest of our code can assume the size is aligned. Signed-off-by: Eric Blake Message-Id: <20190330155704.24191-1-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy --- nbd/client.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/nbd/client.c b/nbd/client.c index de7da48246b..427980bdd22 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -426,6 +426,14 @@ static int nbd_opt_info_or_go(QIOChannel *ioc, uint32_t opt, nbd_send_opt_abort(ioc); return -1; } + if (info->min_block && + !QEMU_IS_ALIGNED(info->size, info->min_block)) { + error_setg(errp, "export size %" PRIu64 "is not multiple of " + "minimum block size %" PRIu32, info->size, + info->min_block); + nbd_send_opt_abort(ioc); + return -1; + } trace_nbd_receive_negotiate_size_flags(info->size, info->flags); break; From patchwork Mon Apr 1 14:09:00 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 1072875 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 44Xvvt2Qvvz9sPv for ; Tue, 2 Apr 2019 01:33:14 +1100 (AEDT) Received: from localhost ([127.0.0.1]:39350 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAxzs-0007hA-8k for incoming@patchwork.ozlabs.org; Mon, 01 Apr 2019 10:33:12 -0400 Received: from eggs.gnu.org ([209.51.188.92]:45978) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAxcp-00027E-RX for qemu-devel@nongnu.org; Mon, 01 Apr 2019 10:09:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hAxco-0004vW-BY for qemu-devel@nongnu.org; Mon, 01 Apr 2019 10:09:23 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48538) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hAxck-0004pb-59; Mon, 01 Apr 2019 10:09:18 -0400 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6EBC630BA356; Mon, 1 Apr 2019 14:09:17 +0000 (UTC) Received: from blue.redhat.com (ovpn-116-75.phx2.redhat.com [10.3.116.75]) by smtp.corp.redhat.com (Postfix) with ESMTP id CCE6327BC7; Mon, 1 Apr 2019 14:09:16 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Date: Mon, 1 Apr 2019 09:09:00 -0500 Message-Id: <20190401140903.19186-12-eblake@redhat.com> In-Reply-To: <20190401140903.19186-1-eblake@redhat.com> References: <20190401140903.19186-1-eblake@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.49]); Mon, 01 Apr 2019 14:09:17 +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] [PULL 11/14] nbd/client: Support qemu-img convert from unaligned size 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 , "Richard W . M . Jones" , "open list:Network Block Dev..." , Max Reitz Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" If an NBD server advertises a size that is not a multiple of a sector, the block layer rounds up that size, even though we set info.size to the exact byte value sent by the server. The block layer then proceeds to let us read or query block status on the hole that it added past EOF, which the NBD server is unlikely to be happy with. Fortunately, qemu as a server never advertizes an unaligned size, so we generally don't run into this problem; but the nbdkit server makes it easy to test: $ printf %1000d 1 > f1 $ ~/nbdkit/nbdkit -fv file f1 & pid=$! $ qemu-img convert -f raw nbd://localhost:10809 f2 $ kill $pid $ qemu-img compare f1 f2 Pre-patch, the server attempts a 1024-byte read, which nbdkit rightfully rejects as going beyond its advertised 1000 byte size; the conversion fails and the output files differ (not even the first sector is copied, because qemu-img does not follow ddrescue's habit of trying smaller reads to get as much information as possible in spite of errors). Post-patch, the client's attempts to read (and query block status, for new enough nbdkit) are properly truncated to the server's length, with sane handling of the hole the block layer forced on us. Although f2 ends up as a larger file (1024 bytes instead of 1000), qemu-img compare shows the two images to have identical contents for display to the guest. I didn't add iotests coverage since I didn't want to add a dependency on nbdkit in iotests. I also did NOT patch write, trim, or write zeroes - these commands continue to fail (usually with ENOSPC, but whatever the server chose), because we really can't write to the end of the file, and because 'qemu-img convert' is the most common case where we care about being tolerant (which is read-only). Perhaps we could truncate the request if the client is writing zeros to the tail, but that seems like more work, especially if the block layer is fixed in 4.1 to track byte-accurate sizing (in which case this patch would be reverted as unnecessary). Signed-off-by: Eric Blake Message-Id: <20190329042750.14704-5-eblake@redhat.com> Tested-by: Richard W.M. Jones --- block/nbd-client.c | 39 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/block/nbd-client.c b/block/nbd-client.c index 3edb508f668..409c2171bc3 100644 --- a/block/nbd-client.c +++ b/block/nbd-client.c @@ -848,6 +848,25 @@ int nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset, if (!bytes) { return 0; } + /* + * Work around the fact that the block layer doesn't do + * byte-accurate sizing yet - if the read exceeds the server's + * advertised size because the block layer rounded size up, then + * truncate the request to the server and tail-pad with zero. + */ + if (offset >= client->info.size) { + assert(bytes < BDRV_SECTOR_SIZE); + qemu_iovec_memset(qiov, 0, 0, bytes); + return 0; + } + if (offset + bytes > client->info.size) { + uint64_t slop = offset + bytes - client->info.size; + + assert(slop < BDRV_SECTOR_SIZE); + qemu_iovec_memset(qiov, bytes - slop, 0, slop); + request.len -= slop; + } + ret = nbd_co_send_request(bs, &request, NULL); if (ret < 0) { return ret; @@ -966,7 +985,8 @@ int coroutine_fn nbd_client_co_block_status(BlockDriverState *bs, .from = offset, .len = MIN(MIN_NON_ZERO(QEMU_ALIGN_DOWN(INT_MAX, bs->bl.request_alignment), - client->info.max_block), bytes), + client->info.max_block), + MIN(bytes, client->info.size - offset)), .flags = NBD_CMD_FLAG_REQ_ONE, }; @@ -977,6 +997,23 @@ int coroutine_fn nbd_client_co_block_status(BlockDriverState *bs, return BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID; } + /* + * Work around the fact that the block layer doesn't do + * byte-accurate sizing yet - if the status request exceeds the + * server's advertised size because the block layer rounded size + * up, we truncated the request to the server (above), or are + * called on just the hole. + */ + if (offset >= client->info.size) { + *pnum = bytes; + assert(bytes < BDRV_SECTOR_SIZE); + /* Intentionally don't report offset_valid for the hole */ + return BDRV_BLOCK_ZERO; + } + + if (client->info.min_block) { + assert(QEMU_IS_ALIGNED(request.len, client->info.min_block)); + } ret = nbd_co_send_request(bs, &request, NULL); if (ret < 0) { return ret; From patchwork Mon Apr 1 14:09:01 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 1072853 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 44Xvdn2vjSz9sPp for ; Tue, 2 Apr 2019 01:21:01 +1100 (AEDT) Received: from localhost ([127.0.0.1]:36109 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAxo3-0005wI-CK for incoming@patchwork.ozlabs.org; Mon, 01 Apr 2019 10:20:59 -0400 Received: from eggs.gnu.org ([209.51.188.92]:46014) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAxcs-0002BG-5P for qemu-devel@nongnu.org; Mon, 01 Apr 2019 10:09:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hAxcp-0004xq-Ni for qemu-devel@nongnu.org; Mon, 01 Apr 2019 10:09:26 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48762) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hAxcm-0004rc-6w; Mon, 01 Apr 2019 10:09:20 -0400 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 3B7A29F728; Mon, 1 Apr 2019 14:09:19 +0000 (UTC) Received: from blue.redhat.com (ovpn-116-75.phx2.redhat.com [10.3.116.75]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9898A19C70; Mon, 1 Apr 2019 14:09:17 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Date: Mon, 1 Apr 2019 09:09:01 -0500 Message-Id: <20190401140903.19186-13-eblake@redhat.com> In-Reply-To: <20190401140903.19186-1-eblake@redhat.com> References: <20190401140903.19186-1-eblake@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Mon, 01 Apr 2019 14:09:19 +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] [PULL 12/14] block: Add bdrv_get_request_alignment() 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 , Vladimir Sementsov-Ogievskiy , "open list:Block layer core" , Max Reitz Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" The next patch needs access to a device's minimum permitted alignment, since NBD wants to advertise this to clients. Add an accessor function, borrowing from blk_get_max_transfer() for accessing a backend's block limits. Signed-off-by: Eric Blake Reviewed-by: Vladimir Sementsov-Ogievskiy Message-Id: <20190329042750.14704-6-eblake@redhat.com> --- include/sysemu/block-backend.h | 1 + block/block-backend.c | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index e2066eb06b2..3be05c2d689 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -177,6 +177,7 @@ bool blk_is_available(BlockBackend *blk); void blk_lock_medium(BlockBackend *blk, bool locked); void blk_eject(BlockBackend *blk, bool eject_flag); int blk_get_flags(BlockBackend *blk); +uint32_t blk_get_request_alignment(BlockBackend *blk); uint32_t blk_get_max_transfer(BlockBackend *blk); int blk_get_max_iov(BlockBackend *blk); void blk_set_guest_block_size(BlockBackend *blk, int align); diff --git a/block/block-backend.c b/block/block-backend.c index edad02a0f2a..f78e82a707f 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -1764,6 +1764,13 @@ int blk_get_flags(BlockBackend *blk) } } +/* Returns the minimum request alignment, in bytes; guaranteed nonzero */ +uint32_t blk_get_request_alignment(BlockBackend *blk) +{ + BlockDriverState *bs = blk_bs(blk); + return bs ? bs->bl.request_alignment : BDRV_SECTOR_SIZE; +} + /* Returns the maximum transfer length, in bytes; guaranteed nonzero */ uint32_t blk_get_max_transfer(BlockBackend *blk) { From patchwork Mon Apr 1 14:09:02 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 1072888 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 44Xw2M1WCYz9sPj for ; Tue, 2 Apr 2019 01:38:50 +1100 (AEDT) Received: from localhost ([127.0.0.1]:40780 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAy5I-0003a2-G5 for incoming@patchwork.ozlabs.org; Mon, 01 Apr 2019 10:38:48 -0400 Received: from eggs.gnu.org ([209.51.188.92]:46034) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAxcv-0002FF-Rp for qemu-devel@nongnu.org; Mon, 01 Apr 2019 10:09:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hAxcs-00050k-5e for qemu-devel@nongnu.org; Mon, 01 Apr 2019 10:09:27 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49090) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hAxco-0004s0-BL; Mon, 01 Apr 2019 10:09:22 -0400 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 08CCC91FEC; Mon, 1 Apr 2019 14:09:20 +0000 (UTC) Received: from blue.redhat.com (ovpn-116-75.phx2.redhat.com [10.3.116.75]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6959927CB3; Mon, 1 Apr 2019 14:09:19 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Date: Mon, 1 Apr 2019 09:09:02 -0500 Message-Id: <20190401140903.19186-14-eblake@redhat.com> In-Reply-To: <20190401140903.19186-1-eblake@redhat.com> References: <20190401140903.19186-1-eblake@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Mon, 01 Apr 2019 14:09:20 +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] [PULL 13/14] nbd/server: Advertise actual minimum block size 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 , Vladimir Sementsov-Ogievskiy , "open list:Network Block Dev..." , Max Reitz Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Both NBD_CMD_BLOCK_STATUS and structured NBD_CMD_READ will split their reply according to bdrv_block_status() boundaries. If the block device has a request_alignment smaller than 512, but we advertise a block alignment of 512 to the client, then this can result in the server reply violating client expectations by reporting a smaller region of the export than what the client is permitted to address (although this is less of an issue for qemu 4.0 clients, given recent client patches to overlook our non-compliance at EOF). Since it's always better to be strict in what we send, it is worth advertising the actual minimum block limit rather than blindly rounding it up to 512. Note that this patch is not foolproof - it is still possible to provoke non-compliant server behavior using: $ qemu-nbd --image-opts driver=blkdebug,align=512,image.driver=file,image.filename=/path/to/non-aligned-file That is arguably a bug in the blkdebug driver (it should never pass back block status smaller than its alignment, even if it has to make multiple bdrv_get_status calls and determine the least-common-denominator status among the group to return). It may also be possible to observe issues with a backing layer with smaller alignment than the active layer, although so far I have been unable to write a reliable iotest for that scenario (but again, an issue like that could be argued to be a bug in the block layer, or something where we need a flag to bdrv_block_status() to state whether the result must be aligned to the current layer's limits or can be subdivided for accuracy when chasing backing files). Anyways, as blkdebug is not normally used, and as this patch makes our server more interoperable with qemu 3.1 clients, it is worth applying now, even while we still work on a larger patch series for the 4.1 timeframe to have byte-accurate file lengths. Note that the iotests output changes - for 223 and 233, we can see the server's better granularity advertisement; and for 241, the three test cases have the following effects: - natural alignment: the server's smaller alignment is now advertised, and the hole reported at EOF is now the right result; we've gotten rid of the server's non-compliance - forced server alignment: the server still advertises 512 bytes, but still sends a mid-sector hole. This is still a server compliance bug, which needs to be fixed in the block layer in a later patch; output does not change because the client is already being tolerant of the non-compliance - forced client alignment: the server's smaller alignment means that the client now sees the server's status change mid-sector without any protocol violations, but the fact that the map shows an unaligned mid-sector hole is evidence of the block layer problems with aligned block status, to be fixed in a later patch Signed-off-by: Eric Blake Message-Id: <20190329042750.14704-7-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy [eblake: rebase to enhanced iotest 241 coverage] --- nbd/server.c | 13 ++++++++----- tests/qemu-iotests/223.out | 4 ++-- tests/qemu-iotests/233.out | 2 +- tests/qemu-iotests/241.out | 10 ++++++---- 4 files changed, 17 insertions(+), 12 deletions(-) diff --git a/nbd/server.c b/nbd/server.c index fd013a2817a..218a2aa5e65 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -607,13 +607,16 @@ static int nbd_negotiate_handle_info(NBDClient *client, uint16_t myflags, /* Send NBD_INFO_BLOCK_SIZE always, but tweak the minimum size * according to whether the client requested it, and according to * whether this is OPT_INFO or OPT_GO. */ - /* minimum - 1 for back-compat, or 512 if client is new enough. - * TODO: consult blk_bs(blk)->bl.request_alignment? */ - sizes[0] = - (client->opt == NBD_OPT_INFO || blocksize) ? BDRV_SECTOR_SIZE : 1; + /* minimum - 1 for back-compat, or actual if client will obey it. */ + if (client->opt == NBD_OPT_INFO || blocksize) { + sizes[0] = blk_get_request_alignment(exp->blk); + } else { + sizes[0] = 1; + } + assert(sizes[0] <= NBD_MAX_BUFFER_SIZE); /* preferred - Hard-code to 4096 for now. * TODO: is blk_bs(blk)->bl.opt_transfer appropriate? */ - sizes[1] = 4096; + sizes[1] = MAX(4096, sizes[0]); /* maximum - At most 32M, but smaller as appropriate. */ sizes[2] = MIN(blk_get_max_transfer(exp->blk), NBD_MAX_BUFFER_SIZE); trace_nbd_negotiate_handle_info_block_size(sizes[0], sizes[1], sizes[2]); diff --git a/tests/qemu-iotests/223.out b/tests/qemu-iotests/223.out index 7828a447392..d5201b2356a 100644 --- a/tests/qemu-iotests/223.out +++ b/tests/qemu-iotests/223.out @@ -41,7 +41,7 @@ exports available: 2 export: 'n' size: 4194304 flags: 0x4ef ( readonly flush fua trim zeroes df cache ) - min block: 512 + min block: 1 opt block: 4096 max block: 33554432 available meta contexts: 2 @@ -50,7 +50,7 @@ exports available: 2 export: 'n2' size: 4194304 flags: 0x4ed ( flush fua trim zeroes df cache ) - min block: 512 + min block: 1 opt block: 4096 max block: 33554432 available meta contexts: 2 diff --git a/tests/qemu-iotests/233.out b/tests/qemu-iotests/233.out index 5acbc13b54a..9511b6ea658 100644 --- a/tests/qemu-iotests/233.out +++ b/tests/qemu-iotests/233.out @@ -38,7 +38,7 @@ exports available: 1 export: '' size: 67108864 flags: 0x4ed ( flush fua trim zeroes df cache ) - min block: 512 + min block: 1 opt block: 4096 max block: 33554432 available meta contexts: 1 diff --git a/tests/qemu-iotests/241.out b/tests/qemu-iotests/241.out index f22eabbf324..f481074a02e 100644 --- a/tests/qemu-iotests/241.out +++ b/tests/qemu-iotests/241.out @@ -3,8 +3,9 @@ QA output created by 241 === Exporting unaligned raw image, natural alignment === size: 1024 - min block: 512 -[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true, "offset": OFFSET}] + min block: 1 +[{ "start": 0, "length": 1000, "depth": 0, "zero": false, "data": true, "offset": OFFSET}, +{ "start": 1000, "length": 24, "depth": 0, "zero": true, "data": true, "offset": OFFSET}] 1 KiB (0x400) bytes allocated at offset 0 bytes (0x0) === Exporting unaligned raw image, forced server sector alignment === @@ -20,7 +21,8 @@ WARNING: Image format was not specified for '/home/eblake/qemu/tests/qemu-iotest === Exporting unaligned raw image, forced client sector alignment === size: 1024 - min block: 512 -[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true, "offset": OFFSET}] + min block: 1 +[{ "start": 0, "length": 1000, "depth": 0, "zero": false, "data": true, "offset": OFFSET}, +{ "start": 1000, "length": 24, "depth": 0, "zero": true, "data": true, "offset": OFFSET}] 1 KiB (0x400) bytes allocated at offset 0 bytes (0x0) *** done From patchwork Mon Apr 1 14:09: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: 1072861 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 44Xvmw3v4hz9sPB for ; Tue, 2 Apr 2019 01:27:12 +1100 (AEDT) Received: from localhost ([127.0.0.1]:37781 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAxu2-0002tr-DF for incoming@patchwork.ozlabs.org; Mon, 01 Apr 2019 10:27:10 -0400 Received: from eggs.gnu.org ([209.51.188.92]:46032) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAxcv-0002FD-Qq for qemu-devel@nongnu.org; Mon, 01 Apr 2019 10:09:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hAxcs-00050l-5Z for qemu-devel@nongnu.org; Mon, 01 Apr 2019 10:09:27 -0400 Received: from mx1.redhat.com ([209.132.183.28]:52308) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hAxco-0004sn-Gn; Mon, 01 Apr 2019 10:09:22 -0400 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id CB2B785A04; Mon, 1 Apr 2019 14:09:20 +0000 (UTC) Received: from blue.redhat.com (ovpn-116-75.phx2.redhat.com [10.3.116.75]) by smtp.corp.redhat.com (Postfix) with ESMTP id 372D827CB3; Mon, 1 Apr 2019 14:09:20 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Date: Mon, 1 Apr 2019 09:09:03 -0500 Message-Id: <20190401140903.19186-15-eblake@redhat.com> In-Reply-To: <20190401140903.19186-1-eblake@redhat.com> References: <20190401140903.19186-1-eblake@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Mon, 01 Apr 2019 14:09:20 +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] [PULL 14/14] nbd/client: Trace server noncompliance on structured reads 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 , Vladimir Sementsov-Ogievskiy , "open list:Network Block Dev..." , Max Reitz Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Just as we recently added a trace for a server sending block status that doesn't match the server's advertised minimum block alignment, let's do the same for read chunks. But since qemu 3.1 is such a server (because it advertised 512-byte alignment, but when serving a file that ends in data but is not sector-aligned, NBD_CMD_READ would detect a mid-sector change between data and hole at EOF and the resulting read chunks are unaligned), we don't want to change our behavior of otherwise tolerating unaligned reads. Note that even though we fixed the server for 4.0 to advertise an actual block alignment (which gets rid of the unaligned reads at EOF for posix files), we can still trigger it via other means: $ qemu-nbd --image-opts driver=blkdebug,align=512,image.driver=file,image.filename=/path/to/non-aligned-file Arguably, that is a bug in the blkdebug block status function, for leaking a block status that is not aligned. It may also be possible to observe issues with a backing layer with smaller alignment than the active layer, although so far I have been unable to write a reliable iotest for that scenario. Signed-off-by: Eric Blake Message-Id: <20190330165349.32256-1-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy --- block/nbd-client.c | 12 ++++++++++-- block/trace-events | 1 + 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/block/nbd-client.c b/block/nbd-client.c index 409c2171bc3..790ecc1ee1c 100644 --- a/block/nbd-client.c +++ b/block/nbd-client.c @@ -211,7 +211,8 @@ static inline uint64_t payload_advance64(uint8_t **payload) return ldq_be_p(*payload - 8); } -static int nbd_parse_offset_hole_payload(NBDStructuredReplyChunk *chunk, +static int nbd_parse_offset_hole_payload(NBDClientSession *client, + NBDStructuredReplyChunk *chunk, uint8_t *payload, uint64_t orig_offset, QEMUIOVector *qiov, Error **errp) { @@ -233,6 +234,10 @@ static int nbd_parse_offset_hole_payload(NBDStructuredReplyChunk *chunk, " region"); return -EINVAL; } + if (client->info.min_block && + !QEMU_IS_ALIGNED(hole_size, client->info.min_block)) { + trace_nbd_structured_read_compliance("hole"); + } qemu_iovec_memset(qiov, offset - orig_offset, 0, hole_size); @@ -390,6 +395,9 @@ static int nbd_co_receive_offset_data_payload(NBDClientSession *s, " region"); return -EINVAL; } + if (s->info.min_block && !QEMU_IS_ALIGNED(data_size, s->info.min_block)) { + trace_nbd_structured_read_compliance("data"); + } qemu_iovec_init(&sub_qiov, qiov->niov); qemu_iovec_concat(&sub_qiov, qiov, offset - orig_offset, data_size); @@ -712,7 +720,7 @@ static int nbd_co_receive_cmdread_reply(NBDClientSession *s, uint64_t handle, * in qiov */ break; case NBD_REPLY_TYPE_OFFSET_HOLE: - ret = nbd_parse_offset_hole_payload(&reply.structured, payload, + ret = nbd_parse_offset_hole_payload(s, &reply.structured, payload, offset, qiov, &local_err); if (ret < 0) { s->quit = true; diff --git a/block/trace-events b/block/trace-events index debb25c0ac8..7335a425404 100644 --- a/block/trace-events +++ b/block/trace-events @@ -158,6 +158,7 @@ iscsi_xcopy(void *src_lun, uint64_t src_off, void *dst_lun, uint64_t dst_off, ui # nbd-client.c nbd_parse_blockstatus_compliance(const char *err) "ignoring extra data from non-compliant server: %s" +nbd_structured_read_compliance(const char *type) "server sent non-compliant unaligned read %s chunk" nbd_read_reply_entry_fail(int ret, const char *err) "ret = %d, err: %s" nbd_co_request_fail(uint64_t from, uint32_t len, uint64_t handle, uint16_t flags, uint16_t type, const char *name, int ret, const char *err) "Request failed { .from = %" PRIu64", .len = %" PRIu32 ", .handle = %" PRIu64 ", .flags = 0x%" PRIx16 ", .type = %" PRIu16 " (%s) } ret = %d, err: %s"