qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: david.edmondson@oracle.com, Kevin Wolf <kwolf@redhat.com>,
	qemu-block@nongnu.org, mreitz@redhat.com
Subject: [PATCH 17/17] qcow2: Let qemu-img check cover all-zero bit
Date: Fri, 31 Jan 2020 11:44:36 -0600	[thread overview]
Message-ID: <20200131174436.2961874-18-eblake@redhat.com> (raw)
In-Reply-To: <20200131174436.2961874-1-eblake@redhat.com>

Since checking an images refcounts already visits every cluster, it's
basically free to also check that the all-zero bit is correctly set.
Only check for the active L1 table, and only output an error on the
first non-zero cluster found.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 block/qcow2-refcount.c     | 60 +++++++++++++++++++++++++++++++++++---
 tests/qemu-iotests/060.out |  6 ++--
 tests/qemu-iotests/285     | 17 +++++++++++
 tests/qemu-iotests/285.out | 20 +++++++++++++
 4 files changed, 97 insertions(+), 6 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index f67ac6b2d893..95c8101df365 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1583,6 +1583,7 @@ int qcow2_inc_refcounts_imrt(BlockDriverState *bs, BdrvCheckResult *res,
 /* Flags for check_refcounts_l1() and check_refcounts_l2() */
 enum {
     CHECK_FRAG_INFO = 0x2,      /* update BlockFragInfo counters */
+    CHECK_ALL_ZERO = 0x4,       /* check autoclear all_zero bit */
 };

 /*
@@ -1596,12 +1597,14 @@ enum {
 static int check_refcounts_l2(BlockDriverState *bs, BdrvCheckResult *res,
                               void **refcount_table,
                               int64_t *refcount_table_size, int64_t l2_offset,
-                              int flags, BdrvCheckMode fix, bool active)
+                              int flags, BdrvCheckMode fix, bool active,
+                              bool *all_zero)
 {
     BDRVQcow2State *s = bs->opaque;
     uint64_t *l2_table, l2_entry;
     uint64_t next_contiguous_offset = 0;
     int i, l2_size, nb_csectors, ret;
+    bool check_all_zero;

     /* Read L2 table from disk */
     l2_size = s->l2_size * sizeof(uint64_t);
@@ -1615,8 +1618,9 @@ static int check_refcounts_l2(BlockDriverState *bs, BdrvCheckResult *res,
     }

     /* Do the actual checks */
-    for(i = 0; i < s->l2_size; i++) {
+    for (i = 0; i < s->l2_size; i++) {
         l2_entry = be64_to_cpu(l2_table[i]);
+        check_all_zero = *all_zero;

         switch (qcow2_get_cluster_type(bs, l2_entry)) {
         case QCOW2_CLUSTER_COMPRESSED:
@@ -1662,6 +1666,8 @@ static int check_refcounts_l2(BlockDriverState *bs, BdrvCheckResult *res,
             break;

         case QCOW2_CLUSTER_ZERO_ALLOC:
+            check_all_zero = false;
+            /* fall through */
         case QCOW2_CLUSTER_NORMAL:
         {
             uint64_t offset = l2_entry & L2E_OFFSET_MASK;
@@ -1740,12 +1746,51 @@ static int check_refcounts_l2(BlockDriverState *bs, BdrvCheckResult *res,
         }

         case QCOW2_CLUSTER_ZERO_PLAIN:
+            check_all_zero = false;
+            break;
+
         case QCOW2_CLUSTER_UNALLOCATED:
+            if (!bs->backing) {
+                check_all_zero = false;
+            }
             break;

         default:
             abort();
         }
+
+        if (check_all_zero) {
+            fprintf(stderr, "%s: all zero bit set but L2 table at offset "
+                    "0x%" PRIx64" contains non-zero cluster at entry %d\n",
+                    fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR",
+                    l2_offset, i);
+            *all_zero = false;
+            if (fix & BDRV_FIX_ERRORS) {
+                uint64_t feat;
+
+                ret = bdrv_pread(bs->file,
+                                 offsetof(QCowHeader, autoclear_features),
+                                 &feat, sizeof(feat));
+                if (ret >= 0) {
+                    feat &= ~cpu_to_be64(QCOW2_AUTOCLEAR_ALL_ZERO);
+                    ret = bdrv_pwrite(bs->file,
+                                      offsetof(QCowHeader, autoclear_features),
+                                      &feat, sizeof(feat));
+                }
+                if (ret < 0) {
+                    fprintf(stderr,
+                            "ERROR: Failed to update all zero bit: %s\n",
+                            strerror(-ret));
+                    res->check_errors++;
+                    /* Continue checking the rest of this L2 table */
+                } else {
+                    res->corruptions_fixed++;
+                }
+                s->autoclear_features &= ~QCOW2_AUTOCLEAR_ALL_ZERO;
+            } else {
+                res->corruptions++;
+            }
+        }
     }

     g_free(l2_table);
@@ -1774,6 +1819,12 @@ static int check_refcounts_l1(BlockDriverState *bs,
     BDRVQcow2State *s = bs->opaque;
     uint64_t *l1_table = NULL, l2_offset, l1_size2;
     int i, ret;
+    bool all_zero = false;
+
+    if (flags & CHECK_ALL_ZERO &&
+        s->autoclear_features & QCOW2_AUTOCLEAR_ALL_ZERO) {
+        all_zero = true;
+    }

     l1_size2 = l1_size * sizeof(uint64_t);

@@ -1825,7 +1876,7 @@ static int check_refcounts_l1(BlockDriverState *bs,
             /* Process and check L2 entries */
             ret = check_refcounts_l2(bs, res, refcount_table,
                                      refcount_table_size, l2_offset, flags,
-                                     fix, active);
+                                     fix, active, &all_zero);
             if (ret < 0) {
                 goto fail;
             }
@@ -2114,7 +2165,8 @@ static int calculate_refcounts(BlockDriverState *bs, BdrvCheckResult *res,

     /* current L1 table */
     ret = check_refcounts_l1(bs, res, refcount_table, nb_clusters,
-                             s->l1_table_offset, s->l1_size, CHECK_FRAG_INFO,
+                             s->l1_table_offset, s->l1_size,
+                             CHECK_FRAG_INFO | CHECK_ALL_ZERO,
                              fix, true);
     if (ret < 0) {
         return ret;
diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out
index d27692a33c0d..d82aca458544 100644
--- a/tests/qemu-iotests/060.out
+++ b/tests/qemu-iotests/060.out
@@ -3,9 +3,10 @@ QA output created by 060
 === Testing L2 reference into L1 ===

 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+ERROR: all zero bit set but L2 table at offset 0x30000 contains non-zero cluster at entry 0
 ERROR cluster 3 refcount=1 reference=3

-1 errors were found on the image.
+2 errors were found on the image.
 Data may be corrupted, or further writes to the image may corrupt it.
 incompatible_features     []
 qcow2: Marking image as corrupt: Preventing invalid write on metadata (overlaps with active L1 table); further corruption events will be suppressed
@@ -28,10 +29,11 @@ read 512/512 bytes at offset 0
 === Testing cluster data reference into refcount block ===

 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+ERROR: all zero bit set but L2 table at offset 0x40000 contains non-zero cluster at entry 0
 ERROR refcount block 0 refcount=2
 ERROR cluster 2 refcount=1 reference=2

-2 errors were found on the image.
+3 errors were found on the image.
 Data may be corrupted, or further writes to the image may corrupt it.
 incompatible_features     []
 qcow2: Marking image as corrupt: Preventing invalid write on metadata (overlaps with refcount block); further corruption events will be suppressed
diff --git a/tests/qemu-iotests/285 b/tests/qemu-iotests/285
index 66037af237a1..c435bb57d749 100755
--- a/tests/qemu-iotests/285
+++ b/tests/qemu-iotests/285
@@ -101,6 +101,23 @@ $QEMU_IMG snapshot -l snap "$TEST_IMG"
 $QEMU_IMG info "$TEST_IMG" | _filter_img_info --format-specific \
     | _filter_date | _filter_vmstate_size

+echo
+echo "=== qemu-img check ==="
+echo
+
+_make_test_img 32M
+$QEMU_IO -c 'w -P 1 0 1M' "$TEST_IMG" | _filter_qemu_io
+# Image should be clean
+_check_test_img
+# Manually corrupt the image by setting the bit
+$PYTHON qcow2.py "$TEST_IMG" set-feature-bit autoclear 2
+# check should detect the problem
+_check_test_img
+# repair should fix it
+_check_test_img -r all
+# the image should be clean again
+_check_test_img
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/285.out b/tests/qemu-iotests/285.out
index e43ff9906b5f..b28c9e266bf6 100644
--- a/tests/qemu-iotests/285.out
+++ b/tests/qemu-iotests/285.out
@@ -254,4 +254,24 @@ Format specific information:
     lazy refcounts: false
     refcount bits: 16
     corrupt: false
+
+=== qemu-img check ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33554432
+wrote 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+ERROR: all zero bit set but L2 table at offset 0x40000 contains non-zero cluster at entry 0
+
+1 errors were found on the image.
+Data may be corrupted, or further writes to the image may corrupt it.
+Repairing: all zero bit set but L2 table at offset 0x40000 contains non-zero cluster at entry 0
+The following inconsistencies were found and repaired:
+
+    0 leaked clusters
+    1 corruptions
+
+Double checking the fixed image now...
+No errors were found on the image.
+No errors were found on the image.
 *** done
-- 
2.24.1



  parent reply	other threads:[~2020-01-31 17:56 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-31 17:44 [PATCH 00/17] Improve qcow2 all-zero detection Eric Blake
2020-01-31 17:44 ` [PATCH 01/17] qcow2: Comment typo fixes Eric Blake
2020-02-04 14:12   ` Vladimir Sementsov-Ogievskiy
2020-02-09 19:34   ` Alberto Garcia
2020-01-31 17:44 ` [PATCH 02/17] qcow2: List autoclear bit names in header Eric Blake
2020-02-04 14:26   ` Vladimir Sementsov-Ogievskiy
2020-01-31 17:44 ` [PATCH 03/17] qcow2: Avoid feature name extension on small cluster size Eric Blake
2020-02-04 14:39   ` Vladimir Sementsov-Ogievskiy
2020-02-09 19:28   ` Alberto Garcia
2020-01-31 17:44 ` [PATCH 04/17] block: Improve documentation of .bdrv_has_zero_init Eric Blake
2020-02-04 15:03   ` Vladimir Sementsov-Ogievskiy
2020-02-04 15:16     ` Eric Blake
2020-01-31 17:44 ` [PATCH 05/17] block: Don't advertise zero_init_truncate with encryption Eric Blake
2020-02-10 18:12   ` Alberto Garcia
2020-01-31 17:44 ` [PATCH 06/17] block: Improve bdrv_has_zero_init_truncate with backing file Eric Blake
2020-02-10 18:13   ` Alberto Garcia
2020-01-31 17:44 ` [PATCH 07/17] gluster: Drop useless has_zero_init callback Eric Blake
2020-02-04 15:06   ` Vladimir Sementsov-Ogievskiy
2020-02-10 18:21   ` Alberto Garcia
2020-02-17  8:06   ` [GEDI] " Niels de Vos
2020-02-17 12:03     ` Eric Blake
2020-02-17 12:22       ` Eric Blake
2020-02-17 14:01       ` Niels de Vos
2020-01-31 17:44 ` [PATCH 08/17] sheepdog: Consistently set bdrv_has_zero_init_truncate Eric Blake
2020-02-04 15:09   ` Vladimir Sementsov-Ogievskiy
2020-01-31 17:44 ` [PATCH 09/17] block: Refactor bdrv_has_zero_init{,_truncate} Eric Blake
2020-02-04 15:35   ` Vladimir Sementsov-Ogievskiy
2020-02-04 15:49     ` Eric Blake
2020-02-04 16:07       ` Vladimir Sementsov-Ogievskiy
2020-02-04 17:42     ` Max Reitz
2020-02-04 17:51       ` Eric Blake
2020-02-05 16:43         ` Max Reitz
2020-02-05  7:51       ` Vladimir Sementsov-Ogievskiy
2020-02-05 14:07         ` Eric Blake
2020-02-05 14:25           ` Vladimir Sementsov-Ogievskiy
2020-02-05 14:36             ` Eric Blake
2020-02-05 17:55           ` Max Reitz
2020-02-04 17:53   ` Max Reitz
2020-02-04 19:03     ` Eric Blake
2020-02-05 17:22       ` Max Reitz
2020-02-05 18:39         ` Eric Blake
2020-02-06  9:18           ` Max Reitz
2020-01-31 17:44 ` [PATCH 10/17] block: Add new BDRV_ZERO_OPEN flag Eric Blake
2020-01-31 18:03   ` Eric Blake
2020-02-04 17:34   ` Max Reitz
2020-02-04 17:50     ` Eric Blake
2020-02-05  8:39       ` Vladimir Sementsov-Ogievskiy
2020-02-05 17:26       ` Max Reitz
2020-01-31 17:44 ` [PATCH 11/17] file-posix: Support BDRV_ZERO_OPEN Eric Blake
2020-01-31 17:44 ` [PATCH 12/17] gluster: " Eric Blake
2020-02-17  8:16   ` [GEDI] " Niels de Vos
2020-01-31 17:44 ` [PATCH 13/17] qcow2: Add new autoclear feature for all zero image Eric Blake
2020-02-03 17:45   ` Vladimir Sementsov-Ogievskiy
2020-02-04 13:12     ` Eric Blake
2020-02-04 13:29       ` Vladimir Sementsov-Ogievskiy
2020-01-31 17:44 ` [PATCH 14/17] qcow2: Expose all zero bit through .bdrv_known_zeroes Eric Blake
2020-01-31 17:44 ` [PATCH 15/17] qcow2: Implement all-zero autoclear bit Eric Blake
2020-01-31 17:44 ` [PATCH 16/17] iotests: Add new test for qcow2 all-zero bit Eric Blake
2020-01-31 17:44 ` Eric Blake [this message]
2020-02-04 17:32 ` [PATCH 00/17] Improve qcow2 all-zero detection Max Reitz
2020-02-04 18:53   ` Eric Blake
2020-02-05 17:04     ` Max Reitz
2020-02-05 19:21       ` Eric Blake
2020-02-06  9:12         ` Max Reitz
2020-02-05  9:04 ` Vladimir Sementsov-Ogievskiy
2020-02-05  9:25   ` Vladimir Sementsov-Ogievskiy
2020-02-05 14:26     ` Eric Blake
2020-02-05 14:47       ` Vladimir Sementsov-Ogievskiy
2020-02-05 15:14         ` Vladimir Sementsov-Ogievskiy
2020-02-05 17:58           ` Max Reitz
2020-02-05 14:22   ` Eric Blake
2020-02-05 14:43     ` Vladimir Sementsov-Ogievskiy
2020-02-05 14:58       ` Vladimir Sementsov-Ogievskiy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200131174436.2961874-18-eblake@redhat.com \
    --to=eblake@redhat.com \
    --cc=david.edmondson@oracle.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).