aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--block.c11
-rw-r--r--block/dirty-bitmap.c79
-rw-r--r--block/qcow2-bitmap.c16
-rw-r--r--block/qcow2.c86
-rw-r--r--block/qcow2.h1
-rw-r--r--blockdev.c144
-rw-r--r--include/block/block_int.h2
-rw-r--r--include/block/dirty-bitmap.h5
-rw-r--r--include/qemu/hbitmap.h25
-rw-r--r--migration/block-dirty-bitmap.c12
-rw-r--r--nbd/server.c4
-rw-r--r--qapi/block-core.json7
-rw-r--r--qapi/transaction.json2
-rwxr-xr-xtests/qemu-iotests/16970
-rw-r--r--tests/qemu-iotests/169.out4
-rw-r--r--util/hbitmap.c14
16 files changed, 344 insertions, 138 deletions
diff --git a/block.c b/block.c
index 08d64cdc61..95d8635aa1 100644
--- a/block.c
+++ b/block.c
@@ -4403,6 +4403,7 @@ static void coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs,
uint64_t perm, shared_perm;
Error *local_err = NULL;
int ret;
+ BdrvDirtyBitmap *bm;
if (!bs->drv) {
return;
@@ -4452,6 +4453,12 @@ 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))
+ {
+ bdrv_dirty_bitmap_set_migration(bm, false);
+ }
+
ret = refresh_total_sectors(bs, bs->total_sectors);
if (ret < 0) {
bs->open_flags |= BDRV_O_INACTIVE;
@@ -4566,10 +4573,6 @@ static int bdrv_inactivate_recurse(BlockDriverState *bs,
}
}
- /* At this point persistent bitmaps should be already stored by the format
- * driver */
- bdrv_release_persistent_dirty_bitmaps(bs);
-
return 0;
}
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index c9b8a6fd52..89fd1d7f8b 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -55,6 +55,10 @@ struct BdrvDirtyBitmap {
and this bitmap must remain unchanged while
this flag is set. */
bool persistent; /* bitmap must be saved to owner disk image */
+ bool migration; /* Bitmap is selected for migration, it should
+ not be stored on the next inactivation
+ (persistent flag doesn't matter until next
+ invalidation).*/
QLIST_ENTRY(BdrvDirtyBitmap) list;
};
@@ -176,6 +180,12 @@ bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap)
return bitmap->successor;
}
+/* Both conditions disallow user-modification via QMP. */
+bool bdrv_dirty_bitmap_user_locked(BdrvDirtyBitmap *bitmap) {
+ return bdrv_dirty_bitmap_frozen(bitmap) ||
+ bdrv_dirty_bitmap_qmp_locked(bitmap);
+}
+
void bdrv_dirty_bitmap_set_qmp_locked(BdrvDirtyBitmap *bitmap, bool qmp_locked)
{
qemu_mutex_lock(bitmap->mutex);
@@ -314,7 +324,7 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BlockDriverState *bs,
return NULL;
}
- if (!hbitmap_merge(parent->bitmap, successor->bitmap)) {
+ if (!hbitmap_merge(parent->bitmap, successor->bitmap, parent->bitmap)) {
error_setg(errp, "Merging of parent and successor bitmap failed");
return NULL;
}
@@ -384,26 +394,6 @@ void bdrv_release_named_dirty_bitmaps(BlockDriverState *bs)
}
/**
- * Release all persistent dirty bitmaps attached to a BDS (for use in
- * bdrv_inactivate_recurse()).
- * There must not be any frozen bitmaps attached.
- * This function does not remove persistent bitmaps from the storage.
- * Called with BQL taken.
- */
-void bdrv_release_persistent_dirty_bitmaps(BlockDriverState *bs)
-{
- BdrvDirtyBitmap *bm, *next;
-
- bdrv_dirty_bitmaps_lock(bs);
- QLIST_FOREACH_SAFE(bm, &bs->dirty_bitmaps, list, next) {
- if (bdrv_dirty_bitmap_get_persistance(bm)) {
- bdrv_release_dirty_bitmap_locked(bm);
- }
- }
- bdrv_dirty_bitmaps_unlock(bs);
-}
-
-/**
* Remove persistent dirty bitmap from the storage if it exists.
* Absence of bitmap is not an error, because we have the following scenario:
* BdrvDirtyBitmap can have .persistent = true but not yet saved and have no
@@ -619,7 +609,6 @@ void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap **out)
{
- assert(bdrv_dirty_bitmap_enabled(bitmap));
assert(!bdrv_dirty_bitmap_readonly(bitmap));
bdrv_dirty_bitmap_lock(bitmap);
if (!out) {
@@ -633,12 +622,12 @@ void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap **out)
bdrv_dirty_bitmap_unlock(bitmap);
}
-void bdrv_undo_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap *in)
+void bdrv_restore_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap *backup)
{
HBitmap *tmp = bitmap->bitmap;
assert(bdrv_dirty_bitmap_enabled(bitmap));
assert(!bdrv_dirty_bitmap_readonly(bitmap));
- bitmap->bitmap = in;
+ bitmap->bitmap = backup;
hbitmap_free(tmp);
}
@@ -756,16 +745,24 @@ void bdrv_dirty_bitmap_set_persistance(BdrvDirtyBitmap *bitmap, bool persistent)
qemu_mutex_unlock(bitmap->mutex);
}
+/* Called with BQL taken. */
+void bdrv_dirty_bitmap_set_migration(BdrvDirtyBitmap *bitmap, bool migration)
+{
+ qemu_mutex_lock(bitmap->mutex);
+ bitmap->migration = migration;
+ qemu_mutex_unlock(bitmap->mutex);
+}
+
bool bdrv_dirty_bitmap_get_persistance(BdrvDirtyBitmap *bitmap)
{
- return bitmap->persistent;
+ return bitmap->persistent && !bitmap->migration;
}
bool bdrv_has_changed_persistent_bitmaps(BlockDriverState *bs)
{
BdrvDirtyBitmap *bm;
QLIST_FOREACH(bm, &bs->dirty_bitmaps, list) {
- if (bm->persistent && !bm->readonly) {
+ if (bm->persistent && !bm->readonly && !bm->migration) {
return true;
}
}
@@ -791,19 +788,41 @@ int64_t bdrv_dirty_bitmap_next_zero(BdrvDirtyBitmap *bitmap, uint64_t offset)
}
void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src,
- Error **errp)
+ HBitmap **backup, Error **errp)
{
+ bool ret;
+
/* only bitmaps from one bds are supported */
assert(dest->mutex == src->mutex);
qemu_mutex_lock(dest->mutex);
- assert(bdrv_dirty_bitmap_enabled(dest));
- assert(!bdrv_dirty_bitmap_readonly(dest));
+ if (bdrv_dirty_bitmap_user_locked(dest)) {
+ error_setg(errp, "Bitmap '%s' is currently in use by another"
+ " operation and cannot be modified", dest->name);
+ goto out;
+ }
+
+ if (bdrv_dirty_bitmap_readonly(dest)) {
+ error_setg(errp, "Bitmap '%s' is readonly and cannot be modified",
+ dest->name);
+ goto out;
+ }
- if (!hbitmap_merge(dest->bitmap, src->bitmap)) {
+ if (!hbitmap_can_merge(dest->bitmap, src->bitmap)) {
error_setg(errp, "Bitmaps are incompatible and can't be merged");
+ goto out;
+ }
+
+ if (backup) {
+ *backup = dest->bitmap;
+ dest->bitmap = hbitmap_alloc(dest->size, hbitmap_granularity(*backup));
+ ret = hbitmap_merge(*backup, src->bitmap, dest->bitmap);
+ } else {
+ ret = hbitmap_merge(dest->bitmap, src->bitmap, dest->bitmap);
}
+ assert(ret);
+out:
qemu_mutex_unlock(dest->mutex);
}
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index ba978ad2aa..b5f1b3563d 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -1418,6 +1418,22 @@ 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;
+ }
+
+ bdrv_release_dirty_bitmap(bs, bm->dirty_bitmap);
+ }
+
bitmap_list_free(bm_list);
return;
diff --git a/block/qcow2.c b/block/qcow2.c
index 4f8d2fa7bd..30689b7688 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1153,7 +1153,6 @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
uint64_t ext_end;
uint64_t l1_vm_state_index;
bool update_header = false;
- bool header_updated = false;
ret = bdrv_pread(bs->file, 0, &header, sizeof(header));
if (ret < 0) {
@@ -1492,23 +1491,70 @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
s->autoclear_features &= QCOW2_AUTOCLEAR_MASK;
}
- if (s->dirty_bitmaps_loaded) {
- /* It's some kind of reopen. There are no known cases where we need to
- * reload bitmaps in such a situation, so it's safer to skip them.
- *
- * Moreover, if we have some readonly bitmaps and we are reopening for
- * rw we should reopen bitmaps correspondingly.
- */
- if (bdrv_has_readonly_bitmaps(bs) &&
- !bdrv_is_read_only(bs) && !(bdrv_get_flags(bs) & BDRV_O_INACTIVE))
- {
- qcow2_reopen_bitmaps_rw_hint(bs, &header_updated, &local_err);
- }
- } else {
- header_updated = qcow2_load_dirty_bitmaps(bs, &local_err);
- s->dirty_bitmaps_loaded = true;
+ /* == Handle persistent dirty bitmaps ==
+ *
+ * We want load dirty bitmaps in three cases:
+ *
+ * 1. Normal open of the disk in active mode, not related to invalidation
+ * after migration.
+ *
+ * 2. Invalidation of the target vm after pre-copy phase of migration, if
+ * bitmaps are _not_ migrating through migration channel, i.e.
+ * 'dirty-bitmaps' capability is disabled.
+ *
+ * 3. Invalidation of source vm after failed or canceled migration.
+ * This is a very interesting case. There are two possible types of
+ * bitmaps:
+ *
+ * A. Stored on inactivation and removed. They should be loaded from the
+ * image.
+ *
+ * B. Not stored: not-persistent bitmaps and bitmaps, migrated through
+ * the migration channel (with dirty-bitmaps capability).
+ *
+ * On the other hand, there are two possible sub-cases:
+ *
+ * 3.1 disk was changed by somebody else while were inactive. In this
+ * case all in-RAM dirty bitmaps (both persistent and not) are
+ * definitely invalid. And we don't have any method to determine
+ * this.
+ *
+ * Simple and safe thing is to just drop all the bitmaps of type B on
+ * inactivation. But in this case we lose bitmaps in valid 4.2 case.
+ *
+ * On the other hand, resuming source vm, if disk was already changed
+ * is a bad thing anyway: not only bitmaps, the whole vm state is
+ * out of sync with disk.
+ *
+ * This means, that user or management tool, who for some reason
+ * decided to resume source vm, after disk was already changed by
+ * target vm, should at least drop all dirty bitmaps by hand.
+ *
+ * So, we can ignore this case for now, but TODO: "generation"
+ * extension for qcow2, to determine, that image was changed after
+ * last inactivation. And if it is changed, we will drop (or at least
+ * mark as 'invalid' all the bitmaps of type B, both persistent
+ * and not).
+ *
+ * 3.2 disk was _not_ changed while were inactive. Bitmaps may be saved
+ * to disk ('dirty-bitmaps' capability disabled), or not saved
+ * ('dirty-bitmaps' capability enabled), but we don't need to care
+ * of: let's load bitmaps as always: stored bitmaps will be loaded,
+ * and not stored has flag IN_USE=1 in the image and will be skipped
+ * on loading.
+ *
+ * One remaining possible case when we don't want load bitmaps:
+ *
+ * 4. Open disk in inactive mode in target vm (bitmaps are migrating or
+ * will be loaded on invalidation, no needs try loading them before)
+ */
+
+ if (!(bdrv_get_flags(bs) & BDRV_O_INACTIVE)) {
+ /* It's case 1, 2 or 3.2. Or 3.1 which is BUG in management layer. */
+ bool header_updated = qcow2_load_dirty_bitmaps(bs, &local_err);
+
+ update_header = update_header && !header_updated;
}
- update_header = update_header && !header_updated;
if (local_err != NULL) {
error_propagate(errp, local_err);
ret = -EINVAL;
@@ -2123,9 +2169,9 @@ static int qcow2_inactivate(BlockDriverState *bs)
qcow2_store_persistent_dirty_bitmaps(bs, &local_err);
if (local_err != NULL) {
result = -EINVAL;
- error_report_err(local_err);
- error_report("Persistent bitmaps are lost for node '%s'",
- bdrv_get_device_or_node_name(bs));
+ error_reportf_err(local_err, "Lost persistent bitmaps during "
+ "inactivation of node '%s': ",
+ bdrv_get_device_or_node_name(bs));
}
ret = qcow2_cache_flush(bs, s->l2_table_cache);
diff --git a/block/qcow2.h b/block/qcow2.h
index ba430316b9..29c98d87a0 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -300,7 +300,6 @@ typedef struct BDRVQcow2State {
uint32_t nb_bitmaps;
uint64_t bitmap_directory_size;
uint64_t bitmap_directory_offset;
- bool dirty_bitmaps_loaded;
int flags;
int qcow_version;
diff --git a/blockdev.c b/blockdev.c
index 574adbcb7f..c30495d035 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2010,14 +2010,8 @@ static void block_dirty_bitmap_clear_prepare(BlkActionState *common,
return;
}
- if (bdrv_dirty_bitmap_frozen(state->bitmap)) {
- error_setg(errp, "Cannot modify a frozen bitmap");
- return;
- } else if (bdrv_dirty_bitmap_qmp_locked(state->bitmap)) {
- error_setg(errp, "Cannot modify a locked bitmap");
- return;
- } else if (!bdrv_dirty_bitmap_enabled(state->bitmap)) {
- error_setg(errp, "Cannot clear a disabled bitmap");
+ if (bdrv_dirty_bitmap_user_locked(state->bitmap)) {
+ error_setg(errp, "Cannot modify a bitmap in use by another operation");
return;
} else if (bdrv_dirty_bitmap_readonly(state->bitmap)) {
error_setg(errp, "Cannot clear a readonly bitmap");
@@ -2027,17 +2021,17 @@ static void block_dirty_bitmap_clear_prepare(BlkActionState *common,
bdrv_clear_dirty_bitmap(state->bitmap, &state->backup);
}
-static void block_dirty_bitmap_clear_abort(BlkActionState *common)
+static void block_dirty_bitmap_restore(BlkActionState *common)
{
BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
common, common);
if (state->backup) {
- bdrv_undo_clear_dirty_bitmap(state->bitmap, state->backup);
+ bdrv_restore_dirty_bitmap(state->bitmap, state->backup);
}
}
-static void block_dirty_bitmap_clear_commit(BlkActionState *common)
+static void block_dirty_bitmap_free_backup(BlkActionState *common)
{
BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
common, common);
@@ -2065,6 +2059,13 @@ static void block_dirty_bitmap_enable_prepare(BlkActionState *common,
return;
}
+ if (bdrv_dirty_bitmap_user_locked(state->bitmap)) {
+ error_setg(errp,
+ "Bitmap '%s' is currently in use by another operation"
+ " and cannot be enabled", action->name);
+ return;
+ }
+
state->was_enabled = bdrv_dirty_bitmap_enabled(state->bitmap);
bdrv_enable_dirty_bitmap(state->bitmap);
}
@@ -2099,6 +2100,13 @@ static void block_dirty_bitmap_disable_prepare(BlkActionState *common,
return;
}
+ if (bdrv_dirty_bitmap_user_locked(state->bitmap)) {
+ error_setg(errp,
+ "Bitmap '%s' is currently in use by another operation"
+ " and cannot be disabled", action->name);
+ return;
+ }
+
state->was_enabled = bdrv_dirty_bitmap_enabled(state->bitmap);
bdrv_disable_dirty_bitmap(state->bitmap);
}
@@ -2113,6 +2121,35 @@ static void block_dirty_bitmap_disable_abort(BlkActionState *common)
}
}
+static void block_dirty_bitmap_merge_prepare(BlkActionState *common,
+ Error **errp)
+{
+ BlockDirtyBitmapMerge *action;
+ BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+ common, common);
+ BdrvDirtyBitmap *merge_source;
+
+ if (action_check_completion_mode(common, errp) < 0) {
+ return;
+ }
+
+ action = common->action->u.x_block_dirty_bitmap_merge.data;
+ state->bitmap = block_dirty_bitmap_lookup(action->node,
+ action->dst_name,
+ &state->bs,
+ errp);
+ if (!state->bitmap) {
+ return;
+ }
+
+ merge_source = bdrv_find_dirty_bitmap(state->bs, action->src_name);
+ if (!merge_source) {
+ return;
+ }
+
+ bdrv_merge_dirty_bitmap(state->bitmap, merge_source, &state->backup, errp);
+}
+
static void abort_prepare(BlkActionState *common, Error **errp)
{
error_setg(errp, "Transaction aborted using Abort action");
@@ -2171,8 +2208,8 @@ static const BlkActionOps actions[] = {
[TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_CLEAR] = {
.instance_size = sizeof(BlockDirtyBitmapState),
.prepare = block_dirty_bitmap_clear_prepare,
- .commit = block_dirty_bitmap_clear_commit,
- .abort = block_dirty_bitmap_clear_abort,
+ .commit = block_dirty_bitmap_free_backup,
+ .abort = block_dirty_bitmap_restore,
},
[TRANSACTION_ACTION_KIND_X_BLOCK_DIRTY_BITMAP_ENABLE] = {
.instance_size = sizeof(BlockDirtyBitmapState),
@@ -2184,6 +2221,12 @@ static const BlkActionOps actions[] = {
.prepare = block_dirty_bitmap_disable_prepare,
.abort = block_dirty_bitmap_disable_abort,
},
+ [TRANSACTION_ACTION_KIND_X_BLOCK_DIRTY_BITMAP_MERGE] = {
+ .instance_size = sizeof(BlockDirtyBitmapState),
+ .prepare = block_dirty_bitmap_merge_prepare,
+ .commit = block_dirty_bitmap_free_backup,
+ .abort = block_dirty_bitmap_restore,
+ },
/* Where are transactions for MIRROR, COMMIT and STREAM?
* Although these blockjobs use transaction callbacks like the backup job,
* these jobs do not necessarily adhere to transaction semantics.
@@ -2848,15 +2891,10 @@ void qmp_block_dirty_bitmap_remove(const char *node, const char *name,
return;
}
- if (bdrv_dirty_bitmap_frozen(bitmap)) {
+ if (bdrv_dirty_bitmap_user_locked(bitmap)) {
error_setg(errp,
- "Bitmap '%s' is currently frozen and cannot be removed",
- name);
- return;
- } else if (bdrv_dirty_bitmap_qmp_locked(bitmap)) {
- error_setg(errp,
- "Bitmap '%s' is currently locked and cannot be removed",
- name);
+ "Bitmap '%s' is currently in use by another operation and"
+ " cannot be removed", name);
return;
}
@@ -2886,20 +2924,10 @@ void qmp_block_dirty_bitmap_clear(const char *node, const char *name,
return;
}
- if (bdrv_dirty_bitmap_frozen(bitmap)) {
- error_setg(errp,
- "Bitmap '%s' is currently frozen and cannot be modified",
- name);
- return;
- } else if (bdrv_dirty_bitmap_qmp_locked(bitmap)) {
- error_setg(errp,
- "Bitmap '%s' is currently locked and cannot be modified",
- name);
- return;
- } else if (!bdrv_dirty_bitmap_enabled(bitmap)) {
+ if (bdrv_dirty_bitmap_user_locked(bitmap)) {
error_setg(errp,
- "Bitmap '%s' is currently disabled and cannot be cleared",
- name);
+ "Bitmap '%s' is currently in use by another operation"
+ " and cannot be cleared", name);
return;
} else if (bdrv_dirty_bitmap_readonly(bitmap)) {
error_setg(errp, "Bitmap '%s' is readonly and cannot be cleared", name);
@@ -2920,10 +2948,10 @@ void qmp_x_block_dirty_bitmap_enable(const char *node, const char *name,
return;
}
- if (bdrv_dirty_bitmap_frozen(bitmap)) {
+ if (bdrv_dirty_bitmap_user_locked(bitmap)) {
error_setg(errp,
- "Bitmap '%s' is currently frozen and cannot be enabled",
- name);
+ "Bitmap '%s' is currently in use by another operation"
+ " and cannot be enabled", name);
return;
}
@@ -2941,10 +2969,10 @@ void qmp_x_block_dirty_bitmap_disable(const char *node, const char *name,
return;
}
- if (bdrv_dirty_bitmap_frozen(bitmap)) {
+ if (bdrv_dirty_bitmap_user_locked(bitmap)) {
error_setg(errp,
- "Bitmap '%s' is currently frozen and cannot be disabled",
- name);
+ "Bitmap '%s' is currently in use by another operation"
+ " and cannot be disabled", name);
return;
}
@@ -2962,23 +2990,13 @@ void qmp_x_block_dirty_bitmap_merge(const char *node, const char *dst_name,
return;
}
- if (bdrv_dirty_bitmap_frozen(dst)) {
- error_setg(errp, "Bitmap '%s' is frozen and cannot be modified",
- dst_name);
- return;
- } else if (bdrv_dirty_bitmap_readonly(dst)) {
- error_setg(errp, "Bitmap '%s' is readonly and cannot be modified",
- dst_name);
- return;
- }
-
src = bdrv_find_dirty_bitmap(bs, src_name);
if (!src) {
error_setg(errp, "Dirty bitmap '%s' not found", src_name);
return;
}
- bdrv_merge_dirty_bitmap(dst, src, errp);
+ bdrv_merge_dirty_bitmap(dst, src, NULL, errp);
}
BlockDirtyBitmapSha256 *qmp_x_debug_block_dirty_bitmap_sha256(const char *node,
@@ -3495,10 +3513,10 @@ static BlockJob *do_drive_backup(DriveBackup *backup, JobTxn *txn,
bdrv_unref(target_bs);
goto out;
}
- if (bdrv_dirty_bitmap_qmp_locked(bmap)) {
+ if (bdrv_dirty_bitmap_user_locked(bmap)) {
error_setg(errp,
- "Bitmap '%s' is currently locked and cannot be used for "
- "backup", backup->bitmap);
+ "Bitmap '%s' is currently in use by another operation"
+ " and cannot be used for backup", backup->bitmap);
goto out;
}
}
@@ -3545,6 +3563,7 @@ BlockJob *do_blockdev_backup(BlockdevBackup *backup, JobTxn *txn,
BlockDriverState *bs;
BlockDriverState *target_bs;
Error *local_err = NULL;
+ BdrvDirtyBitmap *bmap = NULL;
AioContext *aio_context;
BlockJob *job = NULL;
int job_flags = JOB_DEFAULT;
@@ -3595,6 +3614,21 @@ BlockJob *do_blockdev_backup(BlockdevBackup *backup, JobTxn *txn,
goto out;
}
}
+
+ if (backup->has_bitmap) {
+ bmap = bdrv_find_dirty_bitmap(bs, backup->bitmap);
+ if (!bmap) {
+ error_setg(errp, "Bitmap '%s' could not be found", backup->bitmap);
+ goto out;
+ }
+ if (bdrv_dirty_bitmap_user_locked(bmap)) {
+ error_setg(errp,
+ "Bitmap '%s' is currently in use by another operation"
+ " and cannot be used for backup", backup->bitmap);
+ goto out;
+ }
+ }
+
if (!backup->auto_finalize) {
job_flags |= JOB_MANUAL_FINALIZE;
}
@@ -3602,7 +3636,7 @@ BlockJob *do_blockdev_backup(BlockdevBackup *backup, JobTxn *txn,
job_flags |= JOB_MANUAL_DISMISS;
}
job = backup_job_create(backup->job_id, bs, target_bs, backup->speed,
- backup->sync, NULL, backup->compress,
+ backup->sync, bmap, backup->compress,
backup->on_source_error, backup->on_target_error,
job_flags, NULL, NULL, txn, &local_err);
if (local_err != NULL) {
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 92ecbd866e..f605622216 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -1155,7 +1155,7 @@ bool blk_dev_is_medium_locked(BlockBackend *blk);
void bdrv_set_dirty(BlockDriverState *bs, int64_t offset, int64_t bytes);
void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap **out);
-void bdrv_undo_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap *in);
+void bdrv_restore_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap *backup);
void bdrv_inc_in_flight(BlockDriverState *bs);
void bdrv_dec_in_flight(BlockDriverState *bs);
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 259bd27c40..8f38a3dec1 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -26,7 +26,6 @@ BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
const char *name);
void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
void bdrv_release_named_dirty_bitmaps(BlockDriverState *bs);
-void bdrv_release_persistent_dirty_bitmaps(BlockDriverState *bs);
void bdrv_remove_persistent_dirty_bitmap(BlockDriverState *bs,
const char *name,
Error **errp);
@@ -71,7 +70,8 @@ void bdrv_dirty_bitmap_set_persistance(BdrvDirtyBitmap *bitmap,
bool persistent);
void bdrv_dirty_bitmap_set_qmp_locked(BdrvDirtyBitmap *bitmap, bool qmp_locked);
void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src,
- Error **errp);
+ HBitmap **backup, Error **errp);
+void bdrv_dirty_bitmap_set_migration(BdrvDirtyBitmap *bitmap, bool migration);
/* Functions that require manual locking. */
void bdrv_dirty_bitmap_lock(BdrvDirtyBitmap *bitmap);
@@ -94,6 +94,7 @@ bool bdrv_has_readonly_bitmaps(BlockDriverState *bs);
bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap);
bool bdrv_dirty_bitmap_get_persistance(BdrvDirtyBitmap *bitmap);
bool bdrv_dirty_bitmap_qmp_locked(BdrvDirtyBitmap *bitmap);
+bool bdrv_dirty_bitmap_user_locked(BdrvDirtyBitmap *bitmap);
bool bdrv_has_changed_persistent_bitmaps(BlockDriverState *bs);
BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BlockDriverState *bs,
BdrvDirtyBitmap *bitmap);
diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
index ddca52c48e..a7cb780592 100644
--- a/include/qemu/hbitmap.h
+++ b/include/qemu/hbitmap.h
@@ -73,16 +73,23 @@ void hbitmap_truncate(HBitmap *hb, uint64_t size);
/**
* hbitmap_merge:
- * @a: The bitmap to store the result in.
- * @b: The bitmap to merge into @a.
- * @return true if the merge was successful,
- * false if it was not attempted.
- *
- * Merge two bitmaps together.
- * A := A (BITOR) B.
- * B is left unmodified.
+ *
+ * Store result of merging @a and @b into @result.
+ * @result is allowed to be equal to @a or @b.
+ *
+ * Return true if the merge was successful,
+ * false if it was not attempted.
+ */
+bool hbitmap_merge(const HBitmap *a, const HBitmap *b, HBitmap *result);
+
+/**
+ * hbitmap_can_merge:
+ *
+ * hbitmap_can_merge(a, b) && hbitmap_can_merge(a, result) is sufficient and
+ * necessary for hbitmap_merge will not fail.
+ *
*/
-bool hbitmap_merge(HBitmap *a, const HBitmap *b);
+bool hbitmap_can_merge(const HBitmap *a, const HBitmap *b);
/**
* hbitmap_empty:
diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index 477826330c..5e90f44c2f 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -301,14 +301,14 @@ static int init_dirty_bitmap_migration(void)
goto fail;
}
- if (bdrv_dirty_bitmap_frozen(bitmap)) {
- error_report("Can't migrate frozen dirty bitmap: '%s",
+ if (bdrv_dirty_bitmap_user_locked(bitmap)) {
+ error_report("Can't migrate a bitmap that is in use by another operation: '%s'",
bdrv_dirty_bitmap_name(bitmap));
goto fail;
}
- if (bdrv_dirty_bitmap_qmp_locked(bitmap)) {
- error_report("Can't migrate locked dirty bitmap: '%s",
+ if (bdrv_dirty_bitmap_readonly(bitmap)) {
+ error_report("Can't migrate read-only dirty bitmap: '%s",
bdrv_dirty_bitmap_name(bitmap));
goto fail;
}
@@ -335,9 +335,9 @@ static int init_dirty_bitmap_migration(void)
}
}
- /* unset persistance here, to not roll back it */
+ /* unset migration flags here, to not roll back it */
QSIMPLEQ_FOREACH(dbms, &dirty_bitmap_mig_state.dbms_list, entry) {
- bdrv_dirty_bitmap_set_persistance(dbms->bitmap, false);
+ bdrv_dirty_bitmap_set_migration(dbms->bitmap, true);
}
if (QSIMPLEQ_EMPTY(&dirty_bitmap_mig_state.dbms_list)) {
diff --git a/nbd/server.c b/nbd/server.c
index a1eda0114f..4e8f5ae51b 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -2456,8 +2456,8 @@ void nbd_export_bitmap(NBDExport *exp, const char *bitmap,
return;
}
- if (bdrv_dirty_bitmap_qmp_locked(bm)) {
- error_setg(errp, "Bitmap '%s' is locked", bitmap);
+ if (bdrv_dirty_bitmap_user_locked(bm)) {
+ error_setg(errp, "Bitmap '%s' is in use", bitmap);
return;
}
diff --git a/qapi/block-core.json b/qapi/block-core.json
index cfb37f8c1d..0fc1590c1b 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1316,6 +1316,10 @@
# @speed: the maximum speed, in bytes per second. The default is 0,
# for unlimited.
#
+# @bitmap: the name of dirty bitmap if sync is "incremental".
+# Must be present if sync is "incremental", must NOT be present
+# otherwise. (Since 3.1)
+#
# @compress: true to compress data, if the target format supports it.
# (default: false) (since 2.8)
#
@@ -1348,7 +1352,8 @@
##
{ 'struct': 'BlockdevBackup',
'data': { '*job-id': 'str', 'device': 'str', 'target': 'str',
- 'sync': 'MirrorSyncMode', '*speed': 'int', '*compress': 'bool',
+ 'sync': 'MirrorSyncMode', '*speed': 'int',
+ '*bitmap': 'str', '*compress': 'bool',
'*on-source-error': 'BlockdevOnError',
'*on-target-error': 'BlockdevOnError',
'*auto-finalize': 'bool', '*auto-dismiss': 'bool' } }
diff --git a/qapi/transaction.json b/qapi/transaction.json
index d7e4274550..5875cdb16c 100644
--- a/qapi/transaction.json
+++ b/qapi/transaction.json
@@ -48,6 +48,7 @@
# - @block-dirty-bitmap-clear: since 2.5
# - @x-block-dirty-bitmap-enable: since 3.0
# - @x-block-dirty-bitmap-disable: since 3.0
+# - @x-block-dirty-bitmap-merge: since 3.1
# - @blockdev-backup: since 2.3
# - @blockdev-snapshot: since 2.5
# - @blockdev-snapshot-internal-sync: since 1.7
@@ -63,6 +64,7 @@
'block-dirty-bitmap-clear': 'BlockDirtyBitmap',
'x-block-dirty-bitmap-enable': 'BlockDirtyBitmap',
'x-block-dirty-bitmap-disable': 'BlockDirtyBitmap',
+ 'x-block-dirty-bitmap-merge': 'BlockDirtyBitmapMerge',
'blockdev-backup': 'BlockdevBackup',
'blockdev-snapshot': 'BlockdevSnapshot',
'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal',
diff --git a/tests/qemu-iotests/169 b/tests/qemu-iotests/169
index f243db9955..69850c4c67 100755
--- a/tests/qemu-iotests/169
+++ b/tests/qemu-iotests/169
@@ -24,6 +24,7 @@ import time
import itertools
import operator
import new
+import re
from iotests import qemu_img
@@ -58,7 +59,6 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
'granularity': granularity}
if persistent:
params['persistent'] = True
- params['autoload'] = True
result = vm.qmp('block-dirty-bitmap-add', **params)
self.assert_qmp(result, 'return', {});
@@ -77,6 +77,58 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
self.assert_qmp(result, 'error/desc',
"Dirty bitmap 'bitmap0' not found");
+ def do_test_migration_resume_source(self, persistent, migrate_bitmaps):
+ granularity = 512
+
+ # regions = ((start, count), ...)
+ regions = ((0, 0x10000),
+ (0xf0000, 0x10000),
+ (0xa0201, 0x1000))
+
+ mig_caps = [{'capability': 'events', 'state': True}]
+ if migrate_bitmaps:
+ mig_caps.append({'capability': 'dirty-bitmaps', 'state': True})
+
+ result = self.vm_a.qmp('migrate-set-capabilities',
+ capabilities=mig_caps)
+ self.assert_qmp(result, 'return', {})
+
+ self.add_bitmap(self.vm_a, granularity, persistent)
+ for r in regions:
+ self.vm_a.hmp_qemu_io('drive0', 'write %d %d' % r)
+ sha256 = self.get_bitmap_hash(self.vm_a)
+
+ result = self.vm_a.qmp('migrate', uri=mig_cmd)
+ while True:
+ event = self.vm_a.event_wait('MIGRATION')
+ if event['data']['status'] == 'completed':
+ break
+
+ # test that bitmap is still here
+ removed = (not migrate_bitmaps) and persistent
+ self.check_bitmap(self.vm_a, False if removed else sha256)
+
+ self.vm_a.qmp('cont')
+
+ # test that bitmap is still here after invalidation
+ self.check_bitmap(self.vm_a, sha256)
+
+ # shutdown and check that invalidation didn't fail
+ self.vm_a.shutdown()
+
+ # catch 'Could not reopen qcow2 layer: Bitmap already exists'
+ # possible error
+ log = self.vm_a.get_log()
+ log = re.sub(r'^\[I \d+\.\d+\] OPENED\n', '', log)
+ log = re.sub(r'^(wrote .* bytes at offset .*\n.*KiB.*ops.*sec.*\n){3}',
+ '', log)
+ log = re.sub(r'\[I \+\d+\.\d+\] CLOSED\n?$', '', log)
+ self.assertEqual(log, '')
+
+ # test that bitmap is still persistent
+ self.vm_a.launch()
+ self.check_bitmap(self.vm_a, sha256 if persistent else False)
+
def do_test_migration(self, persistent, migrate_bitmaps, online,
shared_storage):
granularity = 512
@@ -134,6 +186,14 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
if should_migrate:
self.vm_b.shutdown()
+
+ # catch 'Could not reopen qcow2 layer: Bitmap already exists'
+ # possible error
+ log = self.vm_b.get_log()
+ log = re.sub(r'^\[I \d+\.\d+\] OPENED\n', '', log)
+ log = re.sub(r'\[I \+\d+\.\d+\] CLOSED\n?$', '', log)
+ self.assertEqual(log, '')
+
# recreate vm_b, as we don't want -incoming option (this will lead
# to "cat" process left alive after test finish)
self.vm_b = iotests.VM(path_suffix='b')
@@ -144,7 +204,7 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
def inject_test_case(klass, name, method, *args, **kwargs):
mc = operator.methodcaller(method, *args, **kwargs)
- setattr(klass, 'test_' + name, new.instancemethod(mc, None, klass))
+ setattr(klass, 'test_' + method + name, new.instancemethod(mc, None, klass))
for cmb in list(itertools.product((True, False), repeat=4)):
name = ('_' if cmb[0] else '_not_') + 'persistent_'
@@ -155,6 +215,12 @@ for cmb in list(itertools.product((True, False), repeat=4)):
inject_test_case(TestDirtyBitmapMigration, name, 'do_test_migration',
*list(cmb))
+for cmb in list(itertools.product((True, False), repeat=2)):
+ name = ('_' if cmb[0] else '_not_') + 'persistent_'
+ name += ('_' if cmb[1] else '_not_') + 'migbitmap'
+
+ inject_test_case(TestDirtyBitmapMigration, name,
+ 'do_test_migration_resume_source', *list(cmb))
if __name__ == '__main__':
iotests.main(supported_fmts=['qcow2'])
diff --git a/tests/qemu-iotests/169.out b/tests/qemu-iotests/169.out
index b6f257674e..3a89159833 100644
--- a/tests/qemu-iotests/169.out
+++ b/tests/qemu-iotests/169.out
@@ -1,5 +1,5 @@
-................
+....................
----------------------------------------------------------------------
-Ran 16 tests
+Ran 20 tests
OK
diff --git a/util/hbitmap.c b/util/hbitmap.c
index bcd304041a..8d402c59d9 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -723,6 +723,10 @@ void hbitmap_truncate(HBitmap *hb, uint64_t size)
}
}
+bool hbitmap_can_merge(const HBitmap *a, const HBitmap *b)
+{
+ return (a->size == b->size) && (a->granularity == b->granularity);
+}
/**
* Given HBitmaps A and B, let A := A (BITOR) B.
@@ -731,14 +735,15 @@ void hbitmap_truncate(HBitmap *hb, uint64_t size)
* @return true if the merge was successful,
* false if it was not attempted.
*/
-bool hbitmap_merge(HBitmap *a, const HBitmap *b)
+bool hbitmap_merge(const HBitmap *a, const HBitmap *b, HBitmap *result)
{
int i;
uint64_t j;
- if ((a->size != b->size) || (a->granularity != b->granularity)) {
+ if (!hbitmap_can_merge(a, b) || !hbitmap_can_merge(a, result)) {
return false;
}
+ assert(hbitmap_can_merge(b, result));
if (hbitmap_count(b) == 0) {
return true;
@@ -750,10 +755,13 @@ bool hbitmap_merge(HBitmap *a, const HBitmap *b)
*/
for (i = HBITMAP_LEVELS - 1; i >= 0; i--) {
for (j = 0; j < a->sizes[i]; j++) {
- a->levels[i][j] |= b->levels[i][j];
+ result->levels[i][j] = a->levels[i][j] | b->levels[i][j];
}
}
+ /* Recompute the dirty count */
+ result->count = hb_count_between(result, 0, result->size - 1);
+
return true;
}