From: Kevin Wolf <kwolf@redhat.com>
To: qemu-devel@nongnu.org
Cc: kwolf@redhat.com
Subject: [Qemu-devel] [PATCH 2/2] qcow2/vdi: Change check to distinguish error cases
Date: Fri, 2 Jul 2010 19:15:00 +0200 [thread overview]
Message-ID: <1278090900-12832-3-git-send-email-kwolf@redhat.com> (raw)
In-Reply-To: <1278090900-12832-1-git-send-email-kwolf@redhat.com>
This distinguishes between harmless leaks and real corruption. Hopefully users
better understand what qemu-img check wants to tell them.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 3 +-
block/qcow2-refcount.c | 120 ++++++++++++++++++++++++++----------------------
block/qcow2.c | 4 +-
block/qcow2.h | 2 +-
block/vdi.c | 10 ++--
block_int.h | 7 ++-
6 files changed, 79 insertions(+), 67 deletions(-)
diff --git a/block.c b/block.c
index b0ceef0..65cf4dc 100644
--- a/block.c
+++ b/block.c
@@ -721,8 +721,7 @@ int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res)
}
memset(res, 0, sizeof(*res));
- res->corruptions = bs->drv->bdrv_check(bs);
- return res->corruptions < 0 ? res->corruptions : 0;
+ return bs->drv->bdrv_check(bs, res);
}
/* commit COW file into the raw image */
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 4a96d98..4c19e7e 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -884,9 +884,10 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
* This is used to construct a temporary refcount table out of L1 and L2 tables
* which can be compared the the refcount table saved in the image.
*
- * Returns the number of errors in the image that were found
+ * Modifies the number of errors in res.
*/
-static int inc_refcounts(BlockDriverState *bs,
+static void inc_refcounts(BlockDriverState *bs,
+ BdrvCheckResult *res,
uint16_t *refcount_table,
int refcount_table_size,
int64_t offset, int64_t size)
@@ -894,30 +895,32 @@ static int inc_refcounts(BlockDriverState *bs,
BDRVQcowState *s = bs->opaque;
int64_t start, last, cluster_offset;
int k;
- int errors = 0;
if (size <= 0)
- return 0;
+ return;
start = offset & ~(s->cluster_size - 1);
last = (offset + size - 1) & ~(s->cluster_size - 1);
for(cluster_offset = start; cluster_offset <= last;
cluster_offset += s->cluster_size) {
k = cluster_offset >> s->cluster_bits;
- if (k < 0 || k >= refcount_table_size) {
+ if (k < 0) {
fprintf(stderr, "ERROR: invalid cluster offset=0x%" PRIx64 "\n",
cluster_offset);
- errors++;
+ res->corruptions++;
+ } else if (k >= refcount_table_size) {
+ fprintf(stderr, "Warning: cluster offset=0x%" PRIx64 " is after "
+ "the end of the image file, can't properly check refcounts.\n",
+ cluster_offset);
+ res->check_errors++;
} else {
if (++refcount_table[k] == 0) {
fprintf(stderr, "ERROR: overflow cluster offset=0x%" PRIx64
"\n", cluster_offset);
- errors++;
+ res->corruptions++;
}
}
}
-
- return errors;
}
/*
@@ -928,14 +931,13 @@ static int inc_refcounts(BlockDriverState *bs,
* Returns the number of errors found by the checks or -errno if an internal
* error occurred.
*/
-static int check_refcounts_l2(BlockDriverState *bs,
+static int check_refcounts_l2(BlockDriverState *bs, BdrvCheckResult *res,
uint16_t *refcount_table, int refcount_table_size, int64_t l2_offset,
int check_copied)
{
BDRVQcowState *s = bs->opaque;
uint64_t *l2_table, offset;
int i, l2_size, nb_csectors, refcount;
- int errors = 0;
/* Read L2 table from disk */
l2_size = s->l2_size * sizeof(uint64_t);
@@ -955,16 +957,15 @@ static int check_refcounts_l2(BlockDriverState *bs,
"copied flag must never be set for compressed "
"clusters\n", offset >> s->cluster_bits);
offset &= ~QCOW_OFLAG_COPIED;
- errors++;
+ res->corruptions++;
}
/* Mark cluster as used */
nb_csectors = ((offset >> s->csize_shift) &
s->csize_mask) + 1;
offset &= s->cluster_offset_mask;
- errors += inc_refcounts(bs, refcount_table,
- refcount_table_size,
- offset & ~511, nb_csectors * 512);
+ inc_refcounts(bs, res, refcount_table, refcount_table_size,
+ offset & ~511, nb_csectors * 512);
} else {
/* QCOW_OFLAG_COPIED must be set iff refcount == 1 */
if (check_copied) {
@@ -974,35 +975,35 @@ static int check_refcounts_l2(BlockDriverState *bs,
if (refcount < 0) {
fprintf(stderr, "Can't get refcount for offset %"
PRIx64 ": %s\n", entry, strerror(-refcount));
+ goto fail;
}
if ((refcount == 1) != ((entry & QCOW_OFLAG_COPIED) != 0)) {
fprintf(stderr, "ERROR OFLAG_COPIED: offset=%"
PRIx64 " refcount=%d\n", entry, refcount);
- errors++;
+ res->corruptions++;
}
}
/* Mark cluster as used */
offset &= ~QCOW_OFLAG_COPIED;
- errors += inc_refcounts(bs, refcount_table,
- refcount_table_size,
- offset, s->cluster_size);
+ inc_refcounts(bs, res, refcount_table,refcount_table_size,
+ offset, s->cluster_size);
/* Correct offsets are cluster aligned */
if (offset & (s->cluster_size - 1)) {
fprintf(stderr, "ERROR offset=%" PRIx64 ": Cluster is not "
"properly aligned; L2 entry corrupted.\n", offset);
- errors++;
+ res->corruptions++;
}
}
}
}
qemu_free(l2_table);
- return errors;
+ return 0;
fail:
- fprintf(stderr, "ERROR: I/O error in check_refcounts_l1\n");
+ fprintf(stderr, "ERROR: I/O error in check_refcounts_l2\n");
qemu_free(l2_table);
return -EIO;
}
@@ -1016,6 +1017,7 @@ fail:
* error occurred.
*/
static int check_refcounts_l1(BlockDriverState *bs,
+ BdrvCheckResult *res,
uint16_t *refcount_table,
int refcount_table_size,
int64_t l1_table_offset, int l1_size,
@@ -1024,13 +1026,12 @@ static int check_refcounts_l1(BlockDriverState *bs,
BDRVQcowState *s = bs->opaque;
uint64_t *l1_table, l2_offset, l1_size2;
int i, refcount, ret;
- int errors = 0;
l1_size2 = l1_size * sizeof(uint64_t);
/* Mark L1 table as used */
- errors += inc_refcounts(bs, refcount_table, refcount_table_size,
- l1_table_offset, l1_size2);
+ inc_refcounts(bs, res, refcount_table, refcount_table_size,
+ l1_table_offset, l1_size2);
/* Read L1 table entries from disk */
if (l1_size2 == 0) {
@@ -1055,42 +1056,41 @@ static int check_refcounts_l1(BlockDriverState *bs,
if (refcount < 0) {
fprintf(stderr, "Can't get refcount for l2_offset %"
PRIx64 ": %s\n", l2_offset, strerror(-refcount));
+ goto fail;
}
if ((refcount == 1) != ((l2_offset & QCOW_OFLAG_COPIED) != 0)) {
fprintf(stderr, "ERROR OFLAG_COPIED: l2_offset=%" PRIx64
" refcount=%d\n", l2_offset, refcount);
- errors++;
+ res->corruptions++;
}
}
/* Mark L2 table as used */
l2_offset &= ~QCOW_OFLAG_COPIED;
- errors += inc_refcounts(bs, refcount_table,
- refcount_table_size,
- l2_offset,
- s->cluster_size);
+ inc_refcounts(bs, res, refcount_table, refcount_table_size,
+ l2_offset, s->cluster_size);
/* L2 tables are cluster aligned */
if (l2_offset & (s->cluster_size - 1)) {
fprintf(stderr, "ERROR l2_offset=%" PRIx64 ": Table is not "
"cluster aligned; L1 entry corrupted\n", l2_offset);
- errors++;
+ res->corruptions++;
}
/* Process and check L2 entries */
- ret = check_refcounts_l2(bs, refcount_table, refcount_table_size,
- l2_offset, check_copied);
+ ret = check_refcounts_l2(bs, res, refcount_table,
+ refcount_table_size, l2_offset, check_copied);
if (ret < 0) {
goto fail;
}
- errors += ret;
}
}
qemu_free(l1_table);
- return errors;
+ return 0;
fail:
fprintf(stderr, "ERROR: I/O error in check_refcounts_l1\n");
+ res->check_errors++;
qemu_free(l1_table);
return -EIO;
}
@@ -1101,44 +1101,47 @@ fail:
* Returns 0 if no errors are found, the number of errors in case the image is
* detected as corrupted, and -errno when an internal error occured.
*/
-int qcow2_check_refcounts(BlockDriverState *bs)
+int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res)
{
BDRVQcowState *s = bs->opaque;
int64_t size;
int nb_clusters, refcount1, refcount2, i;
QCowSnapshot *sn;
uint16_t *refcount_table;
- int ret, errors = 0;
+ int ret;
size = bdrv_getlength(bs->file);
nb_clusters = size_to_clusters(s, size);
refcount_table = qemu_mallocz(nb_clusters * sizeof(uint16_t));
/* header */
- errors += inc_refcounts(bs, refcount_table, nb_clusters,
- 0, s->cluster_size);
+ inc_refcounts(bs, res, refcount_table, nb_clusters,
+ 0, s->cluster_size);
/* current L1 table */
- ret = check_refcounts_l1(bs, refcount_table, nb_clusters,
+ ret = check_refcounts_l1(bs, res, refcount_table, nb_clusters,
s->l1_table_offset, s->l1_size, 1);
if (ret < 0) {
return ret;
}
- errors += ret;
/* snapshots */
for(i = 0; i < s->nb_snapshots; i++) {
sn = s->snapshots + i;
- check_refcounts_l1(bs, refcount_table, nb_clusters,
- sn->l1_table_offset, sn->l1_size, 0);
+ ret = check_refcounts_l1(bs, res, refcount_table, nb_clusters,
+ sn->l1_table_offset, sn->l1_size, 0);
+ if (ret < 0) {
+ return ret;
+ }
}
- errors += inc_refcounts(bs, refcount_table, nb_clusters,
- s->snapshots_offset, s->snapshots_size);
+ inc_refcounts(bs, res, refcount_table, nb_clusters,
+ s->snapshots_offset, s->snapshots_size);
/* refcount data */
- errors += inc_refcounts(bs, refcount_table, nb_clusters,
- s->refcount_table_offset,
- s->refcount_table_size * sizeof(uint64_t));
+ inc_refcounts(bs, res, refcount_table, nb_clusters,
+ s->refcount_table_offset,
+ s->refcount_table_size * sizeof(uint64_t));
+
for(i = 0; i < s->refcount_table_size; i++) {
uint64_t offset, cluster;
offset = s->refcount_table[i];
@@ -1148,22 +1151,23 @@ int qcow2_check_refcounts(BlockDriverState *bs)
if (offset & (s->cluster_size - 1)) {
fprintf(stderr, "ERROR refcount block %d is not "
"cluster aligned; refcount table entry corrupted\n", i);
- errors++;
+ res->corruptions++;
continue;
}
if (cluster >= nb_clusters) {
fprintf(stderr, "ERROR refcount block %d is outside image\n", i);
- errors++;
+ res->corruptions++;
continue;
}
if (offset != 0) {
- errors += inc_refcounts(bs, refcount_table, nb_clusters,
- offset, s->cluster_size);
+ inc_refcounts(bs, res, refcount_table, nb_clusters,
+ offset, s->cluster_size);
if (refcount_table[cluster] != 1) {
fprintf(stderr, "ERROR refcount block %d refcount=%d\n",
i, refcount_table[cluster]);
+ res->corruptions++;
}
}
}
@@ -1174,19 +1178,25 @@ int qcow2_check_refcounts(BlockDriverState *bs)
if (refcount1 < 0) {
fprintf(stderr, "Can't get refcount for cluster %d: %s\n",
i, strerror(-refcount1));
+ res->check_errors++;
continue;
}
refcount2 = refcount_table[i];
if (refcount1 != refcount2) {
- fprintf(stderr, "ERROR cluster %d refcount=%d reference=%d\n",
+ fprintf(stderr, "%s cluster %d refcount=%d reference=%d\n",
+ refcount1 < refcount2 ? "ERROR" : "Leaked",
i, refcount1, refcount2);
- errors++;
+ if (refcount1 < refcount2) {
+ res->corruptions++;
+ } else {
+ res->leaks++;
+ }
}
}
qemu_free(refcount_table);
- return errors;
+ return 0;
}
diff --git a/block/qcow2.c b/block/qcow2.c
index 24651fc..f2b1b1c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1146,9 +1146,9 @@ static int qcow_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
}
-static int qcow_check(BlockDriverState *bs)
+static int qcow_check(BlockDriverState *bs, BdrvCheckResult *result)
{
- return qcow2_check_refcounts(bs);
+ return qcow2_check_refcounts(bs, result);
}
#if 0
diff --git a/block/qcow2.h b/block/qcow2.h
index c59b827..3ff162e 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -185,7 +185,7 @@ void qcow2_create_refcount_update(QCowCreateState *s, int64_t offset,
int qcow2_update_snapshot_refcount(BlockDriverState *bs,
int64_t l1_table_offset, int l1_size, int addend);
-int qcow2_check_refcounts(BlockDriverState *bs);
+int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res);
/* qcow2-cluster.c functions */
int qcow2_grow_l1_table(BlockDriverState *bs, int min_size);
diff --git a/block/vdi.c b/block/vdi.c
index ee8cc7b..f72633c 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -291,11 +291,10 @@ static void vdi_header_print(VdiHeader *header)
}
#endif
-static int vdi_check(BlockDriverState *bs)
+static int vdi_check(BlockDriverState *bs, BdrvCheckResult *res)
{
/* TODO: additional checks possible. */
BDRVVdiState *s = (BDRVVdiState *)bs->opaque;
- int n_errors = 0;
uint32_t blocks_allocated = 0;
uint32_t block;
uint32_t *bmap;
@@ -315,11 +314,12 @@ static int vdi_check(BlockDriverState *bs)
} else {
fprintf(stderr, "ERROR: block index %" PRIu32
" also used by %" PRIu32 "\n", bmap[bmap_entry], bmap_entry);
+ res->corruptions++;
}
} else {
fprintf(stderr, "ERROR: block index %" PRIu32
" too large, is %" PRIu32 "\n", block, bmap_entry);
- n_errors++;
+ res->corruptions++;
}
}
}
@@ -327,12 +327,12 @@ static int vdi_check(BlockDriverState *bs)
fprintf(stderr, "ERROR: allocated blocks mismatch, is %" PRIu32
", should be %" PRIu32 "\n",
blocks_allocated, s->header.blocks_allocated);
- n_errors++;
+ res->corruptions++;
}
qemu_free(bmap);
- return n_errors;
+ return 0;
}
static int vdi_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
diff --git a/block_int.h b/block_int.h
index a94b801..877e1e5 100644
--- a/block_int.h
+++ b/block_int.h
@@ -119,8 +119,11 @@ struct BlockDriver {
QEMUOptionParameter *create_options;
- /* Returns number of errors in image, -errno for internal errors */
- int (*bdrv_check)(BlockDriverState* bs);
+ /*
+ * Returns 0 for completed check, -errno for internal errors.
+ * The check results are stored in result.
+ */
+ int (*bdrv_check)(BlockDriverState* bs, BdrvCheckResult *result);
void (*bdrv_debug_event)(BlockDriverState *bs, BlkDebugEvent event);
--
1.6.6.1
prev parent reply other threads:[~2010-07-02 17:15 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-07-02 17:14 [Qemu-devel] [PATCH 0/2] Improve qemu-img check output Kevin Wolf
2010-07-02 17:14 ` [Qemu-devel] [PATCH 1/2] qemu-img check: Distinguish different kinds of errors Kevin Wolf
2010-07-06 10:51 ` MORITA Kazutaka
2010-07-06 11:01 ` Kevin Wolf
2010-07-06 11:03 ` [Qemu-devel] [PATCH v2 " Kevin Wolf
2010-07-02 17:15 ` Kevin Wolf [this message]
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=1278090900-12832-3-git-send-email-kwolf@redhat.com \
--to=kwolf@redhat.com \
--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.