From nobody Tue May 7 05:07:07 2024 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.zoho.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; Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1494115687362986.7531097730074; Sat, 6 May 2017 17:08:07 -0700 (PDT) Received: from localhost ([::1]:53090 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d79k5-0004F5-Cy for importer@patchew.org; Sat, 06 May 2017 20:08:05 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45723) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d79iA-0002bQ-6j for qemu-devel@nongnu.org; Sat, 06 May 2017 20:06:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1d79i9-0007iC-0C for qemu-devel@nongnu.org; Sat, 06 May 2017 20:06:06 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38888) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1d79i4-0007gU-WB; Sat, 06 May 2017 20:06:01 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 04DB08553F; Sun, 7 May 2017 00:06:00 +0000 (UTC) Received: from red.redhat.com (ovpn-122-206.rdu2.redhat.com [10.10.122.206]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4B8B91850A; Sun, 7 May 2017 00:05:59 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 04DB08553F Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=eblake@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 04DB08553F From: Eric Blake To: qemu-devel@nongnu.org Date: Sat, 6 May 2017 19:05:41 -0500 Message-Id: <20170507000552.20847-2-eblake@redhat.com> In-Reply-To: <20170507000552.20847-1-eblake@redhat.com> References: <20170507000552.20847-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Sun, 07 May 2017 00:06:00 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v13 01/12] qcow2: Nicer variable names in qcow2_update_snapshot_refcount() 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: kwolf@redhat.com, qemu-block@nongnu.org, mreitz@redhat.com 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" In order to keep checkpatch happy when the next patch changes indentation, we first have to shorten some long lines. The easiest approach is to use a new variable in place of 'offset & L2E_OFFSET_MASK', except that 'offset' is the best name for that variable. Change '[old_]offset' to '[old_]entry' to make room. While touching things, also fix checkpatch warnings about unusual 'for' statements. Suggested by Max Reitz Signed-off-by: Eric Blake Reviewed-by: Max Reitz --- v13: new patch --- block/qcow2-refcount.c | 42 ++++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index 4efca7e..db0af2c 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -1059,9 +1059,9 @@ int qcow2_update_snapshot_refcount(BlockDriverState *= bs, int64_t l1_table_offset, int l1_size, int addend) { BDRVQcow2State *s =3D bs->opaque; - uint64_t *l1_table, *l2_table, l2_offset, offset, l1_size2, refcount; + uint64_t *l1_table, *l2_table, l2_offset, entry, l1_size2, refcount; bool l1_allocated =3D false; - int64_t old_offset, old_l2_offset; + int64_t old_entry, old_l2_offset; int i, j, l1_modified =3D 0, nb_csectors; int ret; @@ -1089,15 +1089,16 @@ int qcow2_update_snapshot_refcount(BlockDriverState= *bs, goto fail; } - for(i =3D 0;i < l1_size; i++) + for (i =3D 0; i < l1_size; i++) { be64_to_cpus(&l1_table[i]); + } } else { assert(l1_size =3D=3D s->l1_size); l1_table =3D s->l1_table; l1_allocated =3D false; } - for(i =3D 0; i < l1_size; i++) { + for (i =3D 0; i < l1_size; i++) { l2_offset =3D l1_table[i]; if (l2_offset) { old_l2_offset =3D l2_offset; @@ -1117,20 +1118,22 @@ int qcow2_update_snapshot_refcount(BlockDriverState= *bs, goto fail; } - for(j =3D 0; j < s->l2_size; j++) { + for (j =3D 0; j < s->l2_size; j++) { uint64_t cluster_index; + uint64_t offset; - offset =3D be64_to_cpu(l2_table[j]); - old_offset =3D offset; - offset &=3D ~QCOW_OFLAG_COPIED; + entry =3D be64_to_cpu(l2_table[j]); + old_entry =3D entry; + entry &=3D ~QCOW_OFLAG_COPIED; + offset =3D entry & L2E_OFFSET_MASK; - switch (qcow2_get_cluster_type(offset)) { + switch (qcow2_get_cluster_type(entry)) { case QCOW2_CLUSTER_COMPRESSED: - nb_csectors =3D ((offset >> s->csize_shift) & + nb_csectors =3D ((entry >> s->csize_shift) & s->csize_mask) + 1; if (addend !=3D 0) { ret =3D update_refcount(bs, - (offset & s->cluster_offset_mask) & ~511, + (entry & s->cluster_offset_mask) & ~511, nb_csectors * 512, abs(addend), addend < 0, QCOW2_DISCARD_SNAPSHOT); if (ret < 0) { @@ -1143,18 +1146,17 @@ int qcow2_update_snapshot_refcount(BlockDriverState= *bs, case QCOW2_CLUSTER_NORMAL: case QCOW2_CLUSTER_ZERO: - if (offset_into_cluster(s, offset & L2E_OFFSET_MAS= K)) { + if (offset_into_cluster(s, offset)) { qcow2_signal_corruption(bs, true, -1, -1, "Dat= a " - "cluster offset %#llx " - "unaligned (L2 offset:= %#" + "cluster offset %#" PR= Ix64 + " unaligned (L2 offset= : %#" PRIx64 ", L2 index: %#= x)", - offset & L2E_OFFSET_MA= SK, - l2_offset, j); + offset, l2_offset, j); ret =3D -EIO; goto fail; } - cluster_index =3D (offset & L2E_OFFSET_MASK) >> s-= >cluster_bits; + cluster_index =3D offset >> s->cluster_bits; if (!cluster_index) { /* unallocated */ refcount =3D 0; @@ -1184,14 +1186,14 @@ int qcow2_update_snapshot_refcount(BlockDriverState= *bs, } if (refcount =3D=3D 1) { - offset |=3D QCOW_OFLAG_COPIED; + entry |=3D QCOW_OFLAG_COPIED; } - if (offset !=3D old_offset) { + if (entry !=3D old_entry) { if (addend > 0) { qcow2_cache_set_dependency(bs, s->l2_table_cache, s->refcount_block_cache); } - l2_table[j] =3D cpu_to_be64(offset); + l2_table[j] =3D cpu_to_be64(entry); qcow2_cache_entry_mark_dirty(bs, s->l2_table_cache, l2_table); } --=20 2.9.3 From nobody Tue May 7 05:07:07 2024 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.zoho.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; Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1494115853121538.7264394339354; Sat, 6 May 2017 17:10:53 -0700 (PDT) Received: from localhost ([::1]:53105 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d79ml-0006Xz-D8 for importer@patchew.org; Sat, 06 May 2017 20:10:51 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45712) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d79i9-0002bI-Vr for qemu-devel@nongnu.org; Sat, 06 May 2017 20:06:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1d79i8-0007hy-UQ for qemu-devel@nongnu.org; Sat, 06 May 2017 20:06:05 -0400 Received: from mx1.redhat.com ([209.132.183.28]:57226) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1d79i5-0007gu-V7; Sat, 06 May 2017 20:06:02 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E920480F63; Sun, 7 May 2017 00:06:00 +0000 (UTC) Received: from red.redhat.com (ovpn-122-206.rdu2.redhat.com [10.10.122.206]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3AFA418345; Sun, 7 May 2017 00:06:00 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com E920480F63 Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=eblake@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com E920480F63 From: Eric Blake To: qemu-devel@nongnu.org Date: Sat, 6 May 2017 19:05:42 -0500 Message-Id: <20170507000552.20847-3-eblake@redhat.com> In-Reply-To: <20170507000552.20847-1-eblake@redhat.com> References: <20170507000552.20847-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Sun, 07 May 2017 00:06:01 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v13 02/12] qcow2: Use consistent switch indentation 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: kwolf@redhat.com, qemu-block@nongnu.org, mreitz@redhat.com 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" Fix a couple of inconsistent indentations, before an upcoming patch further tweaks the switch statements. (best viewed with 'git diff -b'). Signed-off-by: Eric Blake Reviewed-by: Max Reitz --- v13: split off non-indentation changes v12: new patch --- block/qcow2-cluster.c | 32 +++++++++---------- block/qcow2-refcount.c | 84 +++++++++++++++++++++++++---------------------= ---- 2 files changed, 58 insertions(+), 58 deletions(-) diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 31077d8..335a505 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -1504,25 +1504,25 @@ static int discard_single_l2(BlockDriverState *bs, = uint64_t offset, * but rather fall through to the backing file. */ switch (qcow2_get_cluster_type(old_l2_entry)) { - case QCOW2_CLUSTER_UNALLOCATED: - if (full_discard || !bs->backing) { - continue; - } - break; + case QCOW2_CLUSTER_UNALLOCATED: + if (full_discard || !bs->backing) { + continue; + } + break; - case QCOW2_CLUSTER_ZERO: - /* Preallocated zero clusters should be discarded in any c= ase */ - if (!full_discard && (old_l2_entry & L2E_OFFSET_MASK) =3D= =3D 0) { - continue; - } - break; + case QCOW2_CLUSTER_ZERO: + /* Preallocated zero clusters should be discarded in any case = */ + if (!full_discard && (old_l2_entry & L2E_OFFSET_MASK) =3D=3D 0= ) { + continue; + } + break; - case QCOW2_CLUSTER_NORMAL: - case QCOW2_CLUSTER_COMPRESSED: - break; + case QCOW2_CLUSTER_NORMAL: + case QCOW2_CLUSTER_COMPRESSED: + break; - default: - abort(); + default: + abort(); } /* First remove L2 entries */ diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index db0af2c..908dbe5 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -1128,61 +1128,61 @@ int qcow2_update_snapshot_refcount(BlockDriverState= *bs, offset =3D entry & L2E_OFFSET_MASK; switch (qcow2_get_cluster_type(entry)) { - case QCOW2_CLUSTER_COMPRESSED: - nb_csectors =3D ((entry >> s->csize_shift) & - s->csize_mask) + 1; - if (addend !=3D 0) { - ret =3D update_refcount(bs, + case QCOW2_CLUSTER_COMPRESSED: + nb_csectors =3D ((entry >> s->csize_shift) & + s->csize_mask) + 1; + if (addend !=3D 0) { + ret =3D update_refcount(bs, (entry & s->cluster_offset_mask) & ~511, nb_csectors * 512, abs(addend), addend < 0, QCOW2_DISCARD_SNAPSHOT); - if (ret < 0) { - goto fail; - } - } - /* compressed clusters are never modified */ - refcount =3D 2; - break; - - case QCOW2_CLUSTER_NORMAL: - case QCOW2_CLUSTER_ZERO: - if (offset_into_cluster(s, offset)) { - qcow2_signal_corruption(bs, true, -1, -1, "Dat= a " - "cluster offset %#" PR= Ix64 - " unaligned (L2 offset= : %#" - PRIx64 ", L2 index: %#= x)", - offset, l2_offset, j); - ret =3D -EIO; + if (ret < 0) { goto fail; } + } + /* compressed clusters are never modified */ + refcount =3D 2; + break; - cluster_index =3D offset >> s->cluster_bits; - if (!cluster_index) { - /* unallocated */ - refcount =3D 0; - break; - } - if (addend !=3D 0) { - ret =3D qcow2_update_cluster_refcount(bs, + case QCOW2_CLUSTER_NORMAL: + case QCOW2_CLUSTER_ZERO: + if (offset_into_cluster(s, offset)) { + qcow2_signal_corruption(bs, true, -1, -1, "Data " + "cluster offset %#" PRIx64 + " unaligned (L2 offset: %#" + PRIx64 ", L2 index: %#x)", + offset, l2_offset, j); + ret =3D -EIO; + goto fail; + } + + cluster_index =3D offset >> s->cluster_bits; + if (!cluster_index) { + /* unallocated */ + refcount =3D 0; + break; + } + if (addend !=3D 0) { + ret =3D qcow2_update_cluster_refcount(bs, cluster_index, abs(addend), addend < 0, QCOW2_DISCARD_SNAPSHOT); - if (ret < 0) { - goto fail; - } - } - - ret =3D qcow2_get_refcount(bs, cluster_index, &ref= count); if (ret < 0) { goto fail; } - break; + } - case QCOW2_CLUSTER_UNALLOCATED: - refcount =3D 0; - break; + ret =3D qcow2_get_refcount(bs, cluster_index, &refcoun= t); + if (ret < 0) { + goto fail; + } + break; - default: - abort(); + case QCOW2_CLUSTER_UNALLOCATED: + refcount =3D 0; + break; + + default: + abort(); } if (refcount =3D=3D 1) { --=20 2.9.3 From nobody Tue May 7 05:07:07 2024 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.zoho.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; Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1494115701406798.916531927878; Sat, 6 May 2017 17:08:21 -0700 (PDT) Received: from localhost ([::1]:53093 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d79kJ-0004Qg-SP for importer@patchew.org; Sat, 06 May 2017 20:08:19 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45732) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d79iA-0002br-K4 for qemu-devel@nongnu.org; Sat, 06 May 2017 20:06:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1d79i9-0007ig-Iy for qemu-devel@nongnu.org; Sat, 06 May 2017 20:06:06 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38906) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1d79i6-0007hA-W5; Sat, 06 May 2017 20:06:03 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D6F4E8553F; Sun, 7 May 2017 00:06:01 +0000 (UTC) Received: from red.redhat.com (ovpn-122-206.rdu2.redhat.com [10.10.122.206]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2ACC618345; Sun, 7 May 2017 00:06:01 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com D6F4E8553F Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=eblake@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com D6F4E8553F From: Eric Blake To: qemu-devel@nongnu.org Date: Sat, 6 May 2017 19:05:43 -0500 Message-Id: <20170507000552.20847-4-eblake@redhat.com> In-Reply-To: <20170507000552.20847-1-eblake@redhat.com> References: <20170507000552.20847-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Sun, 07 May 2017 00:06:02 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v13 03/12] block: Update comments on BDRV_BLOCK_* meanings 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: kwolf@redhat.com, qemu-block@nongnu.org, mreitz@redhat.com 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" We had some conflicting documentation: a nice 8-way table that described all possible combinations of DATA, ZERO, and OFFSET_VALID, contrasted with text that implied that OFFSET_VALID always meant raw data could be read directly. Furthermore, the text refers a lot to bs->file, even though the interface was updated back in 67a0fd2a to let the driver pass back a specific BDS (not necessarily bs->file). As the 8-way table is the intended semantics, simplify the rest of the text to get rid of the confusion. ALLOCATED is always set by the block layer for convenience (drivers do not have to worry about it). RAW is used only internally, but by more than the raw driver. Document these additional items on the driver callback. Suggested-by: Max Reitz Signed-off-by: Eric Blake Reviewed-by: Max Reitz --- v13: commit message tweaks v12: even more wording tweaks v11: reserved for blkdebug half of v10 v10: new patch --- include/block/block.h | 35 +++++++++++++++++++---------------- include/block/block_int.h | 7 +++++++ 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/include/block/block.h b/include/block/block.h index 877fbb0..14fba5e 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -121,29 +121,32 @@ typedef struct HDGeometry { #define BDRV_REQUEST_MAX_BYTES (BDRV_REQUEST_MAX_SECTORS << BDRV_SECTOR_BI= TS) /* - * Allocation status flags - * BDRV_BLOCK_DATA: data is read from a file returned by bdrv_get_block_st= atus. - * BDRV_BLOCK_ZERO: sectors read as zero - * BDRV_BLOCK_OFFSET_VALID: sector stored as raw data in a file returned by - * bdrv_get_block_status. + * Allocation status flags for bdrv_get_block_status() and friends. + * + * Public flags: + * BDRV_BLOCK_DATA: allocation for data at offset is tied to this layer + * BDRV_BLOCK_ZERO: offset reads as zero + * BDRV_BLOCK_OFFSET_VALID: an associated offset exists for accessing raw = data * BDRV_BLOCK_ALLOCATED: the content of the block is determined by this - * layer (as opposed to the backing file) - * BDRV_BLOCK_RAW: used internally to indicate that the request - * was answered by the raw driver and that one - * should look in bs->file directly. + * layer (short for DATA || ZERO), set by block layer * - * If BDRV_BLOCK_OFFSET_VALID is set, bits 9-62 represent the offset in - * bs->file where sector data can be read from as raw data. + * Internal flag: + * BDRV_BLOCK_RAW: used internally to indicate that the request was + * answered by a passthrough driver such as raw and that t= he + * block layer should recompute the answer from bs->file. * - * DATA =3D=3D 0 && ZERO =3D=3D 0 means that data is read from backing_hd = if present. + * If BDRV_BLOCK_OFFSET_VALID is set, bits 9-62 (BDRV_BLOCK_OFFSET_MASK) + * represent the offset in the returned BDS that is allocated for the + * corresponding raw data; however, whether that offset actually contains + * data also depends on BDRV_BLOCK_DATA and BDRV_BLOCK_ZERO, as follows: * * DATA ZERO OFFSET_VALID - * t t t sectors read as zero, bs->file is zero at offset - * t f t sectors read as valid from bs->file at offset - * f t t sectors preallocated, read as zero, bs->file not + * t t t sectors read as zero, returned file is zero at o= ffset + * t f t sectors read as valid from file at offset + * f t t sectors preallocated, read as zero, returned fil= e not * necessarily zero at offset * f f t sectors preallocated but read from backing_hd, - * bs->file contains garbage at offset + * returned file contains garbage at offset * t t f sectors preallocated, read as zero, unknown offs= et * t f f sectors read from unknown file or offset * f t f not allocated or unknown offset, read as zero diff --git a/include/block/block_int.h b/include/block/block_int.h index 1b4d08e..771fa97 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -165,6 +165,13 @@ struct BlockDriver { int64_t offset, int count, BdrvRequestFlags flags); int coroutine_fn (*bdrv_co_pdiscard)(BlockDriverState *bs, int64_t offset, int count); + + /* + * Building block for bdrv_block_status[_above]. The driver should + * answer only according to the current layer, and should not + * set BDRV_BLOCK_ALLOCATED, but may set BDRV_BLOCK_RAW. See block.h + * for the meaning of _DATA, _ZERO, and _OFFSET_VALID. + */ int64_t coroutine_fn (*bdrv_co_get_block_status)(BlockDriverState *bs, int64_t sector_num, int nb_sectors, int *pnum, BlockDriverState **file); --=20 2.9.3 From nobody Tue May 7 05:07:07 2024 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.zoho.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; Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1494115967685656.9160774842255; Sat, 6 May 2017 17:12:47 -0700 (PDT) Received: from localhost ([::1]:53116 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d79oc-0008FS-9w for importer@patchew.org; Sat, 06 May 2017 20:12:46 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45762) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d79iB-0002dJ-N0 for qemu-devel@nongnu.org; Sat, 06 May 2017 20:06:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1d79iA-0007jM-JL for qemu-devel@nongnu.org; Sat, 06 May 2017 20:06:07 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53504) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1d79i7-0007hL-To; Sat, 06 May 2017 20:06:04 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E411EC05681A; Sun, 7 May 2017 00:06:02 +0000 (UTC) Received: from red.redhat.com (ovpn-122-206.rdu2.redhat.com [10.10.122.206]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1994718345; Sun, 7 May 2017 00:06:01 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com E411EC05681A Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=eblake@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com E411EC05681A From: Eric Blake To: qemu-devel@nongnu.org Date: Sat, 6 May 2017 19:05:44 -0500 Message-Id: <20170507000552.20847-5-eblake@redhat.com> In-Reply-To: <20170507000552.20847-1-eblake@redhat.com> References: <20170507000552.20847-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Sun, 07 May 2017 00:06:03 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v13 04/12] qcow2: Correctly report status of preallocated zero clusters 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: kwolf@redhat.com, qemu-block@nongnu.org, mreitz@redhat.com 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" We were throwing away the preallocation information associated with zero clusters. But we should be matching the well-defined semantics in bdrv_get_block_status(), where (BDRV_BLOCK_ZERO | BDRV_BLOCK_OFFSET_VALID) informs the user which offset is reserved, while still reminding the user that reading from that offset is likely to read garbage. count_contiguous_clusters_by_type() is now used only for unallocated cluster runs, hence it gets renamed and tightened. Making this change lets us see which portions of an image are zero but preallocated, when using qemu-img map --output=3Djson. The --output=3Dhuman side intentionally ignores all zero clusters, whether or not they are preallocated. The fact that there is no change to qemu-iotests './check -qcow2' merely means that we aren't yet testing this aspect of qemu-img; a later patch will add a test. Signed-off-by: Eric Blake Reviewed-by: Max Reitz --- v13: no change v12: rename helper function v11: reserved for blkdebug half of v10 v10: new patch --- block/qcow2-cluster.c | 45 +++++++++++++++++++++++++++++++++++---------- 1 file changed, 35 insertions(+), 10 deletions(-) diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 335a505..f3bfce6 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -334,16 +334,23 @@ static int count_contiguous_clusters(int nb_clusters,= int cluster_size, return i; } -static int count_contiguous_clusters_by_type(int nb_clusters, - uint64_t *l2_table, - int wanted_type) +/* + * Checks how many consecutive unallocated clusters in a given L2 + * table have the same cluster type. + */ +static int count_contiguous_clusters_unallocated(int nb_clusters, + uint64_t *l2_table, + int wanted_type) { int i; + assert(wanted_type =3D=3D QCOW2_CLUSTER_ZERO || + wanted_type =3D=3D QCOW2_CLUSTER_UNALLOCATED); for (i =3D 0; i < nb_clusters; i++) { - int type =3D qcow2_get_cluster_type(be64_to_cpu(l2_table[i])); + uint64_t entry =3D be64_to_cpu(l2_table[i]); + int type =3D qcow2_get_cluster_type(entry); - if (type !=3D wanted_type) { + if (type !=3D wanted_type || entry & L2E_OFFSET_MASK) { break; } } @@ -565,14 +572,32 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, ui= nt64_t offset, ret =3D -EIO; goto fail; } - c =3D count_contiguous_clusters_by_type(nb_clusters, &l2_table[l2_= index], - QCOW2_CLUSTER_ZERO); - *cluster_offset =3D 0; + /* Distinguish between pure zero clusters and pre-allocated ones */ + if (*cluster_offset & L2E_OFFSET_MASK) { + c =3D count_contiguous_clusters(nb_clusters, s->cluster_size, + &l2_table[l2_index], QCOW_OFLAG_= ZERO); + *cluster_offset &=3D L2E_OFFSET_MASK; + if (offset_into_cluster(s, *cluster_offset)) { + qcow2_signal_corruption(bs, true, -1, -1, + "Preallocated zero cluster offset = %#" + PRIx64 " unaligned (L2 offset: %#" + PRIx64 ", L2 index: %#x)", + *cluster_offset, l2_offset, l2_ind= ex); + ret =3D -EIO; + goto fail; + } + } else { + c =3D count_contiguous_clusters_unallocated(nb_clusters, + &l2_table[l2_index], + QCOW2_CLUSTER_ZERO); + *cluster_offset =3D 0; + } break; case QCOW2_CLUSTER_UNALLOCATED: /* how many empty clusters ? */ - c =3D count_contiguous_clusters_by_type(nb_clusters, &l2_table[l2_= index], - QCOW2_CLUSTER_UNALLOCATED); + c =3D count_contiguous_clusters_unallocated(nb_clusters, + &l2_table[l2_index], + QCOW2_CLUSTER_UNALLOCATE= D); *cluster_offset =3D 0; break; case QCOW2_CLUSTER_NORMAL: --=20 2.9.3 From nobody Tue May 7 05:07:07 2024 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.zoho.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; Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1494115996564933.5740426230441; Sat, 6 May 2017 17:13:16 -0700 (PDT) Received: from localhost ([::1]:53117 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d79p5-0000Ug-5Z for importer@patchew.org; Sat, 06 May 2017 20:13:15 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45790) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d79iC-0002eT-UH for qemu-devel@nongnu.org; Sat, 06 May 2017 20:06:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1d79iB-0007jy-R0 for qemu-devel@nongnu.org; Sat, 06 May 2017 20:06:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53102) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1d79i8-0007hi-TA; Sat, 06 May 2017 20:06:05 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id DB046201FB; Sun, 7 May 2017 00:06:03 +0000 (UTC) Received: from red.redhat.com (ovpn-122-206.rdu2.redhat.com [10.10.122.206]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2A6E718345; Sun, 7 May 2017 00:06:03 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com DB046201FB Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=eblake@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com DB046201FB From: Eric Blake To: qemu-devel@nongnu.org Date: Sat, 6 May 2017 19:05:45 -0500 Message-Id: <20170507000552.20847-6-eblake@redhat.com> In-Reply-To: <20170507000552.20847-1-eblake@redhat.com> References: <20170507000552.20847-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Sun, 07 May 2017 00:06:04 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v13 05/12] qcow2: Name typedef for cluster type 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: kwolf@redhat.com, qemu-block@nongnu.org, mreitz@redhat.com 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" Although it doesn't add all that much type safety (this is C, after all), it does add a bit of legibility to use the name QCow2ClusterType instead of a plain int. In particular, qcow2_get_cluster_offset() has an overloaded return type; a QCow2ClusterType on success, and -errno on failure; keeping the cluster type in a separate variable makes it slightly easier for the next patch to make further computations based on the type. Suggested-by: Max Reitz Signed-off-by: Eric Blake --- v13: new patch --- block/qcow2.h | 6 +++--- block/qcow2-cluster.c | 17 +++++++++-------- block/qcow2-refcount.c | 2 +- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/block/qcow2.h b/block/qcow2.h index 8731f24..c148bbc 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -349,12 +349,12 @@ typedef struct QCowL2Meta QLIST_ENTRY(QCowL2Meta) next_in_flight; } QCowL2Meta; -enum { +typedef enum QCow2ClusterType { QCOW2_CLUSTER_UNALLOCATED, QCOW2_CLUSTER_NORMAL, QCOW2_CLUSTER_COMPRESSED, QCOW2_CLUSTER_ZERO -}; +} QCow2ClusterType; typedef enum QCow2MetadataOverlap { QCOW2_OL_MAIN_HEADER_BITNR =3D 0, @@ -443,7 +443,7 @@ static inline uint64_t qcow2_max_refcount_clusters(BDRV= Qcow2State *s) return QCOW_MAX_REFTABLE_SIZE >> s->cluster_bits; } -static inline int qcow2_get_cluster_type(uint64_t l2_entry) +static inline QCow2ClusterType qcow2_get_cluster_type(uint64_t l2_entry) { if (l2_entry & QCOW_OFLAG_COMPRESSED) { return QCOW2_CLUSTER_COMPRESSED; diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index f3bfce6..ed78a30 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -340,7 +340,7 @@ static int count_contiguous_clusters(int nb_clusters, i= nt cluster_size, */ static int count_contiguous_clusters_unallocated(int nb_clusters, uint64_t *l2_table, - int wanted_type) + QCow2ClusterType wanted_t= ype) { int i; @@ -500,6 +500,7 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, uint= 64_t offset, int l1_bits, c; unsigned int offset_in_cluster; uint64_t bytes_available, bytes_needed, nb_clusters; + QCow2ClusterType type; int ret; offset_in_cluster =3D offset_into_cluster(s, offset); @@ -522,13 +523,13 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, ui= nt64_t offset, l1_index =3D offset >> l1_bits; if (l1_index >=3D s->l1_size) { - ret =3D QCOW2_CLUSTER_UNALLOCATED; + type =3D QCOW2_CLUSTER_UNALLOCATED; goto out; } l2_offset =3D s->l1_table[l1_index] & L1E_OFFSET_MASK; if (!l2_offset) { - ret =3D QCOW2_CLUSTER_UNALLOCATED; + type =3D QCOW2_CLUSTER_UNALLOCATED; goto out; } @@ -557,8 +558,8 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, uint= 64_t offset, * true */ assert(nb_clusters <=3D INT_MAX); - ret =3D qcow2_get_cluster_type(*cluster_offset); - switch (ret) { + type =3D qcow2_get_cluster_type(*cluster_offset); + switch (type) { case QCOW2_CLUSTER_COMPRESSED: /* Compressed clusters can only be processed one by one */ c =3D 1; @@ -633,7 +634,7 @@ out: assert(bytes_available - offset_in_cluster <=3D UINT_MAX); *bytes =3D bytes_available - offset_in_cluster; - return ret; + return type; fail: qcow2_cache_put(bs, s->l2_table_cache, (void **)&l2_table); @@ -891,7 +892,7 @@ static int count_cow_clusters(BDRVQcow2State *s, int nb= _clusters, for (i =3D 0; i < nb_clusters; i++) { uint64_t l2_entry =3D be64_to_cpu(l2_table[l2_index + i]); - int cluster_type =3D qcow2_get_cluster_type(l2_entry); + QCow2ClusterType cluster_type =3D qcow2_get_cluster_type(l2_entry); switch(cluster_type) { case QCOW2_CLUSTER_NORMAL: @@ -1757,7 +1758,7 @@ static int expand_zero_clusters_in_l1(BlockDriverStat= e *bs, uint64_t *l1_table, for (j =3D 0; j < s->l2_size; j++) { uint64_t l2_entry =3D be64_to_cpu(l2_table[j]); int64_t offset =3D l2_entry & L2E_OFFSET_MASK; - int cluster_type =3D qcow2_get_cluster_type(l2_entry); + QCow2ClusterType cluster_type =3D qcow2_get_cluster_type(l2_en= try); bool preallocated =3D offset !=3D 0; if (cluster_type !=3D QCOW2_CLUSTER_ZERO) { diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index 908dbe5..e639b34 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -1640,7 +1640,7 @@ static int check_oflag_copied(BlockDriverState *bs, B= drvCheckResult *res, for (j =3D 0; j < s->l2_size; j++) { uint64_t l2_entry =3D be64_to_cpu(l2_table[j]); uint64_t data_offset =3D l2_entry & L2E_OFFSET_MASK; - int cluster_type =3D qcow2_get_cluster_type(l2_entry); + QCow2ClusterType cluster_type =3D qcow2_get_cluster_type(l2_en= try); if ((cluster_type =3D=3D QCOW2_CLUSTER_NORMAL) || ((cluster_type =3D=3D QCOW2_CLUSTER_ZERO) && (data_offset = !=3D 0))) { --=20 2.9.3 From nobody Tue May 7 05:07:07 2024 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.zoho.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; Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1494115699878926.3875354349924; Sat, 6 May 2017 17:08:19 -0700 (PDT) Received: from localhost ([::1]:53092 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d79kH-0004P9-QO for importer@patchew.org; Sat, 06 May 2017 20:08:17 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45855) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d79iG-0002if-R6 for qemu-devel@nongnu.org; Sat, 06 May 2017 20:06:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1d79iE-0007l1-OE for qemu-devel@nongnu.org; Sat, 06 May 2017 20:06:12 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39172) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1d79i9-0007iR-UK; Sat, 06 May 2017 20:06:06 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E66078123D; Sun, 7 May 2017 00:06:04 +0000 (UTC) Received: from red.redhat.com (ovpn-122-206.rdu2.redhat.com [10.10.122.206]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1DD3A18345; Sun, 7 May 2017 00:06:04 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com E66078123D Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=eblake@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com E66078123D From: Eric Blake To: qemu-devel@nongnu.org Date: Sat, 6 May 2017 19:05:46 -0500 Message-Id: <20170507000552.20847-7-eblake@redhat.com> In-Reply-To: <20170507000552.20847-1-eblake@redhat.com> References: <20170507000552.20847-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Sun, 07 May 2017 00:06:05 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v13 06/12] qcow2: Make distinction between zero cluster types obvious 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: kwolf@redhat.com, qemu-block@nongnu.org, mreitz@redhat.com 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" Treat plain zero clusters differently from allocated ones, so that we can simplify the logic of checking whether an offset is present. Do this by splitting QCOW2_CLUSTER_ZERO into two new enums, QCOW2_CLUSTER_ZERO_PLAIN and QCOW2_CLUSTER_ZERO_ALLOC. I tried to arrange the enum so that we could use 'ret <=3D QCOW2_CLUSTER_ZERO_PLAIN' for all unallocated types, and 'ret >=3D QCOW2_CLUSTER_ZERO_ALLOC' for allocated types, although I didn't actually end up taking advantage of the layout. In many cases, this leads to simpler code, by properly combining cases (sometimes, both zero types pair together, other times, plain zero is more like unallocated while allocated zero is more like normal). Signed-off-by: Eric Blake Reviewed-by: Max Reitz --- v13: s/!preallocated/cluster_type =3D=3D QCOW2_CLUSTER_ZERO_PLAIN/, fix error message text (including iotest fallout), rebase to earlier cleanups v12: new patch --- block/qcow2.h | 8 +++-- block/qcow2-cluster.c | 81 ++++++++++++++++++------------------------= ---- block/qcow2-refcount.c | 44 +++++++++++-------------- block/qcow2.c | 9 ++++-- tests/qemu-iotests/060.out | 6 ++-- 5 files changed, 64 insertions(+), 84 deletions(-) diff --git a/block/qcow2.h b/block/qcow2.h index c148bbc..810ceb8 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -351,9 +351,10 @@ typedef struct QCowL2Meta typedef enum QCow2ClusterType { QCOW2_CLUSTER_UNALLOCATED, + QCOW2_CLUSTER_ZERO_PLAIN, + QCOW2_CLUSTER_ZERO_ALLOC, QCOW2_CLUSTER_NORMAL, QCOW2_CLUSTER_COMPRESSED, - QCOW2_CLUSTER_ZERO } QCow2ClusterType; typedef enum QCow2MetadataOverlap { @@ -448,7 +449,10 @@ static inline QCow2ClusterType qcow2_get_cluster_type(= uint64_t l2_entry) if (l2_entry & QCOW_OFLAG_COMPRESSED) { return QCOW2_CLUSTER_COMPRESSED; } else if (l2_entry & QCOW_OFLAG_ZERO) { - return QCOW2_CLUSTER_ZERO; + if (l2_entry & L2E_OFFSET_MASK) { + return QCOW2_CLUSTER_ZERO_ALLOC; + } + return QCOW2_CLUSTER_ZERO_PLAIN; } else if (!(l2_entry & L2E_OFFSET_MASK)) { return QCOW2_CLUSTER_UNALLOCATED; } else { diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index ed78a30..a4a3229 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -321,8 +321,7 @@ static int count_contiguous_clusters(int nb_clusters, i= nt cluster_size, /* must be allocated */ first_cluster_type =3D qcow2_get_cluster_type(first_entry); assert(first_cluster_type =3D=3D QCOW2_CLUSTER_NORMAL || - (first_cluster_type =3D=3D QCOW2_CLUSTER_ZERO && - (first_entry & L2E_OFFSET_MASK) !=3D 0)); + first_cluster_type =3D=3D QCOW2_CLUSTER_ZERO_ALLOC); for (i =3D 0; i < nb_clusters; i++) { uint64_t l2_entry =3D be64_to_cpu(l2_table[i]) & mask; @@ -344,13 +343,13 @@ static int count_contiguous_clusters_unallocated(int = nb_clusters, { int i; - assert(wanted_type =3D=3D QCOW2_CLUSTER_ZERO || + assert(wanted_type =3D=3D QCOW2_CLUSTER_ZERO_PLAIN || wanted_type =3D=3D QCOW2_CLUSTER_UNALLOCATED); for (i =3D 0; i < nb_clusters; i++) { uint64_t entry =3D be64_to_cpu(l2_table[i]); - int type =3D qcow2_get_cluster_type(entry); + QCow2ClusterType type =3D qcow2_get_cluster_type(entry); - if (type !=3D wanted_type || entry & L2E_OFFSET_MASK) { + if (type !=3D wanted_type) { break; } } @@ -559,55 +558,36 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, ui= nt64_t offset, assert(nb_clusters <=3D INT_MAX); type =3D qcow2_get_cluster_type(*cluster_offset); + if (s->qcow_version < 3 && (type =3D=3D QCOW2_CLUSTER_ZERO_PLAIN || + type =3D=3D QCOW2_CLUSTER_ZERO_ALLOC)) { + qcow2_signal_corruption(bs, true, -1, -1, "Zero cluster entry foun= d" + " in pre-v3 image (L2 offset: %#" PRIx64 + ", L2 index: %#x)", l2_offset, l2_index); + ret =3D -EIO; + goto fail; + } switch (type) { case QCOW2_CLUSTER_COMPRESSED: /* Compressed clusters can only be processed one by one */ c =3D 1; *cluster_offset &=3D L2E_COMPRESSED_OFFSET_SIZE_MASK; break; - case QCOW2_CLUSTER_ZERO: - if (s->qcow_version < 3) { - qcow2_signal_corruption(bs, true, -1, -1, "Zero cluster entry = found" - " in pre-v3 image (L2 offset: %#" PRIx= 64 - ", L2 index: %#x)", l2_offset, l2_inde= x); - ret =3D -EIO; - goto fail; - } - /* Distinguish between pure zero clusters and pre-allocated ones */ - if (*cluster_offset & L2E_OFFSET_MASK) { - c =3D count_contiguous_clusters(nb_clusters, s->cluster_size, - &l2_table[l2_index], QCOW_OFLAG_= ZERO); - *cluster_offset &=3D L2E_OFFSET_MASK; - if (offset_into_cluster(s, *cluster_offset)) { - qcow2_signal_corruption(bs, true, -1, -1, - "Preallocated zero cluster offset = %#" - PRIx64 " unaligned (L2 offset: %#" - PRIx64 ", L2 index: %#x)", - *cluster_offset, l2_offset, l2_ind= ex); - ret =3D -EIO; - goto fail; - } - } else { - c =3D count_contiguous_clusters_unallocated(nb_clusters, - &l2_table[l2_index], - QCOW2_CLUSTER_ZERO); - *cluster_offset =3D 0; - } - break; + case QCOW2_CLUSTER_ZERO_PLAIN: case QCOW2_CLUSTER_UNALLOCATED: /* how many empty clusters ? */ c =3D count_contiguous_clusters_unallocated(nb_clusters, - &l2_table[l2_index], - QCOW2_CLUSTER_UNALLOCATE= D); + &l2_table[l2_index], typ= e); *cluster_offset =3D 0; break; + case QCOW2_CLUSTER_ZERO_ALLOC: case QCOW2_CLUSTER_NORMAL: /* how many allocated clusters ? */ c =3D count_contiguous_clusters(nb_clusters, s->cluster_size, - &l2_table[l2_index], QCOW_OFLAG_ZERO); + &l2_table[l2_index], QCOW_OFLAG_ZERO= ); *cluster_offset &=3D L2E_OFFSET_MASK; if (offset_into_cluster(s, *cluster_offset)) { - qcow2_signal_corruption(bs, true, -1, -1, "Data cluster offset= %#" + qcow2_signal_corruption(bs, true, -1, -1, + "Cluster allocation offset %#" PRIx64 " unaligned (L2 offset: %#" PRI= x64 ", L2 index: %#x)", *cluster_offset, l2_offset, l2_index); @@ -902,7 +882,8 @@ static int count_cow_clusters(BDRVQcow2State *s, int nb= _clusters, break; case QCOW2_CLUSTER_UNALLOCATED: case QCOW2_CLUSTER_COMPRESSED: - case QCOW2_CLUSTER_ZERO: + case QCOW2_CLUSTER_ZERO_PLAIN: + case QCOW2_CLUSTER_ZERO_ALLOC: break; default: abort(); @@ -1203,8 +1184,8 @@ static int handle_alloc(BlockDriverState *bs, uint64_= t guest_offset, * wrong with our code. */ assert(nb_clusters > 0); - if (qcow2_get_cluster_type(entry) =3D=3D QCOW2_CLUSTER_ZERO && - (entry & L2E_OFFSET_MASK) !=3D 0 && (entry & QCOW_OFLAG_COPIED) && + if (qcow2_get_cluster_type(entry) =3D=3D QCOW2_CLUSTER_ZERO_ALLOC && + (entry & QCOW_OFLAG_COPIED) && (!*host_offset || start_of_cluster(s, *host_offset) =3D=3D (entry & L2E_OFFSET_MASK= ))) { @@ -1536,13 +1517,13 @@ static int discard_single_l2(BlockDriverState *bs, = uint64_t offset, } break; - case QCOW2_CLUSTER_ZERO: - /* Preallocated zero clusters should be discarded in any case = */ - if (!full_discard && (old_l2_entry & L2E_OFFSET_MASK) =3D=3D 0= ) { + case QCOW2_CLUSTER_ZERO_PLAIN: + if (!full_discard) { continue; } break; + case QCOW2_CLUSTER_ZERO_ALLOC: case QCOW2_CLUSTER_NORMAL: case QCOW2_CLUSTER_COMPRESSED: break; @@ -1759,13 +1740,13 @@ static int expand_zero_clusters_in_l1(BlockDriverSt= ate *bs, uint64_t *l1_table, uint64_t l2_entry =3D be64_to_cpu(l2_table[j]); int64_t offset =3D l2_entry & L2E_OFFSET_MASK; QCow2ClusterType cluster_type =3D qcow2_get_cluster_type(l2_en= try); - bool preallocated =3D offset !=3D 0; - if (cluster_type !=3D QCOW2_CLUSTER_ZERO) { + if (cluster_type !=3D QCOW2_CLUSTER_ZERO_PLAIN && + cluster_type !=3D QCOW2_CLUSTER_ZERO_ALLOC) { continue; } - if (!preallocated) { + if (cluster_type =3D=3D QCOW2_CLUSTER_ZERO_PLAIN) { if (!bs->backing) { /* not backed; therefore we can simply deallocate the * cluster */ @@ -1800,7 +1781,7 @@ static int expand_zero_clusters_in_l1(BlockDriverStat= e *bs, uint64_t *l1_table, "%#" PRIx64 " unaligned (L2 offset= : %#" PRIx64 ", L2 index: %#x)", offset, l2_offset, j); - if (!preallocated) { + if (cluster_type =3D=3D QCOW2_CLUSTER_ZERO_PLAIN) { qcow2_free_clusters(bs, offset, s->cluster_size, QCOW2_DISCARD_ALWAYS); } @@ -1810,7 +1791,7 @@ static int expand_zero_clusters_in_l1(BlockDriverStat= e *bs, uint64_t *l1_table, ret =3D qcow2_pre_write_overlap_check(bs, 0, offset, s->cluste= r_size); if (ret < 0) { - if (!preallocated) { + if (cluster_type =3D=3D QCOW2_CLUSTER_ZERO_PLAIN) { qcow2_free_clusters(bs, offset, s->cluster_size, QCOW2_DISCARD_ALWAYS); } @@ -1819,7 +1800,7 @@ static int expand_zero_clusters_in_l1(BlockDriverStat= e *bs, uint64_t *l1_table, ret =3D bdrv_pwrite_zeroes(bs->file, offset, s->cluster_size, = 0); if (ret < 0) { - if (!preallocated) { + if (cluster_type =3D=3D QCOW2_CLUSTER_ZERO_PLAIN) { qcow2_free_clusters(bs, offset, s->cluster_size, QCOW2_DISCARD_ALWAYS); } diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index e639b34..7c06061 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -1028,18 +1028,17 @@ void qcow2_free_any_clusters(BlockDriverState *bs, = uint64_t l2_entry, } break; case QCOW2_CLUSTER_NORMAL: - case QCOW2_CLUSTER_ZERO: - if (l2_entry & L2E_OFFSET_MASK) { - if (offset_into_cluster(s, l2_entry & L2E_OFFSET_MASK)) { - qcow2_signal_corruption(bs, false, -1, -1, - "Cannot free unaligned cluster %#l= lx", - l2_entry & L2E_OFFSET_MASK); - } else { - qcow2_free_clusters(bs, l2_entry & L2E_OFFSET_MASK, - nb_clusters << s->cluster_bits, type); - } + case QCOW2_CLUSTER_ZERO_ALLOC: + if (offset_into_cluster(s, l2_entry & L2E_OFFSET_MASK)) { + qcow2_signal_corruption(bs, false, -1, -1, + "Cannot free unaligned cluster %#llx", + l2_entry & L2E_OFFSET_MASK); + } else { + qcow2_free_clusters(bs, l2_entry & L2E_OFFSET_MASK, + nb_clusters << s->cluster_bits, type); } break; + case QCOW2_CLUSTER_ZERO_PLAIN: case QCOW2_CLUSTER_UNALLOCATED: break; default: @@ -1145,10 +1144,10 @@ int qcow2_update_snapshot_refcount(BlockDriverState= *bs, break; case QCOW2_CLUSTER_NORMAL: - case QCOW2_CLUSTER_ZERO: + case QCOW2_CLUSTER_ZERO_ALLOC: if (offset_into_cluster(s, offset)) { - qcow2_signal_corruption(bs, true, -1, -1, "Data " - "cluster offset %#" PRIx64 + qcow2_signal_corruption(bs, true, -1, -1, "Cluster= " + "allocation offset %#" PRI= x64 " unaligned (L2 offset: %#" PRIx64 ", L2 index: %#x)", offset, l2_offset, j); @@ -1157,11 +1156,7 @@ int qcow2_update_snapshot_refcount(BlockDriverState = *bs, } cluster_index =3D offset >> s->cluster_bits; - if (!cluster_index) { - /* unallocated */ - refcount =3D 0; - break; - } + assert(cluster_index); if (addend !=3D 0) { ret =3D qcow2_update_cluster_refcount(bs, cluster_index, abs(addend), addend < 0, @@ -1177,6 +1172,7 @@ int qcow2_update_snapshot_refcount(BlockDriverState *= bs, } break; + case QCOW2_CLUSTER_ZERO_PLAIN: case QCOW2_CLUSTER_UNALLOCATED: refcount =3D 0; break; @@ -1443,12 +1439,7 @@ static int check_refcounts_l2(BlockDriverState *bs, = BdrvCheckResult *res, } break; - case QCOW2_CLUSTER_ZERO: - if ((l2_entry & L2E_OFFSET_MASK) =3D=3D 0) { - break; - } - /* fall through */ - + case QCOW2_CLUSTER_ZERO_ALLOC: case QCOW2_CLUSTER_NORMAL: { uint64_t offset =3D l2_entry & L2E_OFFSET_MASK; @@ -1478,6 +1469,7 @@ static int check_refcounts_l2(BlockDriverState *bs, B= drvCheckResult *res, break; } + case QCOW2_CLUSTER_ZERO_PLAIN: case QCOW2_CLUSTER_UNALLOCATED: break; @@ -1642,8 +1634,8 @@ static int check_oflag_copied(BlockDriverState *bs, B= drvCheckResult *res, uint64_t data_offset =3D l2_entry & L2E_OFFSET_MASK; QCow2ClusterType cluster_type =3D qcow2_get_cluster_type(l2_en= try); - if ((cluster_type =3D=3D QCOW2_CLUSTER_NORMAL) || - ((cluster_type =3D=3D QCOW2_CLUSTER_ZERO) && (data_offset = !=3D 0))) { + if (cluster_type =3D=3D QCOW2_CLUSTER_NORMAL || + cluster_type =3D=3D QCOW2_CLUSTER_ZERO_ALLOC) { ret =3D qcow2_get_refcount(bs, data_offset >> s->cluster_bits, &refcount); diff --git a/block/qcow2.c b/block/qcow2.c index f5a72a4..dded5a0 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1385,7 +1385,7 @@ static int64_t coroutine_fn qcow2_co_get_block_status= (BlockDriverState *bs, *file =3D bs->file->bs; status |=3D BDRV_BLOCK_OFFSET_VALID | cluster_offset; } - if (ret =3D=3D QCOW2_CLUSTER_ZERO) { + if (ret =3D=3D QCOW2_CLUSTER_ZERO_PLAIN || ret =3D=3D QCOW2_CLUSTER_ZE= RO_ALLOC) { status |=3D BDRV_BLOCK_ZERO; } else if (ret !=3D QCOW2_CLUSTER_UNALLOCATED) { status |=3D BDRV_BLOCK_DATA; @@ -1482,7 +1482,8 @@ static coroutine_fn int qcow2_co_preadv(BlockDriverSt= ate *bs, uint64_t offset, } break; - case QCOW2_CLUSTER_ZERO: + case QCOW2_CLUSTER_ZERO_PLAIN: + case QCOW2_CLUSTER_ZERO_ALLOC: qemu_iovec_memset(&hd_qiov, 0, 0, cur_bytes); break; @@ -2491,7 +2492,9 @@ static coroutine_fn int qcow2_co_pwrite_zeroes(BlockD= riverState *bs, count =3D s->cluster_size; nr =3D s->cluster_size; ret =3D qcow2_get_cluster_offset(bs, offset, &nr, &off); - if (ret !=3D QCOW2_CLUSTER_UNALLOCATED && ret !=3D QCOW2_CLUSTER_Z= ERO) { + if (ret !=3D QCOW2_CLUSTER_UNALLOCATED && + ret !=3D QCOW2_CLUSTER_ZERO_PLAIN && + ret !=3D QCOW2_CLUSTER_ZERO_ALLOC) { qemu_co_mutex_unlock(&s->lock); return -ENOTSUP; } diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out index 5d40206..9e8f5b9 100644 --- a/tests/qemu-iotests/060.out +++ b/tests/qemu-iotests/060.out @@ -135,7 +135,7 @@ qemu-img: Error while amending options: Input/output er= ror Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D67108864 wrote 65536/65536 bytes at offset 0 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -qcow2: Marking image as corrupt: Data cluster offset 0x52a00 unaligned (L2= offset: 0x40000, L2 index: 0); further corruption events will be suppressed +qcow2: Marking image as corrupt: Cluster allocation offset 0x52a00 unalign= ed (L2 offset: 0x40000, L2 index: 0); further corruption events will be sup= pressed read failed: Input/output error =3D=3D=3D Testing unaligned pre-allocated zero cluster =3D=3D=3D @@ -166,7 +166,7 @@ discard 65536/65536 bytes at offset 0 Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D67108864 wrote 65536/65536 bytes at offset 0 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -qcow2: Image is corrupt: Data cluster offset 0x52a00 unaligned (L2 offset:= 0x40000, L2 index: 0); further non-fatal corruption events will be suppres= sed +qcow2: Image is corrupt: Cluster allocation offset 0x52a00 unaligned (L2 o= ffset: 0x40000, L2 index: 0); further non-fatal corruption events will be s= uppressed read failed: Input/output error read failed: Input/output error @@ -176,7 +176,7 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D671= 08864 wrote 131072/131072 bytes at offset 0 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) qcow2: Image is corrupt: Cannot free unaligned cluster 0x52a00; further no= n-fatal corruption events will be suppressed -qcow2: Marking image as corrupt: Data cluster offset 0x62a00 unaligned (L2= offset: 0x40000, L2 index: 0x1); further corruption events will be suppres= sed +qcow2: Marking image as corrupt: Cluster allocation offset 0x62a00 unalign= ed (L2 offset: 0x40000, L2 index: 0x1); further corruption events will be s= uppressed discard 65536/65536 bytes at offset 0 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) read failed: Input/output error --=20 2.9.3 From nobody Tue May 7 05:07:07 2024 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.zoho.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; Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1494115696979336.9071534401555; Sat, 6 May 2017 17:08:16 -0700 (PDT) Received: from localhost ([::1]:53091 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d79kF-0004NC-EE for importer@patchew.org; Sat, 06 May 2017 20:08:15 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45839) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d79iG-0002i0-6w for qemu-devel@nongnu.org; Sat, 06 May 2017 20:06:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1d79iD-0007ke-Rs for qemu-devel@nongnu.org; Sat, 06 May 2017 20:06:12 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53116) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1d79iB-0007jA-0H; Sat, 06 May 2017 20:06:07 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 047C7201FB; Sun, 7 May 2017 00:06:06 +0000 (UTC) Received: from red.redhat.com (ovpn-122-206.rdu2.redhat.com [10.10.122.206]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2813918345; Sun, 7 May 2017 00:06:05 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 047C7201FB Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=eblake@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 047C7201FB From: Eric Blake To: qemu-devel@nongnu.org Date: Sat, 6 May 2017 19:05:47 -0500 Message-Id: <20170507000552.20847-8-eblake@redhat.com> In-Reply-To: <20170507000552.20847-1-eblake@redhat.com> References: <20170507000552.20847-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Sun, 07 May 2017 00:06:06 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v13 07/12] qcow2: Optimize zero_single_l2() to minimize L2 churn 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: kwolf@redhat.com, qemu-block@nongnu.org, mreitz@redhat.com 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" Similar to discard_single_l2(), we should try to avoid dirtying the L2 cache when the cluster we are changing already has the right characteristics. Note that by the time we get to zero_single_l2(), BDRV_REQ_MAY_UNMAP is a requirement to unallocate a cluster (this is because the block layer clears that flag if discard.* flags during open requested that we never punch holes - see the conversation around commit 170f4b2e, https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg07306.html). Therefore, this patch can only reuse a zero cluster as-is if either unmapping is not requested, or if the zero cluster was not associated with an allocation. Technically, there are some cases where an unallocated cluster already reads as all zeroes (namely, when there is no backing file [easy: check bs->backing], or when the backing file also reads as zeroes [harder: we can't check bdrv_get_block_status since we are already holding the lock]), where the guest would not immediately see a difference if we left that cluster unallocated. But if the user did not request unmapping, leaving an unallocated cluster is wrong; and even if the user DID request unmapping, keeping a cluster unallocated risks a subtle semantic change of guest-visible contents if a backing file is later added, and it is not worth auditing whether all internal uses such as mirror properly avoid an unmap request. Thus, this patch is intentionally limited to just clusters that are already marked as zero. Signed-off-by: Eric Blake Reviewed-by: Max Reitz --- v13: use enum name v12: store cluster type in temporary v11: reserved for blkdebug half of v10 v10: new patch, replacing earlier attempt to use unallocated clusters, and ditching any optimization of v2 files --- block/qcow2-cluster.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index a4a3229..6c59708 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -1601,6 +1601,7 @@ static int zero_single_l2(BlockDriverState *bs, uint6= 4_t offset, int l2_index; int ret; int i; + bool unmap =3D !!(flags & BDRV_REQ_MAY_UNMAP); ret =3D get_cluster_table(bs, offset, &l2_table, &l2_index); if (ret < 0) { @@ -1613,12 +1614,22 @@ static int zero_single_l2(BlockDriverState *bs, uin= t64_t offset, for (i =3D 0; i < nb_clusters; i++) { uint64_t old_offset; + QCow2ClusterType cluster_type; old_offset =3D be64_to_cpu(l2_table[l2_index + i]); - /* Update L2 entries */ + /* + * Minimize L2 changes if the cluster already reads back as + * zeroes with correct allocation. + */ + cluster_type =3D qcow2_get_cluster_type(old_offset); + if (cluster_type =3D=3D QCOW2_CLUSTER_ZERO_PLAIN || + (cluster_type =3D=3D QCOW2_CLUSTER_ZERO_ALLOC && !unmap)) { + continue; + } + qcow2_cache_entry_mark_dirty(bs, s->l2_table_cache, l2_table); - if (old_offset & QCOW_OFLAG_COMPRESSED || flags & BDRV_REQ_MAY_UNM= AP) { + if (cluster_type =3D=3D QCOW2_CLUSTER_COMPRESSED || unmap) { l2_table[l2_index + i] =3D cpu_to_be64(QCOW_OFLAG_ZERO); qcow2_free_any_clusters(bs, old_offset, 1, QCOW2_DISCARD_REQUE= ST); } else { --=20 2.9.3 From nobody Tue May 7 05:07:07 2024 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.zoho.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; Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1494115848498558.6442148927308; Sat, 6 May 2017 17:10:48 -0700 (PDT) Received: from localhost ([::1]:53104 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d79mh-0006T5-20 for importer@patchew.org; Sat, 06 May 2017 20:10:47 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45888) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d79iI-0002kR-Cc for qemu-devel@nongnu.org; Sat, 06 May 2017 20:06:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1d79iG-0007la-BN for qemu-devel@nongnu.org; Sat, 06 May 2017 20:06:14 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46112) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1d79iB-0007jd-U5; Sat, 06 May 2017 20:06:08 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E4A4F4E326; Sun, 7 May 2017 00:06:06 +0000 (UTC) Received: from red.redhat.com (ovpn-122-206.rdu2.redhat.com [10.10.122.206]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3A77218345; Sun, 7 May 2017 00:06:06 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com E4A4F4E326 Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=eblake@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com E4A4F4E326 From: Eric Blake To: qemu-devel@nongnu.org Date: Sat, 6 May 2017 19:05:48 -0500 Message-Id: <20170507000552.20847-9-eblake@redhat.com> In-Reply-To: <20170507000552.20847-1-eblake@redhat.com> References: <20170507000552.20847-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Sun, 07 May 2017 00:06:07 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v13 08/12] iotests: Improve _filter_qemu_img_map 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: kwolf@redhat.com, qemu-block@nongnu.org, mreitz@redhat.com 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" Although _filter_qemu_img_map documents that it scrubs offsets, it was only doing so for human mode. Of the existing tests using the filter (97, 122, 150, 154, 176), two of them are affected, but it does not hurt the validity of the tests to not require particular mappings (another test, 66, uses offsets but intentionally does not pass through _filter_qemu_img_map, because it checks that offsets are unchanged before and after an operation). Another justification for this patch is that it will allow a future patch to utilize 'qemu-img map --output=3Djson' to check the status of preallocated zero clusters without regards to the mapping (since the qcow2 mapping can be very sensitive to the chosen cluster size, when preallocation is not in use). Signed-off-by: Eric Blake Reviewed-by: Max Reitz --- v13: kill TAB damage v12: new patch --- tests/qemu-iotests/common.filter | 4 +++- tests/qemu-iotests/122.out | 16 ++++++++-------- tests/qemu-iotests/154.out | 30 +++++++++++++++--------------- 3 files changed, 26 insertions(+), 24 deletions(-) diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.f= ilter index f58548d..2f595b2 100644 --- a/tests/qemu-iotests/common.filter +++ b/tests/qemu-iotests/common.filter @@ -152,10 +152,12 @@ _filter_img_info() -e "/log_size: [0-9]\\+/d" } -# filter out offsets and file names from qemu-img map +# filter out offsets and file names from qemu-img map; good for both +# human and json output _filter_qemu_img_map() { sed -e 's/\([0-9a-fx]* *[0-9a-fx]* *\)[0-9a-fx]* */\1/g' \ + -e 's/"offset": [0-9]\+/"offset": OFFSET/g' \ -e 's/Mapped to *//' | _filter_testdir | _filter_imgfmt } diff --git a/tests/qemu-iotests/122.out b/tests/qemu-iotests/122.out index 9317d80..47d8656 100644 --- a/tests/qemu-iotests/122.out +++ b/tests/qemu-iotests/122.out @@ -112,7 +112,7 @@ read 3145728/3145728 bytes at offset 0 3 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) read 63963136/63963136 bytes at offset 3145728 61 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -[{ "start": 0, "length": 67108864, "depth": 0, "zero": false, "data": true= , "offset": 327680}] +[{ "start": 0, "length": 67108864, "depth": 0, "zero": false, "data": true= , "offset": OFFSET}] convert -c -S 0: read 3145728/3145728 bytes at offset 0 @@ -134,7 +134,7 @@ read 30408704/30408704 bytes at offset 3145728 29 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) read 33554432/33554432 bytes at offset 33554432 32 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -[{ "start": 0, "length": 67108864, "depth": 0, "zero": false, "data": true= , "offset": 327680}] +[{ "start": 0, "length": 67108864, "depth": 0, "zero": false, "data": true= , "offset": OFFSET}] convert -c -S 0 with source backing file: read 3145728/3145728 bytes at offset 0 @@ -152,7 +152,7 @@ read 30408704/30408704 bytes at offset 3145728 29 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) read 33554432/33554432 bytes at offset 33554432 32 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -[{ "start": 0, "length": 67108864, "depth": 0, "zero": false, "data": true= , "offset": 327680}] +[{ "start": 0, "length": 67108864, "depth": 0, "zero": false, "data": true= , "offset": OFFSET}] convert -c -S 0 -B ... read 3145728/3145728 bytes at offset 0 @@ -176,11 +176,11 @@ wrote 1024/1024 bytes at offset 17408 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) convert -S 4k -[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true, "o= ffset": 8192}, +[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true, "o= ffset": OFFSET}, { "start": 1024, "length": 7168, "depth": 0, "zero": true, "data": false}, -{ "start": 8192, "length": 1024, "depth": 0, "zero": false, "data": true, = "offset": 9216}, +{ "start": 8192, "length": 1024, "depth": 0, "zero": false, "data": true, = "offset": OFFSET}, { "start": 9216, "length": 8192, "depth": 0, "zero": true, "data": false}, -{ "start": 17408, "length": 1024, "depth": 0, "zero": false, "data": true,= "offset": 10240}, +{ "start": 17408, "length": 1024, "depth": 0, "zero": false, "data": true,= "offset": OFFSET}, { "start": 18432, "length": 67090432, "depth": 0, "zero": true, "data": fa= lse}] convert -c -S 4k @@ -192,9 +192,9 @@ convert -c -S 4k { "start": 18432, "length": 67090432, "depth": 0, "zero": true, "data": fa= lse}] convert -S 8k -[{ "start": 0, "length": 9216, "depth": 0, "zero": false, "data": true, "o= ffset": 8192}, +[{ "start": 0, "length": 9216, "depth": 0, "zero": false, "data": true, "o= ffset": OFFSET}, { "start": 9216, "length": 8192, "depth": 0, "zero": true, "data": false}, -{ "start": 17408, "length": 1024, "depth": 0, "zero": false, "data": true,= "offset": 17408}, +{ "start": 17408, "length": 1024, "depth": 0, "zero": false, "data": true,= "offset": OFFSET}, { "start": 18432, "length": 67090432, "depth": 0, "zero": true, "data": fa= lse}] convert -c -S 8k diff --git a/tests/qemu-iotests/154.out b/tests/qemu-iotests/154.out index da9eabd..d3b68e7 100644 --- a/tests/qemu-iotests/154.out +++ b/tests/qemu-iotests/154.out @@ -42,9 +42,9 @@ read 1024/1024 bytes at offset 65536 read 2048/2048 bytes at offset 67584 2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) [{ "start": 0, "length": 32768, "depth": 1, "zero": true, "data": false}, -{ "start": 32768, "length": 4096, "depth": 0, "zero": false, "data": true,= "offset": 20480}, +{ "start": 32768, "length": 4096, "depth": 0, "zero": false, "data": true,= "offset": OFFSET}, { "start": 36864, "length": 28672, "depth": 1, "zero": true, "data": false= }, -{ "start": 65536, "length": 4096, "depth": 0, "zero": false, "data": true,= "offset": 24576}, +{ "start": 65536, "length": 4096, "depth": 0, "zero": false, "data": true,= "offset": OFFSET}, { "start": 69632, "length": 134148096, "depth": 1, "zero": true, "data": f= alse}] =3D=3D backing file contains non-zero data after write_zeroes =3D=3D @@ -69,9 +69,9 @@ read 1024/1024 bytes at offset 44032 read 3072/3072 bytes at offset 40960 3 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) [{ "start": 0, "length": 32768, "depth": 1, "zero": true, "data": false}, -{ "start": 32768, "length": 4096, "depth": 0, "zero": false, "data": true,= "offset": 20480}, +{ "start": 32768, "length": 4096, "depth": 0, "zero": false, "data": true,= "offset": OFFSET}, { "start": 36864, "length": 4096, "depth": 1, "zero": true, "data": false}, -{ "start": 40960, "length": 4096, "depth": 0, "zero": false, "data": true,= "offset": 24576}, +{ "start": 40960, "length": 4096, "depth": 0, "zero": false, "data": true,= "offset": OFFSET}, { "start": 45056, "length": 134172672, "depth": 1, "zero": true, "data": f= alse}] =3D=3D write_zeroes covers non-zero data =3D=3D @@ -143,13 +143,13 @@ read 1024/1024 bytes at offset 67584 read 5120/5120 bytes at offset 68608 5 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) [{ "start": 0, "length": 32768, "depth": 1, "zero": true, "data": false}, -{ "start": 32768, "length": 4096, "depth": 0, "zero": false, "data": true,= "offset": 20480}, +{ "start": 32768, "length": 4096, "depth": 0, "zero": false, "data": true,= "offset": OFFSET}, { "start": 36864, "length": 4096, "depth": 0, "zero": true, "data": false}, { "start": 40960, "length": 8192, "depth": 1, "zero": true, "data": false}, -{ "start": 49152, "length": 4096, "depth": 0, "zero": false, "data": true,= "offset": 24576}, +{ "start": 49152, "length": 4096, "depth": 0, "zero": false, "data": true,= "offset": OFFSET}, { "start": 53248, "length": 4096, "depth": 0, "zero": true, "data": false}, { "start": 57344, "length": 8192, "depth": 1, "zero": true, "data": false}, -{ "start": 65536, "length": 4096, "depth": 0, "zero": false, "data": true,= "offset": 28672}, +{ "start": 65536, "length": 4096, "depth": 0, "zero": false, "data": true,= "offset": OFFSET}, { "start": 69632, "length": 4096, "depth": 0, "zero": true, "data": false}, { "start": 73728, "length": 134144000, "depth": 1, "zero": true, "data": f= alse}] @@ -186,13 +186,13 @@ read 1024/1024 bytes at offset 72704 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) [{ "start": 0, "length": 32768, "depth": 1, "zero": true, "data": false}, { "start": 32768, "length": 4096, "depth": 0, "zero": true, "data": false}, -{ "start": 36864, "length": 4096, "depth": 0, "zero": false, "data": true,= "offset": 20480}, +{ "start": 36864, "length": 4096, "depth": 0, "zero": false, "data": true,= "offset": OFFSET}, { "start": 40960, "length": 8192, "depth": 1, "zero": true, "data": false}, { "start": 49152, "length": 4096, "depth": 0, "zero": true, "data": false}, -{ "start": 53248, "length": 4096, "depth": 0, "zero": false, "data": true,= "offset": 24576}, +{ "start": 53248, "length": 4096, "depth": 0, "zero": false, "data": true,= "offset": OFFSET}, { "start": 57344, "length": 8192, "depth": 1, "zero": true, "data": false}, { "start": 65536, "length": 4096, "depth": 0, "zero": true, "data": false}, -{ "start": 69632, "length": 4096, "depth": 0, "zero": false, "data": true,= "offset": 28672}, +{ "start": 69632, "length": 4096, "depth": 0, "zero": false, "data": true,= "offset": OFFSET}, { "start": 73728, "length": 134144000, "depth": 1, "zero": true, "data": f= alse}] =3D=3D spanning two clusters, partially overwriting backing file =3D=3D @@ -212,7 +212,7 @@ read 1024/1024 bytes at offset 5120 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) read 2048/2048 bytes at offset 6144 2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -[{ "start": 0, "length": 8192, "depth": 0, "zero": false, "data": true, "o= ffset": 20480}, +[{ "start": 0, "length": 8192, "depth": 0, "zero": false, "data": true, "o= ffset": OFFSET}, { "start": 8192, "length": 134209536, "depth": 1, "zero": true, "data": fa= lse}] =3D=3D spanning multiple clusters, non-zero in first cluster =3D=3D @@ -227,7 +227,7 @@ read 2048/2048 bytes at offset 65536 read 10240/10240 bytes at offset 67584 10 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) [{ "start": 0, "length": 65536, "depth": 1, "zero": true, "data": false}, -{ "start": 65536, "length": 4096, "depth": 0, "zero": false, "data": true,= "offset": 20480}, +{ "start": 65536, "length": 4096, "depth": 0, "zero": false, "data": true,= "offset": OFFSET}, { "start": 69632, "length": 8192, "depth": 0, "zero": true, "data": false}, { "start": 77824, "length": 134139904, "depth": 1, "zero": true, "data": f= alse}] @@ -257,7 +257,7 @@ read 2048/2048 bytes at offset 75776 2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) [{ "start": 0, "length": 65536, "depth": 1, "zero": true, "data": false}, { "start": 65536, "length": 8192, "depth": 0, "zero": true, "data": false}, -{ "start": 73728, "length": 4096, "depth": 0, "zero": false, "data": true,= "offset": 20480}, +{ "start": 73728, "length": 4096, "depth": 0, "zero": false, "data": true,= "offset": OFFSET}, { "start": 77824, "length": 134139904, "depth": 1, "zero": true, "data": f= alse}] =3D=3D spanning multiple clusters, partially overwriting backing file =3D= =3D @@ -278,8 +278,8 @@ read 2048/2048 bytes at offset 74752 read 1024/1024 bytes at offset 76800 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) [{ "start": 0, "length": 65536, "depth": 1, "zero": true, "data": false}, -{ "start": 65536, "length": 4096, "depth": 0, "zero": false, "data": true,= "offset": 20480}, +{ "start": 65536, "length": 4096, "depth": 0, "zero": false, "data": true,= "offset": OFFSET}, { "start": 69632, "length": 4096, "depth": 0, "zero": true, "data": false}, -{ "start": 73728, "length": 4096, "depth": 0, "zero": false, "data": true,= "offset": 24576}, +{ "start": 73728, "length": 4096, "depth": 0, "zero": false, "data": true,= "offset": OFFSET}, { "start": 77824, "length": 134139904, "depth": 1, "zero": true, "data": f= alse}] *** done --=20 2.9.3 From nobody Tue May 7 05:07:07 2024 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.zoho.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; Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1494116155372857.5648915668814; Sat, 6 May 2017 17:15:55 -0700 (PDT) Received: from localhost ([::1]:53136 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d79rd-0002JU-Hj for importer@patchew.org; Sat, 06 May 2017 20:15:53 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45936) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d79iO-0002ob-3B for qemu-devel@nongnu.org; Sat, 06 May 2017 20:06:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1d79iL-0007mx-Bi for qemu-devel@nongnu.org; Sat, 06 May 2017 20:06:20 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60002) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1d79iC-0007kB-U4; Sat, 06 May 2017 20:06:09 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E12073D940; Sun, 7 May 2017 00:06:07 +0000 (UTC) Received: from red.redhat.com (ovpn-122-206.rdu2.redhat.com [10.10.122.206]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2996C18345; Sun, 7 May 2017 00:06:07 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com E12073D940 Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=eblake@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com E12073D940 From: Eric Blake To: qemu-devel@nongnu.org Date: Sat, 6 May 2017 19:05:49 -0500 Message-Id: <20170507000552.20847-10-eblake@redhat.com> In-Reply-To: <20170507000552.20847-1-eblake@redhat.com> References: <20170507000552.20847-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Sun, 07 May 2017 00:06:08 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v13 09/12] iotests: Add test 179 to cover write zeroes with unmap 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: kwolf@redhat.com, qemu-block@nongnu.org, mreitz@redhat.com 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" No tests were covering write zeroes with unmap. Additionally, I needed to prove that my previous patches for correct status reporting and write zeroes optimizations actually had an impact. The test works for cluster_size between 8k and 2M (for smaller sizes, it fails because our allocation patterns are not contiguous with small clusters - in part, the largest consecutive allocation we tend to get is often bounded by the size covered by one L2 table). Note that testing for zero clusters is tricky: 'qemu-io map' reports whether data comes from the current layer of the image (useful for sniffing out which regions of the file have QCOW_OFLAG_ZERO) - but doesn't show which clusters have mappings; while 'qemu-img map' sees "zero":true for both unallocated and zero clusters for any qcow2 with no backing layer (so less useful at detecting true zero clusters), but reliably shows mappings. So we have to rely on both queries side-by-side at each point of the test. Signed-off-by: Eric Blake Reviewed-by: Max Reitz --- v13: drop final failing test of blkdebug v12: probe the map in more places, to make test easier to follow v11: reserved for blkdebug half of v10 v10: drop any changes to v2 files, rewrite test to work with updates earlier in the series, add a blkdebug probe v9: new patch --- tests/qemu-iotests/179 | 130 +++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/179.out | 156 +++++++++++++++++++++++++++++++++++++++++= ++++ tests/qemu-iotests/group | 1 + 3 files changed, 287 insertions(+) create mode 100755 tests/qemu-iotests/179 create mode 100644 tests/qemu-iotests/179.out diff --git a/tests/qemu-iotests/179 b/tests/qemu-iotests/179 new file mode 100755 index 0000000..7bc8db8 --- /dev/null +++ b/tests/qemu-iotests/179 @@ -0,0 +1,130 @@ +#!/bin/bash +# +# Test case for write zeroes with unmap +# +# Copyright (C) 2017 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +# creator +owner=3Deblake@redhat.com + +seq=3D"$(basename $0)" +echo "QA output created by $seq" + +here=3D"$PWD" +status=3D1 # failure is the default! + +_cleanup() +{ + _cleanup_test_img +} +trap "_cleanup; exit \$status" 0 1 2 3 15 + +# get standard environment, filters and checks +. ./common.rc +. ./common.filter + +_supported_fmt qcow2 +_supported_proto file +_supported_os Linux + +# v2 images can't mark clusters as zero +_unsupported_imgopts compat=3D0.10 + +echo +echo '=3D=3D=3D Testing write zeroes with unmap =3D=3D=3D' +echo + +TEST_IMG=3D"$TEST_IMG.base" _make_test_img 64M +_make_test_img -b "$TEST_IMG.base" + +# Offsets chosen at or near 2M boundaries so test works at all cluster siz= es +# 8k and larger (smaller clusters fail due to non-contiguous allocations) + +# Aligned writes to unallocated cluster should not allocate mapping, but m= ust +# mark cluster as zero, whether or not unmap was requested +$QEMU_IO -c "write -z -u 2M 2M" "$TEST_IMG.base" | _filter_qemu_io +$QEMU_IO -c "write -z 6M 2M" "$TEST_IMG.base" | _filter_qemu_io +$QEMU_IO -c "map" "$TEST_IMG.base" | _filter_qemu_io +$QEMU_IMG map --output=3Djson "$TEST_IMG.base" | _filter_qemu_img_map + +# Unaligned writes need not allocate mapping if the cluster already reads +# as zero, but must mark cluster as zero, whether or not unmap was request= ed +$QEMU_IO -c "write -z -u 10485761 2097150" "$TEST_IMG.base" | _filter_qemu= _io +$QEMU_IO -c "write -z 14680065 2097150" "$TEST_IMG.base" | _filter_qemu_io +$QEMU_IO -c "map" "$TEST_IMG.base" | _filter_qemu_io +$QEMU_IMG map --output=3Djson "$TEST_IMG.base" | _filter_qemu_img_map + +# Requesting unmap of normal data must deallocate; omitting unmap should +# preserve the mapping +$QEMU_IO -c "write 18M 14M" "$TEST_IMG.base" | _filter_qemu_io +$QEMU_IO -c "write -z -u 20M 2M" "$TEST_IMG.base" | _filter_qemu_io +$QEMU_IO -c "write -z 24M 6M" "$TEST_IMG.base" | _filter_qemu_io +$QEMU_IO -c "map" "$TEST_IMG.base" | _filter_qemu_io +$QEMU_IMG map --output=3Djson "$TEST_IMG.base" | _filter_qemu_img_map + +# Likewise when writing on already-mapped zero data +$QEMU_IO -c "write -z -u 26M 2M" "$TEST_IMG.base" | _filter_qemu_io +$QEMU_IO -c "write -z 28M 2M" "$TEST_IMG.base" | _filter_qemu_io +$QEMU_IO -c "map" "$TEST_IMG.base" | _filter_qemu_io +$QEMU_IMG map --output=3Djson "$TEST_IMG.base" | _filter_qemu_img_map + +# Writing on unmapped zeroes does not allocate +$QEMU_IO -c "write -z 32M 8M" "$TEST_IMG.base" | _filter_qemu_io +$QEMU_IO -c "write -z -u 34M 2M" "$TEST_IMG.base" | _filter_qemu_io +$QEMU_IO -c "write -z 36M 2M" "$TEST_IMG.base" | _filter_qemu_io +$QEMU_IO -c "map" "$TEST_IMG.base" | _filter_qemu_io +$QEMU_IMG map --output=3Djson "$TEST_IMG.base" | _filter_qemu_img_map + +# Writing zero overrides a backing file, regardless of backing cluster type +$QEMU_IO -c "write -z 40M 8M" "$TEST_IMG.base" | _filter_qemu_io +$QEMU_IO -c "write 48M 8M" "$TEST_IMG.base" | _filter_qemu_io +$QEMU_IO -c "write -z -u 42M 2M" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "write -z 44M 2M" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "write -z -u 50M 2M" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "write -z 52M 2M" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "write -z -u 58M 2M" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "write -z 60M 2M" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "map" "$TEST_IMG" | _filter_qemu_io +$QEMU_IMG map --output=3Djson "$TEST_IMG" | _filter_qemu_img_map + +# Final check that mappings are correct and images are still sane +TEST_IMG=3D"$TEST_IMG.base" _check_test_img +_check_test_img + +echo +echo '=3D=3D=3D Testing cache optimization =3D=3D=3D' +echo + +BLKDBG_TEST_IMG=3D"blkdebug:$TEST_DIR/blkdebug.conf:$TEST_IMG.base" + +cat > "$TEST_DIR/blkdebug.conf" < Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1494116110186217.82436338408047; Sat, 6 May 2017 17:15:10 -0700 (PDT) Received: from localhost ([::1]:53129 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d79qt-0001hV-Ra for importer@patchew.org; Sat, 06 May 2017 20:15:07 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45938) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d79iO-0002oh-4r for qemu-devel@nongnu.org; Sat, 06 May 2017 20:06:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1d79iL-0007n7-D9 for qemu-devel@nongnu.org; Sat, 06 May 2017 20:06:20 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60018) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1d79iD-0007kY-St; Sat, 06 May 2017 20:06:10 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E6CA03D944; Sun, 7 May 2017 00:06:08 +0000 (UTC) Received: from red.redhat.com (ovpn-122-206.rdu2.redhat.com [10.10.122.206]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2068F18345; Sun, 7 May 2017 00:06:08 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com E6CA03D944 Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=eblake@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com E6CA03D944 From: Eric Blake To: qemu-devel@nongnu.org Date: Sat, 6 May 2017 19:05:50 -0500 Message-Id: <20170507000552.20847-11-eblake@redhat.com> In-Reply-To: <20170507000552.20847-1-eblake@redhat.com> References: <20170507000552.20847-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Sun, 07 May 2017 00:06:09 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v13 10/12] qcow2: Optimize write zero of unaligned tail cluster 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: kwolf@redhat.com, qemu-block@nongnu.org, mreitz@redhat.com 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" We've already improved discards to operate efficiently on the tail of an unaligned qcow2 image; it's time to make a similar improvement to write zeroes. The special case is only valid at the tail cluster of a file, where we must recognize that any sectors beyond the image end would implicitly read as zero, and therefore should not penalize our logic for widening a partial cluster into writing the whole cluster as zero. However, note that for now, the special case of end-of-file is only recognized if there is no backing file, or if the backing file has the same length; that's because when the backing file is shorter than the active layer, we don't have code in place to recognize that reads of a sector unallocated at the top and beyond the backing end-of-file are implicitly zero. It's not much of a real loss, because most people don't use images that aren't cluster-aligned, or where the active layer is a different size than the backing layer (especially where the difference falls within a single cluster). Update test 154 to cover the new scenarios, using two images of intentionally differing length. While at it, fix the test to gracefully skip when run as ./check -qcow2 -o compat=3D0.10 154 since the older format lacks zero clusters already required earlier in the test. Signed-off-by: Eric Blake Reviewed-by: Max Reitz --- v13: fix typo that left pattern verification failure in test v12: fix testsuite problems, document shortcoming of differing v11: reserved for blkdebug half of v10 size of backing file v10: rebase to better reporting of preallocated zero clusters v9: new patch --- block/qcow2.c | 7 ++ tests/qemu-iotests/154 | 160 +++++++++++++++++++++++++++++++++++++++++= +++- tests/qemu-iotests/154.out | 128 ++++++++++++++++++++++++++++++++++++ 3 files changed, 293 insertions(+), 2 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index dded5a0..3478bb6 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -2451,6 +2451,10 @@ static bool is_zero_sectors(BlockDriverState *bs, in= t64_t start, BlockDriverState *file; int64_t res; + if (start + count > bs->total_sectors) { + count =3D bs->total_sectors - start; + } + if (!count) { return true; } @@ -2469,6 +2473,9 @@ static coroutine_fn int qcow2_co_pwrite_zeroes(BlockD= riverState *bs, uint32_t tail =3D (offset + count) % s->cluster_size; trace_qcow2_pwrite_zeroes_start_req(qemu_coroutine_self(), offset, cou= nt); + if (offset + count =3D=3D bs->total_sectors * BDRV_SECTOR_SIZE) { + tail =3D 0; + } if (head || tail) { int64_t cl_start =3D (offset - head) >> BDRV_SECTOR_BITS; diff --git a/tests/qemu-iotests/154 b/tests/qemu-iotests/154 index 7ca7219..dd8a426 100755 --- a/tests/qemu-iotests/154 +++ b/tests/qemu-iotests/154 @@ -2,7 +2,7 @@ # # qcow2 specific bdrv_pwrite_zeroes tests with backing files (complements = 034) # -# Copyright (C) 2016 Red Hat, Inc. +# Copyright (C) 2016-2017 Red Hat, Inc. # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -42,7 +42,10 @@ _supported_proto file _supported_os Linux CLUSTER_SIZE=3D4k -size=3D128M +size=3D$((128 * 1024 * 1024)) + +# This test requires zero clusters, added in v3 images +_unsupported_imgopts compat=3D0.10 echo echo =3D=3D backing file contains zeros =3D=3D @@ -299,6 +302,159 @@ $QEMU_IO -c "read -P 0 75k 1k" "$TEST_IMG" | _filter_= qemu_io $QEMU_IMG map --output=3Djson "$TEST_IMG" | _filter_qemu_img_map +echo +echo =3D=3D unaligned image tail cluster, no allocation needed =3D=3D + +# With no backing file, write to all or part of unallocated partial cluster +# will mark the cluster as zero, but does not allocate. +# Re-create the image each time to get back to unallocated clusters. + +# Write at the front: sector-wise, the request is: 128m... | 00 -- -- -- +_make_test_img $((size + 2048)) +$QEMU_IO -c "write -z $size 512" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "alloc $size 2048" "$TEST_IMG" | _filter_qemu_io +$QEMU_IMG map --output=3Djson "$TEST_IMG" | _filter_qemu_img_map + +# Write at the back: sector-wise, the request is: 128m... | -- -- -- 00 +_make_test_img $((size + 2048)) +$QEMU_IO -c "write -z $((size + 1536)) 512" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "alloc $size 2048" "$TEST_IMG" | _filter_qemu_io +$QEMU_IMG map --output=3Djson "$TEST_IMG" | _filter_qemu_img_map + +# Write at middle: sector-wise, the request is: 128m... | -- 00 00 -- +_make_test_img $((size + 2048)) +$QEMU_IO -c "write -z $((size + 512)) 1024" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "alloc $size 2048" "$TEST_IMG" | _filter_qemu_io +$QEMU_IMG map --output=3Djson "$TEST_IMG" | _filter_qemu_img_map + +# Write entire cluster: sector-wise, the request is: 128m... | 00 00 00 00 +_make_test_img $((size + 2048)) +$QEMU_IO -c "write -z $size 2048" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "alloc $size 2048" "$TEST_IMG" | _filter_qemu_io +$QEMU_IMG map --output=3Djson "$TEST_IMG" | _filter_qemu_img_map + +# Repeat with backing file holding unallocated cluster. +# TODO: Note that this forces an allocation, because we aren't yet able to +# quickly detect that reads beyond EOF of the backing file are always zero +CLUSTER_SIZE=3D2048 TEST_IMG=3D"$TEST_IMG.base" _make_test_img $((size + 1= 024)) + +# Write at the front: sector-wise, the request is: +# backing: 128m... | -- -- +# active: 128m... | 00 -- -- -- +_make_test_img -b "$TEST_IMG.base" $((size + 2048)) +$QEMU_IO -c "write -z $size 512" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "alloc $size 2048" "$TEST_IMG" | _filter_qemu_io +$QEMU_IMG map --output=3Djson "$TEST_IMG" | _filter_qemu_img_map + +# Write at the back: sector-wise, the request is: +# backing: 128m... | -- -- +# active: 128m... | -- -- -- 00 +_make_test_img -b "$TEST_IMG.base" $((size + 2048)) +$QEMU_IO -c "write -z $((size + 1536)) 512" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "alloc $size 2048" "$TEST_IMG" | _filter_qemu_io +$QEMU_IMG map --output=3Djson "$TEST_IMG" | _filter_qemu_img_map + +# Write at middle: sector-wise, the request is: +# backing: 128m... | -- -- +# active: 128m... | -- 00 00 -- +_make_test_img -b "$TEST_IMG.base" $((size + 2048)) +$QEMU_IO -c "write -z $((size + 512)) 1024" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "alloc $size 2048" "$TEST_IMG" | _filter_qemu_io +$QEMU_IMG map --output=3Djson "$TEST_IMG" | _filter_qemu_img_map + +# Write entire cluster: sector-wise, the request is: +# backing: 128m... | -- -- +# active: 128m... | 00 00 00 00 +_make_test_img -b "$TEST_IMG.base" $((size + 2048)) +$QEMU_IO -c "write -z $size 2048" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "alloc $size 2048" "$TEST_IMG" | _filter_qemu_io +$QEMU_IMG map --output=3Djson "$TEST_IMG" | _filter_qemu_img_map + +# Repeat with backing file holding zero'd cluster +# TODO: Note that this forces an allocation, because we aren't yet able to +# quickly detect that reads beyond EOF of the backing file are always zero +$QEMU_IO -c "write -z $size 512" "$TEST_IMG.base" | _filter_qemu_io + +# Write at the front: sector-wise, the request is: +# backing: 128m... | 00 00 +# active: 128m... | 00 -- -- -- +_make_test_img -b "$TEST_IMG.base" $((size + 2048)) +$QEMU_IO -c "write -z $size 512" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "alloc $size 2048" "$TEST_IMG" | _filter_qemu_io +$QEMU_IMG map --output=3Djson "$TEST_IMG" | _filter_qemu_img_map + +# Write at the back: sector-wise, the request is: +# backing: 128m... | 00 00 +# active: 128m... | -- -- -- 00 +_make_test_img -b "$TEST_IMG.base" $((size + 2048)) +$QEMU_IO -c "write -z $((size + 1536)) 512" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "alloc $size 2048" "$TEST_IMG" | _filter_qemu_io +$QEMU_IMG map --output=3Djson "$TEST_IMG" | _filter_qemu_img_map + +# Write at middle: sector-wise, the request is: +# backing: 128m... | 00 00 +# active: 128m... | -- 00 00 -- +_make_test_img -b "$TEST_IMG.base" $((size + 2048)) +$QEMU_IO -c "write -z $((size + 512)) 1024" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "alloc $size 2048" "$TEST_IMG" | _filter_qemu_io +$QEMU_IMG map --output=3Djson "$TEST_IMG" | _filter_qemu_img_map + +# Write entire cluster: sector-wise, the request is: +# backing: 128m... | 00 00 +# active: 128m... | 00 00 00 00 +_make_test_img -b "$TEST_IMG.base" $((size + 2048)) +$QEMU_IO -c "write -z $size 2048" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "alloc $size 2048" "$TEST_IMG" | _filter_qemu_io +$QEMU_IMG map --output=3Djson "$TEST_IMG" | _filter_qemu_img_map + +# A preallocated cluster maintains its allocation, whether it stays as +# data due to a partial write: +# Convert 128m... | XX XX =3D> ... | XX 00 +_make_test_img $((size + 1024)) +$QEMU_IO -c "write -P 1 $((size)) 1024" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "write -z $((size + 512)) 512" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "read -P 1 $((size)) 512" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "read -P 0 $((size + 512)) 512" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "alloc $size 1024" "$TEST_IMG" | _filter_qemu_io +$QEMU_IMG map --output=3Djson "$TEST_IMG" | _filter_qemu_img_map + +# or because it is the entire cluster and can use the zero flag: +# Convert 128m... | XX XX =3D> ... | 00 00 +$QEMU_IO -c "write -z $((size)) 1024" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "alloc $size 1024" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "read -P 0 $size 1024" "$TEST_IMG" | _filter_qemu_io +$QEMU_IMG map --output=3Djson "$TEST_IMG" | _filter_qemu_img_map + +echo +echo =3D=3D unaligned image tail cluster, allocation required =3D=3D + +# Write beyond backing file must COW +# Backing file: 128m... | XX -- +# Active layer: 128m... | -- -- 00 -- +CLUSTER_SIZE=3D512 TEST_IMG=3D"$TEST_IMG.base" _make_test_img $((size + 10= 24)) +_make_test_img -b "$TEST_IMG.base" $((size + 2048)) +$QEMU_IO -c "write -P 1 $((size)) 512" "$TEST_IMG.base" | _filter_qemu_io +$QEMU_IO -c "write -z $((size + 1024)) 512" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "read -P 1 $((size)) 512" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "read -P 0 $((size + 512)) 1536" "$TEST_IMG" | _filter_qemu_io +$QEMU_IMG map --output=3Djson "$TEST_IMG" | _filter_qemu_img_map + +# Writes at boundaries of (partial) cluster must not lose mid-cluster data +# Backing file: 128m: ... | -- XX +# Active layer: 128m: ... | 00 -- -- 00 +CLUSTER_SIZE=3D512 TEST_IMG=3D"$TEST_IMG.base" _make_test_img $((size + 10= 24)) +_make_test_img -b "$TEST_IMG.base" $((size + 2048)) +$QEMU_IO -c "write -P 1 $((size + 512)) 512" "$TEST_IMG.base" | _filter_qe= mu_io +$QEMU_IO -c "write -z $((size)) 512" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "read -P 0 $((size)) 512" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "read -P 1 $((size + 512)) 512" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "read -P 0 $((size + 1024)) 1024" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "write -z $((size + 1536)) 512" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "read -P 0 $((size)) 512" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "read -P 1 $((size + 512)) 512" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "read -P 0 $((size + 1024)) 1024" "$TEST_IMG" | _filter_qemu_io +$QEMU_IMG map --output=3Djson "$TEST_IMG" | _filter_qemu_img_map + # success, all done echo "*** done" rm -f $seq.full diff --git a/tests/qemu-iotests/154.out b/tests/qemu-iotests/154.out index d3b68e7..d8485ee 100644 --- a/tests/qemu-iotests/154.out +++ b/tests/qemu-iotests/154.out @@ -282,4 +282,132 @@ read 1024/1024 bytes at offset 76800 { "start": 69632, "length": 4096, "depth": 0, "zero": true, "data": false}, { "start": 73728, "length": 4096, "depth": 0, "zero": false, "data": true,= "offset": OFFSET}, { "start": 77824, "length": 134139904, "depth": 1, "zero": true, "data": f= alse}] + +=3D=3D unaligned image tail cluster, no allocation needed =3D=3D +Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D134219776 +wrote 512/512 bytes at offset 134217728 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +2048/2048 bytes allocated at offset 128 MiB +[{ "start": 0, "length": 134219776, "depth": 0, "zero": true, "data": fals= e}] +Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D134219776 +wrote 512/512 bytes at offset 134219264 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +2048/2048 bytes allocated at offset 128 MiB +[{ "start": 0, "length": 134219776, "depth": 0, "zero": true, "data": fals= e}] +Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D134219776 +wrote 1024/1024 bytes at offset 134218240 +1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +2048/2048 bytes allocated at offset 128 MiB +[{ "start": 0, "length": 134219776, "depth": 0, "zero": true, "data": fals= e}] +Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D134219776 +wrote 2048/2048 bytes at offset 134217728 +2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +2048/2048 bytes allocated at offset 128 MiB +[{ "start": 0, "length": 134219776, "depth": 0, "zero": true, "data": fals= e}] +Formatting 'TEST_DIR/t.IMGFMT.base', fmt=3DIMGFMT size=3D134218752 +Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D134219776 backing_file= =3DTEST_DIR/t.IMGFMT.base +wrote 512/512 bytes at offset 134217728 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +2048/2048 bytes allocated at offset 128 MiB +[{ "start": 0, "length": 134217728, "depth": 1, "zero": true, "data": fals= e}, +{ "start": 134217728, "length": 2048, "depth": 0, "zero": false, "data": t= rue, "offset": OFFSET}] +Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D134219776 backing_file= =3DTEST_DIR/t.IMGFMT.base +wrote 512/512 bytes at offset 134219264 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +2048/2048 bytes allocated at offset 128 MiB +[{ "start": 0, "length": 134217728, "depth": 1, "zero": true, "data": fals= e}, +{ "start": 134217728, "length": 2048, "depth": 0, "zero": false, "data": t= rue, "offset": OFFSET}] +Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D134219776 backing_file= =3DTEST_DIR/t.IMGFMT.base +wrote 1024/1024 bytes at offset 134218240 +1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +2048/2048 bytes allocated at offset 128 MiB +[{ "start": 0, "length": 134217728, "depth": 1, "zero": true, "data": fals= e}, +{ "start": 134217728, "length": 2048, "depth": 0, "zero": false, "data": t= rue, "offset": OFFSET}] +Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D134219776 backing_file= =3DTEST_DIR/t.IMGFMT.base +wrote 2048/2048 bytes at offset 134217728 +2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +2048/2048 bytes allocated at offset 128 MiB +[{ "start": 0, "length": 134217728, "depth": 1, "zero": true, "data": fals= e}, +{ "start": 134217728, "length": 2048, "depth": 0, "zero": true, "data": fa= lse}] +wrote 512/512 bytes at offset 134217728 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D134219776 backing_file= =3DTEST_DIR/t.IMGFMT.base +wrote 512/512 bytes at offset 134217728 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +2048/2048 bytes allocated at offset 128 MiB +[{ "start": 0, "length": 134217728, "depth": 1, "zero": true, "data": fals= e}, +{ "start": 134217728, "length": 2048, "depth": 0, "zero": false, "data": t= rue, "offset": OFFSET}] +Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D134219776 backing_file= =3DTEST_DIR/t.IMGFMT.base +wrote 512/512 bytes at offset 134219264 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +2048/2048 bytes allocated at offset 128 MiB +[{ "start": 0, "length": 134217728, "depth": 1, "zero": true, "data": fals= e}, +{ "start": 134217728, "length": 2048, "depth": 0, "zero": false, "data": t= rue, "offset": OFFSET}] +Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D134219776 backing_file= =3DTEST_DIR/t.IMGFMT.base +wrote 1024/1024 bytes at offset 134218240 +1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +2048/2048 bytes allocated at offset 128 MiB +[{ "start": 0, "length": 134217728, "depth": 1, "zero": true, "data": fals= e}, +{ "start": 134217728, "length": 2048, "depth": 0, "zero": false, "data": t= rue, "offset": OFFSET}] +Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D134219776 backing_file= =3DTEST_DIR/t.IMGFMT.base +wrote 2048/2048 bytes at offset 134217728 +2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +2048/2048 bytes allocated at offset 128 MiB +[{ "start": 0, "length": 134217728, "depth": 1, "zero": true, "data": fals= e}, +{ "start": 134217728, "length": 2048, "depth": 0, "zero": true, "data": fa= lse}] +Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D134218752 +wrote 1024/1024 bytes at offset 134217728 +1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +wrote 512/512 bytes at offset 134218240 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 512/512 bytes at offset 134217728 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 512/512 bytes at offset 134218240 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +1024/1024 bytes allocated at offset 128 MiB +[{ "start": 0, "length": 134217728, "depth": 0, "zero": true, "data": fals= e}, +{ "start": 134217728, "length": 1024, "depth": 0, "zero": false, "data": t= rue, "offset": OFFSET}] +wrote 1024/1024 bytes at offset 134217728 +1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +1024/1024 bytes allocated at offset 128 MiB +read 1024/1024 bytes at offset 134217728 +1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +[{ "start": 0, "length": 134217728, "depth": 0, "zero": true, "data": fals= e}, +{ "start": 134217728, "length": 1024, "depth": 0, "zero": true, "data": fa= lse, "offset": OFFSET}] + +=3D=3D unaligned image tail cluster, allocation required =3D=3D +Formatting 'TEST_DIR/t.IMGFMT.base', fmt=3DIMGFMT size=3D134218752 +Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D134219776 backing_file= =3DTEST_DIR/t.IMGFMT.base +wrote 512/512 bytes at offset 134217728 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +wrote 512/512 bytes at offset 134218752 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 512/512 bytes at offset 134217728 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 1536/1536 bytes at offset 134218240 +1.500 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +[{ "start": 0, "length": 134217728, "depth": 1, "zero": true, "data": fals= e}, +{ "start": 134217728, "length": 2048, "depth": 0, "zero": false, "data": t= rue, "offset": OFFSET}] +Formatting 'TEST_DIR/t.IMGFMT.base', fmt=3DIMGFMT size=3D134218752 +Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D134219776 backing_file= =3DTEST_DIR/t.IMGFMT.base +wrote 512/512 bytes at offset 134218240 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +wrote 512/512 bytes at offset 134217728 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 512/512 bytes at offset 134217728 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 512/512 bytes at offset 134218240 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 1024/1024 bytes at offset 134218752 +1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +wrote 512/512 bytes at offset 134219264 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 512/512 bytes at offset 134217728 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 512/512 bytes at offset 134218240 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 1024/1024 bytes at offset 134218752 +1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +[{ "start": 0, "length": 134217728, "depth": 1, "zero": true, "data": fals= e}, +{ "start": 134217728, "length": 2048, "depth": 0, "zero": false, "data": t= rue, "offset": OFFSET}] *** done --=20 2.9.3 From nobody Tue May 7 05:07:07 2024 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.zoho.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; Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 149411606536075.25900997552492; Sat, 6 May 2017 17:14:25 -0700 (PDT) Received: from localhost ([::1]:53127 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d79qB-0001Ea-Tt for importer@patchew.org; Sat, 06 May 2017 20:14:23 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45882) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d79iI-0002kJ-9D for qemu-devel@nongnu.org; Sat, 06 May 2017 20:06:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1d79iH-0007lv-AT for qemu-devel@nongnu.org; Sat, 06 May 2017 20:06:14 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50454) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1d79iE-0007kv-Pe; Sat, 06 May 2017 20:06:10 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D3DA9C20AE81; Sun, 7 May 2017 00:06:09 +0000 (UTC) Received: from red.redhat.com (ovpn-122-206.rdu2.redhat.com [10.10.122.206]) by smtp.corp.redhat.com (Postfix) with ESMTP id 28B2F18345; Sun, 7 May 2017 00:06:09 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com D3DA9C20AE81 Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=eblake@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com D3DA9C20AE81 From: Eric Blake To: qemu-devel@nongnu.org Date: Sat, 6 May 2017 19:05:51 -0500 Message-Id: <20170507000552.20847-12-eblake@redhat.com> In-Reply-To: <20170507000552.20847-1-eblake@redhat.com> References: <20170507000552.20847-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Sun, 07 May 2017 00:06:10 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v13 11/12] qcow2: Assert that cluster operations are aligned 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: kwolf@redhat.com, qemu-block@nongnu.org, mreitz@redhat.com 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" We already audited (in commit 0c1bd469) that qcow2_discard_clusters() is only passed cluster-aligned start values; but we can further tighten the assertion that the only unaligned end value is at EOF. Recent commits have taken advantage of an unaligned tail cluster, for both discard and write zeroes. Signed-off-by: Eric Blake Reviewed-by: Max Reitz --- v13: no change v12: no change v11: reserved for blkdebug half of v10 v10: rebase to context v9: rebase to master, by asserting that only tail cluster is unaligned v7, v8: only earlier half of series submitted for 2.9 v6: avoid assertion on non-cluster-aligned image, use s->cluster_sectors to avoid a shift, drop R-b v5: no change v4: new patch --- block/qcow2-cluster.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 6c59708..1b9592d 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -1559,11 +1559,10 @@ int qcow2_discard_clusters(BlockDriverState *bs, ui= nt64_t offset, end_offset =3D offset + (nb_sectors << BDRV_SECTOR_BITS); - /* The caller must cluster-align start; round end down except at EOF */ + /* Caller must pass aligned values, except at image end */ assert(QEMU_IS_ALIGNED(offset, s->cluster_size)); - if (end_offset !=3D bs->total_sectors * BDRV_SECTOR_SIZE) { - end_offset =3D start_of_cluster(s, end_offset); - } + assert(QEMU_IS_ALIGNED(end_offset, s->cluster_size) || + end_offset =3D=3D bs->total_sectors << BDRV_SECTOR_BITS); nb_clusters =3D size_to_clusters(s, end_offset - offset); @@ -1646,9 +1645,17 @@ int qcow2_zero_clusters(BlockDriverState *bs, uint64= _t offset, int nb_sectors, int flags) { BDRVQcow2State *s =3D bs->opaque; + uint64_t end_offset; uint64_t nb_clusters; int ret; + end_offset =3D offset + (nb_sectors << BDRV_SECTOR_BITS); + + /* Caller must pass aligned values, except at image end */ + assert(QEMU_IS_ALIGNED(offset, s->cluster_size)); + assert(QEMU_IS_ALIGNED(end_offset, s->cluster_size) || + end_offset =3D=3D bs->total_sectors << BDRV_SECTOR_BITS); + /* The zero flag is only supported by version 3 and newer */ if (s->qcow_version < 3) { return -ENOTSUP; --=20 2.9.3 From nobody Tue May 7 05:07:07 2024 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.zoho.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; Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1494115853181765.5138379203217; Sat, 6 May 2017 17:10:53 -0700 (PDT) Received: from localhost ([::1]:53106 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d79ml-0006Y7-Io for importer@patchew.org; Sat, 06 May 2017 20:10:51 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45924) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d79iN-0002nR-1Q for qemu-devel@nongnu.org; Sat, 06 May 2017 20:06:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1d79iL-0007n2-CP for qemu-devel@nongnu.org; Sat, 06 May 2017 20:06:19 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53094) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1d79iF-0007lC-Ow; Sat, 06 May 2017 20:06:11 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C648761BB8; Sun, 7 May 2017 00:06:10 +0000 (UTC) Received: from red.redhat.com (ovpn-122-206.rdu2.redhat.com [10.10.122.206]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1972718345; Sun, 7 May 2017 00:06:09 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com C648761BB8 Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=eblake@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com C648761BB8 From: Eric Blake To: qemu-devel@nongnu.org Date: Sat, 6 May 2017 19:05:52 -0500 Message-Id: <20170507000552.20847-13-eblake@redhat.com> In-Reply-To: <20170507000552.20847-1-eblake@redhat.com> References: <20170507000552.20847-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Sun, 07 May 2017 00:06:10 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v13 12/12] qcow2: Discard/zero clusters by byte count 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: kwolf@redhat.com, qemu-block@nongnu.org, mreitz@redhat.com 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" Passing a byte offset, but sector count, when we ultimately want to operate on cluster granularity, is madness. Clean up the external interfaces to take both offset and count as bytes, while still keeping the assertion added previously that the caller must align the values to a cluster. Then rename things to make sure backports don't get confused by changed units: instead of qcow2_discard_clusters() and qcow2_zero_clusters(), we now have qcow2_cluster_discard() and qcow2_cluster_zeroize(). The internal functions still operate on clusters at a time, and return an int for number of cleared clusters; but on an image with 2M clusters, a single L2 table holds 256k entries that each represent a 2M cluster, totalling well over INT_MAX bytes if we ever had a request for that many bytes at once. All our callers currently limit themselves to 32-bit bytes (and therefore fewer clusters), but by making this function 64-bit clean, we have one less place to clean up if we later improve the block layer to support 64-bit bytes through all operations (with the block layer auto-fragmenting on behalf of more-limited drivers), rather than the current state where some interfaces are artificially limited to INT_MAX at a time. Signed-off-by: Eric Blake Reviewed-by: Max Reitz --- v13: no change v12: minor tweaks suggested by Max v11: reserved for blkdebug half of v10 v10: squash in fixup accounting for unaligned file end v9: rebase to earlier changes, drop R-b v7, v8: only earlier half of series submitted for 2.9 v6: rebase due to context v5: s/count/byte/ to make the units obvious, and rework the math to ensure no 32-bit integer overflow on large clusters v4: improve function names, split assertion additions into earlier patch [no v3 or v2] v1: https://lists.gnu.org/archive/html/qemu-devel/2016-12/msg00339.html --- block/qcow2.h | 9 +++++---- block/qcow2-cluster.c | 42 ++++++++++++++++++++++-------------------- block/qcow2-snapshot.c | 7 +++---- block/qcow2.c | 22 +++++++++------------- 4 files changed, 39 insertions(+), 41 deletions(-) diff --git a/block/qcow2.h b/block/qcow2.h index 810ceb8..1801dc3 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -551,10 +551,11 @@ uint64_t qcow2_alloc_compressed_cluster_offset(BlockD= riverState *bs, int compressed_size); int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m); -int qcow2_discard_clusters(BlockDriverState *bs, uint64_t offset, - int nb_sectors, enum qcow2_discard_type type, bool full_discard); -int qcow2_zero_clusters(BlockDriverState *bs, uint64_t offset, int nb_sect= ors, - int flags); +int qcow2_cluster_discard(BlockDriverState *bs, uint64_t offset, + uint64_t bytes, enum qcow2_discard_type type, + bool full_discard); +int qcow2_cluster_zeroize(BlockDriverState *bs, uint64_t offset, + uint64_t bytes, int flags); int qcow2_expand_zero_clusters(BlockDriverState *bs, BlockDriverAmendStatusCB *status_cb, diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 1b9592d..89d23e5 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -1549,34 +1549,36 @@ static int discard_single_l2(BlockDriverState *bs, = uint64_t offset, return nb_clusters; } -int qcow2_discard_clusters(BlockDriverState *bs, uint64_t offset, - int nb_sectors, enum qcow2_discard_type type, bool full_discard) +int qcow2_cluster_discard(BlockDriverState *bs, uint64_t offset, + uint64_t bytes, enum qcow2_discard_type type, + bool full_discard) { BDRVQcow2State *s =3D bs->opaque; - uint64_t end_offset; + uint64_t end_offset =3D offset + bytes; uint64_t nb_clusters; + int64_t cleared; int ret; - end_offset =3D offset + (nb_sectors << BDRV_SECTOR_BITS); - /* Caller must pass aligned values, except at image end */ assert(QEMU_IS_ALIGNED(offset, s->cluster_size)); assert(QEMU_IS_ALIGNED(end_offset, s->cluster_size) || end_offset =3D=3D bs->total_sectors << BDRV_SECTOR_BITS); - nb_clusters =3D size_to_clusters(s, end_offset - offset); + nb_clusters =3D size_to_clusters(s, bytes); s->cache_discards =3D true; /* Each L2 table is handled by its own loop iteration */ while (nb_clusters > 0) { - ret =3D discard_single_l2(bs, offset, nb_clusters, type, full_disc= ard); - if (ret < 0) { + cleared =3D discard_single_l2(bs, offset, nb_clusters, type, + full_discard); + if (cleared < 0) { + ret =3D cleared; goto fail; } - nb_clusters -=3D ret; - offset +=3D (ret * s->cluster_size); + nb_clusters -=3D cleared; + offset +=3D (cleared * s->cluster_size); } ret =3D 0; @@ -1641,16 +1643,15 @@ static int zero_single_l2(BlockDriverState *bs, uin= t64_t offset, return nb_clusters; } -int qcow2_zero_clusters(BlockDriverState *bs, uint64_t offset, int nb_sect= ors, - int flags) +int qcow2_cluster_zeroize(BlockDriverState *bs, uint64_t offset, + uint64_t bytes, int flags) { BDRVQcow2State *s =3D bs->opaque; - uint64_t end_offset; + uint64_t end_offset =3D offset + bytes; uint64_t nb_clusters; + int64_t cleared; int ret; - end_offset =3D offset + (nb_sectors << BDRV_SECTOR_BITS); - /* Caller must pass aligned values, except at image end */ assert(QEMU_IS_ALIGNED(offset, s->cluster_size)); assert(QEMU_IS_ALIGNED(end_offset, s->cluster_size) || @@ -1662,18 +1663,19 @@ int qcow2_zero_clusters(BlockDriverState *bs, uint6= 4_t offset, int nb_sectors, } /* Each L2 table is handled by its own loop iteration */ - nb_clusters =3D size_to_clusters(s, nb_sectors << BDRV_SECTOR_BITS); + nb_clusters =3D size_to_clusters(s, bytes); s->cache_discards =3D true; while (nb_clusters > 0) { - ret =3D zero_single_l2(bs, offset, nb_clusters, flags); - if (ret < 0) { + cleared =3D zero_single_l2(bs, offset, nb_clusters, flags); + if (cleared < 0) { + ret =3D cleared; goto fail; } - nb_clusters -=3D ret; - offset +=3D (ret * s->cluster_size); + nb_clusters -=3D cleared; + offset +=3D (cleared * s->cluster_size); } ret =3D 0; diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c index 0324243..44243e0 100644 --- a/block/qcow2-snapshot.c +++ b/block/qcow2-snapshot.c @@ -440,10 +440,9 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSn= apshotInfo *sn_info) /* The VM state isn't needed any more in the active L1 table; in fact,= it * hurts by causing expensive COW for the next snapshot. */ - qcow2_discard_clusters(bs, qcow2_vm_state_offset(s), - align_offset(sn->vm_state_size, s->cluster_size) - >> BDRV_SECTOR_BITS, - QCOW2_DISCARD_NEVER, false); + qcow2_cluster_discard(bs, qcow2_vm_state_offset(s), + align_offset(sn->vm_state_size, s->cluster_size), + QCOW2_DISCARD_NEVER, false); #ifdef DEBUG_ALLOC { diff --git a/block/qcow2.c b/block/qcow2.c index 3478bb6..ce571ea 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -2512,7 +2512,7 @@ static coroutine_fn int qcow2_co_pwrite_zeroes(BlockD= riverState *bs, trace_qcow2_pwrite_zeroes(qemu_coroutine_self(), offset, count); /* Whatever is left can use real zero clusters */ - ret =3D qcow2_zero_clusters(bs, offset, count >> BDRV_SECTOR_BITS, fla= gs); + ret =3D qcow2_cluster_zeroize(bs, offset, count, flags); qemu_co_mutex_unlock(&s->lock); return ret; @@ -2535,8 +2535,8 @@ static coroutine_fn int qcow2_co_pdiscard(BlockDriver= State *bs, } qemu_co_mutex_lock(&s->lock); - ret =3D qcow2_discard_clusters(bs, offset, count >> BDRV_SECTOR_BITS, - QCOW2_DISCARD_REQUEST, false); + ret =3D qcow2_cluster_discard(bs, offset, count, QCOW2_DISCARD_REQUEST, + false); qemu_co_mutex_unlock(&s->lock); return ret; } @@ -2843,9 +2843,8 @@ fail: static int qcow2_make_empty(BlockDriverState *bs) { BDRVQcow2State *s =3D bs->opaque; - uint64_t start_sector; - int sector_step =3D (QEMU_ALIGN_DOWN(INT_MAX, s->cluster_size) / - BDRV_SECTOR_SIZE); + uint64_t offset, end_offset; + int step =3D QEMU_ALIGN_DOWN(INT_MAX, s->cluster_size); int l1_clusters, ret =3D 0; l1_clusters =3D DIV_ROUND_UP(s->l1_size, s->cluster_size / sizeof(uint= 64_t)); @@ -2862,18 +2861,15 @@ static int qcow2_make_empty(BlockDriverState *bs) /* This fallback code simply discards every active cluster; this is sl= ow, * but works in all cases */ - for (start_sector =3D 0; start_sector < bs->total_sectors; - start_sector +=3D sector_step) - { + end_offset =3D bs->total_sectors * BDRV_SECTOR_SIZE; + for (offset =3D 0; offset < end_offset; offset +=3D step) { /* As this function is generally used after committing an external * snapshot, QCOW2_DISCARD_SNAPSHOT seems appropriate. Also, the * default action for this kind of discard is to pass the discard, * which will ideally result in an actually smaller image file, as * is probably desired. */ - ret =3D qcow2_discard_clusters(bs, start_sector * BDRV_SECTOR_SIZE, - MIN(sector_step, - bs->total_sectors - start_sector), - QCOW2_DISCARD_SNAPSHOT, true); + ret =3D qcow2_cluster_discard(bs, offset, MIN(step, end_offset - o= ffset), + QCOW2_DISCARD_SNAPSHOT, true); if (ret < 0) { break; } --=20 2.9.3