On 06/15/2015 10:05 AM, Vladimir Sementsov-Ogievskiy wrote:
> On 12.06.2015 02:04, John Snow wrote:
>>
>> On 06/08/2015 11:21 AM, Vladimir Sementsov-Ogievskiy wrote:
>>> From: Vladimir Sementsov-Ogievskiy <vsement...@parallels.com>
>>>
>>> Adds dirty-bitmaps feature to qcow2 format as specified in
>>> docs/specs/qcow2.txt
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@parallels.com>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
>>> ---
>>> block/Makefile.objs | 2 +-
>>> block/qcow2-dirty-bitmap.c | 503
>>> +++++++++++++++++++++++++++++++++++++++++++++
>>> block/qcow2.c | 56 +++++
>>> block/qcow2.h | 50 +++++
>>> include/block/block_int.h | 10 +
>>> 5 files changed, 620 insertions(+), 1 deletion(-)
>>> create mode 100644 block/qcow2-dirty-bitmap.c
>>>
[snip]
>>> +int qcow2_dirty_bitmap_store(BlockDriverState *bs, uint8_t *buf,
>>> + const char *name, uint64_t size,
>>> + int granularity)
>>> +{
>>> + BDRVQcowState *s = bs->opaque;
>>> + int cl_size = s->cluster_size;
>>> + int i, dirty_bitmap_index, ret = 0, n;
>>> + uint64_t *l1_table;
>>> + QCowDirtyBitmap *bm;
>>> + uint64_t buf_size;
>>> + uint8_t *p;
>>> + int sector_granularity = granularity >> BDRV_SECTOR_BITS;
>>> +
>>> + /* find/create dirty bitmap */
>>> + dirty_bitmap_index = find_dirty_bitmap_by_name(bs, name);
>>> + if (dirty_bitmap_index >= 0) {
>>> + bm = s->dirty_bitmaps + dirty_bitmap_index;
>>> +
>>> + if (size != bm->bitmap_size ||
>>> + granularity != bm->bitmap_granularity) {
>>> + qcow2_dirty_bitmap_delete(bs, name, NULL);
>>> + dirty_bitmap_index = -1;
>>> + }
>>> + }
>>> + if (dirty_bitmap_index < 0) {
>>> + qcow2_dirty_bitmap_create(bs, name, size, granularity);
>>> + dirty_bitmap_index = s->nb_dirty_bitmaps - 1;
>>> + }
>>> + bm = s->dirty_bitmaps + dirty_bitmap_index;
>> I catch a segfault right around here if I do the following:
>>
>> ./x86_64-softmmu/qemu-system-x86_64 --dirty-bitmap
>> file=bitmaps.qcow2,name=bitmap0,drive=drive0 -drive
>> if=none,file=hda.qcow2,id=drive0 -device ide-hd,drive=drive0
>>
>> hda.qcow2 and bitmaps.qcow2 are both empty files, but bitmaps.qcow2 has
>> a size of '0'.
> empty file or qcow2 files of size 0 (with header) ?
Sorry, that was ambiguous. A properly formatted qcow2 file with a size of 0.
I tried two configurations:
1) Saving a bitmap to the file it describes, using e.g. hda.qcow2, which
is an 8GiB qcow2 file that has been formatted, but contains no
allocations yet.
2) Saving a bitmap to a bitmaps.qcow2 file, which is a formatted qcow2
with a size of 0, to describe a file hda.qcow2 which is 8GiB and has no
allocations either.
Both crash, because I think you are confusing s->l1_size with
bm->l1_size during the storage routine.
The following patch fixes my initial issues with the series, at least at
a cursory glance:
commit 824ed0e9f56425c98ec600abb0e31791d12e628f
Author: John Snow <js...@redhat.com>
Date: Mon Jun 15 12:06:43 2015 -0400
fix persistence
diff --git a/block/qcow2-dirty-bitmap.c b/block/qcow2-dirty-bitmap.c
index 686a121..eaaec14 100644
--- a/block/qcow2-dirty-bitmap.c
+++ b/block/qcow2-dirty-bitmap.c
@@ -300,7 +300,8 @@ int qcow2_dirty_bitmap_store(BlockDriverState *bs,
uint8_t *buf,
}
}
if (dirty_bitmap_index < 0) {
- qcow2_dirty_bitmap_create(bs, name, size, granularity);
+ ret = qcow2_dirty_bitmap_create(bs, name, size, granularity);
+ if (ret < 0) { return ret; }
dirty_bitmap_index = s->nb_dirty_bitmaps - 1;
}
bm = s->dirty_bitmaps + dirty_bitmap_index;
@@ -384,7 +385,7 @@ int qcow2_dirty_bitmap_create(BlockDriverState *bs,
const char *name,
bm->l1_size =
size_to_clusters(s, (((size - 1) / sector_granularity) >> 3) + 1);
l1_table_offset =
- qcow2_alloc_clusters(bs, s->l1_size * sizeof(uint64_t));
+ qcow2_alloc_clusters(bs, bm->l1_size * sizeof(uint64_t));
if (l1_table_offset < 0) {
ret = l1_table_offset;
goto fail;
@@ -398,18 +399,18 @@ int qcow2_dirty_bitmap_create(BlockDriverState
*bs, const char *name,
}
/* initialize with zero clusters */
- for (i = 0; i < s->l1_size; i++) {
+ for (i = 0; i < bm->l1_size; i++) {
l1_table[i] = cpu_to_be64(1);
}
ret = qcow2_pre_write_overlap_check(bs, 0, bm->l1_table_offset,
- s->l1_size * sizeof(uint64_t));
+ bm->l1_size * sizeof(uint64_t));
if (ret < 0) {
goto fail;
}
ret = bdrv_pwrite(bs->file, bm->l1_table_offset, l1_table,
- s->l1_size * sizeof(uint64_t));
+ bm->l1_size * sizeof(uint64_t));
if (ret < 0) {
goto fail;
}
diff --git a/vl.c b/vl.c
index 4cae8a6..8f6d79f 100644
--- a/vl.c
+++ b/vl.c
@@ -1146,7 +1146,7 @@ static int dirty_bitmap_func(void *opaque,
QemuOpts *opts, Error **errp)
qdict_put(options, "node-name", qstring_from_str(file_id));
}
- bdrv_open(&file_bs, file, NULL, options, 0, NULL, errp);
+ bdrv_open(&file_bs, file, NULL, options, BDRV_O_RDWR, NULL, errp);
if (options) {
QDECREF(options);
}
