[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PULL v3 16/19] block/qcow2-bitmap: fix and improve qcow2_reopen_bitmaps
From: |
John Snow |
Subject: |
[PULL v3 16/19] block/qcow2-bitmap: fix and improve qcow2_reopen_bitmaps_rw |
Date: |
Thu, 17 Oct 2019 17:54:33 -0400 |
From: Vladimir Sementsov-Ogievskiy <address@hidden>
- 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 <address@hidden>
Message-id: address@hidden
Signed-off-by: John Snow <address@hidden>
---
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
- [PULL v3 07/19] block/dirty-bitmap: drop BdrvDirtyBitmap.mutex, (continued)
- [PULL v3 07/19] block/dirty-bitmap: drop BdrvDirtyBitmap.mutex, John Snow, 2019/10/17
- [PULL v3 08/19] block/dirty-bitmap: refactor bdrv_dirty_bitmap_next, John Snow, 2019/10/17
- [PULL v3 06/19] block/dirty-bitmap: add bs link, John Snow, 2019/10/17
- [PULL v3 09/19] block: switch reopen queue from QSIMPLEQ to QTAILQ, John Snow, 2019/10/17
- [PULL v3 10/19] block: reverse order for reopen commits, John Snow, 2019/10/17
- [PULL v3 11/19] iotests: add test-case to 165 to test reopening qcow2 bitmaps to RW, John Snow, 2019/10/17
- [PULL v3 12/19] block/qcow2-bitmap: get rid of bdrv_has_changed_persistent_bitmaps, John Snow, 2019/10/17
- [PULL v3 13/19] block/qcow2-bitmap: drop qcow2_reopen_bitmaps_rw_hint(), John Snow, 2019/10/17
- [PULL v3 14/19] block/qcow2-bitmap: do not remove bitmaps on reopen-ro, John Snow, 2019/10/17
- [PULL v3 15/19] iotests: add test 260 to check bitmap life after snapshot + commit, John Snow, 2019/10/17
- [PULL v3 16/19] block/qcow2-bitmap: fix and improve qcow2_reopen_bitmaps_rw,
John Snow <=
- [PULL v3 17/19] qcow2-bitmap: move bitmap reopen-rw code to qcow2_reopen_commit, John Snow, 2019/10/17
- [PULL v3 18/19] MAINTAINERS: Add Vladimir as a reviewer for bitmaps, John Snow, 2019/10/17
- [PULL v3 19/19] dirty-bitmaps: remove deprecated autoload parameter, John Snow, 2019/10/17
- Re: [PULL v3 00/19] Bitmaps patches, Peter Maydell, 2019/10/18