Message ID | 20170123121036.4823-22-vsementsov@virtuozzo.com |
---|---|
State | New |
Headers | show |
On 23.01.2017 13:10, Vladimir Sementsov-Ogievskiy wrote: > Calculate refcounts for qcow2 bitmaps. It is needed for qcow2's qemu-img > check implementation. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > block/qcow2-bitmap.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++ > block/qcow2-refcount.c | 6 ++++ > block/qcow2.h | 3 ++ > 3 files changed, 89 insertions(+) > > diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c > index 8b8f2d11ec..2687a3acd5 100644 > --- a/block/qcow2-bitmap.c > +++ b/block/qcow2-bitmap.c > @@ -1232,3 +1232,83 @@ common_errp: > name, bdrv_get_device_or_node_name(bs), reason); > return false; > } > + > +int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, BdrvCheckResult *res, > + void **refcount_table, > + int64_t *refcount_table_size) > +{ > + int ret; > + BDRVQcow2State *s = bs->opaque; > + Qcow2BitmapList *bm_list; > + Qcow2Bitmap *bm; > + > + if (s->nb_bitmaps == 0) { > + return 0; > + } > + > + ret = qcow2_inc_refcounts_imrt(bs, res, refcount_table, refcount_table_size, > + s->bitmap_directory_offset, > + s->bitmap_directory_size); > + if (ret < 0) { > + return ret; > + } > + > + bm_list = bitmap_list_load(bs, s->bitmap_directory_offset, > + s->bitmap_directory_size, NULL); > + if (bm_list == NULL) { > + res->corruptions++; > + return -EINVAL; > + } > + > + QSIMPLEQ_FOREACH(bm, bm_list, entry) { > + uint64_t *bitmap_table = NULL; > + int i; > + > + ret = qcow2_inc_refcounts_imrt(bs, res, > + refcount_table, refcount_table_size, > + bm->table.offset, > + bm->table.size * sizeof(uint64_t)); > + if (ret < 0) { > + goto out; > + } > + > + ret = bitmap_table_load(bs, &bm->table, &bitmap_table); > + if (ret < 0) { > + res->corruptions++; > + goto out; > + } > + > + for (i = 0; i < bm->table.size; ++i) { > + uint64_t entry = bitmap_table[i]; > + uint64_t offset = entry & BME_TABLE_ENTRY_OFFSET_MASK; > + > + if (check_table_entry(entry, s->cluster_size) < 0) { > + res->corruptions++; > + continue; > + } > + > + if (offset == 0) { > + continue; > + } > + > + ret = qcow2_inc_refcounts_imrt(bs, res, > + refcount_table, refcount_table_size, > + offset, s->cluster_size); > + if (ret < 0) { > + g_free(bitmap_table); > + goto out; > + } > + } > + > + g_free(bitmap_table); > + } > + > + if (res->corruptions > 0) { > + ret = -EINVAL; > + } res->corruptions may already be greater than zero when this function is invoked. But in any case, you don't need to return a negative value just because some corruption was found. These check functions should only return negative values if they were unable to do what they are supposed to do (whatever that is), e.g. because of an I/O error or ENOMEM. In these cases, however, they usually return to the caller immediately. It wouldn't be strictly wrong to return a negative value if some table entry is corrupted, but I don't think it's necessary either. So if you just drop the above three lines: Reviewed-by: Max Reitz <mreitz@redhat.com>
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c index 8b8f2d11ec..2687a3acd5 100644 --- a/block/qcow2-bitmap.c +++ b/block/qcow2-bitmap.c @@ -1232,3 +1232,83 @@ common_errp: name, bdrv_get_device_or_node_name(bs), reason); return false; } + +int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, BdrvCheckResult *res, + void **refcount_table, + int64_t *refcount_table_size) +{ + int ret; + BDRVQcow2State *s = bs->opaque; + Qcow2BitmapList *bm_list; + Qcow2Bitmap *bm; + + if (s->nb_bitmaps == 0) { + return 0; + } + + ret = qcow2_inc_refcounts_imrt(bs, res, refcount_table, refcount_table_size, + s->bitmap_directory_offset, + s->bitmap_directory_size); + if (ret < 0) { + return ret; + } + + bm_list = bitmap_list_load(bs, s->bitmap_directory_offset, + s->bitmap_directory_size, NULL); + if (bm_list == NULL) { + res->corruptions++; + return -EINVAL; + } + + QSIMPLEQ_FOREACH(bm, bm_list, entry) { + uint64_t *bitmap_table = NULL; + int i; + + ret = qcow2_inc_refcounts_imrt(bs, res, + refcount_table, refcount_table_size, + bm->table.offset, + bm->table.size * sizeof(uint64_t)); + if (ret < 0) { + goto out; + } + + ret = bitmap_table_load(bs, &bm->table, &bitmap_table); + if (ret < 0) { + res->corruptions++; + goto out; + } + + for (i = 0; i < bm->table.size; ++i) { + uint64_t entry = bitmap_table[i]; + uint64_t offset = entry & BME_TABLE_ENTRY_OFFSET_MASK; + + if (check_table_entry(entry, s->cluster_size) < 0) { + res->corruptions++; + continue; + } + + if (offset == 0) { + continue; + } + + ret = qcow2_inc_refcounts_imrt(bs, res, + refcount_table, refcount_table_size, + offset, s->cluster_size); + if (ret < 0) { + g_free(bitmap_table); + goto out; + } + } + + g_free(bitmap_table); + } + + if (res->corruptions > 0) { + ret = -EINVAL; + } + +out: + bitmap_list_free(bm_list); + + return ret; +} diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index 14a736d245..cdb74ba4de 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -1846,6 +1846,12 @@ static int calculate_refcounts(BlockDriverState *bs, BdrvCheckResult *res, return ret; } + /* bitmaps */ + ret = qcow2_check_bitmaps_refcounts(bs, res, refcount_table, nb_clusters); + if (ret < 0) { + return ret; + } + return check_refblocks(bs, res, fix, rebuild, refcount_table, nb_clusters); } diff --git a/block/qcow2.h b/block/qcow2.h index 0a3708d890..eaad34a527 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -624,5 +624,8 @@ bool qcow2_can_store_new_dirty_bitmap(BlockDriverState *bs, const char *name, uint32_t granularity, Error **errp); +int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, BdrvCheckResult *res, + void **refcount_table, + int64_t *refcount_table_size); #endif
Calculate refcounts for qcow2 bitmaps. It is needed for qcow2's qemu-img check implementation. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- block/qcow2-bitmap.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++ block/qcow2-refcount.c | 6 ++++ block/qcow2.h | 3 ++ 3 files changed, 89 insertions(+)