From 558939c6c7e06ef4610906e04bc9a39619e6f1df Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 21 Oct 2014 15:12:13 +0200 Subject: MAINTAINERS: add aio to block layer AioContext falls under the block layer, mark it as such. Signed-off-by: Paolo Bonzini Signed-off-by: Kevin Wolf --- MAINTAINERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 8eed800a37..b8e61171b3 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -710,6 +710,8 @@ Block M: Kevin Wolf M: Stefan Hajnoczi S: Supported +F: async.c +F: aio-*.c F: block* F: block/ F: hw/block/ -- cgit v1.2.1 From 292420918d4500624c096900f7834d25b23f1ad5 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 21 Oct 2014 17:43:17 +0200 Subject: MAINTAINERS: qemu-iotests belongs to the block layer Signed-off-by: Kevin Wolf --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index b8e61171b3..9a7818d3de 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -717,6 +717,7 @@ F: block/ F: hw/block/ F: qemu-img* F: qemu-io* +F: tests/qemu-iotests/ T: git git://repo.or.cz/qemu/kevin.git block T: git git://github.com/stefanha/qemu.git block -- cgit v1.2.1 From 8113fb529282897892f8010ed679565b0f1488d9 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 22 Oct 2014 00:29:58 +0200 Subject: MAINTAINERS: add the image fuzzer to the block layer More work for the block device maintainers! Signed-off-by: Paolo Bonzini Signed-off-by: Kevin Wolf --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 9a7818d3de..95553d99e5 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -717,6 +717,7 @@ F: block/ F: hw/block/ F: qemu-img* F: qemu-io* +F: tests/image-fuzzer/ F: tests/qemu-iotests/ T: git git://repo.or.cz/qemu/kevin.git block T: git git://github.com/stefanha/qemu.git block -- cgit v1.2.1 From e9082e47364ac63922ea967aa5d82dbc8ac39960 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 21 Oct 2014 10:51:25 +0200 Subject: block/vdi: Use {DIV_,}ROUND_UP There are macros for these operations, so make use of them. Signed-off-by: Max Reitz Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- block/vdi.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/block/vdi.c b/block/vdi.c index 9604721476..19701ee00d 100644 --- a/block/vdi.c +++ b/block/vdi.c @@ -407,8 +407,7 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags, We accept them but round the disk size to the next multiple of SECTOR_SIZE. */ logout("odd disk size %" PRIu64 " B, round up\n", header.disk_size); - header.disk_size += SECTOR_SIZE - 1; - header.disk_size &= ~(SECTOR_SIZE - 1); + header.disk_size = ROUND_UP(header.disk_size, SECTOR_SIZE); } if (header.signature != VDI_SIGNATURE) { @@ -475,7 +474,7 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags, s->header = header; bmap_size = header.blocks_in_image * sizeof(uint32_t); - bmap_size = (bmap_size + SECTOR_SIZE - 1) / SECTOR_SIZE; + bmap_size = DIV_ROUND_UP(bmap_size, SECTOR_SIZE); s->bmap = qemu_try_blockalign(bs->file, bmap_size * SECTOR_SIZE); if (s->bmap == NULL) { ret = -ENOMEM; @@ -736,10 +735,10 @@ static int vdi_create(const char *filename, QemuOpts *opts, Error **errp) /* We need enough blocks to store the given disk size, so always round up. */ - blocks = (bytes + block_size - 1) / block_size; + blocks = DIV_ROUND_UP(bytes, block_size); bmap_size = blocks * sizeof(uint32_t); - bmap_size = ((bmap_size + SECTOR_SIZE - 1) & ~(SECTOR_SIZE -1)); + bmap_size = ROUND_UP(bmap_size, SECTOR_SIZE); memset(&header, 0, sizeof(header)); pstrcpy(header.text, sizeof(header.text), VDI_TEXT); -- cgit v1.2.1 From c5f7c0af473cadb8b0b5fc6d399e4ede1fc9408d Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Mon, 20 Oct 2014 13:47:11 +0200 Subject: block: qemu-iotests change _supported_proto to file once more. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In preparation to possible automatic regression and performance testing for the block layer I found that the iotests don't work for all protocols anymore. In commit 1f7bf7d0 I started to change supported protocols from generic to file for various tests. Unfortunately, some tests added in the meantime again carry generic protocol altough they can only work with file because they require local file access. The other way around for some tests that only support file I added NFS protocol after confirming they work. Signed-off-by: Peter Lieven Reviewed-by: Benoît Canet Signed-off-by: Kevin Wolf --- tests/qemu-iotests/075 | 2 +- tests/qemu-iotests/076 | 2 +- tests/qemu-iotests/078 | 2 +- tests/qemu-iotests/079 | 2 +- tests/qemu-iotests/080 | 2 +- tests/qemu-iotests/081 | 2 +- tests/qemu-iotests/082 | 2 +- tests/qemu-iotests/084 | 2 +- tests/qemu-iotests/086 | 2 +- tests/qemu-iotests/088 | 2 +- tests/qemu-iotests/090 | 2 +- tests/qemu-iotests/092 | 2 +- tests/qemu-iotests/103 | 2 +- 13 files changed, 13 insertions(+), 13 deletions(-) diff --git a/tests/qemu-iotests/075 b/tests/qemu-iotests/075 index 40032c563d..6117660c58 100755 --- a/tests/qemu-iotests/075 +++ b/tests/qemu-iotests/075 @@ -39,7 +39,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 . ./common.filter _supported_fmt cloop -_supported_proto generic +_supported_proto file _supported_os Linux block_size_offset=128 diff --git a/tests/qemu-iotests/076 b/tests/qemu-iotests/076 index b614a7dd6e..bc47457a85 100755 --- a/tests/qemu-iotests/076 +++ b/tests/qemu-iotests/076 @@ -39,7 +39,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 . ./common.filter _supported_fmt parallels -_supported_proto generic +_supported_proto file _supported_os Linux tracks_offset=$((0x1c)) diff --git a/tests/qemu-iotests/078 b/tests/qemu-iotests/078 index d4d6da7b09..7be2c3f691 100755 --- a/tests/qemu-iotests/078 +++ b/tests/qemu-iotests/078 @@ -39,7 +39,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 . ./common.filter _supported_fmt bochs -_supported_proto generic +_supported_proto file _supported_os Linux catalog_size_offset=$((0x48)) diff --git a/tests/qemu-iotests/079 b/tests/qemu-iotests/079 index 2142bbb377..6613cfb184 100755 --- a/tests/qemu-iotests/079 +++ b/tests/qemu-iotests/079 @@ -39,7 +39,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 . ./common.filter _supported_fmt qcow2 -_supported_proto file +_supported_proto file nfs _supported_os Linux function test_qemu_img() diff --git a/tests/qemu-iotests/080 b/tests/qemu-iotests/080 index 6b3a3e77a5..9de337c407 100755 --- a/tests/qemu-iotests/080 +++ b/tests/qemu-iotests/080 @@ -40,7 +40,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 . ./common.filter _supported_fmt qcow2 -_supported_proto generic +_supported_proto file _supported_os Linux header_size=104 diff --git a/tests/qemu-iotests/081 b/tests/qemu-iotests/081 index 7ae4be2053..ed3c29e13c 100755 --- a/tests/qemu-iotests/081 +++ b/tests/qemu-iotests/081 @@ -41,7 +41,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 . ./common.filter _supported_fmt raw -_supported_proto generic +_supported_proto file _supported_os Linux function do_run_qemu() diff --git a/tests/qemu-iotests/082 b/tests/qemu-iotests/082 index 910b13e8f0..e64de27733 100755 --- a/tests/qemu-iotests/082 +++ b/tests/qemu-iotests/082 @@ -39,7 +39,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 . ./common.filter _supported_fmt qcow2 -_supported_proto file +_supported_proto file nfs _supported_os Linux function run_qemu_img() diff --git a/tests/qemu-iotests/084 b/tests/qemu-iotests/084 index ae33c2cba4..2712c023a9 100755 --- a/tests/qemu-iotests/084 +++ b/tests/qemu-iotests/084 @@ -41,7 +41,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 # This tests vdi-specific header fields _supported_fmt vdi -_supported_proto generic +_supported_proto file _supported_os Linux size=64M diff --git a/tests/qemu-iotests/086 b/tests/qemu-iotests/086 index d9a80cf863..234eb9a91c 100755 --- a/tests/qemu-iotests/086 +++ b/tests/qemu-iotests/086 @@ -39,7 +39,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 . ./common.filter _supported_fmt qcow2 -_supported_proto file +_supported_proto file nfs _supported_os Linux function run_qemu_img() diff --git a/tests/qemu-iotests/088 b/tests/qemu-iotests/088 index c09adf8023..f9c3129182 100755 --- a/tests/qemu-iotests/088 +++ b/tests/qemu-iotests/088 @@ -40,7 +40,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 . ./common.filter _supported_fmt vpc -_supported_proto generic +_supported_proto file _supported_os Linux offset_block_size=$((512 + 32)) diff --git a/tests/qemu-iotests/090 b/tests/qemu-iotests/090 index 8d032f8111..70b5a6fd73 100755 --- a/tests/qemu-iotests/090 +++ b/tests/qemu-iotests/090 @@ -39,7 +39,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 . ./common.filter _supported_fmt qcow2 -_supported_proto file +_supported_proto file nfs _supported_os Linux IMG_SIZE=128K diff --git a/tests/qemu-iotests/092 b/tests/qemu-iotests/092 index a8c0c9ca2b..52c529bae3 100755 --- a/tests/qemu-iotests/092 +++ b/tests/qemu-iotests/092 @@ -40,7 +40,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 . ./common.filter _supported_fmt qcow -_supported_proto generic +_supported_proto file _supported_os Linux offset_backing_file_offset=8 diff --git a/tests/qemu-iotests/103 b/tests/qemu-iotests/103 index 0f1dc9fa7d..ccab551f63 100755 --- a/tests/qemu-iotests/103 +++ b/tests/qemu-iotests/103 @@ -39,7 +39,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 . ./common.filter _supported_fmt qcow2 -_supported_proto file +_supported_proto file nfs _supported_os Linux IMG_SIZE=64K -- cgit v1.2.1 From 9ebd84480583bb6d9a7666c079d99ff3266c423d Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 22 Oct 2014 14:09:27 +0200 Subject: block: Add qemu_{,try_}blockalign0() These functions call their non-0-counterparts and then fill the allocated buffer with 0 (if the allocation has been successful). Signed-off-by: Max Reitz Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- block.c | 16 ++++++++++++++++ include/block/block.h | 2 ++ 2 files changed, 18 insertions(+) diff --git a/block.c b/block.c index 773e87ef08..bbb04e7b95 100644 --- a/block.c +++ b/block.c @@ -5200,6 +5200,11 @@ void *qemu_blockalign(BlockDriverState *bs, size_t size) return qemu_memalign(bdrv_opt_mem_align(bs), size); } +void *qemu_blockalign0(BlockDriverState *bs, size_t size) +{ + return memset(qemu_blockalign(bs, size), 0, size); +} + void *qemu_try_blockalign(BlockDriverState *bs, size_t size) { size_t align = bdrv_opt_mem_align(bs); @@ -5213,6 +5218,17 @@ void *qemu_try_blockalign(BlockDriverState *bs, size_t size) return qemu_try_memalign(align, size); } +void *qemu_try_blockalign0(BlockDriverState *bs, size_t size) +{ + void *mem = qemu_try_blockalign(bs, size); + + if (mem) { + memset(mem, 0, size); + } + + return mem; +} + /* * Check if all memory in this vector is sector aligned. */ diff --git a/include/block/block.h b/include/block/block.h index c9ec0ab386..341054dcf7 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -412,7 +412,9 @@ void bdrv_img_create(const char *filename, const char *fmt, size_t bdrv_opt_mem_align(BlockDriverState *bs); void bdrv_set_guest_block_size(BlockDriverState *bs, int align); void *qemu_blockalign(BlockDriverState *bs, size_t size); +void *qemu_blockalign0(BlockDriverState *bs, size_t size); void *qemu_try_blockalign(BlockDriverState *bs, size_t size); +void *qemu_try_blockalign0(BlockDriverState *bs, size_t size); bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov); struct HBitmapIter; -- cgit v1.2.1 From 1d13d654666a7fd6d6a85a0ce9285dbf0d0444c2 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 22 Oct 2014 14:09:28 +0200 Subject: qcow2: Calculate refcount block entry count The size of a refblock entry is (in theory) variable; calculate therefore the number of entries per refblock and the according bit shift (1 << x == entry count) when opening an image. Signed-off-by: Max Reitz Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- block/qcow2.c | 3 +++ block/qcow2.h | 2 ++ 2 files changed, 5 insertions(+) diff --git a/block/qcow2.c b/block/qcow2.c index 156a219993..3c8b88198b 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -698,6 +698,9 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags, s->l2_bits = s->cluster_bits - 3; /* L2 is always one cluster */ s->l2_size = 1 << s->l2_bits; + /* 2^(s->refcount_order - 3) is the refcount width in bytes */ + s->refcount_block_bits = s->cluster_bits - (s->refcount_order - 3); + s->refcount_block_size = 1 << s->refcount_block_bits; bs->total_sectors = header.size / 512; s->csize_shift = (62 - (s->cluster_bits - 8)); s->csize_mask = (1 << (s->cluster_bits - 8)) - 1; diff --git a/block/qcow2.h b/block/qcow2.h index 7d61e615ff..47cd4b3f76 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -223,6 +223,8 @@ typedef struct BDRVQcowState { int l2_size; int l1_size; int l1_vm_state_index; + int refcount_block_bits; + int refcount_block_size; int csize_shift; int csize_mask; uint64_t cluster_offset_mask; -- cgit v1.2.1 From 5b84106bd91bc67519738042c8890a09e2967513 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 22 Oct 2014 14:09:29 +0200 Subject: qcow2: Fix leaks in dirty images MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When opening dirty images, qcow2's repair function should not only repair errors but leaks as well. Signed-off-by: Max Reitz Reviewed-by: Eric Blake Reviewed-by: Benoît Canet Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- block/qcow2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/qcow2.c b/block/qcow2.c index 3c8b88198b..7a2c66f92b 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -910,7 +910,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags, (s->incompatible_features & QCOW2_INCOMPAT_DIRTY)) { BdrvCheckResult result = {0}; - ret = qcow2_check(bs, &result, BDRV_FIX_ERRORS); + ret = qcow2_check(bs, &result, BDRV_FIX_ERRORS | BDRV_FIX_LEAKS); if (ret < 0) { error_setg_errno(errp, -ret, "Could not repair dirty image"); goto fail; -- cgit v1.2.1 From 6ca56bf5e90aa167395727667d17c699950c545c Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 22 Oct 2014 14:09:30 +0200 Subject: qcow2: Split qcow2_check_refcounts() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Put the code for calculating the reference counts and comparing them during qemu-img check into own functions. Signed-off-by: Max Reitz Reviewed-by: Benoît Canet Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- block/qcow2-refcount.c | 153 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 102 insertions(+), 51 deletions(-) diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index c31d85a840..e5f7876957 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -1535,71 +1535,70 @@ done: return new_offset; } +static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res, + BdrvCheckMode fix, uint16_t **refcount_table, + int64_t *nb_clusters); + /* - * Checks an image for refcount consistency. - * - * 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 occurred. + * Calculates an in-memory refcount table. */ -int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res, - BdrvCheckMode fix) +static int calculate_refcounts(BlockDriverState *bs, BdrvCheckResult *res, + BdrvCheckMode fix, uint16_t **refcount_table, + int64_t *nb_clusters) { BDRVQcowState *s = bs->opaque; - int64_t size, i, highest_cluster, nb_clusters; - int refcount1, refcount2; + int64_t i; QCowSnapshot *sn; - uint16_t *refcount_table; int ret; - size = bdrv_getlength(bs->file); - if (size < 0) { - res->check_errors++; - return size; - } - - nb_clusters = size_to_clusters(s, size); - if (nb_clusters > INT_MAX) { - res->check_errors++; - return -EFBIG; - } - - refcount_table = g_try_new0(uint16_t, nb_clusters); - if (nb_clusters && refcount_table == NULL) { + *refcount_table = g_try_new0(uint16_t, *nb_clusters); + if (*nb_clusters && *refcount_table == NULL) { res->check_errors++; return -ENOMEM; } - res->bfi.total_clusters = - size_to_clusters(s, bs->total_sectors * BDRV_SECTOR_SIZE); - /* header */ - inc_refcounts(bs, res, refcount_table, nb_clusters, + inc_refcounts(bs, res, *refcount_table, *nb_clusters, 0, s->cluster_size); /* current L1 table */ - ret = check_refcounts_l1(bs, res, refcount_table, nb_clusters, + ret = check_refcounts_l1(bs, res, *refcount_table, *nb_clusters, s->l1_table_offset, s->l1_size, CHECK_FRAG_INFO); if (ret < 0) { - goto fail; + return ret; } /* snapshots */ - for(i = 0; i < s->nb_snapshots; i++) { + for (i = 0; i < s->nb_snapshots; i++) { sn = s->snapshots + i; - ret = check_refcounts_l1(bs, res, refcount_table, nb_clusters, + ret = check_refcounts_l1(bs, res, *refcount_table, *nb_clusters, sn->l1_table_offset, sn->l1_size, 0); if (ret < 0) { - goto fail; + return ret; } } - inc_refcounts(bs, res, refcount_table, nb_clusters, + inc_refcounts(bs, res, *refcount_table, *nb_clusters, s->snapshots_offset, s->snapshots_size); /* refcount data */ - inc_refcounts(bs, res, refcount_table, nb_clusters, + inc_refcounts(bs, res, *refcount_table, *nb_clusters, s->refcount_table_offset, s->refcount_table_size * sizeof(uint64_t)); + return check_refblocks(bs, res, fix, refcount_table, nb_clusters); +} + +/* + * Checks consistency of refblocks and accounts for each refblock in + * *refcount_table. + */ +static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res, + BdrvCheckMode fix, uint16_t **refcount_table, + int64_t *nb_clusters) +{ + BDRVQcowState *s = bs->opaque; + int64_t i; + for(i = 0; i < s->refcount_table_size; i++) { uint64_t offset, cluster; offset = s->refcount_table[i]; @@ -1613,7 +1612,7 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res, continue; } - if (cluster >= nb_clusters) { + if (cluster >= *nb_clusters) { fprintf(stderr, "ERROR refcount block %" PRId64 " is outside image\n", i); res->corruptions++; @@ -1621,14 +1620,14 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res, } if (offset != 0) { - inc_refcounts(bs, res, refcount_table, nb_clusters, + inc_refcounts(bs, res, *refcount_table, *nb_clusters, offset, s->cluster_size); - if (refcount_table[cluster] != 1) { + if ((*refcount_table)[cluster] != 1) { fprintf(stderr, "%s refcount block %" PRId64 " refcount=%d\n", fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR", - i, refcount_table[cluster]); + i, (*refcount_table)[cluster]); if (fix & BDRV_FIX_ERRORS) { int64_t new_offset; @@ -1640,17 +1639,18 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res, } /* update refcounts */ - if ((new_offset >> s->cluster_bits) >= nb_clusters) { + if ((new_offset >> s->cluster_bits) >= *nb_clusters) { /* increase refcount_table size if necessary */ - int old_nb_clusters = nb_clusters; - nb_clusters = (new_offset >> s->cluster_bits) + 1; - refcount_table = g_renew(uint16_t, refcount_table, - nb_clusters); - memset(&refcount_table[old_nb_clusters], 0, (nb_clusters - - old_nb_clusters) * sizeof(uint16_t)); + int old_nb_clusters = *nb_clusters; + *nb_clusters = (new_offset >> s->cluster_bits) + 1; + *refcount_table = g_renew(uint16_t, *refcount_table, + *nb_clusters); + memset(&(*refcount_table)[old_nb_clusters], 0, + (*nb_clusters - old_nb_clusters) * + sizeof(uint16_t)); } - refcount_table[cluster]--; - inc_refcounts(bs, res, refcount_table, nb_clusters, + (*refcount_table)[cluster]--; + inc_refcounts(bs, res, *refcount_table, *nb_clusters, new_offset, s->cluster_size); res->corruptions_fixed++; @@ -1661,8 +1661,22 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res, } } - /* compare ref counts */ - for (i = 0, highest_cluster = 0; i < nb_clusters; i++) { + return 0; +} + +/* + * Compares the actual reference count for each cluster in the image against the + * refcount as reported by the refcount structures on-disk. + */ +static void compare_refcounts(BlockDriverState *bs, BdrvCheckResult *res, + BdrvCheckMode fix, int64_t *highest_cluster, + uint16_t *refcount_table, int64_t nb_clusters) +{ + BDRVQcowState *s = bs->opaque; + int64_t i; + int refcount1, refcount2, ret; + + for (i = 0, *highest_cluster = 0; i < nb_clusters; i++) { refcount1 = get_refcount(bs, i); if (refcount1 < 0) { fprintf(stderr, "Can't get refcount for cluster %" PRId64 ": %s\n", @@ -1674,11 +1688,10 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res, refcount2 = refcount_table[i]; if (refcount1 > 0 || refcount2 > 0) { - highest_cluster = i; + *highest_cluster = i; } if (refcount1 != refcount2) { - /* Check if we're allowed to fix the mismatch */ int *num_fixed = NULL; if (refcount1 > refcount2 && (fix & BDRV_FIX_LEAKS)) { @@ -1711,6 +1724,44 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res, } } } +} + +/* + * Checks an image for refcount consistency. + * + * 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 occurred. + */ +int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res, + BdrvCheckMode fix) +{ + BDRVQcowState *s = bs->opaque; + int64_t size, highest_cluster, nb_clusters; + uint16_t *refcount_table; + int ret; + + size = bdrv_getlength(bs->file); + if (size < 0) { + res->check_errors++; + return size; + } + + nb_clusters = size_to_clusters(s, size); + if (nb_clusters > INT_MAX) { + res->check_errors++; + return -EFBIG; + } + + res->bfi.total_clusters = + size_to_clusters(s, bs->total_sectors * BDRV_SECTOR_SIZE); + + ret = calculate_refcounts(bs, res, fix, &refcount_table, &nb_clusters); + if (ret < 0) { + goto fail; + } + + compare_refcounts(bs, res, fix, &highest_cluster, refcount_table, + nb_clusters); /* check OFLAG_COPIED */ ret = check_oflag_copied(bs, res, fix); -- cgit v1.2.1 From 78fb328e854542d79bebe54f3a426cba6d46dbf1 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 22 Oct 2014 14:09:31 +0200 Subject: qcow2: Use sizeof(**refcount_table) When implementing variable refcounts, we want to be able to easily find all the places in qemu which are tied to a certain refcount order. Replace sizeof(uint16_t) in the check code by sizeof(**refcount_table) so we can later find it more easily. Signed-off-by: Max Reitz Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- block/qcow2-refcount.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index e5f7876957..e8b9df9306 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -1647,7 +1647,7 @@ static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res, *nb_clusters); memset(&(*refcount_table)[old_nb_clusters], 0, (*nb_clusters - old_nb_clusters) * - sizeof(uint16_t)); + sizeof(**refcount_table)); } (*refcount_table)[cluster]--; inc_refcounts(bs, res, *refcount_table, *nb_clusters, -- cgit v1.2.1 From 057a3fe57e740e5e1cc3d62c9b8e0085e9fffa74 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 22 Oct 2014 14:09:32 +0200 Subject: qcow2: Pull check_refblocks() up MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pull check_refblocks() before calculate_refcounts() so we can drop its static declaration. Signed-off-by: Max Reitz Reviewed-by: Benoît Canet Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- block/qcow2-refcount.c | 102 ++++++++++++++++++++++++------------------------- 1 file changed, 49 insertions(+), 53 deletions(-) diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index e8b9df9306..24f297f561 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -1535,59 +1535,6 @@ done: return new_offset; } -static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res, - BdrvCheckMode fix, uint16_t **refcount_table, - int64_t *nb_clusters); - -/* - * Calculates an in-memory refcount table. - */ -static int calculate_refcounts(BlockDriverState *bs, BdrvCheckResult *res, - BdrvCheckMode fix, uint16_t **refcount_table, - int64_t *nb_clusters) -{ - BDRVQcowState *s = bs->opaque; - int64_t i; - QCowSnapshot *sn; - int ret; - - *refcount_table = g_try_new0(uint16_t, *nb_clusters); - if (*nb_clusters && *refcount_table == NULL) { - res->check_errors++; - return -ENOMEM; - } - - /* header */ - inc_refcounts(bs, res, *refcount_table, *nb_clusters, - 0, s->cluster_size); - - /* current L1 table */ - ret = check_refcounts_l1(bs, res, *refcount_table, *nb_clusters, - s->l1_table_offset, s->l1_size, CHECK_FRAG_INFO); - if (ret < 0) { - return ret; - } - - /* snapshots */ - for (i = 0; i < s->nb_snapshots; i++) { - sn = s->snapshots + i; - ret = check_refcounts_l1(bs, res, *refcount_table, *nb_clusters, - sn->l1_table_offset, sn->l1_size, 0); - if (ret < 0) { - return ret; - } - } - inc_refcounts(bs, res, *refcount_table, *nb_clusters, - s->snapshots_offset, s->snapshots_size); - - /* refcount data */ - inc_refcounts(bs, res, *refcount_table, *nb_clusters, - s->refcount_table_offset, - s->refcount_table_size * sizeof(uint64_t)); - - return check_refblocks(bs, res, fix, refcount_table, nb_clusters); -} - /* * Checks consistency of refblocks and accounts for each refblock in * *refcount_table. @@ -1664,6 +1611,55 @@ static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res, return 0; } +/* + * Calculates an in-memory refcount table. + */ +static int calculate_refcounts(BlockDriverState *bs, BdrvCheckResult *res, + BdrvCheckMode fix, uint16_t **refcount_table, + int64_t *nb_clusters) +{ + BDRVQcowState *s = bs->opaque; + int64_t i; + QCowSnapshot *sn; + int ret; + + *refcount_table = g_try_new0(uint16_t, *nb_clusters); + if (*nb_clusters && *refcount_table == NULL) { + res->check_errors++; + return -ENOMEM; + } + + /* header */ + inc_refcounts(bs, res, *refcount_table, *nb_clusters, + 0, s->cluster_size); + + /* current L1 table */ + ret = check_refcounts_l1(bs, res, *refcount_table, *nb_clusters, + s->l1_table_offset, s->l1_size, CHECK_FRAG_INFO); + if (ret < 0) { + return ret; + } + + /* snapshots */ + for (i = 0; i < s->nb_snapshots; i++) { + sn = s->snapshots + i; + ret = check_refcounts_l1(bs, res, *refcount_table, *nb_clusters, + sn->l1_table_offset, sn->l1_size, 0); + if (ret < 0) { + return ret; + } + } + inc_refcounts(bs, res, *refcount_table, *nb_clusters, + s->snapshots_offset, s->snapshots_size); + + /* refcount data */ + inc_refcounts(bs, res, *refcount_table, *nb_clusters, + s->refcount_table_offset, + s->refcount_table_size * sizeof(uint64_t)); + + return check_refblocks(bs, res, fix, refcount_table, nb_clusters); +} + /* * Compares the actual reference count for each cluster in the image against the * refcount as reported by the refcount structures on-disk. -- cgit v1.2.1 From 713d9675e0e31c627d08b6a33d3a92e4b8505b40 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 22 Oct 2014 14:09:33 +0200 Subject: qcow2: Use int64_t for in-memory reftable size Use int64_t for the entry count of the in-memory refcount table throughout the check functions. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/qcow2-refcount.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index 24f297f561..a3f4d47543 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -1084,7 +1084,7 @@ fail: static void inc_refcounts(BlockDriverState *bs, BdrvCheckResult *res, uint16_t *refcount_table, - int refcount_table_size, + int64_t refcount_table_size, int64_t offset, int64_t size) { BDRVQcowState *s = bs->opaque; @@ -1127,7 +1127,7 @@ enum { * error occurred. */ static int check_refcounts_l2(BlockDriverState *bs, BdrvCheckResult *res, - uint16_t *refcount_table, int refcount_table_size, int64_t l2_offset, + uint16_t *refcount_table, int64_t refcount_table_size, int64_t l2_offset, int flags) { BDRVQcowState *s = bs->opaque; @@ -1237,7 +1237,7 @@ fail: static int check_refcounts_l1(BlockDriverState *bs, BdrvCheckResult *res, uint16_t *refcount_table, - int refcount_table_size, + int64_t refcount_table_size, int64_t l1_table_offset, int l1_size, int flags) { -- cgit v1.2.1 From ad27390c85c50df402c7ec0d3864fc43e6559fb3 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 22 Oct 2014 14:09:34 +0200 Subject: qcow2: Split fail code in L1 and L2 checks Instead of printing out an error message, incrementing check_errors and returning a fixed -errno, just do cleanups and return -ret, with ret set by the code which threw the exception (jumped to the fail label). Also, increment check_errors on error in check_refcounts_l2(). Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/qcow2-refcount.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index a3f4d47543..59fb400e81 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -1133,14 +1133,18 @@ static int check_refcounts_l2(BlockDriverState *bs, BdrvCheckResult *res, BDRVQcowState *s = bs->opaque; uint64_t *l2_table, l2_entry; uint64_t next_contiguous_offset = 0; - int i, l2_size, nb_csectors; + int i, l2_size, nb_csectors, ret; /* Read L2 table from disk */ l2_size = s->l2_size * sizeof(uint64_t); l2_table = g_malloc(l2_size); - if (bdrv_pread(bs->file, l2_offset, l2_table, l2_size) != l2_size) + ret = bdrv_pread(bs->file, l2_offset, l2_table, l2_size); + if (ret < 0) { + fprintf(stderr, "ERROR: I/O error in check_refcounts_l2\n"); + res->check_errors++; goto fail; + } /* Do the actual checks */ for(i = 0; i < s->l2_size; i++) { @@ -1221,9 +1225,8 @@ static int check_refcounts_l2(BlockDriverState *bs, BdrvCheckResult *res, return 0; fail: - fprintf(stderr, "ERROR: I/O error in check_refcounts_l2\n"); g_free(l2_table); - return -EIO; + return ret; } /* @@ -1258,11 +1261,15 @@ static int check_refcounts_l1(BlockDriverState *bs, l1_table = g_try_malloc(l1_size2); if (l1_table == NULL) { ret = -ENOMEM; + res->check_errors++; goto fail; } - if (bdrv_pread(bs->file, l1_table_offset, - l1_table, l1_size2) != l1_size2) + ret = bdrv_pread(bs->file, l1_table_offset, l1_table, l1_size2); + if (ret < 0) { + fprintf(stderr, "ERROR: I/O error in check_refcounts_l1\n"); + res->check_errors++; goto fail; + } for(i = 0;i < l1_size; i++) be64_to_cpus(&l1_table[i]); } @@ -1295,10 +1302,8 @@ static int check_refcounts_l1(BlockDriverState *bs, return 0; fail: - fprintf(stderr, "ERROR: I/O error in check_refcounts_l1\n"); - res->check_errors++; g_free(l1_table); - return -EIO; + return ret; } /* -- cgit v1.2.1 From fef4d3d5644f984e9fa427dea4f7cfa15de9059c Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 22 Oct 2014 14:09:35 +0200 Subject: qcow2: Let inc_refcounts() return -errno As of a future patch, inc_refcounts() will have to throw errors which are generally signaled by returning -errno. Therefore, let it return an integer which is either 0 for success or -errno and handle the -errno case in all callers. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/qcow2-refcount.c | 91 +++++++++++++++++++++++++++++++++----------------- 1 file changed, 60 insertions(+), 31 deletions(-) diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index 59fb400e81..6001c852ae 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -1081,17 +1081,18 @@ fail: * * Modifies the number of errors in res. */ -static void inc_refcounts(BlockDriverState *bs, - BdrvCheckResult *res, - uint16_t *refcount_table, - int64_t refcount_table_size, - int64_t offset, int64_t size) +static int inc_refcounts(BlockDriverState *bs, + BdrvCheckResult *res, + uint16_t *refcount_table, + int64_t refcount_table_size, + int64_t offset, int64_t size) { BDRVQcowState *s = bs->opaque; uint64_t start, last, cluster_offset, k; - if (size <= 0) - return; + if (size <= 0) { + return 0; + } start = start_of_cluster(s, offset); last = start_of_cluster(s, offset + size - 1); @@ -1111,6 +1112,8 @@ static void inc_refcounts(BlockDriverState *bs, } } } + + return 0; } /* Flags for check_refcounts_l1() and check_refcounts_l2() */ @@ -1165,8 +1168,11 @@ static int check_refcounts_l2(BlockDriverState *bs, BdrvCheckResult *res, nb_csectors = ((l2_entry >> s->csize_shift) & s->csize_mask) + 1; l2_entry &= s->cluster_offset_mask; - inc_refcounts(bs, res, refcount_table, refcount_table_size, - l2_entry & ~511, nb_csectors * 512); + ret = inc_refcounts(bs, res, refcount_table, refcount_table_size, + l2_entry & ~511, nb_csectors * 512); + if (ret < 0) { + goto fail; + } if (flags & CHECK_FRAG_INFO) { res->bfi.allocated_clusters++; @@ -1201,8 +1207,11 @@ static int check_refcounts_l2(BlockDriverState *bs, BdrvCheckResult *res, } /* Mark cluster as used */ - inc_refcounts(bs, res, refcount_table,refcount_table_size, - offset, s->cluster_size); + ret = inc_refcounts(bs, res, refcount_table, refcount_table_size, + offset, s->cluster_size); + if (ret < 0) { + goto fail; + } /* Correct offsets are cluster aligned */ if (offset_into_cluster(s, offset)) { @@ -1245,19 +1254,20 @@ static int check_refcounts_l1(BlockDriverState *bs, int flags) { BDRVQcowState *s = bs->opaque; - uint64_t *l1_table, l2_offset, l1_size2; + uint64_t *l1_table = NULL, l2_offset, l1_size2; int i, ret; l1_size2 = l1_size * sizeof(uint64_t); /* Mark L1 table as used */ - inc_refcounts(bs, res, refcount_table, refcount_table_size, - l1_table_offset, l1_size2); + ret = inc_refcounts(bs, res, refcount_table, refcount_table_size, + l1_table_offset, l1_size2); + if (ret < 0) { + goto fail; + } /* Read L1 table entries from disk */ - if (l1_size2 == 0) { - l1_table = NULL; - } else { + if (l1_size2 > 0) { l1_table = g_try_malloc(l1_size2); if (l1_table == NULL) { ret = -ENOMEM; @@ -1280,8 +1290,11 @@ static int check_refcounts_l1(BlockDriverState *bs, if (l2_offset) { /* Mark L2 table as used */ l2_offset &= L1E_OFFSET_MASK; - inc_refcounts(bs, res, refcount_table, refcount_table_size, - l2_offset, s->cluster_size); + ret = inc_refcounts(bs, res, refcount_table, refcount_table_size, + l2_offset, s->cluster_size); + if (ret < 0) { + goto fail; + } /* L2 tables are cluster aligned */ if (offset_into_cluster(s, l2_offset)) { @@ -1550,6 +1563,7 @@ static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res, { BDRVQcowState *s = bs->opaque; int64_t i; + int ret; for(i = 0; i < s->refcount_table_size; i++) { uint64_t offset, cluster; @@ -1572,8 +1586,11 @@ static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res, } if (offset != 0) { - inc_refcounts(bs, res, *refcount_table, *nb_clusters, - offset, s->cluster_size); + ret = inc_refcounts(bs, res, *refcount_table, *nb_clusters, + offset, s->cluster_size); + if (ret < 0) { + return ret; + } if ((*refcount_table)[cluster] != 1) { fprintf(stderr, "%s refcount block %" PRId64 " refcount=%d\n", @@ -1602,8 +1619,11 @@ static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res, sizeof(**refcount_table)); } (*refcount_table)[cluster]--; - inc_refcounts(bs, res, *refcount_table, *nb_clusters, - new_offset, s->cluster_size); + ret = inc_refcounts(bs, res, *refcount_table, *nb_clusters, + new_offset, s->cluster_size); + if (ret < 0) { + return ret; + } res->corruptions_fixed++; } else { @@ -1635,8 +1655,11 @@ static int calculate_refcounts(BlockDriverState *bs, BdrvCheckResult *res, } /* header */ - inc_refcounts(bs, res, *refcount_table, *nb_clusters, - 0, s->cluster_size); + ret = inc_refcounts(bs, res, *refcount_table, *nb_clusters, + 0, s->cluster_size); + if (ret < 0) { + return ret; + } /* current L1 table */ ret = check_refcounts_l1(bs, res, *refcount_table, *nb_clusters, @@ -1649,18 +1672,24 @@ static int calculate_refcounts(BlockDriverState *bs, BdrvCheckResult *res, for (i = 0; i < s->nb_snapshots; i++) { sn = s->snapshots + i; ret = check_refcounts_l1(bs, res, *refcount_table, *nb_clusters, - sn->l1_table_offset, sn->l1_size, 0); + sn->l1_table_offset, sn->l1_size, 0); if (ret < 0) { return ret; } } - inc_refcounts(bs, res, *refcount_table, *nb_clusters, - s->snapshots_offset, s->snapshots_size); + ret = inc_refcounts(bs, res, *refcount_table, *nb_clusters, + s->snapshots_offset, s->snapshots_size); + if (ret < 0) { + return ret; + } /* refcount data */ - inc_refcounts(bs, res, *refcount_table, *nb_clusters, - s->refcount_table_offset, - s->refcount_table_size * sizeof(uint64_t)); + ret = inc_refcounts(bs, res, *refcount_table, *nb_clusters, + s->refcount_table_offset, + s->refcount_table_size * sizeof(uint64_t)); + if (ret < 0) { + return ret; + } return check_refblocks(bs, res, fix, refcount_table, nb_clusters); } -- cgit v1.2.1 From 641bb63cd6b003ab0ca2e312a014449037d71647 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 22 Oct 2014 14:09:36 +0200 Subject: qcow2: Let inc_refcounts() resize the reftable Now that the refcount table can be passed around by reference, do that for inc_refcounts() (and subsequently check_refcounts_l1() and check_refcounts_l2()) and use it for resizing it when a cluster after the image end is encountered. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/qcow2-refcount.c | 57 +++++++++++++++++++++++++++++++------------------- 1 file changed, 35 insertions(+), 22 deletions(-) diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index 6001c852ae..d653124cf2 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -1083,8 +1083,8 @@ fail: */ static int inc_refcounts(BlockDriverState *bs, BdrvCheckResult *res, - uint16_t *refcount_table, - int64_t refcount_table_size, + uint16_t **refcount_table, + int64_t *refcount_table_size, int64_t offset, int64_t size) { BDRVQcowState *s = bs->opaque; @@ -1099,17 +1099,30 @@ static int inc_refcounts(BlockDriverState *bs, for(cluster_offset = start; cluster_offset <= last; cluster_offset += s->cluster_size) { k = cluster_offset >> s->cluster_bits; - 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); - res->corruptions++; + if (k >= *refcount_table_size) { + int64_t old_refcount_table_size = *refcount_table_size; + uint16_t *new_refcount_table; + + *refcount_table_size = k + 1; + new_refcount_table = g_try_realloc(*refcount_table, + *refcount_table_size * + sizeof(**refcount_table)); + if (!new_refcount_table) { + *refcount_table_size = old_refcount_table_size; + res->check_errors++; + return -ENOMEM; } + *refcount_table = new_refcount_table; + + memset(*refcount_table + old_refcount_table_size, 0, + (*refcount_table_size - old_refcount_table_size) * + sizeof(**refcount_table)); + } + + if (++(*refcount_table)[k] == 0) { + fprintf(stderr, "ERROR: overflow cluster offset=0x%" PRIx64 + "\n", cluster_offset); + res->corruptions++; } } @@ -1130,7 +1143,7 @@ enum { * error occurred. */ static int check_refcounts_l2(BlockDriverState *bs, BdrvCheckResult *res, - uint16_t *refcount_table, int64_t refcount_table_size, int64_t l2_offset, + uint16_t **refcount_table, int64_t *refcount_table_size, int64_t l2_offset, int flags) { BDRVQcowState *s = bs->opaque; @@ -1248,8 +1261,8 @@ fail: */ static int check_refcounts_l1(BlockDriverState *bs, BdrvCheckResult *res, - uint16_t *refcount_table, - int64_t refcount_table_size, + uint16_t **refcount_table, + int64_t *refcount_table_size, int64_t l1_table_offset, int l1_size, int flags) { @@ -1586,7 +1599,7 @@ static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res, } if (offset != 0) { - ret = inc_refcounts(bs, res, *refcount_table, *nb_clusters, + ret = inc_refcounts(bs, res, refcount_table, nb_clusters, offset, s->cluster_size); if (ret < 0) { return ret; @@ -1619,7 +1632,7 @@ static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res, sizeof(**refcount_table)); } (*refcount_table)[cluster]--; - ret = inc_refcounts(bs, res, *refcount_table, *nb_clusters, + ret = inc_refcounts(bs, res, refcount_table, nb_clusters, new_offset, s->cluster_size); if (ret < 0) { return ret; @@ -1655,14 +1668,14 @@ static int calculate_refcounts(BlockDriverState *bs, BdrvCheckResult *res, } /* header */ - ret = inc_refcounts(bs, res, *refcount_table, *nb_clusters, + ret = inc_refcounts(bs, res, refcount_table, nb_clusters, 0, s->cluster_size); if (ret < 0) { return ret; } /* current L1 table */ - ret = check_refcounts_l1(bs, res, *refcount_table, *nb_clusters, + ret = check_refcounts_l1(bs, res, refcount_table, nb_clusters, s->l1_table_offset, s->l1_size, CHECK_FRAG_INFO); if (ret < 0) { return ret; @@ -1671,20 +1684,20 @@ static int calculate_refcounts(BlockDriverState *bs, BdrvCheckResult *res, /* snapshots */ for (i = 0; i < s->nb_snapshots; i++) { sn = s->snapshots + i; - ret = check_refcounts_l1(bs, res, *refcount_table, *nb_clusters, + ret = check_refcounts_l1(bs, res, refcount_table, nb_clusters, sn->l1_table_offset, sn->l1_size, 0); if (ret < 0) { return ret; } } - ret = inc_refcounts(bs, res, *refcount_table, *nb_clusters, + ret = inc_refcounts(bs, res, refcount_table, nb_clusters, s->snapshots_offset, s->snapshots_size); if (ret < 0) { return ret; } /* refcount data */ - ret = inc_refcounts(bs, res, *refcount_table, *nb_clusters, + ret = inc_refcounts(bs, res, refcount_table, nb_clusters, s->refcount_table_offset, s->refcount_table_size * sizeof(uint64_t)); if (ret < 0) { -- cgit v1.2.1 From 9696df219a71c6608f058ade8873d6d0b4e352fe Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 22 Oct 2014 14:09:37 +0200 Subject: qcow2: Reuse refcount table in calculate_refcounts() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We will later call calculate_refcounts multiple times, so reuse the refcount table if possible. Signed-off-by: Max Reitz Reviewed-by: Benoît Canet Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- block/qcow2-refcount.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index d653124cf2..c92e1fc62c 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -1661,10 +1661,12 @@ static int calculate_refcounts(BlockDriverState *bs, BdrvCheckResult *res, QCowSnapshot *sn; int ret; - *refcount_table = g_try_new0(uint16_t, *nb_clusters); - if (*nb_clusters && *refcount_table == NULL) { - res->check_errors++; - return -ENOMEM; + if (!*refcount_table) { + *refcount_table = g_try_new0(uint16_t, *nb_clusters); + if (*nb_clusters && *refcount_table == NULL) { + res->check_errors++; + return -ENOMEM; + } } /* header */ @@ -1780,7 +1782,7 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res, { BDRVQcowState *s = bs->opaque; int64_t size, highest_cluster, nb_clusters; - uint16_t *refcount_table; + uint16_t *refcount_table = NULL; int ret; size = bdrv_getlength(bs->file); -- cgit v1.2.1 From 001c158defb65e88e6c50c85d6f20501f7149ddd Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 22 Oct 2014 14:09:38 +0200 Subject: qcow2: Fix refcount blocks beyond image end If the qcow2 check function detects a refcount block located beyond the image end, grow the image appropriately. This cannot break anything and is the logical fix for such a case. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/qcow2-refcount.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 63 insertions(+), 4 deletions(-) diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index c92e1fc62c..b87eafcc31 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -1575,7 +1575,7 @@ static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res, int64_t *nb_clusters) { BDRVQcowState *s = bs->opaque; - int64_t i; + int64_t i, size; int ret; for(i = 0; i < s->refcount_table_size; i++) { @@ -1592,9 +1592,68 @@ static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res, } if (cluster >= *nb_clusters) { - fprintf(stderr, "ERROR refcount block %" PRId64 - " is outside image\n", i); - res->corruptions++; + fprintf(stderr, "%s refcount block %" PRId64 " is outside image\n", + fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR", i); + + if (fix & BDRV_FIX_ERRORS) { + int64_t old_nb_clusters = *nb_clusters; + uint16_t *new_refcount_table; + + if (offset > INT64_MAX - s->cluster_size) { + ret = -EINVAL; + goto resize_fail; + } + + ret = bdrv_truncate(bs->file, offset + s->cluster_size); + if (ret < 0) { + goto resize_fail; + } + size = bdrv_getlength(bs->file); + if (size < 0) { + ret = size; + goto resize_fail; + } + + *nb_clusters = size_to_clusters(s, size); + assert(*nb_clusters >= old_nb_clusters); + + new_refcount_table = g_try_realloc(*refcount_table, + *nb_clusters * + sizeof(**refcount_table)); + if (!new_refcount_table) { + *nb_clusters = old_nb_clusters; + res->check_errors++; + return -ENOMEM; + } + *refcount_table = new_refcount_table; + + memset(*refcount_table + old_nb_clusters, 0, + (*nb_clusters - old_nb_clusters) * + sizeof(**refcount_table)); + + if (cluster >= *nb_clusters) { + ret = -EINVAL; + goto resize_fail; + } + + res->corruptions_fixed++; + ret = inc_refcounts(bs, res, refcount_table, nb_clusters, + offset, s->cluster_size); + if (ret < 0) { + return ret; + } + /* No need to check whether the refcount is now greater than 1: + * This area was just allocated and zeroed, so it can only be + * exactly 1 after inc_refcounts() */ + continue; + +resize_fail: + res->corruptions++; + fprintf(stderr, "ERROR could not resize image: %s\n", + strerror(-ret)); + } else { + res->corruptions++; + } continue; } -- cgit v1.2.1 From f307b2558f61e068ce514f2dde2cad74c62036d6 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 22 Oct 2014 14:09:39 +0200 Subject: qcow2: Do not perform potentially damaging repairs If a referenced cluster has a refcount of 0, increasing its refcount may result in clusters being allocated for the refcount structures. This may overwrite the referenced cluster, therefore we cannot simply increase the refcount then. In such cases, we can either try to replicate all the refcount operations solely for the check operation, basing the allocations on the in-memory refcount table; or we can simply rebuild the whole refcount structure based on the in-memory refcount table. Since the latter will be much easier, do that. To prepare for this, introduce a "rebuild" boolean which should be set to true whenever a fix is rather dangerous or too complicated using the current refcount structures. Another example for this is refcount blocks being referenced more than once. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/qcow2-refcount.c | 188 +++++++------------------------------------------ 1 file changed, 27 insertions(+), 161 deletions(-) diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index b87eafcc31..e96466635a 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -1451,128 +1451,13 @@ fail: return ret; } -/* - * Writes one sector of the refcount table to the disk - */ -#define RT_ENTRIES_PER_SECTOR (512 / sizeof(uint64_t)) -static int write_reftable_entry(BlockDriverState *bs, int rt_index) -{ - BDRVQcowState *s = bs->opaque; - uint64_t buf[RT_ENTRIES_PER_SECTOR]; - int rt_start_index; - int i, ret; - - rt_start_index = rt_index & ~(RT_ENTRIES_PER_SECTOR - 1); - for (i = 0; i < RT_ENTRIES_PER_SECTOR; i++) { - buf[i] = cpu_to_be64(s->refcount_table[rt_start_index + i]); - } - - ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_REFCOUNT_TABLE, - s->refcount_table_offset + rt_start_index * sizeof(uint64_t), - sizeof(buf)); - if (ret < 0) { - return ret; - } - - BLKDBG_EVENT(bs->file, BLKDBG_REFTABLE_UPDATE); - ret = bdrv_pwrite_sync(bs->file, s->refcount_table_offset + - rt_start_index * sizeof(uint64_t), buf, sizeof(buf)); - if (ret < 0) { - return ret; - } - - return 0; -} - -/* - * Allocates a new cluster for the given refcount block (represented by its - * offset in the image file) and copies the current content there. This function - * does _not_ decrement the reference count for the currently occupied cluster. - * - * This function prints an informative message to stderr on error (and returns - * -errno); on success, the offset of the newly allocated cluster is returned. - */ -static int64_t realloc_refcount_block(BlockDriverState *bs, int reftable_index, - uint64_t offset) -{ - BDRVQcowState *s = bs->opaque; - int64_t new_offset = 0; - void *refcount_block = NULL; - int ret; - - /* allocate new refcount block */ - new_offset = qcow2_alloc_clusters(bs, s->cluster_size); - if (new_offset < 0) { - fprintf(stderr, "Could not allocate new cluster: %s\n", - strerror(-new_offset)); - ret = new_offset; - goto done; - } - - /* fetch current refcount block content */ - ret = qcow2_cache_get(bs, s->refcount_block_cache, offset, &refcount_block); - if (ret < 0) { - fprintf(stderr, "Could not fetch refcount block: %s\n", strerror(-ret)); - goto fail_free_cluster; - } - - /* new block has not yet been entered into refcount table, therefore it is - * no refcount block yet (regarding this check) */ - ret = qcow2_pre_write_overlap_check(bs, 0, new_offset, s->cluster_size); - if (ret < 0) { - fprintf(stderr, "Could not write refcount block; metadata overlap " - "check failed: %s\n", strerror(-ret)); - /* the image will be marked corrupt, so don't even attempt on freeing - * the cluster */ - goto done; - } - - /* write to new block */ - ret = bdrv_write(bs->file, new_offset / BDRV_SECTOR_SIZE, refcount_block, - s->cluster_sectors); - if (ret < 0) { - fprintf(stderr, "Could not write refcount block: %s\n", strerror(-ret)); - goto fail_free_cluster; - } - - /* update refcount table */ - assert(!offset_into_cluster(s, new_offset)); - s->refcount_table[reftable_index] = new_offset; - ret = write_reftable_entry(bs, reftable_index); - if (ret < 0) { - fprintf(stderr, "Could not update refcount table: %s\n", - strerror(-ret)); - goto fail_free_cluster; - } - - goto done; - -fail_free_cluster: - qcow2_free_clusters(bs, new_offset, s->cluster_size, QCOW2_DISCARD_OTHER); - -done: - if (refcount_block) { - /* This should never fail, as it would only do so if the given refcount - * block cannot be found in the cache. As this is impossible as long as - * there are no bugs, assert the success. */ - int tmp = qcow2_cache_put(bs, s->refcount_block_cache, &refcount_block); - assert(tmp == 0); - } - - if (ret < 0) { - return ret; - } - - return new_offset; -} - /* * Checks consistency of refblocks and accounts for each refblock in * *refcount_table. */ static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res, - BdrvCheckMode fix, uint16_t **refcount_table, - int64_t *nb_clusters) + BdrvCheckMode fix, bool *rebuild, + uint16_t **refcount_table, int64_t *nb_clusters) { BDRVQcowState *s = bs->opaque; int64_t i, size; @@ -1588,6 +1473,7 @@ static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res, fprintf(stderr, "ERROR refcount block %" PRId64 " is not " "cluster aligned; refcount table entry corrupted\n", i); res->corruptions++; + *rebuild = true; continue; } @@ -1649,6 +1535,7 @@ static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res, resize_fail: res->corruptions++; + *rebuild = true; fprintf(stderr, "ERROR could not resize image: %s\n", strerror(-ret)); } else { @@ -1664,43 +1551,10 @@ resize_fail: return ret; } if ((*refcount_table)[cluster] != 1) { - fprintf(stderr, "%s refcount block %" PRId64 - " refcount=%d\n", - fix & BDRV_FIX_ERRORS ? "Repairing" : - "ERROR", - i, (*refcount_table)[cluster]); - - if (fix & BDRV_FIX_ERRORS) { - int64_t new_offset; - - new_offset = realloc_refcount_block(bs, i, offset); - if (new_offset < 0) { - res->corruptions++; - continue; - } - - /* update refcounts */ - if ((new_offset >> s->cluster_bits) >= *nb_clusters) { - /* increase refcount_table size if necessary */ - int old_nb_clusters = *nb_clusters; - *nb_clusters = (new_offset >> s->cluster_bits) + 1; - *refcount_table = g_renew(uint16_t, *refcount_table, - *nb_clusters); - memset(&(*refcount_table)[old_nb_clusters], 0, - (*nb_clusters - old_nb_clusters) * - sizeof(**refcount_table)); - } - (*refcount_table)[cluster]--; - ret = inc_refcounts(bs, res, refcount_table, nb_clusters, - new_offset, s->cluster_size); - if (ret < 0) { - return ret; - } - - res->corruptions_fixed++; - } else { - res->corruptions++; - } + fprintf(stderr, "ERROR refcount block %" PRId64 + " refcount=%d\n", i, (*refcount_table)[cluster]); + res->corruptions++; + *rebuild = true; } } } @@ -1712,8 +1566,8 @@ resize_fail: * Calculates an in-memory refcount table. */ static int calculate_refcounts(BlockDriverState *bs, BdrvCheckResult *res, - BdrvCheckMode fix, uint16_t **refcount_table, - int64_t *nb_clusters) + BdrvCheckMode fix, bool *rebuild, + uint16_t **refcount_table, int64_t *nb_clusters) { BDRVQcowState *s = bs->opaque; int64_t i; @@ -1765,7 +1619,7 @@ static int calculate_refcounts(BlockDriverState *bs, BdrvCheckResult *res, return ret; } - return check_refblocks(bs, res, fix, refcount_table, nb_clusters); + return check_refblocks(bs, res, fix, rebuild, refcount_table, nb_clusters); } /* @@ -1773,7 +1627,8 @@ static int calculate_refcounts(BlockDriverState *bs, BdrvCheckResult *res, * refcount as reported by the refcount structures on-disk. */ static void compare_refcounts(BlockDriverState *bs, BdrvCheckResult *res, - BdrvCheckMode fix, int64_t *highest_cluster, + BdrvCheckMode fix, bool *rebuild, + int64_t *highest_cluster, uint16_t *refcount_table, int64_t nb_clusters) { BDRVQcowState *s = bs->opaque; @@ -1798,7 +1653,9 @@ static void compare_refcounts(BlockDriverState *bs, BdrvCheckResult *res, if (refcount1 != refcount2) { /* Check if we're allowed to fix the mismatch */ int *num_fixed = NULL; - if (refcount1 > refcount2 && (fix & BDRV_FIX_LEAKS)) { + if (refcount1 == 0) { + *rebuild = true; + } else if (refcount1 > refcount2 && (fix & BDRV_FIX_LEAKS)) { num_fixed = &res->leaks_fixed; } else if (refcount1 < refcount2 && (fix & BDRV_FIX_ERRORS)) { num_fixed = &res->corruptions_fixed; @@ -1842,6 +1699,7 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res, BDRVQcowState *s = bs->opaque; int64_t size, highest_cluster, nb_clusters; uint16_t *refcount_table = NULL; + bool rebuild = false; int ret; size = bdrv_getlength(bs->file); @@ -1859,14 +1717,22 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res, res->bfi.total_clusters = size_to_clusters(s, bs->total_sectors * BDRV_SECTOR_SIZE); - ret = calculate_refcounts(bs, res, fix, &refcount_table, &nb_clusters); + ret = calculate_refcounts(bs, res, fix, &rebuild, &refcount_table, + &nb_clusters); if (ret < 0) { goto fail; } - compare_refcounts(bs, res, fix, &highest_cluster, refcount_table, + compare_refcounts(bs, res, fix, &rebuild, &highest_cluster, refcount_table, nb_clusters); + if (rebuild) { + fprintf(stderr, "ERROR need to rebuild refcount structures\n"); + res->check_errors++; + /* Just carry on, the rest does not rely on the on-disk refcount + * structures */ + } + /* check OFLAG_COPIED */ ret = check_oflag_copied(bs, res, fix); if (ret < 0) { -- cgit v1.2.1 From c7c0681bc8a781e0319b7cf969b904dfe50d083e Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 22 Oct 2014 14:09:40 +0200 Subject: qcow2: Rebuild refcount structure during check The previous commit introduced the "rebuild" variable to qcow2's implementation of the image consistency check. Now make use of this by adding a function which creates a completely new refcount structure based solely on the in-memory information gathered before. The old refcount structure will be leaked, however. This leak will be dealt with in a follow-up commit. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/qcow2-refcount.c | 311 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 305 insertions(+), 6 deletions(-) diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index e96466635a..9bfc75e9cf 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -1687,6 +1687,285 @@ static void compare_refcounts(BlockDriverState *bs, BdrvCheckResult *res, } } +/* + * Allocates clusters using an in-memory refcount table (IMRT) in contrast to + * the on-disk refcount structures. + * + * On input, *first_free_cluster tells where to start looking, and need not + * actually be a free cluster; the returned offset will not be before that + * cluster. On output, *first_free_cluster points to the first gap found, even + * if that gap was too small to be used as the returned offset. + * + * Note that *first_free_cluster is a cluster index whereas the return value is + * an offset. + */ +static int64_t alloc_clusters_imrt(BlockDriverState *bs, + int cluster_count, + uint16_t **refcount_table, + int64_t *imrt_nb_clusters, + int64_t *first_free_cluster) +{ + BDRVQcowState *s = bs->opaque; + int64_t cluster = *first_free_cluster, i; + bool first_gap = true; + int contiguous_free_clusters; + + /* Starting at *first_free_cluster, find a range of at least cluster_count + * continuously free clusters */ + for (contiguous_free_clusters = 0; + cluster < *imrt_nb_clusters && + contiguous_free_clusters < cluster_count; + cluster++) + { + if (!(*refcount_table)[cluster]) { + contiguous_free_clusters++; + if (first_gap) { + /* If this is the first free cluster found, update + * *first_free_cluster accordingly */ + *first_free_cluster = cluster; + first_gap = false; + } + } else if (contiguous_free_clusters) { + contiguous_free_clusters = 0; + } + } + + /* If contiguous_free_clusters is greater than zero, it contains the number + * of continuously free clusters until the current cluster; the first free + * cluster in the current "gap" is therefore + * cluster - contiguous_free_clusters */ + + /* If no such range could be found, grow the in-memory refcount table + * accordingly to append free clusters at the end of the image */ + if (contiguous_free_clusters < cluster_count) { + int64_t old_imrt_nb_clusters = *imrt_nb_clusters; + uint16_t *new_refcount_table; + + /* contiguous_free_clusters clusters are already empty at the image end; + * we need cluster_count clusters; therefore, we have to allocate + * cluster_count - contiguous_free_clusters new clusters at the end of + * the image (which is the current value of cluster; note that cluster + * may exceed old_imrt_nb_clusters if *first_free_cluster pointed beyond + * the image end) */ + *imrt_nb_clusters = cluster + cluster_count - contiguous_free_clusters; + new_refcount_table = g_try_realloc(*refcount_table, + *imrt_nb_clusters * + sizeof(**refcount_table)); + if (!new_refcount_table) { + *imrt_nb_clusters = old_imrt_nb_clusters; + return -ENOMEM; + } + *refcount_table = new_refcount_table; + + memset(*refcount_table + old_imrt_nb_clusters, 0, + (*imrt_nb_clusters - old_imrt_nb_clusters) * + sizeof(**refcount_table)); + } + + /* Go back to the first free cluster */ + cluster -= contiguous_free_clusters; + for (i = 0; i < cluster_count; i++) { + (*refcount_table)[cluster + i] = 1; + } + + return cluster << s->cluster_bits; +} + +/* + * Creates a new refcount structure based solely on the in-memory information + * given through *refcount_table. All necessary allocations will be reflected + * in that array. + * + * On success, the old refcount structure is leaked (it will be covered by the + * new refcount structure). + */ +static int rebuild_refcount_structure(BlockDriverState *bs, + BdrvCheckResult *res, + uint16_t **refcount_table, + int64_t *nb_clusters) +{ + BDRVQcowState *s = bs->opaque; + int64_t first_free_cluster = 0, reftable_offset = -1, cluster = 0; + int64_t refblock_offset, refblock_start, refblock_index; + uint32_t reftable_size = 0; + uint64_t *on_disk_reftable = NULL; + uint16_t *on_disk_refblock; + int i, ret = 0; + struct { + uint64_t reftable_offset; + uint32_t reftable_clusters; + } QEMU_PACKED reftable_offset_and_clusters; + + qcow2_cache_empty(bs, s->refcount_block_cache); + +write_refblocks: + for (; cluster < *nb_clusters; cluster++) { + if (!(*refcount_table)[cluster]) { + continue; + } + + refblock_index = cluster >> s->refcount_block_bits; + refblock_start = refblock_index << s->refcount_block_bits; + + /* Don't allocate a cluster in a refblock already written to disk */ + if (first_free_cluster < refblock_start) { + first_free_cluster = refblock_start; + } + refblock_offset = alloc_clusters_imrt(bs, 1, refcount_table, + nb_clusters, &first_free_cluster); + if (refblock_offset < 0) { + fprintf(stderr, "ERROR allocating refblock: %s\n", + strerror(-refblock_offset)); + res->check_errors++; + ret = refblock_offset; + goto fail; + } + + if (reftable_size <= refblock_index) { + uint32_t old_reftable_size = reftable_size; + uint64_t *new_on_disk_reftable; + + reftable_size = ROUND_UP((refblock_index + 1) * sizeof(uint64_t), + s->cluster_size) / sizeof(uint64_t); + new_on_disk_reftable = g_try_realloc(on_disk_reftable, + reftable_size * + sizeof(uint64_t)); + if (!new_on_disk_reftable) { + res->check_errors++; + ret = -ENOMEM; + goto fail; + } + on_disk_reftable = new_on_disk_reftable; + + memset(on_disk_reftable + old_reftable_size, 0, + (reftable_size - old_reftable_size) * sizeof(uint64_t)); + + /* The offset we have for the reftable is now no longer valid; + * this will leak that range, but we can easily fix that by running + * a leak-fixing check after this rebuild operation */ + reftable_offset = -1; + } + on_disk_reftable[refblock_index] = refblock_offset; + + /* If this is apparently the last refblock (for now), try to squeeze the + * reftable in */ + if (refblock_index == (*nb_clusters - 1) >> s->refcount_block_bits && + reftable_offset < 0) + { + uint64_t reftable_clusters = size_to_clusters(s, reftable_size * + sizeof(uint64_t)); + reftable_offset = alloc_clusters_imrt(bs, reftable_clusters, + refcount_table, nb_clusters, + &first_free_cluster); + if (reftable_offset < 0) { + fprintf(stderr, "ERROR allocating reftable: %s\n", + strerror(-reftable_offset)); + res->check_errors++; + ret = reftable_offset; + goto fail; + } + } + + ret = qcow2_pre_write_overlap_check(bs, 0, refblock_offset, + s->cluster_size); + if (ret < 0) { + fprintf(stderr, "ERROR writing refblock: %s\n", strerror(-ret)); + goto fail; + } + + on_disk_refblock = qemu_blockalign0(bs->file, s->cluster_size); + for (i = 0; i < s->refcount_block_size && + refblock_start + i < *nb_clusters; i++) + { + on_disk_refblock[i] = + cpu_to_be16((*refcount_table)[refblock_start + i]); + } + + ret = bdrv_write(bs->file, refblock_offset / BDRV_SECTOR_SIZE, + (void *)on_disk_refblock, s->cluster_sectors); + qemu_vfree(on_disk_refblock); + if (ret < 0) { + fprintf(stderr, "ERROR writing refblock: %s\n", strerror(-ret)); + goto fail; + } + + /* Go to the end of this refblock */ + cluster = refblock_start + s->refcount_block_size - 1; + } + + if (reftable_offset < 0) { + uint64_t post_refblock_start, reftable_clusters; + + post_refblock_start = ROUND_UP(*nb_clusters, s->refcount_block_size); + reftable_clusters = size_to_clusters(s, + reftable_size * sizeof(uint64_t)); + /* Not pretty but simple */ + if (first_free_cluster < post_refblock_start) { + first_free_cluster = post_refblock_start; + } + reftable_offset = alloc_clusters_imrt(bs, reftable_clusters, + refcount_table, nb_clusters, + &first_free_cluster); + if (reftable_offset < 0) { + fprintf(stderr, "ERROR allocating reftable: %s\n", + strerror(-reftable_offset)); + res->check_errors++; + ret = reftable_offset; + goto fail; + } + + goto write_refblocks; + } + + assert(on_disk_reftable); + + for (refblock_index = 0; refblock_index < reftable_size; refblock_index++) { + cpu_to_be64s(&on_disk_reftable[refblock_index]); + } + + ret = qcow2_pre_write_overlap_check(bs, 0, reftable_offset, + reftable_size * sizeof(uint64_t)); + if (ret < 0) { + fprintf(stderr, "ERROR writing reftable: %s\n", strerror(-ret)); + goto fail; + } + + assert(reftable_size < INT_MAX / sizeof(uint64_t)); + ret = bdrv_pwrite(bs->file, reftable_offset, on_disk_reftable, + reftable_size * sizeof(uint64_t)); + if (ret < 0) { + fprintf(stderr, "ERROR writing reftable: %s\n", strerror(-ret)); + goto fail; + } + + /* Enter new reftable into the image header */ + cpu_to_be64w(&reftable_offset_and_clusters.reftable_offset, + reftable_offset); + cpu_to_be32w(&reftable_offset_and_clusters.reftable_clusters, + size_to_clusters(s, reftable_size * sizeof(uint64_t))); + ret = bdrv_pwrite_sync(bs->file, offsetof(QCowHeader, + refcount_table_offset), + &reftable_offset_and_clusters, + sizeof(reftable_offset_and_clusters)); + if (ret < 0) { + fprintf(stderr, "ERROR setting reftable: %s\n", strerror(-ret)); + goto fail; + } + + for (refblock_index = 0; refblock_index < reftable_size; refblock_index++) { + be64_to_cpus(&on_disk_reftable[refblock_index]); + } + s->refcount_table = on_disk_reftable; + s->refcount_table_offset = reftable_offset; + s->refcount_table_size = reftable_size; + + return 0; + +fail: + g_free(on_disk_reftable); + return ret; +} + /* * Checks an image for refcount consistency. * @@ -1697,6 +1976,7 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res, BdrvCheckMode fix) { BDRVQcowState *s = bs->opaque; + BdrvCheckResult pre_compare_res; int64_t size, highest_cluster, nb_clusters; uint16_t *refcount_table = NULL; bool rebuild = false; @@ -1723,14 +2003,33 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res, goto fail; } - compare_refcounts(bs, res, fix, &rebuild, &highest_cluster, refcount_table, + /* In case we don't need to rebuild the refcount structure (but want to fix + * something), this function is immediately called again, in which case the + * result should be ignored */ + pre_compare_res = *res; + compare_refcounts(bs, res, 0, &rebuild, &highest_cluster, refcount_table, nb_clusters); - if (rebuild) { - fprintf(stderr, "ERROR need to rebuild refcount structures\n"); - res->check_errors++; - /* Just carry on, the rest does not rely on the on-disk refcount - * structures */ + if (rebuild && (fix & BDRV_FIX_ERRORS)) { + fprintf(stderr, "Rebuilding refcount structure\n"); + ret = rebuild_refcount_structure(bs, res, &refcount_table, + &nb_clusters); + if (ret < 0) { + goto fail; + } + } else if (fix) { + if (rebuild) { + fprintf(stderr, "ERROR need to rebuild refcount structures\n"); + res->check_errors++; + ret = -EIO; + goto fail; + } + + if (res->leaks || res->corruptions) { + *res = pre_compare_res; + compare_refcounts(bs, res, fix, &rebuild, &highest_cluster, + refcount_table, nb_clusters); + } } /* check OFLAG_COPIED */ -- cgit v1.2.1 From 791230d8bbd5c09d80845755a54074cd2d8b5a22 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 22 Oct 2014 14:09:41 +0200 Subject: qcow2: Clean up after refcount rebuild Because the old refcount structure will be leaked after having rebuilt it, we need to recalculate the refcounts and run a leak-fixing operation afterwards (if leaks should be fixed at all). Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/qcow2-refcount.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index 9bfc75e9cf..6f1b118112 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -2011,12 +2011,57 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res, nb_clusters); if (rebuild && (fix & BDRV_FIX_ERRORS)) { + BdrvCheckResult old_res = *res; + int fresh_leaks = 0; + fprintf(stderr, "Rebuilding refcount structure\n"); ret = rebuild_refcount_structure(bs, res, &refcount_table, &nb_clusters); if (ret < 0) { goto fail; } + + res->corruptions = 0; + res->leaks = 0; + + /* Because the old reftable has been exchanged for a new one the + * references have to be recalculated */ + rebuild = false; + memset(refcount_table, 0, nb_clusters * sizeof(uint16_t)); + ret = calculate_refcounts(bs, res, 0, &rebuild, &refcount_table, + &nb_clusters); + if (ret < 0) { + goto fail; + } + + if (fix & BDRV_FIX_LEAKS) { + /* The old refcount structures are now leaked, fix it; the result + * can be ignored, aside from leaks which were introduced by + * rebuild_refcount_structure() that could not be fixed */ + BdrvCheckResult saved_res = *res; + *res = (BdrvCheckResult){ 0 }; + + compare_refcounts(bs, res, BDRV_FIX_LEAKS, &rebuild, + &highest_cluster, refcount_table, nb_clusters); + if (rebuild) { + fprintf(stderr, "ERROR rebuilt refcount structure is still " + "broken\n"); + } + + /* Any leaks accounted for here were introduced by + * rebuild_refcount_structure() because that function has created a + * new refcount structure from scratch */ + fresh_leaks = res->leaks; + *res = saved_res; + } + + if (res->corruptions < old_res.corruptions) { + res->corruptions_fixed += old_res.corruptions - res->corruptions; + } + if (res->leaks < old_res.leaks) { + res->leaks_fixed += old_res.leaks - res->leaks; + } + res->leaks += fresh_leaks; } else if (fix) { if (rebuild) { fprintf(stderr, "ERROR need to rebuild refcount structures\n"); -- cgit v1.2.1 From d26e6ec052b8768ab45654dbf35d5213818a2cb8 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 22 Oct 2014 14:09:42 +0200 Subject: iotests: Fix test outputs 039, 060 and 061 all create images with referenced clusters having a refcount of 0. Because previous commits changed handling of such errors, these tests now have a different output. Fix it. Furthermore, 060 created a refblock with a refcount greater than one which now results in having to rebuild the refcount structure as well. Signed-off-by: Max Reitz Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- tests/qemu-iotests/039.out | 10 ++++++++-- tests/qemu-iotests/060.out | 10 ++++++++-- tests/qemu-iotests/061.out | 18 ++++++++++++------ 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/tests/qemu-iotests/039.out b/tests/qemu-iotests/039.out index 67e774430f..0adf1535ef 100644 --- a/tests/qemu-iotests/039.out +++ b/tests/qemu-iotests/039.out @@ -25,7 +25,10 @@ read 512/512 bytes at offset 0 incompatible_features 0x1 == Repairing the image file must succeed == -Repairing cluster 5 refcount=0 reference=1 +ERROR cluster 5 refcount=0 reference=1 +Rebuilding refcount structure +Repairing cluster 1 refcount=1 reference=0 +Repairing cluster 2 refcount=1 reference=0 The following inconsistencies were found and repaired: 0 leaked clusters @@ -45,7 +48,10 @@ wrote 512/512 bytes at offset 0 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) ./039: Aborted ( ulimit -c 0; exec "$@" ) incompatible_features 0x1 -Repairing cluster 5 refcount=0 reference=1 +ERROR cluster 5 refcount=0 reference=1 +Rebuilding refcount structure +Repairing cluster 1 refcount=1 reference=0 +Repairing cluster 2 refcount=1 reference=0 wrote 512/512 bytes at offset 0 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) incompatible_features 0x0 diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out index cd679f9454..9419da1b41 100644 --- a/tests/qemu-iotests/060.out +++ b/tests/qemu-iotests/060.out @@ -36,11 +36,15 @@ incompatible_features 0x0 qcow2: Marking image as corrupt: Preventing invalid write on metadata (overlaps with refcount block); further corruption events will be suppressed write failed: Input/output error incompatible_features 0x2 -Repairing refcount block 0 refcount=2 +ERROR refcount block 0 refcount=2 +ERROR cluster 2 refcount=1 reference=2 +Rebuilding refcount structure +Repairing cluster 1 refcount=1 reference=0 +Repairing cluster 2 refcount=2 reference=1 The following inconsistencies were found and repaired: 0 leaked clusters - 1 corruptions + 2 corruptions Double checking the fixed image now... No errors were found on the image. @@ -68,6 +72,8 @@ incompatible_features 0x0 qcow2: Marking image as corrupt: Preventing invalid write on metadata (overlaps with inactive L2 table); further corruption events will be suppressed write failed: Input/output error incompatible_features 0x2 +ERROR cluster 4 refcount=1 reference=2 +Leaked cluster 9 refcount=1 reference=0 Repairing cluster 4 refcount=1 reference=2 Repairing cluster 9 refcount=1 reference=0 Repairing OFLAG_COPIED data cluster: l2_entry=8000000000040000 refcount=2 diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out index e3724700b9..4ae6472c73 100644 --- a/tests/qemu-iotests/061.out +++ b/tests/qemu-iotests/061.out @@ -76,8 +76,11 @@ autoclear_features 0x0 refcount_order 4 header_length 104 -Repairing cluster 5 refcount=0 reference=1 -Repairing cluster 6 refcount=0 reference=1 +ERROR cluster 5 refcount=0 reference=1 +ERROR cluster 6 refcount=0 reference=1 +Rebuilding refcount structure +Repairing cluster 1 refcount=1 reference=0 +Repairing cluster 2 refcount=1 reference=0 magic 0x514649fb version 2 backing_file_offset 0x0 @@ -87,7 +90,7 @@ size 67108864 crypt_method 0 l1_size 1 l1_table_offset 0x30000 -refcount_table_offset 0x10000 +refcount_table_offset 0x80000 refcount_table_clusters 1 nb_snapshots 0 snapshot_offset 0x0 @@ -230,8 +233,11 @@ autoclear_features 0x0 refcount_order 4 header_length 104 -Repairing cluster 5 refcount=0 reference=1 -Repairing cluster 6 refcount=0 reference=1 +ERROR cluster 5 refcount=0 reference=1 +ERROR cluster 6 refcount=0 reference=1 +Rebuilding refcount structure +Repairing cluster 1 refcount=1 reference=0 +Repairing cluster 2 refcount=1 reference=0 magic 0x514649fb version 3 backing_file_offset 0x0 @@ -241,7 +247,7 @@ size 67108864 crypt_method 0 l1_size 1 l1_table_offset 0x30000 -refcount_table_offset 0x10000 +refcount_table_offset 0x80000 refcount_table_clusters 1 nb_snapshots 0 snapshot_offset 0x0 -- cgit v1.2.1 From 234764eed1aab56a657a161e9a0c65730442e6f8 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 22 Oct 2014 14:09:43 +0200 Subject: iotests: Add test for potentially damaging repairs There are certain cases where repairing a qcow2 image might actually damage it further (or rather, where repairing it has in fact damaged it further with the old qcow2 check implementation). This should not happen, so add a test for these cases. Furthermore, the repair function now repairs refblocks beyond the image end by resizing the image accordingly. Add several tests for this as well. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- tests/qemu-iotests/108 | 141 +++++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/108.out | 110 +++++++++++++++++++++++++++++++++++ tests/qemu-iotests/group | 1 + 3 files changed, 252 insertions(+) create mode 100755 tests/qemu-iotests/108 create mode 100644 tests/qemu-iotests/108.out diff --git a/tests/qemu-iotests/108 b/tests/qemu-iotests/108 new file mode 100755 index 0000000000..12fc92a633 --- /dev/null +++ b/tests/qemu-iotests/108 @@ -0,0 +1,141 @@ +#!/bin/bash +# +# Test case for repairing qcow2 images which cannot be repaired using +# the on-disk refcount structures +# +# Copyright (C) 2014 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=mreitz@redhat.com + +seq="$(basename $0)" +echo "QA output created by $seq" + +here="$PWD" +tmp=/tmp/$$ +status=1 # 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 + +# This tests qocw2-specific low-level functionality +_supported_fmt qcow2 +_supported_proto file +_supported_os Linux + +echo +echo '=== Repairing an image without any refcount table ===' +echo + +_make_test_img 64M +# just write some data +$QEMU_IO -c 'write -P 42 0 64k' "$TEST_IMG" | _filter_qemu_io + +# refcount_table_offset +poke_file "$TEST_IMG" $((0x30)) "\x00\x00\x00\x00\x00\x00\x00\x00" +# refcount_table_clusters +poke_file "$TEST_IMG" $((0x38)) "\x00\x00\x00\x00" + +_check_test_img -r all + +$QEMU_IO -c 'read -P 42 0 64k' "$TEST_IMG" | _filter_qemu_io + +echo +echo '=== Repairing unreferenced data cluster in new refblock area ===' +echo + +IMGOPTS='cluster_size=512' _make_test_img 64M +# Allocate the first 128 kB in the image (first refblock) +$QEMU_IO -c 'write 0 0x1b200' "$TEST_IMG" | _filter_qemu_io +# should be 131072 == 0x20000 +stat -c '%s' "$TEST_IMG" + +# Enter a cluster at 128 kB (0x20000) +# XXX: This should be the first free entry in the last L2 table, but we cannot +# be certain +poke_file "$TEST_IMG" $((0x1ccc8)) "\x80\x00\x00\x00\x00\x02\x00\x00" + +# Fill the cluster +truncate -s $((0x20200)) "$TEST_IMG" +$QEMU_IO -c "open -o driver=raw $TEST_IMG" -c 'write -P 42 128k 512' \ + | _filter_qemu_io + +# The data should now appear at this guest offset +$QEMU_IO -c 'read -P 42 0x1b200 512' "$TEST_IMG" | _filter_qemu_io + +# This cluster is unallocated; fix it +_check_test_img -r all + +# This repair operation must have allocated a new refblock; and that refblock +# should not overlap with the unallocated data cluster. If it does, the data +# will be damaged, so check it. +$QEMU_IO -c 'read -P 42 0x1b200 512' "$TEST_IMG" | _filter_qemu_io + +echo +echo '=== Repairing refblock beyond the image end ===' +echo + +echo +echo '--- Otherwise clean ---' +echo + +_make_test_img 64M +# Normally, qemu doesn't create empty refblocks, so we just have to do it by +# hand +# XXX: This should be the entry for the second refblock +poke_file "$TEST_IMG" $((0x10008)) "\x00\x00\x00\x00\x00\x10\x00\x00" +# Mark that refblock as used +# XXX: This should be the 17th entry (cluster 16) of the first +# refblock +poke_file "$TEST_IMG" $((0x20020)) "\x00\x01" +_check_test_img -r all + +echo +echo '--- Refblock is unallocated ---' +echo + +_make_test_img 64M +poke_file "$TEST_IMG" $((0x10008)) "\x00\x00\x00\x00\x00\x10\x00\x00" +_check_test_img -r all + +echo +echo '--- Signed overflow after the refblock ---' +echo + +_make_test_img 64M +poke_file "$TEST_IMG" $((0x10008)) "\x7f\xff\xff\xff\xff\xff\x00\x00" +_check_test_img -r all + +echo +echo '--- Unsigned overflow after the refblock ---' +echo + +_make_test_img 64M +poke_file "$TEST_IMG" $((0x10008)) "\xff\xff\xff\xff\xff\xff\x00\x00" +_check_test_img -r all + +# success, all done +echo '*** done' +rm -f $seq.full +status=0 diff --git a/tests/qemu-iotests/108.out b/tests/qemu-iotests/108.out new file mode 100644 index 0000000000..824d5cf139 --- /dev/null +++ b/tests/qemu-iotests/108.out @@ -0,0 +1,110 @@ +QA output created by 108 + +=== Repairing an image without any refcount table === + +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 +wrote 65536/65536 bytes at offset 0 +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +ERROR cluster 0 refcount=0 reference=1 +ERROR cluster 3 refcount=0 reference=1 +ERROR cluster 4 refcount=0 reference=1 +ERROR cluster 5 refcount=0 reference=1 +Rebuilding refcount structure +The following inconsistencies were found and repaired: + + 0 leaked clusters + 4 corruptions + +Double checking the fixed image now... +No errors were found on the image. +read 65536/65536 bytes at offset 0 +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +=== Repairing unreferenced data cluster in new refblock area === + +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 +wrote 111104/111104 bytes at offset 0 +108.500 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +131072 +wrote 512/512 bytes at offset 131072 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 512/512 bytes at offset 111104 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +ERROR cluster 256 refcount=0 reference=1 +Rebuilding refcount structure +Repairing cluster 1 refcount=1 reference=0 +Repairing cluster 2 refcount=1 reference=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. +read 512/512 bytes at offset 111104 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +=== Repairing refblock beyond the image end === + + +--- Otherwise clean --- + +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 +Repairing refcount block 1 is outside image +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. + +--- Refblock is unallocated --- + +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 +Repairing refcount block 1 is outside image +ERROR cluster 16 refcount=0 reference=1 +Rebuilding refcount structure +Repairing cluster 1 refcount=1 reference=0 +Repairing cluster 2 refcount=1 reference=0 +Repairing cluster 16 refcount=1 reference=0 +The following inconsistencies were found and repaired: + + 0 leaked clusters + 2 corruptions + +Double checking the fixed image now... +No errors were found on the image. + +--- Signed overflow after the refblock --- + +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 +Repairing refcount block 1 is outside image +ERROR could not resize image: Invalid argument +Rebuilding refcount structure +Repairing cluster 1 refcount=1 reference=0 +Repairing cluster 2 refcount=1 reference=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. + +--- Unsigned overflow after the refblock --- + +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 +Repairing refcount block 1 is outside image +ERROR could not resize image: Invalid argument +Rebuilding refcount structure +Repairing cluster 1 refcount=1 reference=0 +Repairing cluster 2 refcount=1 reference=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. +*** done diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index b230996528..be2054f8a5 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -106,3 +106,4 @@ 103 rw auto quick 104 rw auto 105 rw auto quick +108 rw auto quick -- cgit v1.2.1 From 17bd5f472754acd2458b53dc02a30d5651e6dd79 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 3 Sep 2014 00:25:07 +0200 Subject: qcow2: Drop REFCOUNT_SHIFT With BDRVQcowState.refcount_block_bits, we don't need REFCOUNT_SHIFT anymore. Signed-off-by: Max Reitz Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- block/qcow2-refcount.c | 32 ++++++++++++++------------------ block/qcow2.c | 2 +- block/qcow2.h | 2 -- 3 files changed, 15 insertions(+), 21 deletions(-) diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index 6f1b118112..1477031206 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -100,7 +100,7 @@ static int get_refcount(BlockDriverState *bs, int64_t cluster_index) uint16_t *refcount_block; uint16_t refcount; - refcount_table_index = cluster_index >> (s->cluster_bits - REFCOUNT_SHIFT); + refcount_table_index = cluster_index >> s->refcount_block_bits; if (refcount_table_index >= s->refcount_table_size) return 0; refcount_block_offset = @@ -121,8 +121,7 @@ static int get_refcount(BlockDriverState *bs, int64_t cluster_index) return ret; } - block_index = cluster_index & - ((1 << (s->cluster_bits - REFCOUNT_SHIFT)) - 1); + block_index = cluster_index & (s->refcount_block_size - 1); refcount = be16_to_cpu(refcount_block[block_index]); ret = qcow2_cache_put(bs, s->refcount_block_cache, @@ -157,8 +156,8 @@ static unsigned int next_refcount_table_size(BDRVQcowState *s, static int in_same_refcount_block(BDRVQcowState *s, uint64_t offset_a, uint64_t offset_b) { - uint64_t block_a = offset_a >> (2 * s->cluster_bits - REFCOUNT_SHIFT); - uint64_t block_b = offset_b >> (2 * s->cluster_bits - REFCOUNT_SHIFT); + uint64_t block_a = offset_a >> (s->cluster_bits + s->refcount_block_bits); + uint64_t block_b = offset_b >> (s->cluster_bits + s->refcount_block_bits); return (block_a == block_b); } @@ -179,7 +178,7 @@ static int alloc_refcount_block(BlockDriverState *bs, BLKDBG_EVENT(bs->file, BLKDBG_REFBLOCK_ALLOC); /* Find the refcount block for the given cluster */ - refcount_table_index = cluster_index >> (s->cluster_bits - REFCOUNT_SHIFT); + refcount_table_index = cluster_index >> s->refcount_block_bits; if (refcount_table_index < s->refcount_table_size) { @@ -256,7 +255,7 @@ static int alloc_refcount_block(BlockDriverState *bs, /* The block describes itself, need to update the cache */ int block_index = (new_block >> s->cluster_bits) & - ((1 << (s->cluster_bits - REFCOUNT_SHIFT)) - 1); + (s->refcount_block_size - 1); (*refcount_block)[block_index] = cpu_to_be16(1); } else { /* Described somewhere else. This can recurse at most twice before we @@ -328,8 +327,7 @@ static int alloc_refcount_block(BlockDriverState *bs, BLKDBG_EVENT(bs->file, BLKDBG_REFTABLE_GROW); /* Calculate the number of refcount blocks needed so far */ - uint64_t refcount_block_clusters = 1 << (s->cluster_bits - REFCOUNT_SHIFT); - uint64_t blocks_used = DIV_ROUND_UP(cluster_index, refcount_block_clusters); + uint64_t blocks_used = DIV_ROUND_UP(cluster_index, s->refcount_block_size); if (blocks_used > QCOW_MAX_REFTABLE_SIZE / sizeof(uint64_t)) { return -EFBIG; @@ -343,14 +341,14 @@ static int alloc_refcount_block(BlockDriverState *bs, uint64_t table_clusters = size_to_clusters(s, table_size * sizeof(uint64_t)); blocks_clusters = 1 + - ((table_clusters + refcount_block_clusters - 1) - / refcount_block_clusters); + ((table_clusters + s->refcount_block_size - 1) + / s->refcount_block_size); uint64_t meta_clusters = table_clusters + blocks_clusters; last_table_size = table_size; table_size = next_refcount_table_size(s, blocks_used + - ((meta_clusters + refcount_block_clusters - 1) - / refcount_block_clusters)); + ((meta_clusters + s->refcount_block_size - 1) + / s->refcount_block_size)); } while (last_table_size != table_size); @@ -360,7 +358,7 @@ static int alloc_refcount_block(BlockDriverState *bs, #endif /* Create the new refcount table and blocks */ - uint64_t meta_offset = (blocks_used * refcount_block_clusters) * + uint64_t meta_offset = (blocks_used * s->refcount_block_size) * s->cluster_size; uint64_t table_offset = meta_offset + blocks_clusters * s->cluster_size; uint64_t *new_table = g_try_new0(uint64_t, table_size); @@ -560,8 +558,7 @@ static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs, { int block_index, refcount; int64_t cluster_index = cluster_offset >> s->cluster_bits; - int64_t table_index = - cluster_index >> (s->cluster_bits - REFCOUNT_SHIFT); + int64_t table_index = cluster_index >> s->refcount_block_bits; /* Load the refcount block and allocate it if needed */ if (table_index != old_table_index) { @@ -583,8 +580,7 @@ static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs, qcow2_cache_entry_mark_dirty(s->refcount_block_cache, refcount_block); /* we can update the count and save it */ - block_index = cluster_index & - ((1 << (s->cluster_bits - REFCOUNT_SHIFT)) - 1); + block_index = cluster_index & (s->refcount_block_size - 1); refcount = be16_to_cpu(refcount_block[block_index]); refcount += addend; diff --git a/block/qcow2.c b/block/qcow2.c index 7a2c66f92b..d031515838 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1874,7 +1874,7 @@ static int qcow2_create2(const char *filename, int64_t total_size, .l1_size = cpu_to_be32(0), .refcount_table_offset = cpu_to_be64(cluster_size), .refcount_table_clusters = cpu_to_be32(1), - .refcount_order = cpu_to_be32(3 + REFCOUNT_SHIFT), + .refcount_order = cpu_to_be32(4), .header_length = cpu_to_be32(sizeof(*header)), }; diff --git a/block/qcow2.h b/block/qcow2.h index 47cd4b3f76..577ccd1bf0 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -59,8 +59,6 @@ /* The cluster reads as all zeros */ #define QCOW_OFLAG_ZERO (1ULL << 0) -#define REFCOUNT_SHIFT 1 /* refcount size is 2 bytes */ - #define MIN_CLUSTER_BITS 9 #define MAX_CLUSTER_BITS 21 -- cgit v1.2.1 From 4b318d6ca66545e59eafbf595f66e31bf1625d9a Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 3 Sep 2014 00:25:08 +0200 Subject: docs/qcow2: Correct refcount_block_entries A refblock entry may have a different size than 16 bits, it may even be smaller than a byte. Correct the refcount_block_entries calculation accordingly. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- docs/specs/qcow2.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt index cfbc8b070c..0a878aa95d 100644 --- a/docs/specs/qcow2.txt +++ b/docs/specs/qcow2.txt @@ -183,7 +183,7 @@ blocks and are exactly one cluster in size. Given a offset into the image file, the refcount of its cluster can be obtained as follows: - refcount_block_entries = (cluster_size / sizeof(uint16_t)) + refcount_block_entries = (cluster_size * 8 / refcount_bits) refcount_block_index = (offset / cluster_size) % refcount_block_entries refcount_table_index = (offset / cluster_size) / refcount_block_entries -- cgit v1.2.1 From 7f75a07d50710e7f1371c4b248e0550549f77ead Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 3 Sep 2014 00:25:09 +0200 Subject: docs/qcow2: Limit refcount_order to [0, 6] Specify the upper limit of refcount_order to be 6 (that is, refcount_bits = 64). Any larger value does not make much sense when all offsets, sizes, cluster counts etc. "only" have a width of 64 bit as well, and very large values would be very difficult to support. Therefore, just cap it at the largest reasonable value. Suggested-by: Eric Blake Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- docs/specs/qcow2.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt index 0a878aa95d..121dfc8cc1 100644 --- a/docs/specs/qcow2.txt +++ b/docs/specs/qcow2.txt @@ -110,6 +110,7 @@ in the description of a field. in bits: refcount_bits = 1 << refcount_order). For version 2 images, the order is always assumed to be 4 (i.e. refcount_bits = 16). + This value may not exceed 6 (i.e. refcount_bits = 64). 100 - 103: header_length Length of the header structure in bytes. For version 2 -- cgit v1.2.1 From 59c9a95fd29cfb3296ee58e8a446df251d14a459 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 22 Oct 2014 17:00:15 +0200 Subject: block: Respect underlying file's EOF When falling through to the underlying file in bdrv_co_get_block_status(), if it returns that the query offset is beyond the file end (by setting *pnum to 0), return the range to be zero and do not let the number of sectors for which information could be obtained be overwritten. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/block.c b/block.c index bbb04e7b95..88f6d9b236 100644 --- a/block.c +++ b/block.c @@ -3954,13 +3954,24 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs, if (bs->file && (ret & BDRV_BLOCK_DATA) && !(ret & BDRV_BLOCK_ZERO) && (ret & BDRV_BLOCK_OFFSET_VALID)) { + int file_pnum; + ret2 = bdrv_co_get_block_status(bs->file, ret >> BDRV_SECTOR_BITS, - *pnum, pnum); + *pnum, &file_pnum); if (ret2 >= 0) { /* Ignore errors. This is just providing extra information, it * is useful but not necessary. */ - ret |= (ret2 & BDRV_BLOCK_ZERO); + if (!file_pnum) { + /* !file_pnum indicates an offset at or beyond the EOF; it is + * perfectly valid for the format block driver to point to such + * offsets, so catch it and mark everything as zero */ + ret |= BDRV_BLOCK_ZERO; + } else { + /* Limit request to the range reported by the protocol driver */ + *pnum = file_pnum; + ret |= (ret2 & BDRV_BLOCK_ZERO); + } } } -- cgit v1.2.1 From 4b25bbc4c22cf39350b75bd250d568a4d975f7c5 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 22 Oct 2014 17:00:16 +0200 Subject: qemu-io: Respect early image end for map MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit bdrv_is_allocated() may report zero clusters which most probably means the image (file) is shorter than expected. Respect this case in order to avoid an infinite loop. Signed-off-by: Max Reitz Reviewed-by: Eric Blake Reviewed-by: Benoît Canet Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- qemu-io-cmds.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c index b224ede5fa..d94fb1e2ea 100644 --- a/qemu-io-cmds.c +++ b/qemu-io-cmds.c @@ -1900,7 +1900,7 @@ static int map_is_allocated(BlockDriverState *bs, int64_t sector_num, num_checked = MIN(nb_sectors, INT_MAX); ret = bdrv_is_allocated(bs, sector_num, num_checked, &num); - if (ret == firstret) { + if (ret == firstret && num) { *pnum += num; } else { break; @@ -1927,6 +1927,9 @@ static int map_f(BlockDriverState *bs, int argc, char **argv) if (ret < 0) { error_report("Failed to get allocation status: %s", strerror(-ret)); return 0; + } else if (!num) { + error_report("Unexpected end of image"); + return 0; } retstr = ret ? " allocated" : "not allocated"; -- cgit v1.2.1 From 925bb3238d498172e8ee9a57842320b1d1bfc3a5 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 22 Oct 2014 17:00:17 +0200 Subject: iotests: Add test for map commands Add a test for qemu-img map and qemu-io -c map on truncated files. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- tests/qemu-iotests/102 | 64 ++++++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/102.out | 10 ++++++++ tests/qemu-iotests/group | 1 + 3 files changed, 75 insertions(+) create mode 100755 tests/qemu-iotests/102 create mode 100644 tests/qemu-iotests/102.out diff --git a/tests/qemu-iotests/102 b/tests/qemu-iotests/102 new file mode 100755 index 0000000000..34b363f78f --- /dev/null +++ b/tests/qemu-iotests/102 @@ -0,0 +1,64 @@ +#!/bin/bash +# +# Test case for qemu-io -c map and qemu-img map +# +# Copyright (C) 2014 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=mreitz@redhat.com + +seq=$(basename $0) +echo "QA output created by $seq" + +here=$PWD +tmp=/tmp/$$ +status=1 # 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 + +IMG_SIZE=64K + +echo +echo '=== Testing map command on truncated image ===' +echo + +_make_test_img $IMG_SIZE +# Create cluster +$QEMU_IO -c 'write 0 64k' "$TEST_IMG" | _filter_qemu_io +# Remove data cluster from image (first cluster: image header, second: reftable, +# third: refblock, fourth: L1 table, fifth: L2 table) +truncate -s $((5 * 64 * 1024)) "$TEST_IMG" + +$QEMU_IO -c map "$TEST_IMG" +$QEMU_IMG map "$TEST_IMG" + +# success, all done +echo '*** done' +rm -f $seq.full +status=0 diff --git a/tests/qemu-iotests/102.out b/tests/qemu-iotests/102.out new file mode 100644 index 0000000000..e0e9cdc541 --- /dev/null +++ b/tests/qemu-iotests/102.out @@ -0,0 +1,10 @@ +QA output created by 102 + +=== Testing map command on truncated image === + +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=65536 +wrote 65536/65536 bytes at offset 0 +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +[ 0] 128/ 128 sectors allocated at offset 0 bytes (1) +Offset Length Mapped to File +*** done diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index be2054f8a5..c9752e9f0b 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -103,6 +103,7 @@ 099 rw auto quick 100 rw auto quick 101 rw auto quick +102 rw auto quick 103 rw auto quick 104 rw auto 105 rw auto quick -- cgit v1.2.1 From a1391444fe1cfef14976458f3293a2c6945e725c Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Thu, 16 Oct 2014 15:25:56 +0200 Subject: qcow2: Do not overflow when writing an L1 sector While writing an L1 table sector, qcow2_write_l1_entry() copies the respective range from s->l1_table to the local "buf" array. The size of s->l1_table does not have to be a multiple of L1_ENTRIES_PER_SECTOR; thus, limit the index which is used for copying all entries to the L1 size. Cc: qemu-stable@nongnu.org Signed-off-by: Max Reitz Reviewed-by: Peter Lieven Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- block/qcow2-cluster.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index f7dd8c0985..4d888c785f 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -164,12 +164,14 @@ static int l2_load(BlockDriverState *bs, uint64_t l2_offset, int qcow2_write_l1_entry(BlockDriverState *bs, int l1_index) { BDRVQcowState *s = bs->opaque; - uint64_t buf[L1_ENTRIES_PER_SECTOR]; + uint64_t buf[L1_ENTRIES_PER_SECTOR] = { 0 }; int l1_start_index; int i, ret; l1_start_index = l1_index & ~(L1_ENTRIES_PER_SECTOR - 1); - for (i = 0; i < L1_ENTRIES_PER_SECTOR; i++) { + for (i = 0; i < L1_ENTRIES_PER_SECTOR && l1_start_index + i < s->l1_size; + i++) + { buf[i] = cpu_to_be64(s->l1_table[l1_start_index + i]); } -- cgit v1.2.1 From 1b2dd0bee6bd03045b90c8a7549c8134466b2938 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Thu, 16 Oct 2014 15:25:57 +0200 Subject: iotests: Add test for qcow2 L1 table update Updating the L1 table should not result in random data being written. This adds a test for that. Signed-off-by: Max Reitz Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- tests/qemu-iotests/107 | 61 ++++++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/107.out | 10 ++++++++ tests/qemu-iotests/group | 1 + 3 files changed, 72 insertions(+) create mode 100755 tests/qemu-iotests/107 create mode 100644 tests/qemu-iotests/107.out diff --git a/tests/qemu-iotests/107 b/tests/qemu-iotests/107 new file mode 100755 index 0000000000..cad1cf98a0 --- /dev/null +++ b/tests/qemu-iotests/107 @@ -0,0 +1,61 @@ +#!/bin/bash +# +# Tests updates of the qcow2 L1 table +# +# Copyright (C) 2014 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=mreitz@redhat.com + +seq="$(basename $0)" +echo "QA output created by $seq" + +here="$PWD" +tmp=/tmp/$$ +status=1 # 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 + + +IMG_SIZE=64K + +echo +echo '=== Updates should not write random data ===' +echo + +_make_test_img $IMG_SIZE +$QEMU_IO -c 'write 0 64k' "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "open -o driver=raw $TEST_IMG" -c 'read -p -P 0 196616 65528' \ + | _filter_qemu_io + +# success, all done +echo "*** done" +rm -f $seq.full +status=0 + diff --git a/tests/qemu-iotests/107.out b/tests/qemu-iotests/107.out new file mode 100644 index 0000000000..93445b731f --- /dev/null +++ b/tests/qemu-iotests/107.out @@ -0,0 +1,10 @@ +QA output created by 107 + +=== Updates should not write random data === + +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=65536 +wrote 65536/65536 bytes at offset 0 +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 65528/65528 bytes at offset 196616 +63.992 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +*** done diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index c9752e9f0b..9bbd5d3a17 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -107,4 +107,5 @@ 103 rw auto quick 104 rw auto 105 rw auto quick +107 rw auto quick 108 rw auto quick -- cgit v1.2.1 From 3cad83075c7b847fe0eb6e61316fdf50984d4570 Mon Sep 17 00:00:00 2001 From: Roger Pau Monne Date: Tue, 21 Oct 2014 16:03:03 +0200 Subject: block: char devices on FreeBSD are not behind a pager MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce a new flag to mark devices that require requests to be aligned and replace the usage of BDRV_O_NOCACHE and O_DIRECT with this flag when appropriate. If a character device is used as a backend on a FreeBSD host set this flag unconditionally. Signed-off-by: Roger Pau Monné Reviewed-by: Max Reitz Cc: Kevin Wolf Cc: Stefan Hajnoczi Signed-off-by: Max Reitz --- block/raw-posix.c | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/block/raw-posix.c b/block/raw-posix.c index ee4ca3c01e..475cf74655 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -150,6 +150,7 @@ typedef struct BDRVRawState { bool has_discard:1; bool has_write_zeroes:1; bool discard_zeroes:1; + bool needs_alignment; #ifdef CONFIG_FIEMAP bool skip_fiemap; #endif @@ -230,7 +231,7 @@ static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp) /* For /dev/sg devices the alignment is not really used. With buffered I/O, we don't have any restrictions. */ - if (bs->sg || !(s->open_flags & O_DIRECT)) { + if (bs->sg || !s->needs_alignment) { bs->request_alignment = 1; s->buf_align = 1; return; @@ -446,6 +447,9 @@ static int raw_open_common(BlockDriverState *bs, QDict *options, s->has_discard = true; s->has_write_zeroes = true; + if ((bs->open_flags & BDRV_O_NOCACHE) != 0) { + s->needs_alignment = true; + } if (fstat(s->fd, &st) < 0) { error_setg_errno(errp, errno, "Could not stat file"); @@ -472,6 +476,17 @@ static int raw_open_common(BlockDriverState *bs, QDict *options, } #endif } +#ifdef __FreeBSD__ + if (S_ISCHR(st.st_mode)) { + /* + * The file is a char device (disk), which on FreeBSD isn't behind + * a pager, so force all requests to be aligned. This is needed + * so QEMU makes sure all IO operations on the device are aligned + * to sector size, or else FreeBSD will reject them with EINVAL. + */ + s->needs_alignment = true; + } +#endif #ifdef CONFIG_XFS if (platform_test_xfs_fd(s->fd)) { @@ -1076,11 +1091,12 @@ static BlockAIOCB *raw_aio_submit(BlockDriverState *bs, return NULL; /* - * If O_DIRECT is used the buffer needs to be aligned on a sector - * boundary. Check if this is the case or tell the low-level - * driver that it needs to copy the buffer. + * Check if the underlying device requires requests to be aligned, + * and if the request we are trying to submit is aligned or not. + * If this is the case tell the low-level driver that it needs + * to copy the buffer. */ - if ((bs->open_flags & BDRV_O_NOCACHE)) { + if (s->needs_alignment) { if (!bdrv_qiov_is_aligned(bs, qiov)) { type |= QEMU_AIO_MISALIGNED; #ifdef CONFIG_LINUX_AIO -- cgit v1.2.1 From 832390a5ed11e6c516db0986bf302d098e3ae36c Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Thu, 23 Oct 2014 15:29:12 +0200 Subject: qemu-img: Print error if check failed Currently, if bdrv_check() fails either by returning -errno or having check_errors set, qemu-img check just exits with 1 after having told the user that there were no errors on the image. This is bad. Instead of printing the check result if there were internal errors which were so bad that bdrv_check() could not even complete with 0 as a return value, qemu-img check should inform the user about the error. Signed-off-by: Max Reitz Reviewed-by: Eric Blake Reviewed-by: Markus Armbruster --- qemu-img.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/qemu-img.c b/qemu-img.c index 09e7e72f5a..731502c2ce 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -687,16 +687,23 @@ static int img_check(int argc, char **argv) check->corruptions_fixed = corruptions_fixed; } - switch (output_format) { - case OFORMAT_HUMAN: - dump_human_image_check(check, quiet); - break; - case OFORMAT_JSON: - dump_json_image_check(check, quiet); - break; + if (!ret) { + switch (output_format) { + case OFORMAT_HUMAN: + dump_human_image_check(check, quiet); + break; + case OFORMAT_JSON: + dump_json_image_check(check, quiet); + break; + } } if (ret || check->check_errors) { + if (ret) { + error_report("Check failed: %s", strerror(-ret)); + } else { + error_report("Check failed"); + } ret = 1; goto fail; } -- cgit v1.2.1