2:29 p.m.
From: Vladimir Sementsov-Ogievskiy <vsementsov(a)virtuozzo.com>
- Correct check for write access to file child, and in correct place
(only if we want to write).
- Support reopen rw -> rw (which will be used in following commit),
for example, !bdrv_dirty_bitmap_readonly() is not a corruption if
bitmap is marked IN_USE in the image.
- Consider unexpected bitmap as a corruption and check other
combinations of in-image and in-RAM bitmaps.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov(a)virtuozzo.com>
Message-id: 20190927122355.7344-9-vsementsov(a)virtuozzo.com
Signed-off-by: John Snow <jsnow(a)redhat.com>
---
block/qcow2-bitmap.c | 77 +++++++++++++++++++++++++++++++++-----------
1 file changed, 58 insertions(+), 19 deletions(-)
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index f7dfb40256..98294a7696 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -1108,18 +1108,14 @@ int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp)
Qcow2BitmapList *bm_list;
Qcow2Bitmap *bm;
GSList *ro_dirty_bitmaps = NULL;
- int ret = 0;
+ int ret = -EINVAL;
+ bool need_header_update = false;
if (s->nb_bitmaps == 0) {
/* No bitmaps - nothing to do */
return 0;
}
- if (!can_write(bs)) {
- error_setg(errp, "Can't write to the image on reopening bitmaps
rw");
- return -EINVAL;
- }
-
bm_list = bitmap_list_load(bs, s->bitmap_directory_offset,
s->bitmap_directory_size, errp);
if (bm_list == NULL) {
@@ -1128,32 +1124,75 @@ int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp)
QSIMPLEQ_FOREACH(bm, bm_list, entry) {
BdrvDirtyBitmap *bitmap = bdrv_find_dirty_bitmap(bs, bm->name);
- if (bitmap == NULL) {
- continue;
- }
- if (!bdrv_dirty_bitmap_readonly(bitmap)) {
- error_setg(errp, "Bitmap %s was loaded prior to rw-reopen, but was
"
- "not marked as readonly. This is a bug, something went
"
- "wrong. All of the bitmaps may be corrupted",
bm->name);
- ret = -EINVAL;
+ if (!bitmap) {
+ error_setg(errp, "Unexpected bitmap '%s' in image
'%s'",
+ bm->name, bs->filename);
goto out;
}
- bm->flags |= BME_FLAG_IN_USE;
- ro_dirty_bitmaps = g_slist_append(ro_dirty_bitmaps, bitmap);
+ if (!(bm->flags & BME_FLAG_IN_USE)) {
+ if (!bdrv_dirty_bitmap_readonly(bitmap)) {
+ error_setg(errp, "Corruption: bitmap '%s' is not marked
IN_USE "
+ "in the image '%s' and not marked readonly in
RAM",
+ bm->name, bs->filename);
+ goto out;
+ }
+ if (bdrv_dirty_bitmap_inconsistent(bitmap)) {
+ error_setg(errp, "Corruption: bitmap '%s' is inconsistent
but "
+ "is not marked IN_USE in the image '%s'",
bm->name,
+ bs->filename);
+ goto out;
+ }
+
+ bm->flags |= BME_FLAG_IN_USE;
+ need_header_update = true;
+ } else {
+ /*
+ * What if flags already has BME_FLAG_IN_USE ?
+ *
+ * 1. if we are reopening RW -> RW it's OK, of course.
+ * 2. if we are reopening RO -> RW:
+ * 2.1 if @bitmap is inconsistent, it's OK. It means that it was
+ * inconsistent (IN_USE) when we loaded it
+ * 2.2 if @bitmap is not inconsistent. This seems to be impossible
+ * and implies third party interaction. Let's error-out for
+ * safety.
+ */
+ if (bdrv_dirty_bitmap_readonly(bitmap) &&
+ !bdrv_dirty_bitmap_inconsistent(bitmap))
+ {
+ error_setg(errp, "Corruption: bitmap '%s' is marked IN_USE
"
+ "in the image '%s' but it is readonly and "
+ "consistent in RAM",
+ bm->name, bs->filename);
+ goto out;
+ }
+ }
+
+ if (bdrv_dirty_bitmap_readonly(bitmap)) {
+ ro_dirty_bitmaps = g_slist_append(ro_dirty_bitmaps, bitmap);
+ }
}
- if (ro_dirty_bitmaps != NULL) {
+ if (need_header_update) {
+ if (!can_write(bs->file->bs) || !(bs->file->perm &
BLK_PERM_WRITE)) {
+ error_setg(errp, "Failed to reopen bitmaps rw: no write access "
+ "the protocol file");
+ goto out;
+ }
+
/* in_use flags must be updated */
ret = update_ext_header_and_dir_in_place(bs, bm_list);
if (ret < 0) {
- error_setg_errno(errp, -ret, "Can't update bitmap directory");
+ error_setg_errno(errp, -ret, "Cannot update bitmap directory");
goto out;
}
- g_slist_foreach(ro_dirty_bitmaps, set_readonly_helper, false);
}
+ g_slist_foreach(ro_dirty_bitmaps, set_readonly_helper, false);
+ ret = 0;
+
out:
g_slist_free(ro_dirty_bitmaps);
bitmap_list_free(bm_list);
--
2.21.0