{"id":813519,"url":"http://patchwork.ozlabs.org/api/patches/813519/?format=json","web_url":"http://patchwork.ozlabs.org/project/qemu-devel/patch/20170913160333.23622-21-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":"<20170913160333.23622-21-eblake@redhat.com>","list_archive_url":null,"date":"2017-09-13T16:03:30","name":"[v4,20/23] qemu-img: Change img_compare() to be byte-based","commit_ref":null,"pull_url":null,"state":"new","archived":false,"hash":"e657566157d4b6fad669387cc42c20a00e5e3160","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/20170913160333.23622-21-eblake@redhat.com/mbox/","series":[{"id":2944,"url":"http://patchwork.ozlabs.org/api/series/2944/?format=json","web_url":"http://patchwork.ozlabs.org/project/qemu-devel/list/?series=2944","date":"2017-09-13T16:03:10","name":"make bdrv_get_block_status byte-based","version":4,"mbox":"http://patchwork.ozlabs.org/series/2944/mbox/"}],"comments":"http://patchwork.ozlabs.org/api/patches/813519/comments/","check":"pending","checks":"http://patchwork.ozlabs.org/api/patches/813519/checks/","tags":{},"related":[],"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=208.118.235.17; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx04.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx04.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=eblake@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [208.118.235.17])\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 3xsmxf4gJfz9s72\n\tfor ;\n\tThu, 14 Sep 2017 02:16:22 +1000 (AEST)","from localhost ([::1]:43415 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dsAKq-0006gy-H1\n\tfor incoming@patchwork.ozlabs.org; Wed, 13 Sep 2017 12:16:20 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:46411)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dsAAF-0002oz-6f\n\tfor qemu-devel@nongnu.org; Wed, 13 Sep 2017 12:05:27 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dsAAA-0006pv-G0\n\tfor qemu-devel@nongnu.org; Wed, 13 Sep 2017 12:05:23 -0400","from mx1.redhat.com ([209.132.183.28]:54222)\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 1dsA9x-0006ag-Eg; Wed, 13 Sep 2017 12:05:05 -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 7ED257EA87;\n\tWed, 13 Sep 2017 16:05:04 +0000 (UTC)","from red.redhat.com (ovpn-120-201.rdu2.redhat.com [10.10.120.201])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 5A04669FBC;\n\tWed, 13 Sep 2017 16:04:49 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 7ED257EA87","From":"Eric Blake ","To":"qemu-devel@nongnu.org","Date":"Wed, 13 Sep 2017 11:03:30 -0500","Message-Id":"<20170913160333.23622-21-eblake@redhat.com>","In-Reply-To":"<20170913160333.23622-1-eblake@redhat.com>","References":"<20170913160333.23622-1-eblake@redhat.com>","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.28]);\n\tWed, 13 Sep 2017 16:05:04 +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":"[Qemu-devel] [PATCH v4 20/23] qemu-img: Change img_compare() to be\n\tbyte-based","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, jsnow@redhat.com, famz@redhat.com,\n\tqemu-block@nongnu.org, Max Reitz ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"},"content":"In the continuing quest to make more things byte-based, change\nthe internal iteration of img_compare(). We can finally drop the\nTODO assertion added earlier, now that the entire algorithm is\nbyte-based and no longer has to shift from bytes to sectors.\n\nMost of the change is mechanical ('total_sectors' becomes\n'total_size', 'sector_num' becomes 'offset', 'nb_sectors' becomes\n'chunk', 'progress_base' goes from sectors to bytes); some of it\nis also a cleanup (sectors_to_bytes() is now unused, loss of\nvariable 'count' added earlier in commit 51b0a488).\n\nSigned-off-by: Eric Blake \n\n---\nv3: new patch\n---\n qemu-img.c | 119 ++++++++++++++++++++++++-------------------------------------\n 1 file changed, 46 insertions(+), 73 deletions(-)","diff":"diff --git a/qemu-img.c b/qemu-img.c\nindex 028c34a2cc..ef7062649d 100644\n--- a/qemu-img.c\n+++ b/qemu-img.c\n@@ -1185,11 +1185,6 @@ static int compare_buffers(const uint8_t *buf1, const uint8_t *buf2,\n\n #define IO_BUF_SIZE (2 * 1024 * 1024)\n\n-static int64_t sectors_to_bytes(int64_t sectors)\n-{\n- return sectors << BDRV_SECTOR_BITS;\n-}\n-\n /*\n * Check if passed sectors are empty (not allocated or contain only 0 bytes)\n *\n@@ -1240,7 +1235,7 @@ static int img_compare(int argc, char **argv)\n const char *fmt1 = NULL, *fmt2 = NULL, *cache, *filename1, *filename2;\n BlockBackend *blk1, *blk2;\n BlockDriverState *bs1, *bs2;\n- int64_t total_sectors1, total_sectors2;\n+ int64_t total_size1, total_size2;\n uint8_t *buf1 = NULL, *buf2 = NULL;\n int64_t pnum1, pnum2;\n int allocated1, allocated2;\n@@ -1248,9 +1243,9 @@ static int img_compare(int argc, char **argv)\n bool progress = false, quiet = false, strict = false;\n int flags;\n bool writethrough;\n- int64_t total_sectors;\n- int64_t sector_num = 0;\n- int64_t nb_sectors;\n+ int64_t total_size;\n+ int64_t offset = 0;\n+ int64_t chunk;\n int c;\n uint64_t progress_base;\n bool image_opts = false;\n@@ -1364,39 +1359,36 @@ static int img_compare(int argc, char **argv)\n\n buf1 = blk_blockalign(blk1, IO_BUF_SIZE);\n buf2 = blk_blockalign(blk2, IO_BUF_SIZE);\n- total_sectors1 = blk_nb_sectors(blk1);\n- if (total_sectors1 < 0) {\n+ total_size1 = blk_getlength(blk1);\n+ if (total_size1 < 0) {\n error_report(\"Can't get size of %s: %s\",\n- filename1, strerror(-total_sectors1));\n+ filename1, strerror(-total_size1));\n ret = 4;\n goto out;\n }\n- total_sectors2 = blk_nb_sectors(blk2);\n- if (total_sectors2 < 0) {\n+ total_size2 = blk_getlength(blk2);\n+ if (total_size2 < 0) {\n error_report(\"Can't get size of %s: %s\",\n- filename2, strerror(-total_sectors2));\n+ filename2, strerror(-total_size2));\n ret = 4;\n goto out;\n }\n- total_sectors = MIN(total_sectors1, total_sectors2);\n- progress_base = MAX(total_sectors1, total_sectors2);\n+ total_size = MIN(total_size1, total_size2);\n+ progress_base = MAX(total_size1, total_size2);\n\n qemu_progress_print(0, 100);\n\n- if (strict && total_sectors1 != total_sectors2) {\n+ if (strict && total_size1 != total_size2) {\n ret = 1;\n qprintf(quiet, \"Strict mode: Image size mismatch!\\n\");\n goto out;\n }\n\n- while (sector_num < total_sectors) {\n+ while (offset < total_size) {\n int64_t status1, status2;\n\n- status1 = bdrv_block_status_above(bs1, NULL,\n- sector_num * BDRV_SECTOR_SIZE,\n- (total_sectors1 - sector_num) *\n- BDRV_SECTOR_SIZE,\n- &pnum1, NULL);\n+ status1 = bdrv_block_status_above(bs1, NULL, offset,\n+ total_size1 - offset, &pnum1, NULL);\n if (status1 < 0) {\n ret = 3;\n error_report(\"Sector allocation test failed for %s\", filename1);\n@@ -1404,11 +1396,8 @@ static int img_compare(int argc, char **argv)\n }\n allocated1 = status1 & BDRV_BLOCK_ALLOCATED;\n\n- status2 = bdrv_block_status_above(bs2, NULL,\n- sector_num * BDRV_SECTOR_SIZE,\n- (total_sectors2 - sector_num) *\n- BDRV_SECTOR_SIZE,\n- &pnum2, NULL);\n+ status2 = bdrv_block_status_above(bs2, NULL, offset,\n+ total_size2 - offset, &pnum2, NULL);\n if (status2 < 0) {\n ret = 3;\n error_report(\"Sector allocation test failed for %s\", filename2);\n@@ -1417,15 +1406,14 @@ static int img_compare(int argc, char **argv)\n allocated2 = status2 & BDRV_BLOCK_ALLOCATED;\n\n assert(pnum1 && pnum2);\n- nb_sectors = DIV_ROUND_UP(MIN(pnum1, pnum2), BDRV_SECTOR_SIZE);\n+ chunk = MIN(pnum1, pnum2);\n\n if (strict) {\n if ((status1 & ~BDRV_BLOCK_OFFSET_MASK) !=\n (status2 & ~BDRV_BLOCK_OFFSET_MASK)) {\n ret = 1;\n qprintf(quiet, \"Strict mode: Offset %\" PRId64\n- \" block status mismatch!\\n\",\n- sectors_to_bytes(sector_num));\n+ \" block status mismatch!\\n\", offset);\n goto out;\n }\n }\n@@ -1435,59 +1423,54 @@ static int img_compare(int argc, char **argv)\n if (allocated1) {\n int64_t pnum;\n\n- nb_sectors = MIN(nb_sectors, IO_BUF_SIZE >> BDRV_SECTOR_BITS);\n- ret = blk_pread(blk1, sector_num << BDRV_SECTOR_BITS, buf1,\n- nb_sectors << BDRV_SECTOR_BITS);\n+ chunk = MIN(chunk, IO_BUF_SIZE);\n+ ret = blk_pread(blk1, offset, buf1, chunk);\n if (ret < 0) {\n- error_report(\"Error while reading offset %\" PRId64 \" of %s:\"\n- \" %s\", sectors_to_bytes(sector_num), filename1,\n- strerror(-ret));\n+ error_report(\"Error while reading offset %\" PRId64\n+ \" of %s: %s\",\n+ offset, filename1, strerror(-ret));\n ret = 4;\n goto out;\n }\n- ret = blk_pread(blk2, sector_num << BDRV_SECTOR_BITS, buf2,\n- nb_sectors << BDRV_SECTOR_BITS);\n+ ret = blk_pread(blk2, offset, buf2, chunk);\n if (ret < 0) {\n error_report(\"Error while reading offset %\" PRId64\n- \" of %s: %s\", sectors_to_bytes(sector_num),\n- filename2, strerror(-ret));\n+ \" of %s: %s\",\n+ offset, filename2, strerror(-ret));\n ret = 4;\n goto out;\n }\n- ret = compare_buffers(buf1, buf2,\n- nb_sectors * BDRV_SECTOR_SIZE, &pnum);\n- if (ret || pnum != nb_sectors * BDRV_SECTOR_SIZE) {\n+ ret = compare_buffers(buf1, buf2, chunk, &pnum);\n+ if (ret || pnum != chunk) {\n qprintf(quiet, \"Content mismatch at offset %\" PRId64 \"!\\n\",\n- sectors_to_bytes(sector_num) + (ret ? 0 : pnum));\n+ offset + (ret ? 0 : pnum));\n ret = 1;\n goto out;\n }\n }\n } else {\n- nb_sectors = MIN(nb_sectors, IO_BUF_SIZE >> BDRV_SECTOR_BITS);\n+ chunk = MIN(chunk, IO_BUF_SIZE);\n if (allocated1) {\n- ret = check_empty_sectors(blk1, sector_num * BDRV_SECTOR_SIZE,\n- nb_sectors * BDRV_SECTOR_SIZE,\n+ ret = check_empty_sectors(blk1, offset, chunk,\n filename1, buf1, quiet);\n } else {\n- ret = check_empty_sectors(blk2, sector_num * BDRV_SECTOR_SIZE,\n- nb_sectors * BDRV_SECTOR_SIZE,\n+ ret = check_empty_sectors(blk2, offset, chunk,\n filename2, buf1, quiet);\n }\n if (ret) {\n goto out;\n }\n }\n- sector_num += nb_sectors;\n- qemu_progress_print(((float) nb_sectors / progress_base)*100, 100);\n+ offset += chunk;\n+ qemu_progress_print(((float) chunk / progress_base) * 100, 100);\n }\n\n- if (total_sectors1 != total_sectors2) {\n+ if (total_size1 != total_size2) {\n BlockBackend *blk_over;\n const char *filename_over;\n\n qprintf(quiet, \"Warning: Image size mismatch!\\n\");\n- if (total_sectors1 > total_sectors2) {\n+ if (total_size1 > total_size2) {\n blk_over = blk1;\n filename_over = filename1;\n } else {\n@@ -1495,14 +1478,10 @@ static int img_compare(int argc, char **argv)\n filename_over = filename2;\n }\n\n- while (sector_num < progress_base) {\n- int64_t count;\n-\n- ret = bdrv_block_status_above(blk_bs(blk_over), NULL,\n- sector_num * BDRV_SECTOR_SIZE,\n- (progress_base - sector_num) *\n- BDRV_SECTOR_SIZE,\n- &count, NULL);\n+ while (offset < progress_base) {\n+ ret = bdrv_block_status_above(blk_bs(blk_over), NULL, offset,\n+ progress_base - offset, &chunk,\n+ NULL);\n if (ret < 0) {\n ret = 3;\n error_report(\"Sector allocation test failed for %s\",\n@@ -1510,22 +1489,16 @@ static int img_compare(int argc, char **argv)\n goto out;\n\n }\n- /* TODO relax this once bdrv_block_status_above does not enforce\n- * sector alignment */\n- assert(QEMU_IS_ALIGNED(count, BDRV_SECTOR_SIZE));\n- nb_sectors = count >> BDRV_SECTOR_BITS;\n if (ret & BDRV_BLOCK_ALLOCATED && !(ret & BDRV_BLOCK_ZERO)) {\n- nb_sectors = MIN(nb_sectors, IO_BUF_SIZE >> BDRV_SECTOR_BITS);\n- ret = check_empty_sectors(blk_over,\n- sector_num * BDRV_SECTOR_SIZE,\n- nb_sectors * BDRV_SECTOR_SIZE,\n+ chunk = MIN(chunk, IO_BUF_SIZE);\n+ ret = check_empty_sectors(blk_over, offset, chunk,\n filename_over, buf1, quiet);\n if (ret) {\n goto out;\n }\n }\n- sector_num += nb_sectors;\n- qemu_progress_print(((float) nb_sectors / progress_base)*100, 100);\n+ offset += chunk;\n+ qemu_progress_print(((float) chunk / progress_base) * 100, 100);\n }\n }\n\n","prefixes":["v4","20/23"]}