qcow2: Release read-only bitmaps when inactivated

During migration, we release all bitmaps after storing them on disk, as
long as they are (1) stored on disk, (2) not read-only, and (3)
consistent.

(2) seems arbitrary, though.  The reason we do not release them is
because we do not write them, as there is no need to; and then we just
forget about all bitmaps that we have not written to the file.  However,
read-only persistent bitmaps are still in the file and in sync with
their in-memory representation, so we may as well release them just like
any R/W bitmap that we have updated.

It leads to actual problems, too: After migration, letting the source
continue may result in an error if there were any bitmaps on read-only
nodes (such as backing images), because those have not been released by
bdrv_inactive_all(), but bdrv_invalidate_cache_all() attempts to reload
them (which fails, because they are still present in memory).

Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200730120234.49288-2-mreitz@redhat.com>
Tested-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
This commit is contained in:
Max Reitz 2020-07-30 14:02:33 +02:00 committed by Eric Blake
parent 6c5dfc9ccb
commit fe16c7ddf8

View File

@ -1562,11 +1562,22 @@ void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs,
Qcow2Bitmap *bm; Qcow2Bitmap *bm;
if (!bdrv_dirty_bitmap_get_persistence(bitmap) || if (!bdrv_dirty_bitmap_get_persistence(bitmap) ||
bdrv_dirty_bitmap_readonly(bitmap) ||
bdrv_dirty_bitmap_inconsistent(bitmap)) { bdrv_dirty_bitmap_inconsistent(bitmap)) {
continue; continue;
} }
if (bdrv_dirty_bitmap_readonly(bitmap)) {
/*
* Store the bitmap in the associated Qcow2Bitmap so it
* can be released later
*/
bm = find_bitmap_by_name(bm_list, name);
if (bm) {
bm->dirty_bitmap = bitmap;
}
continue;
}
need_write = true; need_write = true;
if (check_constraints_on_bitmap(bs, name, granularity, errp) < 0) { if (check_constraints_on_bitmap(bs, name, granularity, errp) < 0) {
@ -1618,7 +1629,9 @@ void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs,
/* allocate clusters and store bitmaps */ /* allocate clusters and store bitmaps */
QSIMPLEQ_FOREACH(bm, bm_list, entry) { QSIMPLEQ_FOREACH(bm, bm_list, entry) {
if (bm->dirty_bitmap == NULL) { BdrvDirtyBitmap *bitmap = bm->dirty_bitmap;
if (bitmap == NULL || bdrv_dirty_bitmap_readonly(bitmap)) {
continue; continue;
} }
@ -1641,6 +1654,7 @@ void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs,
g_free(tb); g_free(tb);
} }
success:
if (release_stored) { if (release_stored) {
QSIMPLEQ_FOREACH(bm, bm_list, entry) { QSIMPLEQ_FOREACH(bm, bm_list, entry) {
if (bm->dirty_bitmap == NULL) { if (bm->dirty_bitmap == NULL) {
@ -1651,13 +1665,14 @@ void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs,
} }
} }
success:
bitmap_list_free(bm_list); bitmap_list_free(bm_list);
return; return;
fail: fail:
QSIMPLEQ_FOREACH(bm, bm_list, entry) { QSIMPLEQ_FOREACH(bm, bm_list, entry) {
if (bm->dirty_bitmap == NULL || bm->table.offset == 0) { if (bm->dirty_bitmap == NULL || bm->table.offset == 0 ||
bdrv_dirty_bitmap_readonly(bm->dirty_bitmap))
{
continue; continue;
} }