{"id":1072854,"url":"http://patchwork.ozlabs.org/api/patches/1072854/?format=json","web_url":"http://patchwork.ozlabs.org/project/qemu-devel/patch/20190401140903.19186-7-eblake@redhat.com/","project":{"id":14,"url":"http://patchwork.ozlabs.org/api/projects/14/?format=json","name":"QEMU Development","link_name":"qemu-devel","list_id":"qemu-devel.nongnu.org","list_email":"qemu-devel@nongnu.org","web_url":"","scm_url":"","webscm_url":"","list_archive_url":"","list_archive_url_format":"","commit_url_format":""},"msgid":"<20190401140903.19186-7-eblake@redhat.com>","list_archive_url":null,"date":"2019-04-01T14:08:55","name":"[PULL,06/14] nbd-client: Work around server BLOCK_STATUS misalignment at EOF","commit_ref":null,"pull_url":null,"state":"new","archived":false,"hash":"6b44ff63d30b76d6c5e0804c5667f91874983303","submitter":{"id":6591,"url":"http://patchwork.ozlabs.org/api/people/6591/?format=json","name":"Eric Blake","email":"eblake@redhat.com"},"delegate":null,"mbox":"http://patchwork.ozlabs.org/project/qemu-devel/patch/20190401140903.19186-7-eblake@redhat.com/mbox/","series":[{"id":100345,"url":"http://patchwork.ozlabs.org/api/series/100345/?format=json","web_url":"http://patchwork.ozlabs.org/project/qemu-devel/list/?series=100345","date":"2019-04-01T14:08:49","name":"[PULL,01/14] qemu-img: Report bdrv_block_status failures","version":1,"mbox":"http://patchwork.ozlabs.org/series/100345/mbox/"}],"comments":"http://patchwork.ozlabs.org/api/patches/1072854/comments/","check":"pending","checks":"http://patchwork.ozlabs.org/api/patches/1072854/checks/","tags":{},"related":[],"headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=209.51.188.17; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","ozlabs.org;\n\tdmarc=fail (p=none dis=none) header.from=redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [209.51.188.17])\n\t(using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 44Xvf3720yz9sPp\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue,  2 Apr 2019 01:21:15 +1100 (AEDT)","from localhost ([127.0.0.1]:36206 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1hAxoI-0006BA-0G\n\tfor incoming@patchwork.ozlabs.org; Mon, 01 Apr 2019 10:21:14 -0400","from eggs.gnu.org ([209.51.188.92]:45885)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <eblake@redhat.com>) id 1hAxck-00021Q-V4\n\tfor qemu-devel@nongnu.org; Mon, 01 Apr 2019 10:09:20 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <eblake@redhat.com>) id 1hAxcj-0004o3-0i\n\tfor qemu-devel@nongnu.org; Mon, 01 Apr 2019 10:09:18 -0400","from mx1.redhat.com ([209.132.183.28]:49020)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <eblake@redhat.com>)\n\tid 1hAxcg-0004kF-5l; Mon, 01 Apr 2019 10:09:14 -0400","from smtp.corp.redhat.com\n\t(int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 776B719D05E;\n\tMon,  1 Apr 2019 14:09:13 +0000 (UTC)","from blue.redhat.com (ovpn-116-75.phx2.redhat.com [10.3.116.75])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id DBEF819C70;\n\tMon,  1 Apr 2019 14:09:12 +0000 (UTC)"],"From":"Eric Blake <eblake@redhat.com>","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\n\t(mx1.redhat.com [10.5.110.29]);\n\tMon, 01 Apr 2019 14:09:13 +0000 (UTC)","Content-Transfer-Encoding":"quoted-printable","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\n\tBLOCK_STATUS misalignment at EOF","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"Kevin Wolf <kwolf@redhat.com>,\n\tVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,\n\t\"open list:Network Block Dev...\" <qemu-block@nongnu.org>,\n\tMax Reitz <mreitz@redhat.com>","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"},"content":"The NBD spec is clear that a server that advertises a minimum block\nsize should reply to NBD_CMD_BLOCK_STATUS with extents aligned\naccordingly. However, we know that the qemu NBD server implementation\nhas had a corner-case bug where it is not compliant with the spec,\npresent since the introduction of NBD_CMD_BLOCK_STATUS in qemu 2.12\n(and unlikely to be patched in time for 4.0). Namely, when qemu is\nserving a file that is not a multiple of 512 bytes, it rounds the size\nadvertised over NBD up to the next sector boundary (someday, I'd like\nto fix that to be byte-accurate, but it's a much bigger audit not\nappropriate for this release); yet if the final sector contains data\nprior to EOF, lseek(SEEK_HOLE) will point to the implicit hole\nmid-sector which qemu then reported over NBD.\n\nWe are well within our rights to hang up on a server that can't follow\nthe spec, but it is more useful to try and keep the connection alive\nin spite of the problem. Do so by tracing a message about the problem,\nand then either truncating the request back to an aligned boundary (if\nit covered more than the final sector) or widening it out to the full\nboundary with a forced status of data (since truncating would result\nin 0 bytes, but we have to make progress, and valid since data is a\ndefault-safe answer). And in practice, since the problem only happens\non a sector that starts with data and ends with a hole, we are going\nto want to read that full sector anyway (where qemu as the server\nfills in the tail beyond EOF with appropriate NUL bytes).\n\nEasy reproduction:\n$ printf %1000d 1 > file\n$ qemu-nbd -f raw -t file & pid=$!\n$ qemu-img map --output=json -f raw nbd://localhost:10809\nqemu-img: Could not read file metadata: Invalid argument\n$ kill $pid\n\nwhere the patched version instead succeeds with:\n[{ \"start\": 0, \"length\": 1024, \"depth\": 0, \"zero\": false, \"data\": true}]\n\nSigned-off-by: Eric Blake <eblake@redhat.com>\nMessage-Id: <20190326171317.4036-1-eblake@redhat.com>\nReviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>\n---\n block/nbd-client.c | 30 ++++++++++++++++++++++++++----\n 1 file changed, 26 insertions(+), 4 deletions(-)","diff":"diff --git a/block/nbd-client.c b/block/nbd-client.c\nindex a3b70d14004..150af9cc46f 100644\n--- a/block/nbd-client.c\n+++ b/block/nbd-client.c\n@@ -269,14 +269,36 @@ static int nbd_parse_blockstatus_payload(NBDClientSession *client,\n     extent->length = payload_advance32(&payload);\n     extent->flags = payload_advance32(&payload);\n\n-    if (extent->length == 0 ||\n-        (client->info.min_block && !QEMU_IS_ALIGNED(extent->length,\n-                                                    client->info.min_block))) {\n+    if (extent->length == 0) {\n         error_setg(errp, \"Protocol error: server sent status chunk with \"\n-                   \"invalid length\");\n+                   \"zero length\");\n         return -EINVAL;\n     }\n\n+    /*\n+     * A server sending unaligned block status is in violation of the\n+     * protocol, but as qemu-nbd 3.1 is such a server (at least for\n+     * POSIX files that are not a multiple of 512 bytes, since qemu\n+     * rounds files up to 512-byte multiples but lseek(SEEK_HOLE)\n+     * still sees an implicit hole beyond the real EOF), it's nicer to\n+     * work around the misbehaving server. If the request included\n+     * more than the final unaligned block, truncate it back to an\n+     * aligned result; if the request was only the final block, round\n+     * up to the full block and change the status to fully-allocated\n+     * (always a safe status, even if it loses information).\n+     */\n+    if (client->info.min_block && !QEMU_IS_ALIGNED(extent->length,\n+                                                   client->info.min_block)) {\n+        trace_nbd_parse_blockstatus_compliance(\"extent length is unaligned\");\n+        if (extent->length > client->info.min_block) {\n+            extent->length = QEMU_ALIGN_DOWN(extent->length,\n+                                             client->info.min_block);\n+        } else {\n+            extent->length = client->info.min_block;\n+            extent->flags = 0;\n+        }\n+    }\n+\n     /*\n      * We used NBD_CMD_FLAG_REQ_ONE, so the server should not have\n      * sent us any more than one extent, nor should it have included\n","prefixes":["PULL","06/14"]}