On 06/08/2015 11:21 AM, Vladimir Sementsov-Ogievskiy wrote:
From: Vladimir Sementsov-Ogievskiy <vsement...@parallels.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
---
block/qcow2-dirty-bitmap.c | 5 +++++
block/qcow2.c | 13 +++++++++++--
block/qcow2.h | 9 +++++++++
3 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/block/qcow2-dirty-bitmap.c b/block/qcow2-dirty-bitmap.c
index db83112..686a121 100644
--- a/block/qcow2-dirty-bitmap.c
+++ b/block/qcow2-dirty-bitmap.c
@@ -188,6 +188,11 @@ static int qcow2_write_dirty_bitmaps(BlockDriverState *bs)
s->dirty_bitmaps_offset = dirty_bitmaps_offset;
s->dirty_bitmaps_size = dirty_bitmaps_size;
+ if (s->nb_dirty_bitmaps > 0) {
+ s->autoclear_features |= QCOW2_AUTOCLEAR_DIRTY_BITMAPS;
+ } else {
+ s->autoclear_features &= ~QCOW2_AUTOCLEAR_DIRTY_BITMAPS;
+ }
ret = qcow2_update_header(bs);
if (ret < 0) {
fprintf(stderr, "Could not update qcow2 header\n");
diff --git a/block/qcow2.c b/block/qcow2.c
index 406e55d..f85a55a 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -182,6 +182,14 @@ static int qcow2_read_extensions(BlockDriverState *bs,
uint64_t start_offset,
return ret;
}
+ if (!(s->autoclear_features & QCOW2_AUTOCLEAR_DIRTY_BITMAPS) &&
+ s->nb_dirty_bitmaps > 0) {
+ ret = qcow2_delete_all_dirty_bitmaps(bs, errp);
+ if (ret < 0) {
+ return ret;
+ }
+ }
+
#ifdef DEBUG_EXT
printf("Qcow2: Got dirty bitmaps extension:"
" offset=%" PRIu64 " nb_bitmaps=%" PRIu32 "\n",
@@ -928,8 +936,9 @@ static int qcow2_open(BlockDriverState *bs, QDict *options,
int flags,
}
/* Clear unknown autoclear feature bits */
- if (!bs->read_only && !(flags & BDRV_O_INCOMING) && s->autoclear_features)
{
- s->autoclear_features = 0;
+ if (!bs->read_only && !(flags & BDRV_O_INCOMING) &&
+ (s->autoclear_features & ~QCOW2_AUTOCLEAR_MASK)) {
+ s->autoclear_features |= QCOW2_AUTOCLEAR_MASK;
Like Stefan already mentioned, fixing this |= to &= will fix iotest 036,
which is otherwise broken by this patch.
ret = qcow2_update_header(bs);
if (ret < 0) {
error_setg_errno(errp, -ret, "Could not update qcow2 header");
diff --git a/block/qcow2.h b/block/qcow2.h
index b5e576c..14bd6f9 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -215,6 +215,15 @@ enum {
QCOW2_COMPAT_FEAT_MASK = QCOW2_COMPAT_LAZY_REFCOUNTS,
};
+/* Autoclear feature bits */
+enum {
+ QCOW2_AUTOCLEAR_DIRTY_BITMAPS_BITNR = 0,
+ QCOW2_AUTOCLEAR_DIRTY_BITMAPS =
+ 1 << QCOW2_AUTOCLEAR_DIRTY_BITMAPS_BITNR,
+
+ QCOW2_AUTOCLEAR_MASK = QCOW2_AUTOCLEAR_DIRTY_BITMAPS,
+};
+
I find it a little awkward to have an enum with three different kinds of
data in it, unless I am reading this incorrectly. (bit position, bit
masks, and accumulated bit mask.)
Just enumerating the indices is probably sufficient:
enum {
QCOW2_AUTOCLEAR_BEGIN = 0,
QCOW2_AUTOCLEAR_DIRTY_BITMAPS = QCOW2_AUTOCLEAR_BEGIN,
...,
QCOW2_AUTOCLEAR_END
}
and then the QCOW2_AUTOCLEAR_MASK can either be programmatically defined
via a function, or just pre-computed as a #define.
If you still want the mask definitions, you could do something cheeky
like this:
#define AUTOCLEAR_MASK(X) (1 << QCOW2_AUTOCLEAR_ ## X)
and then you can use things like AUTOCLEAR_MASK(DIRTY_BITMAPS) without
having to create and maintain two separate tables if you want both forms
easily available.
