* [Qemu-devel] [PULL 00/13] Bitmaps patches
@ 2018-03-13 21:14 John Snow
2018-03-13 21:14 ` [Qemu-devel] [PULL 01/13] block/dirty-bitmap: add bdrv_dirty_bitmap_enable_successor() John Snow
` (14 more replies)
0 siblings, 15 replies; 24+ messages in thread
From: John Snow @ 2018-03-13 21:14 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, jsnow
The following changes since commit 026aaf47c02b79036feb830206cfebb2a726510d:
Merge remote-tracking branch 'remotes/ehabkost/tags/python-next-pull-request' into staging (2018-03-13 16:26:44 +0000)
are available in the Git repository at:
https://github.com/jnsnow/qemu.git tags/bitmaps-pull-request
for you to fetch changes up to ac8bd439bb7b5dffeb5ff8a17317ca2b192044b6:
iotests: add dirty bitmap postcopy test (2018-03-13 17:06:32 -0400)
----------------------------------------------------------------
PR touching migration sent with permission (and reviews) from
David Gilbert.
----------------------------------------------------------------
Vladimir Sementsov-Ogievskiy (13):
block/dirty-bitmap: add bdrv_dirty_bitmap_enable_successor()
block/dirty-bitmap: fix locking in bdrv_reclaim_dirty_bitmap
block/dirty-bitmap: add _locked version of bdrv_reclaim_dirty_bitmap
dirty-bitmap: add locked state
migration: introduce postcopy-only pending
qapi: add dirty-bitmaps migration capability
migration: include migrate_dirty_bitmaps in migrate_postcopy
migration/qemu-file: add qemu_put_counted_string()
migration: add is_active_iterate handler
migration: allow qmp command migrate-start-postcopy for any postcopy
migration: add postcopy migration of dirty bitmaps
iotests: add dirty bitmap migration test
iotests: add dirty bitmap postcopy test
block/dirty-bitmap.c | 123 +++++--
blockdev.c | 19 ++
hw/s390x/s390-stattrib.c | 7 +-
include/block/dirty-bitmap.h | 7 +
include/migration/misc.h | 3 +
include/migration/register.h | 26 +-
migration/Makefile.objs | 1 +
migration/block-dirty-bitmap.c | 746 +++++++++++++++++++++++++++++++++++++++++
migration/block.c | 7 +-
migration/migration.c | 30 +-
migration/migration.h | 4 +
migration/qemu-file.c | 13 +
migration/qemu-file.h | 2 +
migration/ram.c | 9 +-
migration/savevm.c | 20 +-
migration/savevm.h | 5 +-
migration/trace-events | 16 +-
qapi/block-core.json | 5 +-
qapi/migration.json | 6 +-
tests/qemu-iotests/169 | 156 +++++++++
tests/qemu-iotests/169.out | 5 +
tests/qemu-iotests/199 | 118 +++++++
tests/qemu-iotests/199.out | 5 +
tests/qemu-iotests/group | 2 +
tests/qemu-iotests/iotests.py | 7 +-
vl.c | 1 +
26 files changed, 1277 insertions(+), 66 deletions(-)
create mode 100644 migration/block-dirty-bitmap.c
create mode 100755 tests/qemu-iotests/169
create mode 100644 tests/qemu-iotests/169.out
create mode 100755 tests/qemu-iotests/199
create mode 100644 tests/qemu-iotests/199.out
--
2.14.3
^ permalink raw reply [flat|nested] 24+ messages in thread
* [Qemu-devel] [PULL 01/13] block/dirty-bitmap: add bdrv_dirty_bitmap_enable_successor()
2018-03-13 21:14 [Qemu-devel] [PULL 00/13] Bitmaps patches John Snow
@ 2018-03-13 21:14 ` John Snow
2018-03-13 21:14 ` [Qemu-devel] [PULL 02/13] block/dirty-bitmap: fix locking in bdrv_reclaim_dirty_bitmap John Snow
` (13 subsequent siblings)
14 siblings, 0 replies; 24+ messages in thread
From: John Snow @ 2018-03-13 21:14 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, jsnow, Vladimir Sementsov-Ogievskiy
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Enabling bitmap successor is necessary to enable successors of bitmaps
being migrated before target vm start.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Message-id: 20180207155837.92351-2-vsementsov@virtuozzo.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
block/dirty-bitmap.c | 8 ++++++++
include/block/dirty-bitmap.h | 1 +
2 files changed, 9 insertions(+)
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 909f0517f8..0d0e807216 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -234,6 +234,14 @@ int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
return 0;
}
+/* Called with BQL taken. */
+void bdrv_dirty_bitmap_enable_successor(BdrvDirtyBitmap *bitmap)
+{
+ qemu_mutex_lock(bitmap->mutex);
+ bdrv_enable_dirty_bitmap(bitmap->successor);
+ qemu_mutex_unlock(bitmap->mutex);
+}
+
/**
* For a bitmap with a successor, yield our name to the successor,
* delete the old bitmap, and return a handle to the new bitmap.
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 09efec609f..e0ebc96b01 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -21,6 +21,7 @@ BdrvDirtyBitmap *bdrv_dirty_bitmap_abdicate(BlockDriverState *bs,
BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
BdrvDirtyBitmap *bitmap,
Error **errp);
+void bdrv_dirty_bitmap_enable_successor(BdrvDirtyBitmap *bitmap);
BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
const char *name);
void bdrv_dirty_bitmap_make_anon(BdrvDirtyBitmap *bitmap);
--
2.14.3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PULL 02/13] block/dirty-bitmap: fix locking in bdrv_reclaim_dirty_bitmap
2018-03-13 21:14 [Qemu-devel] [PULL 00/13] Bitmaps patches John Snow
2018-03-13 21:14 ` [Qemu-devel] [PULL 01/13] block/dirty-bitmap: add bdrv_dirty_bitmap_enable_successor() John Snow
@ 2018-03-13 21:14 ` John Snow
2018-03-13 21:14 ` [Qemu-devel] [PULL 03/13] block/dirty-bitmap: add _locked version of bdrv_reclaim_dirty_bitmap John Snow
` (12 subsequent siblings)
14 siblings, 0 replies; 24+ messages in thread
From: John Snow @ 2018-03-13 21:14 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, jsnow, Vladimir Sementsov-Ogievskiy
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Like other setters here these functions should take a lock.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 20180207155837.92351-3-vsementsov@virtuozzo.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
block/dirty-bitmap.c | 85 ++++++++++++++++++++++++++++++++--------------------
1 file changed, 53 insertions(+), 32 deletions(-)
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 0d0e807216..75435f6c2f 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -242,6 +242,51 @@ void bdrv_dirty_bitmap_enable_successor(BdrvDirtyBitmap *bitmap)
qemu_mutex_unlock(bitmap->mutex);
}
+/* Called within bdrv_dirty_bitmap_lock..unlock */
+static void bdrv_do_release_matching_dirty_bitmap_locked(
+ BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
+ bool (*cond)(BdrvDirtyBitmap *bitmap))
+{
+ BdrvDirtyBitmap *bm, *next;
+
+ QLIST_FOREACH_SAFE(bm, &bs->dirty_bitmaps, list, next) {
+ if ((!bitmap || bm == bitmap) && (!cond || cond(bm))) {
+ assert(!bm->active_iterators);
+ assert(!bdrv_dirty_bitmap_frozen(bm));
+ assert(!bm->meta);
+ QLIST_REMOVE(bm, list);
+ hbitmap_free(bm->bitmap);
+ g_free(bm->name);
+ g_free(bm);
+
+ if (bitmap) {
+ return;
+ }
+ }
+ }
+
+ if (bitmap) {
+ abort();
+ }
+}
+
+/* Called with BQL taken. */
+static void bdrv_do_release_matching_dirty_bitmap(
+ BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
+ bool (*cond)(BdrvDirtyBitmap *bitmap))
+{
+ bdrv_dirty_bitmaps_lock(bs);
+ bdrv_do_release_matching_dirty_bitmap_locked(bs, bitmap, cond);
+ bdrv_dirty_bitmaps_unlock(bs);
+}
+
+/* Called within bdrv_dirty_bitmap_lock..unlock */
+static void bdrv_release_dirty_bitmap_locked(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap)
+{
+ bdrv_do_release_matching_dirty_bitmap_locked(bs, bitmap, NULL);
+}
+
/**
* For a bitmap with a successor, yield our name to the successor,
* delete the old bitmap, and return a handle to the new bitmap.
@@ -281,7 +326,11 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
BdrvDirtyBitmap *parent,
Error **errp)
{
- BdrvDirtyBitmap *successor = parent->successor;
+ BdrvDirtyBitmap *successor;
+
+ qemu_mutex_lock(parent->mutex);
+
+ successor = parent->successor;
if (!successor) {
error_setg(errp, "Cannot reclaim a successor when none is present");
@@ -292,9 +341,11 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
error_setg(errp, "Merging of parent and successor bitmap failed");
return NULL;
}
- bdrv_release_dirty_bitmap(bs, successor);
+ bdrv_release_dirty_bitmap_locked(bs, successor);
parent->successor = NULL;
+ qemu_mutex_unlock(parent->mutex);
+
return parent;
}
@@ -321,36 +372,6 @@ static bool bdrv_dirty_bitmap_has_name(BdrvDirtyBitmap *bitmap)
return !!bdrv_dirty_bitmap_name(bitmap);
}
-/* Called with BQL taken. */
-static void bdrv_do_release_matching_dirty_bitmap(
- BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
- bool (*cond)(BdrvDirtyBitmap *bitmap))
-{
- BdrvDirtyBitmap *bm, *next;
- bdrv_dirty_bitmaps_lock(bs);
- QLIST_FOREACH_SAFE(bm, &bs->dirty_bitmaps, list, next) {
- if ((!bitmap || bm == bitmap) && (!cond || cond(bm))) {
- assert(!bm->active_iterators);
- assert(!bdrv_dirty_bitmap_frozen(bm));
- assert(!bm->meta);
- QLIST_REMOVE(bm, list);
- hbitmap_free(bm->bitmap);
- g_free(bm->name);
- g_free(bm);
-
- if (bitmap) {
- goto out;
- }
- }
- }
- if (bitmap) {
- abort();
- }
-
-out:
- bdrv_dirty_bitmaps_unlock(bs);
-}
-
/* Called with BQL taken. */
void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
{
--
2.14.3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PULL 03/13] block/dirty-bitmap: add _locked version of bdrv_reclaim_dirty_bitmap
2018-03-13 21:14 [Qemu-devel] [PULL 00/13] Bitmaps patches John Snow
2018-03-13 21:14 ` [Qemu-devel] [PULL 01/13] block/dirty-bitmap: add bdrv_dirty_bitmap_enable_successor() John Snow
2018-03-13 21:14 ` [Qemu-devel] [PULL 02/13] block/dirty-bitmap: fix locking in bdrv_reclaim_dirty_bitmap John Snow
@ 2018-03-13 21:14 ` John Snow
2018-03-13 21:14 ` [Qemu-devel] [PULL 04/13] dirty-bitmap: add locked state John Snow
` (11 subsequent siblings)
14 siblings, 0 replies; 24+ messages in thread
From: John Snow @ 2018-03-13 21:14 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, jsnow, Vladimir Sementsov-Ogievskiy
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 20180207155837.92351-4-vsementsov@virtuozzo.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
block/dirty-bitmap.c | 30 +++++++++++++++++++-----------
include/block/dirty-bitmap.h | 3 +++
2 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 75435f6c2f..ce00ff3474 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -320,17 +320,13 @@ BdrvDirtyBitmap *bdrv_dirty_bitmap_abdicate(BlockDriverState *bs,
* In cases of failure where we can no longer safely delete the parent,
* we may wish to re-join the parent and child/successor.
* The merged parent will be un-frozen, but not explicitly re-enabled.
- * Called with BQL taken.
+ * Called within bdrv_dirty_bitmap_lock..unlock and with BQL taken.
*/
-BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
- BdrvDirtyBitmap *parent,
- Error **errp)
+BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BlockDriverState *bs,
+ BdrvDirtyBitmap *parent,
+ Error **errp)
{
- BdrvDirtyBitmap *successor;
-
- qemu_mutex_lock(parent->mutex);
-
- successor = parent->successor;
+ BdrvDirtyBitmap *successor = parent->successor;
if (!successor) {
error_setg(errp, "Cannot reclaim a successor when none is present");
@@ -344,11 +340,23 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
bdrv_release_dirty_bitmap_locked(bs, successor);
parent->successor = NULL;
- qemu_mutex_unlock(parent->mutex);
-
return parent;
}
+/* Called with BQL taken. */
+BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
+ 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);
+
+ return ret;
+}
+
/**
* Truncates _all_ bitmaps attached to a BDS.
* Called with BQL taken.
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index e0ebc96b01..5c239be74d 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -93,5 +93,8 @@ BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BlockDriverState *bs,
BdrvDirtyBitmap *bitmap);
char *bdrv_dirty_bitmap_sha256(const BdrvDirtyBitmap *bitmap, Error **errp);
int64_t bdrv_dirty_bitmap_next_zero(BdrvDirtyBitmap *bitmap, uint64_t start);
+BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap,
+ Error **errp);
#endif
--
2.14.3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PULL 04/13] dirty-bitmap: add locked state
2018-03-13 21:14 [Qemu-devel] [PULL 00/13] Bitmaps patches John Snow
` (2 preceding siblings ...)
2018-03-13 21:14 ` [Qemu-devel] [PULL 03/13] block/dirty-bitmap: add _locked version of bdrv_reclaim_dirty_bitmap John Snow
@ 2018-03-13 21:14 ` John Snow
2018-03-13 21:14 ` [Qemu-devel] [PULL 05/13] migration: introduce postcopy-only pending John Snow
` (10 subsequent siblings)
14 siblings, 0 replies; 24+ messages in thread
From: John Snow @ 2018-03-13 21:14 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, jsnow, Vladimir Sementsov-Ogievskiy
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Add special state, when qmp operations on the bitmap are disabled.
It is needed during bitmap migration. "Frozen" state is not
appropriate here, because it looks like bitmap is unchanged.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 20180207155837.92351-5-vsementsov@virtuozzo.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
block/dirty-bitmap.c | 16 ++++++++++++++++
blockdev.c | 19 +++++++++++++++++++
include/block/dirty-bitmap.h | 3 +++
qapi/block-core.json | 5 ++++-
4 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index ce00ff3474..967159479d 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -40,6 +40,8 @@ struct BdrvDirtyBitmap {
QemuMutex *mutex;
HBitmap *bitmap; /* Dirty bitmap implementation */
HBitmap *meta; /* Meta dirty bitmap */
+ bool qmp_locked; /* Bitmap is locked, it can't be modified
+ through QMP */
BdrvDirtyBitmap *successor; /* Anonymous child; implies frozen status */
char *name; /* Optional non-empty unique ID */
int64_t size; /* Size of the bitmap, in bytes */
@@ -183,6 +185,18 @@ bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap)
return bitmap->successor;
}
+void bdrv_dirty_bitmap_set_qmp_locked(BdrvDirtyBitmap *bitmap, bool qmp_locked)
+{
+ qemu_mutex_lock(bitmap->mutex);
+ bitmap->qmp_locked = qmp_locked;
+ qemu_mutex_unlock(bitmap->mutex);
+}
+
+bool bdrv_dirty_bitmap_qmp_locked(BdrvDirtyBitmap *bitmap)
+{
+ return bitmap->qmp_locked;
+}
+
/* Called with BQL taken. */
bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap)
{
@@ -194,6 +208,8 @@ DirtyBitmapStatus bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap)
{
if (bdrv_dirty_bitmap_frozen(bitmap)) {
return DIRTY_BITMAP_STATUS_FROZEN;
+ } else if (bdrv_dirty_bitmap_qmp_locked(bitmap)) {
+ return DIRTY_BITMAP_STATUS_LOCKED;
} else if (!bdrv_dirty_bitmap_enabled(bitmap)) {
return DIRTY_BITMAP_STATUS_DISABLED;
} else {
diff --git a/blockdev.c b/blockdev.c
index 1fbfd3a2c4..b9de18f3b2 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2118,6 +2118,9 @@ static void block_dirty_bitmap_clear_prepare(BlkActionState *common,
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");
return;
@@ -2862,6 +2865,11 @@ void qmp_block_dirty_bitmap_remove(const char *node, const char *name,
"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);
+ return;
}
if (bdrv_dirty_bitmap_get_persistance(bitmap)) {
@@ -2896,6 +2904,11 @@ void qmp_block_dirty_bitmap_clear(const char *node, const char *name,
"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)) {
error_setg(errp,
"Bitmap '%s' is currently disabled and cannot be cleared",
@@ -3370,6 +3383,12 @@ static BlockJob *do_drive_backup(DriveBackup *backup, BlockJobTxn *txn,
bdrv_unref(target_bs);
goto out;
}
+ if (bdrv_dirty_bitmap_qmp_locked(bmap)) {
+ error_setg(errp,
+ "Bitmap '%s' is currently locked and cannot be used for "
+ "backup", backup->bitmap);
+ goto out;
+ }
}
job = backup_job_create(backup->job_id, bs, target_bs, backup->speed,
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 5c239be74d..1ff8949b1b 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -69,6 +69,8 @@ void bdrv_dirty_bitmap_deserialize_finish(BdrvDirtyBitmap *bitmap);
void bdrv_dirty_bitmap_set_readonly(BdrvDirtyBitmap *bitmap, bool value);
void bdrv_dirty_bitmap_set_persistance(BdrvDirtyBitmap *bitmap,
bool persistent);
+void bdrv_dirty_bitmap_set_qmp_locked(BdrvDirtyBitmap *bitmap, bool qmp_locked);
+
/* Functions that require manual locking. */
void bdrv_dirty_bitmap_lock(BdrvDirtyBitmap *bitmap);
@@ -88,6 +90,7 @@ 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_persistance(BdrvDirtyBitmap *bitmap);
+bool bdrv_dirty_bitmap_qmp_locked(BdrvDirtyBitmap *bitmap);
bool bdrv_has_changed_persistent_bitmaps(BlockDriverState *bs);
BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BlockDriverState *bs,
BdrvDirtyBitmap *bitmap);
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 524d51567a..2b378f510a 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -426,10 +426,13 @@
# @active: The bitmap is actively monitoring for new writes, and can be cleared,
# deleted, or used for backup operations.
#
+# @locked: The bitmap is currently in-use by some operation and can not be
+# cleared, deleted, or used for backup operations. (Since 2.12)
+#
# Since: 2.4
##
{ 'enum': 'DirtyBitmapStatus',
- 'data': ['active', 'disabled', 'frozen'] }
+ 'data': ['active', 'disabled', 'frozen', 'locked'] }
##
# @BlockDirtyInfo:
--
2.14.3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PULL 05/13] migration: introduce postcopy-only pending
2018-03-13 21:14 [Qemu-devel] [PULL 00/13] Bitmaps patches John Snow
` (3 preceding siblings ...)
2018-03-13 21:14 ` [Qemu-devel] [PULL 04/13] dirty-bitmap: add locked state John Snow
@ 2018-03-13 21:14 ` John Snow
2018-03-13 21:14 ` [Qemu-devel] [PULL 06/13] qapi: add dirty-bitmaps migration capability John Snow
` (9 subsequent siblings)
14 siblings, 0 replies; 24+ messages in thread
From: John Snow @ 2018-03-13 21:14 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, jsnow, Vladimir Sementsov-Ogievskiy
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
There would be savevm states (dirty-bitmap) which can migrate only in
postcopy stage. The corresponding pending is introduced here.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-id: 20180313180320.339796-6-vsementsov@virtuozzo.com
---
hw/s390x/s390-stattrib.c | 7 ++++---
include/migration/register.h | 17 +++++++++++++++--
migration/block.c | 7 ++++---
migration/migration.c | 12 ++++++------
migration/ram.c | 9 +++++----
migration/savevm.c | 13 ++++++++-----
migration/savevm.h | 5 +++--
migration/trace-events | 2 +-
8 files changed, 46 insertions(+), 26 deletions(-)
diff --git a/hw/s390x/s390-stattrib.c b/hw/s390x/s390-stattrib.c
index adf07ef312..70b95550a8 100644
--- a/hw/s390x/s390-stattrib.c
+++ b/hw/s390x/s390-stattrib.c
@@ -183,15 +183,16 @@ static int cmma_save_setup(QEMUFile *f, void *opaque)
}
static void cmma_save_pending(QEMUFile *f, void *opaque, uint64_t max_size,
- uint64_t *non_postcopiable_pending,
- uint64_t *postcopiable_pending)
+ uint64_t *res_precopy_only,
+ uint64_t *res_compatible,
+ uint64_t *res_postcopy_only)
{
S390StAttribState *sas = S390_STATTRIB(opaque);
S390StAttribClass *sac = S390_STATTRIB_GET_CLASS(sas);
long long res = sac->get_dirtycount(sas);
if (res >= 0) {
- *non_postcopiable_pending += res;
+ *res_precopy_only += res;
}
}
diff --git a/include/migration/register.h b/include/migration/register.h
index f4f7bdc177..9436a87678 100644
--- a/include/migration/register.h
+++ b/include/migration/register.h
@@ -37,8 +37,21 @@ typedef struct SaveVMHandlers {
int (*save_setup)(QEMUFile *f, void *opaque);
void (*save_live_pending)(QEMUFile *f, void *opaque,
uint64_t threshold_size,
- uint64_t *non_postcopiable_pending,
- uint64_t *postcopiable_pending);
+ uint64_t *res_precopy_only,
+ uint64_t *res_compatible,
+ uint64_t *res_postcopy_only);
+ /* Note for save_live_pending:
+ * - res_precopy_only is for data which must be migrated in precopy phase
+ * or in stopped state, in other words - before target vm start
+ * - res_compatible is for data which may be migrated in any phase
+ * - res_postcopy_only is for data which must be migrated in postcopy phase
+ * or in stopped state, in other words - after source vm stop
+ *
+ * Sum of res_postcopy_only, res_compatible and res_postcopy_only is the
+ * whole amount of pending data.
+ */
+
+
LoadStateHandler *load_state;
int (*load_setup)(QEMUFile *f, void *opaque);
int (*load_cleanup)(void *opaque);
diff --git a/migration/block.c b/migration/block.c
index 41b95d1dd8..5c03632257 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -864,8 +864,9 @@ static int block_save_complete(QEMUFile *f, void *opaque)
}
static void block_save_pending(QEMUFile *f, void *opaque, uint64_t max_size,
- uint64_t *non_postcopiable_pending,
- uint64_t *postcopiable_pending)
+ uint64_t *res_precopy_only,
+ uint64_t *res_compatible,
+ uint64_t *res_postcopy_only)
{
/* Estimate pending number of bytes to send */
uint64_t pending;
@@ -886,7 +887,7 @@ static void block_save_pending(QEMUFile *f, void *opaque, uint64_t max_size,
DPRINTF("Enter save live pending %" PRIu64 "\n", pending);
/* We don't do postcopy */
- *non_postcopiable_pending += pending;
+ *res_precopy_only += pending;
}
static int block_load(QEMUFile *f, void *opaque, int version_id)
diff --git a/migration/migration.c b/migration/migration.c
index 6a4780ef6f..90307f8ab5 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2242,20 +2242,20 @@ typedef enum {
*/
static MigIterateState migration_iteration_run(MigrationState *s)
{
- uint64_t pending_size, pend_post, pend_nonpost;
+ uint64_t pending_size, pend_pre, pend_compat, pend_post;
bool in_postcopy = s->state == MIGRATION_STATUS_POSTCOPY_ACTIVE;
- qemu_savevm_state_pending(s->to_dst_file, s->threshold_size,
- &pend_nonpost, &pend_post);
- pending_size = pend_nonpost + pend_post;
+ qemu_savevm_state_pending(s->to_dst_file, s->threshold_size, &pend_pre,
+ &pend_compat, &pend_post);
+ pending_size = pend_pre + pend_compat + pend_post;
trace_migrate_pending(pending_size, s->threshold_size,
- pend_post, pend_nonpost);
+ pend_pre, pend_compat, pend_post);
if (pending_size && pending_size >= s->threshold_size) {
/* Still a significant amount to transfer */
if (migrate_postcopy() && !in_postcopy &&
- pend_nonpost <= s->threshold_size &&
+ pend_pre <= s->threshold_size &&
atomic_read(&s->start_postcopy)) {
if (postcopy_start(s)) {
error_report("%s: postcopy failed to start", __func__);
diff --git a/migration/ram.c b/migration/ram.c
index 7266351fd0..590fceb7e9 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2370,8 +2370,9 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
}
static void ram_save_pending(QEMUFile *f, void *opaque, uint64_t max_size,
- uint64_t *non_postcopiable_pending,
- uint64_t *postcopiable_pending)
+ uint64_t *res_precopy_only,
+ uint64_t *res_compatible,
+ uint64_t *res_postcopy_only)
{
RAMState **temp = opaque;
RAMState *rs = *temp;
@@ -2391,9 +2392,9 @@ static void ram_save_pending(QEMUFile *f, void *opaque, uint64_t max_size,
if (migrate_postcopy_ram()) {
/* We can do postcopy, and all the data is postcopiable */
- *postcopiable_pending += remaining_size;
+ *res_compatible += remaining_size;
} else {
- *non_postcopiable_pending += remaining_size;
+ *res_precopy_only += remaining_size;
}
}
diff --git a/migration/savevm.c b/migration/savevm.c
index 358c5b51e2..cd5944b81f 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1220,13 +1220,15 @@ int qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only,
* for units that can't do postcopy.
*/
void qemu_savevm_state_pending(QEMUFile *f, uint64_t threshold_size,
- uint64_t *res_non_postcopiable,
- uint64_t *res_postcopiable)
+ uint64_t *res_precopy_only,
+ uint64_t *res_compatible,
+ uint64_t *res_postcopy_only)
{
SaveStateEntry *se;
- *res_non_postcopiable = 0;
- *res_postcopiable = 0;
+ *res_precopy_only = 0;
+ *res_compatible = 0;
+ *res_postcopy_only = 0;
QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
@@ -1239,7 +1241,8 @@ void qemu_savevm_state_pending(QEMUFile *f, uint64_t threshold_size,
}
}
se->ops->save_live_pending(f, se->opaque, threshold_size,
- res_non_postcopiable, res_postcopiable);
+ res_precopy_only, res_compatible,
+ res_postcopy_only);
}
}
diff --git a/migration/savevm.h b/migration/savevm.h
index 295c4a1f2c..cf4f0d37ca 100644
--- a/migration/savevm.h
+++ b/migration/savevm.h
@@ -38,8 +38,9 @@ void qemu_savevm_state_complete_postcopy(QEMUFile *f);
int qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only,
bool inactivate_disks);
void qemu_savevm_state_pending(QEMUFile *f, uint64_t max_size,
- uint64_t *res_non_postcopiable,
- uint64_t *res_postcopiable);
+ uint64_t *res_precopy_only,
+ uint64_t *res_compatible,
+ uint64_t *res_postcopy_only);
void qemu_savevm_send_ping(QEMUFile *f, uint32_t value);
void qemu_savevm_send_open_return_path(QEMUFile *f);
int qemu_savevm_send_packaged(QEMUFile *f, const uint8_t *buf, size_t len);
diff --git a/migration/trace-events b/migration/trace-events
index 93961dea16..8ec84d82fd 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -86,7 +86,7 @@ migrate_fd_cleanup(void) ""
migrate_fd_error(const char *error_desc) "error=%s"
migrate_fd_cancel(void) ""
migrate_handle_rp_req_pages(const char *rbname, size_t start, size_t len) "in %s at 0x%zx len 0x%zx"
-migrate_pending(uint64_t size, uint64_t max, uint64_t post, uint64_t nonpost) "pending size %" PRIu64 " max %" PRIu64 " (post=%" PRIu64 " nonpost=%" PRIu64 ")"
+migrate_pending(uint64_t size, uint64_t max, uint64_t pre, uint64_t compat, uint64_t post) "pending size %" PRIu64 " max %" PRIu64 " (pre = %" PRIu64 " compat=%" PRIu64 " post=%" PRIu64 ")"
migrate_send_rp_message(int msg_type, uint16_t len) "%d: len %d"
migration_completion_file_err(void) ""
migration_completion_postcopy_end(void) ""
--
2.14.3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PULL 06/13] qapi: add dirty-bitmaps migration capability
2018-03-13 21:14 [Qemu-devel] [PULL 00/13] Bitmaps patches John Snow
` (4 preceding siblings ...)
2018-03-13 21:14 ` [Qemu-devel] [PULL 05/13] migration: introduce postcopy-only pending John Snow
@ 2018-03-13 21:14 ` John Snow
2018-03-13 21:14 ` [Qemu-devel] [PULL 07/13] migration: include migrate_dirty_bitmaps in migrate_postcopy John Snow
` (8 subsequent siblings)
14 siblings, 0 replies; 24+ messages in thread
From: John Snow @ 2018-03-13 21:14 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, jsnow, Vladimir Sementsov-Ogievskiy
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Message-id: 20180313180320.339796-7-vsementsov@virtuozzo.com
---
migration/migration.c | 9 +++++++++
migration/migration.h | 1 +
qapi/migration.json | 6 +++++-
3 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/migration/migration.c b/migration/migration.c
index 90307f8ab5..e0aff5c814 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1565,6 +1565,15 @@ int migrate_decompress_threads(void)
return s->parameters.decompress_threads;
}
+bool migrate_dirty_bitmaps(void)
+{
+ MigrationState *s;
+
+ s = migrate_get_current();
+
+ return s->enabled_capabilities[MIGRATION_CAPABILITY_DIRTY_BITMAPS];
+}
+
bool migrate_use_events(void)
{
MigrationState *s;
diff --git a/migration/migration.h b/migration/migration.h
index 08c5d2ded1..da6bc37de8 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -205,6 +205,7 @@ bool migrate_postcopy(void);
bool migrate_release_ram(void);
bool migrate_postcopy_ram(void);
bool migrate_zero_blocks(void);
+bool migrate_dirty_bitmaps(void);
bool migrate_auto_converge(void);
bool migrate_use_multifd(void);
diff --git a/qapi/migration.json b/qapi/migration.json
index 7f465a1902..9d0bf82cf4 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -354,12 +354,16 @@
#
# @x-multifd: Use more than one fd for migration (since 2.11)
#
+# @dirty-bitmaps: If enabled, QEMU will migrate named dirty bitmaps.
+# (since 2.12)
+#
# Since: 1.2
##
{ 'enum': 'MigrationCapability',
'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks',
'compress', 'events', 'postcopy-ram', 'x-colo', 'release-ram',
- 'block', 'return-path', 'pause-before-switchover', 'x-multifd' ] }
+ 'block', 'return-path', 'pause-before-switchover', 'x-multifd',
+ 'dirty-bitmaps' ] }
##
# @MigrationCapabilityStatus:
--
2.14.3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PULL 07/13] migration: include migrate_dirty_bitmaps in migrate_postcopy
2018-03-13 21:14 [Qemu-devel] [PULL 00/13] Bitmaps patches John Snow
` (5 preceding siblings ...)
2018-03-13 21:14 ` [Qemu-devel] [PULL 06/13] qapi: add dirty-bitmaps migration capability John Snow
@ 2018-03-13 21:14 ` John Snow
2018-03-13 21:14 ` [Qemu-devel] [PULL 08/13] migration/qemu-file: add qemu_put_counted_string() John Snow
` (7 subsequent siblings)
14 siblings, 0 replies; 24+ messages in thread
From: John Snow @ 2018-03-13 21:14 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, jsnow, Vladimir Sementsov-Ogievskiy
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Enable postcopy if dirty bitmap migration is enabled.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Message-id: 20180313180320.339796-8-vsementsov@virtuozzo.com
---
migration/migration.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/migration/migration.c b/migration/migration.c
index e0aff5c814..094196c236 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1508,7 +1508,7 @@ bool migrate_postcopy_ram(void)
bool migrate_postcopy(void)
{
- return migrate_postcopy_ram();
+ return migrate_postcopy_ram() || migrate_dirty_bitmaps();
}
bool migrate_auto_converge(void)
--
2.14.3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PULL 08/13] migration/qemu-file: add qemu_put_counted_string()
2018-03-13 21:14 [Qemu-devel] [PULL 00/13] Bitmaps patches John Snow
` (6 preceding siblings ...)
2018-03-13 21:14 ` [Qemu-devel] [PULL 07/13] migration: include migrate_dirty_bitmaps in migrate_postcopy John Snow
@ 2018-03-13 21:14 ` John Snow
2018-03-13 21:14 ` [Qemu-devel] [PULL 09/13] migration: add is_active_iterate handler John Snow
` (6 subsequent siblings)
14 siblings, 0 replies; 24+ messages in thread
From: John Snow @ 2018-03-13 21:14 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, jsnow, Vladimir Sementsov-Ogievskiy
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Add function opposite to qemu_get_counted_string.
qemu_put_counted_string puts one-byte length of the string (string
should not be longer than 255 characters), and then it puts the string,
without last zero byte.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Message-id: 20180313180320.339796-9-vsementsov@virtuozzo.com
---
migration/qemu-file.c | 13 +++++++++++++
migration/qemu-file.h | 2 ++
2 files changed, 15 insertions(+)
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 2ab2bf362d..e85f501f86 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -733,6 +733,19 @@ size_t qemu_get_counted_string(QEMUFile *f, char buf[256])
return res == len ? res : 0;
}
+/*
+ * Put a string with one preceding byte containing its length. The length of
+ * the string should be less than 256.
+ */
+void qemu_put_counted_string(QEMUFile *f, const char *str)
+{
+ size_t len = strlen(str);
+
+ assert(len < 256);
+ qemu_put_byte(f, len);
+ qemu_put_buffer(f, (const uint8_t *)str, len);
+}
+
/*
* Set the blocking state of the QEMUFile.
* Note: On some transports the OS only keeps a single blocking state for
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index aae4e5ed36..f4f356ab12 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -174,4 +174,6 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset,
ram_addr_t offset, size_t size,
uint64_t *bytes_sent);
+void qemu_put_counted_string(QEMUFile *f, const char *name);
+
#endif
--
2.14.3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PULL 09/13] migration: add is_active_iterate handler
2018-03-13 21:14 [Qemu-devel] [PULL 00/13] Bitmaps patches John Snow
` (7 preceding siblings ...)
2018-03-13 21:14 ` [Qemu-devel] [PULL 08/13] migration/qemu-file: add qemu_put_counted_string() John Snow
@ 2018-03-13 21:14 ` John Snow
2018-03-13 21:14 ` [Qemu-devel] [PULL 10/13] migration: allow qmp command migrate-start-postcopy for any postcopy John Snow
` (5 subsequent siblings)
14 siblings, 0 replies; 24+ messages in thread
From: John Snow @ 2018-03-13 21:14 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, jsnow, Vladimir Sementsov-Ogievskiy
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Only-postcopy savevm states (dirty-bitmap) don't need live iteration, so
to disable them and stop transporting empty sections there is a new
savevm handler.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Message-id: 20180313180320.339796-10-vsementsov@virtuozzo.com
---
include/migration/register.h | 9 +++++++++
migration/savevm.c | 5 +++++
2 files changed, 14 insertions(+)
diff --git a/include/migration/register.h b/include/migration/register.h
index 9436a87678..f6f12f9b1a 100644
--- a/include/migration/register.h
+++ b/include/migration/register.h
@@ -26,6 +26,15 @@ typedef struct SaveVMHandlers {
bool (*is_active)(void *opaque);
bool (*has_postcopy)(void *opaque);
+ /* is_active_iterate
+ * If it is not NULL then qemu_savevm_state_iterate will skip iteration if
+ * it returns false. For example, it is needed for only-postcopy-states,
+ * which needs to be handled by qemu_savevm_state_setup and
+ * qemu_savevm_state_pending, but do not need iterations until not in
+ * postcopy stage.
+ */
+ bool (*is_active_iterate)(void *opaque);
+
/* This runs outside the iothread lock in the migration case, and
* within the lock in the savevm case. The callback had better only
* use data that is local to the migration thread or protected
diff --git a/migration/savevm.c b/migration/savevm.c
index cd5944b81f..a60819ec2e 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1028,6 +1028,11 @@ int qemu_savevm_state_iterate(QEMUFile *f, bool postcopy)
continue;
}
}
+ if (se->ops && se->ops->is_active_iterate) {
+ if (!se->ops->is_active_iterate(se->opaque)) {
+ continue;
+ }
+ }
/*
* In the postcopy phase, any device that doesn't know how to
* do postcopy should have saved it's state in the _complete
--
2.14.3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PULL 10/13] migration: allow qmp command migrate-start-postcopy for any postcopy
2018-03-13 21:14 [Qemu-devel] [PULL 00/13] Bitmaps patches John Snow
` (8 preceding siblings ...)
2018-03-13 21:14 ` [Qemu-devel] [PULL 09/13] migration: add is_active_iterate handler John Snow
@ 2018-03-13 21:14 ` John Snow
2018-03-13 21:14 ` [Qemu-devel] [PULL 11/13] migration: add postcopy migration of dirty bitmaps John Snow
` (4 subsequent siblings)
14 siblings, 0 replies; 24+ messages in thread
From: John Snow @ 2018-03-13 21:14 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, jsnow, Vladimir Sementsov-Ogievskiy
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Allow migrate-start-postcopy for any postcopy type
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-id: 20180313180320.339796-11-vsementsov@virtuozzo.com
---
migration/migration.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/migration/migration.c b/migration/migration.c
index 094196c236..59b4fe6090 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1022,7 +1022,7 @@ void qmp_migrate_start_postcopy(Error **errp)
{
MigrationState *s = migrate_get_current();
- if (!migrate_postcopy_ram()) {
+ if (!migrate_postcopy()) {
error_setg(errp, "Enable postcopy with migrate_set_capability before"
" the start of migration");
return;
--
2.14.3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PULL 11/13] migration: add postcopy migration of dirty bitmaps
2018-03-13 21:14 [Qemu-devel] [PULL 00/13] Bitmaps patches John Snow
` (9 preceding siblings ...)
2018-03-13 21:14 ` [Qemu-devel] [PULL 10/13] migration: allow qmp command migrate-start-postcopy for any postcopy John Snow
@ 2018-03-13 21:14 ` John Snow
2018-04-27 13:22 ` Peter Maydell
2018-04-27 13:24 ` Peter Maydell
2018-03-13 21:14 ` [Qemu-devel] [PULL 12/13] iotests: add dirty bitmap migration test John Snow
` (3 subsequent siblings)
14 siblings, 2 replies; 24+ messages in thread
From: John Snow @ 2018-03-13 21:14 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, jsnow, Vladimir Sementsov-Ogievskiy
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Postcopy migration of dirty bitmaps. Only named dirty bitmaps are migrated.
If destination qemu is already containing a dirty bitmap with the same name
as a migrated bitmap (for the same node), then, if their granularities are
the same the migration will be done, otherwise the error will be generated.
If destination qemu doesn't contain such bitmap it will be created.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-id: 20180313180320.339796-12-vsementsov@virtuozzo.com
[Changed '+' to '*' as per list discussion. --js]
Signed-off-by: John Snow <jsnow@redhat.com>
---
include/migration/misc.h | 3 +
migration/Makefile.objs | 1 +
migration/block-dirty-bitmap.c | 746 +++++++++++++++++++++++++++++++++++++++++
migration/migration.c | 5 +
migration/migration.h | 3 +
migration/savevm.c | 2 +
migration/trace-events | 14 +
vl.c | 1 +
8 files changed, 775 insertions(+)
create mode 100644 migration/block-dirty-bitmap.c
diff --git a/include/migration/misc.h b/include/migration/misc.h
index 77fd4f587c..4ebf24c6c2 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -56,4 +56,7 @@ bool migration_has_failed(MigrationState *);
bool migration_in_postcopy_after_devices(MigrationState *);
void migration_global_dump(Monitor *mon);
+/* migration/block-dirty-bitmap.c */
+void dirty_bitmap_mig_init(void);
+
#endif
diff --git a/migration/Makefile.objs b/migration/Makefile.objs
index 99e038024d..c83ec47ba8 100644
--- a/migration/Makefile.objs
+++ b/migration/Makefile.objs
@@ -6,6 +6,7 @@ common-obj-y += qemu-file.o global_state.o
common-obj-y += qemu-file-channel.o
common-obj-y += xbzrle.o postcopy-ram.o
common-obj-y += qjson.o
+common-obj-y += block-dirty-bitmap.o
common-obj-$(CONFIG_RDMA) += rdma.o
diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
new file mode 100644
index 0000000000..dd04f102d8
--- /dev/null
+++ b/migration/block-dirty-bitmap.c
@@ -0,0 +1,746 @@
+/*
+ * Block dirty bitmap postcopy migration
+ *
+ * Copyright IBM, Corp. 2009
+ * Copyright (c) 2016-2017 Virtuozzo International GmbH. All rights reserved.
+ *
+ * Authors:
+ * Liran Schour <lirans@il.ibm.com>
+ * Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ * This file is derived from migration/block.c, so it's author and IBM copyright
+ * are here, although content is quite different.
+ *
+ * Contributions after 2012-01-13 are licensed under the terms of the
+ * GNU GPL, version 2 or (at your option) any later version.
+ *
+ * ***
+ *
+ * Here postcopy migration of dirty bitmaps is realized. Only QMP-addressable
+ * bitmaps are migrated.
+ *
+ * Bitmap migration implies creating bitmap with the same name and granularity
+ * in destination QEMU. If the bitmap with the same name (for the same node)
+ * already exists on destination an error will be generated.
+ *
+ * format of migration:
+ *
+ * # Header (shared for different chunk types)
+ * 1, 2 or 4 bytes: flags (see qemu_{put,put}_flags)
+ * [ 1 byte: node name size ] \ flags & DEVICE_NAME
+ * [ n bytes: node name ] /
+ * [ 1 byte: bitmap name size ] \ flags & BITMAP_NAME
+ * [ n bytes: bitmap name ] /
+ *
+ * # Start of bitmap migration (flags & START)
+ * header
+ * be64: granularity
+ * 1 byte: bitmap flags (corresponds to BdrvDirtyBitmap)
+ * bit 0 - bitmap is enabled
+ * bit 1 - bitmap is persistent
+ * bit 2 - bitmap is autoloading
+ * bits 3-7 - reserved, must be zero
+ *
+ * # Complete of bitmap migration (flags & COMPLETE)
+ * header
+ *
+ * # Data chunk of bitmap migration
+ * header
+ * be64: start sector
+ * be32: number of sectors
+ * [ be64: buffer size ] \ ! (flags & ZEROES)
+ * [ n bytes: buffer ] /
+ *
+ * The last chunk in stream should contain flags & EOS. The chunk may skip
+ * device and/or bitmap names, assuming them to be the same with the previous
+ * chunk.
+ */
+
+#include "qemu/osdep.h"
+#include "block/block.h"
+#include "block/block_int.h"
+#include "sysemu/block-backend.h"
+#include "qemu/main-loop.h"
+#include "qemu/error-report.h"
+#include "migration/misc.h"
+#include "migration/migration.h"
+#include "migration/qemu-file.h"
+#include "migration/vmstate.h"
+#include "migration/register.h"
+#include "qemu/hbitmap.h"
+#include "sysemu/sysemu.h"
+#include "qemu/cutils.h"
+#include "qapi/error.h"
+#include "trace.h"
+
+#define CHUNK_SIZE (1 << 10)
+
+/* Flags occupy one, two or four bytes (Big Endian). The size is determined as
+ * follows:
+ * in first (most significant) byte bit 8 is clear --> one byte
+ * in first byte bit 8 is set --> two or four bytes, depending on second
+ * byte:
+ * | in second byte bit 8 is clear --> two bytes
+ * | in second byte bit 8 is set --> four bytes
+ */
+#define DIRTY_BITMAP_MIG_FLAG_EOS 0x01
+#define DIRTY_BITMAP_MIG_FLAG_ZEROES 0x02
+#define DIRTY_BITMAP_MIG_FLAG_BITMAP_NAME 0x04
+#define DIRTY_BITMAP_MIG_FLAG_DEVICE_NAME 0x08
+#define DIRTY_BITMAP_MIG_FLAG_START 0x10
+#define DIRTY_BITMAP_MIG_FLAG_COMPLETE 0x20
+#define DIRTY_BITMAP_MIG_FLAG_BITS 0x40
+
+#define DIRTY_BITMAP_MIG_EXTRA_FLAGS 0x80
+
+#define DIRTY_BITMAP_MIG_START_FLAG_ENABLED 0x01
+#define DIRTY_BITMAP_MIG_START_FLAG_PERSISTENT 0x02
+/* 0x04 was "AUTOLOAD" flags on elder versions, no it is ignored */
+#define DIRTY_BITMAP_MIG_START_FLAG_RESERVED_MASK 0xf8
+
+typedef struct DirtyBitmapMigBitmapState {
+ /* Written during setup phase. */
+ BlockDriverState *bs;
+ const char *node_name;
+ BdrvDirtyBitmap *bitmap;
+ uint64_t total_sectors;
+ uint64_t sectors_per_chunk;
+ QSIMPLEQ_ENTRY(DirtyBitmapMigBitmapState) entry;
+ uint8_t flags;
+
+ /* For bulk phase. */
+ bool bulk_completed;
+ uint64_t cur_sector;
+} DirtyBitmapMigBitmapState;
+
+typedef struct DirtyBitmapMigState {
+ QSIMPLEQ_HEAD(dbms_list, DirtyBitmapMigBitmapState) dbms_list;
+
+ bool bulk_completed;
+ bool no_bitmaps;
+
+ /* for send_bitmap_bits() */
+ BlockDriverState *prev_bs;
+ BdrvDirtyBitmap *prev_bitmap;
+} DirtyBitmapMigState;
+
+typedef struct DirtyBitmapLoadState {
+ uint32_t flags;
+ char node_name[256];
+ char bitmap_name[256];
+ BlockDriverState *bs;
+ BdrvDirtyBitmap *bitmap;
+} DirtyBitmapLoadState;
+
+static DirtyBitmapMigState dirty_bitmap_mig_state;
+
+typedef struct DirtyBitmapLoadBitmapState {
+ BlockDriverState *bs;
+ BdrvDirtyBitmap *bitmap;
+ bool migrated;
+} DirtyBitmapLoadBitmapState;
+static GSList *enabled_bitmaps;
+QemuMutex finish_lock;
+
+void init_dirty_bitmap_incoming_migration(void)
+{
+ qemu_mutex_init(&finish_lock);
+}
+
+static uint32_t qemu_get_bitmap_flags(QEMUFile *f)
+{
+ uint8_t flags = qemu_get_byte(f);
+ if (flags & DIRTY_BITMAP_MIG_EXTRA_FLAGS) {
+ flags = flags << 8 | qemu_get_byte(f);
+ if (flags & DIRTY_BITMAP_MIG_EXTRA_FLAGS) {
+ flags = flags << 16 | qemu_get_be16(f);
+ }
+ }
+
+ return flags;
+}
+
+static void qemu_put_bitmap_flags(QEMUFile *f, uint32_t flags)
+{
+ /* The code currently do not send flags more than one byte */
+ assert(!(flags & (0xffffff00 | DIRTY_BITMAP_MIG_EXTRA_FLAGS)));
+
+ qemu_put_byte(f, flags);
+}
+
+static void send_bitmap_header(QEMUFile *f, DirtyBitmapMigBitmapState *dbms,
+ uint32_t additional_flags)
+{
+ BlockDriverState *bs = dbms->bs;
+ BdrvDirtyBitmap *bitmap = dbms->bitmap;
+ uint32_t flags = additional_flags;
+ trace_send_bitmap_header_enter();
+
+ if (bs != dirty_bitmap_mig_state.prev_bs) {
+ dirty_bitmap_mig_state.prev_bs = bs;
+ flags |= DIRTY_BITMAP_MIG_FLAG_DEVICE_NAME;
+ }
+
+ if (bitmap != dirty_bitmap_mig_state.prev_bitmap) {
+ dirty_bitmap_mig_state.prev_bitmap = bitmap;
+ flags |= DIRTY_BITMAP_MIG_FLAG_BITMAP_NAME;
+ }
+
+ qemu_put_bitmap_flags(f, flags);
+
+ if (flags & DIRTY_BITMAP_MIG_FLAG_DEVICE_NAME) {
+ qemu_put_counted_string(f, dbms->node_name);
+ }
+
+ if (flags & DIRTY_BITMAP_MIG_FLAG_BITMAP_NAME) {
+ qemu_put_counted_string(f, bdrv_dirty_bitmap_name(bitmap));
+ }
+}
+
+static void send_bitmap_start(QEMUFile *f, DirtyBitmapMigBitmapState *dbms)
+{
+ send_bitmap_header(f, dbms, DIRTY_BITMAP_MIG_FLAG_START);
+ qemu_put_be32(f, bdrv_dirty_bitmap_granularity(dbms->bitmap));
+ qemu_put_byte(f, dbms->flags);
+}
+
+static void send_bitmap_complete(QEMUFile *f, DirtyBitmapMigBitmapState *dbms)
+{
+ send_bitmap_header(f, dbms, DIRTY_BITMAP_MIG_FLAG_COMPLETE);
+}
+
+static void send_bitmap_bits(QEMUFile *f, DirtyBitmapMigBitmapState *dbms,
+ uint64_t start_sector, uint32_t nr_sectors)
+{
+ /* align for buffer_is_zero() */
+ uint64_t align = 4 * sizeof(long);
+ uint64_t unaligned_size =
+ bdrv_dirty_bitmap_serialization_size(
+ dbms->bitmap, start_sector << BDRV_SECTOR_BITS,
+ (uint64_t)nr_sectors << BDRV_SECTOR_BITS);
+ uint64_t buf_size = QEMU_ALIGN_UP(unaligned_size, align);
+ uint8_t *buf = g_malloc0(buf_size);
+ uint32_t flags = DIRTY_BITMAP_MIG_FLAG_BITS;
+
+ bdrv_dirty_bitmap_serialize_part(
+ dbms->bitmap, buf, start_sector << BDRV_SECTOR_BITS,
+ (uint64_t)nr_sectors << BDRV_SECTOR_BITS);
+
+ if (buffer_is_zero(buf, buf_size)) {
+ g_free(buf);
+ buf = NULL;
+ flags |= DIRTY_BITMAP_MIG_FLAG_ZEROES;
+ }
+
+ trace_send_bitmap_bits(flags, start_sector, nr_sectors, buf_size);
+
+ send_bitmap_header(f, dbms, flags);
+
+ qemu_put_be64(f, start_sector);
+ qemu_put_be32(f, nr_sectors);
+
+ /* if a block is zero we need to flush here since the network
+ * bandwidth is now a lot higher than the storage device bandwidth.
+ * thus if we queue zero blocks we slow down the migration. */
+ if (flags & DIRTY_BITMAP_MIG_FLAG_ZEROES) {
+ qemu_fflush(f);
+ } else {
+ qemu_put_be64(f, buf_size);
+ qemu_put_buffer(f, buf, buf_size);
+ }
+
+ g_free(buf);
+}
+
+/* Called with iothread lock taken. */
+static void dirty_bitmap_mig_cleanup(void)
+{
+ DirtyBitmapMigBitmapState *dbms;
+
+ while ((dbms = QSIMPLEQ_FIRST(&dirty_bitmap_mig_state.dbms_list)) != NULL) {
+ QSIMPLEQ_REMOVE_HEAD(&dirty_bitmap_mig_state.dbms_list, entry);
+ bdrv_dirty_bitmap_set_qmp_locked(dbms->bitmap, false);
+ bdrv_unref(dbms->bs);
+ g_free(dbms);
+ }
+}
+
+/* Called with iothread lock taken. */
+static int init_dirty_bitmap_migration(void)
+{
+ BlockDriverState *bs;
+ BdrvDirtyBitmap *bitmap;
+ DirtyBitmapMigBitmapState *dbms;
+ BdrvNextIterator it;
+
+ dirty_bitmap_mig_state.bulk_completed = false;
+ dirty_bitmap_mig_state.prev_bs = NULL;
+ dirty_bitmap_mig_state.prev_bitmap = NULL;
+ dirty_bitmap_mig_state.no_bitmaps = false;
+
+ for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
+ const char *drive_name = bdrv_get_device_or_node_name(bs);
+
+ /* skip automatically inserted nodes */
+ while (bs && bs->drv && bs->implicit) {
+ bs = backing_bs(bs);
+ }
+
+ for (bitmap = bdrv_dirty_bitmap_next(bs, NULL); bitmap;
+ bitmap = bdrv_dirty_bitmap_next(bs, bitmap))
+ {
+ if (!bdrv_dirty_bitmap_name(bitmap)) {
+ continue;
+ }
+
+ if (drive_name == NULL) {
+ error_report("Found bitmap '%s' in unnamed node %p. It can't "
+ "be migrated", bdrv_dirty_bitmap_name(bitmap), bs);
+ goto fail;
+ }
+
+ if (bdrv_dirty_bitmap_frozen(bitmap)) {
+ error_report("Can't migrate frozen dirty bitmap: '%s",
+ bdrv_dirty_bitmap_name(bitmap));
+ goto fail;
+ }
+
+ if (bdrv_dirty_bitmap_qmp_locked(bitmap)) {
+ error_report("Can't migrate locked dirty bitmap: '%s",
+ bdrv_dirty_bitmap_name(bitmap));
+ goto fail;
+ }
+
+ bdrv_ref(bs);
+ bdrv_dirty_bitmap_set_qmp_locked(bitmap, true);
+
+ dbms = g_new0(DirtyBitmapMigBitmapState, 1);
+ dbms->bs = bs;
+ dbms->node_name = drive_name;
+ dbms->bitmap = bitmap;
+ dbms->total_sectors = bdrv_nb_sectors(bs);
+ dbms->sectors_per_chunk = CHUNK_SIZE * 8 *
+ bdrv_dirty_bitmap_granularity(bitmap) >> BDRV_SECTOR_BITS;
+ if (bdrv_dirty_bitmap_enabled(bitmap)) {
+ dbms->flags |= DIRTY_BITMAP_MIG_START_FLAG_ENABLED;
+ }
+ if (bdrv_dirty_bitmap_get_persistance(bitmap)) {
+ dbms->flags |= DIRTY_BITMAP_MIG_START_FLAG_PERSISTENT;
+ }
+
+ QSIMPLEQ_INSERT_TAIL(&dirty_bitmap_mig_state.dbms_list,
+ dbms, entry);
+ }
+ }
+
+ /* unset persistance here, to not roll back it */
+ QSIMPLEQ_FOREACH(dbms, &dirty_bitmap_mig_state.dbms_list, entry) {
+ bdrv_dirty_bitmap_set_persistance(dbms->bitmap, false);
+ }
+
+ if (QSIMPLEQ_EMPTY(&dirty_bitmap_mig_state.dbms_list)) {
+ dirty_bitmap_mig_state.no_bitmaps = true;
+ }
+
+ return 0;
+
+fail:
+ dirty_bitmap_mig_cleanup();
+
+ return -1;
+}
+
+/* Called with no lock taken. */
+static void bulk_phase_send_chunk(QEMUFile *f, DirtyBitmapMigBitmapState *dbms)
+{
+ uint32_t nr_sectors = MIN(dbms->total_sectors - dbms->cur_sector,
+ dbms->sectors_per_chunk);
+
+ send_bitmap_bits(f, dbms, dbms->cur_sector, nr_sectors);
+
+ dbms->cur_sector += nr_sectors;
+ if (dbms->cur_sector >= dbms->total_sectors) {
+ dbms->bulk_completed = true;
+ }
+}
+
+/* Called with no lock taken. */
+static void bulk_phase(QEMUFile *f, bool limit)
+{
+ DirtyBitmapMigBitmapState *dbms;
+
+ QSIMPLEQ_FOREACH(dbms, &dirty_bitmap_mig_state.dbms_list, entry) {
+ while (!dbms->bulk_completed) {
+ bulk_phase_send_chunk(f, dbms);
+ if (limit && qemu_file_rate_limit(f)) {
+ return;
+ }
+ }
+ }
+
+ dirty_bitmap_mig_state.bulk_completed = true;
+}
+
+/* for SaveVMHandlers */
+static void dirty_bitmap_save_cleanup(void *opaque)
+{
+ dirty_bitmap_mig_cleanup();
+}
+
+static int dirty_bitmap_save_iterate(QEMUFile *f, void *opaque)
+{
+ trace_dirty_bitmap_save_iterate(migration_in_postcopy());
+
+ if (migration_in_postcopy() && !dirty_bitmap_mig_state.bulk_completed) {
+ bulk_phase(f, true);
+ }
+
+ qemu_put_bitmap_flags(f, DIRTY_BITMAP_MIG_FLAG_EOS);
+
+ return dirty_bitmap_mig_state.bulk_completed;
+}
+
+/* Called with iothread lock taken. */
+
+static int dirty_bitmap_save_complete(QEMUFile *f, void *opaque)
+{
+ DirtyBitmapMigBitmapState *dbms;
+ trace_dirty_bitmap_save_complete_enter();
+
+ if (!dirty_bitmap_mig_state.bulk_completed) {
+ bulk_phase(f, false);
+ }
+
+ QSIMPLEQ_FOREACH(dbms, &dirty_bitmap_mig_state.dbms_list, entry) {
+ send_bitmap_complete(f, dbms);
+ }
+
+ qemu_put_bitmap_flags(f, DIRTY_BITMAP_MIG_FLAG_EOS);
+
+ trace_dirty_bitmap_save_complete_finish();
+
+ dirty_bitmap_mig_cleanup();
+ return 0;
+}
+
+static void dirty_bitmap_save_pending(QEMUFile *f, void *opaque,
+ uint64_t max_size,
+ uint64_t *res_precopy_only,
+ uint64_t *res_compatible,
+ uint64_t *res_postcopy_only)
+{
+ DirtyBitmapMigBitmapState *dbms;
+ uint64_t pending = 0;
+
+ qemu_mutex_lock_iothread();
+
+ QSIMPLEQ_FOREACH(dbms, &dirty_bitmap_mig_state.dbms_list, entry) {
+ uint64_t gran = bdrv_dirty_bitmap_granularity(dbms->bitmap);
+ uint64_t sectors = dbms->bulk_completed ? 0 :
+ dbms->total_sectors - dbms->cur_sector;
+
+ pending += DIV_ROUND_UP(sectors * BDRV_SECTOR_SIZE, gran);
+ }
+
+ qemu_mutex_unlock_iothread();
+
+ trace_dirty_bitmap_save_pending(pending, max_size);
+
+ *res_postcopy_only += pending;
+}
+
+/* First occurrence of this bitmap. It should be created if doesn't exist */
+static int dirty_bitmap_load_start(QEMUFile *f, DirtyBitmapLoadState *s)
+{
+ Error *local_err = NULL;
+ uint32_t granularity = qemu_get_be32(f);
+ uint8_t flags = qemu_get_byte(f);
+
+ if (s->bitmap) {
+ error_report("Bitmap with the same name ('%s') already exists on "
+ "destination", bdrv_dirty_bitmap_name(s->bitmap));
+ return -EINVAL;
+ } else {
+ s->bitmap = bdrv_create_dirty_bitmap(s->bs, granularity,
+ s->bitmap_name, &local_err);
+ if (!s->bitmap) {
+ error_report_err(local_err);
+ return -EINVAL;
+ }
+ }
+
+ if (flags & DIRTY_BITMAP_MIG_START_FLAG_RESERVED_MASK) {
+ error_report("Unknown flags in migrated dirty bitmap header: %x",
+ flags);
+ return -EINVAL;
+ }
+
+ if (flags & DIRTY_BITMAP_MIG_START_FLAG_PERSISTENT) {
+ bdrv_dirty_bitmap_set_persistance(s->bitmap, true);
+ }
+
+ bdrv_disable_dirty_bitmap(s->bitmap);
+ if (flags & DIRTY_BITMAP_MIG_START_FLAG_ENABLED) {
+ DirtyBitmapLoadBitmapState *b;
+
+ bdrv_dirty_bitmap_create_successor(s->bs, s->bitmap, &local_err);
+ if (local_err) {
+ error_report_err(local_err);
+ return -EINVAL;
+ }
+
+ b = g_new(DirtyBitmapLoadBitmapState, 1);
+ b->bs = s->bs;
+ b->bitmap = s->bitmap;
+ b->migrated = false;
+ enabled_bitmaps = g_slist_prepend(enabled_bitmaps, b);
+ }
+
+ return 0;
+}
+
+void dirty_bitmap_mig_before_vm_start(void)
+{
+ GSList *item;
+
+ qemu_mutex_lock(&finish_lock);
+
+ for (item = enabled_bitmaps; item; item = g_slist_next(item)) {
+ DirtyBitmapLoadBitmapState *b = item->data;
+
+ if (b->migrated) {
+ bdrv_enable_dirty_bitmap(b->bitmap);
+ } else {
+ bdrv_dirty_bitmap_enable_successor(b->bitmap);
+ }
+
+ g_free(b);
+ }
+
+ g_slist_free(enabled_bitmaps);
+ enabled_bitmaps = NULL;
+
+ qemu_mutex_unlock(&finish_lock);
+}
+
+static void dirty_bitmap_load_complete(QEMUFile *f, DirtyBitmapLoadState *s)
+{
+ GSList *item;
+ trace_dirty_bitmap_load_complete();
+ bdrv_dirty_bitmap_deserialize_finish(s->bitmap);
+
+ qemu_mutex_lock(&finish_lock);
+
+ for (item = enabled_bitmaps; item; item = g_slist_next(item)) {
+ DirtyBitmapLoadBitmapState *b = item->data;
+
+ if (b->bitmap == s->bitmap) {
+ b->migrated = true;
+ break;
+ }
+ }
+
+ if (bdrv_dirty_bitmap_frozen(s->bitmap)) {
+ bdrv_dirty_bitmap_lock(s->bitmap);
+ if (enabled_bitmaps == NULL) {
+ /* in postcopy */
+ bdrv_reclaim_dirty_bitmap_locked(s->bs, s->bitmap, &error_abort);
+ bdrv_enable_dirty_bitmap(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,
+ 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
+ * bitmap is empty
+ */
+ assert(ret == s->bitmap &&
+ count == bdrv_get_dirty_count(s->bitmap));
+ }
+ bdrv_dirty_bitmap_unlock(s->bitmap);
+ }
+
+ qemu_mutex_unlock(&finish_lock);
+}
+
+static int dirty_bitmap_load_bits(QEMUFile *f, DirtyBitmapLoadState *s)
+{
+ uint64_t first_byte = qemu_get_be64(f) << BDRV_SECTOR_BITS;
+ uint64_t nr_bytes = (uint64_t)qemu_get_be32(f) << BDRV_SECTOR_BITS;
+ trace_dirty_bitmap_load_bits_enter(first_byte >> BDRV_SECTOR_BITS,
+ nr_bytes >> BDRV_SECTOR_BITS);
+
+ if (s->flags & DIRTY_BITMAP_MIG_FLAG_ZEROES) {
+ trace_dirty_bitmap_load_bits_zeroes();
+ bdrv_dirty_bitmap_deserialize_zeroes(s->bitmap, first_byte, nr_bytes,
+ false);
+ } else {
+ size_t ret;
+ uint8_t *buf;
+ uint64_t buf_size = qemu_get_be64(f);
+ uint64_t needed_size =
+ bdrv_dirty_bitmap_serialization_size(s->bitmap,
+ first_byte, nr_bytes);
+
+ if (needed_size > buf_size ||
+ buf_size > QEMU_ALIGN_UP(needed_size, 4 * sizeof(long))
+ /* Here used same alignment as in send_bitmap_bits */
+ ) {
+ error_report("Migrated bitmap granularity doesn't "
+ "match the destination bitmap '%s' granularity",
+ bdrv_dirty_bitmap_name(s->bitmap));
+ return -EINVAL;
+ }
+
+ buf = g_malloc(buf_size);
+ ret = qemu_get_buffer(f, buf, buf_size);
+ if (ret != buf_size) {
+ error_report("Failed to read bitmap bits");
+ return -EIO;
+ }
+
+ bdrv_dirty_bitmap_deserialize_part(s->bitmap, buf, first_byte, nr_bytes,
+ false);
+ g_free(buf);
+ }
+
+ return 0;
+}
+
+static int dirty_bitmap_load_header(QEMUFile *f, DirtyBitmapLoadState *s)
+{
+ Error *local_err = NULL;
+ bool nothing;
+ s->flags = qemu_get_bitmap_flags(f);
+ trace_dirty_bitmap_load_header(s->flags);
+
+ nothing = s->flags == (s->flags & DIRTY_BITMAP_MIG_FLAG_EOS);
+
+ if (s->flags & DIRTY_BITMAP_MIG_FLAG_DEVICE_NAME) {
+ if (!qemu_get_counted_string(f, s->node_name)) {
+ error_report("Unable to read node name string");
+ return -EINVAL;
+ }
+ s->bs = bdrv_lookup_bs(s->node_name, s->node_name, &local_err);
+ if (!s->bs) {
+ error_report_err(local_err);
+ return -EINVAL;
+ }
+ } else if (!s->bs && !nothing) {
+ error_report("Error: block device name is not set");
+ return -EINVAL;
+ }
+
+ if (s->flags & DIRTY_BITMAP_MIG_FLAG_BITMAP_NAME) {
+ if (!qemu_get_counted_string(f, s->bitmap_name)) {
+ error_report("Unable to read bitmap name string");
+ return -EINVAL;
+ }
+ s->bitmap = bdrv_find_dirty_bitmap(s->bs, s->bitmap_name);
+
+ /* bitmap may be NULL here, it wouldn't be an error if it is the
+ * first occurrence of the bitmap */
+ if (!s->bitmap && !(s->flags & DIRTY_BITMAP_MIG_FLAG_START)) {
+ error_report("Error: unknown dirty bitmap "
+ "'%s' for block device '%s'",
+ s->bitmap_name, s->node_name);
+ return -EINVAL;
+ }
+ } else if (!s->bitmap && !nothing) {
+ error_report("Error: block device name is not set");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int dirty_bitmap_load(QEMUFile *f, void *opaque, int version_id)
+{
+ static DirtyBitmapLoadState s;
+ int ret = 0;
+
+ trace_dirty_bitmap_load_enter();
+
+ if (version_id != 1) {
+ return -EINVAL;
+ }
+
+ do {
+ ret = dirty_bitmap_load_header(f, &s);
+
+ if (s.flags & DIRTY_BITMAP_MIG_FLAG_START) {
+ ret = dirty_bitmap_load_start(f, &s);
+ } else if (s.flags & DIRTY_BITMAP_MIG_FLAG_COMPLETE) {
+ dirty_bitmap_load_complete(f, &s);
+ } else if (s.flags & DIRTY_BITMAP_MIG_FLAG_BITS) {
+ ret = dirty_bitmap_load_bits(f, &s);
+ }
+
+ if (!ret) {
+ ret = qemu_file_get_error(f);
+ }
+
+ if (ret) {
+ return ret;
+ }
+ } while (!(s.flags & DIRTY_BITMAP_MIG_FLAG_EOS));
+
+ trace_dirty_bitmap_load_success();
+ return 0;
+}
+
+static int dirty_bitmap_save_setup(QEMUFile *f, void *opaque)
+{
+ DirtyBitmapMigBitmapState *dbms = NULL;
+ if (init_dirty_bitmap_migration() < 0) {
+ return -1;
+ }
+
+ QSIMPLEQ_FOREACH(dbms, &dirty_bitmap_mig_state.dbms_list, entry) {
+ send_bitmap_start(f, dbms);
+ }
+ qemu_put_bitmap_flags(f, DIRTY_BITMAP_MIG_FLAG_EOS);
+
+ return 0;
+}
+
+static bool dirty_bitmap_is_active(void *opaque)
+{
+ return migrate_dirty_bitmaps() && !dirty_bitmap_mig_state.no_bitmaps;
+}
+
+static bool dirty_bitmap_is_active_iterate(void *opaque)
+{
+ return dirty_bitmap_is_active(opaque) && !runstate_is_running();
+}
+
+static bool dirty_bitmap_has_postcopy(void *opaque)
+{
+ return true;
+}
+
+static SaveVMHandlers savevm_dirty_bitmap_handlers = {
+ .save_setup = dirty_bitmap_save_setup,
+ .save_live_complete_postcopy = dirty_bitmap_save_complete,
+ .save_live_complete_precopy = dirty_bitmap_save_complete,
+ .has_postcopy = dirty_bitmap_has_postcopy,
+ .save_live_pending = dirty_bitmap_save_pending,
+ .save_live_iterate = dirty_bitmap_save_iterate,
+ .is_active_iterate = dirty_bitmap_is_active_iterate,
+ .load_state = dirty_bitmap_load,
+ .save_cleanup = dirty_bitmap_save_cleanup,
+ .is_active = dirty_bitmap_is_active,
+};
+
+void dirty_bitmap_mig_init(void)
+{
+ QSIMPLEQ_INIT(&dirty_bitmap_mig_state.dbms_list);
+
+ register_savevm_live(NULL, "dirty-bitmap", 0, 1,
+ &savevm_dirty_bitmap_handlers,
+ &dirty_bitmap_mig_state);
+}
diff --git a/migration/migration.c b/migration/migration.c
index 59b4fe6090..623f373326 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -157,6 +157,9 @@ MigrationIncomingState *migration_incoming_get_current(void)
memset(&mis_current, 0, sizeof(MigrationIncomingState));
qemu_mutex_init(&mis_current.rp_mutex);
qemu_event_init(&mis_current.main_thread_load_event, false);
+
+ init_dirty_bitmap_incoming_migration();
+
once = true;
}
return &mis_current;
@@ -320,6 +323,8 @@ static void process_incoming_migration_bh(void *opaque)
state, we need to obey autostart. Any other state is set with
runstate_set. */
+ dirty_bitmap_mig_before_vm_start();
+
if (!global_state_received() ||
global_state_get_runstate() == RUN_STATE_RUNNING) {
if (autostart) {
diff --git a/migration/migration.h b/migration/migration.h
index da6bc37de8..a79540b99c 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -235,4 +235,7 @@ void migrate_send_rp_pong(MigrationIncomingState *mis,
int migrate_send_rp_req_pages(MigrationIncomingState *mis, const char* rbname,
ram_addr_t start, size_t len);
+void dirty_bitmap_mig_before_vm_start(void);
+void init_dirty_bitmap_incoming_migration(void);
+
#endif
diff --git a/migration/savevm.c b/migration/savevm.c
index a60819ec2e..f1a11f742d 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1693,6 +1693,8 @@ static void loadvm_postcopy_handle_run_bh(void *opaque)
trace_loadvm_postcopy_handle_run_vmstart();
+ dirty_bitmap_mig_before_vm_start();
+
if (autostart) {
/* Hold onto your hats, starting the CPU */
vm_start();
diff --git a/migration/trace-events b/migration/trace-events
index 8ec84d82fd..314e1be6bc 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -227,3 +227,17 @@ colo_vm_state_change(const char *old, const char *new) "Change '%s' => '%s'"
colo_send_message(const char *msg) "Send '%s' message"
colo_receive_message(const char *msg) "Receive '%s' message"
colo_failover_set_state(const char *new_state) "new state %s"
+
+# migration/block-dirty-bitmap.c
+send_bitmap_header_enter(void) ""
+send_bitmap_bits(uint32_t flags, uint64_t start_sector, uint32_t nr_sectors, uint64_t data_size) "flags: 0x%x, start_sector: %" PRIu64 ", nr_sectors: %" PRIu32 ", data_size: %" PRIu64
+dirty_bitmap_save_iterate(int in_postcopy) "in postcopy: %d"
+dirty_bitmap_save_complete_enter(void) ""
+dirty_bitmap_save_complete_finish(void) ""
+dirty_bitmap_save_pending(uint64_t pending, uint64_t max_size) "pending %" PRIu64 " max: %" PRIu64
+dirty_bitmap_load_complete(void) ""
+dirty_bitmap_load_bits_enter(uint64_t first_sector, uint32_t nr_sectors) "chunk: %" PRIu64 " %" PRIu32
+dirty_bitmap_load_bits_zeroes(void) ""
+dirty_bitmap_load_header(uint32_t flags) "flags 0x%x"
+dirty_bitmap_load_enter(void) ""
+dirty_bitmap_load_success(void) ""
diff --git a/vl.c b/vl.c
index 3ef04ce991..5fa9e5f615 100644
--- a/vl.c
+++ b/vl.c
@@ -4502,6 +4502,7 @@ int main(int argc, char **argv, char **envp)
blk_mig_init();
ram_mig_init();
+ dirty_bitmap_mig_init();
/* If the currently selected machine wishes to override the units-per-bus
* property of its default HBA interface type, do so now. */
--
2.14.3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PULL 12/13] iotests: add dirty bitmap migration test
2018-03-13 21:14 [Qemu-devel] [PULL 00/13] Bitmaps patches John Snow
` (10 preceding siblings ...)
2018-03-13 21:14 ` [Qemu-devel] [PULL 11/13] migration: add postcopy migration of dirty bitmaps John Snow
@ 2018-03-13 21:14 ` John Snow
2018-03-13 21:14 ` [Qemu-devel] [PULL 13/13] iotests: add dirty bitmap postcopy test John Snow
` (2 subsequent siblings)
14 siblings, 0 replies; 24+ messages in thread
From: John Snow @ 2018-03-13 21:14 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, jsnow, Vladimir Sementsov-Ogievskiy
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
The test starts two vms (vm_a, vm_b), create dirty bitmap in
the first one, do several writes to corresponding device and
then migrate vm_a to vm_b.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20180313180320.339796-13-vsementsov@virtuozzo.com
---
tests/qemu-iotests/169 | 156 +++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/169.out | 5 ++
tests/qemu-iotests/group | 1 +
3 files changed, 162 insertions(+)
create mode 100755 tests/qemu-iotests/169
create mode 100644 tests/qemu-iotests/169.out
diff --git a/tests/qemu-iotests/169 b/tests/qemu-iotests/169
new file mode 100755
index 0000000000..3a8db91f6f
--- /dev/null
+++ b/tests/qemu-iotests/169
@@ -0,0 +1,156 @@
+#!/usr/bin/env python
+#
+# Tests for dirty bitmaps migration.
+#
+# Copyright (c) 2016-2017 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 os
+import iotests
+import time
+import itertools
+import operator
+import new
+from iotests import qemu_img
+
+
+disk_a = os.path.join(iotests.test_dir, 'disk_a')
+disk_b = os.path.join(iotests.test_dir, 'disk_b')
+size = '1M'
+mig_file = os.path.join(iotests.test_dir, 'mig_file')
+
+
+class TestDirtyBitmapMigration(iotests.QMPTestCase):
+ def tearDown(self):
+ self.vm_a.shutdown()
+ self.vm_b.shutdown()
+ os.remove(disk_a)
+ os.remove(disk_b)
+ os.remove(mig_file)
+
+ def setUp(self):
+ qemu_img('create', '-f', iotests.imgfmt, disk_a, size)
+ qemu_img('create', '-f', iotests.imgfmt, disk_b, size)
+
+ self.vm_a = iotests.VM(path_suffix='a').add_drive(disk_a)
+ self.vm_a.launch()
+
+ self.vm_b = iotests.VM(path_suffix='b')
+ self.vm_b.add_incoming("exec: cat '" + mig_file + "'")
+
+ def add_bitmap(self, vm, granularity, persistent):
+ params = {'node': 'drive0',
+ 'name': 'bitmap0',
+ 'granularity': granularity}
+ if persistent:
+ params['persistent'] = True
+ params['autoload'] = True
+
+ result = vm.qmp('block-dirty-bitmap-add', **params)
+ self.assert_qmp(result, 'return', {});
+
+ def get_bitmap_hash(self, vm):
+ result = vm.qmp('x-debug-block-dirty-bitmap-sha256',
+ node='drive0', name='bitmap0')
+ return result['return']['sha256']
+
+ def check_bitmap(self, vm, sha256):
+ result = vm.qmp('x-debug-block-dirty-bitmap-sha256',
+ node='drive0', name='bitmap0')
+ if sha256:
+ self.assert_qmp(result, 'return/sha256', sha256);
+ else:
+ self.assert_qmp(result, 'error/desc',
+ "Dirty bitmap 'bitmap0' not found");
+
+ def do_test_migration(self, persistent, migrate_bitmaps, online,
+ shared_storage):
+ granularity = 512
+
+ # regions = ((start, count), ...)
+ regions = ((0, 0x10000),
+ (0xf0000, 0x10000),
+ (0xa0201, 0x1000))
+
+ should_migrate = migrate_bitmaps or persistent and shared_storage
+
+ self.vm_b.add_drive(disk_a if shared_storage else disk_b)
+
+ if online:
+ os.mkfifo(mig_file)
+ self.vm_b.launch()
+
+ 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)
+
+ if migrate_bitmaps:
+ capabilities = [{'capability': 'dirty-bitmaps', 'state': True}]
+
+ result = self.vm_a.qmp('migrate-set-capabilities',
+ capabilities=capabilities)
+ self.assert_qmp(result, 'return', {})
+
+ if online:
+ result = self.vm_b.qmp('migrate-set-capabilities',
+ capabilities=capabilities)
+ self.assert_qmp(result, 'return', {})
+
+ result = self.vm_a.qmp('migrate-set-capabilities',
+ capabilities=[{'capability': 'events',
+ 'state': True}])
+ self.assert_qmp(result, 'return', {})
+
+ result = self.vm_a.qmp('migrate', uri='exec:cat>' + mig_file)
+ while True:
+ event = self.vm_a.event_wait('MIGRATION')
+ if event['data']['status'] == 'completed':
+ break
+
+ if not online:
+ self.vm_a.shutdown()
+ self.vm_b.launch()
+ # TODO enable bitmap capability for vm_b in this case
+
+ self.vm_b.event_wait("RESUME", timeout=10.0)
+
+ self.check_bitmap(self.vm_b, sha256 if should_migrate else False)
+
+ if should_migrate:
+ self.vm_b.shutdown()
+ self.vm_b.launch()
+ self.check_bitmap(self.vm_b, sha256 if persistent else False)
+
+
+def inject_test_case(klass, name, method, *args, **kwargs):
+ mc = operator.methodcaller(method, *args, **kwargs)
+ setattr(klass, 'test_' + name, new.instancemethod(mc, None, klass))
+
+for cmb in list(itertools.product((True, False), repeat=3)):
+ name = ('_' if cmb[0] else '_not_') + 'persistent_'
+ name += ('_' if cmb[1] else '_not_') + 'migbitmap_'
+ name += '_online' if cmb[2] else '_offline'
+
+ # TODO fix shared-storage bitmap migration and enable cases for it
+ args = list(cmb) + [False]
+
+ inject_test_case(TestDirtyBitmapMigration, name, 'do_test_migration',
+ *args)
+
+
+if __name__ == '__main__':
+ iotests.main(supported_fmts=['qcow2'])
diff --git a/tests/qemu-iotests/169.out b/tests/qemu-iotests/169.out
new file mode 100644
index 0000000000..594c16f49f
--- /dev/null
+++ b/tests/qemu-iotests/169.out
@@ -0,0 +1,5 @@
+........
+----------------------------------------------------------------------
+Ran 8 tests
+
+OK
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index c401791fcd..c7dc59897e 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -169,6 +169,7 @@
162 auto quick
163 rw auto quick
165 rw auto quick
+169 rw auto quick
170 rw auto quick
171 rw auto quick
172 auto
--
2.14.3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PULL 13/13] iotests: add dirty bitmap postcopy test
2018-03-13 21:14 [Qemu-devel] [PULL 00/13] Bitmaps patches John Snow
` (11 preceding siblings ...)
2018-03-13 21:14 ` [Qemu-devel] [PULL 12/13] iotests: add dirty bitmap migration test John Snow
@ 2018-03-13 21:14 ` John Snow
2018-03-13 21:36 ` [Qemu-devel] [PULL 00/13] Bitmaps patches no-reply
2018-03-16 15:05 ` Peter Maydell
14 siblings, 0 replies; 24+ messages in thread
From: John Snow @ 2018-03-13 21:14 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, jsnow, Vladimir Sementsov-Ogievskiy
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Test
- start two vms (vm_a, vm_b)
- in a
- do writes from set A
- do writes from set B
- fix bitmap sha256
- clear bitmap
- do writes from set A
- start migration
- than, in b
- wait vm start (postcopy should start)
- do writes from set B
- check bitmap sha256
The test should verify postcopy migration and then merging with delta
(changes in target, during postcopy process).
Reduce supported cache modes to only 'none', because with cache on time
from source.STOP to target.RESUME is unpredictable and we can fail with
timout while waiting for target.RESUME.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20180313180320.339796-14-vsementsov@virtuozzo.com
---
tests/qemu-iotests/199 | 118 ++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/199.out | 5 ++
tests/qemu-iotests/group | 1 +
tests/qemu-iotests/iotests.py | 7 ++-
4 files changed, 130 insertions(+), 1 deletion(-)
create mode 100755 tests/qemu-iotests/199
create mode 100644 tests/qemu-iotests/199.out
diff --git a/tests/qemu-iotests/199 b/tests/qemu-iotests/199
new file mode 100755
index 0000000000..651e8df5d9
--- /dev/null
+++ b/tests/qemu-iotests/199
@@ -0,0 +1,118 @@
+#!/usr/bin/env python
+#
+# Tests for dirty bitmaps postcopy migration.
+#
+# Copyright (c) 2016-2017 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 os
+import iotests
+import time
+from iotests import qemu_img
+
+disk_a = os.path.join(iotests.test_dir, 'disk_a')
+disk_b = os.path.join(iotests.test_dir, 'disk_b')
+size = '256G'
+fifo = os.path.join(iotests.test_dir, 'mig_fifo')
+
+class TestDirtyBitmapPostcopyMigration(iotests.QMPTestCase):
+
+ def tearDown(self):
+ self.vm_a.shutdown()
+ self.vm_b.shutdown()
+ os.remove(disk_a)
+ os.remove(disk_b)
+ os.remove(fifo)
+
+ def setUp(self):
+ os.mkfifo(fifo)
+ qemu_img('create', '-f', iotests.imgfmt, disk_a, size)
+ qemu_img('create', '-f', iotests.imgfmt, disk_b, size)
+ self.vm_a = iotests.VM(path_suffix='a').add_drive(disk_a)
+ self.vm_b = iotests.VM(path_suffix='b').add_drive(disk_b)
+ self.vm_b.add_incoming("exec: cat '" + fifo + "'")
+ self.vm_a.launch()
+ self.vm_b.launch()
+
+ def test_postcopy(self):
+ write_size = 0x40000000
+ granularity = 512
+ chunk = 4096
+
+ result = self.vm_a.qmp('block-dirty-bitmap-add', node='drive0',
+ name='bitmap', granularity=granularity)
+ self.assert_qmp(result, 'return', {});
+
+ s = 0
+ while s < write_size:
+ self.vm_a.hmp_qemu_io('drive0', 'write %d %d' % (s, chunk))
+ s += 0x10000
+ s = 0x8000
+ while s < write_size:
+ self.vm_a.hmp_qemu_io('drive0', 'write %d %d' % (s, chunk))
+ s += 0x10000
+
+ result = self.vm_a.qmp('x-debug-block-dirty-bitmap-sha256',
+ node='drive0', name='bitmap')
+ sha256 = result['return']['sha256']
+
+ result = self.vm_a.qmp('block-dirty-bitmap-clear', node='drive0',
+ name='bitmap')
+ self.assert_qmp(result, 'return', {});
+ s = 0
+ while s < write_size:
+ self.vm_a.hmp_qemu_io('drive0', 'write %d %d' % (s, chunk))
+ s += 0x10000
+
+ bitmaps_cap = {'capability': 'dirty-bitmaps', 'state': True}
+ events_cap = {'capability': 'events', 'state': True}
+
+ result = self.vm_a.qmp('migrate-set-capabilities',
+ capabilities=[bitmaps_cap, events_cap])
+ self.assert_qmp(result, 'return', {})
+
+ result = self.vm_b.qmp('migrate-set-capabilities',
+ capabilities=[bitmaps_cap])
+ self.assert_qmp(result, 'return', {})
+
+ result = self.vm_a.qmp('migrate', uri='exec:cat>' + fifo)
+ self.assert_qmp(result, 'return', {})
+
+ result = self.vm_a.qmp('migrate-start-postcopy')
+ self.assert_qmp(result, 'return', {})
+
+ while True:
+ event = self.vm_a.event_wait('MIGRATION')
+ if event['data']['status'] == 'completed':
+ break
+
+ s = 0x8000
+ while s < write_size:
+ self.vm_b.hmp_qemu_io('drive0', 'write %d %d' % (s, chunk))
+ s += 0x10000
+
+ result = self.vm_b.qmp('query-block');
+ while len(result['return'][0]['dirty-bitmaps']) > 1:
+ time.sleep(2)
+ result = self.vm_b.qmp('query-block');
+
+ result = self.vm_b.qmp('x-debug-block-dirty-bitmap-sha256',
+ node='drive0', name='bitmap')
+
+ self.assert_qmp(result, 'return/sha256', sha256);
+
+if __name__ == '__main__':
+ iotests.main(supported_fmts=['qcow2'], supported_cache_modes=['none'])
diff --git a/tests/qemu-iotests/199.out b/tests/qemu-iotests/199.out
new file mode 100644
index 0000000000..ae1213e6f8
--- /dev/null
+++ b/tests/qemu-iotests/199.out
@@ -0,0 +1,5 @@
+.
+----------------------------------------------------------------------
+Ran 1 tests
+
+OK
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index c7dc59897e..e7747fea3d 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -197,6 +197,7 @@
196 rw auto quick
197 rw auto quick
198 rw auto
+199 rw auto
200 rw auto
201 rw auto migration
202 rw auto quick
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 1bcc9ca57d..68b06c7d97 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -504,6 +504,10 @@ def verify_platform(supported_oses=['linux']):
if True not in [sys.platform.startswith(x) for x in supported_oses]:
notrun('not suitable for this OS: %s' % sys.platform)
+def verify_cache_mode(supported_cache_modes=[]):
+ if supported_cache_modes and (cachemode not in supported_cache_modes):
+ notrun('not suitable for this cache mode: %s' % cachemode)
+
def supports_quorum():
return 'quorum' in qemu_img_pipe('--help')
@@ -512,7 +516,7 @@ def verify_quorum():
if not supports_quorum():
notrun('quorum support missing')
-def main(supported_fmts=[], supported_oses=['linux']):
+def main(supported_fmts=[], supported_oses=['linux'], supported_cache_modes=[]):
'''Run tests'''
global debug
@@ -529,6 +533,7 @@ def main(supported_fmts=[], supported_oses=['linux']):
verbosity = 1
verify_image_format(supported_fmts)
verify_platform(supported_oses)
+ verify_cache_mode(supported_cache_modes)
# We need to filter out the time taken from the output so that qemu-iotest
# can reliably diff the results against master output.
--
2.14.3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PULL 00/13] Bitmaps patches
2018-03-13 21:14 [Qemu-devel] [PULL 00/13] Bitmaps patches John Snow
` (12 preceding siblings ...)
2018-03-13 21:14 ` [Qemu-devel] [PULL 13/13] iotests: add dirty bitmap postcopy test John Snow
@ 2018-03-13 21:36 ` no-reply
2018-03-16 17:44 ` Vladimir Sementsov-Ogievskiy
2018-03-16 15:05 ` Peter Maydell
14 siblings, 1 reply; 24+ messages in thread
From: no-reply @ 2018-03-13 21:36 UTC (permalink / raw)
To: jsnow; +Cc: famz, qemu-devel, peter.maydell
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20180313211441.5179-1-jsnow@redhat.com
Subject: [Qemu-devel] [PULL 00/13] Bitmaps patches
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
t [tag update] patchew/20180312172124.56461-1-dgilbert@redhat.com -> patchew/20180312172124.56461-1-dgilbert@redhat.com
* [new tag] patchew/20180313211441.5179-1-jsnow@redhat.com -> patchew/20180313211441.5179-1-jsnow@redhat.com
Switched to a new branch 'test'
6dbaff4909 iotests: add dirty bitmap postcopy test
970272a769 iotests: add dirty bitmap migration test
0a4cfd0266 migration: add postcopy migration of dirty bitmaps
39fdbb0e78 migration: allow qmp command migrate-start-postcopy for any postcopy
95be37b4df migration: add is_active_iterate handler
ebce5a11c9 migration/qemu-file: add qemu_put_counted_string()
b2e3a3e0f7 migration: include migrate_dirty_bitmaps in migrate_postcopy
d234483b40 qapi: add dirty-bitmaps migration capability
6b24eebd32 migration: introduce postcopy-only pending
a8ca389528 dirty-bitmap: add locked state
48cfc1176c block/dirty-bitmap: add _locked version of bdrv_reclaim_dirty_bitmap
8bc9e17e62 block/dirty-bitmap: fix locking in bdrv_reclaim_dirty_bitmap
ce2cf17b98 block/dirty-bitmap: add bdrv_dirty_bitmap_enable_successor()
=== OUTPUT BEGIN ===
Checking PATCH 1/13: block/dirty-bitmap: add bdrv_dirty_bitmap_enable_successor()...
Checking PATCH 2/13: block/dirty-bitmap: fix locking in bdrv_reclaim_dirty_bitmap...
Checking PATCH 3/13: block/dirty-bitmap: add _locked version of bdrv_reclaim_dirty_bitmap...
Checking PATCH 4/13: dirty-bitmap: add locked state...
Checking PATCH 5/13: migration: introduce postcopy-only pending...
Checking PATCH 6/13: qapi: add dirty-bitmaps migration capability...
Checking PATCH 7/13: migration: include migrate_dirty_bitmaps in migrate_postcopy...
Checking PATCH 8/13: migration/qemu-file: add qemu_put_counted_string()...
Checking PATCH 9/13: migration: add is_active_iterate handler...
Checking PATCH 10/13: migration: allow qmp command migrate-start-postcopy for any postcopy...
Checking PATCH 11/13: migration: add postcopy migration of dirty bitmaps...
ERROR: braces {} are necessary for all arms of this statement
#740: FILE: migration/block-dirty-bitmap.c:690:
+ } while (!(s.flags & DIRTY_BITMAP_MIG_FLAG_EOS));
[...]
total: 1 errors, 0 warnings, 816 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 12/13: iotests: add dirty bitmap migration test...
Checking PATCH 13/13: iotests: add dirty bitmap postcopy test...
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PULL 00/13] Bitmaps patches
2018-03-13 21:14 [Qemu-devel] [PULL 00/13] Bitmaps patches John Snow
` (13 preceding siblings ...)
2018-03-13 21:36 ` [Qemu-devel] [PULL 00/13] Bitmaps patches no-reply
@ 2018-03-16 15:05 ` Peter Maydell
14 siblings, 0 replies; 24+ messages in thread
From: Peter Maydell @ 2018-03-16 15:05 UTC (permalink / raw)
To: John Snow; +Cc: QEMU Developers
On 13 March 2018 at 21:14, John Snow <jsnow@redhat.com> wrote:
> The following changes since commit 026aaf47c02b79036feb830206cfebb2a726510d:
>
> Merge remote-tracking branch 'remotes/ehabkost/tags/python-next-pull-request' into staging (2018-03-13 16:26:44 +0000)
>
> are available in the Git repository at:
>
> https://github.com/jnsnow/qemu.git tags/bitmaps-pull-request
>
> for you to fetch changes up to ac8bd439bb7b5dffeb5ff8a17317ca2b192044b6:
>
> iotests: add dirty bitmap postcopy test (2018-03-13 17:06:32 -0400)
>
> ----------------------------------------------------------------
>
> PR touching migration sent with permission (and reviews) from
> David Gilbert.
>
> ----------------------------------------------------------------
Applied, thanks.
-- PMM
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PULL 00/13] Bitmaps patches
2018-03-13 21:36 ` [Qemu-devel] [PULL 00/13] Bitmaps patches no-reply
@ 2018-03-16 17:44 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 24+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2018-03-16 17:44 UTC (permalink / raw)
To: qemu-devel, no-reply, jsnow; +Cc: peter.maydell, famz
Just note here too, that this relates to bug in checkpatch, work on
fixing is in progress:
https://lists.nongnu.org/archive/html/qemu-devel/2018-03/msg04651.html
14.03.2018 00:36, no-reply@patchew.org wrote:
> Hi,
>
> This series seems to have some coding style problems. See output below for
> more information:
>
> Type: series
> Message-id: 20180313211441.5179-1-jsnow@redhat.com
> Subject: [Qemu-devel] [PULL 00/13] Bitmaps patches
>
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
>
> BASE=base
> n=1
> total=$(git log --oneline $BASE.. | wc -l)
> failed=0
>
> git config --local diff.renamelimit 0
> git config --local diff.renames True
> git config --local diff.algorithm histogram
>
> commits="$(git log --format=%H --reverse $BASE..)"
> for c in $commits; do
> echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
> if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
> failed=1
> echo
> fi
> n=$((n+1))
> done
>
> exit $failed
> === TEST SCRIPT END ===
>
> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> From https://github.com/patchew-project/qemu
> t [tag update] patchew/20180312172124.56461-1-dgilbert@redhat.com -> patchew/20180312172124.56461-1-dgilbert@redhat.com
> * [new tag] patchew/20180313211441.5179-1-jsnow@redhat.com -> patchew/20180313211441.5179-1-jsnow@redhat.com
> Switched to a new branch 'test'
> 6dbaff4909 iotests: add dirty bitmap postcopy test
> 970272a769 iotests: add dirty bitmap migration test
> 0a4cfd0266 migration: add postcopy migration of dirty bitmaps
> 39fdbb0e78 migration: allow qmp command migrate-start-postcopy for any postcopy
> 95be37b4df migration: add is_active_iterate handler
> ebce5a11c9 migration/qemu-file: add qemu_put_counted_string()
> b2e3a3e0f7 migration: include migrate_dirty_bitmaps in migrate_postcopy
> d234483b40 qapi: add dirty-bitmaps migration capability
> 6b24eebd32 migration: introduce postcopy-only pending
> a8ca389528 dirty-bitmap: add locked state
> 48cfc1176c block/dirty-bitmap: add _locked version of bdrv_reclaim_dirty_bitmap
> 8bc9e17e62 block/dirty-bitmap: fix locking in bdrv_reclaim_dirty_bitmap
> ce2cf17b98 block/dirty-bitmap: add bdrv_dirty_bitmap_enable_successor()
>
> === OUTPUT BEGIN ===
> Checking PATCH 1/13: block/dirty-bitmap: add bdrv_dirty_bitmap_enable_successor()...
> Checking PATCH 2/13: block/dirty-bitmap: fix locking in bdrv_reclaim_dirty_bitmap...
> Checking PATCH 3/13: block/dirty-bitmap: add _locked version of bdrv_reclaim_dirty_bitmap...
> Checking PATCH 4/13: dirty-bitmap: add locked state...
> Checking PATCH 5/13: migration: introduce postcopy-only pending...
> Checking PATCH 6/13: qapi: add dirty-bitmaps migration capability...
> Checking PATCH 7/13: migration: include migrate_dirty_bitmaps in migrate_postcopy...
> Checking PATCH 8/13: migration/qemu-file: add qemu_put_counted_string()...
> Checking PATCH 9/13: migration: add is_active_iterate handler...
> Checking PATCH 10/13: migration: allow qmp command migrate-start-postcopy for any postcopy...
> Checking PATCH 11/13: migration: add postcopy migration of dirty bitmaps...
> ERROR: braces {} are necessary for all arms of this statement
> #740: FILE: migration/block-dirty-bitmap.c:690:
> + } while (!(s.flags & DIRTY_BITMAP_MIG_FLAG_EOS));
> [...]
>
> total: 1 errors, 0 warnings, 816 lines checked
>
> Your patch has style problems, please review. If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
>
> Checking PATCH 12/13: iotests: add dirty bitmap migration test...
> Checking PATCH 13/13: iotests: add dirty bitmap postcopy test...
> === OUTPUT END ===
>
> Test command exited with code: 1
>
>
> ---
> Email generated automatically by Patchew [http://patchew.org/].
> Please send your feedback to patchew-devel@freelists.org
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PULL 11/13] migration: add postcopy migration of dirty bitmaps
2018-03-13 21:14 ` [Qemu-devel] [PULL 11/13] migration: add postcopy migration of dirty bitmaps John Snow
@ 2018-04-27 13:22 ` Peter Maydell
2018-04-30 18:29 ` John Snow
2018-06-20 16:43 ` Peter Maydell
2018-04-27 13:24 ` Peter Maydell
1 sibling, 2 replies; 24+ messages in thread
From: Peter Maydell @ 2018-04-27 13:22 UTC (permalink / raw)
To: John Snow; +Cc: QEMU Developers, Vladimir Sementsov-Ogievskiy
On 13 March 2018 at 21:14, John Snow <jsnow@redhat.com> wrote:
> From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>
> Postcopy migration of dirty bitmaps. Only named dirty bitmaps are migrated.
>
> If destination qemu is already containing a dirty bitmap with the same name
> as a migrated bitmap (for the same node), then, if their granularities are
> the same the migration will be done, otherwise the error will be generated.
>
> If destination qemu doesn't contain such bitmap it will be created.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Message-id: 20180313180320.339796-12-vsementsov@virtuozzo.com
> [Changed '+' to '*' as per list discussion. --js]
> Signed-off-by: John Snow <jsnow@redhat.com>
> +static int init_dirty_bitmap_migration(void)
> +{
Hi; Coverity (CID1390625) complains about a possible dereference
after NULL check in this function:
> + BlockDriverState *bs;
> + BdrvDirtyBitmap *bitmap;
> + DirtyBitmapMigBitmapState *dbms;
> + BdrvNextIterator it;
> +
> + dirty_bitmap_mig_state.bulk_completed = false;
> + dirty_bitmap_mig_state.prev_bs = NULL;
> + dirty_bitmap_mig_state.prev_bitmap = NULL;
> + dirty_bitmap_mig_state.no_bitmaps = false;
> +
> + for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
> + const char *drive_name = bdrv_get_device_or_node_name(bs);
> +
> + /* skip automatically inserted nodes */
> + while (bs && bs->drv && bs->implicit) {
> + bs = backing_bs(bs);
> + }
The 'bs' test in this while() loop implies that we might
leave the loop because bs == NULL...
> +
> + for (bitmap = bdrv_dirty_bitmap_next(bs, NULL); bitmap;
...but this call to bdrv_dirty_bitmap_next() will always
dereference bs, so if it's NULL we'll crash.
> + bitmap = bdrv_dirty_bitmap_next(bs, bitmap))
thanks
-- PMM
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PULL 11/13] migration: add postcopy migration of dirty bitmaps
2018-03-13 21:14 ` [Qemu-devel] [PULL 11/13] migration: add postcopy migration of dirty bitmaps John Snow
2018-04-27 13:22 ` Peter Maydell
@ 2018-04-27 13:24 ` Peter Maydell
1 sibling, 0 replies; 24+ messages in thread
From: Peter Maydell @ 2018-04-27 13:24 UTC (permalink / raw)
To: John Snow; +Cc: QEMU Developers, Vladimir Sementsov-Ogievskiy
On 13 March 2018 at 21:14, John Snow <jsnow@redhat.com> wrote:
> From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>
> Postcopy migration of dirty bitmaps. Only named dirty bitmaps are migrated.
>
> If destination qemu is already containing a dirty bitmap with the same name
> as a migrated bitmap (for the same node), then, if their granularities are
> the same the migration will be done, otherwise the error will be generated.
>
> If destination qemu doesn't contain such bitmap it will be created.
Hi. Coverity complains (CID1390627) about a resource leak in this function;
> +static int dirty_bitmap_load_bits(QEMUFile *f, DirtyBitmapLoadState *s)
> +{
> + uint64_t first_byte = qemu_get_be64(f) << BDRV_SECTOR_BITS;
> + uint64_t nr_bytes = (uint64_t)qemu_get_be32(f) << BDRV_SECTOR_BITS;
> + trace_dirty_bitmap_load_bits_enter(first_byte >> BDRV_SECTOR_BITS,
> + nr_bytes >> BDRV_SECTOR_BITS);
> +
> + if (s->flags & DIRTY_BITMAP_MIG_FLAG_ZEROES) {
> + trace_dirty_bitmap_load_bits_zeroes();
> + bdrv_dirty_bitmap_deserialize_zeroes(s->bitmap, first_byte, nr_bytes,
> + false);
> + } else {
> + size_t ret;
> + uint8_t *buf;
> + uint64_t buf_size = qemu_get_be64(f);
> + uint64_t needed_size =
> + bdrv_dirty_bitmap_serialization_size(s->bitmap,
> + first_byte, nr_bytes);
> +
> + if (needed_size > buf_size ||
> + buf_size > QEMU_ALIGN_UP(needed_size, 4 * sizeof(long))
> + /* Here used same alignment as in send_bitmap_bits */
> + ) {
> + error_report("Migrated bitmap granularity doesn't "
> + "match the destination bitmap '%s' granularity",
> + bdrv_dirty_bitmap_name(s->bitmap));
> + return -EINVAL;
> + }
> +
> + buf = g_malloc(buf_size);
Here we allocate memory into buf...
> + ret = qemu_get_buffer(f, buf, buf_size);
> + if (ret != buf_size) {
> + error_report("Failed to read bitmap bits");
> + return -EIO;
...but in this error-exit path we do not free it.
> + }
> +
> + bdrv_dirty_bitmap_deserialize_part(s->bitmap, buf, first_byte, nr_bytes,
> + false);
> + g_free(buf);
> + }
> +
> + return 0;
> +}
thanks
-- PMM
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PULL 11/13] migration: add postcopy migration of dirty bitmaps
2018-04-27 13:22 ` Peter Maydell
@ 2018-04-30 18:29 ` John Snow
2018-06-20 16:43 ` Peter Maydell
1 sibling, 0 replies; 24+ messages in thread
From: John Snow @ 2018-04-30 18:29 UTC (permalink / raw)
To: Peter Maydell; +Cc: QEMU Developers, Vladimir Sementsov-Ogievskiy
On 04/27/2018 09:22 AM, Peter Maydell wrote:
> On 13 March 2018 at 21:14, John Snow <jsnow@redhat.com> wrote:
>> From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>
>> Postcopy migration of dirty bitmaps. Only named dirty bitmaps are migrated.
>>
>> If destination qemu is already containing a dirty bitmap with the same name
>> as a migrated bitmap (for the same node), then, if their granularities are
>> the same the migration will be done, otherwise the error will be generated.
>>
>> If destination qemu doesn't contain such bitmap it will be created.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> Message-id: 20180313180320.339796-12-vsementsov@virtuozzo.com
>> [Changed '+' to '*' as per list discussion. --js]
>> Signed-off-by: John Snow <jsnow@redhat.com>
>
>> +static int init_dirty_bitmap_migration(void)
>> +{
>
> Hi; Coverity (CID1390625) complains about a possible dereference
> after NULL check in this function:
>
>> + BlockDriverState *bs;
>> + BdrvDirtyBitmap *bitmap;
>> + DirtyBitmapMigBitmapState *dbms;
>> + BdrvNextIterator it;
>> +
>> + dirty_bitmap_mig_state.bulk_completed = false;
>> + dirty_bitmap_mig_state.prev_bs = NULL;
>> + dirty_bitmap_mig_state.prev_bitmap = NULL;
>> + dirty_bitmap_mig_state.no_bitmaps = false;
>> +
>> + for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
>> + const char *drive_name = bdrv_get_device_or_node_name(bs);
>> +
>> + /* skip automatically inserted nodes */
>> + while (bs && bs->drv && bs->implicit) {
>> + bs = backing_bs(bs);
>> + }
>
> The 'bs' test in this while() loop implies that we might
> leave the loop because bs == NULL...
>
>> +
>> + for (bitmap = bdrv_dirty_bitmap_next(bs, NULL); bitmap;
>
> ...but this call to bdrv_dirty_bitmap_next() will always
> dereference bs, so if it's NULL we'll crash.
>
>> + bitmap = bdrv_dirty_bitmap_next(bs, bitmap))
>
> thanks
> -- PMM
>
I have some patches on the way to clean up this file. Sorry for the delay.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PULL 11/13] migration: add postcopy migration of dirty bitmaps
2018-04-27 13:22 ` Peter Maydell
2018-04-30 18:29 ` John Snow
@ 2018-06-20 16:43 ` Peter Maydell
2018-06-20 16:58 ` John Snow
1 sibling, 1 reply; 24+ messages in thread
From: Peter Maydell @ 2018-06-20 16:43 UTC (permalink / raw)
To: John Snow; +Cc: QEMU Developers, Vladimir Sementsov-Ogievskiy
On 27 April 2018 at 14:22, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 13 March 2018 at 21:14, John Snow <jsnow@redhat.com> wrote:
>> From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>
>> Postcopy migration of dirty bitmaps. Only named dirty bitmaps are migrated.
>>
>> If destination qemu is already containing a dirty bitmap with the same name
>> as a migrated bitmap (for the same node), then, if their granularities are
>> the same the migration will be done, otherwise the error will be generated.
>>
>> If destination qemu doesn't contain such bitmap it will be created.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> Message-id: 20180313180320.339796-12-vsementsov@virtuozzo.com
>> [Changed '+' to '*' as per list discussion. --js]
>> Signed-off-by: John Snow <jsnow@redhat.com>
>
>> +static int init_dirty_bitmap_migration(void)
>> +{
>
> Hi; Coverity (CID1390625) complains about a possible dereference
> after NULL check in this function:
>
>> + BlockDriverState *bs;
>> + BdrvDirtyBitmap *bitmap;
>> + DirtyBitmapMigBitmapState *dbms;
>> + BdrvNextIterator it;
>> +
>> + dirty_bitmap_mig_state.bulk_completed = false;
>> + dirty_bitmap_mig_state.prev_bs = NULL;
>> + dirty_bitmap_mig_state.prev_bitmap = NULL;
>> + dirty_bitmap_mig_state.no_bitmaps = false;
>> +
>> + for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
>> + const char *drive_name = bdrv_get_device_or_node_name(bs);
>> +
>> + /* skip automatically inserted nodes */
>> + while (bs && bs->drv && bs->implicit) {
>> + bs = backing_bs(bs);
>> + }
>
> The 'bs' test in this while() loop implies that we might
> leave the loop because bs == NULL...
>
>> +
>> + for (bitmap = bdrv_dirty_bitmap_next(bs, NULL); bitmap;
>
> ...but this call to bdrv_dirty_bitmap_next() will always
> dereference bs, so if it's NULL we'll crash.
>
>> + bitmap = bdrv_dirty_bitmap_next(bs, bitmap))
Hi -- just a nudge that Coverity thinks this one is still unfixed.
thanks
-- PMM
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PULL 11/13] migration: add postcopy migration of dirty bitmaps
2018-06-20 16:43 ` Peter Maydell
@ 2018-06-20 16:58 ` John Snow
2018-10-16 12:25 ` Peter Maydell
0 siblings, 1 reply; 24+ messages in thread
From: John Snow @ 2018-06-20 16:58 UTC (permalink / raw)
To: Peter Maydell; +Cc: QEMU Developers, Vladimir Sementsov-Ogievskiy
On 06/20/2018 12:43 PM, Peter Maydell wrote:
> On 27 April 2018 at 14:22, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 13 March 2018 at 21:14, John Snow <jsnow@redhat.com> wrote:
>>> From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>>
>>> Postcopy migration of dirty bitmaps. Only named dirty bitmaps are migrated.
>>>
>>> If destination qemu is already containing a dirty bitmap with the same name
>>> as a migrated bitmap (for the same node), then, if their granularities are
>>> the same the migration will be done, otherwise the error will be generated.
>>>
>>> If destination qemu doesn't contain such bitmap it will be created.
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>>> Message-id: 20180313180320.339796-12-vsementsov@virtuozzo.com
>>> [Changed '+' to '*' as per list discussion. --js]
>>> Signed-off-by: John Snow <jsnow@redhat.com>
>>
>>> +static int init_dirty_bitmap_migration(void)
>>> +{
>>
>> Hi; Coverity (CID1390625) complains about a possible dereference
>> after NULL check in this function:
>>
>>> + BlockDriverState *bs;
>>> + BdrvDirtyBitmap *bitmap;
>>> + DirtyBitmapMigBitmapState *dbms;
>>> + BdrvNextIterator it;
>>> +
>>> + dirty_bitmap_mig_state.bulk_completed = false;
>>> + dirty_bitmap_mig_state.prev_bs = NULL;
>>> + dirty_bitmap_mig_state.prev_bitmap = NULL;
>>> + dirty_bitmap_mig_state.no_bitmaps = false;
>>> +
>>> + for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
>>> + const char *drive_name = bdrv_get_device_or_node_name(bs);
>>> +
>>> + /* skip automatically inserted nodes */
>>> + while (bs && bs->drv && bs->implicit) {
>>> + bs = backing_bs(bs);
>>> + }
>>
>> The 'bs' test in this while() loop implies that we might
>> leave the loop because bs == NULL...
>>
>>> +
>>> + for (bitmap = bdrv_dirty_bitmap_next(bs, NULL); bitmap;
>>
>> ...but this call to bdrv_dirty_bitmap_next() will always
>> dereference bs, so if it's NULL we'll crash.
>>
>>> + bitmap = bdrv_dirty_bitmap_next(bs, bitmap))
>
> Hi -- just a nudge that Coverity thinks this one is still unfixed.
>
> thanks
> -- PMM
>
Thank you for the reminder, I've been a bit scatter-brained recently.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PULL 11/13] migration: add postcopy migration of dirty bitmaps
2018-06-20 16:58 ` John Snow
@ 2018-10-16 12:25 ` Peter Maydell
2018-10-16 13:19 ` Vladimir Sementsov-Ogievskiy
0 siblings, 1 reply; 24+ messages in thread
From: Peter Maydell @ 2018-10-16 12:25 UTC (permalink / raw)
To: John Snow; +Cc: QEMU Developers, Vladimir Sementsov-Ogievskiy
On 20 June 2018 at 17:58, John Snow <jsnow@redhat.com> wrote:
>
>
> On 06/20/2018 12:43 PM, Peter Maydell wrote:
>> On 27 April 2018 at 14:22, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> On 13 March 2018 at 21:14, John Snow <jsnow@redhat.com> wrote:
>>>> From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>>>
>>>> Postcopy migration of dirty bitmaps. Only named dirty bitmaps are migrated.
>>>>
>>>> If destination qemu is already containing a dirty bitmap with the same name
>>>> as a migrated bitmap (for the same node), then, if their granularities are
>>>> the same the migration will be done, otherwise the error will be generated.
>>>>
>>>> If destination qemu doesn't contain such bitmap it will be created.
>>>>
>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>>>> Message-id: 20180313180320.339796-12-vsementsov@virtuozzo.com
>>>> [Changed '+' to '*' as per list discussion. --js]
>>>> Signed-off-by: John Snow <jsnow@redhat.com>
>>>
>>>> +static int init_dirty_bitmap_migration(void)
>>>> +{
>>>
>>> Hi; Coverity (CID1390625) complains about a possible dereference
>>> after NULL check in this function:
>>>
>>>> + BlockDriverState *bs;
>>>> + BdrvDirtyBitmap *bitmap;
>>>> + DirtyBitmapMigBitmapState *dbms;
>>>> + BdrvNextIterator it;
>>>> +
>>>> + dirty_bitmap_mig_state.bulk_completed = false;
>>>> + dirty_bitmap_mig_state.prev_bs = NULL;
>>>> + dirty_bitmap_mig_state.prev_bitmap = NULL;
>>>> + dirty_bitmap_mig_state.no_bitmaps = false;
>>>> +
>>>> + for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
>>>> + const char *drive_name = bdrv_get_device_or_node_name(bs);
>>>> +
>>>> + /* skip automatically inserted nodes */
>>>> + while (bs && bs->drv && bs->implicit) {
>>>> + bs = backing_bs(bs);
>>>> + }
>>>
>>> The 'bs' test in this while() loop implies that we might
>>> leave the loop because bs == NULL...
>>>
>>>> +
>>>> + for (bitmap = bdrv_dirty_bitmap_next(bs, NULL); bitmap;
>>>
>>> ...but this call to bdrv_dirty_bitmap_next() will always
>>> dereference bs, so if it's NULL we'll crash.
>>>
>>>> + bitmap = bdrv_dirty_bitmap_next(bs, bitmap))
>>
>> Hi -- just a nudge that Coverity thinks this one is still unfixed.
> Thank you for the reminder, I've been a bit scatter-brained recently.
Ping? This is still in Coverity's list of unfixed issues.
thanks
-- PMM
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PULL 11/13] migration: add postcopy migration of dirty bitmaps
2018-10-16 12:25 ` Peter Maydell
@ 2018-10-16 13:19 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 24+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2018-10-16 13:19 UTC (permalink / raw)
To: Peter Maydell, John Snow; +Cc: QEMU Developers
16.10.2018 15:25, Peter Maydell wrote:
> On 20 June 2018 at 17:58, John Snow <jsnow@redhat.com> wrote:
>>
>> On 06/20/2018 12:43 PM, Peter Maydell wrote:
>>> On 27 April 2018 at 14:22, Peter Maydell <peter.maydell@linaro.org> wrote:
>>>> On 13 March 2018 at 21:14, John Snow <jsnow@redhat.com> wrote:
>>>>> From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>>>>
>>>>> Postcopy migration of dirty bitmaps. Only named dirty bitmaps are migrated.
>>>>>
>>>>> If destination qemu is already containing a dirty bitmap with the same name
>>>>> as a migrated bitmap (for the same node), then, if their granularities are
>>>>> the same the migration will be done, otherwise the error will be generated.
>>>>>
>>>>> If destination qemu doesn't contain such bitmap it will be created.
>>>>>
>>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>>>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>>>>> Message-id: 20180313180320.339796-12-vsementsov@virtuozzo.com
>>>>> [Changed '+' to '*' as per list discussion. --js]
>>>>> Signed-off-by: John Snow <jsnow@redhat.com>
>>>>> +static int init_dirty_bitmap_migration(void)
>>>>> +{
>>>> Hi; Coverity (CID1390625) complains about a possible dereference
>>>> after NULL check in this function:
>>>>
>>>>> + BlockDriverState *bs;
>>>>> + BdrvDirtyBitmap *bitmap;
>>>>> + DirtyBitmapMigBitmapState *dbms;
>>>>> + BdrvNextIterator it;
>>>>> +
>>>>> + dirty_bitmap_mig_state.bulk_completed = false;
>>>>> + dirty_bitmap_mig_state.prev_bs = NULL;
>>>>> + dirty_bitmap_mig_state.prev_bitmap = NULL;
>>>>> + dirty_bitmap_mig_state.no_bitmaps = false;
>>>>> +
>>>>> + for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
>>>>> + const char *drive_name = bdrv_get_device_or_node_name(bs);
>>>>> +
>>>>> + /* skip automatically inserted nodes */
>>>>> + while (bs && bs->drv && bs->implicit) {
>>>>> + bs = backing_bs(bs);
>>>>> + }
>>>> The 'bs' test in this while() loop implies that we might
>>>> leave the loop because bs == NULL...
>>>>
>>>>> +
>>>>> + for (bitmap = bdrv_dirty_bitmap_next(bs, NULL); bitmap;
>>>> ...but this call to bdrv_dirty_bitmap_next() will always
>>>> dereference bs, so if it's NULL we'll crash.
>>>>
>>>>> + bitmap = bdrv_dirty_bitmap_next(bs, bitmap))
>>> Hi -- just a nudge that Coverity thinks this one is still unfixed.
>> Thank you for the reminder, I've been a bit scatter-brained recently.
> Ping? This is still in Coverity's list of unfixed issues.
>
> thanks
> -- PMM
Will send in few seconds, sorry for such a terrible delay :(
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2018-10-16 13:19 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-13 21:14 [Qemu-devel] [PULL 00/13] Bitmaps patches John Snow
2018-03-13 21:14 ` [Qemu-devel] [PULL 01/13] block/dirty-bitmap: add bdrv_dirty_bitmap_enable_successor() John Snow
2018-03-13 21:14 ` [Qemu-devel] [PULL 02/13] block/dirty-bitmap: fix locking in bdrv_reclaim_dirty_bitmap John Snow
2018-03-13 21:14 ` [Qemu-devel] [PULL 03/13] block/dirty-bitmap: add _locked version of bdrv_reclaim_dirty_bitmap John Snow
2018-03-13 21:14 ` [Qemu-devel] [PULL 04/13] dirty-bitmap: add locked state John Snow
2018-03-13 21:14 ` [Qemu-devel] [PULL 05/13] migration: introduce postcopy-only pending John Snow
2018-03-13 21:14 ` [Qemu-devel] [PULL 06/13] qapi: add dirty-bitmaps migration capability John Snow
2018-03-13 21:14 ` [Qemu-devel] [PULL 07/13] migration: include migrate_dirty_bitmaps in migrate_postcopy John Snow
2018-03-13 21:14 ` [Qemu-devel] [PULL 08/13] migration/qemu-file: add qemu_put_counted_string() John Snow
2018-03-13 21:14 ` [Qemu-devel] [PULL 09/13] migration: add is_active_iterate handler John Snow
2018-03-13 21:14 ` [Qemu-devel] [PULL 10/13] migration: allow qmp command migrate-start-postcopy for any postcopy John Snow
2018-03-13 21:14 ` [Qemu-devel] [PULL 11/13] migration: add postcopy migration of dirty bitmaps John Snow
2018-04-27 13:22 ` Peter Maydell
2018-04-30 18:29 ` John Snow
2018-06-20 16:43 ` Peter Maydell
2018-06-20 16:58 ` John Snow
2018-10-16 12:25 ` Peter Maydell
2018-10-16 13:19 ` Vladimir Sementsov-Ogievskiy
2018-04-27 13:24 ` Peter Maydell
2018-03-13 21:14 ` [Qemu-devel] [PULL 12/13] iotests: add dirty bitmap migration test John Snow
2018-03-13 21:14 ` [Qemu-devel] [PULL 13/13] iotests: add dirty bitmap postcopy test John Snow
2018-03-13 21:36 ` [Qemu-devel] [PULL 00/13] Bitmaps patches no-reply
2018-03-16 17:44 ` Vladimir Sementsov-Ogievskiy
2018-03-16 15:05 ` Peter Maydell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.