| Message ID | 1487153430-17260-22-git-send-email-vsementsov@virtuozzo.com |
|---|---|
| State | New |
| Headers | show |
Am 15.02.2017 um 11:10 hat Vladimir Sementsov-Ogievskiy geschrieben: > Calculate refcounts for qcow2 bitmaps. It is needed for qcow2's qemu-img > check implementation. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > Reviewed-by: Max Reitz <mreitz@redhat.com> > Reviewed-by: John Snow <jsnow@redhat.com> Maybe this should come earlier in the series so that qemu-img check doesn't corrupt them. Basically it's needed as soon as we add the autoclear flag as supported. Anyway, the actual point I was going to make is that it would be good to have a qemu-iotests case which actually invokes qemu-img check on an image with dirty bitmaps. In general, the test case side of the series seems to be rather weak so far. Just keep in mind that I won't feel bad for breaking anything that isn't tested by a test case. So if you like to keep persistent bitmaps working in the long run, you'd better write tests and give me no chance to break anything without them failing. Kevin
16.02.2017 17:27, Kevin Wolf wrote: > Am 15.02.2017 um 11:10 hat Vladimir Sementsov-Ogievskiy geschrieben: >> Calculate refcounts for qcow2 bitmaps. It is needed for qcow2's qemu-img >> check implementation. >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >> Reviewed-by: Max Reitz <mreitz@redhat.com> >> Reviewed-by: John Snow <jsnow@redhat.com> > Maybe this should come earlier in the series so that qemu-img check > doesn't corrupt them. Basically it's needed as soon as we add the > autoclear flag as supported. so, it should be merged into 07 > > Anyway, the actual point I was going to make is that it would be good to > have a qemu-iotests case which actually invokes qemu-img check on an > image with dirty bitmaps. In general, the test case side of the series > seems to be rather weak so far. Just keep in mind that I won't feel bad > for breaking anything that isn't tested by a test case. So if you like > to keep persistent bitmaps working in the long run, you'd better write > tests and give me no chance to break anything without them failing. > > Kevin
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c index 1ee89e4..05d3f28 100644 --- a/block/qcow2-bitmap.c +++ b/block/qcow2-bitmap.c @@ -1234,3 +1234,79 @@ fail: name, bdrv_get_device_or_node_name(bs)); 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); + } + +out: + bitmap_list_free(bm_list); + + return ret; +} diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index 14a736d..cdb74ba 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 0a3708d..eaad34a 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