All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.