From nobody Fri Oct 24 09:58:54 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1519664867627248.38634695153598; Mon, 26 Feb 2018 09:07:47 -0800 (PST) Received: from localhost ([::1]:60271 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eqMFV-0003T9-Op for importer@patchew.org; Mon, 26 Feb 2018 12:07:37 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40318) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eqMBW-0000fJ-4u for qemu-devel@nongnu.org; Mon, 26 Feb 2018 12:03:31 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eqMBU-0005SF-LA for qemu-devel@nongnu.org; Mon, 26 Feb 2018 12:03:30 -0500 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:37060 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eqMBL-0005NT-QP; Mon, 26 Feb 2018 12:03:19 -0500 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 4E14040A0971; Mon, 26 Feb 2018 17:03:19 +0000 (UTC) Received: from localhost (unknown [10.40.205.24]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C212FAFD47; Mon, 26 Feb 2018 17:03:14 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Mon, 26 Feb 2018 18:03:13 +0100 Message-Id: <20180226170313.8178-1-mreitz@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.11.54.5 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Mon, 26 Feb 2018 17:03:19 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Mon, 26 Feb 2018 17:03:19 +0000 (UTC) for IP:'10.11.54.5' DOMAIN:'int-mx05.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'mreitz@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [RFC] qemu-img: Drop BLK_ZERO from convert X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , qemu-devel@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" There are filesystems (among which is tmpfs) that have a hard time reporting allocation status. That is definitely a bug in them. However, there is no good reason why qemu-img convert should query the allocation status in the first place. It does zero detection by itself anyway, so we can detect unallocated areas ourselves. Furthermore, if a filesystem driver has any sense, reading unallocated data should take just as much time as lseek(SEEK_DATA) + memset(). So the only overhead we introduce by dropping the manual lseek() call is a memset() in the driver and a buffer_is_zero() in qemu-img, both of which should be relatively quick. On the other hand, if we query the allocation status repeatedly for a file that is nearly fully allocated, we do many lseek(SEEK_DATA/HOLE) calls for nothing. While we can easily blame bugs in filesystem drivers, it still is a fact that this can cause considerable overhead. Note that we still have to invoke bdrv_is_allocated() when copying only the top overlay image. But this is quick and completely under our control because it only involves the format layer and does not go down to the protocol level; so this will never leave qemu. Buglink: https://bugs.launchpad.net/qemu/+bug/1751264 Signed-off-by: Max Reitz --- If we decide against this patch, we can still do the same if -S 0 has been specified. Then, if the user uses a buggy filesystem, we can tell them to use -S 0 so that converting at least works (even if we don't do any zero detection then). (And we definitely should do this for -S 0. Our current implementation then is to detect zero areas, create a bounce buffer, zero it, and write that to the destination. But that's exactly what the source filesystem driver would do if we simply read the data from it, so we're just introducing overhead because of another lseek(SEEK_DATA).) --- qemu-img.c | 41 +++++++++---------------------- tests/qemu-iotests/086.out | 60 ++++++++++++++++++++++++++++++++++++++++++= ++++ 2 files changed, 71 insertions(+), 30 deletions(-) diff --git a/qemu-img.c b/qemu-img.c index aa99fd32e9..d9e39c8901 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1522,7 +1522,6 @@ out4: =20 enum ImgConvertBlockStatus { BLK_DATA, - BLK_ZERO, BLK_BACKING_FILE, }; =20 @@ -1581,30 +1580,20 @@ static int convert_iteration_sectors(ImgConvertStat= e *s, int64_t sector_num) int64_t count =3D n * BDRV_SECTOR_SIZE; =20 if (s->target_has_backing) { - - ret =3D bdrv_block_status(blk_bs(s->src[src_cur]), + ret =3D bdrv_is_allocated(blk_bs(s->src[src_cur]), (sector_num - src_cur_offset) * BDRV_SECTOR_SIZE, - count, &count, NULL, NULL); - } else { - ret =3D bdrv_block_status_above(blk_bs(s->src[src_cur]), NULL, - (sector_num - src_cur_offset) * - BDRV_SECTOR_SIZE, - count, &count, NULL, NULL); - } - if (ret < 0) { - return ret; - } - n =3D DIV_ROUND_UP(count, BDRV_SECTOR_SIZE); + count, &count); + if (ret < 0) { + return ret; + } =20 - if (ret & BDRV_BLOCK_ZERO) { - s->status =3D BLK_ZERO; - } else if (ret & BDRV_BLOCK_DATA) { - s->status =3D BLK_DATA; + s->status =3D ret ? BLK_DATA : BLK_BACKING_FILE; } else { - s->status =3D s->target_has_backing ? BLK_BACKING_FILE : BLK_D= ATA; + s->status =3D BLK_DATA; } =20 + n =3D DIV_ROUND_UP(count, BDRV_SECTOR_SIZE); s->sector_next_status =3D sector_num + n; } =20 @@ -1713,9 +1702,8 @@ static int coroutine_fn convert_co_write(ImgConvertSt= ate *s, int64_t sector_num, } break; } - /* fall-through */ =20 - case BLK_ZERO: + /* Zero the target */ if (s->has_zero_init) { assert(!s->target_has_backing); break; @@ -1774,15 +1762,12 @@ static void coroutine_fn convert_co_do_copy(void *o= paque) /* save current sector and allocation status to local variables */ sector_num =3D s->sector_num; status =3D s->status; - if (!s->min_sparse && s->status =3D=3D BLK_ZERO) { - n =3D MIN(n, s->buf_sectors); - } /* increment global sector counter so that other coroutines can * already continue reading beyond this request */ s->sector_num +=3D n; qemu_co_mutex_unlock(&s->lock); =20 - if (status =3D=3D BLK_DATA || (!s->min_sparse && status =3D=3D BLK= _ZERO)) { + if (status =3D=3D BLK_DATA) { s->allocated_done +=3D n; qemu_progress_print(100.0 * s->allocated_done / s->allocated_sectors, 0); @@ -1795,9 +1780,6 @@ static void coroutine_fn convert_co_do_copy(void *opa= que) ": %s", sector_num, strerror(-ret)); s->ret =3D ret; } - } else if (!s->min_sparse && status =3D=3D BLK_ZERO) { - status =3D BLK_DATA; - memset(buf, 0x00, n * BDRV_SECTOR_SIZE); } =20 if (s->wr_in_order) { @@ -1879,8 +1861,7 @@ static int convert_do_copy(ImgConvertState *s) if (n < 0) { return n; } - if (s->status =3D=3D BLK_DATA || (!s->min_sparse && s->status =3D= =3D BLK_ZERO)) - { + if (s->status =3D=3D BLK_DATA) { s->allocated_sectors +=3D n; } sector_num +=3D n; diff --git a/tests/qemu-iotests/086.out b/tests/qemu-iotests/086.out index 5ff996101b..057a21abdb 100644 --- a/tests/qemu-iotests/086.out +++ b/tests/qemu-iotests/086.out @@ -9,9 +9,69 @@ wrote 1048576/1048576 bytes at offset 4194304 wrote 1048576/1048576 bytes at offset 33554432 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) (0.00/100%) + (1.56/100%) + (3.12/100%) + (4.69/100%) + (6.25/100%) + (7.81/100%) + (9.38/100%) + (10.94/100%) + (12.50/100%) + (14.06/100%) + (15.62/100%) + (17.19/100%) + (18.75/100%) + (20.31/100%) + (21.88/100%) + (23.44/100%) (25.00/100%) + (26.56/100%) + (28.12/100%) + (29.69/100%) + (31.25/100%) + (32.81/100%) + (34.38/100%) + (35.94/100%) + (37.50/100%) + (39.06/100%) + (40.62/100%) + (42.19/100%) + (43.75/100%) + (45.31/100%) + (46.88/100%) + (48.44/100%) (50.00/100%) + (51.56/100%) + (53.12/100%) + (54.69/100%) + (56.25/100%) + (57.81/100%) + (59.38/100%) + (60.94/100%) + (62.50/100%) + (64.06/100%) + (65.62/100%) + (67.19/100%) + (68.75/100%) + (70.31/100%) + (71.88/100%) + (73.44/100%) (75.00/100%) + (76.56/100%) + (78.12/100%) + (79.69/100%) + (81.25/100%) + (82.81/100%) + (84.38/100%) + (85.94/100%) + (87.50/100%) + (89.06/100%) + (90.62/100%) + (92.19/100%) + (93.75/100%) + (95.31/100%) + (96.88/100%) + (98.44/100%) (100.00/100%) (100.00/100%) =20 --=20 2.14.3