Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/1072870/?format=api
{ "id": 1072870, "url": "http://patchwork.ozlabs.org/api/patches/1072870/?format=api", "web_url": "http://patchwork.ozlabs.org/project/qemu-devel/patch/20190401140903.19186-9-eblake@redhat.com/", "project": { "id": 14, "url": "http://patchwork.ozlabs.org/api/projects/14/?format=api", "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-9-eblake@redhat.com>", "list_archive_url": null, "date": "2019-04-01T14:08:57", "name": "[PULL,08/14] nbd/client: Lower min_block for block-status, unaligned size", "commit_ref": null, "pull_url": null, "state": "new", "archived": false, "hash": "456d92dd06ad76ae244165431018bdbe319e9035", "submitter": { "id": 6591, "url": "http://patchwork.ozlabs.org/api/people/6591/?format=api", "name": "Eric Blake", "email": "eblake@redhat.com" }, "delegate": null, "mbox": "http://patchwork.ozlabs.org/project/qemu-devel/patch/20190401140903.19186-9-eblake@redhat.com/mbox/", "series": [ { "id": 100345, "url": "http://patchwork.ozlabs.org/api/series/100345/?format=api", "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/1072870/comments/", "check": "pending", "checks": "http://patchwork.ozlabs.org/api/patches/1072870/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 44Xvqq2WhVz9sPj\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 2 Apr 2019 01:29:42 +1100 (AEDT)", "from localhost ([127.0.0.1]:38490 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 1hAxwS-0004y3-Tb\n\tfor incoming@patchwork.ozlabs.org; Mon, 01 Apr 2019 10:29:40 -0400", "from eggs.gnu.org ([209.51.188.92]:45939)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <eblake@redhat.com>) id 1hAxco-000257-BU\n\tfor qemu-devel@nongnu.org; Mon, 01 Apr 2019 10:09:23 -0400", "from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <eblake@redhat.com>) id 1hAxcl-0004rP-4q\n\tfor qemu-devel@nongnu.org; Mon, 01 Apr 2019 10:09:22 -0400", "from mx1.redhat.com ([209.132.183.28]:40326)\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 1hAxch-0004l7-QL; Mon, 01 Apr 2019 10:09:16 -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 25AD63097034;\n\tMon, 1 Apr 2019 14:09:15 +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 7430E19C70;\n\tMon, 1 Apr 2019 14:09:14 +0000 (UTC)" ], "From": "Eric Blake <eblake@redhat.com>", "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\n\t(mx1.redhat.com [10.5.110.43]);\n\tMon, 01 Apr 2019 14:09:15 +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 08/14] nbd/client: Lower min_block for\n\tblock-status, unaligned size", "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\"Richard W . M . Jones\" <rjones@redhat.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": "We have a latent bug in our NBD client code, tickled by the brand new\nnbdkit 1.11.10 block status support:\n\n$ nbdkit --filter=log --filter=truncate -U - \\\n data data=\"1\" size=511 truncate=64K logfile=/dev/stdout \\\n --run 'qemu-img convert $nbd /var/tmp/out'\n...\nqemu-img: block/io.c:2122: bdrv_co_block_status: Assertion `*pnum && QEMU_IS_ALIGNED(*pnum, align) && align > offset - aligned_offset' failed.\n\nThe culprit? Our implementation of .bdrv_co_block_status can return\nunaligned block status for any server that operates with a lower\nactual alignment than what we tell the block layer in\nrequest_alignment, in violation of the block layer's constraints. To\ndate, we've been unable to trip the bug, because qemu as NBD server\nalways advertises block sizing (at which point it is a server bug if\nthe server sends unaligned status - although qemu 3.1 is such a server\nand I've sent separate patches for 4.0 both to get the server to obey\nthe spec, and to let the client to tolerate server oddities at EOF).\n\nBut nbdkit does not (yet) advertise block sizing, and therefore is not\nin violation of the spec for returning block status at whatever\nboundaries it wants, and those unaligned results can occur anywhere\nrather than just at EOF. While we are still wise to avoid sending\nsub-sector read/write requests to a server of unknown origin, we MUST\nconsider that a server telling us block status without an advertised\nblock size is correct. So, we either have to munge unaligned answers\nfrom the server into aligned ones that we hand back to the block\nlayer, or we have to tell the block layer about a smaller alignment.\n\nSimilarly, if the server advertises an image size that is not\nsector-aligned, we might as well assume that the server intends to let\nus access those tail bytes, and therefore supports a minimum block\nsize of 1, regardless of whether the server supports block status\n(although we still need more patches to fix the problem that with an\nunaligned image, we can send read or block status requests that exceed\nEOF to the server). Again, qemu as server cannot trip this problem\n(because it rounds images to sector alignment), but nbdkit advertised\nunaligned size even before it gained block status support.\n\nSolve both alignment problems at once by using better heuristics on\nwhat alignment to report to the block layer when the server did not\ngive us something to work with. Note that very few NBD servers\nimplement block status (to date, only qemu and nbdkit are known to do\nso); and as the NBD spec mentioned block sizing constraints prior to\ndocumenting block status, it can be assumed that any future\nimplementations of block status are aware that they must advertise\nblock size if they want a minimum size other than 1.\n\nWe've had a long history of struggles with picking the right alignment\nto use in the block layer, as evidenced by the commit message of\nfd8d372d (v2.12) that introduced the current choice of forced 512-byte\nalignment.\n\nThere is no iotest coverage for this fix, because qemu can't provoke\nit, and I didn't want to make test 241 dependent on nbdkit.\n\nFixes: fd8d372d\nReported-by: Richard W.M. Jones <rjones@redhat.com>\nSigned-off-by: Eric Blake <eblake@redhat.com>\nMessage-Id: <20190329042750.14704-3-eblake@redhat.com>\nReviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>\nTested-by: Richard W.M. Jones <rjones@redhat.com>\n---\n block/nbd.c | 19 ++++++++++++++++++-\n 1 file changed, 18 insertions(+), 1 deletion(-)", "diff": "diff --git a/block/nbd.c b/block/nbd.c\nindex 2e72df528ac..208be596027 100644\n--- a/block/nbd.c\n+++ b/block/nbd.c\n@@ -437,7 +437,24 @@ static void nbd_refresh_limits(BlockDriverState *bs, Error **errp)\n uint32_t min = s->info.min_block;\n uint32_t max = MIN_NON_ZERO(NBD_MAX_BUFFER_SIZE, s->info.max_block);\n\n- bs->bl.request_alignment = min ? min : BDRV_SECTOR_SIZE;\n+ /*\n+ * If the server did not advertise an alignment:\n+ * - a size that is not sector-aligned implies that an alignment\n+ * of 1 can be used to access those tail bytes\n+ * - advertisement of block status requires an alignment of 1, so\n+ * that we don't violate block layer constraints that block\n+ * status is always aligned (as we can't control whether the\n+ * server will report sub-sector extents, such as a hole at EOF\n+ * on an unaligned POSIX file)\n+ * - otherwise, assume the server is so old that we are safer avoiding\n+ * sub-sector requests\n+ */\n+ if (!min) {\n+ min = (!QEMU_IS_ALIGNED(s->info.size, BDRV_SECTOR_SIZE) ||\n+ s->info.base_allocation) ? 1 : BDRV_SECTOR_SIZE;\n+ }\n+\n+ bs->bl.request_alignment = min;\n bs->bl.max_pdiscard = max;\n bs->bl.max_pwrite_zeroes = max;\n bs->bl.max_transfer = max;\n", "prefixes": [ "PULL", "08/14" ] }