diff options
Diffstat (limited to '')
| -rw-r--r-- | MAINTAINERS | 3 | ||||
| -rw-r--r-- | block.c | 79 | ||||
| -rw-r--r-- | block/backup.c | 8 | ||||
| -rw-r--r-- | block/block-copy.c | 2 | ||||
| -rw-r--r-- | block/dirty-bitmap.c | 290 | ||||
| -rw-r--r-- | block/mirror.c | 4 | ||||
| -rw-r--r-- | block/qcow2-bitmap.c | 212 | ||||
| -rw-r--r-- | block/qcow2.c | 22 | ||||
| -rw-r--r-- | block/qcow2.h | 19 | ||||
| -rw-r--r-- | blockdev.c | 40 | ||||
| -rw-r--r-- | include/block/block.h | 2 | ||||
| -rw-r--r-- | include/block/block_int.h | 20 | ||||
| -rw-r--r-- | include/block/dirty-bitmap.h | 34 | ||||
| -rw-r--r-- | include/qemu/hbitmap.h | 5 | ||||
| -rw-r--r-- | migration/block-dirty-bitmap.c | 11 | ||||
| -rw-r--r-- | migration/block.c | 4 | ||||
| -rw-r--r-- | qapi/block-core.json | 6 | ||||
| -rw-r--r-- | qemu-deprecated.texi | 20 | ||||
| -rwxr-xr-x | tests/qemu-iotests/165 | 57 | ||||
| -rw-r--r-- | tests/qemu-iotests/165.out | 4 | ||||
| -rwxr-xr-x | tests/qemu-iotests/260 | 89 | ||||
| -rw-r--r-- | tests/qemu-iotests/260.out | 52 | ||||
| -rw-r--r-- | tests/qemu-iotests/group | 1 | ||||
| -rw-r--r-- | tests/test-hbitmap.c | 2 | ||||
| -rw-r--r-- | util/hbitmap.c | 4 |
25 files changed, 623 insertions, 367 deletions
diff --git a/MAINTAINERS b/MAINTAINERS index fe4dc51b08..250ce8e7a1 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1816,8 +1816,8 @@ F: qapi/transaction.json T: git https://repo.or.cz/qemu/armbru.git block-next Dirty Bitmaps -M: Fam Zheng <fam@euphon.net> M: John Snow <jsnow@redhat.com> +R: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> L: qemu-block@nongnu.org S: Supported F: util/hbitmap.c @@ -1826,7 +1826,6 @@ F: include/qemu/hbitmap.h F: include/block/dirty-bitmap.h F: tests/test-hbitmap.c F: docs/interop/bitmaps.rst -T: git https://github.com/famz/qemu.git bitmaps T: git https://github.com/jnsnow/qemu.git bitmaps Character device backends diff --git a/block.c b/block.c index 1946fc6f57..dad5a3d8e0 100644 --- a/block.c +++ b/block.c @@ -1719,7 +1719,7 @@ typedef struct BlockReopenQueueEntry { bool prepared; bool perms_checked; BDRVReopenState state; - QSIMPLEQ_ENTRY(BlockReopenQueueEntry) entry; + QTAILQ_ENTRY(BlockReopenQueueEntry) entry; } BlockReopenQueueEntry; /* @@ -1732,7 +1732,7 @@ static int bdrv_reopen_get_flags(BlockReopenQueue *q, BlockDriverState *bs) BlockReopenQueueEntry *entry; if (q != NULL) { - QSIMPLEQ_FOREACH(entry, q, entry) { + QTAILQ_FOREACH(entry, q, entry) { if (entry->state.bs == bs) { return entry->state.flags; } @@ -3249,7 +3249,7 @@ static bool bdrv_recurse_has_child(BlockDriverState *bs, * Adds a BlockDriverState to a simple queue for an atomic, transactional * reopen of multiple devices. * - * bs_queue can either be an existing BlockReopenQueue that has had QSIMPLE_INIT + * bs_queue can either be an existing BlockReopenQueue that has had QTAILQ_INIT * already performed, or alternatively may be NULL a new BlockReopenQueue will * be created and initialized. This newly created BlockReopenQueue should be * passed back in for subsequent calls that are intended to be of the same @@ -3290,7 +3290,7 @@ static BlockReopenQueue *bdrv_reopen_queue_child(BlockReopenQueue *bs_queue, if (bs_queue == NULL) { bs_queue = g_new0(BlockReopenQueue, 1); - QSIMPLEQ_INIT(bs_queue); + QTAILQ_INIT(bs_queue); } if (!options) { @@ -3298,7 +3298,7 @@ static BlockReopenQueue *bdrv_reopen_queue_child(BlockReopenQueue *bs_queue, } /* Check if this BlockDriverState is already in the queue */ - QSIMPLEQ_FOREACH(bs_entry, bs_queue, entry) { + QTAILQ_FOREACH(bs_entry, bs_queue, entry) { if (bs == bs_entry->state.bs) { break; } @@ -3354,7 +3354,7 @@ static BlockReopenQueue *bdrv_reopen_queue_child(BlockReopenQueue *bs_queue, if (!bs_entry) { bs_entry = g_new0(BlockReopenQueueEntry, 1); - QSIMPLEQ_INSERT_TAIL(bs_queue, bs_entry, entry); + QTAILQ_INSERT_TAIL(bs_queue, bs_entry, entry); } else { qobject_unref(bs_entry->state.options); qobject_unref(bs_entry->state.explicit_options); @@ -3455,7 +3455,7 @@ int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp) assert(bs_queue != NULL); - QSIMPLEQ_FOREACH(bs_entry, bs_queue, entry) { + QTAILQ_FOREACH(bs_entry, bs_queue, entry) { assert(bs_entry->state.bs->quiesce_counter > 0); if (bdrv_reopen_prepare(&bs_entry->state, bs_queue, errp)) { goto cleanup; @@ -3463,7 +3463,7 @@ int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp) bs_entry->prepared = true; } - QSIMPLEQ_FOREACH(bs_entry, bs_queue, entry) { + QTAILQ_FOREACH(bs_entry, bs_queue, entry) { BDRVReopenState *state = &bs_entry->state; ret = bdrv_check_perm(state->bs, bs_queue, state->perm, state->shared_perm, NULL, NULL, errp); @@ -3486,16 +3486,22 @@ int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp) bs_entry->perms_checked = true; } - /* If we reach this point, we have success and just need to apply the - * changes + /* + * If we reach this point, we have success and just need to apply the + * changes. + * + * Reverse order is used to comfort qcow2 driver: on commit it need to write + * IN_USE flag to the image, to mark bitmaps in the image as invalid. But + * children are usually goes after parents in reopen-queue, so go from last + * to first element. */ - QSIMPLEQ_FOREACH(bs_entry, bs_queue, entry) { + QTAILQ_FOREACH_REVERSE(bs_entry, bs_queue, entry) { bdrv_reopen_commit(&bs_entry->state); } ret = 0; cleanup_perm: - QSIMPLEQ_FOREACH_SAFE(bs_entry, bs_queue, entry, next) { + QTAILQ_FOREACH_SAFE(bs_entry, bs_queue, entry, next) { BDRVReopenState *state = &bs_entry->state; if (!bs_entry->perms_checked) { @@ -3512,7 +3518,7 @@ cleanup_perm: } } cleanup: - QSIMPLEQ_FOREACH_SAFE(bs_entry, bs_queue, entry, next) { + QTAILQ_FOREACH_SAFE(bs_entry, bs_queue, entry, next) { if (ret) { if (bs_entry->prepared) { bdrv_reopen_abort(&bs_entry->state); @@ -3552,7 +3558,7 @@ static BlockReopenQueueEntry *find_parent_in_reopen_queue(BlockReopenQueue *q, { BlockReopenQueueEntry *entry; - QSIMPLEQ_FOREACH(entry, q, entry) { + QTAILQ_FOREACH(entry, q, entry) { BlockDriverState *bs = entry->state.bs; BdrvChild *child; @@ -3929,16 +3935,12 @@ void bdrv_reopen_commit(BDRVReopenState *reopen_state) BlockDriver *drv; BlockDriverState *bs; BdrvChild *child; - bool old_can_write, new_can_write; assert(reopen_state != NULL); bs = reopen_state->bs; drv = bs->drv; assert(drv != NULL); - old_can_write = - !bdrv_is_read_only(bs) && !(bdrv_get_flags(bs) & BDRV_O_INACTIVE); - /* If there are any driver level actions to take */ if (drv->bdrv_reopen_commit) { drv->bdrv_reopen_commit(reopen_state); @@ -3982,21 +3984,6 @@ void bdrv_reopen_commit(BDRVReopenState *reopen_state) } bdrv_refresh_limits(bs, NULL); - - new_can_write = - !bdrv_is_read_only(bs) && !(bdrv_get_flags(bs) & BDRV_O_INACTIVE); - if (!old_can_write && new_can_write && drv->bdrv_reopen_bitmaps_rw) { - Error *local_err = NULL; - if (drv->bdrv_reopen_bitmaps_rw(bs, &local_err) < 0) { - /* This is not fatal, bitmaps just left read-only, so all following - * writes will fail. User can remove read-only bitmaps to unblock - * writes. - */ - error_reportf_err(local_err, - "%s: Failed to make dirty bitmaps writable: ", - bdrv_get_node_name(bs)); - } - } } /* @@ -5390,9 +5377,7 @@ static void coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs, } } - for (bm = bdrv_dirty_bitmap_next(bs, NULL); bm; - bm = bdrv_dirty_bitmap_next(bs, bm)) - { + FOR_EACH_DIRTY_BITMAP(bs, bm) { bdrv_dirty_bitmap_skip_store(bm, false); } @@ -6582,25 +6567,3 @@ void bdrv_del_child(BlockDriverState *parent_bs, BdrvChild *child, Error **errp) parent_bs->drv->bdrv_del_child(parent_bs, child, 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/block/backup.c b/block/backup.c index 46978c1785..dddcf77f53 100644 --- a/block/backup.c +++ b/block/backup.c @@ -98,13 +98,13 @@ static void backup_cleanup_sync_bitmap(BackupBlockJob *job, int ret) * We succeeded, or we always intended to sync the bitmap. * Delete this bitmap and install the child. */ - bm = bdrv_dirty_bitmap_abdicate(job->source_bs, job->sync_bitmap, NULL); + bm = bdrv_dirty_bitmap_abdicate(job->sync_bitmap, NULL); } else { /* * We failed, or we never intended to sync the bitmap anyway. * Merge the successor back into the parent, keeping all data. */ - bm = bdrv_reclaim_dirty_bitmap(job->source_bs, job->sync_bitmap, NULL); + bm = bdrv_reclaim_dirty_bitmap(job->sync_bitmap, NULL); } assert(bm); @@ -402,7 +402,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs, } /* Create a new bitmap, and freeze/disable this one. */ - if (bdrv_dirty_bitmap_create_successor(bs, sync_bitmap, errp) < 0) { + if (bdrv_dirty_bitmap_create_successor(sync_bitmap, errp) < 0) { return NULL; } } @@ -472,7 +472,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs, error: if (sync_bitmap) { - bdrv_reclaim_dirty_bitmap(bs, sync_bitmap, NULL); + bdrv_reclaim_dirty_bitmap(sync_bitmap, NULL); } if (job) { backup_clean(&job->common.job); diff --git a/block/block-copy.c b/block/block-copy.c index 0f76ea1e63..066e3a7274 100644 --- a/block/block-copy.c +++ b/block/block-copy.c @@ -60,7 +60,7 @@ void block_copy_state_free(BlockCopyState *s) return; } - bdrv_release_dirty_bitmap(s->source->bs, s->copy_bitmap); + bdrv_release_dirty_bitmap(s->copy_bitmap); g_free(s); } diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c index 134e0c9a0c..4bbb251b2c 100644 --- a/block/dirty-bitmap.c +++ b/block/dirty-bitmap.c @@ -26,11 +26,11 @@ #include "trace.h" #include "block/block_int.h" #include "block/blockjob.h" +#include "qemu/main-loop.h" struct BdrvDirtyBitmap { - QemuMutex *mutex; + BlockDriverState *bs; HBitmap *bitmap; /* Dirty bitmap implementation */ - HBitmap *meta; /* Meta dirty bitmap */ bool busy; /* Bitmap is busy, it can't be used via QMP */ BdrvDirtyBitmap *successor; /* Anonymous child, if any. */ char *name; /* Optional non-empty unique ID */ @@ -71,12 +71,12 @@ static inline void bdrv_dirty_bitmaps_unlock(BlockDriverState *bs) void bdrv_dirty_bitmap_lock(BdrvDirtyBitmap *bitmap) { - qemu_mutex_lock(bitmap->mutex); + bdrv_dirty_bitmaps_lock(bitmap->bs); } void bdrv_dirty_bitmap_unlock(BdrvDirtyBitmap *bitmap) { - qemu_mutex_unlock(bitmap->mutex); + bdrv_dirty_bitmaps_unlock(bitmap->bs); } /* Called with BQL or dirty_bitmap lock taken. */ @@ -115,7 +115,7 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, return NULL; } bitmap = g_new0(BdrvDirtyBitmap, 1); - bitmap->mutex = &bs->dirty_bitmap_mutex; + bitmap->bs = bs; bitmap->bitmap = hbitmap_alloc(bitmap_size, ctz32(granularity)); bitmap->size = bitmap_size; bitmap->name = g_strdup(name); @@ -126,36 +126,6 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, return bitmap; } -/* bdrv_create_meta_dirty_bitmap - * - * Create a meta dirty bitmap that tracks the changes of bits in @bitmap. I.e. - * when a dirty status bit in @bitmap is changed (either from reset to set or - * the other way around), its respective meta dirty bitmap bit will be marked - * dirty as well. - * - * @bitmap: the block dirty bitmap for which to create a meta dirty bitmap. - * @chunk_size: how many bytes of bitmap data does each bit in the meta bitmap - * track. - */ -void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap, - int chunk_size) -{ - assert(!bitmap->meta); - qemu_mutex_lock(bitmap->mutex); - bitmap->meta = hbitmap_create_meta(bitmap->bitmap, - chunk_size * BITS_PER_BYTE); - qemu_mutex_unlock(bitmap->mutex); -} - -void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap) -{ - assert(bitmap->meta); - qemu_mutex_lock(bitmap->mutex); - hbitmap_free_meta(bitmap->bitmap); - bitmap->meta = NULL; - qemu_mutex_unlock(bitmap->mutex); -} - int64_t bdrv_dirty_bitmap_size(const BdrvDirtyBitmap *bitmap) { return bitmap->size; @@ -179,9 +149,9 @@ static bool bdrv_dirty_bitmap_busy(const BdrvDirtyBitmap *bitmap) void bdrv_dirty_bitmap_set_busy(BdrvDirtyBitmap *bitmap, bool busy) { - qemu_mutex_lock(bitmap->mutex); + bdrv_dirty_bitmaps_lock(bitmap->bs); bitmap->busy = busy; - qemu_mutex_unlock(bitmap->mutex); + bdrv_dirty_bitmaps_unlock(bitmap->bs); } /* Called with BQL taken. */ @@ -267,8 +237,7 @@ int bdrv_dirty_bitmap_check(const BdrvDirtyBitmap *bitmap, uint32_t flags, * The successor will be enabled if the parent bitmap was. * Called with BQL taken. */ -int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs, - BdrvDirtyBitmap *bitmap, Error **errp) +int bdrv_dirty_bitmap_create_successor(BdrvDirtyBitmap *bitmap, Error **errp) { uint64_t granularity; BdrvDirtyBitmap *child; @@ -284,7 +253,7 @@ int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs, /* Create an anonymous successor */ granularity = bdrv_dirty_bitmap_granularity(bitmap); - child = bdrv_create_dirty_bitmap(bs, granularity, NULL, errp); + child = bdrv_create_dirty_bitmap(bitmap->bs, granularity, NULL, errp); if (!child) { return -1; } @@ -307,10 +276,10 @@ void bdrv_enable_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap) /* Called with BQL taken. */ void bdrv_dirty_bitmap_enable_successor(BdrvDirtyBitmap *bitmap) { - assert(bitmap->mutex == bitmap->successor->mutex); - qemu_mutex_lock(bitmap->mutex); + assert(bitmap->bs == bitmap->successor->bs); + bdrv_dirty_bitmaps_lock(bitmap->bs); bdrv_enable_dirty_bitmap_locked(bitmap->successor); - qemu_mutex_unlock(bitmap->mutex); + bdrv_dirty_bitmaps_unlock(bitmap->bs); } /* Called within bdrv_dirty_bitmap_lock..unlock and with BQL taken. */ @@ -319,7 +288,6 @@ static void bdrv_release_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap) assert(!bitmap->active_iterators); assert(!bdrv_dirty_bitmap_busy(bitmap)); assert(!bdrv_dirty_bitmap_has_successor(bitmap)); - assert(!bitmap->meta); QLIST_REMOVE(bitmap, list); hbitmap_free(bitmap->bitmap); g_free(bitmap->name); @@ -331,8 +299,7 @@ static void bdrv_release_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap) * delete the old bitmap, and return a handle to the new bitmap. * Called with BQL taken. */ -BdrvDirtyBitmap *bdrv_dirty_bitmap_abdicate(BlockDriverState *bs, - BdrvDirtyBitmap *bitmap, +BdrvDirtyBitmap *bdrv_dirty_bitmap_abdicate(BdrvDirtyBitmap *bitmap, Error **errp) { char *name; @@ -351,7 +318,7 @@ BdrvDirtyBitmap *bdrv_dirty_bitmap_abdicate(BlockDriverState *bs, successor->persistent = bitmap->persistent; bitmap->persistent = false; bitmap->busy = false; - bdrv_release_dirty_bitmap(bs, bitmap); + bdrv_release_dirty_bitmap(bitmap); return successor; } @@ -363,8 +330,7 @@ BdrvDirtyBitmap *bdrv_dirty_bitmap_abdicate(BlockDriverState *bs, * The marged parent will be enabled if and only if the successor was enabled. * Called within bdrv_dirty_bitmap_lock..unlock and with BQL taken. */ -BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BlockDriverState *bs, - BdrvDirtyBitmap *parent, +BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BdrvDirtyBitmap *parent, Error **errp) { BdrvDirtyBitmap *successor = parent->successor; @@ -388,15 +354,14 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BlockDriverState *bs, } /* Called with BQL taken. */ -BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs, - BdrvDirtyBitmap *parent, +BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BdrvDirtyBitmap *parent, Error **errp) { BdrvDirtyBitmap *ret; - qemu_mutex_lock(parent->mutex); - ret = bdrv_reclaim_dirty_bitmap_locked(bs, parent, errp); - qemu_mutex_unlock(parent->mutex); + bdrv_dirty_bitmaps_lock(parent->bs); + ret = bdrv_reclaim_dirty_bitmap_locked(parent, errp); + bdrv_dirty_bitmaps_unlock(parent->bs); return ret; } @@ -421,8 +386,10 @@ void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, int64_t bytes) } /* Called with BQL taken. */ -void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap) +void bdrv_release_dirty_bitmap(BdrvDirtyBitmap *bitmap) { + BlockDriverState *bs = bitmap->bs; + bdrv_dirty_bitmaps_lock(bs); bdrv_release_dirty_bitmap_locked(bitmap); bdrv_dirty_bitmaps_unlock(bs); @@ -455,27 +422,135 @@ void bdrv_release_named_dirty_bitmaps(BlockDriverState *bs) * not fail. * This function doesn't release corresponding BdrvDirtyBitmap. */ -void bdrv_remove_persistent_dirty_bitmap(BlockDriverState *bs, - const char *name, - Error **errp) +static int coroutine_fn +bdrv_co_remove_persistent_dirty_bitmap(BlockDriverState *bs, const char *name, + Error **errp) +{ + if (bs->drv && bs->drv->bdrv_co_remove_persistent_dirty_bitmap) { + return bs->drv->bdrv_co_remove_persistent_dirty_bitmap(bs, name, errp); + } + + return 0; +} + +typedef struct BdrvRemovePersistentDirtyBitmapCo { + BlockDriverState *bs; + const char *name; + Error **errp; + int ret; +} BdrvRemovePersistentDirtyBitmapCo; + +static void coroutine_fn +bdrv_co_remove_persistent_dirty_bitmap_entry(void *opaque) +{ + BdrvRemovePersistentDirtyBitmapCo *s = opaque; + + s->ret = bdrv_co_remove_persistent_dirty_bitmap(s->bs, s->name, s->errp); + aio_wait_kick(); +} + +int bdrv_remove_persistent_dirty_bitmap(BlockDriverState *bs, const char *name, + Error **errp) +{ + if (qemu_in_coroutine()) { + return bdrv_co_remove_persistent_dirty_bitmap(bs, name, errp); + } else { + Coroutine *co; + BdrvRemovePersistentDirtyBitmapCo s = { + .bs = bs, + .name = name, + .errp = errp, + .ret = -EINPROGRESS, + }; + + co = qemu_coroutine_create(bdrv_co_remove_persistent_dirty_bitmap_entry, + &s); + bdrv_coroutine_enter(bs, co); + BDRV_POLL_WHILE(bs, s.ret == -EINPROGRESS); + + return s.ret; + } +} + +static bool coroutine_fn +bdrv_co_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_co_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_co_can_store_new_dirty_bitmap(bs, name, granularity, errp); +} + +typedef struct BdrvCanStoreNewDirtyBitmapCo { + BlockDriverState *bs; + const char *name; + uint32_t granularity; + Error **errp; + bool ret; + + bool in_progress; +} BdrvCanStoreNewDirtyBitmapCo; + +static void coroutine_fn bdrv_co_can_store_new_dirty_bitmap_entry(void *opaque) { - if (bs->drv && bs->drv->bdrv_remove_persistent_dirty_bitmap) { - bs->drv->bdrv_remove_persistent_dirty_bitmap(bs, name, errp); + BdrvCanStoreNewDirtyBitmapCo *s = opaque; + + s->ret = bdrv_co_can_store_new_dirty_bitmap(s->bs, s->name, s->granularity, + s->errp); + s->in_progress = false; + aio_wait_kick(); +} + +bool bdrv_can_store_new_dirty_bitmap(BlockDriverState *bs, const char *name, + uint32_t granularity, Error **errp) +{ + if (qemu_in_coroutine()) { + return bdrv_co_can_store_new_dirty_bitmap(bs, name, granularity, errp); + } else { + Coroutine *co; + BdrvCanStoreNewDirtyBitmapCo s = { + .bs = bs, + .name = name, + .granularity = granularity, + .errp = errp, + .in_progress = true, + }; + + co = qemu_coroutine_create(bdrv_co_can_store_new_dirty_bitmap_entry, + &s); + bdrv_coroutine_enter(bs, co); + BDRV_POLL_WHILE(bs, s.in_progress); + + return s.ret; } } void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap) { - bdrv_dirty_bitmap_lock(bitmap); + bdrv_dirty_bitmaps_lock(bitmap->bs); bitmap->disabled = true; - bdrv_dirty_bitmap_unlock(bitmap); + bdrv_dirty_bitmaps_unlock(bitmap->bs); } void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap) { - bdrv_dirty_bitmap_lock(bitmap); + bdrv_dirty_bitmaps_lock(bitmap->bs); bdrv_enable_dirty_bitmap_locked(bitmap); - bdrv_dirty_bitmap_unlock(bitmap); + bdrv_dirty_bitmaps_unlock(bitmap->bs); } BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs) @@ -516,9 +591,9 @@ bool bdrv_dirty_bitmap_get_locked(BdrvDirtyBitmap *bitmap, int64_t offset) bool bdrv_dirty_bitmap_get(BdrvDirtyBitmap *bitmap, int64_t offset) { bool ret; - bdrv_dirty_bitmap_lock(bitmap); + bdrv_dirty_bitmaps_lock(bitmap->bs); ret = bdrv_dirty_bitmap_get_locked(bitmap, offset); - bdrv_dirty_bitmap_unlock(bitmap); + bdrv_dirty_bitmaps_unlock(bitmap->bs); return ret; } @@ -557,15 +632,6 @@ BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap) return iter; } -BdrvDirtyBitmapIter *bdrv_dirty_meta_iter_new(BdrvDirtyBitmap *bitmap) -{ - BdrvDirtyBitmapIter *iter = g_new(BdrvDirtyBitmapIter, 1); - hbitmap_iter_init(&iter->hbi, bitmap->meta, 0); - iter->bitmap = bitmap; - bitmap->active_iterators++; - return iter; -} - void bdrv_dirty_iter_free(BdrvDirtyBitmapIter *iter) { if (!iter) { @@ -592,9 +658,9 @@ void bdrv_set_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap, void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap, int64_t offset, int64_t bytes) { - bdrv_dirty_bitmap_lock(bitmap); + bdrv_dirty_bitmaps_lock(bitmap->bs); bdrv_set_dirty_bitmap_locked(bitmap, offset, bytes); - bdrv_dirty_bitmap_unlock(bitmap); + bdrv_dirty_bitmaps_unlock(bitmap->bs); } /* Called within bdrv_dirty_bitmap_lock..unlock */ @@ -608,15 +674,15 @@ void bdrv_reset_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap, void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap, int64_t offset, int64_t bytes) { - bdrv_dirty_bitmap_lock(bitmap); + bdrv_dirty_bitmaps_lock(bitmap->bs); bdrv_reset_dirty_bitmap_locked(bitmap, offset, bytes); - bdrv_dirty_bitmap_unlock(bitmap); + bdrv_dirty_bitmaps_unlock(bitmap->bs); } void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap **out) { assert(!bdrv_dirty_bitmap_readonly(bitmap)); - bdrv_dirty_bitmap_lock(bitmap); + bdrv_dirty_bitmaps_lock(bitmap->bs); if (!out) { hbitmap_reset_all(bitmap->bitmap); } else { @@ -625,7 +691,7 @@ void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap **out) hbitmap_granularity(backup)); *out = backup; } - bdrv_dirty_bitmap_unlock(bitmap); + bdrv_dirty_bitmaps_unlock(bitmap->bs); } void bdrv_restore_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap *backup) @@ -712,11 +778,6 @@ int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap) return hbitmap_count(bitmap->bitmap); } -int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap) -{ - return hbitmap_count(bitmap->meta); -} - bool bdrv_dirty_bitmap_readonly(const BdrvDirtyBitmap *bitmap) { return bitmap->readonly; @@ -725,9 +786,9 @@ bool bdrv_dirty_bitmap_readonly(const BdrvDirtyBitmap *bitmap) /* Called with BQL taken. */ void bdrv_dirty_bitmap_set_readonly(BdrvDirtyBitmap *bitmap, bool value) { - qemu_mutex_lock(bitmap->mutex); + bdrv_dirty_bitmaps_lock(bitmap->bs); bitmap->readonly = value; - qemu_mutex_unlock(bitmap->mutex); + bdrv_dirty_bitmaps_unlock(bitmap->bs); } bool bdrv_has_readonly_bitmaps(BlockDriverState *bs) @@ -745,27 +806,27 @@ bool bdrv_has_readonly_bitmaps(BlockDriverState *bs) /* Called with BQL taken. */ void bdrv_dirty_bitmap_set_persistence(BdrvDirtyBitmap *bitmap, bool persistent) { - qemu_mutex_lock(bitmap->mutex); + bdrv_dirty_bitmaps_lock(bitmap->bs); bitmap->persistent = persistent; - qemu_mutex_unlock(bitmap->mutex); + bdrv_dirty_bitmaps_unlock(bitmap->bs); } /* Called with BQL taken. */ void bdrv_dirty_bitmap_set_inconsistent(BdrvDirtyBitmap *bitmap) { - qemu_mutex_lock(bitmap->mutex); + bdrv_dirty_bitmaps_lock(bitmap->bs); assert(bitmap->persistent == true); bitmap->inconsistent = true; bitmap->disabled = true; - qemu_mutex_unlock(bitmap->mutex); + bdrv_dirty_bitmaps_unlock(bitmap->bs); } /* Called with BQL taken. */ void bdrv_dirty_bitmap_skip_store(BdrvDirtyBitmap *bitmap, bool skip) { - qemu_mutex_lock(bitmap->mutex); + bdrv_dirty_bitmaps_lock(bitmap->bs); bitmap->skip_store = skip; - qemu_mutex_unlock(bitmap->mutex); + bdrv_dirty_bitmaps_unlock(bitmap->bs); } bool bdrv_dirty_bitmap_get_persistence(BdrvDirtyBitmap *bitmap) @@ -778,23 +839,14 @@ bool bdrv_dirty_bitmap_inconsistent(const BdrvDirtyBitmap *bitmap) return bitmap->inconsistent; } -bool bdrv_has_changed_persistent_bitmaps(BlockDriverState *bs) +BdrvDirtyBitmap *bdrv_dirty_bitmap_first(BlockDriverState *bs) { - BdrvDirtyBitmap *bm; - QLIST_FOREACH(bm, &bs->dirty_bitmaps, list) { - if (bm->persistent && !bm->readonly && !bm->skip_store) { - return true; - } - } - - return false; + return QLIST_FIRST(&bs->dirty_bitmaps); } -BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BlockDriverState *bs, - BdrvDirtyBitmap *bitmap) +BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BdrvDirtyBitmap *bitmap) { - return bitmap == NULL ? QLIST_FIRST(&bs->dirty_bitmaps) : - QLIST_NEXT(bitmap, list); + return QLIST_NEXT(bitmap, list); } char *bdrv_dirty_bitmap_sha256(const BdrvDirtyBitmap *bitmap, Error **errp) @@ -825,9 +877,9 @@ void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src, { bool ret; - qemu_mutex_lock(dest->mutex); - if (src->mutex != dest->mutex) { - qemu_mutex_lock(src->mutex); + bdrv_dirty_bitmaps_lock(dest->bs); + if (src->bs != dest->bs) { + bdrv_dirty_bitmaps_lock(src->bs); } if (bdrv_dirty_bitmap_check(dest, BDRV_BITMAP_DEFAULT, errp)) { @@ -847,9 +899,9 @@ void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src, assert(ret); out: - qemu_mutex_unlock(dest->mutex); - if (src->mutex != dest->mutex) { - qemu_mutex_unlock(src->mutex); + bdrv_dirty_bitmaps_unlock(dest->bs); + if (src->bs != dest->bs) { + bdrv_dirty_bitmaps_unlock(src->bs); } } @@ -873,9 +925,9 @@ bool bdrv_dirty_bitmap_merge_internal(BdrvDirtyBitmap *dest, assert(!bdrv_dirty_bitmap_inconsistent(src)); if (lock) { - qemu_mutex_lock(dest->mutex); - if (src->mutex != dest->mutex) { - qemu_mutex_lock(src->mutex); + bdrv_dirty_bitmaps_lock(dest->bs); + if (src->bs != dest->bs) { + bdrv_dirty_bitmaps_lock(src->bs); } } @@ -888,9 +940,9 @@ bool bdrv_dirty_bitmap_merge_internal(BdrvDirtyBitmap *dest, } if (lock) { - qemu_mutex_unlock(dest->mutex); - if (src->mutex != dest->mutex) { - qemu_mutex_unlock(src->mutex); + bdrv_dirty_bitmaps_unlock(dest->bs); + if (src->bs != dest->bs) { + bdrv_dirty_bitmaps_unlock(src->bs); } } diff --git a/block/mirror.c b/block/mirror.c index fe984efb90..a6c50caea4 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -638,7 +638,7 @@ static int mirror_exit_common(Job *job) bdrv_unfreeze_backing_chain(mirror_top_bs, target_bs); } - bdrv_release_dirty_bitmap(src, s->dirty_bitmap); + bdrv_release_dirty_bitmap(s->dirty_bitmap); /* Make sure that the source BDS doesn't go away during bdrv_replace_node, * before we can call bdrv_drained_end */ @@ -1709,7 +1709,7 @@ fail: blk_unref(s->target); bs_opaque->job = NULL; if (s->dirty_bitmap) { - bdrv_release_dirty_bitmap(bs, s->dirty_bitmap); + bdrv_release_dirty_bitmap(s->dirty_bitmap); } job_early_fail(&s->common.job); } diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c index b2487101ed..98294a7696 100644 --- a/block/qcow2-bitmap.c +++ b/block/qcow2-bitmap.c @@ -374,7 +374,7 @@ static BdrvDirtyBitmap *load_bitmap(BlockDriverState *bs, fail: g_free(bitmap_table); if (bitmap != NULL) { - bdrv_release_dirty_bitmap(bs, bitmap); + bdrv_release_dirty_bitmap(bitmap); } return NULL; @@ -941,7 +941,7 @@ fail: static void release_dirty_bitmap_helper(gpointer bitmap, gpointer bs) { - bdrv_release_dirty_bitmap(bs, bitmap); + bdrv_release_dirty_bitmap(bitmap); } /* for g_slist_foreach for GSList of BdrvDirtyBitmap* elements */ @@ -1102,29 +1102,20 @@ Qcow2BitmapInfoList *qcow2_get_bitmap_info_list(BlockDriverState *bs, return list; } -int qcow2_reopen_bitmaps_rw_hint(BlockDriverState *bs, bool *header_updated, - Error **errp) +int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp) { BDRVQcow2State *s = bs->opaque; Qcow2BitmapList *bm_list; Qcow2Bitmap *bm; GSList *ro_dirty_bitmaps = NULL; - int ret = 0; - - if (header_updated != NULL) { - *header_updated = false; - } + 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) { @@ -1133,35 +1124,75 @@ int qcow2_reopen_bitmaps_rw_hint(BlockDriverState *bs, bool *header_updated, 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; } - if (header_updated != NULL) { - *header_updated = true; - } - 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); @@ -1169,11 +1200,6 @@ out: return ret; } -int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp) -{ - return qcow2_reopen_bitmaps_rw_hint(bs, NULL, errp); -} - /* Checks to see if it's safe to resize bitmaps */ int qcow2_truncate_bitmaps_check(BlockDriverState *bs, Error **errp) { @@ -1404,30 +1430,34 @@ 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 coroutine_fn qcow2_co_remove_persistent_dirty_bitmap(BlockDriverState *bs, + const char *name, + Error **errp) { int ret; BDRVQcow2State *s = bs->opaque; - Qcow2Bitmap *bm; + Qcow2Bitmap *bm = NULL; 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; + return 0; } + qemu_co_mutex_lock(&s->lock); + bm_list = bitmap_list_load(bs, s->bitmap_directory_offset, s->bitmap_directory_size, errp); if (bm_list == NULL) { - return; + ret = -EIO; + goto out; } bm = find_bitmap_by_name(bm_list, name); if (bm == NULL) { - goto fail; + ret = -EINVAL; + goto out; } QSIMPLEQ_REMOVE(bm_list, bm, Qcow2Bitmap, entry); @@ -1435,17 +1465,46 @@ void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs, ret = update_ext_header_and_dir(bs, bm_list); if (ret < 0) { error_setg_errno(errp, -ret, "Failed to update bitmap extension"); - goto fail; + goto out; } free_bitmap_clusters(bs, &bm->table); -fail: +out: + qemu_co_mutex_unlock(&s->lock); + bitmap_free(bm); bitmap_list_free(bm_list); + + return ret; } -void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp) +/* + * qcow2_store_persistent_dirty_bitmaps + * + * Stores persistent BdrvDirtyBitmap objects. + * + * @release_stored: if true, release BdrvDirtyBitmap's after storing to the + * image. This is used in two cases, both via qcow2_inactivate: + * 1. bdrv_close: It's correct to remove bitmaps on close. + * 2. migration: If bitmaps are migrated through migration channel via + * 'dirty-bitmaps' migration capability they are not handled by this code. + * Otherwise, it's OK to drop BdrvDirtyBitmap's and reload them on + * invalidation. + * + * Anyway, it's correct to remove BdrvDirtyBitmap's on inactivation, as + * inactivation means that we lose control on disk, and therefore on bitmaps, + * we should sync them and do not touch more. + * + * Contrariwise, we don't want to release any bitmaps on just reopen-to-ro, + * when we need to store them, as image is still under our control, and it's + * good to keep all the bitmaps in read-only mode. Moreover, keeping them + * read-only is correct because this is what would happen if we opened the node + * readonly to begin with, and whether we opened directly or reopened to that + * state shouldn't matter for the state we get afterward. + */ +void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, + bool release_stored, Error **errp) { BdrvDirtyBitmap *bitmap; BDRVQcow2State *s = bs->opaque; @@ -1456,16 +1515,7 @@ void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp) Qcow2Bitmap *bm; QSIMPLEQ_HEAD(, Qcow2BitmapTable) drop_tables; Qcow2BitmapTable *tb, *tb_next; - - if (!bdrv_has_changed_persistent_bitmaps(bs)) { - /* nothing to do */ - return; - } - - if (!can_write(bs)) { - error_setg(errp, "No write access"); - return; - } + bool need_write = false; QSIMPLEQ_INIT(&drop_tables); @@ -1480,9 +1530,7 @@ void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp) } /* check constraints and names */ - for (bitmap = bdrv_dirty_bitmap_next(bs, NULL); bitmap != NULL; - bitmap = bdrv_dirty_bitmap_next(bs, bitmap)) - { + FOR_EACH_DIRTY_BITMAP(bs, bitmap) { const char *name = bdrv_dirty_bitmap_name(bitmap); uint32_t granularity = bdrv_dirty_bitmap_granularity(bitmap); Qcow2Bitmap *bm; @@ -1493,6 +1541,8 @@ void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp) continue; } + need_write = true; + if (check_constraints_on_bitmap(bs, name, granularity, errp) < 0) { error_prepend(errp, "Bitmap '%s' doesn't satisfy the constraints: ", name); @@ -1531,6 +1581,15 @@ void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp) bm->dirty_bitmap = bitmap; } + if (!need_write) { + goto success; + } + + if (!can_write(bs)) { + error_setg(errp, "No write access"); + goto fail; + } + /* allocate clusters and store bitmaps */ QSIMPLEQ_FOREACH(bm, bm_list, entry) { if (bm->dirty_bitmap == NULL) { @@ -1556,22 +1615,17 @@ void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp) g_free(tb); } - QSIMPLEQ_FOREACH(bm, bm_list, entry) { - /* For safety, we remove bitmap after storing. - * We may be here in two cases: - * 1. bdrv_close. It's ok to drop bitmap. - * 2. inactivation. It means migration without 'dirty-bitmaps' - * capability, so bitmaps are not marked with - * BdrvDirtyBitmap.migration flags. It's not bad to drop them too, - * and reload on invalidation. - */ - if (bm->dirty_bitmap == NULL) { - continue; - } + if (release_stored) { + QSIMPLEQ_FOREACH(bm, bm_list, entry) { + if (bm->dirty_bitmap == NULL) { + continue; + } - bdrv_release_dirty_bitmap(bs, bm->dirty_bitmap); + bdrv_release_dirty_bitmap(bm->dirty_bitmap); + } } +success: bitmap_list_free(bm_list); return; @@ -1596,15 +1650,13 @@ int qcow2_reopen_bitmaps_ro(BlockDriverState *bs, Error **errp) BdrvDirtyBitmap *bitmap; Error *local_err = NULL; - qcow2_store_persistent_dirty_bitmaps(bs, &local_err); + qcow2_store_persistent_dirty_bitmaps(bs, false, &local_err); if (local_err != NULL) { error_propagate(errp, local_err); return -EINVAL; } - for (bitmap = bdrv_dirty_bitmap_next(bs, NULL); bitmap != NULL; - bitmap = bdrv_dirty_bitmap_next(bs, bitmap)) - { + FOR_EACH_DIRTY_BITMAP(bs, bitmap) { if (bdrv_dirty_bitmap_get_persistence(bitmap)) { bdrv_dirty_bitmap_set_readonly(bitmap, true); } @@ -1613,10 +1665,10 @@ int qcow2_reopen_bitmaps_ro(BlockDriverState *bs, Error **errp) return 0; } -bool qcow2_can_store_new_dirty_bitmap(BlockDriverState *bs, - const char *name, - uint32_t granularity, - Error **errp) +bool coroutine_fn qcow2_co_can_store_new_dirty_bitmap(BlockDriverState *bs, + const char *name, + uint32_t granularity, + Error **errp) { BDRVQcow2State *s = bs->opaque; bool found; @@ -1653,8 +1705,10 @@ bool qcow2_can_store_new_dirty_bitmap(BlockDriverState *bs, goto fail; } + qemu_co_mutex_lock(&s->lock); bm_list = bitmap_list_load(bs, s->bitmap_directory_offset, s->bitmap_directory_size, errp); + qemu_co_mutex_unlock(&s->lock); if (bm_list == NULL) { goto fail; } diff --git a/block/qcow2.c b/block/qcow2.c index 7961c05783..8b05933565 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1835,6 +1835,20 @@ fail: static void qcow2_reopen_commit(BDRVReopenState *state) { qcow2_update_options_commit(state->bs, state->opaque); + if (state->flags & BDRV_O_RDWR) { + Error *local_err = NULL; + + if (qcow2_reopen_bitmaps_rw(state->bs, &local_err) < 0) { + /* + * This is not fatal, bitmaps just left read-only, so all following + * writes will fail. User can remove read-only bitmaps to unblock + * writes or retry reopen. + */ + error_reportf_err(local_err, + "%s: Failed to make dirty bitmaps writable: ", + bdrv_get_node_name(state->bs)); + } + } g_free(state->opaque); } @@ -2503,7 +2517,7 @@ static int qcow2_inactivate(BlockDriverState *bs) int ret, result = 0; Error *local_err = NULL; - qcow2_store_persistent_dirty_bitmaps(bs, &local_err); + qcow2_store_persistent_dirty_bitmaps(bs, true, &local_err); if (local_err != NULL) { result = -EINVAL; error_reportf_err(local_err, "Lost persistent bitmaps during " @@ -5406,9 +5420,9 @@ BlockDriver bdrv_qcow2 = { .bdrv_detach_aio_context = qcow2_detach_aio_context, .bdrv_attach_aio_context = qcow2_attach_aio_context, - .bdrv_reopen_bitmaps_rw = qcow2_reopen_bitmaps_rw, - .bdrv_can_store_new_dirty_bitmap = qcow2_can_store_new_dirty_bitmap, - .bdrv_remove_persistent_dirty_bitmap = qcow2_remove_persistent_dirty_bitmap, + .bdrv_co_can_store_new_dirty_bitmap = qcow2_co_can_store_new_dirty_bitmap, + .bdrv_co_remove_persistent_dirty_bitmap = + qcow2_co_remove_persistent_dirty_bitmap, }; static void bdrv_qcow2_init(void) diff --git a/block/qcow2.h b/block/qcow2.h index f51f478e34..5cccd87162 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -740,19 +740,18 @@ int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, BdrvCheckResult *res, bool qcow2_load_dirty_bitmaps(BlockDriverState *bs, Error **errp); Qcow2BitmapInfoList *qcow2_get_bitmap_info_list(BlockDriverState *bs, Error **errp); -int qcow2_reopen_bitmaps_rw_hint(BlockDriverState *bs, bool *header_updated, - Error **errp); int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp); int qcow2_truncate_bitmaps_check(BlockDriverState *bs, Error **errp); -void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp); +void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, + bool release_stored, Error **errp); int qcow2_reopen_bitmaps_ro(BlockDriverState *bs, Error **errp); -bool qcow2_can_store_new_dirty_bitmap(BlockDriverState *bs, - const char *name, - uint32_t granularity, - Error **errp); -void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs, - const char *name, - Error **errp); +bool qcow2_co_can_store_new_dirty_bitmap(BlockDriverState *bs, + const char *name, + uint32_t granularity, + Error **errp); +int qcow2_co_remove_persistent_dirty_bitmap(BlockDriverState *bs, + const char *name, + Error **errp); ssize_t coroutine_fn qcow2_co_compress(BlockDriverState *bs, void *dest, size_t dest_size, diff --git a/blockdev.c b/blockdev.c index f89e48fc79..03c7cd7651 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1966,7 +1966,6 @@ 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, action->has_disabled, action->disabled, &local_err); @@ -2178,7 +2177,7 @@ static void block_dirty_bitmap_remove_commit(BlkActionState *common) common, common); bdrv_dirty_bitmap_set_busy(state->bitmap, false); - bdrv_release_dirty_bitmap(state->bs, state->bitmap); + bdrv_release_dirty_bitmap(state->bitmap); } static void abort_prepare(BlkActionState *common, Error **errp) @@ -2858,7 +2857,6 @@ 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, bool has_disabled, bool disabled, Error **errp) { @@ -2890,24 +2888,14 @@ void qmp_block_dirty_bitmap_add(const char *node, const char *name, persistent = false; } - if (has_autoload) { - warn_report("Autoload option is deprecated and its value is ignored"); - } - if (!has_disabled) { disabled = false; } - if (persistent) { - AioContext *aio_context = bdrv_get_aio_context(bs); - bool ok; - - aio_context_acquire(aio_context); - ok = bdrv_can_store_new_dirty_bitmap(bs, name, granularity, errp); - aio_context_release(aio_context); - if (!ok) { - return; - } + if (persistent && + !bdrv_can_store_new_dirty_bitmap(bs, name, granularity, errp)) + { + return; } bitmap = bdrv_create_dirty_bitmap(bs, granularity, name, errp); @@ -2939,22 +2927,14 @@ static BdrvDirtyBitmap *do_block_dirty_bitmap_remove( return NULL; } - if (bdrv_dirty_bitmap_get_persistence(bitmap)) { - AioContext *aio_context = bdrv_get_aio_context(bs); - Error *local_err = NULL; - - aio_context_acquire(aio_context); - bdrv_remove_persistent_dirty_bitmap(bs, name, &local_err); - aio_context_release(aio_context); - - if (local_err != NULL) { - error_propagate(errp, local_err); + if (bdrv_dirty_bitmap_get_persistence(bitmap) && + bdrv_remove_persistent_dirty_bitmap(bs, name, errp) < 0) + { return NULL; - } } if (release) { - bdrv_release_dirty_bitmap(bs, bitmap); + bdrv_release_dirty_bitmap(bitmap); } if (bitmap_bs) { @@ -3086,7 +3066,7 @@ static BdrvDirtyBitmap *do_block_dirty_bitmap_merge( bdrv_merge_dirty_bitmap(dst, anon, backup, errp); out: - bdrv_release_dirty_bitmap(bs, anon); + bdrv_release_dirty_bitmap(anon); return dst; } diff --git a/include/block/block.h b/include/block/block.h index 792bb826db..89606bd9f8 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -195,7 +195,7 @@ typedef struct HDGeometry { #define BDRV_BLOCK_EOF 0x20 #define BDRV_BLOCK_RECURSE 0x40 -typedef QSIMPLEQ_HEAD(BlockReopenQueue, BlockReopenQueueEntry) BlockReopenQueue; +typedef QTAILQ_HEAD(BlockReopenQueue, BlockReopenQueueEntry) BlockReopenQueue; typedef struct BDRVReopenState { BlockDriverState *bs; diff --git a/include/block/block_int.h b/include/block/block_int.h index 05056b308a..ca4ccac4c1 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -547,19 +547,13 @@ struct BlockDriver { uint64_t parent_perm, uint64_t parent_shared, uint64_t *nperm, uint64_t *nshared); - /** - * Bitmaps should be marked as 'IN_USE' in the image on reopening image - * as rw. This handler should realize it. It also should unset readonly - * field of BlockDirtyBitmap's in case of success. - */ - int (*bdrv_reopen_bitmaps_rw)(BlockDriverState *bs, 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); + bool (*bdrv_co_can_store_new_dirty_bitmap)(BlockDriverState *bs, + const char *name, + uint32_t granularity, + Error **errp); + int (*bdrv_co_remove_persistent_dirty_bitmap)(BlockDriverState *bs, + const char *name, + Error **errp); /** * Register/unregister a buffer for I/O. For example, when the driver is diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h index 4b4b731b46..958e7474fb 100644 --- a/include/block/dirty-bitmap.h +++ b/include/block/dirty-bitmap.h @@ -18,28 +18,21 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, uint32_t granularity, const char *name, Error **errp); -void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap, - int chunk_size); -void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap); -int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs, - BdrvDirtyBitmap *bitmap, +int bdrv_dirty_bitmap_create_successor(BdrvDirtyBitmap *bitmap, Error **errp); -BdrvDirtyBitmap *bdrv_dirty_bitmap_abdicate(BlockDriverState *bs, - BdrvDirtyBitmap *bitmap, +BdrvDirtyBitmap *bdrv_dirty_bitmap_abdicate(BdrvDirtyBitmap *bitmap, Error **errp); -BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs, - BdrvDirtyBitmap *bitmap, +BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BdrvDirtyBitmap *bitmap, Error **errp); void bdrv_dirty_bitmap_enable_successor(BdrvDirtyBitmap *bitmap); BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs, const char *name); int bdrv_dirty_bitmap_check(const BdrvDirtyBitmap *bitmap, uint32_t flags, Error **errp); -void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap); +void bdrv_release_dirty_bitmap(BdrvDirtyBitmap *bitmap); void bdrv_release_named_dirty_bitmaps(BlockDriverState *bs); -void bdrv_remove_persistent_dirty_bitmap(BlockDriverState *bs, - const char *name, - Error **errp); +int bdrv_remove_persistent_dirty_bitmap(BlockDriverState *bs, const char *name, + Error **errp); void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap); void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap); void bdrv_enable_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap); @@ -55,7 +48,6 @@ void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap, int64_t offset, int64_t bytes); void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap, int64_t offset, int64_t bytes); -BdrvDirtyBitmapIter *bdrv_dirty_meta_iter_new(BdrvDirtyBitmap *bitmap); BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap); void bdrv_dirty_iter_free(BdrvDirtyBitmapIter *iter); @@ -97,23 +89,25 @@ void bdrv_reset_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap, int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *iter); void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *hbi, int64_t offset); int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap); -int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap); void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, int64_t bytes); bool bdrv_dirty_bitmap_readonly(const BdrvDirtyBitmap *bitmap); bool bdrv_has_readonly_bitmaps(BlockDriverState *bs); bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap); bool bdrv_dirty_bitmap_get_persistence(BdrvDirtyBitmap *bitmap); bool bdrv_dirty_bitmap_inconsistent(const BdrvDirtyBitmap *bitmap); -bool bdrv_has_changed_persistent_bitmaps(BlockDriverState *bs); -BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BlockDriverState *bs, - BdrvDirtyBitmap *bitmap); + +BdrvDirtyBitmap *bdrv_dirty_bitmap_first(BlockDriverState *bs); +BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BdrvDirtyBitmap *bitmap); +#define FOR_EACH_DIRTY_BITMAP(bs, bitmap) \ +for (bitmap = bdrv_dirty_bitmap_first(bs); bitmap; \ + bitmap = bdrv_dirty_bitmap_next(bitmap)) + char *bdrv_dirty_bitmap_sha256(const BdrvDirtyBitmap *bitmap, Error **errp); int64_t bdrv_dirty_bitmap_next_zero(BdrvDirtyBitmap *bitmap, uint64_t offset, uint64_t bytes); bool bdrv_dirty_bitmap_next_dirty_area(BdrvDirtyBitmap *bitmap, uint64_t *offset, uint64_t *bytes); -BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BlockDriverState *bs, - BdrvDirtyBitmap *bitmap, +BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap, Error **errp); #endif diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h index 4afbe6292e..1bf944ca3d 100644 --- a/include/qemu/hbitmap.h +++ b/include/qemu/hbitmap.h @@ -132,6 +132,11 @@ void hbitmap_set(HBitmap *hb, uint64_t start, uint64_t count); * @count: Number of bits to reset. * * Reset a consecutive range of bits in an HBitmap. + * @start and @count must be aligned to bitmap granularity. The only exception + * is resetting the tail of the bitmap: @count may be equal to hb->orig_size - + * @start, in this case @count may be not aligned. The sum of @start + @count is + * allowed to be greater than hb->orig_size, but only if @start < hb->orig_size + * and @start + @count = ALIGN_UP(hb->orig_size, granularity). */ void hbitmap_reset(HBitmap *hb, uint64_t start, uint64_t count); diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c index 5121f86d73..7eafface61 100644 --- a/migration/block-dirty-bitmap.c +++ b/migration/block-dirty-bitmap.c @@ -283,9 +283,7 @@ static int init_dirty_bitmap_migration(void) for (bs = bdrv_next_all_states(NULL); bs; bs = bdrv_next_all_states(bs)) { const char *name = bdrv_get_device_or_node_name(bs); - for (bitmap = bdrv_dirty_bitmap_next(bs, NULL); bitmap; - bitmap = bdrv_dirty_bitmap_next(bs, bitmap)) - { + FOR_EACH_DIRTY_BITMAP(bs, bitmap) { if (!bdrv_dirty_bitmap_name(bitmap)) { continue; } @@ -474,7 +472,7 @@ static int dirty_bitmap_load_start(QEMUFile *f, DirtyBitmapLoadState *s) if (flags & DIRTY_BITMAP_MIG_START_FLAG_ENABLED) { DirtyBitmapLoadBitmapState *b; - bdrv_dirty_bitmap_create_successor(s->bs, s->bitmap, &local_err); + bdrv_dirty_bitmap_create_successor(s->bitmap, &local_err); if (local_err) { error_report_err(local_err); return -EINVAL; @@ -535,13 +533,12 @@ static void dirty_bitmap_load_complete(QEMUFile *f, DirtyBitmapLoadState *s) bdrv_dirty_bitmap_lock(s->bitmap); if (enabled_bitmaps == NULL) { /* in postcopy */ - bdrv_reclaim_dirty_bitmap_locked(s->bs, s->bitmap, &error_abort); + bdrv_reclaim_dirty_bitmap_locked(s->bitmap, &error_abort); bdrv_enable_dirty_bitmap_locked(s->bitmap); } else { /* target not started, successor must be empty */ int64_t count = bdrv_get_dirty_count(s->bitmap); - BdrvDirtyBitmap *ret = bdrv_reclaim_dirty_bitmap_locked(s->bs, - s->bitmap, + BdrvDirtyBitmap *ret = bdrv_reclaim_dirty_bitmap_locked(s->bitmap, NULL); /* bdrv_reclaim_dirty_bitmap can fail only on no successor (it * must be) or on merge fail, but merge can't fail when second diff --git a/migration/block.c b/migration/block.c index 8e49382070..c90288ed29 100644 --- a/migration/block.c +++ b/migration/block.c @@ -361,7 +361,7 @@ static int set_dirty_tracking(void) fail: QSIMPLEQ_FOREACH(bmds, &block_mig_state.bmds_list, entry) { if (bmds->dirty_bitmap) { - bdrv_release_dirty_bitmap(blk_bs(bmds->blk), bmds->dirty_bitmap); + bdrv_release_dirty_bitmap(bmds->dirty_bitmap); } } return ret; @@ -374,7 +374,7 @@ static void unset_dirty_tracking(void) BlkMigDevState *bmds; QSIMPLEQ_FOREACH(bmds, &block_mig_state.bmds_list, entry) { - bdrv_release_dirty_bitmap(blk_bs(bmds->blk), bmds->dirty_bitmap); + bdrv_release_dirty_bitmap(bmds->dirty_bitmap); } } diff --git a/qapi/block-core.json b/qapi/block-core.json index f66553aac7..b274aef713 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -2052,10 +2052,6 @@ # Qcow2 disks support persistent bitmaps. Default is false for # block-dirty-bitmap-add. (Since: 2.10) # -# @autoload: ignored and deprecated since 2.12. -# Currently, all dirty tracking bitmaps are loaded from Qcow2 on -# open. -# # @disabled: the bitmap is created in the disabled state, which means that # it will not track drive changes. The bitmap may be enabled with # block-dirty-bitmap-enable. Default is false. (Since: 4.0) @@ -2064,7 +2060,7 @@ ## { 'struct': 'BlockDirtyBitmapAdd', 'data': { 'node': 'str', 'name': 'str', '*granularity': 'uint32', - '*persistent': 'bool', '*autoload': 'bool', '*disabled': 'bool' } } + '*persistent': 'bool', '*disabled': 'bool' } } ## # @BlockDirtyBitmapMergeSource: diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi index 01245e0b1c..7239e0959d 100644 --- a/qemu-deprecated.texi +++ b/qemu-deprecated.texi @@ -149,11 +149,6 @@ QEMU 4.1 has three options, please migrate to one of these three: @section QEMU Machine Protocol (QMP) commands -@subsection block-dirty-bitmap-add "autoload" parameter (since 2.12.0) - -"autoload" parameter is now ignored. All bitmaps are automatically loaded -from qcow2 images. - @subsection query-block result field dirty-bitmaps[i].status (since 4.0) The ``status'' field of the ``BlockDirtyInfo'' structure, returned by @@ -356,3 +351,18 @@ existing CPU models. Management software that needs runnability guarantees must resolve the CPU model aliases using te ``alias-of'' field returned by the ``query-cpu-definitions'' QMP command. + + +@node Recently removed features +@appendix Recently removed features + +What follows is a record of recently removed, formerly deprecated +features that serves as a record for users who have encountered +trouble after a recent upgrade. + +@section QEMU Machine Protocol (QMP) commands + +@subsection block-dirty-bitmap-add "autoload" parameter (since 4.2.0) + +The "autoload" parameter has been ignored since 2.12.0. All bitmaps +are automatically loaded from qcow2 images. diff --git a/tests/qemu-iotests/165 b/tests/qemu-iotests/165 index 5650dc7c87..951ea011a2 100755 --- a/tests/qemu-iotests/165 +++ b/tests/qemu-iotests/165 @@ -43,10 +43,10 @@ class TestPersistentDirtyBitmap(iotests.QMPTestCase): os.remove(disk) def mkVm(self): - return iotests.VM().add_drive(disk) + return iotests.VM().add_drive(disk, opts='node-name=node0') def mkVmRo(self): - return iotests.VM().add_drive(disk, opts='readonly=on') + return iotests.VM().add_drive(disk, opts='readonly=on,node-name=node0') def getSha256(self): result = self.vm.qmp('x-debug-block-dirty-bitmap-sha256', @@ -102,6 +102,59 @@ class TestPersistentDirtyBitmap(iotests.QMPTestCase): self.vm.shutdown() + def test_reopen_rw(self): + self.vm = self.mkVm() + self.vm.launch() + self.qmpAddBitmap() + + # Calculate hashes + + self.writeRegions(regions1) + sha256_1 = self.getSha256() + + self.writeRegions(regions2) + sha256_2 = self.getSha256() + assert sha256_1 != sha256_2 # Otherwise, it's not very interesting. + + result = self.vm.qmp('block-dirty-bitmap-clear', node='drive0', + name='bitmap0') + self.assert_qmp(result, 'return', {}) + + # Start with regions1 + + self.writeRegions(regions1) + assert sha256_1 == self.getSha256() + + self.vm.shutdown() + + self.vm = self.mkVmRo() + self.vm.launch() + + assert sha256_1 == self.getSha256() + + # Check that we are in RO mode and can't modify bitmap. + self.writeRegions(regions2) + assert sha256_1 == self.getSha256() + + # Reopen to RW + result = self.vm.qmp('x-blockdev-reopen', **{ + 'node-name': 'node0', + 'driver': iotests.imgfmt, + 'file': { + 'driver': 'file', + 'filename': disk + }, + 'read-only': False + }) + self.assert_qmp(result, 'return', {}) + + # Check that bitmap is reopened to RW and we can write to it. + self.writeRegions(regions2) + assert sha256_2 == self.getSha256() + + self.vm.shutdown() + + if __name__ == '__main__': iotests.main(supported_fmts=['qcow2'], supported_protocols=['file']) diff --git a/tests/qemu-iotests/165.out b/tests/qemu-iotests/165.out index ae1213e6f8..fbc63e62f8 100644 --- a/tests/qemu-iotests/165.out +++ b/tests/qemu-iotests/165.out @@ -1,5 +1,5 @@ -. +.. ---------------------------------------------------------------------- -Ran 1 tests +Ran 2 tests OK diff --git a/tests/qemu-iotests/260 b/tests/qemu-iotests/260 new file mode 100755 index 0000000000..4f6082c9d2 --- /dev/null +++ b/tests/qemu-iotests/260 @@ -0,0 +1,89 @@ +#!/usr/bin/env python +# +# Tests for temporary external snapshot when we have bitmaps. +# +# Copyright (c) 2019 Virtuozzo International GmbH. All rights reserved. +# +# 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 iotests +from iotests import qemu_img_create, file_path, log, filter_qmp_event + +iotests.verify_image_format(supported_fmts=['qcow2']) + +base, top = file_path('base', 'top') +size = 64 * 1024 * 3 + + +def print_bitmap(msg, vm): + result = vm.qmp('query-block')['return'][0] + if 'dirty-bitmaps' in result: + bitmap = result['dirty-bitmaps'][0] + log('{}: name={} dirty-clusters={}'.format(msg, bitmap['name'], + bitmap['count'] // 64 // 1024)) + else: + log(msg + ': not found') + + +def test(persistent, restart): + assert persistent or not restart + log("\nTestcase {}persistent {} restart\n".format( + '' if persistent else 'non-', 'with' if restart else 'without')) + + qemu_img_create('-f', iotests.imgfmt, base, str(size)) + + vm = iotests.VM().add_drive(base) + vm.launch() + + vm.qmp_log('block-dirty-bitmap-add', node='drive0', name='bitmap0', + persistent=persistent) + vm.hmp_qemu_io('drive0', 'write 0 64K') + print_bitmap('initial bitmap', vm) + + vm.qmp_log('blockdev-snapshot-sync', device='drive0', snapshot_file=top, + format=iotests.imgfmt, filters=[iotests.filter_qmp_testfiles]) + vm.hmp_qemu_io('drive0', 'write 64K 512') + print_bitmap('check that no bitmaps are in snapshot', vm) + + if restart: + log("... Restart ...") + vm.shutdown() + vm = iotests.VM().add_drive(top) + vm.launch() + + vm.qmp_log('block-commit', device='drive0', top=top, + filters=[iotests.filter_qmp_testfiles]) + ev = vm.events_wait((('BLOCK_JOB_READY', None), + ('BLOCK_JOB_COMPLETED', None))) + log(filter_qmp_event(ev)) + if (ev['event'] == 'BLOCK_JOB_COMPLETED'): + vm.shutdown() + log(vm.get_log()) + exit() + + vm.qmp_log('block-job-complete', device='drive0') + ev = vm.event_wait('BLOCK_JOB_COMPLETED') + log(filter_qmp_event(ev)) + print_bitmap('check bitmap after commit', vm) + + vm.hmp_qemu_io('drive0', 'write 128K 64K') + print_bitmap('check updated bitmap', vm) + + vm.shutdown() + + +test(persistent=False, restart=False) +test(persistent=True, restart=False) +test(persistent=True, restart=True) diff --git a/tests/qemu-iotests/260.out b/tests/qemu-iotests/260.out new file mode 100644 index 0000000000..2f0d98d036 --- /dev/null +++ b/tests/qemu-iotests/260.out @@ -0,0 +1,52 @@ + +Testcase non-persistent without restart + +{"execute": "block-dirty-bitmap-add", "arguments": {"name": "bitmap0", "node": "drive0", "persistent": false}} +{"return": {}} +initial bitmap: name=bitmap0 dirty-clusters=1 +{"execute": "blockdev-snapshot-sync", "arguments": {"device": "drive0", "format": "qcow2", "snapshot-file": "TEST_DIR/PID-top"}} +{"return": {}} +check that no bitmaps are in snapshot: not found +{"execute": "block-commit", "arguments": {"device": "drive0", "top": "TEST_DIR/PID-top"}} +{"return": {}} +{"data": {"device": "drive0", "len": 65536, "offset": 65536, "speed": 0, "type": "commit"}, "event": "BLOCK_JOB_READY", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}} +{"execute": "block-job-complete", "arguments": {"device": "drive0"}} +{"return": {}} +{"data": {"device": "drive0", "len": 65536, "offset": 65536, "speed": 0, "type": "commit"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}} +check bitmap after commit: name=bitmap0 dirty-clusters=2 +check updated bitmap: name=bitmap0 dirty-clusters=3 + +Testcase persistent without restart + +{"execute": "block-dirty-bitmap-add", "arguments": {"name": "bitmap0", "node": "drive0", "persistent": true}} +{"return": {}} +initial bitmap: name=bitmap0 dirty-clusters=1 +{"execute": "blockdev-snapshot-sync", "arguments": {"device": "drive0", "format": "qcow2", "snapshot-file": "TEST_DIR/PID-top"}} +{"return": {}} +check that no bitmaps are in snapshot: not found +{"execute": "block-commit", "arguments": {"device": "drive0", "top": "TEST_DIR/PID-top"}} +{"return": {}} +{"data": {"device": "drive0", "len": 65536, "offset": 65536, "speed": 0, "type": "commit"}, "event": "BLOCK_JOB_READY", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}} +{"execute": "block-job-complete", "arguments": {"device": "drive0"}} +{"return": {}} +{"data": {"device": "drive0", "len": 65536, "offset": 65536, "speed": 0, "type": "commit"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}} +check bitmap after commit: name=bitmap0 dirty-clusters=2 +check updated bitmap: name=bitmap0 dirty-clusters=3 + +Testcase persistent with restart + +{"execute": "block-dirty-bitmap-add", "arguments": {"name": "bitmap0", "node": "drive0", "persistent": true}} +{"return": {}} +initial bitmap: name=bitmap0 dirty-clusters=1 +{"execute": "blockdev-snapshot-sync", "arguments": {"device": "drive0", "format": "qcow2", "snapshot-file": "TEST_DIR/PID-top"}} +{"return": {}} +check that no bitmaps are in snapshot: not found +... Restart ... +{"execute": "block-commit", "arguments": {"device": "drive0", "top": "TEST_DIR/PID-top"}} +{"return": {}} +{"data": {"device": "drive0", "len": 65536, "offset": 65536, "speed": 0, "type": "commit"}, "event": "BLOCK_JOB_READY", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}} +{"execute": "block-job-complete", "arguments": {"device": "drive0"}} +{"return": {}} +{"data": {"device": "drive0", "len": 65536, "offset": 65536, "speed": 0, "type": "commit"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}} +check bitmap after commit: name=bitmap0 dirty-clusters=2 +check updated bitmap: name=bitmap0 dirty-clusters=3 diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index 7dac79a783..a73df279e5 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -273,6 +273,7 @@ 256 rw quick 257 rw 258 rw quick +260 rw quick 262 rw quick migration 263 rw quick 265 rw auto quick diff --git a/tests/test-hbitmap.c b/tests/test-hbitmap.c index eed5d288cb..e1f867085f 100644 --- a/tests/test-hbitmap.c +++ b/tests/test-hbitmap.c @@ -423,7 +423,7 @@ static void test_hbitmap_granularity(TestHBitmapData *data, hbitmap_test_check(data, 0); hbitmap_test_set(data, 0, 3); g_assert_cmpint(hbitmap_count(data->hb), ==, 4); - hbitmap_test_reset(data, 0, 1); + hbitmap_test_reset(data, 0, 2); g_assert_cmpint(hbitmap_count(data->hb), ==, 2); } diff --git a/util/hbitmap.c b/util/hbitmap.c index fd44c897ab..66db87c6ff 100644 --- a/util/hbitmap.c +++ b/util/hbitmap.c @@ -476,6 +476,10 @@ void hbitmap_reset(HBitmap *hb, uint64_t start, uint64_t count) /* Compute range in the last layer. */ uint64_t first; uint64_t last = start + count - 1; + uint64_t gran = 1ULL << hb->granularity; + + assert(QEMU_IS_ALIGNED(start, gran)); + assert(QEMU_IS_ALIGNED(count, gran) || (start + count == hb->orig_size)); trace_hbitmap_reset(hb, start, count, start >> hb->granularity, last >> hb->granularity); |