[{"id":1776532,"web_url":"http://patchwork.ozlabs.org/comment/1776532/","msgid":"<28a79c88-ca25-70c3-9e5b-fc794854fb34@redhat.com>","list_archive_url":null,"date":"2017-09-27T18:41:04","subject":"Re: [Qemu-devel] [PATCH v4 12/23] block: Convert\n\tbdrv_get_block_status_above() to bytes","submitter":{"id":64343,"url":"http://patchwork.ozlabs.org/api/people/64343/","name":"John Snow","email":"jsnow@redhat.com"},"content":"On 09/13/2017 12:03 PM, Eric Blake wrote:\n> We are gradually moving away from sector-based interfaces, towards\n> byte-based. In the common case, allocation is unlikely to ever use\n> values that are not naturally sector-aligned, but it is possible\n> that byte-based values will let us be more precise about allocation\n> at the end of an unaligned file that can do byte-based access.\n> \n> Changing the name of the function from bdrv_get_block_status_above()\n> to bdrv_block_status_above() ensures that the compiler enforces that\n> all callers are updated. For now, the io.c layer still assert()s\n> that all callers are sector-aligned, but that can be relaxed when a\n> later patch implements byte-based block status in the drivers.\n> \n> For the most part this patch is just the addition of scaling at the\n> callers followed by inverse scaling at bdrv_block_status(). But some\n> code, particularly bdrv_block_status(), gets a lot simpler because\n> it no longer has to mess with sectors. Likewise, mirror code no\n> longer computes s->granularity >> BDRV_SECTOR_BITS, and can therefore\n> drop an assertion (fix a neighboring assertion to use is_power_of_2\n> while there).\n> \n\nHuh, I suppose so, yeah. Do you have a test that covers what happens in\nthis newly available use case?\n\n> For ease of review, bdrv_get_block_status() was tackled separately.\n> \n> Signed-off-by: Eric Blake \n> \n\nLooks mechanically correct, anyway.\n\nReviewed-by: John Snow ","headers":{"Return-Path":"","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=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx05.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx05.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=jsnow@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3y2RW33lj2z9t66\n\tfor ;\n\tThu, 28 Sep 2017 04:41:50 +1000 (AEST)","from localhost ([::1]:55928 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dxHHH-0004BD-Pn\n\tfor incoming@patchwork.ozlabs.org; Wed, 27 Sep 2017 14:41:47 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:55299)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dxHGt-000491-Gb\n\tfor qemu-devel@nongnu.org; Wed, 27 Sep 2017 14:41:24 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dxHGs-0007GF-Or\n\tfor qemu-devel@nongnu.org; Wed, 27 Sep 2017 14:41:23 -0400","from mx1.redhat.com ([209.132.183.28]:1300)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1dxHGm-0007Dw-Kx; Wed, 27 Sep 2017 14:41:16 -0400","from smtp.corp.redhat.com\n\t(int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12])\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 875233DA414;\n\tWed, 27 Sep 2017 18:41:14 +0000 (UTC)","from [10.10.124.80] (ovpn-124-80.rdu2.redhat.com [10.10.124.80])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id D0A0560F91;\n\tWed, 27 Sep 2017 18:41:05 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 875233DA414","To":"Eric Blake , qemu-devel@nongnu.org","References":"<20170913160333.23622-1-eblake@redhat.com>\n\t<20170913160333.23622-13-eblake@redhat.com>","From":"John Snow ","Message-ID":"<28a79c88-ca25-70c3-9e5b-fc794854fb34@redhat.com>","Date":"Wed, 27 Sep 2017 14:41:04 -0400","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20170913160333.23622-13-eblake@redhat.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.12","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.29]);\n\tWed, 27 Sep 2017 18:41:14 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH v4 12/23] block: Convert\n\tbdrv_get_block_status_above() to bytes","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"kwolf@redhat.com, famz@redhat.com, qemu-block@nongnu.org,\n\tJeff Cody , Max Reitz ,\n\tStefan Hajnoczi ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1776542,"web_url":"http://patchwork.ozlabs.org/comment/1776542/","msgid":"<61cc4e97-bfb7-6f58-dbdc-09a7cd8516a7@redhat.com>","list_archive_url":null,"date":"2017-09-27T18:57:45","subject":"Re: [Qemu-devel] [PATCH v4 12/23] block: Convert\n\tbdrv_get_block_status_above() to bytes","submitter":{"id":6591,"url":"http://patchwork.ozlabs.org/api/people/6591/","name":"Eric Blake","email":"eblake@redhat.com"},"content":"On 09/27/2017 01:41 PM, John Snow wrote:\n> \n> \n> On 09/13/2017 12:03 PM, Eric Blake wrote:\n>> We are gradually moving away from sector-based interfaces, towards\n>> byte-based. In the common case, allocation is unlikely to ever use\n>> values that are not naturally sector-aligned, but it is possible\n>> that byte-based values will let us be more precise about allocation\n>> at the end of an unaligned file that can do byte-based access.\n>>\n>> Changing the name of the function from bdrv_get_block_status_above()\n>> to bdrv_block_status_above() ensures that the compiler enforces that\n>> all callers are updated. For now, the io.c layer still assert()s\n>> that all callers are sector-aligned, but that can be relaxed when a\n>> later patch implements byte-based block status in the drivers.\n>>\n>> For the most part this patch is just the addition of scaling at the\n>> callers followed by inverse scaling at bdrv_block_status(). But some\n>> code, particularly bdrv_block_status(), gets a lot simpler because\n>> it no longer has to mess with sectors. Likewise, mirror code no\n>> longer computes s->granularity >> BDRV_SECTOR_BITS, and can therefore\n>> drop an assertion (fix a neighboring assertion to use is_power_of_2\n>> while there).\n>>\n> \n> Huh, I suppose so, yeah. Do you have a test that covers what happens in\n> this newly available use case?\n\nNot directly - the mirror code no longer requires sector alignment, but\nis still unlikely to use sub-sector requests unless a particular driver\nreturns really small status information. I suppose we could tweak the\nblkdebug driver to force status requests to be fragmented at\nridiculously small alignments, and then prove that mirroring still\noccurs correctly, once all the series are in, but it's probably more\neffort than it is worth to force sub-sector mirroring if we don't have a\nreal use case that will rely on it.\n\n> \n>> For ease of review, bdrv_get_block_status() was tackled separately.\n>>\n>> Signed-off-by: Eric Blake \n>>\n> \n> Looks mechanically correct, anyway.\n> \n> Reviewed-by: John Snow \n>","headers":{"Return-Path":"","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=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx10.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx10.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=eblake@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3y2RtK50skz9tX8\n\tfor ;\n\tThu, 28 Sep 2017 04:58:29 +1000 (AEST)","from localhost ([::1]:55968 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dxHXM-0000ih-Sa\n\tfor incoming@patchwork.ozlabs.org; Wed, 27 Sep 2017 14:58:24 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:59553)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dxHX1-0000h0-La\n\tfor qemu-devel@nongnu.org; Wed, 27 Sep 2017 14:58:04 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dxHX0-0006HQ-Lf\n\tfor qemu-devel@nongnu.org; Wed, 27 Sep 2017 14:58:03 -0400","from mx1.redhat.com ([209.132.183.28]:53968)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1dxHWv-0006DZ-2z; Wed, 27 Sep 2017 14:57:57 -0400","from smtp.corp.redhat.com\n\t(int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11])\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 F09CC9FD3;\n\tWed, 27 Sep 2017 18:57:54 +0000 (UTC)","from [10.10.124.97] (ovpn-124-97.rdu2.redhat.com [10.10.124.97])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id BB14718AB5;\n\tWed, 27 Sep 2017 18:57:46 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com F09CC9FD3","To":"John Snow , qemu-devel@nongnu.org","References":"<20170913160333.23622-1-eblake@redhat.com>\n\t<20170913160333.23622-13-eblake@redhat.com>\n\t<28a79c88-ca25-70c3-9e5b-fc794854fb34@redhat.com>","From":"Eric Blake ","Openpgp":"url=http://people.redhat.com/eblake/eblake.gpg","Organization":"Red Hat, Inc.","Message-ID":"<61cc4e97-bfb7-6f58-dbdc-09a7cd8516a7@redhat.com>","Date":"Wed, 27 Sep 2017 13:57:45 -0500","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<28a79c88-ca25-70c3-9e5b-fc794854fb34@redhat.com>","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"ePk6CwGDq6vBm9PfLmRfOKOqCTWsAeRQi\"","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.11","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.39]);\n\tWed, 27 Sep 2017 18:57:55 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","X-Content-Filtered-By":"Mailman/MimeDel 2.1.21","Subject":"Re: [Qemu-devel] [PATCH v4 12/23] block: Convert\n\tbdrv_get_block_status_above() to bytes","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"kwolf@redhat.com, famz@redhat.com, qemu-block@nongnu.org,\n\tJeff Cody , Max Reitz ,\n\tStefan Hajnoczi ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1776571,"web_url":"http://patchwork.ozlabs.org/comment/1776571/","msgid":"<330c8bd5-4d3b-2d5f-a0a0-f0fcd50f8b84@redhat.com>","list_archive_url":null,"date":"2017-09-27T19:40:16","subject":"Re: [Qemu-devel] [PATCH v4 12/23] block: Convert\n\tbdrv_get_block_status_above() to bytes","submitter":{"id":64343,"url":"http://patchwork.ozlabs.org/api/people/64343/","name":"John Snow","email":"jsnow@redhat.com"},"content":"On 09/27/2017 02:57 PM, Eric Blake wrote:\n> On 09/27/2017 01:41 PM, John Snow wrote:\n>>\n>>\n>> On 09/13/2017 12:03 PM, Eric Blake wrote:\n>>> We are gradually moving away from sector-based interfaces, towards\n>>> byte-based. In the common case, allocation is unlikely to ever use\n>>> values that are not naturally sector-aligned, but it is possible\n>>> that byte-based values will let us be more precise about allocation\n>>> at the end of an unaligned file that can do byte-based access.\n>>>\n>>> Changing the name of the function from bdrv_get_block_status_above()\n>>> to bdrv_block_status_above() ensures that the compiler enforces that\n>>> all callers are updated. For now, the io.c layer still assert()s\n>>> that all callers are sector-aligned, but that can be relaxed when a\n>>> later patch implements byte-based block status in the drivers.\n>>>\n>>> For the most part this patch is just the addition of scaling at the\n>>> callers followed by inverse scaling at bdrv_block_status(). But some\n>>> code, particularly bdrv_block_status(), gets a lot simpler because\n>>> it no longer has to mess with sectors. Likewise, mirror code no\n>>> longer computes s->granularity >> BDRV_SECTOR_BITS, and can therefore\n>>> drop an assertion (fix a neighboring assertion to use is_power_of_2\n>>> while there).\n>>>\n>>\n>> Huh, I suppose so, yeah. Do you have a test that covers what happens in\n>> this newly available use case?\n> \n> Not directly - the mirror code no longer requires sector alignment, but\n> is still unlikely to use sub-sector requests unless a particular driver\n> returns really small status information. I suppose we could tweak the\n> blkdebug driver to force status requests to be fragmented at\n> ridiculously small alignments, and then prove that mirroring still\n> occurs correctly, once all the series are in, but it's probably more\n> effort than it is worth to force sub-sector mirroring if we don't have a\n> real use case that will rely on it.\n> \n\nHmm, yeah, the code probably can't be exercised currently but I do\nwonder if we're removing too many breadcrumbs for potential problem\nspots if someone decides to return sub-sector information in the future.\n\nWell, I suppose I haven't been too diligent about complaining about\ntheir removal elsewhere, so for consistency:\n\nEither with or without the assertion removed as you see fit:\n\nReviewed-by: John Snow \n\n>>\n>>> For ease of review, bdrv_get_block_status() was tackled separately.\n>>>\n>>> Signed-off-by: Eric Blake \n>>>\n>>\n>> Looks mechanically correct, anyway.\n>>\n>> Reviewed-by: John Snow \n>>\n>","headers":{"Return-Path":"","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=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx08.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx08.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=jsnow@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3y2SqH1j09z9t49\n\tfor ;\n\tThu, 28 Sep 2017 05:40:58 +1000 (AEST)","from localhost ([::1]:56110 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dxICT-00036u-SF\n\tfor incoming@patchwork.ozlabs.org; Wed, 27 Sep 2017 15:40:53 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:42407)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dxIC7-00035I-VZ\n\tfor qemu-devel@nongnu.org; Wed, 27 Sep 2017 15:40:33 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dxIC7-0003aM-21\n\tfor qemu-devel@nongnu.org; Wed, 27 Sep 2017 15:40:31 -0400","from mx1.redhat.com ([209.132.183.28]:18881)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1dxIC1-0003YQ-Ax; Wed, 27 Sep 2017 15:40:25 -0400","from smtp.corp.redhat.com\n\t(int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12])\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 49CADC049631;\n\tWed, 27 Sep 2017 19:40:24 +0000 (UTC)","from [10.10.124.80] (ovpn-124-80.rdu2.redhat.com [10.10.124.80])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id E88E8672EA;\n\tWed, 27 Sep 2017 19:40:17 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 49CADC049631","To":"Eric Blake , qemu-devel@nongnu.org","References":"<20170913160333.23622-1-eblake@redhat.com>\n\t<20170913160333.23622-13-eblake@redhat.com>\n\t<28a79c88-ca25-70c3-9e5b-fc794854fb34@redhat.com>\n\t<61cc4e97-bfb7-6f58-dbdc-09a7cd8516a7@redhat.com>","From":"John Snow ","Message-ID":"<330c8bd5-4d3b-2d5f-a0a0-f0fcd50f8b84@redhat.com>","Date":"Wed, 27 Sep 2017 15:40:16 -0400","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<61cc4e97-bfb7-6f58-dbdc-09a7cd8516a7@redhat.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.12","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.32]);\n\tWed, 27 Sep 2017 19:40:24 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH v4 12/23] block: Convert\n\tbdrv_get_block_status_above() to bytes","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"kwolf@redhat.com, famz@redhat.com, qemu-block@nongnu.org,\n\tJeff Cody , Max Reitz ,\n\tStefan Hajnoczi ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}}]