* [Qemu-devel] [PATCH v11 00/24] qcow2: persistent dirty bitmaps
@ 2017-01-23 12:10 Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 01/24] specs/qcow2: fix bitmap granularity qemu-specific note Vladimir Sementsov-Ogievskiy
` (23 more replies)
0 siblings, 24 replies; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-01-23 12:10 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: kwolf, mreitz, armbru, eblake, jsnow, famz, den, stefanha,
vsementsov, pbonzini
Hi all!
There is a new update of qcow2-bitmap series - v11.
web: https://src.openvz.org/users/vsementsov/repos/qemu/browse?at=qcow2-bitmap-v11
git: https://src.openvz.org/scm/~vsementsov/qemu.git (tag qcow2-bitmap-v11)
v11:
Fix automatic build test fail.
18: wording - "ASCII representation of SHA256 ..."
24: fix redeclaration error. (This is still RFC, may be not needed patch,
see description in v10 below).
v10:
07: rm John's r-b
not add Max's r-b
fix subject s/dirty bitmaps/bitmaps
improve WARNING message
remove header ext if autoclear bit is unset, through qcow2_updata_header() - r-b blocking change
08: move bdrv_load_autoloading_dirty_bitmaps under asserts block (Max)
fix typo in commit message
move bdrv_load_autoloading_dirty_bitmaps after asserts
John's r-b
09: rewrite check_dir_entry
remove extra feature cluster_size=0 from check_table_entry
which clears autoclear bit before trying to update bitmap directory
bitmap_list_store - do not free old clusters if in_place=true
changes in comments, fix indents
add functions
bitmap_list_count
update_ext_header_and_dir_in_place (unset autoclear bit before trying to
modify bitmap directory)
(for usage in qcow2_load_autoloading_dirty_bitmaps)
11: add node name to error report
Max's r-b
13: add type Qcow2BitmapTable
move from bm.table_* to bm.table.*
rewrite check_constraints_on_bitmap
flush(bs->file) instead of flush(bs) in update_ext_header_and_dir
assert(DIV_ROUND_UP(bm_size, dsc) == tb_size);
and assert(write_size <= s->cluster_size); in store_bitmap_data
fix: s/tb_size/tb_size * sizeof(tb[0]) in store_bitmap
in qcow2_store_persistent_dirty_bitmaps:
fail if !can_write
fix counting of new_nb_bitmaps and new_dir_size
free old clusters _after_ successful directory update (aggregate old bitmap
tables into drop_tables)
2 14: rename to can_store_new_dirty_bitmap
Max's r-b
15: rename to can_store_new_dirty_bitmap
rm extra !!
16: Max's r-b
rename to can_store_new_dirty_bitmap
indenting
Since s/2.8/2.9
17: Max's r-b
Since s/2.8/2.9
18: hashing can fail in comment for qapi
Since s/2.8/2.9
remove dead comment
19: indentation
Max's r-b
20: Max's r-b
21: fix error handling
return 0 if nb_bitmaps == 0
new patches:
22-23: remove bitmap from file on bdrv_release_dirty_bitmap
24: experimental, RFC: cache bitmap list to bs, to not reload it from file.
I'm not sure that is needed now, but if you want I can merge it to earlier
patches.
v9:
rebase on master!
01-04,06,07: John's r-b
03: rewording in comment ("The concurrent resetting ..."), John's r-b
07: reword error messages
commit message: dirty bitmap -> bitmap
09: fix uninitialized ret in bitmap_list_store() (fix compilation warning)
18: fix compilation of tests/test-hbitmap
also, change constants names for qcow2: DIRTY_BITMAP -> BITMAP (as it is not
dirty bitmaps extension, but just bitmaps extension), QCOW -> QCOW2 (bitmaps
are only for qcow2)
v8:
Patches 01-06 are mostly unchanged, except for spelling and wording.
02 - add Eric's r-b
03,04 - add Max's r-b
07-09 is refactored "bitmap-read" part of the feature. Main changes:
- introduce bitmap list - normal list, which represents bitmap directory.
Function bitmap_list_load loads bitmap directory, checks everything and
creates normal list.
- introduce .bdrv_load_autoloading_dirty_bitmaps : instead of loading bitmaps
in qcow2_open, lets load them only in bdrv_open, to avoid reloading in
bdrv_snapshot_goto
- do not delete bitmaps after read, but mark them IN_USE
10 has contextual changes and rewording of comment. I've added Max's r-b, hope it's ok.
11: add error_report("Persistent bitmaps are lost") in case of failed bitmap store
12: add Max's r-b
13 is refactored "bitmap-store" part of the feature. see 07-09 description
- for now I just free old clusters and allocate new. This will be improved
with a separate patch.
patch about "delete bitmaps on truncate" is removed. This case will be handled later.
IN_USE, autoclear, check-constraints things are merged into other patches.
14-15 are mew patches, to early check possibility of creating persistent bitmap with
specified name and granularity in specified BDS
16-17: spelling, rewording, indenting, tiny code simplifying, check can_store (by 14-15),
s/if (autoload && !persistent)/if (has_autoload && !persistent)/,
rebase (deleted qmp-commands.hx)
18: alternative to md5 in query-block:
- separated qmp command -x-debug-block-dirty-bitmap-sha256
- as adviced by Daniel P. Berrange in my parallel thread:
- sha256 instead of md5
- use qcrypto_hash_* instead of GChecksum
- fix bug =) (size was wrong in hbitmap_md5)
19: s/3999/3fff/, use x-debug-block-dirty-bitmap-sha256
20: new patch to rename and publish inc_refcounts
21: some fixes and refactoring mostyly by Max's comments.
Max, Eric, great tanks for your review!
I hope, I've covered most of your comments by this update.
TODO:
- handle reopening image RO->RW and incoming migration, set IN_USE for existing loaded bitmaps
in these cases.
- reuse old, already allocated data clusters for bitmaps storing
- truncate bitmaps in the image on truncate
v7:
https://src.openvz.org/users/vsementsov/repos/qemu/browse?at=refs%2Ftags%2Fqcow2-bitmap-v7
based on block-next (https://github.com/XanClic/qemu/commits/block-next)
- a lot of refactoring and reordering of patches.
- dead code removed (bdrv_dirty_bitmap_load, etc.)
- do not maintain extra data for now
- do not store dirty bitmap directory in memory
(as we use it seldom, we can just reread if needed)
By Kevin's review:
01 - commit message changed: fix->improvement (as it was not a bug)
03 - r-b
04 - r-b
05 - add 21 patch to fix spec, also, removed all (I hope) mentions of
"Bitmap Header", switch to one unified name for it - "Bitmap
Directory Entry", to avoid misunderstanding with Qcow2 header.
(also, add patch 22, to fix it in spec)
v6.06 - improve for_each_dir_entry loop, reorder patches, other small fixes
v6.07 ~> v7.09 - dead code removed, I've moved to one function
.bdrv_store_persistent_bitmaps and have wrapper and callback in one
patch (with also some other staff. If it not ok, I can split them)
v6.08 - about keeping bitmap directory instead of bitmap list: no I don't keep
it at all.
v6.16 - old bdrv_ bitmap-storing related functions are removed. The new one is
bdrv_store_persistent_bitmaps.
v6:
https://src.openvz.org/users/vsementsov/repos/qemu/browse?at=refs%2Ftags%2Fqcow2-bitmap-v6
based on block-next (https://github.com/XanClic/qemu/commits/block-next)
There are a lot of changes, reorderings and additions in comparement with v5.
One principal thing: now bitmaps are removed from image after loading instead
of marking them in_use. It is simpler and we do not need to store superfluous data.
Also, we are no more interested in command line interface to dirty bitmaps.
So it is dropped. If someone needs it I can add it later.
Vladimir Sementsov-Ogievskiy (24):
specs/qcow2: fix bitmap granularity qemu-specific note
specs/qcow2: do not use wording 'bitmap header'
hbitmap: improve dirty iter
tests: add hbitmap iter test
block: fix bdrv_dirty_bitmap_granularity signature
block/dirty-bitmap: add deserialize_ones func
qcow2: add bitmaps extension
block: introduce auto-loading bitmaps
qcow2: add .bdrv_load_autoloading_dirty_bitmaps
block/dirty-bitmap: add autoload field to BdrvDirtyBitmap
block: introduce persistent dirty bitmaps
block/dirty-bitmap: add bdrv_dirty_bitmap_next()
qcow2: add .bdrv_store_persistent_dirty_bitmaps()
block: add bdrv_can_store_new_dirty_bitmap
qcow2: add .bdrv_can_store_new_dirty_bitmap
qmp: add persistent flag to block-dirty-bitmap-add
qmp: add autoload parameter to block-dirty-bitmap-add
qmp: add x-debug-block-dirty-bitmap-sha256
iotests: test qcow2 persistent dirty bitmap
qcow2-refcount: rename inc_refcounts() and make it public
qcow2-bitmap: refcounts
block/dirty-bitmap: deep release dirty bitmaps
qcow2: add .bdrv_remove_persistent_dirty_bitmap
qcow2-bitmap: cache bitmap list in BDRVQcow2State
block.c | 68 ++
block/Makefile.objs | 2 +-
block/dirty-bitmap.c | 84 ++-
block/qcow2-bitmap.c | 1395 ++++++++++++++++++++++++++++++++++++++++++
block/qcow2-refcount.c | 59 +-
block/qcow2.c | 126 +++-
block/qcow2.h | 46 ++
blockdev.c | 61 +-
docs/qmp-commands.txt | 7 +
docs/specs/qcow2.txt | 8 +-
include/block/block.h | 5 +
include/block/block_int.h | 10 +
include/block/dirty-bitmap.h | 18 +-
include/qemu/hbitmap.h | 49 +-
qapi/block-core.json | 38 +-
tests/Makefile.include | 2 +-
tests/qemu-iotests/165 | 89 +++
tests/qemu-iotests/165.out | 5 +
tests/qemu-iotests/group | 1 +
tests/test-hbitmap.c | 19 +
util/hbitmap.c | 51 +-
21 files changed, 2079 insertions(+), 64 deletions(-)
create mode 100644 block/qcow2-bitmap.c
create mode 100755 tests/qemu-iotests/165
create mode 100644 tests/qemu-iotests/165.out
--
2.11.0
^ permalink raw reply [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH 01/24] specs/qcow2: fix bitmap granularity qemu-specific note
2017-01-23 12:10 [Qemu-devel] [PATCH v11 00/24] qcow2: persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
@ 2017-01-23 12:10 ` Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 02/24] specs/qcow2: do not use wording 'bitmap header' Vladimir Sementsov-Ogievskiy
` (22 subsequent siblings)
23 siblings, 0 replies; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-01-23 12:10 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: kwolf, mreitz, armbru, eblake, jsnow, famz, den, stefanha,
vsementsov, pbonzini
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: John Snow <jsnow@redhat.com>
---
docs/specs/qcow2.txt | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
index 80cdfd0e91..dda53dd2a3 100644
--- a/docs/specs/qcow2.txt
+++ b/docs/specs/qcow2.txt
@@ -472,8 +472,7 @@ Structure of a bitmap directory entry:
17: granularity_bits
Granularity bits. Valid values: 0 - 63.
- Note: Qemu currently doesn't support granularity_bits
- greater than 31.
+ Note: Qemu currently supports only values 9 - 31.
Granularity is calculated as
granularity = 1 << granularity_bits
--
2.11.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH 02/24] specs/qcow2: do not use wording 'bitmap header'
2017-01-23 12:10 [Qemu-devel] [PATCH v11 00/24] qcow2: persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 01/24] specs/qcow2: fix bitmap granularity qemu-specific note Vladimir Sementsov-Ogievskiy
@ 2017-01-23 12:10 ` Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 03/24] hbitmap: improve dirty iter Vladimir Sementsov-Ogievskiy
` (21 subsequent siblings)
23 siblings, 0 replies; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-01-23 12:10 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: kwolf, mreitz, armbru, eblake, jsnow, famz, den, stefanha,
vsementsov, pbonzini
A bitmap directory entry is sometimes called a 'bitmap header'. This
patch leaves only one name - 'bitmap directory entry'. The name 'bitmap
header' creates misunderstandings with 'qcow2 header' and 'qcow2 bitmap
header extension' (which is extension of qcow2 header)
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
---
docs/specs/qcow2.txt | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
index dda53dd2a3..8874e8c774 100644
--- a/docs/specs/qcow2.txt
+++ b/docs/specs/qcow2.txt
@@ -201,7 +201,7 @@ The fields of the bitmaps extension are:
8 - 15: bitmap_directory_size
Size of the bitmap directory in bytes. It is the cumulative
- size of all (nb_bitmaps) bitmap headers.
+ size of all (nb_bitmaps) bitmap directory entries.
16 - 23: bitmap_directory_offset
Offset into the image file at which the bitmap directory
@@ -426,8 +426,7 @@ Each bitmap saved in the image is described in a bitmap directory entry. The
bitmap directory is a contiguous area in the image file, whose starting offset
and length are given by the header extension fields bitmap_directory_offset and
bitmap_directory_size. The entries of the bitmap directory have variable
-length, depending on the lengths of the bitmap name and extra data. These
-entries are also called bitmap headers.
+length, depending on the lengths of the bitmap name and extra data.
Structure of a bitmap directory entry:
--
2.11.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH 03/24] hbitmap: improve dirty iter
2017-01-23 12:10 [Qemu-devel] [PATCH v11 00/24] qcow2: persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 01/24] specs/qcow2: fix bitmap granularity qemu-specific note Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 02/24] specs/qcow2: do not use wording 'bitmap header' Vladimir Sementsov-Ogievskiy
@ 2017-01-23 12:10 ` Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 04/24] tests: add hbitmap iter test Vladimir Sementsov-Ogievskiy
` (20 subsequent siblings)
23 siblings, 0 replies; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-01-23 12:10 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: kwolf, mreitz, armbru, eblake, jsnow, famz, den, stefanha,
vsementsov, pbonzini
Make dirty iter resistant to resetting bits in corresponding HBitmap.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
---
include/qemu/hbitmap.h | 26 ++++----------------------
util/hbitmap.c | 23 ++++++++++++++++++++++-
2 files changed, 26 insertions(+), 23 deletions(-)
diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
index eb464759d5..2965f01383 100644
--- a/include/qemu/hbitmap.h
+++ b/include/qemu/hbitmap.h
@@ -243,10 +243,9 @@ void hbitmap_free(HBitmap *hb);
* the lowest-numbered bit that is set in @hb, starting at @first.
*
* Concurrent setting of bits is acceptable, and will at worst cause the
- * iteration to miss some of those bits. Resetting bits before the current
- * position of the iterator is also okay. However, concurrent resetting of
- * bits can lead to unexpected behavior if the iterator has not yet reached
- * those bits.
+ * iteration to miss some of those bits.
+ *
+ * The concurrent resetting of bits is OK.
*/
void hbitmap_iter_init(HBitmapIter *hbi, const HBitmap *hb, uint64_t first);
@@ -285,24 +284,7 @@ void hbitmap_free_meta(HBitmap *hb);
* Return the next bit that is set in @hbi's associated HBitmap,
* or -1 if all remaining bits are zero.
*/
-static inline int64_t hbitmap_iter_next(HBitmapIter *hbi)
-{
- unsigned long cur = hbi->cur[HBITMAP_LEVELS - 1];
- int64_t item;
-
- if (cur == 0) {
- cur = hbitmap_iter_skip_words(hbi);
- if (cur == 0) {
- return -1;
- }
- }
-
- /* The next call will resume work from the next bit. */
- hbi->cur[HBITMAP_LEVELS - 1] = cur & (cur - 1);
- item = ((uint64_t)hbi->pos << BITS_PER_LEVEL) + ctzl(cur);
-
- return item << hbi->granularity;
-}
+int64_t hbitmap_iter_next(HBitmapIter *hbi);
/**
* hbitmap_iter_next_word:
diff --git a/util/hbitmap.c b/util/hbitmap.c
index 9f691b76bd..1dbc8ba8b7 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -106,8 +106,9 @@ unsigned long hbitmap_iter_skip_words(HBitmapIter *hbi)
unsigned long cur;
do {
- cur = hbi->cur[--i];
+ i--;
pos >>= BITS_PER_LEVEL;
+ cur = hbi->cur[i] & hb->levels[i][pos];
} while (cur == 0);
/* Check for end of iteration. We always use fewer than BITS_PER_LONG
@@ -139,6 +140,26 @@ unsigned long hbitmap_iter_skip_words(HBitmapIter *hbi)
return cur;
}
+int64_t hbitmap_iter_next(HBitmapIter *hbi)
+{
+ unsigned long cur = hbi->cur[HBITMAP_LEVELS - 1] &
+ hbi->hb->levels[HBITMAP_LEVELS - 1][hbi->pos];
+ int64_t item;
+
+ if (cur == 0) {
+ cur = hbitmap_iter_skip_words(hbi);
+ if (cur == 0) {
+ return -1;
+ }
+ }
+
+ /* The next call will resume work from the next bit. */
+ hbi->cur[HBITMAP_LEVELS - 1] = cur & (cur - 1);
+ item = ((uint64_t)hbi->pos << BITS_PER_LEVEL) + ctzl(cur);
+
+ return item << hbi->granularity;
+}
+
void hbitmap_iter_init(HBitmapIter *hbi, const HBitmap *hb, uint64_t first)
{
unsigned i, bit;
--
2.11.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH 04/24] tests: add hbitmap iter test
2017-01-23 12:10 [Qemu-devel] [PATCH v11 00/24] qcow2: persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
` (2 preceding siblings ...)
2017-01-23 12:10 ` [Qemu-devel] [PATCH 03/24] hbitmap: improve dirty iter Vladimir Sementsov-Ogievskiy
@ 2017-01-23 12:10 ` Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 05/24] block: fix bdrv_dirty_bitmap_granularity signature Vladimir Sementsov-Ogievskiy
` (19 subsequent siblings)
23 siblings, 0 replies; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-01-23 12:10 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: kwolf, mreitz, armbru, eblake, jsnow, famz, den, stefanha,
vsementsov, pbonzini
Test that hbitmap iter is resistant to bitmap resetting.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
---
tests/test-hbitmap.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/tests/test-hbitmap.c b/tests/test-hbitmap.c
index 9b7495cc32..c76d2b5c52 100644
--- a/tests/test-hbitmap.c
+++ b/tests/test-hbitmap.c
@@ -898,6 +898,22 @@ static void hbitmap_test_add(const char *testpath,
hbitmap_test_teardown);
}
+static void test_hbitmap_iter_and_reset(TestHBitmapData *data,
+ const void *unused)
+{
+ HBitmapIter hbi;
+
+ hbitmap_test_init(data, L1 * 2, 0);
+ hbitmap_set(data->hb, 0, data->size);
+
+ hbitmap_iter_init(&hbi, data->hb, BITS_PER_LONG - 1);
+
+ hbitmap_iter_next(&hbi);
+
+ hbitmap_reset_all(data->hb);
+ hbitmap_iter_next(&hbi);
+}
+
int main(int argc, char **argv)
{
g_test_init(&argc, &argv, NULL);
@@ -955,6 +971,9 @@ int main(int argc, char **argv)
test_hbitmap_serialize_part);
hbitmap_test_add("/hbitmap/serialize/zeroes",
test_hbitmap_serialize_zeroes);
+
+ hbitmap_test_add("/hbitmap/iter/iter_and_reset",
+ test_hbitmap_iter_and_reset);
g_test_run();
return 0;
--
2.11.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH 05/24] block: fix bdrv_dirty_bitmap_granularity signature
2017-01-23 12:10 [Qemu-devel] [PATCH v11 00/24] qcow2: persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
` (3 preceding siblings ...)
2017-01-23 12:10 ` [Qemu-devel] [PATCH 04/24] tests: add hbitmap iter test Vladimir Sementsov-Ogievskiy
@ 2017-01-23 12:10 ` Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 06/24] block/dirty-bitmap: add deserialize_ones func Vladimir Sementsov-Ogievskiy
` (18 subsequent siblings)
23 siblings, 0 replies; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-01-23 12:10 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: kwolf, mreitz, armbru, eblake, jsnow, famz, den, stefanha,
vsementsov, pbonzini
Make getter signature const-correct. This allows other functions with
const dirty bitmap parameter use bdrv_dirty_bitmap_granularity().
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/dirty-bitmap.c | 2 +-
include/block/dirty-bitmap.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 519737c8d3..186941cfc3 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -388,7 +388,7 @@ uint32_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs)
return granularity;
}
-uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap *bitmap)
+uint32_t bdrv_dirty_bitmap_granularity(const BdrvDirtyBitmap *bitmap)
{
return BDRV_SECTOR_SIZE << hbitmap_granularity(bitmap->bitmap);
}
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 9dea14ba03..7cbe623ba7 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -29,7 +29,7 @@ void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap);
void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap);
BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
uint32_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs);
-uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap *bitmap);
+uint32_t bdrv_dirty_bitmap_granularity(const BdrvDirtyBitmap *bitmap);
uint32_t bdrv_dirty_bitmap_meta_granularity(BdrvDirtyBitmap *bitmap);
bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap);
bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap);
--
2.11.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH 06/24] block/dirty-bitmap: add deserialize_ones func
2017-01-23 12:10 [Qemu-devel] [PATCH v11 00/24] qcow2: persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
` (4 preceding siblings ...)
2017-01-23 12:10 ` [Qemu-devel] [PATCH 05/24] block: fix bdrv_dirty_bitmap_granularity signature Vladimir Sementsov-Ogievskiy
@ 2017-01-23 12:10 ` Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 07/24] qcow2: add bitmaps extension Vladimir Sementsov-Ogievskiy
` (17 subsequent siblings)
23 siblings, 0 replies; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-01-23 12:10 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: kwolf, mreitz, armbru, eblake, jsnow, famz, den, stefanha,
vsementsov, pbonzini
Add bdrv_dirty_bitmap_deserialize_ones() function, which is needed for
qcow2 bitmap loading, to handle unallocated bitmap parts, marked as
all-ones.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
---
block/dirty-bitmap.c | 7 +++++++
include/block/dirty-bitmap.h | 3 +++
include/qemu/hbitmap.h | 15 +++++++++++++++
util/hbitmap.c | 17 +++++++++++++++++
4 files changed, 42 insertions(+)
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 186941cfc3..90af37287f 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -499,6 +499,13 @@ void bdrv_dirty_bitmap_deserialize_zeroes(BdrvDirtyBitmap *bitmap,
hbitmap_deserialize_zeroes(bitmap->bitmap, start, count, finish);
}
+void bdrv_dirty_bitmap_deserialize_ones(BdrvDirtyBitmap *bitmap,
+ uint64_t start, uint64_t count,
+ bool finish)
+{
+ hbitmap_deserialize_ones(bitmap->bitmap, start, count, finish);
+}
+
void bdrv_dirty_bitmap_deserialize_finish(BdrvDirtyBitmap *bitmap)
{
hbitmap_deserialize_finish(bitmap->bitmap);
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 7cbe623ba7..1e17729ac2 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -70,6 +70,9 @@ void bdrv_dirty_bitmap_deserialize_part(BdrvDirtyBitmap *bitmap,
void bdrv_dirty_bitmap_deserialize_zeroes(BdrvDirtyBitmap *bitmap,
uint64_t start, uint64_t count,
bool finish);
+void bdrv_dirty_bitmap_deserialize_ones(BdrvDirtyBitmap *bitmap,
+ uint64_t start, uint64_t count,
+ bool finish);
void bdrv_dirty_bitmap_deserialize_finish(BdrvDirtyBitmap *bitmap);
#endif
diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
index 2965f01383..063ec0e7ad 100644
--- a/include/qemu/hbitmap.h
+++ b/include/qemu/hbitmap.h
@@ -216,6 +216,21 @@ void hbitmap_deserialize_zeroes(HBitmap *hb, uint64_t start, uint64_t count,
bool finish);
/**
+ * hbitmap_deserialize_ones
+ * @hb: HBitmap to operate on.
+ * @start: First bit to restore.
+ * @count: Number of bits to restore.
+ * @finish: Whether to call hbitmap_deserialize_finish automatically.
+ *
+ * Fills the bitmap with ones.
+ *
+ * If @finish is false, caller must call hbitmap_serialize_finish before using
+ * the bitmap.
+ */
+void hbitmap_deserialize_ones(HBitmap *hb, uint64_t start, uint64_t count,
+ bool finish);
+
+/**
* hbitmap_deserialize_finish
* @hb: HBitmap to operate on.
*
diff --git a/util/hbitmap.c b/util/hbitmap.c
index 1dbc8ba8b7..47418a915e 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -535,6 +535,23 @@ void hbitmap_deserialize_zeroes(HBitmap *hb, uint64_t start, uint64_t count,
}
}
+void hbitmap_deserialize_ones(HBitmap *hb, uint64_t start, uint64_t count,
+ bool finish)
+{
+ uint64_t el_count;
+ unsigned long *first;
+
+ if (!count) {
+ return;
+ }
+ serialization_chunk(hb, start, count, &first, &el_count);
+
+ memset(first, 0xff, el_count * sizeof(unsigned long));
+ if (finish) {
+ hbitmap_deserialize_finish(hb);
+ }
+}
+
void hbitmap_deserialize_finish(HBitmap *bitmap)
{
int64_t i, size, prev_size;
--
2.11.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH 07/24] qcow2: add bitmaps extension
2017-01-23 12:10 [Qemu-devel] [PATCH v11 00/24] qcow2: persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
` (5 preceding siblings ...)
2017-01-23 12:10 ` [Qemu-devel] [PATCH 06/24] block/dirty-bitmap: add deserialize_ones func Vladimir Sementsov-Ogievskiy
@ 2017-01-23 12:10 ` Vladimir Sementsov-Ogievskiy
2017-01-28 21:29 ` Max Reitz
2017-01-31 11:53 ` Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 08/24] block: introduce auto-loading bitmaps Vladimir Sementsov-Ogievskiy
` (16 subsequent siblings)
23 siblings, 2 replies; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-01-23 12:10 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: kwolf, mreitz, armbru, eblake, jsnow, famz, den, stefanha,
vsementsov, pbonzini
Add bitmap extension as specified in docs/specs/qcow2.txt.
For now, just mirror extension header into Qcow2 state and check
constraints.
For now, disable image resize if it has bitmaps. It will be fixed later.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/qcow2.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
block/qcow2.h | 24 ++++++++++++
2 files changed, 141 insertions(+), 2 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 96fb8a8f16..a8835988c7 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -63,6 +63,7 @@ typedef struct {
#define QCOW2_EXT_MAGIC_END 0
#define QCOW2_EXT_MAGIC_BACKING_FORMAT 0xE2792ACA
#define QCOW2_EXT_MAGIC_FEATURE_TABLE 0x6803f857
+#define QCOW2_EXT_MAGIC_BITMAPS 0x23852875
static int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename)
{
@@ -92,6 +93,8 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
QCowExtension ext;
uint64_t offset;
int ret;
+ Qcow2BitmapHeaderExt bitmaps_ext;
+ bool need_update;
#ifdef DEBUG_EXT
printf("qcow2_read_extensions: start=%ld end=%ld\n", start_offset, end_offset);
@@ -162,6 +165,83 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
}
break;
+ case QCOW2_EXT_MAGIC_BITMAPS:
+ if (ext.len != sizeof(bitmaps_ext)) {
+ error_setg_errno(errp, -ret, "bitmaps_ext: "
+ "Invalid extension length");
+ return -EINVAL;
+ }
+
+ if (!(s->autoclear_features & QCOW2_AUTOCLEAR_BITMAPS)) {
+ fprintf(stderr,
+ "WARNING: a program lacking bitmap support modified "
+ "this file, so all bitmaps are now considered "
+ "inconsistent. Some clusters may be leaked, run "
+ "'qemu-img check -r' on the image file to fix.");
+ need_update = true;
+ break;
+ }
+
+ ret = bdrv_pread(bs->file, offset, &bitmaps_ext, ext.len);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "bitmaps_ext: "
+ "Could not read ext header");
+ return ret;
+ }
+
+ if (bitmaps_ext.reserved32 != 0) {
+ error_setg_errno(errp, -ret, "bitmaps_ext: "
+ "Reserved field is not zero");
+ return -EINVAL;
+ }
+
+ be32_to_cpus(&bitmaps_ext.nb_bitmaps);
+ be64_to_cpus(&bitmaps_ext.bitmap_directory_size);
+ be64_to_cpus(&bitmaps_ext.bitmap_directory_offset);
+
+ if (bitmaps_ext.nb_bitmaps > QCOW2_MAX_BITMAPS) {
+ error_setg(errp,
+ "bitmaps_ext: File %s has %" PRIu32 " bitmaps, "
+ "exceeding the QEMU supported maximum of %d",
+ bs->filename, bitmaps_ext.nb_bitmaps,
+ QCOW2_MAX_BITMAPS);
+ return -EINVAL;
+ }
+
+ if (bitmaps_ext.nb_bitmaps == 0) {
+ error_setg(errp, "found bitmaps extension with zero bitmaps");
+ return -EINVAL;
+ }
+
+ if (bitmaps_ext.bitmap_directory_offset & (s->cluster_size - 1)) {
+ error_setg(errp, "bitmaps_ext: "
+ "invalid bitmap directory offset");
+ return -EINVAL;
+ }
+
+ if (bitmaps_ext.bitmap_directory_size >
+ QCOW2_MAX_BITMAP_DIRECTORY_SIZE) {
+ error_setg(errp, "bitmaps_ext: "
+ "bitmap directory size (%" PRIu64 ") exceeds "
+ "the maximum supported size (%d)",
+ bitmaps_ext.bitmap_directory_size,
+ QCOW2_MAX_BITMAP_DIRECTORY_SIZE);
+ return -EINVAL;
+ }
+
+ s->nb_bitmaps = bitmaps_ext.nb_bitmaps;
+ s->bitmap_directory_offset =
+ bitmaps_ext.bitmap_directory_offset;
+ s->bitmap_directory_size =
+ bitmaps_ext.bitmap_directory_size;
+
+#ifdef DEBUG_EXT
+ printf("Qcow2: Got bitmaps extension: "
+ "offset=%" PRIu64 " nb_bitmaps=%" PRIu32 "\n",
+ s->bitmap_directory_offset, s->nb_bitmaps);
+#endif
+ break;
+
default:
/* unknown magic - save it in case we need to rewrite the header */
{
@@ -185,6 +265,15 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
offset += ((ext.len + 7) & ~7);
}
+ if (need_update && !bdrv_is_root_node(bs) &&
+ !(bdrv_get_flags(bs) & BDRV_O_INACTIVE))
+ {
+ ret = qcow2_update_header(bs);
+ if (ret < 0) {
+ return ret;
+ }
+ }
+
return 0;
}
@@ -1152,8 +1241,9 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
}
/* Clear unknown autoclear feature bits */
- if (!bs->read_only && !(flags & BDRV_O_INACTIVE) && s->autoclear_features) {
- s->autoclear_features = 0;
+ if (!bs->read_only && !(flags & BDRV_O_INACTIVE) &&
+ (s->autoclear_features & ~QCOW2_AUTOCLEAR_MASK)) {
+ s->autoclear_features &= QCOW2_AUTOCLEAR_MASK;
ret = qcow2_update_header(bs);
if (ret < 0) {
error_setg_errno(errp, -ret, "Could not update qcow2 header");
@@ -1953,6 +2043,24 @@ int qcow2_update_header(BlockDriverState *bs)
buflen -= ret;
}
+ if (s->nb_bitmaps > 0) {
+ Qcow2BitmapHeaderExt bitmaps_header = {
+ .nb_bitmaps = cpu_to_be32(s->nb_bitmaps),
+ .bitmap_directory_size =
+ cpu_to_be64(s->bitmap_directory_size),
+ .bitmap_directory_offset =
+ cpu_to_be64(s->bitmap_directory_offset)
+ };
+ ret = header_ext_add(buf, QCOW2_EXT_MAGIC_BITMAPS,
+ &bitmaps_header, sizeof(bitmaps_header),
+ buflen);
+ if (ret < 0) {
+ goto fail;
+ }
+ buf += ret;
+ buflen -= ret;
+ }
+
/* Keep unknown header extensions */
QLIST_FOREACH(uext, &s->unknown_header_ext, next) {
ret = header_ext_add(buf, uext->magic, uext->data, uext->len, buflen);
@@ -2528,6 +2636,13 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset)
return -ENOTSUP;
}
+ /* cannot proceed if image has bitmaps */
+ if (s->nb_bitmaps) {
+ /* TODO: resize bitmaps in the image */
+ error_report("Can't resize an image which has bitmaps");
+ return -ENOTSUP;
+ }
+
/* shrinking is currently not supported */
if (offset < bs->total_sectors * 512) {
error_report("qcow2 doesn't support shrinking images yet");
diff --git a/block/qcow2.h b/block/qcow2.h
index 182341483a..861b5011dd 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -52,6 +52,10 @@
* space for snapshot names and IDs */
#define QCOW_MAX_SNAPSHOTS_SIZE (1024 * QCOW_MAX_SNAPSHOTS)
+/* Bitmap header extension constraints */
+#define QCOW2_MAX_BITMAPS 65535
+#define QCOW2_MAX_BITMAP_DIRECTORY_SIZE (1024 * QCOW2_MAX_BITMAPS)
+
/* indicate that the refcount of the referenced cluster is exactly one. */
#define QCOW_OFLAG_COPIED (1ULL << 63)
/* indicate that the cluster is compressed (they never have the copied flag) */
@@ -195,6 +199,15 @@ enum {
QCOW2_COMPAT_FEAT_MASK = QCOW2_COMPAT_LAZY_REFCOUNTS,
};
+/* Autoclear feature bits */
+enum {
+ QCOW2_AUTOCLEAR_BITMAPS_BITNR = 0,
+ QCOW2_AUTOCLEAR_BITMAPS =
+ 1 << QCOW2_AUTOCLEAR_BITMAPS_BITNR,
+
+ QCOW2_AUTOCLEAR_MASK = QCOW2_AUTOCLEAR_BITMAPS,
+};
+
enum qcow2_discard_type {
QCOW2_DISCARD_NEVER = 0,
QCOW2_DISCARD_ALWAYS,
@@ -222,6 +235,13 @@ typedef uint64_t Qcow2GetRefcountFunc(const void *refcount_array,
typedef void Qcow2SetRefcountFunc(void *refcount_array,
uint64_t index, uint64_t value);
+typedef struct Qcow2BitmapHeaderExt {
+ uint32_t nb_bitmaps;
+ uint32_t reserved32;
+ uint64_t bitmap_directory_size;
+ uint64_t bitmap_directory_offset;
+} QEMU_PACKED Qcow2BitmapHeaderExt;
+
typedef struct BDRVQcow2State {
int cluster_bits;
int cluster_size;
@@ -263,6 +283,10 @@ typedef struct BDRVQcow2State {
unsigned int nb_snapshots;
QCowSnapshot *snapshots;
+ uint32_t nb_bitmaps;
+ uint64_t bitmap_directory_size;
+ uint64_t bitmap_directory_offset;
+
int flags;
int qcow_version;
bool use_lazy_refcounts;
--
2.11.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH 08/24] block: introduce auto-loading bitmaps
2017-01-23 12:10 [Qemu-devel] [PATCH v11 00/24] qcow2: persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
` (6 preceding siblings ...)
2017-01-23 12:10 ` [Qemu-devel] [PATCH 07/24] qcow2: add bitmaps extension Vladimir Sementsov-Ogievskiy
@ 2017-01-23 12:10 ` Vladimir Sementsov-Ogievskiy
2017-01-28 21:31 ` Max Reitz
2017-01-23 12:10 ` [Qemu-devel] [PATCH 09/24] qcow2: add .bdrv_load_autoloading_dirty_bitmaps Vladimir Sementsov-Ogievskiy
` (15 subsequent siblings)
23 siblings, 1 reply; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-01-23 12:10 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: kwolf, mreitz, armbru, eblake, jsnow, famz, den, stefanha,
vsementsov, pbonzini
Auto loading bitmaps are bitmaps stored in the disk image, which should
be loaded when the image is opened and become BdrvDirtyBitmaps for the
corresponding drive.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: John Snow <jsnow@redhat.com>
---
block.c | 14 ++++++++++++++
include/block/block.h | 2 ++
include/block/block_int.h | 3 +++
3 files changed, 19 insertions(+)
diff --git a/block.c b/block.c
index 39ddea3411..10951767f3 100644
--- a/block.c
+++ b/block.c
@@ -1141,6 +1141,13 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
assert(bdrv_min_mem_align(bs) != 0);
assert(is_power_of_2(bs->bl.request_alignment));
+ bdrv_load_autoloading_dirty_bitmaps(bs, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ ret = -EINVAL;
+ goto free_and_fail;
+ }
+
qemu_opts_del(opts);
return 0;
@@ -4093,3 +4100,10 @@ void bdrv_del_child(BlockDriverState *parent_bs, BdrvChild *child, Error **errp)
parent_bs->drv->bdrv_del_child(parent_bs, child, errp);
}
+
+void bdrv_load_autoloading_dirty_bitmaps(BlockDriverState *bs, Error **errp)
+{
+ if (bs->drv && bs->drv->bdrv_load_autoloading_dirty_bitmaps) {
+ bs->drv->bdrv_load_autoloading_dirty_bitmaps(bs, errp);
+ }
+}
diff --git a/include/block/block.h b/include/block/block.h
index 49bb0b239a..f701da2c5d 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -552,4 +552,6 @@ void bdrv_add_child(BlockDriverState *parent, BlockDriverState *child,
Error **errp);
void bdrv_del_child(BlockDriverState *parent, BdrvChild *child, Error **errp);
+void bdrv_load_autoloading_dirty_bitmaps(BlockDriverState *bs, Error **errp);
+
#endif
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 83a423c580..ca955f08b9 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -320,6 +320,9 @@ struct BlockDriver {
void (*bdrv_del_child)(BlockDriverState *parent, BdrvChild *child,
Error **errp);
+ void (*bdrv_load_autoloading_dirty_bitmaps)(BlockDriverState *bs,
+ Error **errp);
+
QLIST_ENTRY(BlockDriver) list;
};
--
2.11.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH 09/24] qcow2: add .bdrv_load_autoloading_dirty_bitmaps
2017-01-23 12:10 [Qemu-devel] [PATCH v11 00/24] qcow2: persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
` (7 preceding siblings ...)
2017-01-23 12:10 ` [Qemu-devel] [PATCH 08/24] block: introduce auto-loading bitmaps Vladimir Sementsov-Ogievskiy
@ 2017-01-23 12:10 ` Vladimir Sementsov-Ogievskiy
2017-01-28 21:45 ` Max Reitz
2017-01-28 22:20 ` Max Reitz
2017-01-23 12:10 ` [Qemu-devel] [PATCH 10/24] block/dirty-bitmap: add autoload field to BdrvDirtyBitmap Vladimir Sementsov-Ogievskiy
` (14 subsequent siblings)
23 siblings, 2 replies; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-01-23 12:10 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: kwolf, mreitz, armbru, eblake, jsnow, famz, den, stefanha,
vsementsov, pbonzini
Auto loading bitmaps are bitmaps in Qcow2, with the AUTO flag set. They
are loaded when the image is opened and become BdrvDirtyBitmaps for the
corresponding drive.
Extra data in bitmaps is not supported for now.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/Makefile.objs | 2 +-
block/qcow2-bitmap.c | 712 +++++++++++++++++++++++++++++++++++++++++++++++++++
block/qcow2.c | 2 +
block/qcow2.h | 3 +
4 files changed, 718 insertions(+), 1 deletion(-)
create mode 100644 block/qcow2-bitmap.c
diff --git a/block/Makefile.objs b/block/Makefile.objs
index 67a036a1df..b8b6cf38a0 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -1,5 +1,5 @@
block-obj-y += raw_bsd.o qcow.o vdi.o vmdk.o cloop.o bochs.o vpc.o vvfat.o dmg.o
-block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-cache.o
+block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-cache.o qcow2-bitmap.o
block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
block-obj-y += qed-check.o
block-obj-y += vhdx.o vhdx-endian.o vhdx-log.o
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
new file mode 100644
index 0000000000..758622dcfc
--- /dev/null
+++ b/block/qcow2-bitmap.c
@@ -0,0 +1,712 @@
+/*
+ * Bitmaps for the QCOW version 2 format
+ *
+ * Copyright (c) 2014-2016 Vladimir Sementsov-Ogievskiy
+ *
+ * This file is derived from qcow2-snapshot.c, original copyright:
+ * Copyright (c) 2004-2006 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "exec/log.h"
+
+#include "block/block_int.h"
+#include "block/qcow2.h"
+
+/* NOTICE: BME here means Bitmaps Extension and used as a namespace for
+ * _internal_ constants. Please do not use this _internal_ abbreviation for
+ * other needs and/or outside of this file. */
+
+/* Bitmap directory entry constraints */
+#define BME_MAX_TABLE_SIZE 0x8000000
+#define BME_MAX_PHYS_SIZE 0x20000000 /* restrict BdrvDirtyBitmap size in RAM */
+#define BME_MAX_GRANULARITY_BITS 31
+#define BME_MIN_GRANULARITY_BITS 9
+#define BME_MAX_NAME_SIZE 1023
+
+/* Bitmap directory entry flags */
+#define BME_RESERVED_FLAGS 0xfffffffcU
+#define BME_FLAG_IN_USE 1
+#define BME_FLAG_AUTO (1U << 1)
+
+/* bits [1, 8] U [56, 63] are reserved */
+#define BME_TABLE_ENTRY_RESERVED_MASK 0xff000000000001feULL
+#define BME_TABLE_ENTRY_OFFSET_MASK 0x00fffffffffffe00ULL
+
+typedef struct QEMU_PACKED Qcow2BitmapDirEntry {
+ /* header is 8 byte aligned */
+ uint64_t bitmap_table_offset;
+
+ uint32_t bitmap_table_size;
+ uint32_t flags;
+
+ uint8_t type;
+ uint8_t granularity_bits;
+ uint16_t name_size;
+ uint32_t extra_data_size;
+ /* extra data follows */
+ /* name follows */
+} Qcow2BitmapDirEntry;
+
+typedef struct Qcow2Bitmap {
+ uint64_t table_offset;
+ uint32_t table_size;
+ uint32_t flags;
+ uint8_t granularity_bits;
+ char *name;
+
+ QSIMPLEQ_ENTRY(Qcow2Bitmap) entry;
+} Qcow2Bitmap;
+typedef QSIMPLEQ_HEAD(Qcow2BitmapList, Qcow2Bitmap) Qcow2BitmapList;
+
+typedef enum BitmapType {
+ BT_DIRTY_TRACKING_BITMAP = 1
+} BitmapType;
+
+static inline bool can_write(BlockDriverState *bs)
+{
+ return !bdrv_is_read_only(bs) && !(bdrv_get_flags(bs) & BDRV_O_INACTIVE);
+}
+
+static inline void bitmap_table_to_cpu(uint64_t *bitmap_table, size_t size)
+{
+ size_t i;
+
+ for (i = 0; i < size; ++i) {
+ be64_to_cpus(&bitmap_table[i]);
+ }
+}
+
+static int check_table_entry(uint64_t entry, int cluster_size)
+{
+ uint64_t offset;
+
+ if (cluster_size <= 0) {
+ return -EINVAL;
+ }
+
+ if (entry & BME_TABLE_ENTRY_RESERVED_MASK) {
+ return -EINVAL;
+ }
+
+ offset = entry & BME_TABLE_ENTRY_OFFSET_MASK;
+ if (offset != 0) {
+ /* if offset specified, bit 0 is reserved */
+ if (entry & 1) {
+ return -EINVAL;
+ }
+
+ if (offset % cluster_size != 0) {
+ return -EINVAL;
+ }
+ }
+
+ return 0;
+}
+
+static int bitmap_table_load(BlockDriverState *bs, Qcow2Bitmap *bm,
+ uint64_t **bitmap_table)
+{
+ int ret;
+ BDRVQcow2State *s = bs->opaque;
+ uint32_t i;
+ uint64_t *table;
+
+ assert(bm->table_size != 0);
+ table = g_try_new(uint64_t, bm->table_size);
+ if (table == NULL) {
+ return -ENOMEM;
+ }
+
+ assert(bm->table_size <= BME_MAX_TABLE_SIZE);
+ ret = bdrv_pread(bs->file, bm->table_offset,
+ table, bm->table_size * sizeof(uint64_t));
+ if (ret < 0) {
+ goto fail;
+ }
+
+ for (i = 0; i < bm->table_size; ++i) {
+ be64_to_cpus(&table[i]);
+ ret = check_table_entry(table[i], s->cluster_size);
+ if (ret < 0) {
+ goto fail;
+ }
+ }
+
+ *bitmap_table = table;
+ return 0;
+
+fail:
+ g_free(table);
+
+ return ret;
+}
+
+/* This function returns the number of disk sectors covered by a single cluster
+ * of bitmap data. */
+static uint64_t disk_sectors_in_bitmap_cluster(const BDRVQcow2State *s,
+ const BdrvDirtyBitmap *bitmap)
+{
+ uint32_t sector_granularity =
+ bdrv_dirty_bitmap_granularity(bitmap) >> BDRV_SECTOR_BITS;
+
+ return (uint64_t)sector_granularity * (s->cluster_size << 3);
+}
+
+/* bitmap table entries must satisfy specification constraints */
+static int load_bitmap_data(BlockDriverState *bs,
+ const uint64_t *bitmap_table,
+ uint32_t bitmap_table_size,
+ BdrvDirtyBitmap *bitmap)
+{
+ int ret = 0;
+ BDRVQcow2State *s = bs->opaque;
+ uint64_t sector, dsc;
+ uint64_t bm_size = bdrv_dirty_bitmap_size(bitmap);
+ uint8_t *buf = NULL;
+ uint64_t i, tab_size =
+ size_to_clusters(s,
+ bdrv_dirty_bitmap_serialization_size(bitmap, 0, bm_size));
+
+ if (tab_size != bitmap_table_size || tab_size > BME_MAX_TABLE_SIZE) {
+ return -EINVAL;
+ }
+
+ bdrv_clear_dirty_bitmap(bitmap, NULL);
+
+ buf = g_malloc(s->cluster_size);
+ dsc = disk_sectors_in_bitmap_cluster(s, bitmap);
+ for (i = 0, sector = 0; i < tab_size; ++i, sector += dsc) {
+ uint64_t count = MIN(bm_size - sector, dsc);
+ uint64_t entry = bitmap_table[i];
+ uint64_t offset = entry & BME_TABLE_ENTRY_OFFSET_MASK;
+
+ assert(check_table_entry(entry, s->cluster_size) == 0);
+
+ if (offset == 0) {
+ if (entry & 1) {
+ bdrv_dirty_bitmap_deserialize_ones(bitmap, sector, count,
+ false);
+ } else {
+ /* No need to deserialize zeros because the dirty bitmap is
+ * already cleared */
+ }
+ } else {
+ ret = bdrv_pread(bs->file, offset, buf, s->cluster_size);
+ if (ret < 0) {
+ goto finish;
+ }
+ bdrv_dirty_bitmap_deserialize_part(bitmap, buf, sector, count,
+ false);
+ }
+ }
+ ret = 0;
+
+ bdrv_dirty_bitmap_deserialize_finish(bitmap);
+
+finish:
+ g_free(buf);
+
+ return ret;
+}
+
+static BdrvDirtyBitmap *load_bitmap(BlockDriverState *bs,
+ Qcow2Bitmap *bm, Error **errp)
+{
+ int ret;
+ uint64_t *bitmap_table = NULL;
+ uint32_t granularity;
+ BdrvDirtyBitmap *bitmap = NULL;
+
+ if (bm->flags & BME_FLAG_IN_USE) {
+ error_setg(errp, "Bitmap '%s' is in use", bm->name);
+ goto fail;
+ }
+
+ ret = bitmap_table_load(bs, bm, &bitmap_table);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret,
+ "Could not read bitmap_table table from image for "
+ "bitmap '%s'", bm->name);
+ goto fail;
+ }
+
+ granularity = 1U << bm->granularity_bits;
+ bitmap = bdrv_create_dirty_bitmap(bs, granularity, bm->name, errp);
+ if (bitmap == NULL) {
+ goto fail;
+ }
+
+ ret = load_bitmap_data(bs, bitmap_table, bm->table_size,
+ bitmap);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not read bitmap '%s' from image",
+ bm->name);
+ goto fail;
+ }
+
+ g_free(bitmap_table);
+ return bitmap;
+
+fail:
+ g_free(bitmap_table);
+ if (bitmap != NULL) {
+ bdrv_release_dirty_bitmap(bs, bitmap);
+ }
+
+ return NULL;
+}
+
+/*
+ * Bitmap List
+ */
+
+/*
+ * Bitmap List private functions
+ * Only Bitmap List knows about bitmap directory structure in Qcow2.
+ */
+
+static inline void bitmap_dir_entry_to_cpu(Qcow2BitmapDirEntry *entry)
+{
+ be64_to_cpus(&entry->bitmap_table_offset);
+ be32_to_cpus(&entry->bitmap_table_size);
+ be32_to_cpus(&entry->flags);
+ be16_to_cpus(&entry->name_size);
+ be32_to_cpus(&entry->extra_data_size);
+}
+
+static inline void bitmap_dir_entry_to_be(Qcow2BitmapDirEntry *entry)
+{
+ cpu_to_be64s(&entry->bitmap_table_offset);
+ cpu_to_be32s(&entry->bitmap_table_size);
+ cpu_to_be32s(&entry->flags);
+ cpu_to_be16s(&entry->name_size);
+ cpu_to_be32s(&entry->extra_data_size);
+}
+
+static inline int calc_dir_entry_size(size_t name_size, size_t extra_data_size)
+{
+ return align_offset(sizeof(Qcow2BitmapDirEntry) +
+ name_size + extra_data_size, 8);
+}
+
+static inline int dir_entry_size(Qcow2BitmapDirEntry *entry)
+{
+ return calc_dir_entry_size(entry->name_size, entry->extra_data_size);
+}
+
+static inline const char *dir_entry_name_field(Qcow2BitmapDirEntry *entry)
+{
+ return (const char *)(entry + 1) + entry->extra_data_size;
+}
+
+static inline char *dir_entry_copy_name(Qcow2BitmapDirEntry *entry)
+{
+ const char *name_field = dir_entry_name_field(entry);
+ return g_strndup(name_field, entry->name_size);
+}
+
+static inline Qcow2BitmapDirEntry *next_dir_entry(Qcow2BitmapDirEntry *entry)
+{
+ return (Qcow2BitmapDirEntry *)((uint8_t *)entry + dir_entry_size(entry));
+}
+
+static int check_dir_entry(BlockDriverState *bs, Qcow2BitmapDirEntry *entry)
+{
+ BDRVQcow2State *s = bs->opaque;
+ uint64_t phys_bitmap_bytes;
+ int64_t len;
+
+ bool fail = (entry->bitmap_table_size == 0) ||
+ (entry->bitmap_table_offset == 0) ||
+ (entry->bitmap_table_offset % s->cluster_size) ||
+ (entry->bitmap_table_size > BME_MAX_TABLE_SIZE) ||
+ (entry->granularity_bits > BME_MAX_GRANULARITY_BITS) ||
+ (entry->granularity_bits < BME_MIN_GRANULARITY_BITS) ||
+ (entry->flags & BME_RESERVED_FLAGS) ||
+ (entry->name_size > BME_MAX_NAME_SIZE) ||
+ (entry->type != BT_DIRTY_TRACKING_BITMAP);
+
+ if (fail) {
+ return -EINVAL;
+ }
+
+ phys_bitmap_bytes = (uint64_t)entry->bitmap_table_size * s->cluster_size;
+ len = bdrv_getlength(bs);
+
+ if (len < 0) {
+ return len;
+ }
+
+ fail = (phys_bitmap_bytes > BME_MAX_PHYS_SIZE) ||
+ (len > ((phys_bitmap_bytes * 8) << entry->granularity_bits));
+
+ return fail ? -EINVAL : 0;
+}
+
+static inline void bitmap_directory_to_be(uint8_t *dir, size_t size)
+{
+ uint8_t *end = dir + size;
+ while (dir < end) {
+ Qcow2BitmapDirEntry *e = (Qcow2BitmapDirEntry *)dir;
+ dir += dir_entry_size(e);
+
+ bitmap_dir_entry_to_be(e);
+ }
+}
+
+/*
+ * Bitmap List public functions
+ */
+
+static void bitmap_free(Qcow2Bitmap *bm)
+{
+ g_free(bm->name);
+ g_free(bm);
+}
+
+static void bitmap_list_free(Qcow2BitmapList *bm_list)
+{
+ Qcow2Bitmap *bm;
+
+ if (bm_list == NULL) {
+ return;
+ }
+
+ while ((bm = QSIMPLEQ_FIRST(bm_list)) != NULL) {
+ QSIMPLEQ_REMOVE_HEAD(bm_list, entry);
+ bitmap_free(bm);
+ }
+
+ g_free(bm_list);
+}
+
+static Qcow2BitmapList *bitmap_list_new(void)
+{
+ Qcow2BitmapList *bm_list = g_new(Qcow2BitmapList, 1);
+ QSIMPLEQ_INIT(bm_list);
+
+ return bm_list;
+}
+
+static uint32_t bitmap_list_count(Qcow2BitmapList *bm_list)
+{
+ Qcow2Bitmap *bm;
+ uint32_t nb_bitmaps = 0;
+
+ QSIMPLEQ_FOREACH(bm, bm_list, entry) {
+ nb_bitmaps++;
+ }
+
+ return nb_bitmaps;
+}
+
+/* bitmap_list_load
+ * Get bitmap list from qcow2 image. Actually reads bitmap directory,
+ * checks it and convert to bitmap list.
+ */
+static Qcow2BitmapList *bitmap_list_load(BlockDriverState *bs, uint64_t offset,
+ uint64_t size, Error **errp)
+{
+ int ret;
+ BDRVQcow2State *s = bs->opaque;
+ uint8_t *dir, *dir_end;
+ Qcow2BitmapDirEntry *e;
+ uint32_t nb_dir_entries = 0;
+ Qcow2BitmapList *bm_list = NULL;
+
+ if (size == 0) {
+ error_setg(errp, "Requested bitmap directory size is zero");
+ return NULL;
+ }
+
+ if (size > QCOW2_MAX_BITMAP_DIRECTORY_SIZE) {
+ error_setg(errp, "Requested bitmap directory size is too big");
+ return NULL;
+ }
+
+ dir = g_try_malloc(size);
+ if (dir == NULL) {
+ error_setg(errp, "Failed to allocate space for bitmap directory");
+ return NULL;
+ }
+ dir_end = dir + size;
+
+ ret = bdrv_pread(bs->file, offset, dir, size);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "Failed to read bitmap directory");
+ goto fail;
+ }
+
+ bm_list = bitmap_list_new();
+ for (e = (Qcow2BitmapDirEntry *)dir;
+ e < (Qcow2BitmapDirEntry *)dir_end;
+ e = next_dir_entry(e))
+ {
+ Qcow2Bitmap *bm;
+
+ if ((uint8_t *)(e + 1) > dir_end) {
+ goto broken_dir;
+ }
+
+ if (++nb_dir_entries > s->nb_bitmaps) {
+ error_setg(errp, "More bitmaps found than specified in header"
+ " extension");
+ goto fail;
+ }
+ bitmap_dir_entry_to_cpu(e);
+
+ if ((uint8_t *)next_dir_entry(e) > dir_end) {
+ goto broken_dir;
+ }
+
+ if (e->extra_data_size != 0) {
+ error_setg(errp, "Bitmap extra data is not supported");
+ goto fail;
+ }
+
+ ret = check_dir_entry(bs, e);
+ if (ret < 0) {
+ error_setg(errp, "Bitmap '%.*s' doesn't satisfy the constraints",
+ e->name_size, dir_entry_name_field(e));
+ goto fail;
+ }
+
+ bm = g_new(Qcow2Bitmap, 1);
+ bm->table_offset = e->bitmap_table_offset;
+ bm->table_size = e->bitmap_table_size;
+ bm->flags = e->flags;
+ bm->granularity_bits = e->granularity_bits;
+ bm->name = dir_entry_copy_name(e);
+ QSIMPLEQ_INSERT_TAIL(bm_list, bm, entry);
+ }
+
+ if (nb_dir_entries != s->nb_bitmaps) {
+ error_setg(errp, "Less bitmaps found than specified in header"
+ " extension");
+ goto fail;
+ }
+
+ if ((uint8_t *)e != dir_end) {
+ goto broken_dir;
+ }
+
+ g_free(dir);
+ return bm_list;
+
+broken_dir:
+ ret = -EINVAL;
+ error_setg(errp, "Broken bitmap directory");
+
+fail:
+ g_free(dir);
+ bitmap_list_free(bm_list);
+
+ return NULL;
+}
+
+/* bitmap_list_store
+ * Store bitmap list to qcow2 image as a bitmap directory.
+ * Everything is checked.
+ */
+static int bitmap_list_store(BlockDriverState *bs, Qcow2BitmapList *bm_list,
+ uint64_t *offset, uint64_t *size, bool in_place)
+{
+ int ret;
+ uint8_t *dir;
+ int64_t dir_offset = 0;
+ uint64_t dir_size = 0;
+ Qcow2Bitmap *bm;
+ Qcow2BitmapDirEntry *e;
+
+ QSIMPLEQ_FOREACH(bm, bm_list, entry) {
+ dir_size += calc_dir_entry_size(strlen(bm->name), 0);
+ }
+
+ if (dir_size == 0 || dir_size > QCOW2_MAX_BITMAP_DIRECTORY_SIZE) {
+ return -EINVAL;
+ }
+
+ if (in_place) {
+ if (*size != dir_size || *offset == 0) {
+ return -EINVAL;
+ }
+
+ dir_offset = *offset;
+ }
+
+ dir = g_try_malloc(dir_size);
+ if (dir == NULL) {
+ return -ENOMEM;
+ }
+
+ e = (Qcow2BitmapDirEntry *)dir;
+ QSIMPLEQ_FOREACH(bm, bm_list, entry) {
+ e->bitmap_table_offset = bm->table_offset;
+ e->bitmap_table_size = bm->table_size;
+ e->flags = bm->flags;
+ e->type = BT_DIRTY_TRACKING_BITMAP;
+ e->granularity_bits = bm->granularity_bits;
+ e->name_size = strlen(bm->name);
+ e->extra_data_size = 0;
+ memcpy(e + 1, bm->name, e->name_size);
+
+ if (check_dir_entry(bs, e) < 0) {
+ ret = -EINVAL;
+ goto fail;
+ }
+
+ e = next_dir_entry(e);
+ }
+
+ bitmap_directory_to_be(dir, dir_size);
+
+ if (!in_place) {
+ dir_offset = qcow2_alloc_clusters(bs, dir_size);
+ if (dir_offset < 0) {
+ ret = dir_offset;
+ goto fail;
+ }
+ }
+
+ ret = qcow2_pre_write_overlap_check(bs, 0, dir_offset, dir_size);
+ if (ret < 0) {
+ goto fail;
+ }
+
+ ret = bdrv_pwrite(bs->file, dir_offset, dir, dir_size);
+ if (ret < 0) {
+ goto fail;
+ }
+
+ g_free(dir);
+
+ if (!in_place) {
+ *size = dir_size;
+ *offset = dir_offset;
+ }
+
+ return 0;
+
+fail:
+ g_free(dir);
+
+ if (!in_place && dir_offset > 0) {
+ qcow2_free_clusters(bs, dir_offset, dir_size, QCOW2_DISCARD_OTHER);
+ }
+
+ return ret;
+}
+
+/*
+ * Bitmap List end
+ */
+
+static int update_ext_header_and_dir_in_place(BlockDriverState *bs,
+ Qcow2BitmapList *bm_list)
+{
+ BDRVQcow2State *s = bs->opaque;
+ int ret;
+
+ if (!(s->autoclear_features & QCOW2_AUTOCLEAR_BITMAPS) ||
+ bm_list == NULL || QSIMPLEQ_EMPTY(bm_list) ||
+ bitmap_list_count(bm_list) != s->nb_bitmaps)
+ {
+ return -EINVAL;
+ }
+
+ s->autoclear_features &= ~(uint64_t)QCOW2_AUTOCLEAR_BITMAPS;
+ ret = update_header_sync(bs);
+ if (ret < 0) {
+ return ret;
+ }
+
+ /* autoclear bit is not set, so we can safely update bitmap directory */
+
+ ret = bitmap_list_store(bs, bm_list, &s->bitmap_directory_offset,
+ &s->bitmap_directory_size, true);
+ if (ret < 0) {
+ /* autoclear bit is cleared, so all leaked clusters would be removed on
+ * qemu-img check */
+ return ret;
+ }
+
+ s->autoclear_features |= QCOW2_AUTOCLEAR_BITMAPS;
+ return update_header_sync(bs);
+}
+
+/* for g_slist_foreach for GSList of BdrvDirtyBitmap* elements */
+static void release_dirty_bitmap_helper(gpointer bitmap,
+ gpointer bs)
+{
+ bdrv_release_dirty_bitmap(bs, bitmap);
+}
+
+void qcow2_load_autoloading_dirty_bitmaps(BlockDriverState *bs, Error **errp)
+{
+ BDRVQcow2State *s = bs->opaque;
+ Qcow2BitmapList *bm_list;
+ Qcow2Bitmap *bm;
+ GSList *created_dirty_bitmaps = NULL;
+
+ if (s->nb_bitmaps == 0) {
+ /* No bitmaps - nothing to do */
+ return;
+ }
+
+ bm_list = bitmap_list_load(bs, s->bitmap_directory_offset,
+ s->bitmap_directory_size, errp);
+ if (bm_list == NULL) {
+ return;
+ }
+
+ QSIMPLEQ_FOREACH(bm, bm_list, entry) {
+ if ((bm->flags & BME_FLAG_AUTO) && !(bm->flags & BME_FLAG_IN_USE)) {
+ BdrvDirtyBitmap *bitmap = load_bitmap(bs, bm, errp);
+ if (bitmap == NULL) {
+ goto fail;
+ }
+ bm->flags |= BME_FLAG_IN_USE;
+ created_dirty_bitmaps =
+ g_slist_append(created_dirty_bitmaps, bitmap);
+ }
+ }
+
+ if (created_dirty_bitmaps != NULL && can_write(bs)) {
+ /* in_use flags must be updated */
+ int ret = update_ext_header_and_dir_in_place(bs, bm_list);
+ if (ret < 0) {
+ error_setg(errp, "Can't update bitmap directory");
+ goto fail;
+ }
+ }
+
+ g_slist_free(created_dirty_bitmaps);
+ bitmap_list_free(bm_list);
+
+ return;
+
+fail:
+ g_slist_foreach(created_dirty_bitmaps, release_dirty_bitmap_helper, bs);
+ g_slist_free(created_dirty_bitmaps);
+ bitmap_list_free(bm_list);
+}
diff --git a/block/qcow2.c b/block/qcow2.c
index a8835988c7..9f64753138 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3540,6 +3540,8 @@ BlockDriver bdrv_qcow2 = {
.bdrv_detach_aio_context = qcow2_detach_aio_context,
.bdrv_attach_aio_context = qcow2_attach_aio_context,
+
+ .bdrv_load_autoloading_dirty_bitmaps = qcow2_load_autoloading_dirty_bitmaps,
};
static void bdrv_qcow2_init(void)
diff --git a/block/qcow2.h b/block/qcow2.h
index 861b5011dd..bcedf5bdf2 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -613,4 +613,7 @@ int qcow2_cache_get_empty(BlockDriverState *bs, Qcow2Cache *c, uint64_t offset,
void **table);
void qcow2_cache_put(BlockDriverState *bs, Qcow2Cache *c, void **table);
+/* qcow2-bitmap.c functions */
+void qcow2_load_autoloading_dirty_bitmaps(BlockDriverState *bs, Error **errp);
+
#endif
--
2.11.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH 10/24] block/dirty-bitmap: add autoload field to BdrvDirtyBitmap
2017-01-23 12:10 [Qemu-devel] [PATCH v11 00/24] qcow2: persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
` (8 preceding siblings ...)
2017-01-23 12:10 ` [Qemu-devel] [PATCH 09/24] qcow2: add .bdrv_load_autoloading_dirty_bitmaps Vladimir Sementsov-Ogievskiy
@ 2017-01-23 12:10 ` Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 11/24] block: introduce persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
` (13 subsequent siblings)
23 siblings, 0 replies; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-01-23 12:10 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: kwolf, mreitz, armbru, eblake, jsnow, famz, den, stefanha,
vsementsov, pbonzini
Mirror AUTO flag from Qcow2 bitmap in BdrvDirtyBitmap. This will be
needed in future, to save this flag back to Qcow2 for persistent
bitmaps.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
block/dirty-bitmap.c | 16 ++++++++++++++++
block/qcow2-bitmap.c | 2 ++
include/block/dirty-bitmap.h | 2 ++
3 files changed, 20 insertions(+)
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 90af37287f..2d27494dc7 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -44,6 +44,8 @@ struct BdrvDirtyBitmap {
int64_t size; /* Size of the bitmap (Number of sectors) */
bool disabled; /* Bitmap is read-only */
int active_iterators; /* How many iterators are active */
+ bool autoload; /* For persistent bitmaps: bitmap must be
+ autoloaded on image opening */
QLIST_ENTRY(BdrvDirtyBitmap) list;
};
@@ -70,6 +72,8 @@ void bdrv_dirty_bitmap_make_anon(BdrvDirtyBitmap *bitmap)
assert(!bdrv_dirty_bitmap_frozen(bitmap));
g_free(bitmap->name);
bitmap->name = NULL;
+
+ bitmap->autoload = false;
}
BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
@@ -238,6 +242,8 @@ BdrvDirtyBitmap *bdrv_dirty_bitmap_abdicate(BlockDriverState *bs,
bitmap->name = NULL;
successor->name = name;
bitmap->successor = NULL;
+ successor->autoload = bitmap->autoload;
+ bitmap->autoload = false;
bdrv_release_dirty_bitmap(bs, bitmap);
return successor;
@@ -540,3 +546,13 @@ int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap)
{
return hbitmap_count(bitmap->meta);
}
+
+void bdrv_dirty_bitmap_set_autoload(BdrvDirtyBitmap *bitmap, bool autoload)
+{
+ bitmap->autoload = autoload;
+}
+
+bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap)
+{
+ return bitmap->autoload;
+}
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index 758622dcfc..277e678e44 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -685,6 +685,8 @@ void qcow2_load_autoloading_dirty_bitmaps(BlockDriverState *bs, Error **errp)
if (bitmap == NULL) {
goto fail;
}
+
+ bdrv_dirty_bitmap_set_autoload(bitmap, true);
bm->flags |= BME_FLAG_IN_USE;
created_dirty_bitmaps =
g_slist_append(created_dirty_bitmaps, bitmap);
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 1e17729ac2..45a389a20a 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -75,4 +75,6 @@ void bdrv_dirty_bitmap_deserialize_ones(BdrvDirtyBitmap *bitmap,
bool finish);
void bdrv_dirty_bitmap_deserialize_finish(BdrvDirtyBitmap *bitmap);
+void bdrv_dirty_bitmap_set_autoload(BdrvDirtyBitmap *bitmap, bool autoload);
+bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap);
#endif
--
2.11.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH 11/24] block: introduce persistent dirty bitmaps
2017-01-23 12:10 [Qemu-devel] [PATCH v11 00/24] qcow2: persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
` (9 preceding siblings ...)
2017-01-23 12:10 ` [Qemu-devel] [PATCH 10/24] block/dirty-bitmap: add autoload field to BdrvDirtyBitmap Vladimir Sementsov-Ogievskiy
@ 2017-01-23 12:10 ` Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 12/24] block/dirty-bitmap: add bdrv_dirty_bitmap_next() Vladimir Sementsov-Ogievskiy
` (12 subsequent siblings)
23 siblings, 0 replies; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-01-23 12:10 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: kwolf, mreitz, armbru, eblake, jsnow, famz, den, stefanha,
vsementsov, pbonzini
New field BdrvDirtyBitmap.persistent means, that bitmap should be saved
on bdrv_close, using format driver. Format driver should maintain bitmap
storing.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
block.c | 32 ++++++++++++++++++++++++++++++++
block/dirty-bitmap.c | 26 ++++++++++++++++++++++++++
block/qcow2-bitmap.c | 1 +
include/block/block.h | 1 +
include/block/block_int.h | 2 ++
include/block/dirty-bitmap.h | 6 ++++++
6 files changed, 68 insertions(+)
diff --git a/block.c b/block.c
index 10951767f3..646ea50316 100644
--- a/block.c
+++ b/block.c
@@ -2321,6 +2321,7 @@ void bdrv_reopen_abort(BDRVReopenState *reopen_state)
static void bdrv_close(BlockDriverState *bs)
{
BdrvAioNotifier *ban, *ban_next;
+ Error *local_err = NULL;
assert(!bs->job);
assert(!bs->refcnt);
@@ -2329,6 +2330,12 @@ static void bdrv_close(BlockDriverState *bs)
bdrv_flush(bs);
bdrv_drain(bs); /* in case flush left pending I/O */
+ bdrv_store_persistent_dirty_bitmaps(bs, &local_err);
+ if (local_err != NULL) {
+ error_report_err(local_err);
+ error_report("Persistent bitmaps are lost for node '%s'",
+ bdrv_get_device_or_node_name(bs));
+ }
bdrv_release_named_dirty_bitmaps(bs);
assert(QLIST_EMPTY(&bs->dirty_bitmaps));
@@ -4107,3 +4114,28 @@ void bdrv_load_autoloading_dirty_bitmaps(BlockDriverState *bs, Error **errp)
bs->drv->bdrv_load_autoloading_dirty_bitmaps(bs, errp);
}
}
+
+void bdrv_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp)
+{
+ BlockDriver *drv = bs->drv;
+
+ if (!bdrv_has_persistent_bitmaps(bs)) {
+ return;
+ }
+
+ if (!drv) {
+ error_setg_errno(errp, ENOMEDIUM,
+ "Can't store persistent bitmaps to %s",
+ bdrv_get_device_or_node_name(bs));
+ return;
+ }
+
+ if (!drv->bdrv_store_persistent_dirty_bitmaps) {
+ error_setg_errno(errp, ENOTSUP,
+ "Can't store persistent bitmaps to %s",
+ bdrv_get_device_or_node_name(bs));
+ return;
+ }
+
+ drv->bdrv_store_persistent_dirty_bitmaps(bs, errp);
+}
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 2d27494dc7..4d026df1bd 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -44,6 +44,7 @@ struct BdrvDirtyBitmap {
int64_t size; /* Size of the bitmap (Number of sectors) */
bool disabled; /* Bitmap is read-only */
int active_iterators; /* How many iterators are active */
+ bool persistent; /* bitmap must be saved to owner disk image */
bool autoload; /* For persistent bitmaps: bitmap must be
autoloaded on image opening */
QLIST_ENTRY(BdrvDirtyBitmap) list;
@@ -73,6 +74,7 @@ void bdrv_dirty_bitmap_make_anon(BdrvDirtyBitmap *bitmap)
g_free(bitmap->name);
bitmap->name = NULL;
+ bitmap->persistent = false;
bitmap->autoload = false;
}
@@ -242,6 +244,8 @@ BdrvDirtyBitmap *bdrv_dirty_bitmap_abdicate(BlockDriverState *bs,
bitmap->name = NULL;
successor->name = name;
bitmap->successor = NULL;
+ successor->persistent = bitmap->persistent;
+ bitmap->persistent = false;
successor->autoload = bitmap->autoload;
bitmap->autoload = false;
bdrv_release_dirty_bitmap(bs, bitmap);
@@ -556,3 +560,25 @@ bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap)
{
return bitmap->autoload;
}
+
+void bdrv_dirty_bitmap_set_persistance(BdrvDirtyBitmap *bitmap, bool persistent)
+{
+ bitmap->persistent = persistent;
+}
+
+bool bdrv_dirty_bitmap_get_persistance(BdrvDirtyBitmap *bitmap)
+{
+ return bitmap->persistent;
+}
+
+bool bdrv_has_persistent_bitmaps(BlockDriverState *bs)
+{
+ BdrvDirtyBitmap *bm;
+ QLIST_FOREACH(bm, &bs->dirty_bitmaps, list) {
+ if (bm->persistent) {
+ return true;
+ }
+ }
+
+ return false;
+}
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index 277e678e44..3e8fd030d9 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -686,6 +686,7 @@ void qcow2_load_autoloading_dirty_bitmaps(BlockDriverState *bs, Error **errp)
goto fail;
}
+ bdrv_dirty_bitmap_set_persistance(bitmap, true);
bdrv_dirty_bitmap_set_autoload(bitmap, true);
bm->flags |= BME_FLAG_IN_USE;
created_dirty_bitmaps =
diff --git a/include/block/block.h b/include/block/block.h
index f701da2c5d..e3175b5e4c 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -553,5 +553,6 @@ void bdrv_add_child(BlockDriverState *parent, BlockDriverState *child,
void bdrv_del_child(BlockDriverState *parent, BdrvChild *child, Error **errp);
void bdrv_load_autoloading_dirty_bitmaps(BlockDriverState *bs, Error **errp);
+void bdrv_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp);
#endif
diff --git a/include/block/block_int.h b/include/block/block_int.h
index ca955f08b9..e8d9210075 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -322,6 +322,8 @@ struct BlockDriver {
void (*bdrv_load_autoloading_dirty_bitmaps)(BlockDriverState *bs,
Error **errp);
+ void (*bdrv_store_persistent_dirty_bitmaps)(BlockDriverState *bs,
+ Error **errp);
QLIST_ENTRY(BlockDriver) list;
};
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 45a389a20a..8dbd16b040 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -77,4 +77,10 @@ void bdrv_dirty_bitmap_deserialize_finish(BdrvDirtyBitmap *bitmap);
void bdrv_dirty_bitmap_set_autoload(BdrvDirtyBitmap *bitmap, bool autoload);
bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap);
+void bdrv_dirty_bitmap_set_persistance(BdrvDirtyBitmap *bitmap,
+ bool persistent);
+bool bdrv_dirty_bitmap_get_persistance(BdrvDirtyBitmap *bitmap);
+
+bool bdrv_has_persistent_bitmaps(BlockDriverState *bs);
+
#endif
--
2.11.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH 12/24] block/dirty-bitmap: add bdrv_dirty_bitmap_next()
2017-01-23 12:10 [Qemu-devel] [PATCH v11 00/24] qcow2: persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
` (10 preceding siblings ...)
2017-01-23 12:10 ` [Qemu-devel] [PATCH 11/24] block: introduce persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
@ 2017-01-23 12:10 ` Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 13/24] qcow2: add .bdrv_store_persistent_dirty_bitmaps() Vladimir Sementsov-Ogievskiy
` (11 subsequent siblings)
23 siblings, 0 replies; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-01-23 12:10 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: kwolf, mreitz, armbru, eblake, jsnow, famz, den, stefanha,
vsementsov, pbonzini
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
block/dirty-bitmap.c | 7 +++++++
include/block/dirty-bitmap.h | 3 +++
2 files changed, 10 insertions(+)
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 4d026df1bd..fe34d488aa 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -582,3 +582,10 @@ bool bdrv_has_persistent_bitmaps(BlockDriverState *bs)
return false;
}
+
+BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap)
+{
+ return bitmap == NULL ? QLIST_FIRST(&bs->dirty_bitmaps) :
+ QLIST_NEXT(bitmap, list);
+}
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 8dbd16b040..d71edc4d13 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -81,6 +81,9 @@ void bdrv_dirty_bitmap_set_persistance(BdrvDirtyBitmap *bitmap,
bool persistent);
bool bdrv_dirty_bitmap_get_persistance(BdrvDirtyBitmap *bitmap);
+BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap);
+
bool bdrv_has_persistent_bitmaps(BlockDriverState *bs);
#endif
--
2.11.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH 13/24] qcow2: add .bdrv_store_persistent_dirty_bitmaps()
2017-01-23 12:10 [Qemu-devel] [PATCH v11 00/24] qcow2: persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
` (11 preceding siblings ...)
2017-01-23 12:10 ` [Qemu-devel] [PATCH 12/24] block/dirty-bitmap: add bdrv_dirty_bitmap_next() Vladimir Sementsov-Ogievskiy
@ 2017-01-23 12:10 ` Vladimir Sementsov-Ogievskiy
2017-01-28 22:30 ` Max Reitz
2017-01-23 12:10 ` [Qemu-devel] [PATCH 14/24] block: add bdrv_can_store_new_dirty_bitmap Vladimir Sementsov-Ogievskiy
` (10 subsequent siblings)
23 siblings, 1 reply; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-01-23 12:10 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: kwolf, mreitz, armbru, eblake, jsnow, famz, den, stefanha,
vsementsov, pbonzini
Realize block bitmap storing interface, to allow qcow2 images store
persistent bitmaps.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/qcow2-bitmap.c | 509 +++++++++++++++++++++++++++++++++++++++++++++++++--
block/qcow2.c | 1 +
block/qcow2.h | 1 +
3 files changed, 496 insertions(+), 15 deletions(-)
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index 3e8fd030d9..a2b857ea72 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -28,6 +28,7 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
#include "exec/log.h"
+#include "qemu/cutils.h"
#include "block/block_int.h"
#include "block/qcow2.h"
@@ -43,6 +44,10 @@
#define BME_MIN_GRANULARITY_BITS 9
#define BME_MAX_NAME_SIZE 1023
+#if BME_MAX_TABLE_SIZE * 8ULL > INT_MAX
+#error In the code bitmap table physical size assumed to fit into int
+#endif
+
/* Bitmap directory entry flags */
#define BME_RESERVED_FLAGS 0xfffffffcU
#define BME_FLAG_IN_USE 1
@@ -67,13 +72,21 @@ typedef struct QEMU_PACKED Qcow2BitmapDirEntry {
/* name follows */
} Qcow2BitmapDirEntry;
+typedef struct Qcow2BitmapTable {
+ uint64_t offset;
+ uint32_t size; /* number of 64bit entries */
+ QSIMPLEQ_ENTRY(Qcow2BitmapTable) entry;
+} Qcow2BitmapTable;
+typedef QSIMPLEQ_HEAD(Qcow2BitmapTableList, Qcow2BitmapTable) Qcow2BitmapTableList;
+
typedef struct Qcow2Bitmap {
- uint64_t table_offset;
- uint32_t table_size;
+ Qcow2BitmapTable table;
uint32_t flags;
uint8_t granularity_bits;
char *name;
+ BdrvDirtyBitmap *dirty_bitmap;
+
QSIMPLEQ_ENTRY(Qcow2Bitmap) entry;
} Qcow2Bitmap;
typedef QSIMPLEQ_HEAD(Qcow2BitmapList, Qcow2Bitmap) Qcow2BitmapList;
@@ -87,6 +100,27 @@ static inline bool can_write(BlockDriverState *bs)
return !bdrv_is_read_only(bs) && !(bdrv_get_flags(bs) & BDRV_O_INACTIVE);
}
+static int update_header_sync(BlockDriverState *bs)
+{
+ int ret;
+
+ ret = qcow2_update_header(bs);
+ if (ret < 0) {
+ return ret;
+ }
+
+ /* We don't return bdrv_flush error code. Even if it fails, write was
+ * successful and it is more logical to consider that header is in the new
+ * state than in the old.
+ */
+ ret = bdrv_flush(bs);
+ if (ret < 0) {
+ fprintf(stderr, "Failed to flush qcow2 header");
+ }
+
+ return 0;
+}
+
static inline void bitmap_table_to_cpu(uint64_t *bitmap_table, size_t size)
{
size_t i;
@@ -96,6 +130,15 @@ static inline void bitmap_table_to_cpu(uint64_t *bitmap_table, size_t size)
}
}
+static inline void bitmap_table_to_be(uint64_t *bitmap_table, size_t size)
+{
+ size_t i;
+
+ for (i = 0; i < size; ++i) {
+ cpu_to_be64s(&bitmap_table[i]);
+ }
+}
+
static int check_table_entry(uint64_t entry, int cluster_size)
{
uint64_t offset;
@@ -123,7 +166,50 @@ static int check_table_entry(uint64_t entry, int cluster_size)
return 0;
}
-static int bitmap_table_load(BlockDriverState *bs, Qcow2Bitmap *bm,
+static int check_constraints_on_bitmap(BlockDriverState *bs,
+ const char *name,
+ uint32_t granularity)
+{
+ BDRVQcow2State *s = bs->opaque;
+ int granularity_bits = ctz32(granularity);
+ int64_t len = bdrv_getlength(bs);
+ bool fail;
+
+ assert(granularity > 0);
+ assert((granularity & (granularity - 1)) == 0);
+
+ if (len < 0) {
+ return len;
+ }
+
+ fail = (granularity_bits > BME_MAX_GRANULARITY_BITS) ||
+ (granularity_bits < BME_MIN_GRANULARITY_BITS) ||
+ (len > (uint64_t)BME_MAX_PHYS_SIZE << granularity_bits) ||
+ (len > (uint64_t)BME_MAX_TABLE_SIZE * s->cluster_size <<
+ granularity_bits) ||
+ (strlen(name) > BME_MAX_NAME_SIZE);
+
+ return fail ? -EINVAL : 0;
+}
+
+static void clear_bitmap_table(BlockDriverState *bs, uint64_t *bitmap_table,
+ uint32_t bitmap_table_size)
+{
+ BDRVQcow2State *s = bs->opaque;
+ int i;
+
+ for (i = 0; i < bitmap_table_size; ++i) {
+ uint64_t addr = bitmap_table[i] & BME_TABLE_ENTRY_OFFSET_MASK;
+ if (!addr) {
+ continue;
+ }
+
+ qcow2_free_clusters(bs, addr, s->cluster_size, QCOW2_DISCARD_OTHER);
+ bitmap_table[i] = 0;
+ }
+}
+
+static int bitmap_table_load(BlockDriverState *bs, Qcow2BitmapTable *tb,
uint64_t **bitmap_table)
{
int ret;
@@ -131,20 +217,20 @@ static int bitmap_table_load(BlockDriverState *bs, Qcow2Bitmap *bm,
uint32_t i;
uint64_t *table;
- assert(bm->table_size != 0);
- table = g_try_new(uint64_t, bm->table_size);
+ assert(tb->size != 0);
+ table = g_try_new(uint64_t, tb->size);
if (table == NULL) {
return -ENOMEM;
}
- assert(bm->table_size <= BME_MAX_TABLE_SIZE);
- ret = bdrv_pread(bs->file, bm->table_offset,
- table, bm->table_size * sizeof(uint64_t));
+ assert(tb->size <= BME_MAX_TABLE_SIZE);
+ ret = bdrv_pread(bs->file, tb->offset,
+ table, tb->size * sizeof(uint64_t));
if (ret < 0) {
goto fail;
}
- for (i = 0; i < bm->table_size; ++i) {
+ for (i = 0; i < tb->size; ++i) {
be64_to_cpus(&table[i]);
ret = check_table_entry(table[i], s->cluster_size);
if (ret < 0) {
@@ -161,6 +247,28 @@ fail:
return ret;
}
+static int free_bitmap_clusters(BlockDriverState *bs, Qcow2BitmapTable *tb)
+{
+ int ret;
+ uint64_t *bitmap_table;
+
+ ret = bitmap_table_load(bs, tb, &bitmap_table);
+ if (ret < 0) {
+ assert(bitmap_table == NULL);
+ return ret;
+ }
+
+ clear_bitmap_table(bs, bitmap_table, tb->size);
+ qcow2_free_clusters(bs, tb->offset, tb->size * sizeof(uint64_t),
+ QCOW2_DISCARD_OTHER);
+ g_free(bitmap_table);
+
+ tb->offset = 0;
+ tb->size = 0;
+
+ return 0;
+}
+
/* This function returns the number of disk sectors covered by a single cluster
* of bitmap data. */
static uint64_t disk_sectors_in_bitmap_cluster(const BDRVQcow2State *s,
@@ -242,7 +350,7 @@ static BdrvDirtyBitmap *load_bitmap(BlockDriverState *bs,
goto fail;
}
- ret = bitmap_table_load(bs, bm, &bitmap_table);
+ ret = bitmap_table_load(bs, &bm->table, &bitmap_table);
if (ret < 0) {
error_setg_errno(errp, -ret,
"Could not read bitmap_table table from image for "
@@ -256,7 +364,7 @@ static BdrvDirtyBitmap *load_bitmap(BlockDriverState *bs,
goto fail;
}
- ret = load_bitmap_data(bs, bitmap_table, bm->table_size,
+ ret = load_bitmap_data(bs, bitmap_table, bm->table.size,
bitmap);
if (ret < 0) {
error_setg_errno(errp, -ret, "Could not read bitmap '%s' from image",
@@ -492,8 +600,8 @@ static Qcow2BitmapList *bitmap_list_load(BlockDriverState *bs, uint64_t offset,
}
bm = g_new(Qcow2Bitmap, 1);
- bm->table_offset = e->bitmap_table_offset;
- bm->table_size = e->bitmap_table_size;
+ bm->table.offset = e->bitmap_table_offset;
+ bm->table.size = e->bitmap_table_size;
bm->flags = e->flags;
bm->granularity_bits = e->granularity_bits;
bm->name = dir_entry_copy_name(e);
@@ -561,8 +669,8 @@ static int bitmap_list_store(BlockDriverState *bs, Qcow2BitmapList *bm_list,
e = (Qcow2BitmapDirEntry *)dir;
QSIMPLEQ_FOREACH(bm, bm_list, entry) {
- e->bitmap_table_offset = bm->table_offset;
- e->bitmap_table_size = bm->table_size;
+ e->bitmap_table_offset = bm->table.offset;
+ e->bitmap_table_size = bm->table.size;
e->flags = bm->flags;
e->type = BT_DIRTY_TRACKING_BITMAP;
e->granularity_bits = bm->granularity_bits;
@@ -654,6 +762,69 @@ static int update_ext_header_and_dir_in_place(BlockDriverState *bs,
return update_header_sync(bs);
}
+static int update_ext_header_and_dir(BlockDriverState *bs,
+ Qcow2BitmapList *bm_list)
+{
+ BDRVQcow2State *s = bs->opaque;
+ int ret;
+ uint64_t new_offset = 0;
+ uint64_t new_size = 0;
+ uint32_t new_nb_bitmaps = 0;
+ uint64_t old_offset = s->bitmap_directory_offset;
+ uint64_t old_size = s->bitmap_directory_size;
+ uint32_t old_nb_bitmaps = s->nb_bitmaps;
+ uint64_t old_autocl = s->autoclear_features;
+
+ if (bm_list != NULL && !QSIMPLEQ_EMPTY(bm_list)) {
+ new_nb_bitmaps = bitmap_list_count(bm_list);
+
+ if (new_nb_bitmaps > QCOW2_MAX_BITMAPS) {
+ return -EINVAL;
+ }
+
+ ret = bitmap_list_store(bs, bm_list, &new_offset, &new_size, false);
+ if (ret < 0) {
+ return ret;
+ }
+
+ ret = bdrv_flush(bs->file->bs);
+ if (ret < 0) {
+ goto fail;
+ }
+
+ s->autoclear_features |= QCOW2_AUTOCLEAR_BITMAPS;
+ } else {
+ s->autoclear_features &= ~(uint64_t)QCOW2_AUTOCLEAR_BITMAPS;
+ }
+
+ s->bitmap_directory_offset = new_offset;
+ s->bitmap_directory_size = new_size;
+ s->nb_bitmaps = new_nb_bitmaps;
+
+ ret = update_header_sync(bs);
+ if (ret < 0) {
+ goto fail;
+ }
+
+ if (old_size > 0) {
+ qcow2_free_clusters(bs, old_offset, old_size, QCOW2_DISCARD_OTHER);
+ }
+
+ return 0;
+
+fail:
+ if (new_offset > 0) {
+ qcow2_free_clusters(bs, new_offset, new_size, QCOW2_DISCARD_OTHER);
+ }
+
+ s->bitmap_directory_offset = old_offset;
+ s->bitmap_directory_size = old_size;
+ s->nb_bitmaps = old_nb_bitmaps;
+ s->autoclear_features = old_autocl;
+
+ return ret;
+}
+
/* for g_slist_foreach for GSList of BdrvDirtyBitmap* elements */
static void release_dirty_bitmap_helper(gpointer bitmap,
gpointer bs)
@@ -713,3 +884,311 @@ fail:
g_slist_free(created_dirty_bitmaps);
bitmap_list_free(bm_list);
}
+
+/* store_bitmap_data()
+ * Store bitmap to image, filling bitmap table accordingly.
+ */
+static uint64_t *store_bitmap_data(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap,
+ uint32_t *bitmap_table_size, Error **errp)
+{
+ int ret;
+ BDRVQcow2State *s = bs->opaque;
+ int64_t sector;
+ uint64_t dsc;
+ uint64_t bm_size = bdrv_dirty_bitmap_size(bitmap);
+ const char *bm_name = bdrv_dirty_bitmap_name(bitmap);
+ uint8_t *buf = NULL;
+ BdrvDirtyBitmapIter *dbi;
+ uint64_t *tb;
+ uint64_t tb_size =
+ size_to_clusters(s,
+ bdrv_dirty_bitmap_serialization_size(bitmap, 0, bm_size));
+
+ if (tb_size > BME_MAX_TABLE_SIZE ||
+ tb_size * s->cluster_size > BME_MAX_PHYS_SIZE)
+ {
+ error_setg(errp, "Bitmap '%s' is too big", bm_name);
+ return NULL;
+ }
+
+ tb = g_try_new0(uint64_t, tb_size);
+ if (tb == NULL) {
+ error_setg(errp, "No memory");
+ return NULL;
+ }
+
+ dbi = bdrv_dirty_iter_new(bitmap, 0);
+ buf = g_malloc(s->cluster_size);
+ dsc = disk_sectors_in_bitmap_cluster(s, bitmap);
+ assert(DIV_ROUND_UP(bm_size, dsc) == tb_size);
+
+ while ((sector = bdrv_dirty_iter_next(dbi)) != -1) {
+ uint64_t cluster = sector / dsc;
+ uint64_t end, write_size;
+ int64_t off;
+
+ sector = cluster * dsc;
+ end = MIN(bm_size, sector + dsc);
+ write_size =
+ bdrv_dirty_bitmap_serialization_size(bitmap, sector, end - sector);
+ assert(write_size <= s->cluster_size);
+
+ off = qcow2_alloc_clusters(bs, s->cluster_size);
+ if (off < 0) {
+ error_setg_errno(errp, -off,
+ "Failed to allocate clusters for bitmap '%s'",
+ bm_name);
+ goto fail;
+ }
+ tb[cluster] = off;
+
+ bdrv_dirty_bitmap_serialize_part(bitmap, buf, sector, end - sector);
+ if (write_size < s->cluster_size) {
+ memset(buf + write_size, 0, s->cluster_size - write_size);
+ }
+
+ ret = qcow2_pre_write_overlap_check(bs, 0, off, s->cluster_size);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "Qcow2 overlap check failed");
+ goto fail;
+ }
+
+ ret = bdrv_pwrite(bs->file, off, buf, s->cluster_size);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "Failed to write bitmap '%s' to file",
+ bm_name);
+ goto fail;
+ }
+
+ if (end >= bm_size) {
+ break;
+ }
+
+ bdrv_set_dirty_iter(dbi, end);
+ }
+
+ *bitmap_table_size = tb_size;
+ g_free(buf);
+ bdrv_dirty_iter_free(dbi);
+
+ return tb;
+
+fail:
+ clear_bitmap_table(bs, tb, tb_size);
+ g_free(buf);
+ bdrv_dirty_iter_free(dbi);
+ g_free(tb);
+
+ return NULL;
+}
+
+/* store_bitmap()
+ * Store bm->dirty_bitmap to qcow2.
+ * Set bm->table_offset and bm->table_size accordingly.
+ */
+static int store_bitmap(BlockDriverState *bs, Qcow2Bitmap *bm, Error **errp)
+{
+ int ret;
+ uint64_t *tb;
+ int64_t tb_offset;
+ uint32_t tb_size;
+ BdrvDirtyBitmap *bitmap = bm->dirty_bitmap;
+ const char *bm_name;
+
+ assert(bitmap != NULL);
+
+ bm_name = bdrv_dirty_bitmap_name(bitmap);
+
+ tb = store_bitmap_data(bs, bitmap, &tb_size, errp);
+ if (tb == NULL) {
+ g_free(tb);
+ return -EINVAL;
+ }
+
+ assert(tb_size <= BME_MAX_TABLE_SIZE);
+ tb_offset = qcow2_alloc_clusters(bs, tb_size * sizeof(tb[0]));
+ if (tb_offset < 0) {
+ error_setg_errno(errp, -tb_offset,
+ "Failed to allocate clusters for bitmap '%s'",
+ bm_name);
+ goto fail;
+ }
+
+ ret = qcow2_pre_write_overlap_check(bs, 0, tb_offset,
+ tb_size * sizeof(tb[0]));
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "Qcow2 overlap check failed");
+ goto fail;
+ }
+
+ bitmap_table_to_be(tb, tb_size);
+ ret = bdrv_pwrite(bs->file, tb_offset, tb, tb_size * sizeof(tb[0]));
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "Failed to write bitmap '%s' to file",
+ bm_name);
+ goto fail;
+ }
+
+ g_free(tb);
+
+ bm->table.offset = tb_offset;
+ bm->table.size = tb_size;
+
+ return 0;
+
+fail:
+ clear_bitmap_table(bs, tb, tb_size);
+
+ if (tb_offset > 0) {
+ qcow2_free_clusters(bs, tb_offset, tb_size * sizeof(tb[0]),
+ QCOW2_DISCARD_OTHER);
+ }
+
+ g_free(tb);
+
+ return ret;
+}
+
+static Qcow2Bitmap *find_bitmap_by_name(Qcow2BitmapList *bm_list,
+ const char *name)
+{
+ Qcow2Bitmap *bm;
+
+ QSIMPLEQ_FOREACH(bm, bm_list, entry) {
+ if (strcmp(name, bm->name) == 0) {
+ return bm;
+ }
+ }
+
+ return NULL;
+}
+
+void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp)
+{
+ BdrvDirtyBitmap *bitmap;
+ BDRVQcow2State *s = bs->opaque;
+ uint32_t new_nb_bitmaps = s->nb_bitmaps;
+ uint64_t new_dir_size = s->bitmap_directory_size;
+ int ret;
+ Qcow2BitmapList *bm_list;
+ Qcow2Bitmap *bm;
+ Qcow2BitmapTableList drop_tables;
+ Qcow2BitmapTable *tb, *tb_next;
+
+ QSIMPLEQ_INIT(&drop_tables);
+
+ if (!can_write(bs)) {
+ error_setg(errp, "No write access");
+ return;
+ }
+
+ if (!bdrv_has_persistent_bitmaps(bs)) {
+ /* nothing to do */
+ return;
+ }
+
+ if (s->nb_bitmaps == 0) {
+ bm_list = bitmap_list_new();
+ } else {
+ bm_list = bitmap_list_load(bs, s->bitmap_directory_offset,
+ s->bitmap_directory_size, errp);
+ if (bm_list == NULL) {
+ /* errp is already set */
+ return;
+ }
+ }
+
+ /* check constraints and names */
+ for (bitmap = bdrv_dirty_bitmap_next(bs, NULL); bitmap != NULL;
+ bitmap = bdrv_dirty_bitmap_next(bs, bitmap))
+ {
+ const char *name = bdrv_dirty_bitmap_name(bitmap);
+ uint32_t granularity = bdrv_dirty_bitmap_granularity(bitmap);
+ Qcow2Bitmap *bm;
+
+ if (!bdrv_dirty_bitmap_get_persistance(bitmap)) {
+ continue;
+ }
+
+ if (check_constraints_on_bitmap(bs, name, granularity) < 0) {
+ error_setg(errp, "Bitmap '%s' doesn't satisfy the constraints",
+ name);
+ goto fail;
+ }
+
+ bm = find_bitmap_by_name(bm_list, name);
+ if (bm == NULL) {
+ if (++new_nb_bitmaps > QCOW2_MAX_BITMAPS) {
+ error_setg(errp, "Too many persistent bitmaps");
+ goto fail;
+ }
+
+ new_dir_size += calc_dir_entry_size(strlen(name), 0);
+ if (new_dir_size > QCOW2_MAX_BITMAP_DIRECTORY_SIZE) {
+ error_setg(errp, "Too large bitmap directory");
+ goto fail;
+ }
+
+ bm = g_new0(Qcow2Bitmap, 1);
+ bm->name = g_strdup(name);
+ QSIMPLEQ_INSERT_TAIL(bm_list, bm, entry);
+ } else {
+ if (!(bm->flags & BME_FLAG_IN_USE)) {
+ error_setg(errp, "Bitmap '%s' already exists in the image",
+ name);
+ goto fail;
+ }
+ tb = g_memdup(&bm->table, sizeof(bm->table));
+ bm->table.offset = 0;
+ bm->table.size = 0;
+ QSIMPLEQ_INSERT_TAIL(&drop_tables, tb, entry);
+ }
+ bm->flags = bdrv_dirty_bitmap_get_autoload(bitmap) ? BME_FLAG_AUTO : 0;
+ bm->granularity_bits = ctz32(bdrv_dirty_bitmap_granularity(bitmap));
+ bm->dirty_bitmap = bitmap;
+ }
+
+ /* allocate clusters and store bitmaps */
+ QSIMPLEQ_FOREACH(bm, bm_list, entry) {
+ if (bm->dirty_bitmap == NULL) {
+ continue;
+ }
+
+ ret = store_bitmap(bs, bm, errp);
+ if (ret < 0) {
+ goto fail;
+ }
+ }
+
+ ret = update_ext_header_and_dir(bs, bm_list);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "Failed to update bitmap extension");
+ goto fail;
+ }
+
+ /* Bitmap directory was successfully updated, so, old data can be dropped.
+ * TODO it is better to reuse these clusters */
+ QSIMPLEQ_FOREACH_SAFE(tb, &drop_tables, entry, tb_next) {
+ free_bitmap_clusters(bs, tb);
+ g_free(tb);
+ }
+
+ bitmap_list_free(bm_list);
+ return;
+
+fail:
+ QSIMPLEQ_FOREACH(bm, bm_list, entry) {
+ if (bm->dirty_bitmap == NULL || bm->table.offset == 0) {
+ continue;
+ }
+
+ free_bitmap_clusters(bs, &bm->table);
+ }
+
+ QSIMPLEQ_FOREACH_SAFE(tb, &drop_tables, entry, tb_next) {
+ g_free(tb);
+ }
+
+ bitmap_list_free(bm_list);
+}
diff --git a/block/qcow2.c b/block/qcow2.c
index 9f64753138..af580d2299 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3542,6 +3542,7 @@ BlockDriver bdrv_qcow2 = {
.bdrv_attach_aio_context = qcow2_attach_aio_context,
.bdrv_load_autoloading_dirty_bitmaps = qcow2_load_autoloading_dirty_bitmaps,
+ .bdrv_store_persistent_dirty_bitmaps = qcow2_store_persistent_dirty_bitmaps,
};
static void bdrv_qcow2_init(void)
diff --git a/block/qcow2.h b/block/qcow2.h
index bcedf5bdf2..d9a7643a60 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -615,5 +615,6 @@ void qcow2_cache_put(BlockDriverState *bs, Qcow2Cache *c, void **table);
/* qcow2-bitmap.c functions */
void qcow2_load_autoloading_dirty_bitmaps(BlockDriverState *bs, Error **errp);
+void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp);
#endif
--
2.11.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH 14/24] block: add bdrv_can_store_new_dirty_bitmap
2017-01-23 12:10 [Qemu-devel] [PATCH v11 00/24] qcow2: persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
` (12 preceding siblings ...)
2017-01-23 12:10 ` [Qemu-devel] [PATCH 13/24] qcow2: add .bdrv_store_persistent_dirty_bitmaps() Vladimir Sementsov-Ogievskiy
@ 2017-01-23 12:10 ` Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 15/24] qcow2: add .bdrv_can_store_new_dirty_bitmap Vladimir Sementsov-Ogievskiy
` (9 subsequent siblings)
23 siblings, 0 replies; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-01-23 12:10 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: kwolf, mreitz, armbru, eblake, jsnow, famz, den, stefanha,
vsementsov, pbonzini
This will be needed to check some restrictions before making bitmap
persistent in qmp-block-dirty-bitmap-add (this functionality will be
added by future patch)
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
block.c | 22 ++++++++++++++++++++++
include/block/block.h | 2 ++
include/block/block_int.h | 2 ++
3 files changed, 26 insertions(+)
diff --git a/block.c b/block.c
index 646ea50316..c20b22ff25 100644
--- a/block.c
+++ b/block.c
@@ -4139,3 +4139,25 @@ void bdrv_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp)
drv->bdrv_store_persistent_dirty_bitmaps(bs, errp);
}
+
+bool bdrv_can_store_new_dirty_bitmap(BlockDriverState *bs, const char *name,
+ uint32_t granularity, Error **errp)
+{
+ BlockDriver *drv = bs->drv;
+
+ if (!drv) {
+ error_setg_errno(errp, ENOMEDIUM,
+ "Can't store persistent bitmaps to %s",
+ bdrv_get_device_or_node_name(bs));
+ return false;
+ }
+
+ if (!drv->bdrv_can_store_new_dirty_bitmap) {
+ error_setg_errno(errp, ENOTSUP,
+ "Can't store persistent bitmaps to %s",
+ bdrv_get_device_or_node_name(bs));
+ return false;
+ }
+
+ return drv->bdrv_can_store_new_dirty_bitmap(bs, name, granularity, errp);
+}
diff --git a/include/block/block.h b/include/block/block.h
index e3175b5e4c..1b5f2704f6 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -554,5 +554,7 @@ void bdrv_del_child(BlockDriverState *parent, BdrvChild *child, Error **errp);
void bdrv_load_autoloading_dirty_bitmaps(BlockDriverState *bs, Error **errp);
void bdrv_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp);
+bool bdrv_can_store_new_dirty_bitmap(BlockDriverState *bs, const char *name,
+ uint32_t granularity, Error **errp);
#endif
diff --git a/include/block/block_int.h b/include/block/block_int.h
index e8d9210075..b38b3aa198 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -324,6 +324,8 @@ struct BlockDriver {
Error **errp);
void (*bdrv_store_persistent_dirty_bitmaps)(BlockDriverState *bs,
Error **errp);
+ bool (*bdrv_can_store_new_dirty_bitmap)(BlockDriverState *bs, const char *name,
+ uint32_t granularity, Error **errp);
QLIST_ENTRY(BlockDriver) list;
};
--
2.11.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH 15/24] qcow2: add .bdrv_can_store_new_dirty_bitmap
2017-01-23 12:10 [Qemu-devel] [PATCH v11 00/24] qcow2: persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
` (13 preceding siblings ...)
2017-01-23 12:10 ` [Qemu-devel] [PATCH 14/24] block: add bdrv_can_store_new_dirty_bitmap Vladimir Sementsov-Ogievskiy
@ 2017-01-23 12:10 ` Vladimir Sementsov-Ogievskiy
2017-01-28 22:33 ` Max Reitz
2017-01-23 12:10 ` [Qemu-devel] [PATCH 16/24] qmp: add persistent flag to block-dirty-bitmap-add Vladimir Sementsov-Ogievskiy
` (8 subsequent siblings)
23 siblings, 1 reply; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-01-23 12:10 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: kwolf, mreitz, armbru, eblake, jsnow, famz, den, stefanha,
vsementsov, pbonzini
Realize .bdrv_can_store_new_dirty_bitmap interface.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/qcow2-bitmap.c | 40 ++++++++++++++++++++++++++++++++++++++++
block/qcow2.c | 1 +
block/qcow2.h | 4 ++++
3 files changed, 45 insertions(+)
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index a2b857ea72..8b8f2d11ec 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -1192,3 +1192,43 @@ fail:
bitmap_list_free(bm_list);
}
+
+bool qcow2_can_store_new_dirty_bitmap(BlockDriverState *bs,
+ const char *name,
+ uint32_t granularity,
+ Error **errp)
+{
+ BDRVQcow2State *s = bs->opaque;
+ const char *reason = NULL;
+ bool found;
+ Qcow2BitmapList *bm_list;
+
+ if (check_constraints_on_bitmap(bs, name, granularity) != 0) {
+ reason = "it doesn't satisfy the constraints";
+ goto common_errp;
+ }
+
+ if (s->nb_bitmaps == 0) {
+ return true;
+ }
+
+ bm_list = bitmap_list_load(bs, s->bitmap_directory_offset,
+ s->bitmap_directory_size, errp);
+ if (bm_list == NULL) {
+ return false;
+ }
+
+ found = find_bitmap_by_name(bm_list, name);
+ bitmap_list_free(bm_list);
+ if (found) {
+ reason = "bitmap with the same name is already stored";
+ goto common_errp;
+ }
+
+ return true;
+
+common_errp:
+ error_setg(errp, "Can't make bitmap '%s' persistent in '%s', as %s.",
+ name, bdrv_get_device_or_node_name(bs), reason);
+ return false;
+}
diff --git a/block/qcow2.c b/block/qcow2.c
index af580d2299..76a6a2ba28 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3543,6 +3543,7 @@ BlockDriver bdrv_qcow2 = {
.bdrv_load_autoloading_dirty_bitmaps = qcow2_load_autoloading_dirty_bitmaps,
.bdrv_store_persistent_dirty_bitmaps = qcow2_store_persistent_dirty_bitmaps,
+ .bdrv_can_store_new_dirty_bitmap = qcow2_can_store_new_dirty_bitmap,
};
static void bdrv_qcow2_init(void)
diff --git a/block/qcow2.h b/block/qcow2.h
index d9a7643a60..749710dc37 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -616,5 +616,9 @@ void qcow2_cache_put(BlockDriverState *bs, Qcow2Cache *c, void **table);
/* qcow2-bitmap.c functions */
void qcow2_load_autoloading_dirty_bitmaps(BlockDriverState *bs, Error **errp);
void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp);
+bool qcow2_can_store_new_dirty_bitmap(BlockDriverState *bs,
+ const char *name,
+ uint32_t granularity,
+ Error **errp);
#endif
--
2.11.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH 16/24] qmp: add persistent flag to block-dirty-bitmap-add
2017-01-23 12:10 [Qemu-devel] [PATCH v11 00/24] qcow2: persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
` (14 preceding siblings ...)
2017-01-23 12:10 ` [Qemu-devel] [PATCH 15/24] qcow2: add .bdrv_can_store_new_dirty_bitmap Vladimir Sementsov-Ogievskiy
@ 2017-01-23 12:10 ` Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 17/24] qmp: add autoload parameter " Vladimir Sementsov-Ogievskiy
` (7 subsequent siblings)
23 siblings, 0 replies; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-01-23 12:10 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: kwolf, mreitz, armbru, eblake, jsnow, famz, den, stefanha,
vsementsov, pbonzini
Add optional 'persistent' flag to qmp command block-dirty-bitmap-add.
Default is false.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
blockdev.c | 18 +++++++++++++++++-
docs/qmp-commands.txt | 3 +++
qapi/block-core.json | 7 ++++++-
3 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 245e1e1d17..40605faccc 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1967,6 +1967,7 @@ static void block_dirty_bitmap_add_prepare(BlkActionState *common,
/* AIO context taken and released within qmp_block_dirty_bitmap_add */
qmp_block_dirty_bitmap_add(action->node, action->name,
action->has_granularity, action->granularity,
+ action->has_persistent, action->persistent,
&local_err);
if (!local_err) {
@@ -2696,10 +2697,12 @@ out:
void qmp_block_dirty_bitmap_add(const char *node, const char *name,
bool has_granularity, uint32_t granularity,
+ bool has_persistent, bool persistent,
Error **errp)
{
AioContext *aio_context;
BlockDriverState *bs;
+ BdrvDirtyBitmap *bitmap;
if (!name || name[0] == '\0') {
error_setg(errp, "Bitmap name cannot be empty");
@@ -2725,7 +2728,20 @@ void qmp_block_dirty_bitmap_add(const char *node, const char *name,
granularity = bdrv_get_default_bitmap_granularity(bs);
}
- bdrv_create_dirty_bitmap(bs, granularity, name, errp);
+ if (!has_persistent) {
+ persistent = false;
+ }
+
+ if (persistent &&
+ !bdrv_can_store_new_dirty_bitmap(bs, name, granularity, errp))
+ {
+ goto out;
+ }
+
+ bitmap = bdrv_create_dirty_bitmap(bs, granularity, name, errp);
+ if (bitmap != NULL) {
+ bdrv_dirty_bitmap_set_persistance(bitmap, persistent);
+ }
out:
aio_context_release(aio_context);
diff --git a/docs/qmp-commands.txt b/docs/qmp-commands.txt
index abf210a596..ef0245d0df 100644
--- a/docs/qmp-commands.txt
+++ b/docs/qmp-commands.txt
@@ -1015,6 +1015,9 @@ Arguments:
- "node": device/node on which to create dirty bitmap (json-string)
- "name": name of the new dirty bitmap (json-string)
- "granularity": granularity to track writes with (int, optional)
+- "persistent": bitmap will be saved to the corresponding block device image
+ file on its close. For now only Qcow2 disks support persistent
+ bitmaps. (json-bool, optional, default false) (Since 2.9)
Example:
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 6b42216960..b6a255395a 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1231,10 +1231,15 @@
# @granularity: #optional the bitmap granularity, default is 64k for
# block-dirty-bitmap-add
#
+# @persistent: #optional the bitmap is persistent, i.e. it will be saved to the
+# corresponding block device image file on its close. Default is
+# false. (Since 2.9)
+#
# Since: 2.4
##
{ 'struct': 'BlockDirtyBitmapAdd',
- 'data': { 'node': 'str', 'name': 'str', '*granularity': 'uint32' } }
+ 'data': { 'node': 'str', 'name': 'str', '*granularity': 'uint32',
+ '*persistent': 'bool' } }
##
# @block-dirty-bitmap-add:
--
2.11.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH 17/24] qmp: add autoload parameter to block-dirty-bitmap-add
2017-01-23 12:10 [Qemu-devel] [PATCH v11 00/24] qcow2: persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
` (15 preceding siblings ...)
2017-01-23 12:10 ` [Qemu-devel] [PATCH 16/24] qmp: add persistent flag to block-dirty-bitmap-add Vladimir Sementsov-Ogievskiy
@ 2017-01-23 12:10 ` Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 18/24] qmp: add x-debug-block-dirty-bitmap-sha256 Vladimir Sementsov-Ogievskiy
` (6 subsequent siblings)
23 siblings, 0 replies; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-01-23 12:10 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: kwolf, mreitz, armbru, eblake, jsnow, famz, den, stefanha,
vsementsov, pbonzini
Optional. Default is false.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
blockdev.c | 18 ++++++++++++++++--
docs/qmp-commands.txt | 4 ++++
qapi/block-core.json | 6 +++++-
3 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 40605faccc..e32ac69e4b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1968,6 +1968,7 @@ static void block_dirty_bitmap_add_prepare(BlkActionState *common,
qmp_block_dirty_bitmap_add(action->node, action->name,
action->has_granularity, action->granularity,
action->has_persistent, action->persistent,
+ action->has_autoload, action->autoload,
&local_err);
if (!local_err) {
@@ -2698,6 +2699,7 @@ out:
void qmp_block_dirty_bitmap_add(const char *node, const char *name,
bool has_granularity, uint32_t granularity,
bool has_persistent, bool persistent,
+ bool has_autoload, bool autoload,
Error **errp)
{
AioContext *aio_context;
@@ -2731,6 +2733,15 @@ void qmp_block_dirty_bitmap_add(const char *node, const char *name,
if (!has_persistent) {
persistent = false;
}
+ if (!has_autoload) {
+ autoload = false;
+ }
+
+ if (has_autoload && !persistent) {
+ error_setg(errp, "Autoload flag must be used only for persistent "
+ "bitmaps");
+ goto out;
+ }
if (persistent &&
!bdrv_can_store_new_dirty_bitmap(bs, name, granularity, errp))
@@ -2739,10 +2750,13 @@ void qmp_block_dirty_bitmap_add(const char *node, const char *name,
}
bitmap = bdrv_create_dirty_bitmap(bs, granularity, name, errp);
- if (bitmap != NULL) {
- bdrv_dirty_bitmap_set_persistance(bitmap, persistent);
+ if (bitmap == NULL) {
+ goto out;
}
+ bdrv_dirty_bitmap_set_persistance(bitmap, persistent);
+ bdrv_dirty_bitmap_set_autoload(bitmap, autoload);
+
out:
aio_context_release(aio_context);
}
diff --git a/docs/qmp-commands.txt b/docs/qmp-commands.txt
index ef0245d0df..62469c06da 100644
--- a/docs/qmp-commands.txt
+++ b/docs/qmp-commands.txt
@@ -1018,6 +1018,10 @@ Arguments:
- "persistent": bitmap will be saved to the corresponding block device image
file on its close. For now only Qcow2 disks support persistent
bitmaps. (json-bool, optional, default false) (Since 2.9)
+- "autoload": the bitmap will be automatically loaded when the image it is
+ stored in is opened. This flag may only be specified for
+ persistent bitmaps (json-bool, optional, default false)
+ (Since 2.9)
Example:
diff --git a/qapi/block-core.json b/qapi/block-core.json
index b6a255395a..2c0aa1f44d 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1235,11 +1235,15 @@
# corresponding block device image file on its close. Default is
# false. (Since 2.9)
#
+# @autoload: #optional the bitmap will be automatically loaded when the image
+# it is stored in is opened. This flag may only be specified for
+# persistent bitmaps. Default is false. (Since 2.9)
+#
# Since: 2.4
##
{ 'struct': 'BlockDirtyBitmapAdd',
'data': { 'node': 'str', 'name': 'str', '*granularity': 'uint32',
- '*persistent': 'bool' } }
+ '*persistent': 'bool', '*autoload': 'bool' } }
##
# @block-dirty-bitmap-add:
--
2.11.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH 18/24] qmp: add x-debug-block-dirty-bitmap-sha256
2017-01-23 12:10 [Qemu-devel] [PATCH v11 00/24] qcow2: persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
` (16 preceding siblings ...)
2017-01-23 12:10 ` [Qemu-devel] [PATCH 17/24] qmp: add autoload parameter " Vladimir Sementsov-Ogievskiy
@ 2017-01-23 12:10 ` Vladimir Sementsov-Ogievskiy
2017-01-29 16:20 ` Max Reitz
2017-01-23 12:10 ` [Qemu-devel] [PATCH 19/24] iotests: test qcow2 persistent dirty bitmap Vladimir Sementsov-Ogievskiy
` (5 subsequent siblings)
23 siblings, 1 reply; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-01-23 12:10 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: kwolf, mreitz, armbru, eblake, jsnow, famz, den, stefanha,
vsementsov, pbonzini
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/dirty-bitmap.c | 5 +++++
blockdev.c | 29 +++++++++++++++++++++++++++++
include/block/dirty-bitmap.h | 2 ++
include/qemu/hbitmap.h | 8 ++++++++
qapi/block-core.json | 27 +++++++++++++++++++++++++++
tests/Makefile.include | 2 +-
util/hbitmap.c | 11 +++++++++++
7 files changed, 83 insertions(+), 1 deletion(-)
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index fe34d488aa..775181c9ab 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -589,3 +589,8 @@ BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BlockDriverState *bs,
return bitmap == NULL ? QLIST_FIRST(&bs->dirty_bitmaps) :
QLIST_NEXT(bitmap, list);
}
+
+char *bdrv_dirty_bitmap_sha256(const BdrvDirtyBitmap *bitmap, Error **errp)
+{
+ return hbitmap_sha256(bitmap->bitmap, errp);
+}
diff --git a/blockdev.c b/blockdev.c
index e32ac69e4b..c41b791289 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2820,6 +2820,35 @@ void qmp_block_dirty_bitmap_clear(const char *node, const char *name,
aio_context_release(aio_context);
}
+BlockDirtyBitmapSha256 *qmp_x_debug_block_dirty_bitmap_sha256(const char *node,
+ const char *name,
+ Error **errp)
+{
+ AioContext *aio_context;
+ BdrvDirtyBitmap *bitmap;
+ BlockDriverState *bs;
+ BlockDirtyBitmapSha256 *ret = NULL;
+ char *sha256;
+
+ bitmap = block_dirty_bitmap_lookup(node, name, &bs, &aio_context, errp);
+ if (!bitmap || !bs) {
+ return NULL;
+ }
+
+ sha256 = bdrv_dirty_bitmap_sha256(bitmap, errp);
+ if (sha256 == NULL) {
+ goto out;
+ }
+
+ ret = g_new(BlockDirtyBitmapSha256, 1);
+ ret->sha256 = sha256;
+
+out:
+ aio_context_release(aio_context);
+
+ return ret;
+}
+
void hmp_drive_del(Monitor *mon, const QDict *qdict)
{
const char *id = qdict_get_str(qdict, "id");
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index d71edc4d13..b022b34964 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -86,4 +86,6 @@ BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BlockDriverState *bs,
bool bdrv_has_persistent_bitmaps(BlockDriverState *bs);
+char *bdrv_dirty_bitmap_sha256(const BdrvDirtyBitmap *bitmap, Error **errp);
+
#endif
diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
index 063ec0e7ad..750346ce40 100644
--- a/include/qemu/hbitmap.h
+++ b/include/qemu/hbitmap.h
@@ -240,6 +240,14 @@ void hbitmap_deserialize_ones(HBitmap *hb, uint64_t start, uint64_t count,
void hbitmap_deserialize_finish(HBitmap *hb);
/**
+ * hbitmap_sha256:
+ * @bitmap: HBitmap to operate on.
+ *
+ * Returns SHA256 hash of the last level.
+ */
+char *hbitmap_sha256(const HBitmap *bitmap, Error **errp);
+
+/**
* hbitmap_free:
* @hb: HBitmap to operate on.
*
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 2c0aa1f44d..956b69567d 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1289,6 +1289,33 @@
'data': 'BlockDirtyBitmap' }
##
+# @BlockDirtyBitmapSha256:
+#
+# SHA256 hash of dirty bitmap data
+#
+# @sha256: ASCII representation of SHA256 bitmap hash
+#
+# Since: 2.9
+##
+ { 'struct': 'BlockDirtyBitmapSha256',
+ 'data': {'sha256': 'str'} }
+
+##
+# @x-debug-block-dirty-bitmap-sha256:
+#
+# Get bitmap SHA256
+#
+# Returns: BlockDirtyBitmapSha256 on success
+# If @node is not a valid block device, DeviceNotFound
+# If @name is not found or if hashing is failed, GenericError with an
+# explanation
+#
+# Since: 2.9
+##
+ { 'command': 'x-debug-block-dirty-bitmap-sha256',
+ 'data': 'BlockDirtyBitmap', 'returns': 'BlockDirtyBitmapSha256' }
+
+##
# @blockdev-mirror:
#
# Start mirroring a block device's writes to a new destination.
diff --git a/tests/Makefile.include b/tests/Makefile.include
index e98d3b6bb3..b85ff7899d 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -496,7 +496,7 @@ tests/test-blockjob$(EXESUF): tests/test-blockjob.o $(test-block-obj-y) $(test-u
tests/test-blockjob-txn$(EXESUF): tests/test-blockjob-txn.o $(test-block-obj-y) $(test-util-obj-y)
tests/test-thread-pool$(EXESUF): tests/test-thread-pool.o $(test-block-obj-y)
tests/test-iov$(EXESUF): tests/test-iov.o $(test-util-obj-y)
-tests/test-hbitmap$(EXESUF): tests/test-hbitmap.o $(test-util-obj-y)
+tests/test-hbitmap$(EXESUF): tests/test-hbitmap.o $(test-util-obj-y) $(test-crypto-obj-y)
tests/test-x86-cpuid$(EXESUF): tests/test-x86-cpuid.o
tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o migration/xbzrle.o page_cache.o $(test-util-obj-y)
tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o
diff --git a/util/hbitmap.c b/util/hbitmap.c
index 47418a915e..2dc09b811a 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -13,6 +13,7 @@
#include "qemu/hbitmap.h"
#include "qemu/host-utils.h"
#include "trace.h"
+#include "crypto/hash.h"
/* HBitmaps provides an array of bits. The bits are stored as usual in an
* array of unsigned longs, but HBitmap is also optimized to provide fast
@@ -711,3 +712,13 @@ void hbitmap_free_meta(HBitmap *hb)
hbitmap_free(hb->meta);
hb->meta = NULL;
}
+
+char *hbitmap_sha256(const HBitmap *bitmap, Error **errp)
+{
+ size_t size = bitmap->sizes[HBITMAP_LEVELS - 1] * sizeof(unsigned long);
+ char *data = (char *)bitmap->levels[HBITMAP_LEVELS - 1];
+ char *hash = NULL;
+ qcrypto_hash_digest(QCRYPTO_HASH_ALG_SHA256, data, size, &hash, errp);
+
+ return hash;
+}
--
2.11.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH 19/24] iotests: test qcow2 persistent dirty bitmap
2017-01-23 12:10 [Qemu-devel] [PATCH v11 00/24] qcow2: persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
` (17 preceding siblings ...)
2017-01-23 12:10 ` [Qemu-devel] [PATCH 18/24] qmp: add x-debug-block-dirty-bitmap-sha256 Vladimir Sementsov-Ogievskiy
@ 2017-01-23 12:10 ` Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 20/24] qcow2-refcount: rename inc_refcounts() and make it public Vladimir Sementsov-Ogievskiy
` (4 subsequent siblings)
23 siblings, 0 replies; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-01-23 12:10 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: kwolf, mreitz, armbru, eblake, jsnow, famz, den, stefanha,
vsementsov, pbonzini
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/165 | 89 ++++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/165.out | 5 +++
tests/qemu-iotests/group | 1 +
3 files changed, 95 insertions(+)
create mode 100755 tests/qemu-iotests/165
create mode 100644 tests/qemu-iotests/165.out
diff --git a/tests/qemu-iotests/165 b/tests/qemu-iotests/165
new file mode 100755
index 0000000000..11d2e22317
--- /dev/null
+++ b/tests/qemu-iotests/165
@@ -0,0 +1,89 @@
+#!/usr/bin/env python
+#
+# Tests for persistent dirty bitmaps.
+#
+# Copyright: Vladimir Sementsov-Ogievskiy 2015-2016
+#
+# 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 <http://www.gnu.org/licenses/>.
+#
+
+import os
+import iotests
+from iotests import qemu_img
+
+disk = os.path.join(iotests.test_dir, 'disk')
+disk_size = 0x40000000 # 1G
+
+# regions for qemu_io: (start, count) in bytes
+regions1 = ((0, 0x100000),
+ (0x200000, 0x100000))
+
+regions2 = ((0x10000000, 0x20000),
+ (0x3fff0000, 0x10000))
+
+class TestPersistentDirtyBitmap(iotests.QMPTestCase):
+
+ def setUp(self):
+ qemu_img('create', '-f', iotests.imgfmt, disk, str(disk_size))
+
+ def tearDown(self):
+ os.remove(disk)
+
+ def mkVm(self):
+ return iotests.VM().add_drive(disk)
+
+ def getSha256(self):
+ result = self.vm.qmp('x-debug-block-dirty-bitmap-sha256',
+ node='drive0', name='bitmap0')
+ return result['return']['sha256']
+
+ def checkBitmap(self, sha256):
+ result = self.vm.qmp('x-debug-block-dirty-bitmap-sha256',
+ node='drive0', name='bitmap0')
+ self.assert_qmp(result, 'return/sha256', sha256);
+
+ def writeRegions(self, regions):
+ for r in regions:
+ self.vm.hmp_qemu_io('drive0',
+ 'write %d %d' % r)
+
+ def qmpAddBitmap(self):
+ self.vm.qmp('block-dirty-bitmap-add', node='drive0',
+ name='bitmap0', persistent=True, autoload=True)
+
+ def test_persistent(self):
+ self.vm = self.mkVm()
+ self.vm.launch()
+ self.qmpAddBitmap()
+
+ self.writeRegions(regions1)
+ sha256 = self.getSha256()
+
+ self.vm.shutdown()
+ self.vm = self.mkVm()
+ self.vm.launch()
+
+ self.checkBitmap(sha256)
+ self.writeRegions(regions2)
+ sha256 = self.getSha256()
+
+ self.vm.shutdown()
+ self.vm.launch()
+
+ self.checkBitmap(sha256)
+
+ self.vm.shutdown()
+
+if __name__ == '__main__':
+ iotests.main(supported_fmts=['qcow2'])
diff --git a/tests/qemu-iotests/165.out b/tests/qemu-iotests/165.out
new file mode 100644
index 0000000000..ae1213e6f8
--- /dev/null
+++ b/tests/qemu-iotests/165.out
@@ -0,0 +1,5 @@
+.
+----------------------------------------------------------------------
+Ran 1 tests
+
+OK
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 866c1a032d..8bd7a84f5a 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -162,6 +162,7 @@
159 rw auto quick
160 rw auto quick
162 auto quick
+165 rw auto quick
170 rw auto quick
171 rw auto quick
172 auto
--
2.11.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH 20/24] qcow2-refcount: rename inc_refcounts() and make it public
2017-01-23 12:10 [Qemu-devel] [PATCH v11 00/24] qcow2: persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
` (18 preceding siblings ...)
2017-01-23 12:10 ` [Qemu-devel] [PATCH 19/24] iotests: test qcow2 persistent dirty bitmap Vladimir Sementsov-Ogievskiy
@ 2017-01-23 12:10 ` Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 21/24] qcow2-bitmap: refcounts Vladimir Sementsov-Ogievskiy
` (3 subsequent siblings)
23 siblings, 0 replies; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-01-23 12:10 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: kwolf, mreitz, armbru, eblake, jsnow, famz, den, stefanha,
vsementsov, pbonzini
This is needed for the following patch, which will introduce refcounts
checking for qcow2 bitmaps.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
block/qcow2-refcount.c | 53 ++++++++++++++++++++++++++------------------------
block/qcow2.h | 4 ++++
2 files changed, 32 insertions(+), 25 deletions(-)
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index cbfb3fe064..14a736d245 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1309,11 +1309,10 @@ static int realloc_refcount_array(BDRVQcow2State *s, void **array,
*
* Modifies the number of errors in res.
*/
-static int inc_refcounts(BlockDriverState *bs,
- BdrvCheckResult *res,
- void **refcount_table,
- int64_t *refcount_table_size,
- int64_t offset, int64_t size)
+int qcow2_inc_refcounts_imrt(BlockDriverState *bs, BdrvCheckResult *res,
+ void **refcount_table,
+ int64_t *refcount_table_size,
+ int64_t offset, int64_t size)
{
BDRVQcow2State *s = bs->opaque;
uint64_t start, last, cluster_offset, k, refcount;
@@ -1406,8 +1405,9 @@ 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;
- ret = inc_refcounts(bs, res, refcount_table, refcount_table_size,
- l2_entry & ~511, nb_csectors * 512);
+ ret = qcow2_inc_refcounts_imrt(bs, res,
+ refcount_table, refcount_table_size,
+ l2_entry & ~511, nb_csectors * 512);
if (ret < 0) {
goto fail;
}
@@ -1445,8 +1445,9 @@ static int check_refcounts_l2(BlockDriverState *bs, BdrvCheckResult *res,
}
/* Mark cluster as used */
- ret = inc_refcounts(bs, res, refcount_table, refcount_table_size,
- offset, s->cluster_size);
+ ret = qcow2_inc_refcounts_imrt(bs, res,
+ refcount_table, refcount_table_size,
+ offset, s->cluster_size);
if (ret < 0) {
goto fail;
}
@@ -1498,8 +1499,8 @@ static int check_refcounts_l1(BlockDriverState *bs,
l1_size2 = l1_size * sizeof(uint64_t);
/* Mark L1 table as used */
- ret = inc_refcounts(bs, res, refcount_table, refcount_table_size,
- l1_table_offset, l1_size2);
+ ret = qcow2_inc_refcounts_imrt(bs, res, refcount_table, refcount_table_size,
+ l1_table_offset, l1_size2);
if (ret < 0) {
goto fail;
}
@@ -1528,8 +1529,9 @@ static int check_refcounts_l1(BlockDriverState *bs,
if (l2_offset) {
/* Mark L2 table as used */
l2_offset &= L1E_OFFSET_MASK;
- ret = inc_refcounts(bs, res, refcount_table, refcount_table_size,
- l2_offset, s->cluster_size);
+ ret = qcow2_inc_refcounts_imrt(bs, res,
+ refcount_table, refcount_table_size,
+ l2_offset, s->cluster_size);
if (ret < 0) {
goto fail;
}
@@ -1744,14 +1746,15 @@ static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res,
}
res->corruptions_fixed++;
- ret = inc_refcounts(bs, res, refcount_table, nb_clusters,
- offset, s->cluster_size);
+ ret = qcow2_inc_refcounts_imrt(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() */
+ * exactly 1 after qcow2_inc_refcounts_imrt() */
continue;
resize_fail:
@@ -1766,8 +1769,8 @@ resize_fail:
}
if (offset != 0) {
- ret = inc_refcounts(bs, res, refcount_table, nb_clusters,
- offset, s->cluster_size);
+ ret = qcow2_inc_refcounts_imrt(bs, res, refcount_table, nb_clusters,
+ offset, s->cluster_size);
if (ret < 0) {
return ret;
}
@@ -1807,8 +1810,8 @@ static int calculate_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
}
/* header */
- ret = inc_refcounts(bs, res, refcount_table, nb_clusters,
- 0, s->cluster_size);
+ ret = qcow2_inc_refcounts_imrt(bs, res, refcount_table, nb_clusters,
+ 0, s->cluster_size);
if (ret < 0) {
return ret;
}
@@ -1829,16 +1832,16 @@ static int calculate_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
return ret;
}
}
- ret = inc_refcounts(bs, res, refcount_table, nb_clusters,
- s->snapshots_offset, s->snapshots_size);
+ ret = qcow2_inc_refcounts_imrt(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,
- s->refcount_table_offset,
- s->refcount_table_size * sizeof(uint64_t));
+ ret = qcow2_inc_refcounts_imrt(bs, res, refcount_table, nb_clusters,
+ s->refcount_table_offset,
+ s->refcount_table_size * sizeof(uint64_t));
if (ret < 0) {
return ret;
}
diff --git a/block/qcow2.h b/block/qcow2.h
index 749710dc37..0a3708d890 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -543,6 +543,10 @@ int qcow2_check_metadata_overlap(BlockDriverState *bs, int ign, int64_t offset,
int64_t size);
int qcow2_pre_write_overlap_check(BlockDriverState *bs, int ign, int64_t offset,
int64_t size);
+int qcow2_inc_refcounts_imrt(BlockDriverState *bs, BdrvCheckResult *res,
+ void **refcount_table,
+ int64_t *refcount_table_size,
+ int64_t offset, int64_t size);
int qcow2_change_refcount_order(BlockDriverState *bs, int refcount_order,
BlockDriverAmendStatusCB *status_cb,
--
2.11.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH 21/24] qcow2-bitmap: refcounts
2017-01-23 12:10 [Qemu-devel] [PATCH v11 00/24] qcow2: persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
` (19 preceding siblings ...)
2017-01-23 12:10 ` [Qemu-devel] [PATCH 20/24] qcow2-refcount: rename inc_refcounts() and make it public Vladimir Sementsov-Ogievskiy
@ 2017-01-23 12:10 ` Vladimir Sementsov-Ogievskiy
2017-01-29 16:42 ` Max Reitz
2017-01-23 12:10 ` [Qemu-devel] [PATCH 22/24] block/dirty-bitmap: deep release dirty bitmaps Vladimir Sementsov-Ogievskiy
` (2 subsequent siblings)
23 siblings, 1 reply; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-01-23 12:10 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: kwolf, mreitz, armbru, eblake, jsnow, famz, den, stefanha,
vsementsov, pbonzini
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;
+ }
+
+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
--
2.11.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH 22/24] block/dirty-bitmap: deep release dirty bitmaps
2017-01-23 12:10 [Qemu-devel] [PATCH v11 00/24] qcow2: persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
` (20 preceding siblings ...)
2017-01-23 12:10 ` [Qemu-devel] [PATCH 21/24] qcow2-bitmap: refcounts Vladimir Sementsov-Ogievskiy
@ 2017-01-23 12:10 ` Vladimir Sementsov-Ogievskiy
2017-01-29 16:54 ` Max Reitz
2017-01-23 12:10 ` [Qemu-devel] [PATCH 23/24] qcow2: add .bdrv_remove_persistent_dirty_bitmap Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 24/24] qcow2-bitmap: cache bitmap list in BDRVQcow2State Vladimir Sementsov-Ogievskiy
23 siblings, 1 reply; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-01-23 12:10 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: kwolf, mreitz, armbru, eblake, jsnow, famz, den, stefanha,
vsementsov, pbonzini
Remove persistent bitmap from its storage on bdrv_release_dirty_bitmap.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/dirty-bitmap.c | 21 ++++++++++++++++++---
include/block/block_int.h | 3 +++
2 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 775181c9ab..0977df6309 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -297,7 +297,8 @@ void bdrv_dirty_bitmap_truncate(BlockDriverState *bs)
static void bdrv_do_release_matching_dirty_bitmap(BlockDriverState *bs,
BdrvDirtyBitmap *bitmap,
- bool only_named)
+ bool only_named,
+ bool deep)
{
BdrvDirtyBitmap *bm, *next;
QLIST_FOREACH_SAFE(bm, &bs->dirty_bitmaps, list, next) {
@@ -305,6 +306,19 @@ static void bdrv_do_release_matching_dirty_bitmap(BlockDriverState *bs,
assert(!bm->active_iterators);
assert(!bdrv_dirty_bitmap_frozen(bm));
assert(!bm->meta);
+
+ if (deep && bm->persistent && bs->drv &&
+ bs->drv->bdrv_remove_persistent_dirty_bitmap)
+ {
+ Error *local_err = NULL;
+ bs->drv->bdrv_remove_persistent_dirty_bitmap(bs, bm->name,
+ &local_err);
+ if (local_err != NULL) {
+ error_report_err(local_err);
+ }
+ }
+
+
QLIST_REMOVE(bm, list);
hbitmap_free(bm->bitmap);
g_free(bm->name);
@@ -322,16 +336,17 @@ static void bdrv_do_release_matching_dirty_bitmap(BlockDriverState *bs,
void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
{
- bdrv_do_release_matching_dirty_bitmap(bs, bitmap, false);
+ bdrv_do_release_matching_dirty_bitmap(bs, bitmap, false, true);
}
/**
* Release all named dirty bitmaps attached to a BDS (for use in bdrv_close()).
* There must not be any frozen bitmaps attached.
+ * This function do not remove persistent bitmaps from the storage.
*/
void bdrv_release_named_dirty_bitmaps(BlockDriverState *bs)
{
- bdrv_do_release_matching_dirty_bitmap(bs, NULL, true);
+ bdrv_do_release_matching_dirty_bitmap(bs, NULL, true, false);
}
void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index b38b3aa198..3e080d6d68 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -326,6 +326,9 @@ struct BlockDriver {
Error **errp);
bool (*bdrv_can_store_new_dirty_bitmap)(BlockDriverState *bs, const char *name,
uint32_t granularity, Error **errp);
+ void (*bdrv_remove_persistent_dirty_bitmap)(BlockDriverState *bs,
+ const char *name,
+ Error **errp);
QLIST_ENTRY(BlockDriver) list;
};
--
2.11.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH 23/24] qcow2: add .bdrv_remove_persistent_dirty_bitmap
2017-01-23 12:10 [Qemu-devel] [PATCH v11 00/24] qcow2: persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
` (21 preceding siblings ...)
2017-01-23 12:10 ` [Qemu-devel] [PATCH 22/24] block/dirty-bitmap: deep release dirty bitmaps Vladimir Sementsov-Ogievskiy
@ 2017-01-23 12:10 ` Vladimir Sementsov-Ogievskiy
2017-01-31 23:20 ` Max Reitz
2017-01-23 12:10 ` [Qemu-devel] [PATCH 24/24] qcow2-bitmap: cache bitmap list in BDRVQcow2State Vladimir Sementsov-Ogievskiy
23 siblings, 1 reply; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-01-23 12:10 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: kwolf, mreitz, armbru, eblake, jsnow, famz, den, stefanha,
vsementsov, pbonzini
Realize .bdrv_remove_persistent_dirty_bitmap interface.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/qcow2-bitmap.c | 40 ++++++++++++++++++++++++++++++++++++++++
block/qcow2.c | 1 +
block/qcow2.h | 3 +++
3 files changed, 44 insertions(+)
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index 2687a3acd5..be026fc80e 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -1064,6 +1064,46 @@ static Qcow2Bitmap *find_bitmap_by_name(Qcow2BitmapList *bm_list,
return NULL;
}
+void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs,
+ const char *name,
+ Error **errp)
+{
+ int ret;
+ BDRVQcow2State *s = bs->opaque;
+ Qcow2Bitmap *bm;
+ Qcow2BitmapList *bm_list;
+
+ if (s->nb_bitmaps == 0) {
+ /* No bitmaps - nothing to do */
+ return;
+ }
+
+ bm_list = bitmap_list_load(bs, s->bitmap_directory_offset,
+ s->bitmap_directory_size, errp);
+ if (bm_list == NULL) {
+ return;
+ }
+
+ bm = find_bitmap_by_name(bm_list, name);
+ if (bm == NULL) {
+ goto fail;
+ }
+
+ QSIMPLEQ_REMOVE(bm_list, bm, Qcow2Bitmap, entry);
+
+ ret = update_ext_header_and_dir(bs, bm_list);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "Failed to update bitmap extension");
+ goto fail;
+ }
+
+ free_bitmap_clusters(bs, &bm->table);
+
+fail:
+ bitmap_free(bm);
+ bitmap_list_free(bm_list);
+}
+
void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp)
{
BdrvDirtyBitmap *bitmap;
diff --git a/block/qcow2.c b/block/qcow2.c
index 76a6a2ba28..f6bac38e51 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3544,6 +3544,7 @@ BlockDriver bdrv_qcow2 = {
.bdrv_load_autoloading_dirty_bitmaps = qcow2_load_autoloading_dirty_bitmaps,
.bdrv_store_persistent_dirty_bitmaps = qcow2_store_persistent_dirty_bitmaps,
.bdrv_can_store_new_dirty_bitmap = qcow2_can_store_new_dirty_bitmap,
+ .bdrv_remove_persistent_dirty_bitmap = qcow2_remove_persistent_dirty_bitmap,
};
static void bdrv_qcow2_init(void)
diff --git a/block/qcow2.h b/block/qcow2.h
index eaad34a527..a009827f60 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -627,5 +627,8 @@ bool qcow2_can_store_new_dirty_bitmap(BlockDriverState *bs,
int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
void **refcount_table,
int64_t *refcount_table_size);
+void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs,
+ const char *name,
+ Error **errp);
#endif
--
2.11.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH 24/24] qcow2-bitmap: cache bitmap list in BDRVQcow2State
2017-01-23 12:10 [Qemu-devel] [PATCH v11 00/24] qcow2: persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
` (22 preceding siblings ...)
2017-01-23 12:10 ` [Qemu-devel] [PATCH 23/24] qcow2: add .bdrv_remove_persistent_dirty_bitmap Vladimir Sementsov-Ogievskiy
@ 2017-01-23 12:10 ` Vladimir Sementsov-Ogievskiy
2017-01-31 23:27 ` Max Reitz
23 siblings, 1 reply; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-01-23 12:10 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: kwolf, mreitz, armbru, eblake, jsnow, famz, den, stefanha,
vsementsov, pbonzini
Do not reload bitmap list every time it is needed.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/qcow2-bitmap.c | 125 ++++++++++++++++++++++++++++++++++-----------------
block/qcow2.c | 2 +
block/qcow2.h | 4 ++
3 files changed, 89 insertions(+), 42 deletions(-)
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index be026fc80e..dd987a111c 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -79,7 +79,7 @@ typedef struct Qcow2BitmapTable {
} Qcow2BitmapTable;
typedef QSIMPLEQ_HEAD(Qcow2BitmapTableList, Qcow2BitmapTable) Qcow2BitmapTableList;
-typedef struct Qcow2Bitmap {
+struct Qcow2Bitmap {
Qcow2BitmapTable table;
uint32_t flags;
uint8_t granularity_bits;
@@ -88,8 +88,7 @@ typedef struct Qcow2Bitmap {
BdrvDirtyBitmap *dirty_bitmap;
QSIMPLEQ_ENTRY(Qcow2Bitmap) entry;
-} Qcow2Bitmap;
-typedef QSIMPLEQ_HEAD(Qcow2BitmapList, Qcow2Bitmap) Qcow2BitmapList;
+};
typedef enum BitmapType {
BT_DIRTY_TRACKING_BITMAP = 1
@@ -492,6 +491,14 @@ static void bitmap_free(Qcow2Bitmap *bm)
g_free(bm);
}
+static Qcow2Bitmap *bitmap_dup(Qcow2Bitmap *bm)
+{
+ Qcow2Bitmap *copy = g_memdup(bm, sizeof(*bm));
+ copy->name = g_strdup(bm->name);
+
+ return copy;
+}
+
static void bitmap_list_free(Qcow2BitmapList *bm_list)
{
Qcow2Bitmap *bm;
@@ -729,6 +736,44 @@ fail:
* Bitmap List end
*/
+static const Qcow2BitmapList *qcow2_get_bitmap_list_const(BlockDriverState *bs,
+ Error **errp)
+{
+ BDRVQcow2State *s = bs->opaque;
+
+ if (s->bitmaps != NULL) {
+ return s->bitmaps;
+ }
+
+ if (s->nb_bitmaps == 0) {
+ return NULL;
+ }
+
+ return s->bitmaps = bitmap_list_load(bs, s->bitmap_directory_offset,
+ s->bitmap_directory_size, errp);
+}
+
+static Qcow2BitmapList *qcow2_get_bitmap_list(BlockDriverState *bs,
+ Error **errp)
+{
+ Qcow2Bitmap *bm;
+ Qcow2BitmapList *bm_list_copy;
+ const Qcow2BitmapList *bm_list = qcow2_get_bitmap_list_const(bs, errp);
+
+ if (bm_list == NULL) {
+ return NULL;
+ }
+
+ bm_list_copy = bitmap_list_new();
+
+ QSIMPLEQ_FOREACH(bm, bm_list, entry) {
+ QSIMPLEQ_INSERT_TAIL(bm_list_copy, bitmap_dup(bm), entry);
+ }
+
+ return bm_list_copy;
+}
+
+/* This function consumes bm_list on success. */
static int update_ext_header_and_dir_in_place(BlockDriverState *bs,
Qcow2BitmapList *bm_list)
{
@@ -759,9 +804,20 @@ static int update_ext_header_and_dir_in_place(BlockDriverState *bs,
}
s->autoclear_features |= QCOW2_AUTOCLEAR_BITMAPS;
- return update_header_sync(bs);
+ ret = update_header_sync(bs);
+ if (ret < 0) {
+ return ret;
+ }
+
+ /* success, consume bm_list */
+ assert(s->bitmaps != bm_list);
+ bitmap_list_free(s->bitmaps);
+ s->bitmaps = bm_list;
+
+ return 0;
}
+/* This function consumes bm_list on success. */
static int update_ext_header_and_dir(BlockDriverState *bs,
Qcow2BitmapList *bm_list)
{
@@ -810,6 +866,11 @@ static int update_ext_header_and_dir(BlockDriverState *bs,
qcow2_free_clusters(bs, old_offset, old_size, QCOW2_DISCARD_OTHER);
}
+ /* success, consume bm_list */
+ assert(s->bitmaps != bm_list);
+ bitmap_list_free(s->bitmaps);
+ s->bitmaps = bm_list;
+
return 0;
fail:
@@ -834,18 +895,11 @@ static void release_dirty_bitmap_helper(gpointer bitmap,
void qcow2_load_autoloading_dirty_bitmaps(BlockDriverState *bs, Error **errp)
{
- BDRVQcow2State *s = bs->opaque;
Qcow2BitmapList *bm_list;
Qcow2Bitmap *bm;
GSList *created_dirty_bitmaps = NULL;
- if (s->nb_bitmaps == 0) {
- /* No bitmaps - nothing to do */
- return;
- }
-
- bm_list = bitmap_list_load(bs, s->bitmap_directory_offset,
- s->bitmap_directory_size, errp);
+ bm_list = qcow2_get_bitmap_list(bs, errp);
if (bm_list == NULL) {
return;
}
@@ -875,9 +929,8 @@ void qcow2_load_autoloading_dirty_bitmaps(BlockDriverState *bs, Error **errp)
}
g_slist_free(created_dirty_bitmaps);
- bitmap_list_free(bm_list);
- return;
+ return; /* bm_list is consumed */
fail:
g_slist_foreach(created_dirty_bitmaps, release_dirty_bitmap_helper, bs);
@@ -1069,17 +1122,10 @@ void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs,
Error **errp)
{
int ret;
- BDRVQcow2State *s = bs->opaque;
Qcow2Bitmap *bm;
Qcow2BitmapList *bm_list;
- if (s->nb_bitmaps == 0) {
- /* No bitmaps - nothing to do */
- return;
- }
-
- bm_list = bitmap_list_load(bs, s->bitmap_directory_offset,
- s->bitmap_directory_size, errp);
+ bm_list = qcow2_get_bitmap_list(bs, errp);
if (bm_list == NULL) {
return;
}
@@ -1131,10 +1177,8 @@ void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp)
if (s->nb_bitmaps == 0) {
bm_list = bitmap_list_new();
} else {
- bm_list = bitmap_list_load(bs, s->bitmap_directory_offset,
- s->bitmap_directory_size, errp);
+ bm_list = qcow2_get_bitmap_list(bs, errp);
if (bm_list == NULL) {
- /* errp is already set */
return;
}
}
@@ -1214,8 +1258,7 @@ void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp)
g_free(tb);
}
- bitmap_list_free(bm_list);
- return;
+ return; /* bm_list is consumed */
fail:
QSIMPLEQ_FOREACH(bm, bm_list, entry) {
@@ -1252,8 +1295,7 @@ bool qcow2_can_store_new_dirty_bitmap(BlockDriverState *bs,
return true;
}
- bm_list = bitmap_list_load(bs, s->bitmap_directory_offset,
- s->bitmap_directory_size, errp);
+ bm_list = qcow2_get_bitmap_list(bs, errp);
if (bm_list == NULL) {
return false;
}
@@ -1279,7 +1321,7 @@ int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
{
int ret;
BDRVQcow2State *s = bs->opaque;
- Qcow2BitmapList *bm_list;
+ const Qcow2BitmapList *bm_list;
Qcow2Bitmap *bm;
if (s->nb_bitmaps == 0) {
@@ -1293,8 +1335,7 @@ int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
return ret;
}
- bm_list = bitmap_list_load(bs, s->bitmap_directory_offset,
- s->bitmap_directory_size, NULL);
+ bm_list = qcow2_get_bitmap_list_const(bs, NULL);
if (bm_list == NULL) {
res->corruptions++;
return -EINVAL;
@@ -1309,13 +1350,13 @@ int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
bm->table.offset,
bm->table.size * sizeof(uint64_t));
if (ret < 0) {
- goto out;
+ return ret;
}
ret = bitmap_table_load(bs, &bm->table, &bitmap_table);
if (ret < 0) {
res->corruptions++;
- goto out;
+ return ret;
}
for (i = 0; i < bm->table.size; ++i) {
@@ -1336,19 +1377,19 @@ int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
offset, s->cluster_size);
if (ret < 0) {
g_free(bitmap_table);
- goto out;
+ return ret;
}
}
g_free(bitmap_table);
}
- if (res->corruptions > 0) {
- ret = -EINVAL;
- }
-
-out:
- bitmap_list_free(bm_list);
+ return res->corruptions > 0 ? -EINVAL : 0;
+}
- return ret;
+void qcow2_free_bitmaps(BlockDriverState *bs)
+{
+ BDRVQcow2State *s = bs->opaque;
+ bitmap_list_free(s->bitmaps);
+ s->bitmaps = NULL;
}
diff --git a/block/qcow2.c b/block/qcow2.c
index f6bac38e51..f6544d52ad 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1850,6 +1850,8 @@ static void qcow2_close(BlockDriverState *bs)
qemu_vfree(s->cluster_data);
qcow2_refcount_close(bs);
qcow2_free_snapshots(bs);
+
+ qcow2_free_bitmaps(bs);
}
static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp)
diff --git a/block/qcow2.h b/block/qcow2.h
index a009827f60..ed8f5a73b0 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -242,6 +242,8 @@ typedef struct Qcow2BitmapHeaderExt {
uint64_t bitmap_directory_offset;
} QEMU_PACKED Qcow2BitmapHeaderExt;
+typedef struct Qcow2Bitmap Qcow2Bitmap;
+typedef QSIMPLEQ_HEAD(Qcow2BitmapList, Qcow2Bitmap) Qcow2BitmapList;
typedef struct BDRVQcow2State {
int cluster_bits;
int cluster_size;
@@ -286,6 +288,7 @@ typedef struct BDRVQcow2State {
uint32_t nb_bitmaps;
uint64_t bitmap_directory_size;
uint64_t bitmap_directory_offset;
+ Qcow2BitmapList *bitmaps; /* cached version of bitmaps list in file */
int flags;
int qcow_version;
@@ -630,5 +633,6 @@ int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs,
const char *name,
Error **errp);
+void qcow2_free_bitmaps(BlockDriverState *bs);
#endif
--
2.11.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH 07/24] qcow2: add bitmaps extension
2017-01-23 12:10 ` [Qemu-devel] [PATCH 07/24] qcow2: add bitmaps extension Vladimir Sementsov-Ogievskiy
@ 2017-01-28 21:29 ` Max Reitz
2017-01-30 7:21 ` Vladimir Sementsov-Ogievskiy
2017-01-31 11:53 ` Vladimir Sementsov-Ogievskiy
1 sibling, 1 reply; 55+ messages in thread
From: Max Reitz @ 2017-01-28 21:29 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block, qemu-devel
Cc: kwolf, armbru, eblake, jsnow, famz, den, stefanha, pbonzini
[-- Attachment #1: Type: text/plain, Size: 1338 bytes --]
On 23.01.2017 13:10, Vladimir Sementsov-Ogievskiy wrote:
> Add bitmap extension as specified in docs/specs/qcow2.txt.
> For now, just mirror extension header into Qcow2 state and check
> constraints.
>
> For now, disable image resize if it has bitmaps. It will be fixed later.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> block/qcow2.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> block/qcow2.h | 24 ++++++++++++
> 2 files changed, 141 insertions(+), 2 deletions(-)
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 96fb8a8f16..a8835988c7 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
[...]
> @@ -185,6 +265,15 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
> offset += ((ext.len + 7) & ~7);
> }
>
> + if (need_update && !bdrv_is_root_node(bs) &&
> + !(bdrv_get_flags(bs) & BDRV_O_INACTIVE))
> + {
> + ret = qcow2_update_header(bs);
Good idea, but qcow2_read_extensions() can be called pretty early in
qcow2_open(). Therefore, some fields in the BDRVQcow2State are not
necessarily set already.
For instance, it is always called before s->snapshots_offset and
s->nb_snapshots are set. Thus, this will effectively always discard all
snapshots.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH 08/24] block: introduce auto-loading bitmaps
2017-01-23 12:10 ` [Qemu-devel] [PATCH 08/24] block: introduce auto-loading bitmaps Vladimir Sementsov-Ogievskiy
@ 2017-01-28 21:31 ` Max Reitz
0 siblings, 0 replies; 55+ messages in thread
From: Max Reitz @ 2017-01-28 21:31 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block, qemu-devel
Cc: kwolf, armbru, eblake, jsnow, famz, den, stefanha, pbonzini
[-- Attachment #1: Type: text/plain, Size: 574 bytes --]
On 23.01.2017 13:10, Vladimir Sementsov-Ogievskiy wrote:
> Auto loading bitmaps are bitmaps stored in the disk image, which should
> be loaded when the image is opened and become BdrvDirtyBitmaps for the
> corresponding drive.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Reviewed-by: John Snow <jsnow@redhat.com>
> ---
> block.c | 14 ++++++++++++++
> include/block/block.h | 2 ++
> include/block/block_int.h | 3 +++
> 3 files changed, 19 insertions(+)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH 09/24] qcow2: add .bdrv_load_autoloading_dirty_bitmaps
2017-01-23 12:10 ` [Qemu-devel] [PATCH 09/24] qcow2: add .bdrv_load_autoloading_dirty_bitmaps Vladimir Sementsov-Ogievskiy
@ 2017-01-28 21:45 ` Max Reitz
2017-01-28 22:20 ` Max Reitz
1 sibling, 0 replies; 55+ messages in thread
From: Max Reitz @ 2017-01-28 21:45 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block, qemu-devel
Cc: kwolf, armbru, eblake, jsnow, famz, den, stefanha, pbonzini
[-- Attachment #1: Type: text/plain, Size: 2816 bytes --]
On 23.01.2017 13:10, Vladimir Sementsov-Ogievskiy wrote:
> Auto loading bitmaps are bitmaps in Qcow2, with the AUTO flag set. They
> are loaded when the image is opened and become BdrvDirtyBitmaps for the
> corresponding drive.
>
> Extra data in bitmaps is not supported for now.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> block/Makefile.objs | 2 +-
> block/qcow2-bitmap.c | 712 +++++++++++++++++++++++++++++++++++++++++++++++++++
> block/qcow2.c | 2 +
> block/qcow2.h | 3 +
> 4 files changed, 718 insertions(+), 1 deletion(-)
> create mode 100644 block/qcow2-bitmap.c
>
> diff --git a/block/Makefile.objs b/block/Makefile.objs
> index 67a036a1df..b8b6cf38a0 100644
> --- a/block/Makefile.objs
> +++ b/block/Makefile.objs
> @@ -1,5 +1,5 @@
> block-obj-y += raw_bsd.o qcow.o vdi.o vmdk.o cloop.o bochs.o vpc.o vvfat.o dmg.o
s/raw_bsd/raw-format/
(yes, just contextual, but prevents git from taking the patch as-is)
> -block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-cache.o
> +block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-cache.o qcow2-bitmap.o
> block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
> block-obj-y += qed-check.o
> block-obj-y += vhdx.o vhdx-endian.o vhdx-log.o
> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
> new file mode 100644
> index 0000000000..758622dcfc
> --- /dev/null
> +++ b/block/qcow2-bitmap.c
[...]
> +static int update_ext_header_and_dir_in_place(BlockDriverState *bs,
> + Qcow2BitmapList *bm_list)
> +{
> + BDRVQcow2State *s = bs->opaque;
> + int ret;
> +
> + if (!(s->autoclear_features & QCOW2_AUTOCLEAR_BITMAPS) ||
> + bm_list == NULL || QSIMPLEQ_EMPTY(bm_list) ||
> + bitmap_list_count(bm_list) != s->nb_bitmaps)
> + {
> + return -EINVAL;
> + }
> +
> + s->autoclear_features &= ~(uint64_t)QCOW2_AUTOCLEAR_BITMAPS;
> + ret = update_header_sync(bs);
> + if (ret < 0) {
> + return ret;
> + }
> +
> + /* autoclear bit is not set, so we can safely update bitmap directory */
> +
> + ret = bitmap_list_store(bs, bm_list, &s->bitmap_directory_offset,
> + &s->bitmap_directory_size, true);
> + if (ret < 0) {
> + /* autoclear bit is cleared, so all leaked clusters would be removed on
> + * qemu-img check */
OK, so then all the bitmaps are gone... Well, if you say that's OK, I'll
trust you. O:-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
> + return ret;
> + }
> +
> + s->autoclear_features |= QCOW2_AUTOCLEAR_BITMAPS;
> + return update_header_sync(bs);
> +}
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH 09/24] qcow2: add .bdrv_load_autoloading_dirty_bitmaps
2017-01-23 12:10 ` [Qemu-devel] [PATCH 09/24] qcow2: add .bdrv_load_autoloading_dirty_bitmaps Vladimir Sementsov-Ogievskiy
2017-01-28 21:45 ` Max Reitz
@ 2017-01-28 22:20 ` Max Reitz
2017-01-31 15:11 ` Vladimir Sementsov-Ogievskiy
1 sibling, 1 reply; 55+ messages in thread
From: Max Reitz @ 2017-01-28 22:20 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block, qemu-devel
Cc: kwolf, armbru, eblake, jsnow, famz, den, stefanha, pbonzini
[-- Attachment #1: Type: text/plain, Size: 1522 bytes --]
On 23.01.2017 13:10, Vladimir Sementsov-Ogievskiy wrote:
> Auto loading bitmaps are bitmaps in Qcow2, with the AUTO flag set. They
> are loaded when the image is opened and become BdrvDirtyBitmaps for the
> corresponding drive.
>
> Extra data in bitmaps is not supported for now.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> block/Makefile.objs | 2 +-
> block/qcow2-bitmap.c | 712 +++++++++++++++++++++++++++++++++++++++++++++++++++
> block/qcow2.c | 2 +
> block/qcow2.h | 3 +
> 4 files changed, 718 insertions(+), 1 deletion(-)
> create mode 100644 block/qcow2-bitmap.c
[...]
> new file mode 100644
> index 0000000000..758622dcfc
> --- /dev/null
> +++ b/block/qcow2-bitmap.c
[...]
> +
> +/*
> + * Bitmap List end
> + */
> +
> +static int update_ext_header_and_dir_in_place(BlockDriverState *bs,
> + Qcow2BitmapList *bm_list)
> +{
> + BDRVQcow2State *s = bs->opaque;
> + int ret;
> +
> + if (!(s->autoclear_features & QCOW2_AUTOCLEAR_BITMAPS) ||
> + bm_list == NULL || QSIMPLEQ_EMPTY(bm_list) ||
> + bitmap_list_count(bm_list) != s->nb_bitmaps)
> + {
> + return -EINVAL;
> + }
> +
> + s->autoclear_features &= ~(uint64_t)QCOW2_AUTOCLEAR_BITMAPS;
> + ret = update_header_sync(bs);
This function is defined only in patch 13, until then the code won't
compile; the definition should be moved into this patch.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH 13/24] qcow2: add .bdrv_store_persistent_dirty_bitmaps()
2017-01-23 12:10 ` [Qemu-devel] [PATCH 13/24] qcow2: add .bdrv_store_persistent_dirty_bitmaps() Vladimir Sementsov-Ogievskiy
@ 2017-01-28 22:30 ` Max Reitz
0 siblings, 0 replies; 55+ messages in thread
From: Max Reitz @ 2017-01-28 22:30 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block, qemu-devel
Cc: kwolf, armbru, eblake, jsnow, famz, den, stefanha, pbonzini
[-- Attachment #1: Type: text/plain, Size: 2033 bytes --]
On 23.01.2017 13:10, Vladimir Sementsov-Ogievskiy wrote:
> Realize block bitmap storing interface, to allow qcow2 images store
> persistent bitmaps.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> block/qcow2-bitmap.c | 509 +++++++++++++++++++++++++++++++++++++++++++++++++--
> block/qcow2.c | 1 +
> block/qcow2.h | 1 +
> 3 files changed, 496 insertions(+), 15 deletions(-)
>
> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
> index 3e8fd030d9..a2b857ea72 100644
> --- a/block/qcow2-bitmap.c
> +++ b/block/qcow2-bitmap.c
> @@ -28,6 +28,7 @@
> #include "qemu/osdep.h"
> #include "qapi/error.h"
> #include "exec/log.h"
> +#include "qemu/cutils.h"
>
> #include "block/block_int.h"
> #include "block/qcow2.h"
> @@ -43,6 +44,10 @@
> #define BME_MIN_GRANULARITY_BITS 9
> #define BME_MAX_NAME_SIZE 1023
>
> +#if BME_MAX_TABLE_SIZE * 8ULL > INT_MAX
> +#error In the code bitmap table physical size assumed to fit into int
> +#endif
> +
> /* Bitmap directory entry flags */
> #define BME_RESERVED_FLAGS 0xfffffffcU
> #define BME_FLAG_IN_USE 1
> @@ -67,13 +72,21 @@ typedef struct QEMU_PACKED Qcow2BitmapDirEntry {
> /* name follows */
> } Qcow2BitmapDirEntry;
>
> +typedef struct Qcow2BitmapTable {
> + uint64_t offset;
> + uint32_t size; /* number of 64bit entries */
> + QSIMPLEQ_ENTRY(Qcow2BitmapTable) entry;
> +} Qcow2BitmapTable;
> +typedef QSIMPLEQ_HEAD(Qcow2BitmapTableList, Qcow2BitmapTable) Qcow2BitmapTableList;
> +
> typedef struct Qcow2Bitmap {
> - uint64_t table_offset;
> - uint32_t table_size;
> + Qcow2BitmapTable table;
This doesn't hurt my reviewing too much, but it wouldn't make sense to
new reviewers. This should have been structured like this from the
beginning (i.e. patch 8).
Anyway, I don't mind and as long as nobody other starts reviewing, it's
fine. In the end, it'll be the same code anyway.
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH 15/24] qcow2: add .bdrv_can_store_new_dirty_bitmap
2017-01-23 12:10 ` [Qemu-devel] [PATCH 15/24] qcow2: add .bdrv_can_store_new_dirty_bitmap Vladimir Sementsov-Ogievskiy
@ 2017-01-28 22:33 ` Max Reitz
0 siblings, 0 replies; 55+ messages in thread
From: Max Reitz @ 2017-01-28 22:33 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block, qemu-devel
Cc: kwolf, armbru, eblake, jsnow, famz, den, stefanha, pbonzini
[-- Attachment #1: Type: text/plain, Size: 422 bytes --]
On 23.01.2017 13:10, Vladimir Sementsov-Ogievskiy wrote:
> Realize .bdrv_can_store_new_dirty_bitmap interface.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> block/qcow2-bitmap.c | 40 ++++++++++++++++++++++++++++++++++++++++
> block/qcow2.c | 1 +
> block/qcow2.h | 4 ++++
> 3 files changed, 45 insertions(+)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH 18/24] qmp: add x-debug-block-dirty-bitmap-sha256
2017-01-23 12:10 ` [Qemu-devel] [PATCH 18/24] qmp: add x-debug-block-dirty-bitmap-sha256 Vladimir Sementsov-Ogievskiy
@ 2017-01-29 16:20 ` Max Reitz
0 siblings, 0 replies; 55+ messages in thread
From: Max Reitz @ 2017-01-29 16:20 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block, qemu-devel
Cc: kwolf, armbru, eblake, jsnow, famz, den, stefanha, pbonzini
[-- Attachment #1: Type: text/plain, Size: 1632 bytes --]
On 23.01.2017 13:10, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> block/dirty-bitmap.c | 5 +++++
> blockdev.c | 29 +++++++++++++++++++++++++++++
> include/block/dirty-bitmap.h | 2 ++
> include/qemu/hbitmap.h | 8 ++++++++
> qapi/block-core.json | 27 +++++++++++++++++++++++++++
> tests/Makefile.include | 2 +-
> util/hbitmap.c | 11 +++++++++++
> 7 files changed, 83 insertions(+), 1 deletion(-)
[...]
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 2c0aa1f44d..956b69567d 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1289,6 +1289,33 @@
> 'data': 'BlockDirtyBitmap' }
>
> ##
> +# @BlockDirtyBitmapSha256:
> +#
> +# SHA256 hash of dirty bitmap data
> +#
> +# @sha256: ASCII representation of SHA256 bitmap hash
> +#
> +# Since: 2.9
> +##
> + { 'struct': 'BlockDirtyBitmapSha256',
> + 'data': {'sha256': 'str'} }
> +
> +##
> +# @x-debug-block-dirty-bitmap-sha256:
> +#
> +# Get bitmap SHA256
> +#
> +# Returns: BlockDirtyBitmapSha256 on success
> +# If @node is not a valid block device, DeviceNotFound
> +# If @name is not found or if hashing is failed, GenericError with an
s/hashing is failed/hashing has failed/
With that fixed:
Reviewed-by: Max Reitz <mreitz@redhat.com>
> +# explanation
> +#
> +# Since: 2.9
> +##
> + { 'command': 'x-debug-block-dirty-bitmap-sha256',
> + 'data': 'BlockDirtyBitmap', 'returns': 'BlockDirtyBitmapSha256' }
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH 21/24] qcow2-bitmap: refcounts
2017-01-23 12:10 ` [Qemu-devel] [PATCH 21/24] qcow2-bitmap: refcounts Vladimir Sementsov-Ogievskiy
@ 2017-01-29 16:42 ` Max Reitz
0 siblings, 0 replies; 55+ messages in thread
From: Max Reitz @ 2017-01-29 16:42 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block, qemu-devel
Cc: kwolf, armbru, eblake, jsnow, famz, den, stefanha, pbonzini
[-- Attachment #1: Type: text/plain, Size: 3869 bytes --]
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>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH 22/24] block/dirty-bitmap: deep release dirty bitmaps
2017-01-23 12:10 ` [Qemu-devel] [PATCH 22/24] block/dirty-bitmap: deep release dirty bitmaps Vladimir Sementsov-Ogievskiy
@ 2017-01-29 16:54 ` Max Reitz
2017-01-31 16:03 ` Vladimir Sementsov-Ogievskiy
0 siblings, 1 reply; 55+ messages in thread
From: Max Reitz @ 2017-01-29 16:54 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block, qemu-devel
Cc: kwolf, armbru, eblake, jsnow, famz, den, stefanha, pbonzini
[-- Attachment #1: Type: text/plain, Size: 4537 bytes --]
On 23.01.2017 13:10, Vladimir Sementsov-Ogievskiy wrote:
> Remove persistent bitmap from its storage on bdrv_release_dirty_bitmap.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> block/dirty-bitmap.c | 21 ++++++++++++++++++---
> include/block/block_int.h | 3 +++
> 2 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index 775181c9ab..0977df6309 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -297,7 +297,8 @@ void bdrv_dirty_bitmap_truncate(BlockDriverState *bs)
>
> static void bdrv_do_release_matching_dirty_bitmap(BlockDriverState *bs,
> BdrvDirtyBitmap *bitmap,
> - bool only_named)
> + bool only_named,
> + bool deep)
I'd rather call it "remove_persistent" or something, which is bad
grammar, but it's better at getting the point across.
> {
> BdrvDirtyBitmap *bm, *next;
> QLIST_FOREACH_SAFE(bm, &bs->dirty_bitmaps, list, next) {
> @@ -305,6 +306,19 @@ static void bdrv_do_release_matching_dirty_bitmap(BlockDriverState *bs,
> assert(!bm->active_iterators);
> assert(!bdrv_dirty_bitmap_frozen(bm));
> assert(!bm->meta);
> +
> + if (deep && bm->persistent && bs->drv &&
> + bs->drv->bdrv_remove_persistent_dirty_bitmap)
> + {
> + Error *local_err = NULL;
> + bs->drv->bdrv_remove_persistent_dirty_bitmap(bs, bm->name,
> + &local_err);
> + if (local_err != NULL) {
> + error_report_err(local_err);
This looks like maybe it's the wrong thing to do... I do agree that
sometimes it may not be fatal, but sometimes it probably is.
> + }
> + }
> +
> +
> QLIST_REMOVE(bm, list);
> hbitmap_free(bm->bitmap);
> g_free(bm->name);
> @@ -322,16 +336,17 @@ static void bdrv_do_release_matching_dirty_bitmap(BlockDriverState *bs,
>
> void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
> {
> - bdrv_do_release_matching_dirty_bitmap(bs, bitmap, false);
> + bdrv_do_release_matching_dirty_bitmap(bs, bitmap, false, true);
The @deep parameter (or however you decide to call it) should be
available to this function's caller, too. I don't believe qcow2's
load_bitmap() wants to delete the persistent bitmap in the error path;
and block-dirty-bitmap-remove should probably allow the user to decide
what to do.
Which brings me to the error thing above: If the user (or management
tool) decides that block-dirty-bitmap-remove should remove the
persistent bitmap, I believe that the operation should be aborted if the
persistent bitmap cannot be removed. It should not just go on and
release the in-memory bitmap but abort altogether. If the user just
wants to release that in-memory bitmap then, they can still go on an
call block-dirty-bitmap-remove with deep=false.
> }
>
> /**
> * Release all named dirty bitmaps attached to a BDS (for use in bdrv_close()).
> * There must not be any frozen bitmaps attached.
> + * This function do not remove persistent bitmaps from the storage.
*does
Max
> */
> void bdrv_release_named_dirty_bitmaps(BlockDriverState *bs)
> {
> - bdrv_do_release_matching_dirty_bitmap(bs, NULL, true);
> + bdrv_do_release_matching_dirty_bitmap(bs, NULL, true, false);
> }
>
> void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap)
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index b38b3aa198..3e080d6d68 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -326,6 +326,9 @@ struct BlockDriver {
> Error **errp);
> bool (*bdrv_can_store_new_dirty_bitmap)(BlockDriverState *bs, const char *name,
> uint32_t granularity, Error **errp);
> + void (*bdrv_remove_persistent_dirty_bitmap)(BlockDriverState *bs,
> + const char *name,
> + Error **errp);
>
> QLIST_ENTRY(BlockDriver) list;
> };
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH 07/24] qcow2: add bitmaps extension
2017-01-28 21:29 ` Max Reitz
@ 2017-01-30 7:21 ` Vladimir Sementsov-Ogievskiy
2017-01-31 22:53 ` Max Reitz
0 siblings, 1 reply; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-01-30 7:21 UTC (permalink / raw)
To: Max Reitz, qemu-block, qemu-devel
Cc: kwolf, armbru, eblake, jsnow, famz, den, stefanha, pbonzini
29.01.2017 00:29, Max Reitz wrote:
> On 23.01.2017 13:10, Vladimir Sementsov-Ogievskiy wrote:
>> Add bitmap extension as specified in docs/specs/qcow2.txt.
>> For now, just mirror extension header into Qcow2 state and check
>> constraints.
>>
>> For now, disable image resize if it has bitmaps. It will be fixed later.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>> block/qcow2.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>> block/qcow2.h | 24 ++++++++++++
>> 2 files changed, 141 insertions(+), 2 deletions(-)
>>
>> diff --git a/block/qcow2.c b/block/qcow2.c
>> index 96fb8a8f16..a8835988c7 100644
>> --- a/block/qcow2.c
>> +++ b/block/qcow2.c
> [...]
>
>> @@ -185,6 +265,15 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
>> offset += ((ext.len + 7) & ~7);
>> }
>>
>> + if (need_update && !bdrv_is_root_node(bs) &&
>> + !(bdrv_get_flags(bs) & BDRV_O_INACTIVE))
>> + {
>> + ret = qcow2_update_header(bs);
> Good idea, but qcow2_read_extensions() can be called pretty early in
> qcow2_open(). Therefore, some fields in the BDRVQcow2State are not
> necessarily set already.
>
> For instance, it is always called before s->snapshots_offset and
> s->nb_snapshots are set. Thus, this will effectively always discard all
> snapshots.
>
> Max
>
So, I need to move need_update to qcow2_read_extensions parameters as
'bool *' and handle it later, in the end of qcow2_open. Ok?
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH 07/24] qcow2: add bitmaps extension
2017-01-23 12:10 ` [Qemu-devel] [PATCH 07/24] qcow2: add bitmaps extension Vladimir Sementsov-Ogievskiy
2017-01-28 21:29 ` Max Reitz
@ 2017-01-31 11:53 ` Vladimir Sementsov-Ogievskiy
1 sibling, 0 replies; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-01-31 11:53 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: kwolf, mreitz, armbru, eblake, jsnow, famz, den, stefanha, pbonzini
23.01.2017 15:10, Vladimir Sementsov-Ogievskiy wrote:
> Add bitmap extension as specified in docs/specs/qcow2.txt.
> For now, just mirror extension header into Qcow2 state and check
> constraints.
>
> For now, disable image resize if it has bitmaps. It will be fixed later.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> block/qcow2.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> block/qcow2.h | 24 ++++++++++++
> 2 files changed, 141 insertions(+), 2 deletions(-)
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 96fb8a8f16..a8835988c7 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -63,6 +63,7 @@ typedef struct {
> #define QCOW2_EXT_MAGIC_END 0
> #define QCOW2_EXT_MAGIC_BACKING_FORMAT 0xE2792ACA
> #define QCOW2_EXT_MAGIC_FEATURE_TABLE 0x6803f857
> +#define QCOW2_EXT_MAGIC_BITMAPS 0x23852875
>
> static int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename)
> {
> @@ -92,6 +93,8 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
> QCowExtension ext;
> uint64_t offset;
> int ret;
> + Qcow2BitmapHeaderExt bitmaps_ext;
> + bool need_update;
>
> #ifdef DEBUG_EXT
> printf("qcow2_read_extensions: start=%ld end=%ld\n", start_offset, end_offset);
> @@ -162,6 +165,83 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
> }
> break;
>
> + case QCOW2_EXT_MAGIC_BITMAPS:
> + if (ext.len != sizeof(bitmaps_ext)) {
> + error_setg_errno(errp, -ret, "bitmaps_ext: "
> + "Invalid extension length");
> + return -EINVAL;
> + }
> +
> + if (!(s->autoclear_features & QCOW2_AUTOCLEAR_BITMAPS)) {
> + fprintf(stderr,
> + "WARNING: a program lacking bitmap support modified "
> + "this file, so all bitmaps are now considered "
> + "inconsistent. Some clusters may be leaked, run "
> + "'qemu-img check -r' on the image file to fix.");
> + need_update = true;
> + break;
> + }
> +
> + ret = bdrv_pread(bs->file, offset, &bitmaps_ext, ext.len);
> + if (ret < 0) {
> + error_setg_errno(errp, -ret, "bitmaps_ext: "
> + "Could not read ext header");
> + return ret;
> + }
> +
> + if (bitmaps_ext.reserved32 != 0) {
> + error_setg_errno(errp, -ret, "bitmaps_ext: "
> + "Reserved field is not zero");
> + return -EINVAL;
> + }
> +
> + be32_to_cpus(&bitmaps_ext.nb_bitmaps);
> + be64_to_cpus(&bitmaps_ext.bitmap_directory_size);
> + be64_to_cpus(&bitmaps_ext.bitmap_directory_offset);
> +
> + if (bitmaps_ext.nb_bitmaps > QCOW2_MAX_BITMAPS) {
> + error_setg(errp,
> + "bitmaps_ext: File %s has %" PRIu32 " bitmaps, "
> + "exceeding the QEMU supported maximum of %d",
> + bs->filename, bitmaps_ext.nb_bitmaps,
> + QCOW2_MAX_BITMAPS);
> + return -EINVAL;
> + }
> +
> + if (bitmaps_ext.nb_bitmaps == 0) {
> + error_setg(errp, "found bitmaps extension with zero bitmaps");
> + return -EINVAL;
> + }
> +
> + if (bitmaps_ext.bitmap_directory_offset & (s->cluster_size - 1)) {
> + error_setg(errp, "bitmaps_ext: "
> + "invalid bitmap directory offset");
> + return -EINVAL;
> + }
> +
> + if (bitmaps_ext.bitmap_directory_size >
> + QCOW2_MAX_BITMAP_DIRECTORY_SIZE) {
> + error_setg(errp, "bitmaps_ext: "
> + "bitmap directory size (%" PRIu64 ") exceeds "
> + "the maximum supported size (%d)",
> + bitmaps_ext.bitmap_directory_size,
> + QCOW2_MAX_BITMAP_DIRECTORY_SIZE);
> + return -EINVAL;
> + }
> +
> + s->nb_bitmaps = bitmaps_ext.nb_bitmaps;
> + s->bitmap_directory_offset =
> + bitmaps_ext.bitmap_directory_offset;
> + s->bitmap_directory_size =
> + bitmaps_ext.bitmap_directory_size;
> +
> +#ifdef DEBUG_EXT
> + printf("Qcow2: Got bitmaps extension: "
> + "offset=%" PRIu64 " nb_bitmaps=%" PRIu32 "\n",
> + s->bitmap_directory_offset, s->nb_bitmaps);
> +#endif
> + break;
> +
> default:
> /* unknown magic - save it in case we need to rewrite the header */
> {
> @@ -185,6 +265,15 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
> offset += ((ext.len + 7) & ~7);
> }
>
> + if (need_update && !bdrv_is_root_node(bs) &&
bdrv_is_read_only() I think)
> + !(bdrv_get_flags(bs) & BDRV_O_INACTIVE))
> + {
> + ret = qcow2_update_header(bs);
> + if (ret < 0) {
> + return ret;
> + }
> + }
> +
> return 0;
> }
>
> @@ -1152,8 +1241,9 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
> }
>
> /* Clear unknown autoclear feature bits */
> - if (!bs->read_only && !(flags & BDRV_O_INACTIVE) && s->autoclear_features) {
> - s->autoclear_features = 0;
> + if (!bs->read_only && !(flags & BDRV_O_INACTIVE) &&
> + (s->autoclear_features & ~QCOW2_AUTOCLEAR_MASK)) {
> + s->autoclear_features &= QCOW2_AUTOCLEAR_MASK;
> ret = qcow2_update_header(bs);
> if (ret < 0) {
> error_setg_errno(errp, -ret, "Could not update qcow2 header");
> @@ -1953,6 +2043,24 @@ int qcow2_update_header(BlockDriverState *bs)
> buflen -= ret;
> }
>
> + if (s->nb_bitmaps > 0) {
> + Qcow2BitmapHeaderExt bitmaps_header = {
> + .nb_bitmaps = cpu_to_be32(s->nb_bitmaps),
> + .bitmap_directory_size =
> + cpu_to_be64(s->bitmap_directory_size),
> + .bitmap_directory_offset =
> + cpu_to_be64(s->bitmap_directory_offset)
> + };
> + ret = header_ext_add(buf, QCOW2_EXT_MAGIC_BITMAPS,
> + &bitmaps_header, sizeof(bitmaps_header),
> + buflen);
> + if (ret < 0) {
> + goto fail;
> + }
> + buf += ret;
> + buflen -= ret;
> + }
> +
> /* Keep unknown header extensions */
> QLIST_FOREACH(uext, &s->unknown_header_ext, next) {
> ret = header_ext_add(buf, uext->magic, uext->data, uext->len, buflen);
> @@ -2528,6 +2636,13 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset)
> return -ENOTSUP;
> }
>
> + /* cannot proceed if image has bitmaps */
> + if (s->nb_bitmaps) {
> + /* TODO: resize bitmaps in the image */
> + error_report("Can't resize an image which has bitmaps");
> + return -ENOTSUP;
> + }
> +
> /* shrinking is currently not supported */
> if (offset < bs->total_sectors * 512) {
> error_report("qcow2 doesn't support shrinking images yet");
> diff --git a/block/qcow2.h b/block/qcow2.h
> index 182341483a..861b5011dd 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -52,6 +52,10 @@
> * space for snapshot names and IDs */
> #define QCOW_MAX_SNAPSHOTS_SIZE (1024 * QCOW_MAX_SNAPSHOTS)
>
> +/* Bitmap header extension constraints */
> +#define QCOW2_MAX_BITMAPS 65535
> +#define QCOW2_MAX_BITMAP_DIRECTORY_SIZE (1024 * QCOW2_MAX_BITMAPS)
> +
> /* indicate that the refcount of the referenced cluster is exactly one. */
> #define QCOW_OFLAG_COPIED (1ULL << 63)
> /* indicate that the cluster is compressed (they never have the copied flag) */
> @@ -195,6 +199,15 @@ enum {
> QCOW2_COMPAT_FEAT_MASK = QCOW2_COMPAT_LAZY_REFCOUNTS,
> };
>
> +/* Autoclear feature bits */
> +enum {
> + QCOW2_AUTOCLEAR_BITMAPS_BITNR = 0,
> + QCOW2_AUTOCLEAR_BITMAPS =
> + 1 << QCOW2_AUTOCLEAR_BITMAPS_BITNR,
> +
> + QCOW2_AUTOCLEAR_MASK = QCOW2_AUTOCLEAR_BITMAPS,
> +};
> +
> enum qcow2_discard_type {
> QCOW2_DISCARD_NEVER = 0,
> QCOW2_DISCARD_ALWAYS,
> @@ -222,6 +235,13 @@ typedef uint64_t Qcow2GetRefcountFunc(const void *refcount_array,
> typedef void Qcow2SetRefcountFunc(void *refcount_array,
> uint64_t index, uint64_t value);
>
> +typedef struct Qcow2BitmapHeaderExt {
> + uint32_t nb_bitmaps;
> + uint32_t reserved32;
> + uint64_t bitmap_directory_size;
> + uint64_t bitmap_directory_offset;
> +} QEMU_PACKED Qcow2BitmapHeaderExt;
> +
> typedef struct BDRVQcow2State {
> int cluster_bits;
> int cluster_size;
> @@ -263,6 +283,10 @@ typedef struct BDRVQcow2State {
> unsigned int nb_snapshots;
> QCowSnapshot *snapshots;
>
> + uint32_t nb_bitmaps;
> + uint64_t bitmap_directory_size;
> + uint64_t bitmap_directory_offset;
> +
> int flags;
> int qcow_version;
> bool use_lazy_refcounts;
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH 09/24] qcow2: add .bdrv_load_autoloading_dirty_bitmaps
2017-01-28 22:20 ` Max Reitz
@ 2017-01-31 15:11 ` Vladimir Sementsov-Ogievskiy
2017-01-31 22:57 ` Max Reitz
0 siblings, 1 reply; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-01-31 15:11 UTC (permalink / raw)
To: Max Reitz, qemu-block, qemu-devel
Cc: kwolf, armbru, eblake, jsnow, famz, den, stefanha, pbonzini
29.01.2017 01:20, Max Reitz wrote:
> On 23.01.2017 13:10, Vladimir Sementsov-Ogievskiy wrote:
>> Auto loading bitmaps are bitmaps in Qcow2, with the AUTO flag set. They
>> are loaded when the image is opened and become BdrvDirtyBitmaps for the
>> corresponding drive.
>>
>> Extra data in bitmaps is not supported for now.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>> block/Makefile.objs | 2 +-
>> block/qcow2-bitmap.c | 712 +++++++++++++++++++++++++++++++++++++++++++++++++++
>> block/qcow2.c | 2 +
>> block/qcow2.h | 3 +
>> 4 files changed, 718 insertions(+), 1 deletion(-)
>> create mode 100644 block/qcow2-bitmap.c
> [...]
>
>> new file mode 100644
>> index 0000000000..758622dcfc
>> --- /dev/null
>> +++ b/block/qcow2-bitmap.c
> [...]
>
>> +
>> +/*
>> + * Bitmap List end
>> + */
>> +
>> +static int update_ext_header_and_dir_in_place(BlockDriverState *bs,
>> + Qcow2BitmapList *bm_list)
>> +{
>> + BDRVQcow2State *s = bs->opaque;
>> + int ret;
>> +
>> + if (!(s->autoclear_features & QCOW2_AUTOCLEAR_BITMAPS) ||
>> + bm_list == NULL || QSIMPLEQ_EMPTY(bm_list) ||
>> + bitmap_list_count(bm_list) != s->nb_bitmaps)
>> + {
>> + return -EINVAL;
>> + }
>> +
>> + s->autoclear_features &= ~(uint64_t)QCOW2_AUTOCLEAR_BITMAPS;
>> + ret = update_header_sync(bs);
> This function is defined only in patch 13, until then the code won't
> compile; the definition should be moved into this patch.
>
> Max
>
Stupid omission, sorry =(. I'll move it and add your r-b, ok?
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH 22/24] block/dirty-bitmap: deep release dirty bitmaps
2017-01-29 16:54 ` Max Reitz
@ 2017-01-31 16:03 ` Vladimir Sementsov-Ogievskiy
2017-01-31 23:00 ` Max Reitz
0 siblings, 1 reply; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-01-31 16:03 UTC (permalink / raw)
To: Max Reitz, qemu-block, qemu-devel
Cc: kwolf, armbru, eblake, jsnow, famz, den, stefanha, pbonzini
29.01.2017 19:54, Max Reitz wrote:
> On 23.01.2017 13:10, Vladimir Sementsov-Ogievskiy wrote:
>> Remove persistent bitmap from its storage on bdrv_release_dirty_bitmap.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>> block/dirty-bitmap.c | 21 ++++++++++++++++++---
>> include/block/block_int.h | 3 +++
>> 2 files changed, 21 insertions(+), 3 deletions(-)
>>
>> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
>> index 775181c9ab..0977df6309 100644
>> --- a/block/dirty-bitmap.c
>> +++ b/block/dirty-bitmap.c
>> @@ -297,7 +297,8 @@ void bdrv_dirty_bitmap_truncate(BlockDriverState *bs)
>>
>> static void bdrv_do_release_matching_dirty_bitmap(BlockDriverState *bs,
>> BdrvDirtyBitmap *bitmap,
>> - bool only_named)
>> + bool only_named,
>> + bool deep)
> I'd rather call it "remove_persistent" or something, which is bad
> grammar, but it's better at getting the point across.
>
>> {
>> BdrvDirtyBitmap *bm, *next;
>> QLIST_FOREACH_SAFE(bm, &bs->dirty_bitmaps, list, next) {
>> @@ -305,6 +306,19 @@ static void bdrv_do_release_matching_dirty_bitmap(BlockDriverState *bs,
>> assert(!bm->active_iterators);
>> assert(!bdrv_dirty_bitmap_frozen(bm));
>> assert(!bm->meta);
>> +
>> + if (deep && bm->persistent && bs->drv &&
>> + bs->drv->bdrv_remove_persistent_dirty_bitmap)
>> + {
>> + Error *local_err = NULL;
>> + bs->drv->bdrv_remove_persistent_dirty_bitmap(bs, bm->name,
>> + &local_err);
>> + if (local_err != NULL) {
>> + error_report_err(local_err);
> This looks like maybe it's the wrong thing to do... I do agree that
> sometimes it may not be fatal, but sometimes it probably is.
>
>> + }
>> + }
>> +
>> +
>> QLIST_REMOVE(bm, list);
>> hbitmap_free(bm->bitmap);
>> g_free(bm->name);
>> @@ -322,16 +336,17 @@ static void bdrv_do_release_matching_dirty_bitmap(BlockDriverState *bs,
>>
>> void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
>> {
>> - bdrv_do_release_matching_dirty_bitmap(bs, bitmap, false);
>> + bdrv_do_release_matching_dirty_bitmap(bs, bitmap, false, true);
> The @deep parameter (or however you decide to call it) should be
> available to this function's caller, too. I don't believe qcow2's
> load_bitmap() wants to delete the persistent bitmap in the error path;
> and block-dirty-bitmap-remove should probably allow the user to decide
> what to do.
>
> Which brings me to the error thing above: If the user (or management
> tool) decides that block-dirty-bitmap-remove should remove the
> persistent bitmap, I believe that the operation should be aborted if the
> persistent bitmap cannot be removed. It should not just go on and
> release the in-memory bitmap but abort altogether. If the user just
> wants to release that in-memory bitmap then, they can still go on an
> call block-dirty-bitmap-remove with deep=false.
I've started to rewrite it in that way and it seems like deep almost
always will be false. What about just move call to
bs->drv->bdrv_remove_persistent_dirty_bitmap() directly to
qmp_block_dirty_bitmap_remove and add parameter
'remove_persistent' (or 'remove_from_storage', to make it maximum
descriptive) only to this qmp command?
Or even without that parameter, as leaving inconsistent version of
bitmap in the storage doesn't seem useful.
>
>> }
>>
>> /**
>> * Release all named dirty bitmaps attached to a BDS (for use in bdrv_close()).
>> * There must not be any frozen bitmaps attached.
>> + * This function do not remove persistent bitmaps from the storage.
> *does
>
> Max
>
>> */
>> void bdrv_release_named_dirty_bitmaps(BlockDriverState *bs)
>> {
>> - bdrv_do_release_matching_dirty_bitmap(bs, NULL, true);
>> + bdrv_do_release_matching_dirty_bitmap(bs, NULL, true, false);
>> }
>>
>> void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap)
>> diff --git a/include/block/block_int.h b/include/block/block_int.h
>> index b38b3aa198..3e080d6d68 100644
>> --- a/include/block/block_int.h
>> +++ b/include/block/block_int.h
>> @@ -326,6 +326,9 @@ struct BlockDriver {
>> Error **errp);
>> bool (*bdrv_can_store_new_dirty_bitmap)(BlockDriverState *bs, const char *name,
>> uint32_t granularity, Error **errp);
>> + void (*bdrv_remove_persistent_dirty_bitmap)(BlockDriverState *bs,
>> + const char *name,
>> + Error **errp);
>>
>> QLIST_ENTRY(BlockDriver) list;
>> };
>>
>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH 07/24] qcow2: add bitmaps extension
2017-01-30 7:21 ` Vladimir Sementsov-Ogievskiy
@ 2017-01-31 22:53 ` Max Reitz
0 siblings, 0 replies; 55+ messages in thread
From: Max Reitz @ 2017-01-31 22:53 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block, qemu-devel
Cc: kwolf, armbru, eblake, jsnow, famz, den, stefanha, pbonzini
[-- Attachment #1: Type: text/plain, Size: 1701 bytes --]
On 30.01.2017 08:21, Vladimir Sementsov-Ogievskiy wrote:
> 29.01.2017 00:29, Max Reitz wrote:
>> On 23.01.2017 13:10, Vladimir Sementsov-Ogievskiy wrote:
>>> Add bitmap extension as specified in docs/specs/qcow2.txt.
>>> For now, just mirror extension header into Qcow2 state and check
>>> constraints.
>>>
>>> For now, disable image resize if it has bitmaps. It will be fixed later.
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>> ---
>>> block/qcow2.c | 119
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>> block/qcow2.h | 24 ++++++++++++
>>> 2 files changed, 141 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/block/qcow2.c b/block/qcow2.c
>>> index 96fb8a8f16..a8835988c7 100644
>>> --- a/block/qcow2.c
>>> +++ b/block/qcow2.c
>> [...]
>>
>>> @@ -185,6 +265,15 @@ static int
>>> qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
>>> offset += ((ext.len + 7) & ~7);
>>> }
>>> + if (need_update && !bdrv_is_root_node(bs) &&
>>> + !(bdrv_get_flags(bs) & BDRV_O_INACTIVE))
>>> + {
>>> + ret = qcow2_update_header(bs);
>> Good idea, but qcow2_read_extensions() can be called pretty early in
>> qcow2_open(). Therefore, some fields in the BDRVQcow2State are not
>> necessarily set already.
>>
>> For instance, it is always called before s->snapshots_offset and
>> s->nb_snapshots are set. Thus, this will effectively always discard all
>> snapshots.
>>
>> Max
>>
>
> So, I need to move need_update to qcow2_read_extensions parameters as
> 'bool *' and handle it later, in the end of qcow2_open. Ok?
Yes, that sounds reasonable.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH 09/24] qcow2: add .bdrv_load_autoloading_dirty_bitmaps
2017-01-31 15:11 ` Vladimir Sementsov-Ogievskiy
@ 2017-01-31 22:57 ` Max Reitz
0 siblings, 0 replies; 55+ messages in thread
From: Max Reitz @ 2017-01-31 22:57 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block, qemu-devel
Cc: kwolf, armbru, eblake, jsnow, famz, den, stefanha, pbonzini
[-- Attachment #1: Type: text/plain, Size: 1811 bytes --]
On 31.01.2017 16:11, Vladimir Sementsov-Ogievskiy wrote:
> 29.01.2017 01:20, Max Reitz wrote:
>> On 23.01.2017 13:10, Vladimir Sementsov-Ogievskiy wrote:
>>> Auto loading bitmaps are bitmaps in Qcow2, with the AUTO flag set. They
>>> are loaded when the image is opened and become BdrvDirtyBitmaps for the
>>> corresponding drive.
>>>
>>> Extra data in bitmaps is not supported for now.
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>> ---
>>> block/Makefile.objs | 2 +-
>>> block/qcow2-bitmap.c | 712 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>> block/qcow2.c | 2 +
>>> block/qcow2.h | 3 +
>>> 4 files changed, 718 insertions(+), 1 deletion(-)
>>> create mode 100644 block/qcow2-bitmap.c
>> [...]
>>
>>> new file mode 100644
>>> index 0000000000..758622dcfc
>>> --- /dev/null
>>> +++ b/block/qcow2-bitmap.c
>> [...]
>>
>>> +
>>> +/*
>>> + * Bitmap List end
>>> + */
>>> +
>>> +static int update_ext_header_and_dir_in_place(BlockDriverState *bs,
>>> + Qcow2BitmapList *bm_list)
>>> +{
>>> + BDRVQcow2State *s = bs->opaque;
>>> + int ret;
>>> +
>>> + if (!(s->autoclear_features & QCOW2_AUTOCLEAR_BITMAPS) ||
>>> + bm_list == NULL || QSIMPLEQ_EMPTY(bm_list) ||
>>> + bitmap_list_count(bm_list) != s->nb_bitmaps)
>>> + {
>>> + return -EINVAL;
>>> + }
>>> +
>>> + s->autoclear_features &= ~(uint64_t)QCOW2_AUTOCLEAR_BITMAPS;
>>> + ret = update_header_sync(bs);
>> This function is defined only in patch 13, until then the code won't
>> compile; the definition should be moved into this patch.
>>
>> Max
>>
>
> Stupid omission, sorry =(. I'll move it and add your r-b, ok?
Yep, that's fine.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH 22/24] block/dirty-bitmap: deep release dirty bitmaps
2017-01-31 16:03 ` Vladimir Sementsov-Ogievskiy
@ 2017-01-31 23:00 ` Max Reitz
2017-01-31 23:16 ` Max Reitz
0 siblings, 1 reply; 55+ messages in thread
From: Max Reitz @ 2017-01-31 23:00 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block, qemu-devel
Cc: kwolf, armbru, eblake, jsnow, famz, den, stefanha, pbonzini
[-- Attachment #1: Type: text/plain, Size: 4247 bytes --]
On 31.01.2017 17:03, Vladimir Sementsov-Ogievskiy wrote:
> 29.01.2017 19:54, Max Reitz wrote:
>> On 23.01.2017 13:10, Vladimir Sementsov-Ogievskiy wrote:
>>> Remove persistent bitmap from its storage on bdrv_release_dirty_bitmap.
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>> ---
>>> block/dirty-bitmap.c | 21 ++++++++++++++++++---
>>> include/block/block_int.h | 3 +++
>>> 2 files changed, 21 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
>>> index 775181c9ab..0977df6309 100644
>>> --- a/block/dirty-bitmap.c
>>> +++ b/block/dirty-bitmap.c
>>> @@ -297,7 +297,8 @@ void bdrv_dirty_bitmap_truncate(BlockDriverState
>>> *bs)
>>> static void
>>> bdrv_do_release_matching_dirty_bitmap(BlockDriverState *bs,
>>> BdrvDirtyBitmap
>>> *bitmap,
>>> - bool only_named)
>>> + bool only_named,
>>> + bool deep)
>> I'd rather call it "remove_persistent" or something, which is bad
>> grammar, but it's better at getting the point across.
>>
>>> {
>>> BdrvDirtyBitmap *bm, *next;
>>> QLIST_FOREACH_SAFE(bm, &bs->dirty_bitmaps, list, next) {
>>> @@ -305,6 +306,19 @@ static void
>>> bdrv_do_release_matching_dirty_bitmap(BlockDriverState *bs,
>>> assert(!bm->active_iterators);
>>> assert(!bdrv_dirty_bitmap_frozen(bm));
>>> assert(!bm->meta);
>>> +
>>> + if (deep && bm->persistent && bs->drv &&
>>> + bs->drv->bdrv_remove_persistent_dirty_bitmap)
>>> + {
>>> + Error *local_err = NULL;
>>> + bs->drv->bdrv_remove_persistent_dirty_bitmap(bs,
>>> bm->name,
>>> +
>>> &local_err);
>>> + if (local_err != NULL) {
>>> + error_report_err(local_err);
>> This looks like maybe it's the wrong thing to do... I do agree that
>> sometimes it may not be fatal, but sometimes it probably is.
>>
>>> + }
>>> + }
>>> +
>>> +
>>> QLIST_REMOVE(bm, list);
>>> hbitmap_free(bm->bitmap);
>>> g_free(bm->name);
>>> @@ -322,16 +336,17 @@ static void
>>> bdrv_do_release_matching_dirty_bitmap(BlockDriverState *bs,
>>> void bdrv_release_dirty_bitmap(BlockDriverState *bs,
>>> BdrvDirtyBitmap *bitmap)
>>> {
>>> - bdrv_do_release_matching_dirty_bitmap(bs, bitmap, false);
>>> + bdrv_do_release_matching_dirty_bitmap(bs, bitmap, false, true);
>> The @deep parameter (or however you decide to call it) should be
>> available to this function's caller, too. I don't believe qcow2's
>> load_bitmap() wants to delete the persistent bitmap in the error path;
>> and block-dirty-bitmap-remove should probably allow the user to decide
>> what to do.
>>
>> Which brings me to the error thing above: If the user (or management
>> tool) decides that block-dirty-bitmap-remove should remove the
>> persistent bitmap, I believe that the operation should be aborted if the
>> persistent bitmap cannot be removed. It should not just go on and
>> release the in-memory bitmap but abort altogether. If the user just
>> wants to release that in-memory bitmap then, they can still go on an
>> call block-dirty-bitmap-remove with deep=false.
>
> I've started to rewrite it in that way and it seems like deep almost
> always will be false. What about just move call to
> bs->drv->bdrv_remove_persistent_dirty_bitmap() directly to
> qmp_block_dirty_bitmap_remove and add parameter
> 'remove_persistent' (or 'remove_from_storage', to make it maximum
> descriptive) only to this qmp command?
Or you could add an explicit bdrv_remove_persistent_dirty_bitmap() which
just wraps the bs->drv function.
> Or even without that parameter, as leaving inconsistent version of
> bitmap in the storage doesn't seem useful.
Hm, right, the bitmap will be automatically removed from the image once
it's closed, right?
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH 22/24] block/dirty-bitmap: deep release dirty bitmaps
2017-01-31 23:00 ` Max Reitz
@ 2017-01-31 23:16 ` Max Reitz
0 siblings, 0 replies; 55+ messages in thread
From: Max Reitz @ 2017-01-31 23:16 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block, qemu-devel
Cc: kwolf, armbru, eblake, jsnow, famz, den, stefanha, pbonzini
[-- Attachment #1: Type: text/plain, Size: 1398 bytes --]
On 01.02.2017 00:00, Max Reitz wrote:
> On 31.01.2017 17:03, Vladimir Sementsov-Ogievskiy wrote:
[...]
>> I've started to rewrite it in that way and it seems like deep almost
>> always will be false. What about just move call to
>> bs->drv->bdrv_remove_persistent_dirty_bitmap() directly to
>> qmp_block_dirty_bitmap_remove and add parameter
>> 'remove_persistent' (or 'remove_from_storage', to make it maximum
>> descriptive) only to this qmp command?
>
> Or you could add an explicit bdrv_remove_persistent_dirty_bitmap() which
> just wraps the bs->drv function.
>
>> Or even without that parameter, as leaving inconsistent version of
>> bitmap in the storage doesn't seem useful.
>
> Hm, right, the bitmap will be automatically removed from the image once
> it's closed, right?
[No, it won't.]
But you're right, if a persistent bitmap is completely removed by the
user, it should probably be always removed from the image file, too.
The only use I could think of for dropping a bitmap from memory but
wanting to keep it in the file is for keeping it at a certain state. It
would make more sense to allow the user to disable a bitmap to achieve
this, though.
We don't have a block-dirty-bitmap-disable command yet, though, but I
guess it shouldn't be too hard to add...?
(Anyway, it's something that's out of scope for this series, I think.)
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH 23/24] qcow2: add .bdrv_remove_persistent_dirty_bitmap
2017-01-23 12:10 ` [Qemu-devel] [PATCH 23/24] qcow2: add .bdrv_remove_persistent_dirty_bitmap Vladimir Sementsov-Ogievskiy
@ 2017-01-31 23:20 ` Max Reitz
2017-02-01 13:57 ` Vladimir Sementsov-Ogievskiy
2017-02-02 10:41 ` Vladimir Sementsov-Ogievskiy
0 siblings, 2 replies; 55+ messages in thread
From: Max Reitz @ 2017-01-31 23:20 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block, qemu-devel
Cc: kwolf, armbru, eblake, jsnow, famz, den, stefanha, pbonzini
[-- Attachment #1: Type: text/plain, Size: 3324 bytes --]
On 23.01.2017 13:10, Vladimir Sementsov-Ogievskiy wrote:
> Realize .bdrv_remove_persistent_dirty_bitmap interface.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> block/qcow2-bitmap.c | 40 ++++++++++++++++++++++++++++++++++++++++
> block/qcow2.c | 1 +
> block/qcow2.h | 3 +++
> 3 files changed, 44 insertions(+)
>
> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
> index 2687a3acd5..be026fc80e 100644
> --- a/block/qcow2-bitmap.c
> +++ b/block/qcow2-bitmap.c
> @@ -1064,6 +1064,46 @@ static Qcow2Bitmap *find_bitmap_by_name(Qcow2BitmapList *bm_list,
> return NULL;
> }
>
> +void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs,
> + const char *name,
> + Error **errp)
> +{
> + int ret;
> + BDRVQcow2State *s = bs->opaque;
> + Qcow2Bitmap *bm;
> + Qcow2BitmapList *bm_list;
> +
> + if (s->nb_bitmaps == 0) {
> + /* No bitmaps - nothing to do */
Shouldn't it be an error? I.e. "bitmap not found"?
> + return;
> + }
> +
> + bm_list = bitmap_list_load(bs, s->bitmap_directory_offset,
> + s->bitmap_directory_size, errp);
> + if (bm_list == NULL) {
> + return;
> + }
> +
> + bm = find_bitmap_by_name(bm_list, name);
> + if (bm == NULL) {
> + goto fail;
What about setting errp? Or do you not consider this an error?
I think it should be an error.
> + }
> +
> + QSIMPLEQ_REMOVE(bm_list, bm, Qcow2Bitmap, entry);
bm->name is leaked here.
Max
> +
> + ret = update_ext_header_and_dir(bs, bm_list);
> + if (ret < 0) {
> + error_setg_errno(errp, -ret, "Failed to update bitmap extension");
> + goto fail;
> + }
> +
> + free_bitmap_clusters(bs, &bm->table);
> +
> +fail:
> + bitmap_free(bm);
> + bitmap_list_free(bm_list);
> +}
> +
> void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp)
> {
> BdrvDirtyBitmap *bitmap;
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 76a6a2ba28..f6bac38e51 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -3544,6 +3544,7 @@ BlockDriver bdrv_qcow2 = {
> .bdrv_load_autoloading_dirty_bitmaps = qcow2_load_autoloading_dirty_bitmaps,
> .bdrv_store_persistent_dirty_bitmaps = qcow2_store_persistent_dirty_bitmaps,
> .bdrv_can_store_new_dirty_bitmap = qcow2_can_store_new_dirty_bitmap,
> + .bdrv_remove_persistent_dirty_bitmap = qcow2_remove_persistent_dirty_bitmap,
> };
>
> static void bdrv_qcow2_init(void)
> diff --git a/block/qcow2.h b/block/qcow2.h
> index eaad34a527..a009827f60 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -627,5 +627,8 @@ bool qcow2_can_store_new_dirty_bitmap(BlockDriverState *bs,
> int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
> void **refcount_table,
> int64_t *refcount_table_size);
> +void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs,
> + const char *name,
> + Error **errp);
>
> #endif
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH 24/24] qcow2-bitmap: cache bitmap list in BDRVQcow2State
2017-01-23 12:10 ` [Qemu-devel] [PATCH 24/24] qcow2-bitmap: cache bitmap list in BDRVQcow2State Vladimir Sementsov-Ogievskiy
@ 2017-01-31 23:27 ` Max Reitz
2017-02-01 10:14 ` Vladimir Sementsov-Ogievskiy
0 siblings, 1 reply; 55+ messages in thread
From: Max Reitz @ 2017-01-31 23:27 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block, qemu-devel
Cc: kwolf, armbru, eblake, jsnow, famz, den, stefanha, pbonzini
[-- Attachment #1: Type: text/plain, Size: 943 bytes --]
On 23.01.2017 13:10, Vladimir Sementsov-Ogievskiy wrote:
> Do not reload bitmap list every time it is needed.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> block/qcow2-bitmap.c | 125 ++++++++++++++++++++++++++++++++++-----------------
> block/qcow2.c | 2 +
> block/qcow2.h | 4 ++
> 3 files changed, 89 insertions(+), 42 deletions(-)
Hm, difficult. On a glance, this patch seems alright, and it probably
makes sense. Maybe it would make the code actually a bit easier to
understand if it was squashed into the other patches. But OTOH it would
mean that I would have to start review from scratch for those patches...
So in my opinion this patch makes sense and you should keep it updated,
but I think we should keep it separate from the rest of the series. Get
the first 23 patches in first and then we can merge this on top later on.
Would that be OK?
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH 24/24] qcow2-bitmap: cache bitmap list in BDRVQcow2State
2017-01-31 23:27 ` Max Reitz
@ 2017-02-01 10:14 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-02-01 10:14 UTC (permalink / raw)
To: Max Reitz, qemu-block, qemu-devel
Cc: kwolf, armbru, eblake, jsnow, famz, den, stefanha, pbonzini
01.02.2017 02:27, Max Reitz wrote:
> On 23.01.2017 13:10, Vladimir Sementsov-Ogievskiy wrote:
>> Do not reload bitmap list every time it is needed.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>> block/qcow2-bitmap.c | 125 ++++++++++++++++++++++++++++++++++-----------------
>> block/qcow2.c | 2 +
>> block/qcow2.h | 4 ++
>> 3 files changed, 89 insertions(+), 42 deletions(-)
> Hm, difficult. On a glance, this patch seems alright, and it probably
> makes sense. Maybe it would make the code actually a bit easier to
> understand if it was squashed into the other patches. But OTOH it would
> mean that I would have to start review from scratch for those patches...
>
> So in my opinion this patch makes sense and you should keep it updated,
> but I think we should keep it separate from the rest of the series. Get
> the first 23 patches in first and then we can merge this on top later on.
>
> Would that be OK?
Yes, that's OK.
>
> Max
>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH 23/24] qcow2: add .bdrv_remove_persistent_dirty_bitmap
2017-01-31 23:20 ` Max Reitz
@ 2017-02-01 13:57 ` Vladimir Sementsov-Ogievskiy
2017-02-01 14:23 ` Vladimir Sementsov-Ogievskiy
2017-02-01 20:00 ` Max Reitz
2017-02-02 10:41 ` Vladimir Sementsov-Ogievskiy
1 sibling, 2 replies; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-02-01 13:57 UTC (permalink / raw)
To: Max Reitz, qemu-block, qemu-devel
Cc: kwolf, armbru, eblake, jsnow, famz, den, stefanha, pbonzini
01.02.2017 02:20, Max Reitz wrote:
> On 23.01.2017 13:10, Vladimir Sementsov-Ogievskiy wrote:
>> Realize .bdrv_remove_persistent_dirty_bitmap interface.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>> block/qcow2-bitmap.c | 40 ++++++++++++++++++++++++++++++++++++++++
>> block/qcow2.c | 1 +
>> block/qcow2.h | 3 +++
>> 3 files changed, 44 insertions(+)
>>
>> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
>> index 2687a3acd5..be026fc80e 100644
>> --- a/block/qcow2-bitmap.c
>> +++ b/block/qcow2-bitmap.c
>> @@ -1064,6 +1064,46 @@ static Qcow2Bitmap *find_bitmap_by_name(Qcow2BitmapList *bm_list,
>> return NULL;
>> }
>>
>> +void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs,
>> + const char *name,
>> + Error **errp)
>> +{
>> + int ret;
>> + BDRVQcow2State *s = bs->opaque;
>> + Qcow2Bitmap *bm;
>> + Qcow2BitmapList *bm_list;
>> +
>> + if (s->nb_bitmaps == 0) {
>> + /* No bitmaps - nothing to do */
> Shouldn't it be an error? I.e. "bitmap not found"?
>
>> + return;
>> + }
>> +
>> + bm_list = bitmap_list_load(bs, s->bitmap_directory_offset,
>> + s->bitmap_directory_size, errp);
>> + if (bm_list == NULL) {
>> + return;
>> + }
>> +
>> + bm = find_bitmap_by_name(bm_list, name);
>> + if (bm == NULL) {
>> + goto fail;
> What about setting errp? Or do you not consider this an error?
>
> I think it should be an error.
Hmm. The following scenery should not be an error:
1. qmp create dirty bitmap with persistent = true (bitmap not exists and
not created in the storage, it will be saved only on storage close)
2. qmp remove dirty bitmap. persistent = true, so we will try to remove
it from storage..
So, I see following variants:
1. as is, just add comment "remove if exists"
2. add return value to this function, return ENOENT if bitmap not found
and ignore this ENOENT in qmp_remove_bitmap
3. add additional bool field 'exist_in_storage' to BdrvDirtyBitmap.
>
>> + }
>> +
>> + QSIMPLEQ_REMOVE(bm_list, bm, Qcow2Bitmap, entry);
> bm->name is leaked here.
>
> Max
>
>> +
>> + ret = update_ext_header_and_dir(bs, bm_list);
>> + if (ret < 0) {
>> + error_setg_errno(errp, -ret, "Failed to update bitmap extension");
>> + goto fail;
>> + }
>> +
>> + free_bitmap_clusters(bs, &bm->table);
>> +
>> +fail:
>> + bitmap_free(bm);
>> + bitmap_list_free(bm_list);
>> +}
>> +
>> void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp)
>> {
>> BdrvDirtyBitmap *bitmap;
>> diff --git a/block/qcow2.c b/block/qcow2.c
>> index 76a6a2ba28..f6bac38e51 100644
>> --- a/block/qcow2.c
>> +++ b/block/qcow2.c
>> @@ -3544,6 +3544,7 @@ BlockDriver bdrv_qcow2 = {
>> .bdrv_load_autoloading_dirty_bitmaps = qcow2_load_autoloading_dirty_bitmaps,
>> .bdrv_store_persistent_dirty_bitmaps = qcow2_store_persistent_dirty_bitmaps,
>> .bdrv_can_store_new_dirty_bitmap = qcow2_can_store_new_dirty_bitmap,
>> + .bdrv_remove_persistent_dirty_bitmap = qcow2_remove_persistent_dirty_bitmap,
>> };
>>
>> static void bdrv_qcow2_init(void)
>> diff --git a/block/qcow2.h b/block/qcow2.h
>> index eaad34a527..a009827f60 100644
>> --- a/block/qcow2.h
>> +++ b/block/qcow2.h
>> @@ -627,5 +627,8 @@ bool qcow2_can_store_new_dirty_bitmap(BlockDriverState *bs,
>> int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
>> void **refcount_table,
>> int64_t *refcount_table_size);
>> +void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs,
>> + const char *name,
>> + Error **errp);
>>
>> #endif
>>
>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH 23/24] qcow2: add .bdrv_remove_persistent_dirty_bitmap
2017-02-01 13:57 ` Vladimir Sementsov-Ogievskiy
@ 2017-02-01 14:23 ` Vladimir Sementsov-Ogievskiy
2017-02-01 20:00 ` Max Reitz
2017-02-01 20:00 ` Max Reitz
1 sibling, 1 reply; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-02-01 14:23 UTC (permalink / raw)
To: Max Reitz, qemu-block, qemu-devel
Cc: kwolf, armbru, eblake, jsnow, famz, den, stefanha, pbonzini
01.02.2017 16:57, Vladimir Sementsov-Ogievskiy wrote:
> 01.02.2017 02:20, Max Reitz wrote:
>> On 23.01.2017 13:10, Vladimir Sementsov-Ogievskiy wrote:
>>> Realize .bdrv_remove_persistent_dirty_bitmap interface.
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>> ---
>>> block/qcow2-bitmap.c | 40 ++++++++++++++++++++++++++++++++++++++++
>>> block/qcow2.c | 1 +
>>> block/qcow2.h | 3 +++
>>> 3 files changed, 44 insertions(+)
>>>
>>> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
>>> index 2687a3acd5..be026fc80e 100644
>>> --- a/block/qcow2-bitmap.c
>>> +++ b/block/qcow2-bitmap.c
>>> @@ -1064,6 +1064,46 @@ static Qcow2Bitmap
>>> *find_bitmap_by_name(Qcow2BitmapList *bm_list,
>>> return NULL;
>>> }
>>> +void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs,
>>> + const char *name,
>>> + Error **errp)
>>> +{
>>> + int ret;
>>> + BDRVQcow2State *s = bs->opaque;
>>> + Qcow2Bitmap *bm;
>>> + Qcow2BitmapList *bm_list;
>>> +
>>> + if (s->nb_bitmaps == 0) {
>>> + /* No bitmaps - nothing to do */
>> Shouldn't it be an error? I.e. "bitmap not found"?
>>
>>> + return;
>>> + }
>>> +
>>> + bm_list = bitmap_list_load(bs, s->bitmap_directory_offset,
>>> + s->bitmap_directory_size, errp);
>>> + if (bm_list == NULL) {
>>> + return;
>>> + }
>>> +
>>> + bm = find_bitmap_by_name(bm_list, name);
>>> + if (bm == NULL) {
>>> + goto fail;
>> What about setting errp? Or do you not consider this an error?
>>
>> I think it should be an error.
>
> Hmm. The following scenery should not be an error:
> 1. qmp create dirty bitmap with persistent = true (bitmap not exists
> and not created in the storage, it will be saved only on storage close)
> 2. qmp remove dirty bitmap. persistent = true, so we will try to
> remove it from storage..
>
> So, I see following variants:
> 1. as is, just add comment "remove if exists"
> 2. add return value to this function, return ENOENT if bitmap not
> found and ignore this ENOENT in qmp_remove_bitmap
> 3. add additional bool field 'exist_in_storage' to BdrvDirtyBitmap.
Also, I can call remove_persistent...() from
bdrv_dirty_bitmap_set_persistance( ..., false), and use this function in
qmp block-remove-dirty-bitmap
>
>>
>>> + }
>>> +
>>> + QSIMPLEQ_REMOVE(bm_list, bm, Qcow2Bitmap, entry);
>> bm->name is leaked here.
>>
>> Max
>>
>>> +
>>> + ret = update_ext_header_and_dir(bs, bm_list);
>>> + if (ret < 0) {
>>> + error_setg_errno(errp, -ret, "Failed to update bitmap
>>> extension");
>>> + goto fail;
>>> + }
>>> +
>>> + free_bitmap_clusters(bs, &bm->table);
>>> +
>>> +fail:
>>> + bitmap_free(bm);
>>> + bitmap_list_free(bm_list);
>>> +}
>>> +
>>> void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs,
>>> Error **errp)
>>> {
>>> BdrvDirtyBitmap *bitmap;
>>> diff --git a/block/qcow2.c b/block/qcow2.c
>>> index 76a6a2ba28..f6bac38e51 100644
>>> --- a/block/qcow2.c
>>> +++ b/block/qcow2.c
>>> @@ -3544,6 +3544,7 @@ BlockDriver bdrv_qcow2 = {
>>> .bdrv_load_autoloading_dirty_bitmaps =
>>> qcow2_load_autoloading_dirty_bitmaps,
>>> .bdrv_store_persistent_dirty_bitmaps =
>>> qcow2_store_persistent_dirty_bitmaps,
>>> .bdrv_can_store_new_dirty_bitmap =
>>> qcow2_can_store_new_dirty_bitmap,
>>> + .bdrv_remove_persistent_dirty_bitmap =
>>> qcow2_remove_persistent_dirty_bitmap,
>>> };
>>> static void bdrv_qcow2_init(void)
>>> diff --git a/block/qcow2.h b/block/qcow2.h
>>> index eaad34a527..a009827f60 100644
>>> --- a/block/qcow2.h
>>> +++ b/block/qcow2.h
>>> @@ -627,5 +627,8 @@ bool
>>> qcow2_can_store_new_dirty_bitmap(BlockDriverState *bs,
>>> int qcow2_check_bitmaps_refcounts(BlockDriverState *bs,
>>> BdrvCheckResult *res,
>>> void **refcount_table,
>>> int64_t *refcount_table_size);
>>> +void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs,
>>> + const char *name,
>>> + Error **errp);
>>> #endif
>>>
>>
>
>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH 23/24] qcow2: add .bdrv_remove_persistent_dirty_bitmap
2017-02-01 13:57 ` Vladimir Sementsov-Ogievskiy
2017-02-01 14:23 ` Vladimir Sementsov-Ogievskiy
@ 2017-02-01 20:00 ` Max Reitz
1 sibling, 0 replies; 55+ messages in thread
From: Max Reitz @ 2017-02-01 20:00 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block, qemu-devel
Cc: kwolf, armbru, eblake, jsnow, famz, den, stefanha, pbonzini
[-- Attachment #1: Type: text/plain, Size: 2581 bytes --]
On 01.02.2017 14:57, Vladimir Sementsov-Ogievskiy wrote:
> 01.02.2017 02:20, Max Reitz wrote:
>> On 23.01.2017 13:10, Vladimir Sementsov-Ogievskiy wrote:
>>> Realize .bdrv_remove_persistent_dirty_bitmap interface.
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>> ---
>>> block/qcow2-bitmap.c | 40 ++++++++++++++++++++++++++++++++++++++++
>>> block/qcow2.c | 1 +
>>> block/qcow2.h | 3 +++
>>> 3 files changed, 44 insertions(+)
>>>
>>> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
>>> index 2687a3acd5..be026fc80e 100644
>>> --- a/block/qcow2-bitmap.c
>>> +++ b/block/qcow2-bitmap.c
>>> @@ -1064,6 +1064,46 @@ static Qcow2Bitmap
>>> *find_bitmap_by_name(Qcow2BitmapList *bm_list,
>>> return NULL;
>>> }
>>> +void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs,
>>> + const char *name,
>>> + Error **errp)
>>> +{
>>> + int ret;
>>> + BDRVQcow2State *s = bs->opaque;
>>> + Qcow2Bitmap *bm;
>>> + Qcow2BitmapList *bm_list;
>>> +
>>> + if (s->nb_bitmaps == 0) {
>>> + /* No bitmaps - nothing to do */
>> Shouldn't it be an error? I.e. "bitmap not found"?
>>
>>> + return;
>>> + }
>>> +
>>> + bm_list = bitmap_list_load(bs, s->bitmap_directory_offset,
>>> + s->bitmap_directory_size, errp);
>>> + if (bm_list == NULL) {
>>> + return;
>>> + }
>>> +
>>> + bm = find_bitmap_by_name(bm_list, name);
>>> + if (bm == NULL) {
>>> + goto fail;
>> What about setting errp? Or do you not consider this an error?
>>
>> I think it should be an error.
>
> Hmm. The following scenery should not be an error:
> 1. qmp create dirty bitmap with persistent = true (bitmap not exists and
> not created in the storage, it will be saved only on storage close)
> 2. qmp remove dirty bitmap. persistent = true, so we will try to remove
> it from storage..
That's a good point.
> So, I see following variants:
> 1. as is, just add comment "remove if exists"
> 2. add return value to this function, return ENOENT if bitmap not found
> and ignore this ENOENT in qmp_remove_bitmap
> 3. add additional bool field 'exist_in_storage' to BdrvDirtyBitmap.
1 sounds the best to me. I don't think there is a reason why a bitmap
may not be found other than the scenario you've described.
It would be good if the comment included that explanation as for why
it's not an error.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH 23/24] qcow2: add .bdrv_remove_persistent_dirty_bitmap
2017-02-01 14:23 ` Vladimir Sementsov-Ogievskiy
@ 2017-02-01 20:00 ` Max Reitz
0 siblings, 0 replies; 55+ messages in thread
From: Max Reitz @ 2017-02-01 20:00 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block, qemu-devel
Cc: kwolf, armbru, eblake, jsnow, famz, den, stefanha, pbonzini
[-- Attachment #1: Type: text/plain, Size: 2642 bytes --]
On 01.02.2017 15:23, Vladimir Sementsov-Ogievskiy wrote:
> 01.02.2017 16:57, Vladimir Sementsov-Ogievskiy wrote:
>> 01.02.2017 02:20, Max Reitz wrote:
>>> On 23.01.2017 13:10, Vladimir Sementsov-Ogievskiy wrote:
>>>> Realize .bdrv_remove_persistent_dirty_bitmap interface.
>>>>
>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>>> ---
>>>> block/qcow2-bitmap.c | 40 ++++++++++++++++++++++++++++++++++++++++
>>>> block/qcow2.c | 1 +
>>>> block/qcow2.h | 3 +++
>>>> 3 files changed, 44 insertions(+)
>>>>
>>>> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
>>>> index 2687a3acd5..be026fc80e 100644
>>>> --- a/block/qcow2-bitmap.c
>>>> +++ b/block/qcow2-bitmap.c
>>>> @@ -1064,6 +1064,46 @@ static Qcow2Bitmap
>>>> *find_bitmap_by_name(Qcow2BitmapList *bm_list,
>>>> return NULL;
>>>> }
>>>> +void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs,
>>>> + const char *name,
>>>> + Error **errp)
>>>> +{
>>>> + int ret;
>>>> + BDRVQcow2State *s = bs->opaque;
>>>> + Qcow2Bitmap *bm;
>>>> + Qcow2BitmapList *bm_list;
>>>> +
>>>> + if (s->nb_bitmaps == 0) {
>>>> + /* No bitmaps - nothing to do */
>>> Shouldn't it be an error? I.e. "bitmap not found"?
>>>
>>>> + return;
>>>> + }
>>>> +
>>>> + bm_list = bitmap_list_load(bs, s->bitmap_directory_offset,
>>>> + s->bitmap_directory_size, errp);
>>>> + if (bm_list == NULL) {
>>>> + return;
>>>> + }
>>>> +
>>>> + bm = find_bitmap_by_name(bm_list, name);
>>>> + if (bm == NULL) {
>>>> + goto fail;
>>> What about setting errp? Or do you not consider this an error?
>>>
>>> I think it should be an error.
>>
>> Hmm. The following scenery should not be an error:
>> 1. qmp create dirty bitmap with persistent = true (bitmap not exists
>> and not created in the storage, it will be saved only on storage close)
>> 2. qmp remove dirty bitmap. persistent = true, so we will try to
>> remove it from storage..
>>
>> So, I see following variants:
>> 1. as is, just add comment "remove if exists"
>> 2. add return value to this function, return ENOENT if bitmap not
>> found and ignore this ENOENT in qmp_remove_bitmap
>> 3. add additional bool field 'exist_in_storage' to BdrvDirtyBitmap.
>
> Also, I can call remove_persistent...() from
> bdrv_dirty_bitmap_set_persistance( ..., false), and use this function in
> qmp block-remove-dirty-bitmap
Right, that would make sense.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH 23/24] qcow2: add .bdrv_remove_persistent_dirty_bitmap
2017-01-31 23:20 ` Max Reitz
2017-02-01 13:57 ` Vladimir Sementsov-Ogievskiy
@ 2017-02-02 10:41 ` Vladimir Sementsov-Ogievskiy
2017-02-03 19:01 ` Max Reitz
1 sibling, 1 reply; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-02-02 10:41 UTC (permalink / raw)
To: Max Reitz, qemu-block, qemu-devel
Cc: kwolf, armbru, eblake, jsnow, famz, den, stefanha, pbonzini
01.02.2017 02:20, Max Reitz wrote:
> On 23.01.2017 13:10, Vladimir Sementsov-Ogievskiy wrote:
>> Realize .bdrv_remove_persistent_dirty_bitmap interface.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>> block/qcow2-bitmap.c | 40 ++++++++++++++++++++++++++++++++++++++++
>> block/qcow2.c | 1 +
>> block/qcow2.h | 3 +++
>> 3 files changed, 44 insertions(+)
>>
>> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
>> index 2687a3acd5..be026fc80e 100644
>> --- a/block/qcow2-bitmap.c
>> +++ b/block/qcow2-bitmap.c
>> @@ -1064,6 +1064,46 @@ static Qcow2Bitmap *find_bitmap_by_name(Qcow2BitmapList *bm_list,
>> return NULL;
>> }
>>
>> +void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs,
>> + const char *name,
>> + Error **errp)
>> +{
>> + int ret;
>> + BDRVQcow2State *s = bs->opaque;
>> + Qcow2Bitmap *bm;
>> + Qcow2BitmapList *bm_list;
>> +
>> + if (s->nb_bitmaps == 0) {
>> + /* No bitmaps - nothing to do */
> Shouldn't it be an error? I.e. "bitmap not found"?
>
>> + return;
>> + }
>> +
>> + bm_list = bitmap_list_load(bs, s->bitmap_directory_offset,
>> + s->bitmap_directory_size, errp);
>> + if (bm_list == NULL) {
>> + return;
>> + }
>> +
>> + bm = find_bitmap_by_name(bm_list, name);
>> + if (bm == NULL) {
>> + goto fail;
> What about setting errp? Or do you not consider this an error?
>
> I think it should be an error.
>
>> + }
>> +
>> + QSIMPLEQ_REMOVE(bm_list, bm, Qcow2Bitmap, entry);
> bm->name is leaked here.
No, bitmap_free(bm) below will free it.
>
> Max
>
>> +
>> + ret = update_ext_header_and_dir(bs, bm_list);
>> + if (ret < 0) {
>> + error_setg_errno(errp, -ret, "Failed to update bitmap extension");
>> + goto fail;
>> + }
>> +
>> + free_bitmap_clusters(bs, &bm->table);
>> +
>> +fail:
>> + bitmap_free(bm);
>> + bitmap_list_free(bm_list);
>> +}
>> +
>> void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp)
>> {
>> BdrvDirtyBitmap *bitmap;
>> diff --git a/block/qcow2.c b/block/qcow2.c
>> index 76a6a2ba28..f6bac38e51 100644
>> --- a/block/qcow2.c
>> +++ b/block/qcow2.c
>> @@ -3544,6 +3544,7 @@ BlockDriver bdrv_qcow2 = {
>> .bdrv_load_autoloading_dirty_bitmaps = qcow2_load_autoloading_dirty_bitmaps,
>> .bdrv_store_persistent_dirty_bitmaps = qcow2_store_persistent_dirty_bitmaps,
>> .bdrv_can_store_new_dirty_bitmap = qcow2_can_store_new_dirty_bitmap,
>> + .bdrv_remove_persistent_dirty_bitmap = qcow2_remove_persistent_dirty_bitmap,
>> };
>>
>> static void bdrv_qcow2_init(void)
>> diff --git a/block/qcow2.h b/block/qcow2.h
>> index eaad34a527..a009827f60 100644
>> --- a/block/qcow2.h
>> +++ b/block/qcow2.h
>> @@ -627,5 +627,8 @@ bool qcow2_can_store_new_dirty_bitmap(BlockDriverState *bs,
>> int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
>> void **refcount_table,
>> int64_t *refcount_table_size);
>> +void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs,
>> + const char *name,
>> + Error **errp);
>>
>> #endif
>>
>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH 23/24] qcow2: add .bdrv_remove_persistent_dirty_bitmap
2017-02-02 10:41 ` Vladimir Sementsov-Ogievskiy
@ 2017-02-03 19:01 ` Max Reitz
0 siblings, 0 replies; 55+ messages in thread
From: Max Reitz @ 2017-02-03 19:01 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block, qemu-devel
Cc: kwolf, armbru, eblake, jsnow, famz, den, stefanha, pbonzini
[-- Attachment #1: Type: text/plain, Size: 1978 bytes --]
On 02.02.2017 11:41, Vladimir Sementsov-Ogievskiy wrote:
> 01.02.2017 02:20, Max Reitz wrote:
>> On 23.01.2017 13:10, Vladimir Sementsov-Ogievskiy wrote:
>>> Realize .bdrv_remove_persistent_dirty_bitmap interface.
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>> ---
>>> block/qcow2-bitmap.c | 40 ++++++++++++++++++++++++++++++++++++++++
>>> block/qcow2.c | 1 +
>>> block/qcow2.h | 3 +++
>>> 3 files changed, 44 insertions(+)
>>>
>>> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
>>> index 2687a3acd5..be026fc80e 100644
>>> --- a/block/qcow2-bitmap.c
>>> +++ b/block/qcow2-bitmap.c
>>> @@ -1064,6 +1064,46 @@ static Qcow2Bitmap
>>> *find_bitmap_by_name(Qcow2BitmapList *bm_list,
>>> return NULL;
>>> }
>>> +void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs,
>>> + const char *name,
>>> + Error **errp)
>>> +{
>>> + int ret;
>>> + BDRVQcow2State *s = bs->opaque;
>>> + Qcow2Bitmap *bm;
>>> + Qcow2BitmapList *bm_list;
>>> +
>>> + if (s->nb_bitmaps == 0) {
>>> + /* No bitmaps - nothing to do */
>> Shouldn't it be an error? I.e. "bitmap not found"?
>>
>>> + return;
>>> + }
>>> +
>>> + bm_list = bitmap_list_load(bs, s->bitmap_directory_offset,
>>> + s->bitmap_directory_size, errp);
>>> + if (bm_list == NULL) {
>>> + return;
>>> + }
>>> +
>>> + bm = find_bitmap_by_name(bm_list, name);
>>> + if (bm == NULL) {
>>> + goto fail;
>> What about setting errp? Or do you not consider this an error?
>>
>> I think it should be an error.
>>
>>> + }
>>> +
>>> + QSIMPLEQ_REMOVE(bm_list, bm, Qcow2Bitmap, entry);
>> bm->name is leaked here.
>
> No, bitmap_free(bm) below will free it.
Oops, right. Well then:
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH 23/24] qcow2: add .bdrv_remove_persistent_dirty_bitmap
2017-02-03 9:40 ` [Qemu-devel] [PATCH 23/24] qcow2: add .bdrv_remove_persistent_dirty_bitmap Vladimir Sementsov-Ogievskiy
@ 2017-02-12 3:25 ` Max Reitz
0 siblings, 0 replies; 55+ messages in thread
From: Max Reitz @ 2017-02-12 3:25 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block, qemu-devel
Cc: kwolf, armbru, eblake, jsnow, famz, den, stefanha, pbonzini
[-- Attachment #1: Type: text/plain, Size: 426 bytes --]
On 03.02.2017 10:40, Vladimir Sementsov-Ogievskiy wrote:
> Realize .bdrv_remove_persistent_dirty_bitmap interface.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> block/qcow2-bitmap.c | 41 +++++++++++++++++++++++++++++++++++++++++
> block/qcow2.c | 1 +
> block/qcow2.h | 3 +++
> 3 files changed, 45 insertions(+)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH 23/24] qcow2: add .bdrv_remove_persistent_dirty_bitmap
2017-02-03 9:39 [Qemu-devel] [PATCH v13 00/24] qcow2: persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
@ 2017-02-03 9:40 ` Vladimir Sementsov-Ogievskiy
2017-02-12 3:25 ` Max Reitz
0 siblings, 1 reply; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-02-03 9:40 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: kwolf, mreitz, armbru, eblake, jsnow, famz, den, stefanha,
vsementsov, pbonzini
Realize .bdrv_remove_persistent_dirty_bitmap interface.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/qcow2-bitmap.c | 41 +++++++++++++++++++++++++++++++++++++++++
block/qcow2.c | 1 +
block/qcow2.h | 3 +++
3 files changed, 45 insertions(+)
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index 590a0fcc8e..cfa1943071 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -1065,6 +1065,47 @@ static Qcow2Bitmap *find_bitmap_by_name(Qcow2BitmapList *bm_list,
return NULL;
}
+void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs,
+ const char *name,
+ Error **errp)
+{
+ int ret;
+ BDRVQcow2State *s = bs->opaque;
+ Qcow2Bitmap *bm;
+ Qcow2BitmapList *bm_list;
+
+ if (s->nb_bitmaps == 0) {
+ /* Absence of the bitmap is not an error: see explanation above
+ * bdrv_remove_persistent_dirty_bitmap() definition. */
+ return;
+ }
+
+ bm_list = bitmap_list_load(bs, s->bitmap_directory_offset,
+ s->bitmap_directory_size, errp);
+ if (bm_list == NULL) {
+ return;
+ }
+
+ bm = find_bitmap_by_name(bm_list, name);
+ if (bm == NULL) {
+ goto fail;
+ }
+
+ QSIMPLEQ_REMOVE(bm_list, bm, Qcow2Bitmap, entry);
+
+ ret = update_ext_header_and_dir(bs, bm_list);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "Failed to update bitmap extension");
+ goto fail;
+ }
+
+ free_bitmap_clusters(bs, &bm->table);
+
+fail:
+ bitmap_free(bm);
+ bitmap_list_free(bm_list);
+}
+
void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp)
{
BdrvDirtyBitmap *bitmap;
diff --git a/block/qcow2.c b/block/qcow2.c
index 166a43bd05..9d3a2f4879 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3544,6 +3544,7 @@ BlockDriver bdrv_qcow2 = {
.bdrv_load_autoloading_dirty_bitmaps = qcow2_load_autoloading_dirty_bitmaps,
.bdrv_store_persistent_dirty_bitmaps = qcow2_store_persistent_dirty_bitmaps,
.bdrv_can_store_new_dirty_bitmap = qcow2_can_store_new_dirty_bitmap,
+ .bdrv_remove_persistent_dirty_bitmap = qcow2_remove_persistent_dirty_bitmap,
};
static void bdrv_qcow2_init(void)
diff --git a/block/qcow2.h b/block/qcow2.h
index eaad34a527..a009827f60 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -627,5 +627,8 @@ bool qcow2_can_store_new_dirty_bitmap(BlockDriverState *bs,
int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
void **refcount_table,
int64_t *refcount_table_size);
+void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs,
+ const char *name,
+ Error **errp);
#endif
--
2.11.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH 23/24] qcow2: add .bdrv_remove_persistent_dirty_bitmap
2017-02-02 13:21 [Qemu-devel] [PATCH v12 00/24] qcow2: persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
@ 2017-02-02 13:21 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-02-02 13:21 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: kwolf, mreitz, armbru, eblake, jsnow, famz, den, stefanha,
vsementsov, pbonzini
Realize .bdrv_remove_persistent_dirty_bitmap interface.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/qcow2-bitmap.c | 41 +++++++++++++++++++++++++++++++++++++++++
block/qcow2.c | 1 +
block/qcow2.h | 3 +++
3 files changed, 45 insertions(+)
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index 8bb7f3e621..eafac039bd 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -1064,6 +1064,47 @@ static Qcow2Bitmap *find_bitmap_by_name(Qcow2BitmapList *bm_list,
return NULL;
}
+void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs,
+ const char *name,
+ Error **errp)
+{
+ int ret;
+ BDRVQcow2State *s = bs->opaque;
+ Qcow2Bitmap *bm;
+ Qcow2BitmapList *bm_list;
+
+ if (s->nb_bitmaps == 0) {
+ /* Absence of the bitmap is not an error: see explanation above
+ * bdrv_remove_persistent_dirty_bitmap() definition. */
+ return;
+ }
+
+ bm_list = bitmap_list_load(bs, s->bitmap_directory_offset,
+ s->bitmap_directory_size, errp);
+ if (bm_list == NULL) {
+ return;
+ }
+
+ bm = find_bitmap_by_name(bm_list, name);
+ if (bm == NULL) {
+ goto fail;
+ }
+
+ QSIMPLEQ_REMOVE(bm_list, bm, Qcow2Bitmap, entry);
+
+ ret = update_ext_header_and_dir(bs, bm_list);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "Failed to update bitmap extension");
+ goto fail;
+ }
+
+ free_bitmap_clusters(bs, &bm->table);
+
+fail:
+ bitmap_free(bm);
+ bitmap_list_free(bm_list);
+}
+
void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp)
{
BdrvDirtyBitmap *bitmap;
diff --git a/block/qcow2.c b/block/qcow2.c
index 166a43bd05..9d3a2f4879 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3544,6 +3544,7 @@ BlockDriver bdrv_qcow2 = {
.bdrv_load_autoloading_dirty_bitmaps = qcow2_load_autoloading_dirty_bitmaps,
.bdrv_store_persistent_dirty_bitmaps = qcow2_store_persistent_dirty_bitmaps,
.bdrv_can_store_new_dirty_bitmap = qcow2_can_store_new_dirty_bitmap,
+ .bdrv_remove_persistent_dirty_bitmap = qcow2_remove_persistent_dirty_bitmap,
};
static void bdrv_qcow2_init(void)
diff --git a/block/qcow2.h b/block/qcow2.h
index eaad34a527..a009827f60 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -627,5 +627,8 @@ bool qcow2_can_store_new_dirty_bitmap(BlockDriverState *bs,
int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
void **refcount_table,
int64_t *refcount_table_size);
+void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs,
+ const char *name,
+ Error **errp);
#endif
--
2.11.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH 23/24] qcow2: add .bdrv_remove_persistent_dirty_bitmap
2016-12-21 14:54 [Qemu-devel] [PATCH v10 00/24] qcow2: persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
@ 2016-12-21 14:55 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 55+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2016-12-21 14:55 UTC (permalink / raw)
To: qemu-block, qemu-devel
Cc: kwolf, mreitz, armbru, eblake, jsnow, famz, den, stefanha,
vsementsov, pbonzini
Realize .bdrv_remove_persistent_dirty_bitmap interface.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/qcow2-bitmap.c | 40 ++++++++++++++++++++++++++++++++++++++++
block/qcow2.c | 1 +
block/qcow2.h | 3 +++
3 files changed, 44 insertions(+)
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index 2687a3a..be026fc 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -1064,6 +1064,46 @@ static Qcow2Bitmap *find_bitmap_by_name(Qcow2BitmapList *bm_list,
return NULL;
}
+void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs,
+ const char *name,
+ Error **errp)
+{
+ int ret;
+ BDRVQcow2State *s = bs->opaque;
+ Qcow2Bitmap *bm;
+ Qcow2BitmapList *bm_list;
+
+ if (s->nb_bitmaps == 0) {
+ /* No bitmaps - nothing to do */
+ return;
+ }
+
+ bm_list = bitmap_list_load(bs, s->bitmap_directory_offset,
+ s->bitmap_directory_size, errp);
+ if (bm_list == NULL) {
+ return;
+ }
+
+ bm = find_bitmap_by_name(bm_list, name);
+ if (bm == NULL) {
+ goto fail;
+ }
+
+ QSIMPLEQ_REMOVE(bm_list, bm, Qcow2Bitmap, entry);
+
+ ret = update_ext_header_and_dir(bs, bm_list);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "Failed to update bitmap extension");
+ goto fail;
+ }
+
+ free_bitmap_clusters(bs, &bm->table);
+
+fail:
+ bitmap_free(bm);
+ bitmap_list_free(bm_list);
+}
+
void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp)
{
BdrvDirtyBitmap *bitmap;
diff --git a/block/qcow2.c b/block/qcow2.c
index 76a6a2b..f6bac38e 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3544,6 +3544,7 @@ BlockDriver bdrv_qcow2 = {
.bdrv_load_autoloading_dirty_bitmaps = qcow2_load_autoloading_dirty_bitmaps,
.bdrv_store_persistent_dirty_bitmaps = qcow2_store_persistent_dirty_bitmaps,
.bdrv_can_store_new_dirty_bitmap = qcow2_can_store_new_dirty_bitmap,
+ .bdrv_remove_persistent_dirty_bitmap = qcow2_remove_persistent_dirty_bitmap,
};
static void bdrv_qcow2_init(void)
diff --git a/block/qcow2.h b/block/qcow2.h
index eaad34a..a009827 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -627,5 +627,8 @@ bool qcow2_can_store_new_dirty_bitmap(BlockDriverState *bs,
int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
void **refcount_table,
int64_t *refcount_table_size);
+void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs,
+ const char *name,
+ Error **errp);
#endif
--
1.8.3.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
end of thread, other threads:[~2017-02-12 3:25 UTC | newest]
Thread overview: 55+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-23 12:10 [Qemu-devel] [PATCH v11 00/24] qcow2: persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 01/24] specs/qcow2: fix bitmap granularity qemu-specific note Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 02/24] specs/qcow2: do not use wording 'bitmap header' Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 03/24] hbitmap: improve dirty iter Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 04/24] tests: add hbitmap iter test Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 05/24] block: fix bdrv_dirty_bitmap_granularity signature Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 06/24] block/dirty-bitmap: add deserialize_ones func Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 07/24] qcow2: add bitmaps extension Vladimir Sementsov-Ogievskiy
2017-01-28 21:29 ` Max Reitz
2017-01-30 7:21 ` Vladimir Sementsov-Ogievskiy
2017-01-31 22:53 ` Max Reitz
2017-01-31 11:53 ` Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 08/24] block: introduce auto-loading bitmaps Vladimir Sementsov-Ogievskiy
2017-01-28 21:31 ` Max Reitz
2017-01-23 12:10 ` [Qemu-devel] [PATCH 09/24] qcow2: add .bdrv_load_autoloading_dirty_bitmaps Vladimir Sementsov-Ogievskiy
2017-01-28 21:45 ` Max Reitz
2017-01-28 22:20 ` Max Reitz
2017-01-31 15:11 ` Vladimir Sementsov-Ogievskiy
2017-01-31 22:57 ` Max Reitz
2017-01-23 12:10 ` [Qemu-devel] [PATCH 10/24] block/dirty-bitmap: add autoload field to BdrvDirtyBitmap Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 11/24] block: introduce persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 12/24] block/dirty-bitmap: add bdrv_dirty_bitmap_next() Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 13/24] qcow2: add .bdrv_store_persistent_dirty_bitmaps() Vladimir Sementsov-Ogievskiy
2017-01-28 22:30 ` Max Reitz
2017-01-23 12:10 ` [Qemu-devel] [PATCH 14/24] block: add bdrv_can_store_new_dirty_bitmap Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 15/24] qcow2: add .bdrv_can_store_new_dirty_bitmap Vladimir Sementsov-Ogievskiy
2017-01-28 22:33 ` Max Reitz
2017-01-23 12:10 ` [Qemu-devel] [PATCH 16/24] qmp: add persistent flag to block-dirty-bitmap-add Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 17/24] qmp: add autoload parameter " Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 18/24] qmp: add x-debug-block-dirty-bitmap-sha256 Vladimir Sementsov-Ogievskiy
2017-01-29 16:20 ` Max Reitz
2017-01-23 12:10 ` [Qemu-devel] [PATCH 19/24] iotests: test qcow2 persistent dirty bitmap Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 20/24] qcow2-refcount: rename inc_refcounts() and make it public Vladimir Sementsov-Ogievskiy
2017-01-23 12:10 ` [Qemu-devel] [PATCH 21/24] qcow2-bitmap: refcounts Vladimir Sementsov-Ogievskiy
2017-01-29 16:42 ` Max Reitz
2017-01-23 12:10 ` [Qemu-devel] [PATCH 22/24] block/dirty-bitmap: deep release dirty bitmaps Vladimir Sementsov-Ogievskiy
2017-01-29 16:54 ` Max Reitz
2017-01-31 16:03 ` Vladimir Sementsov-Ogievskiy
2017-01-31 23:00 ` Max Reitz
2017-01-31 23:16 ` Max Reitz
2017-01-23 12:10 ` [Qemu-devel] [PATCH 23/24] qcow2: add .bdrv_remove_persistent_dirty_bitmap Vladimir Sementsov-Ogievskiy
2017-01-31 23:20 ` Max Reitz
2017-02-01 13:57 ` Vladimir Sementsov-Ogievskiy
2017-02-01 14:23 ` Vladimir Sementsov-Ogievskiy
2017-02-01 20:00 ` Max Reitz
2017-02-01 20:00 ` Max Reitz
2017-02-02 10:41 ` Vladimir Sementsov-Ogievskiy
2017-02-03 19:01 ` Max Reitz
2017-01-23 12:10 ` [Qemu-devel] [PATCH 24/24] qcow2-bitmap: cache bitmap list in BDRVQcow2State Vladimir Sementsov-Ogievskiy
2017-01-31 23:27 ` Max Reitz
2017-02-01 10:14 ` Vladimir Sementsov-Ogievskiy
-- strict thread matches above, loose matches on Subject: below --
2017-02-03 9:39 [Qemu-devel] [PATCH v13 00/24] qcow2: persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
2017-02-03 9:40 ` [Qemu-devel] [PATCH 23/24] qcow2: add .bdrv_remove_persistent_dirty_bitmap Vladimir Sementsov-Ogievskiy
2017-02-12 3:25 ` Max Reitz
2017-02-02 13:21 [Qemu-devel] [PATCH v12 00/24] qcow2: persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
2017-02-02 13:21 ` [Qemu-devel] [PATCH 23/24] qcow2: add .bdrv_remove_persistent_dirty_bitmap Vladimir Sementsov-Ogievskiy
2016-12-21 14:54 [Qemu-devel] [PATCH v10 00/24] qcow2: persistent dirty bitmaps Vladimir Sementsov-Ogievskiy
2016-12-21 14:55 ` [Qemu-devel] [PATCH 23/24] qcow2: add .bdrv_remove_persistent_dirty_bitmap Vladimir Sementsov-Ogievskiy
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.