Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/1072888/?format=api
{ "id": 1072888, "url": "http://patchwork.ozlabs.org/api/patches/1072888/?format=api", "web_url": "http://patchwork.ozlabs.org/project/qemu-devel/patch/20190401140903.19186-14-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-14-eblake@redhat.com>", "list_archive_url": null, "date": "2019-04-01T14:09:02", "name": "[PULL,13/14] nbd/server: Advertise actual minimum block size", "commit_ref": null, "pull_url": null, "state": "new", "archived": false, "hash": "daac890736663bc08c9c033d3d5e52c29a6a72f4", "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-14-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/1072888/comments/", "check": "pending", "checks": "http://patchwork.ozlabs.org/api/patches/1072888/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 44Xw2M1WCYz9sPj\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 2 Apr 2019 01:38:50 +1100 (AEDT)", "from localhost ([127.0.0.1]:40780 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 1hAy5I-0003a2-G5\n\tfor incoming@patchwork.ozlabs.org; Mon, 01 Apr 2019 10:38:48 -0400", "from eggs.gnu.org ([209.51.188.92]:46034)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <eblake@redhat.com>) id 1hAxcv-0002FF-Rp\n\tfor qemu-devel@nongnu.org; Mon, 01 Apr 2019 10:09:31 -0400", "from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <eblake@redhat.com>) id 1hAxcs-00050k-5e\n\tfor qemu-devel@nongnu.org; Mon, 01 Apr 2019 10:09:27 -0400", "from mx1.redhat.com ([209.132.183.28]:49090)\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 1hAxco-0004s0-BL; Mon, 01 Apr 2019 10:09:22 -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 08CCC91FEC;\n\tMon, 1 Apr 2019 14:09:20 +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 6959927CB3;\n\tMon, 1 Apr 2019 14:09:19 +0000 (UTC)" ], "From": "Eric Blake <eblake@redhat.com>", "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\n\t(mx1.redhat.com [10.5.110.29]);\n\tMon, 01 Apr 2019 14:09:20 +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 13/14] nbd/server: Advertise actual minimum\n\tblock 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\"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": "Both NBD_CMD_BLOCK_STATUS and structured NBD_CMD_READ will split their\nreply according to bdrv_block_status() boundaries. If the block device\nhas a request_alignment smaller than 512, but we advertise a block\nalignment of 512 to the client, then this can result in the server\nreply violating client expectations by reporting a smaller region of\nthe export than what the client is permitted to address (although this\nis less of an issue for qemu 4.0 clients, given recent client patches\nto overlook our non-compliance at EOF). Since it's always better to\nbe strict in what we send, it is worth advertising the actual minimum\nblock limit rather than blindly rounding it up to 512.\n\nNote that this patch is not foolproof - it is still possible to\nprovoke non-compliant server behavior using:\n\n$ qemu-nbd --image-opts driver=blkdebug,align=512,image.driver=file,image.filename=/path/to/non-aligned-file\n\nThat is arguably a bug in the blkdebug driver (it should never pass\nback block status smaller than its alignment, even if it has to make\nmultiple bdrv_get_status calls and determine the\nleast-common-denominator status among the group to return). It may\nalso be possible to observe issues with a backing layer with smaller\nalignment than the active layer, although so far I have been unable to\nwrite a reliable iotest for that scenario (but again, an issue like\nthat could be argued to be a bug in the block layer, or something\nwhere we need a flag to bdrv_block_status() to state whether the\nresult must be aligned to the current layer's limits or can be\nsubdivided for accuracy when chasing backing files).\n\nAnyways, as blkdebug is not normally used, and as this patch makes our\nserver more interoperable with qemu 3.1 clients, it is worth applying\nnow, even while we still work on a larger patch series for the 4.1\ntimeframe to have byte-accurate file lengths.\n\nNote that the iotests output changes - for 223 and 233, we can see the\nserver's better granularity advertisement; and for 241, the three test\ncases have the following effects:\n- natural alignment: the server's smaller alignment is now advertised,\nand the hole reported at EOF is now the right result; we've gotten rid\nof the server's non-compliance\n- forced server alignment: the server still advertises 512 bytes, but\nstill sends a mid-sector hole. This is still a server compliance bug,\nwhich needs to be fixed in the block layer in a later patch; output\ndoes not change because the client is already being tolerant of the\nnon-compliance\n- forced client alignment: the server's smaller alignment means that\nthe client now sees the server's status change mid-sector without any\nprotocol violations, but the fact that the map shows an unaligned\nmid-sector hole is evidence of the block layer problems with aligned\nblock status, to be fixed in a later patch\n\nSigned-off-by: Eric Blake <eblake@redhat.com>\nMessage-Id: <20190329042750.14704-7-eblake@redhat.com>\nReviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>\n[eblake: rebase to enhanced iotest 241 coverage]\n---\n nbd/server.c | 13 ++++++++-----\n tests/qemu-iotests/223.out | 4 ++--\n tests/qemu-iotests/233.out | 2 +-\n tests/qemu-iotests/241.out | 10 ++++++----\n 4 files changed, 17 insertions(+), 12 deletions(-)", "diff": "diff --git a/nbd/server.c b/nbd/server.c\nindex fd013a2817a..218a2aa5e65 100644\n--- a/nbd/server.c\n+++ b/nbd/server.c\n@@ -607,13 +607,16 @@ static int nbd_negotiate_handle_info(NBDClient *client, uint16_t myflags,\n /* Send NBD_INFO_BLOCK_SIZE always, but tweak the minimum size\n * according to whether the client requested it, and according to\n * whether this is OPT_INFO or OPT_GO. */\n- /* minimum - 1 for back-compat, or 512 if client is new enough.\n- * TODO: consult blk_bs(blk)->bl.request_alignment? */\n- sizes[0] =\n- (client->opt == NBD_OPT_INFO || blocksize) ? BDRV_SECTOR_SIZE : 1;\n+ /* minimum - 1 for back-compat, or actual if client will obey it. */\n+ if (client->opt == NBD_OPT_INFO || blocksize) {\n+ sizes[0] = blk_get_request_alignment(exp->blk);\n+ } else {\n+ sizes[0] = 1;\n+ }\n+ assert(sizes[0] <= NBD_MAX_BUFFER_SIZE);\n /* preferred - Hard-code to 4096 for now.\n * TODO: is blk_bs(blk)->bl.opt_transfer appropriate? */\n- sizes[1] = 4096;\n+ sizes[1] = MAX(4096, sizes[0]);\n /* maximum - At most 32M, but smaller as appropriate. */\n sizes[2] = MIN(blk_get_max_transfer(exp->blk), NBD_MAX_BUFFER_SIZE);\n trace_nbd_negotiate_handle_info_block_size(sizes[0], sizes[1], sizes[2]);\ndiff --git a/tests/qemu-iotests/223.out b/tests/qemu-iotests/223.out\nindex 7828a447392..d5201b2356a 100644\n--- a/tests/qemu-iotests/223.out\n+++ b/tests/qemu-iotests/223.out\n@@ -41,7 +41,7 @@ exports available: 2\n export: 'n'\n size: 4194304\n flags: 0x4ef ( readonly flush fua trim zeroes df cache )\n- min block: 512\n+ min block: 1\n opt block: 4096\n max block: 33554432\n available meta contexts: 2\n@@ -50,7 +50,7 @@ exports available: 2\n export: 'n2'\n size: 4194304\n flags: 0x4ed ( flush fua trim zeroes df cache )\n- min block: 512\n+ min block: 1\n opt block: 4096\n max block: 33554432\n available meta contexts: 2\ndiff --git a/tests/qemu-iotests/233.out b/tests/qemu-iotests/233.out\nindex 5acbc13b54a..9511b6ea658 100644\n--- a/tests/qemu-iotests/233.out\n+++ b/tests/qemu-iotests/233.out\n@@ -38,7 +38,7 @@ exports available: 1\n export: ''\n size: 67108864\n flags: 0x4ed ( flush fua trim zeroes df cache )\n- min block: 512\n+ min block: 1\n opt block: 4096\n max block: 33554432\n available meta contexts: 1\ndiff --git a/tests/qemu-iotests/241.out b/tests/qemu-iotests/241.out\nindex f22eabbf324..f481074a02e 100644\n--- a/tests/qemu-iotests/241.out\n+++ b/tests/qemu-iotests/241.out\n@@ -3,8 +3,9 @@ QA output created by 241\n === Exporting unaligned raw image, natural alignment ===\n\n size: 1024\n- min block: 512\n-[{ \"start\": 0, \"length\": 1024, \"depth\": 0, \"zero\": false, \"data\": true, \"offset\": OFFSET}]\n+ min block: 1\n+[{ \"start\": 0, \"length\": 1000, \"depth\": 0, \"zero\": false, \"data\": true, \"offset\": OFFSET},\n+{ \"start\": 1000, \"length\": 24, \"depth\": 0, \"zero\": true, \"data\": true, \"offset\": OFFSET}]\n 1 KiB (0x400) bytes allocated at offset 0 bytes (0x0)\n\n === Exporting unaligned raw image, forced server sector alignment ===\n@@ -20,7 +21,8 @@ WARNING: Image format was not specified for '/home/eblake/qemu/tests/qemu-iotest\n === Exporting unaligned raw image, forced client sector alignment ===\n\n size: 1024\n- min block: 512\n-[{ \"start\": 0, \"length\": 1024, \"depth\": 0, \"zero\": false, \"data\": true, \"offset\": OFFSET}]\n+ min block: 1\n+[{ \"start\": 0, \"length\": 1000, \"depth\": 0, \"zero\": false, \"data\": true, \"offset\": OFFSET},\n+{ \"start\": 1000, \"length\": 24, \"depth\": 0, \"zero\": true, \"data\": true, \"offset\": OFFSET}]\n 1 KiB (0x400) bytes allocated at offset 0 bytes (0x0)\n *** done\n", "prefixes": [ "PULL", "13/14" ] }