* [Qemu-devel] [PATCH v2 0/4] Block migration (migrate -b) fixes
@ 2017-05-30 15:22 Kevin Wolf
2017-05-30 15:22 ` [Qemu-devel] [PATCH v2 1/4] block: Fix anonymous BBs in blk_root_inactivate() Kevin Wolf
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Kevin Wolf @ 2017-05-30 15:22 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, eblake, qemu-devel
v2:
- Detect when migrate -b is compiled out [Eric]
- Switched the whole test to QMP for this, HMP is a bit hard to
deal with
Kevin Wolf (4):
block: Fix anonymous BBs in blk_root_inactivate()
migration: Inactivate images after .save_live_complete_precopy()
migration/block: Clean up BBs in block_save_complete()
qemu-iotests: Block migration test
block/block-backend.c | 2 +-
migration/block.c | 22 +++++--
migration/migration.c | 12 ++--
tests/qemu-iotests/183 | 143 +++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/183.out | 46 +++++++++++++++
tests/qemu-iotests/group | 1 +
6 files changed, 215 insertions(+), 11 deletions(-)
create mode 100755 tests/qemu-iotests/183
create mode 100644 tests/qemu-iotests/183.out
--
1.8.3.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v2 1/4] block: Fix anonymous BBs in blk_root_inactivate()
2017-05-30 15:22 [Qemu-devel] [PATCH v2 0/4] Block migration (migrate -b) fixes Kevin Wolf
@ 2017-05-30 15:22 ` Kevin Wolf
2017-05-30 15:28 ` [Qemu-devel] [Qemu-block] " Jeff Cody
2017-05-30 15:22 ` [Qemu-devel] [PATCH v2 2/4] migration: Inactivate images after .save_live_complete_precopy() Kevin Wolf
` (2 subsequent siblings)
3 siblings, 1 reply; 13+ messages in thread
From: Kevin Wolf @ 2017-05-30 15:22 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, eblake, qemu-devel
blk->name isn't an array, but a pointer that can be NULL. Checking for
an anonymous BB must involve a NULL check first, otherwise we get
crashes.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/block-backend.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index f3a6008..7d7f369 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -168,7 +168,7 @@ static int blk_root_inactivate(BdrvChild *child)
* this point because the VM is stopped) and unattached monitor-owned
* BlockBackends. If there is still any other user like a block job, then
* we simply can't inactivate the image. */
- if (!blk->dev && !blk->name[0]) {
+ if (!blk->dev && !blk_name(blk)[0]) {
return -EPERM;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v2 2/4] migration: Inactivate images after .save_live_complete_precopy()
2017-05-30 15:22 [Qemu-devel] [PATCH v2 0/4] Block migration (migrate -b) fixes Kevin Wolf
2017-05-30 15:22 ` [Qemu-devel] [PATCH v2 1/4] block: Fix anonymous BBs in blk_root_inactivate() Kevin Wolf
@ 2017-05-30 15:22 ` Kevin Wolf
2017-05-30 15:29 ` [Qemu-devel] [Qemu-block] " Jeff Cody
2017-05-30 15:22 ` [Qemu-devel] [PATCH v2 3/4] migration/block: Clean up BBs in block_save_complete() Kevin Wolf
2017-05-30 15:22 ` [Qemu-devel] [PATCH v2 4/4] qemu-iotests: Block migration test Kevin Wolf
3 siblings, 1 reply; 13+ messages in thread
From: Kevin Wolf @ 2017-05-30 15:22 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, eblake, qemu-devel
Block migration may still access the image during its
.save_live_complete_precopy() implementation, so we should only
inactivate the image afterwards.
Another reason for the change is that inactivating an image fails when
there is still a non-device BlockBackend using it, which includes the
BBs used by block migration. We want to give block migration a chance to
release the BBs before trying to inactivate the image (this will be done
in another patch).
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
migration/migration.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index ad29e53..77a05d1 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1820,17 +1820,19 @@ static void migration_completion(MigrationState *s, int current_active_state,
if (!ret) {
ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
+ if (ret >= 0) {
+ qemu_file_set_rate_limit(s->to_dst_file, INT64_MAX);
+ qemu_savevm_state_complete_precopy(s->to_dst_file, false);
+ }
/*
* Don't mark the image with BDRV_O_INACTIVE flag if
* we will go into COLO stage later.
*/
if (ret >= 0 && !migrate_colo_enabled()) {
ret = bdrv_inactivate_all();
- }
- if (ret >= 0) {
- qemu_file_set_rate_limit(s->to_dst_file, INT64_MAX);
- qemu_savevm_state_complete_precopy(s->to_dst_file, false);
- s->block_inactive = true;
+ if (ret >= 0) {
+ s->block_inactive = true;
+ }
}
}
qemu_mutex_unlock_iothread();
--
1.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v2 3/4] migration/block: Clean up BBs in block_save_complete()
2017-05-30 15:22 [Qemu-devel] [PATCH v2 0/4] Block migration (migrate -b) fixes Kevin Wolf
2017-05-30 15:22 ` [Qemu-devel] [PATCH v2 1/4] block: Fix anonymous BBs in blk_root_inactivate() Kevin Wolf
2017-05-30 15:22 ` [Qemu-devel] [PATCH v2 2/4] migration: Inactivate images after .save_live_complete_precopy() Kevin Wolf
@ 2017-05-30 15:22 ` Kevin Wolf
2017-05-30 15:32 ` [Qemu-devel] [Qemu-block] " Jeff Cody
2017-05-30 15:22 ` [Qemu-devel] [PATCH v2 4/4] qemu-iotests: Block migration test Kevin Wolf
3 siblings, 1 reply; 13+ messages in thread
From: Kevin Wolf @ 2017-05-30 15:22 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, eblake, qemu-devel
We need to release any block migrations BlockBackends on the source
before successfully completing the migration because otherwise
inactivating the images will fail (inactivation only tolerates device
BBs).
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
migration/block.c | 22 +++++++++++++++++-----
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/migration/block.c b/migration/block.c
index 13f90d3..deb4b0e 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -673,16 +673,14 @@ static int64_t get_remaining_dirty(void)
return dirty << BDRV_SECTOR_BITS;
}
-/* Called with iothread lock taken. */
-static void block_migration_cleanup(void *opaque)
+
+/* Called with iothread lock taken. */
+static void block_migration_cleanup_bmds(void)
{
BlkMigDevState *bmds;
- BlkMigBlock *blk;
AioContext *ctx;
- bdrv_drain_all();
-
unset_dirty_tracking();
while ((bmds = QSIMPLEQ_FIRST(&block_mig_state.bmds_list)) != NULL) {
@@ -700,6 +698,16 @@ static void block_migration_cleanup(void *opaque)
g_free(bmds->aio_bitmap);
g_free(bmds);
}
+}
+
+/* Called with iothread lock taken. */
+static void block_migration_cleanup(void *opaque)
+{
+ BlkMigBlock *blk;
+
+ bdrv_drain_all();
+
+ block_migration_cleanup_bmds();
blk_mig_lock();
while ((blk = QSIMPLEQ_FIRST(&block_mig_state.blk_list)) != NULL) {
@@ -843,6 +851,10 @@ static int block_save_complete(QEMUFile *f, void *opaque)
qemu_put_be64(f, BLK_MIG_FLAG_EOS);
+ /* Make sure that our BlockBackends are gone, so that the block driver
+ * nodes can be inactivated. */
+ block_migration_cleanup_bmds();
+
return 0;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v2 4/4] qemu-iotests: Block migration test
2017-05-30 15:22 [Qemu-devel] [PATCH v2 0/4] Block migration (migrate -b) fixes Kevin Wolf
` (2 preceding siblings ...)
2017-05-30 15:22 ` [Qemu-devel] [PATCH v2 3/4] migration/block: Clean up BBs in block_save_complete() Kevin Wolf
@ 2017-05-30 15:22 ` Kevin Wolf
2017-05-30 15:52 ` [Qemu-devel] [Qemu-block] " Jeff Cody
2017-05-30 16:03 ` [Qemu-devel] " Eric Blake
3 siblings, 2 replies; 13+ messages in thread
From: Kevin Wolf @ 2017-05-30 15:22 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, eblake, qemu-devel
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qemu-iotests/183 | 143 +++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/183.out | 46 +++++++++++++++
tests/qemu-iotests/group | 1 +
3 files changed, 190 insertions(+)
create mode 100755 tests/qemu-iotests/183
create mode 100644 tests/qemu-iotests/183.out
diff --git a/tests/qemu-iotests/183 b/tests/qemu-iotests/183
new file mode 100755
index 0000000..5eda0e9
--- /dev/null
+++ b/tests/qemu-iotests/183
@@ -0,0 +1,143 @@
+#!/bin/bash
+#
+# Test old-style block migration (migrate -b)
+#
+# Copyright (C) 2017 Red Hat, Inc.
+#
+# 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/>.
+#
+
+# creator
+owner=kwolf@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+status=1 # failure is the default!
+
+MIG_SOCKET="${TEST_DIR}/migrate"
+
+_cleanup()
+{
+ rm -f "${MIG_SOCKET}"
+ rm -f "${TEST_IMG}.dest"
+ _cleanup_test_img
+ _cleanup_qemu
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.qemu
+
+_supported_fmt generic
+_supported_proto file
+_supported_os Linux
+
+size=64M
+_make_test_img $size
+TEST_IMG="${TEST_IMG}.dest" _make_test_img $size
+
+echo
+echo === Starting VMs ===
+echo
+
+qemu_comm_method="qmp"
+
+_launch_qemu \
+ -drive file="${TEST_IMG}",cache=${CACHEMODE},driver=$IMGFMT,id=disk
+src=$QEMU_HANDLE
+_send_qemu_cmd $src "{ 'execute': 'qmp_capabilities' }" 'return'
+
+_launch_qemu \
+ -drive file="${TEST_IMG}.dest",cache=${CACHEMODE},driver=$IMGFMT,id=disk \
+ -incoming "unix:${MIG_SOCKET}"
+dest=$QEMU_HANDLE
+_send_qemu_cmd $dest "{ 'execute': 'qmp_capabilities' }" 'return'
+
+echo
+echo === Write something on the source ===
+echo
+
+_send_qemu_cmd $src \
+ "{ 'execute': 'human-monitor-command',
+ 'arguments': { 'command-line':
+ 'qemu-io disk \"write -P 0x55 0 64k\"' } }" \
+ 'return'
+_send_qemu_cmd $src \
+ "{ 'execute': 'human-monitor-command',
+ 'arguments': { 'command-line':
+ 'qemu-io disk \"read -P 0x55 0 64k\"' } }" \
+ 'return'
+
+echo
+echo === Do block migration to destination ===
+echo
+
+reply="$(_send_qemu_cmd $src \
+ "{ 'execute': 'migrate',
+ 'arguments': { 'uri': 'unix:${MIG_SOCKET}', 'blk': true } }" \
+ 'return\|error')"
+echo "$reply"
+if echo "$reply" | grep "compiled without old-style" > /dev/null; then
+ _notrun "migrate -b support not compiled in"
+fi
+
+while _send_qemu_cmd $src "{ 'execute': 'query-migrate' }" "return" |
+ grep '"status": "active"' > /dev/null
+do
+ sleep 0.1
+done
+_send_qemu_cmd $src "{ 'execute': 'query-status' }" "return"
+
+echo
+echo === Do some I/O on the destination ===
+echo
+
+# It is important that we use the BlockBackend of the guest device here instead
+# of the node name, which would create a new BlockBackend and not test whether
+# the guest has the necessary permissions to access the image now
+silent=yes _send_qemu_cmd $dest "" "100 %"
+_send_qemu_cmd $dest "{ 'execute': 'query-status' }" "return"
+_send_qemu_cmd $dest \
+ "{ 'execute': 'human-monitor-command',
+ 'arguments': { 'command-line':
+ 'qemu-io disk \"read -P 0x55 0 64k\"' } }" \
+ 'return'
+_send_qemu_cmd $dest \
+ "{ 'execute': 'human-monitor-command',
+ 'arguments': { 'command-line':
+ 'qemu-io disk \"write -P 0x66 1M 64k\"' } }" \
+ 'return'
+
+echo
+echo === Shut down and check image ===
+echo
+
+_send_qemu_cmd $src '{"execute":"quit"}' 'return'
+_send_qemu_cmd $dest '{"execute":"quit"}' 'return'
+wait=1 _cleanup_qemu
+
+_check_test_img
+TEST_IMG="${TEST_IMG}.dest" _check_test_img
+
+$QEMU_IO -c 'write -P 0x66 1M 64k' "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG compare "$TEST_IMG.dest" "$TEST_IMG"
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/183.out b/tests/qemu-iotests/183.out
new file mode 100644
index 0000000..6647488
--- /dev/null
+++ b/tests/qemu-iotests/183.out
@@ -0,0 +1,46 @@
+QA output created by 183
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+Formatting 'TEST_DIR/t.IMGFMT.dest', fmt=IMGFMT size=67108864
+
+=== Starting VMs ===
+
+{"return": {}}
+{"return": {}}
+
+=== Write something on the source ===
+
+wrote 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+{"return": ""}
+read 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+{"return": ""}
+
+=== Do block migration to destination ===
+
+{"return": {}}
+{"return": {"status": "postmigrate", "singlestep": false, "running": false}}
+
+=== Do some I/O on the destination ===
+
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "RESUME"}
+{"return": {"status": "running", "singlestep": false, "running": true}}
+read 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+{"return": ""}
+wrote 65536/65536 bytes at offset 1048576
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+{"return": ""}
+
+=== Shut down and check image ===
+
+{"return": {}}
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN"}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN"}
+No errors were found on the image.
+No errors were found on the image.
+wrote 65536/65536 bytes at offset 1048576
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Images are identical.
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 5c8ea0f..a6acaff 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -174,3 +174,4 @@
179 rw auto quick
181 rw auto migration
182 rw auto quick
+183 rw auto migration
--
1.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v2 1/4] block: Fix anonymous BBs in blk_root_inactivate()
2017-05-30 15:22 ` [Qemu-devel] [PATCH v2 1/4] block: Fix anonymous BBs in blk_root_inactivate() Kevin Wolf
@ 2017-05-30 15:28 ` Jeff Cody
0 siblings, 0 replies; 13+ messages in thread
From: Jeff Cody @ 2017-05-30 15:28 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, qemu-devel
On Tue, May 30, 2017 at 05:22:50PM +0200, Kevin Wolf wrote:
> blk->name isn't an array, but a pointer that can be NULL. Checking for
> an anonymous BB must involve a NULL check first, otherwise we get
> crashes.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Fam Zheng <famz@redhat.com>
> Reviewed-by: Juan Quintela <quintela@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
> block/block-backend.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/block/block-backend.c b/block/block-backend.c
> index f3a6008..7d7f369 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -168,7 +168,7 @@ static int blk_root_inactivate(BdrvChild *child)
> * this point because the VM is stopped) and unattached monitor-owned
> * BlockBackends. If there is still any other user like a block job, then
> * we simply can't inactivate the image. */
> - if (!blk->dev && !blk->name[0]) {
> + if (!blk->dev && !blk_name(blk)[0]) {
> return -EPERM;
> }
>
> --
> 1.8.3.1
>
>
Reviewed-by: Jeff Cody <jcody@redhat.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v2 2/4] migration: Inactivate images after .save_live_complete_precopy()
2017-05-30 15:22 ` [Qemu-devel] [PATCH v2 2/4] migration: Inactivate images after .save_live_complete_precopy() Kevin Wolf
@ 2017-05-30 15:29 ` Jeff Cody
0 siblings, 0 replies; 13+ messages in thread
From: Jeff Cody @ 2017-05-30 15:29 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, qemu-devel
On Tue, May 30, 2017 at 05:22:51PM +0200, Kevin Wolf wrote:
> Block migration may still access the image during its
> .save_live_complete_precopy() implementation, so we should only
> inactivate the image afterwards.
>
> Another reason for the change is that inactivating an image fails when
> there is still a non-device BlockBackend using it, which includes the
> BBs used by block migration. We want to give block migration a chance to
> release the BBs before trying to inactivate the image (this will be done
> in another patch).
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Fam Zheng <famz@redhat.com>
> Reviewed-by: Juan Quintela <quintela@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
> migration/migration.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index ad29e53..77a05d1 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1820,17 +1820,19 @@ static void migration_completion(MigrationState *s, int current_active_state,
>
> if (!ret) {
> ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
> + if (ret >= 0) {
> + qemu_file_set_rate_limit(s->to_dst_file, INT64_MAX);
> + qemu_savevm_state_complete_precopy(s->to_dst_file, false);
> + }
> /*
> * Don't mark the image with BDRV_O_INACTIVE flag if
> * we will go into COLO stage later.
> */
> if (ret >= 0 && !migrate_colo_enabled()) {
> ret = bdrv_inactivate_all();
> - }
> - if (ret >= 0) {
> - qemu_file_set_rate_limit(s->to_dst_file, INT64_MAX);
> - qemu_savevm_state_complete_precopy(s->to_dst_file, false);
> - s->block_inactive = true;
> + if (ret >= 0) {
> + s->block_inactive = true;
> + }
> }
> }
> qemu_mutex_unlock_iothread();
> --
> 1.8.3.1
>
>
Reviewed-by: Jeff Cody <jcody@redhat.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v2 3/4] migration/block: Clean up BBs in block_save_complete()
2017-05-30 15:22 ` [Qemu-devel] [PATCH v2 3/4] migration/block: Clean up BBs in block_save_complete() Kevin Wolf
@ 2017-05-30 15:32 ` Jeff Cody
0 siblings, 0 replies; 13+ messages in thread
From: Jeff Cody @ 2017-05-30 15:32 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, qemu-devel
On Tue, May 30, 2017 at 05:22:52PM +0200, Kevin Wolf wrote:
> We need to release any block migrations BlockBackends on the source
> before successfully completing the migration because otherwise
> inactivating the images will fail (inactivation only tolerates device
> BBs).
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Fam Zheng <famz@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
> migration/block.c | 22 +++++++++++++++++-----
> 1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/migration/block.c b/migration/block.c
> index 13f90d3..deb4b0e 100644
> --- a/migration/block.c
> +++ b/migration/block.c
> @@ -673,16 +673,14 @@ static int64_t get_remaining_dirty(void)
> return dirty << BDRV_SECTOR_BITS;
> }
>
> -/* Called with iothread lock taken. */
>
> -static void block_migration_cleanup(void *opaque)
> +
> +/* Called with iothread lock taken. */
> +static void block_migration_cleanup_bmds(void)
> {
> BlkMigDevState *bmds;
> - BlkMigBlock *blk;
> AioContext *ctx;
>
> - bdrv_drain_all();
> -
> unset_dirty_tracking();
>
> while ((bmds = QSIMPLEQ_FIRST(&block_mig_state.bmds_list)) != NULL) {
> @@ -700,6 +698,16 @@ static void block_migration_cleanup(void *opaque)
> g_free(bmds->aio_bitmap);
> g_free(bmds);
> }
> +}
> +
> +/* Called with iothread lock taken. */
> +static void block_migration_cleanup(void *opaque)
> +{
> + BlkMigBlock *blk;
> +
> + bdrv_drain_all();
> +
> + block_migration_cleanup_bmds();
>
> blk_mig_lock();
> while ((blk = QSIMPLEQ_FIRST(&block_mig_state.blk_list)) != NULL) {
> @@ -843,6 +851,10 @@ static int block_save_complete(QEMUFile *f, void *opaque)
>
> qemu_put_be64(f, BLK_MIG_FLAG_EOS);
>
> + /* Make sure that our BlockBackends are gone, so that the block driver
> + * nodes can be inactivated. */
> + block_migration_cleanup_bmds();
> +
> return 0;
> }
>
> --
> 1.8.3.1
>
>
Reviewed-by: Jeff Cody <jcody@redhat.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v2 4/4] qemu-iotests: Block migration test
2017-05-30 15:22 ` [Qemu-devel] [PATCH v2 4/4] qemu-iotests: Block migration test Kevin Wolf
@ 2017-05-30 15:52 ` Jeff Cody
2017-05-30 16:57 ` Kevin Wolf
2017-05-30 16:03 ` [Qemu-devel] " Eric Blake
1 sibling, 1 reply; 13+ messages in thread
From: Jeff Cody @ 2017-05-30 15:52 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, qemu-devel
On Tue, May 30, 2017 at 05:22:53PM +0200, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> tests/qemu-iotests/183 | 143 +++++++++++++++++++++++++++++++++++++++++++++
> tests/qemu-iotests/183.out | 46 +++++++++++++++
> tests/qemu-iotests/group | 1 +
> 3 files changed, 190 insertions(+)
> create mode 100755 tests/qemu-iotests/183
> create mode 100644 tests/qemu-iotests/183.out
>
> diff --git a/tests/qemu-iotests/183 b/tests/qemu-iotests/183
> new file mode 100755
> index 0000000..5eda0e9
> --- /dev/null
> +++ b/tests/qemu-iotests/183
> @@ -0,0 +1,143 @@
> +#!/bin/bash
> +#
> +# Test old-style block migration (migrate -b)
> +#
> +# Copyright (C) 2017 Red Hat, Inc.
> +#
> +# 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/>.
> +#
> +
> +# creator
> +owner=kwolf@redhat.com
> +
> +seq=`basename $0`
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +status=1 # failure is the default!
> +
> +MIG_SOCKET="${TEST_DIR}/migrate"
> +
> +_cleanup()
> +{
> + rm -f "${MIG_SOCKET}"
> + rm -f "${TEST_IMG}.dest"
> + _cleanup_test_img
> + _cleanup_qemu
> +}
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +# get standard environment, filters and checks
> +. ./common.rc
> +. ./common.filter
> +. ./common.qemu
> +
> +_supported_fmt generic
Not sure to what extent we are really keeping the _supported_fmt up to date,
but this test will only work on qcow2 and raw, right?
I'd recommend changing this to:
+ _supported_fmt qcow2 raw
(that can probably be done when applying, however).
> +_supported_proto file
> +_supported_os Linux
> +
> +size=64M
> +_make_test_img $size
> +TEST_IMG="${TEST_IMG}.dest" _make_test_img $size
> +
> +echo
> +echo === Starting VMs ===
> +echo
> +
> +qemu_comm_method="qmp"
> +
> +_launch_qemu \
> + -drive file="${TEST_IMG}",cache=${CACHEMODE},driver=$IMGFMT,id=disk
> +src=$QEMU_HANDLE
> +_send_qemu_cmd $src "{ 'execute': 'qmp_capabilities' }" 'return'
> +
> +_launch_qemu \
> + -drive file="${TEST_IMG}.dest",cache=${CACHEMODE},driver=$IMGFMT,id=disk \
> + -incoming "unix:${MIG_SOCKET}"
> +dest=$QEMU_HANDLE
> +_send_qemu_cmd $dest "{ 'execute': 'qmp_capabilities' }" 'return'
> +
> +echo
> +echo === Write something on the source ===
> +echo
> +
> +_send_qemu_cmd $src \
> + "{ 'execute': 'human-monitor-command',
> + 'arguments': { 'command-line':
> + 'qemu-io disk \"write -P 0x55 0 64k\"' } }" \
> + 'return'
> +_send_qemu_cmd $src \
> + "{ 'execute': 'human-monitor-command',
> + 'arguments': { 'command-line':
> + 'qemu-io disk \"read -P 0x55 0 64k\"' } }" \
> + 'return'
> +
> +echo
> +echo === Do block migration to destination ===
> +echo
> +
> +reply="$(_send_qemu_cmd $src \
> + "{ 'execute': 'migrate',
> + 'arguments': { 'uri': 'unix:${MIG_SOCKET}', 'blk': true } }" \
> + 'return\|error')"
> +echo "$reply"
> +if echo "$reply" | grep "compiled without old-style" > /dev/null; then
> + _notrun "migrate -b support not compiled in"
> +fi
> +
> +while _send_qemu_cmd $src "{ 'execute': 'query-migrate' }" "return" |
> + grep '"status": "active"' > /dev/null
> +do
> + sleep 0.1
Would it make sense here to add a timeout? It would be nice to be able to
reply on scripts always failing for git-bisect (rather than potentially
hanging at a spot).
> +done
Modulo the updated _supported_fmt line:
Reviewed-by: Jeff Cody <jcody@redhat.com>
> +_send_qemu_cmd $src "{ 'execute': 'query-status' }" "return"
> +
> +echo
> +echo === Do some I/O on the destination ===
> +echo
> +
> +# It is important that we use the BlockBackend of the guest device here instead
> +# of the node name, which would create a new BlockBackend and not test whether
> +# the guest has the necessary permissions to access the image now
> +silent=yes _send_qemu_cmd $dest "" "100 %"
> +_send_qemu_cmd $dest "{ 'execute': 'query-status' }" "return"
> +_send_qemu_cmd $dest \
> + "{ 'execute': 'human-monitor-command',
> + 'arguments': { 'command-line':
> + 'qemu-io disk \"read -P 0x55 0 64k\"' } }" \
> + 'return'
> +_send_qemu_cmd $dest \
> + "{ 'execute': 'human-monitor-command',
> + 'arguments': { 'command-line':
> + 'qemu-io disk \"write -P 0x66 1M 64k\"' } }" \
> + 'return'
> +
> +echo
> +echo === Shut down and check image ===
> +echo
> +
> +_send_qemu_cmd $src '{"execute":"quit"}' 'return'
> +_send_qemu_cmd $dest '{"execute":"quit"}' 'return'
> +wait=1 _cleanup_qemu
> +
> +_check_test_img
> +TEST_IMG="${TEST_IMG}.dest" _check_test_img
> +
> +$QEMU_IO -c 'write -P 0x66 1M 64k' "$TEST_IMG" | _filter_qemu_io
> +$QEMU_IMG compare "$TEST_IMG.dest" "$TEST_IMG"
> +
> +# success, all done
> +echo "*** done"
> +rm -f $seq.full
> +status=0
> diff --git a/tests/qemu-iotests/183.out b/tests/qemu-iotests/183.out
> new file mode 100644
> index 0000000..6647488
> --- /dev/null
> +++ b/tests/qemu-iotests/183.out
> @@ -0,0 +1,46 @@
> +QA output created by 183
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
> +Formatting 'TEST_DIR/t.IMGFMT.dest', fmt=IMGFMT size=67108864
> +
> +=== Starting VMs ===
> +
> +{"return": {}}
> +{"return": {}}
> +
> +=== Write something on the source ===
> +
> +wrote 65536/65536 bytes at offset 0
> +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +{"return": ""}
> +read 65536/65536 bytes at offset 0
> +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +{"return": ""}
> +
> +=== Do block migration to destination ===
> +
> +{"return": {}}
> +{"return": {"status": "postmigrate", "singlestep": false, "running": false}}
> +
> +=== Do some I/O on the destination ===
> +
> +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "RESUME"}
> +{"return": {"status": "running", "singlestep": false, "running": true}}
> +read 65536/65536 bytes at offset 0
> +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +{"return": ""}
> +wrote 65536/65536 bytes at offset 1048576
> +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +{"return": ""}
> +
> +=== Shut down and check image ===
> +
> +{"return": {}}
> +{"return": {}}
> +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN"}
> +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN"}
> +No errors were found on the image.
> +No errors were found on the image.
> +wrote 65536/65536 bytes at offset 1048576
> +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +Images are identical.
> +*** done
> diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
> index 5c8ea0f..a6acaff 100644
> --- a/tests/qemu-iotests/group
> +++ b/tests/qemu-iotests/group
> @@ -174,3 +174,4 @@
> 179 rw auto quick
> 181 rw auto migration
> 182 rw auto quick
> +183 rw auto migration
> --
> 1.8.3.1
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 4/4] qemu-iotests: Block migration test
2017-05-30 15:22 ` [Qemu-devel] [PATCH v2 4/4] qemu-iotests: Block migration test Kevin Wolf
2017-05-30 15:52 ` [Qemu-devel] [Qemu-block] " Jeff Cody
@ 2017-05-30 16:03 ` Eric Blake
1 sibling, 0 replies; 13+ messages in thread
From: Eric Blake @ 2017-05-30 16:03 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1725 bytes --]
On 05/30/2017 10:22 AM, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> tests/qemu-iotests/183 | 143 +++++++++++++++++++++++++++++++++++++++++++++
> tests/qemu-iotests/183.out | 46 +++++++++++++++
> tests/qemu-iotests/group | 1 +
> 3 files changed, 190 insertions(+)
> create mode 100755 tests/qemu-iotests/183
> create mode 100644 tests/qemu-iotests/183.out
>
In addition to Jeff's review comments:
> +
> +_cleanup()
> +{
> + rm -f "${MIG_SOCKET}"
> + rm -f "${TEST_IMG}.dest"
> + _cleanup_test_img
> + _cleanup_qemu
Odd indentation.
> +echo
> +echo === Do block migration to destination ===
> +echo
> +
> +reply="$(_send_qemu_cmd $src \
> + "{ 'execute': 'migrate',
> + 'arguments': { 'uri': 'unix:${MIG_SOCKET}', 'blk': true } }" \
> + 'return\|error')"
> +echo "$reply"
> +if echo "$reply" | grep "compiled without old-style" > /dev/null; then
> + _notrun "migrate -b support not compiled in"
> +fi
Oh cool - I didn't realize that _notrun already existed as our way to
skip a test, even if we've already produced unexpected output (if we
ever want to play with exit status 77 in the future for skipped tests,
then _notrun would be a great place to centralize that).
> +
> +while _send_qemu_cmd $src "{ 'execute': 'query-migrate' }" "return" |
> + grep '"status": "active"' > /dev/null
> +do
> + sleep 0.1
More interesting indentation (I would have probably stuck to 4 spaces on
both lines)
With the nits fixed,
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v2 4/4] qemu-iotests: Block migration test
2017-05-30 15:52 ` [Qemu-devel] [Qemu-block] " Jeff Cody
@ 2017-05-30 16:57 ` Kevin Wolf
2017-05-30 18:48 ` Jeff Cody
0 siblings, 1 reply; 13+ messages in thread
From: Kevin Wolf @ 2017-05-30 16:57 UTC (permalink / raw)
To: Jeff Cody; +Cc: qemu-block, qemu-devel
Am 30.05.2017 um 17:52 hat Jeff Cody geschrieben:
> On Tue, May 30, 2017 at 05:22:53PM +0200, Kevin Wolf wrote:
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> > tests/qemu-iotests/183 | 143 +++++++++++++++++++++++++++++++++++++++++++++
> > tests/qemu-iotests/183.out | 46 +++++++++++++++
> > tests/qemu-iotests/group | 1 +
> > 3 files changed, 190 insertions(+)
> > create mode 100755 tests/qemu-iotests/183
> > create mode 100644 tests/qemu-iotests/183.out
> >
> > diff --git a/tests/qemu-iotests/183 b/tests/qemu-iotests/183
> > new file mode 100755
> > index 0000000..5eda0e9
> > --- /dev/null
> > +++ b/tests/qemu-iotests/183
> > @@ -0,0 +1,143 @@
> > +#!/bin/bash
> > +#
> > +# Test old-style block migration (migrate -b)
> > +#
> > +# Copyright (C) 2017 Red Hat, Inc.
> > +#
> > +# 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/>.
> > +#
> > +
> > +# creator
> > +owner=kwolf@redhat.com
> > +
> > +seq=`basename $0`
> > +echo "QA output created by $seq"
> > +
> > +here=`pwd`
> > +status=1 # failure is the default!
> > +
> > +MIG_SOCKET="${TEST_DIR}/migrate"
> > +
> > +_cleanup()
> > +{
> > + rm -f "${MIG_SOCKET}"
> > + rm -f "${TEST_IMG}.dest"
> > + _cleanup_test_img
> > + _cleanup_qemu
> > +}
> > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > +
> > +# get standard environment, filters and checks
> > +. ./common.rc
> > +. ./common.filter
> > +. ./common.qemu
> > +
> > +_supported_fmt generic
>
> Not sure to what extent we are really keeping the _supported_fmt up to date,
> but this test will only work on qcow2 and raw, right?
>
> I'd recommend changing this to:
>
> + _supported_fmt qcow2 raw
>
> (that can probably be done when applying, however).
Seems you are right, but I fail to see why the other formats shouldn't
work? Are these just bugs or do I make any assumption in the test script
that is invalid for other image formats?
> > +_supported_proto file
> > +_supported_os Linux
> > +
> > +size=64M
> > +_make_test_img $size
> > +TEST_IMG="${TEST_IMG}.dest" _make_test_img $size
> > +
> > +echo
> > +echo === Starting VMs ===
> > +echo
> > +
> > +qemu_comm_method="qmp"
> > +
> > +_launch_qemu \
> > + -drive file="${TEST_IMG}",cache=${CACHEMODE},driver=$IMGFMT,id=disk
> > +src=$QEMU_HANDLE
> > +_send_qemu_cmd $src "{ 'execute': 'qmp_capabilities' }" 'return'
> > +
> > +_launch_qemu \
> > + -drive file="${TEST_IMG}.dest",cache=${CACHEMODE},driver=$IMGFMT,id=disk \
> > + -incoming "unix:${MIG_SOCKET}"
> > +dest=$QEMU_HANDLE
> > +_send_qemu_cmd $dest "{ 'execute': 'qmp_capabilities' }" 'return'
> > +
> > +echo
> > +echo === Write something on the source ===
> > +echo
> > +
> > +_send_qemu_cmd $src \
> > + "{ 'execute': 'human-monitor-command',
> > + 'arguments': { 'command-line':
> > + 'qemu-io disk \"write -P 0x55 0 64k\"' } }" \
> > + 'return'
> > +_send_qemu_cmd $src \
> > + "{ 'execute': 'human-monitor-command',
> > + 'arguments': { 'command-line':
> > + 'qemu-io disk \"read -P 0x55 0 64k\"' } }" \
> > + 'return'
> > +
> > +echo
> > +echo === Do block migration to destination ===
> > +echo
> > +
> > +reply="$(_send_qemu_cmd $src \
> > + "{ 'execute': 'migrate',
> > + 'arguments': { 'uri': 'unix:${MIG_SOCKET}', 'blk': true } }" \
> > + 'return\|error')"
> > +echo "$reply"
> > +if echo "$reply" | grep "compiled without old-style" > /dev/null; then
> > + _notrun "migrate -b support not compiled in"
> > +fi
> > +
> > +while _send_qemu_cmd $src "{ 'execute': 'query-migrate' }" "return" |
> > + grep '"status": "active"' > /dev/null
> > +do
> > + sleep 0.1
>
> Would it make sense here to add a timeout? It would be nice to be able to
> reply on scripts always failing for git-bisect (rather than potentially
> hanging at a spot).
What would you think about squashing this in:
--- a/tests/qemu-iotests/183
+++ b/tests/qemu-iotests/183
@@ -96,11 +96,8 @@ if echo "$reply" | grep "compiled without old-style" > /dev/null; then
_notrun "migrate -b support not compiled in"
fi
-while _send_qemu_cmd $src "{ 'execute': 'query-migrate' }" "return" |
- grep '"status": "active"' > /dev/null
-do
- sleep 0.1
-done
+QEMU_COMM_TIMEOUT=0.1 qemu_cmd_repeat=50 silent=yes \
+ _send_qemu_cmd $src "{ 'execute': 'query-migrate' }" '"status": "completed"'
_send_qemu_cmd $src "{ 'execute': 'query-status' }" "return"
echo
Kevin
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v2 4/4] qemu-iotests: Block migration test
2017-05-30 16:57 ` Kevin Wolf
@ 2017-05-30 18:48 ` Jeff Cody
2017-05-31 9:00 ` Kevin Wolf
0 siblings, 1 reply; 13+ messages in thread
From: Jeff Cody @ 2017-05-30 18:48 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, qemu-devel
On Tue, May 30, 2017 at 06:57:05PM +0200, Kevin Wolf wrote:
> Am 30.05.2017 um 17:52 hat Jeff Cody geschrieben:
> > On Tue, May 30, 2017 at 05:22:53PM +0200, Kevin Wolf wrote:
> > > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > > ---
> > > tests/qemu-iotests/183 | 143 +++++++++++++++++++++++++++++++++++++++++++++
> > > tests/qemu-iotests/183.out | 46 +++++++++++++++
> > > tests/qemu-iotests/group | 1 +
> > > 3 files changed, 190 insertions(+)
> > > create mode 100755 tests/qemu-iotests/183
> > > create mode 100644 tests/qemu-iotests/183.out
> > >
> > > diff --git a/tests/qemu-iotests/183 b/tests/qemu-iotests/183
> > > new file mode 100755
> > > index 0000000..5eda0e9
> > > --- /dev/null
> > > +++ b/tests/qemu-iotests/183
> > > @@ -0,0 +1,143 @@
> > > +#!/bin/bash
> > > +#
> > > +# Test old-style block migration (migrate -b)
> > > +#
> > > +# Copyright (C) 2017 Red Hat, Inc.
> > > +#
> > > +# 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/>.
> > > +#
> > > +
> > > +# creator
> > > +owner=kwolf@redhat.com
> > > +
> > > +seq=`basename $0`
> > > +echo "QA output created by $seq"
> > > +
> > > +here=`pwd`
> > > +status=1 # failure is the default!
> > > +
> > > +MIG_SOCKET="${TEST_DIR}/migrate"
> > > +
> > > +_cleanup()
> > > +{
> > > + rm -f "${MIG_SOCKET}"
> > > + rm -f "${TEST_IMG}.dest"
> > > + _cleanup_test_img
> > > + _cleanup_qemu
> > > +}
> > > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > > +
> > > +# get standard environment, filters and checks
> > > +. ./common.rc
> > > +. ./common.filter
> > > +. ./common.qemu
> > > +
> > > +_supported_fmt generic
> >
> > Not sure to what extent we are really keeping the _supported_fmt up to date,
> > but this test will only work on qcow2 and raw, right?
> >
> > I'd recommend changing this to:
> >
> > + _supported_fmt qcow2 raw
> >
> > (that can probably be done when applying, however).
>
> Seems you are right, but I fail to see why the other formats shouldn't
> work? Are these just bugs or do I make any assumption in the test script
> that is invalid for other image formats?
>
Well, the following formats have live migration blockers:
vmdk, vhdx, vdi, vpc, qcow, vvfat
So that leaves qed, dmg, quorum.
Of those, qed is the only one that fails. And I would guess it is a bug?
While waiting for "status: active" from the query-migrate (in the section
where I proposed a timeout), the migration is hanging, and appears to never
get started. If I redirect that output to a debug file, I just see this
over and over (I prettified the json output here):
{
"return": {
"expected-downtime": 300,
"status": "active",
"disk": {
"total": 67108864,
"postcopy-requests": 0,
"dirty-sync-count": 0,
"page-size": 0,
"remaining": 63963136,
"mbps": 0,
"transferred": 3145728,
"duplicate": 0,
"dirty-pages-rate": 0,
"skipped": 0,
"normal-bytes": 0,
"normal": 0
},
"setup-time": 1,
"total-time": 4956,
"ram": {
"total": 134750208,
"postcopy-requests": 0,
"dirty-sync-count": 1,
"page-size": 4096,
"remaining": 134750208,
"mbps": 0,
"transferred": 0,
"duplicate": 0,
"dirty-pages-rate": 0,
"skipped": 0,
"normal-bytes": 0,
"normal": 0
}
}
}
The only thing that advances is "total-time".
So I would make _supported_fmt be 'qcow2 raw qed dmg quorum', and the
failure of qed is an actual failure.
[...]
> > > +echo === Do block migration to destination ===
> > > +echo
> > > +
> > > +reply="$(_send_qemu_cmd $src \
> > > + "{ 'execute': 'migrate',
> > > + 'arguments': { 'uri': 'unix:${MIG_SOCKET}', 'blk': true } }" \
> > > + 'return\|error')"
> > > +echo "$reply"
> > > +if echo "$reply" | grep "compiled without old-style" > /dev/null; then
> > > + _notrun "migrate -b support not compiled in"
> > > +fi
> > > +
> > > +while _send_qemu_cmd $src "{ 'execute': 'query-migrate' }" "return" |
> > > + grep '"status": "active"' > /dev/null
> > > +do
> > > + sleep 0.1
> >
> > Would it make sense here to add a timeout? It would be nice to be able to
> > reply on scripts always failing for git-bisect (rather than potentially
> > hanging at a spot).
>
> What would you think about squashing this in:
>
> --- a/tests/qemu-iotests/183
> +++ b/tests/qemu-iotests/183
> @@ -96,11 +96,8 @@ if echo "$reply" | grep "compiled without old-style" > /dev/null; then
> _notrun "migrate -b support not compiled in"
> fi
>
> -while _send_qemu_cmd $src "{ 'execute': 'query-migrate' }" "return" |
> - grep '"status": "active"' > /dev/null
> -do
> - sleep 0.1
> -done
> +QEMU_COMM_TIMEOUT=0.1 qemu_cmd_repeat=50 silent=yes \
> + _send_qemu_cmd $src "{ 'execute': 'query-migrate' }" '"status": "completed"'
> _send_qemu_cmd $src "{ 'execute': 'query-status' }" "return"
>
> echo
>
I like that approach. It also worked well with the qed failure case.
Rather than hanging during the query-migrate, it times out nicely. All
other formats expected to work, still worked for me.
-Jeff
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v2 4/4] qemu-iotests: Block migration test
2017-05-30 18:48 ` Jeff Cody
@ 2017-05-31 9:00 ` Kevin Wolf
0 siblings, 0 replies; 13+ messages in thread
From: Kevin Wolf @ 2017-05-31 9:00 UTC (permalink / raw)
To: Jeff Cody; +Cc: qemu-block, qemu-devel
Am 30.05.2017 um 20:48 hat Jeff Cody geschrieben:
> On Tue, May 30, 2017 at 06:57:05PM +0200, Kevin Wolf wrote:
> > Seems you are right, but I fail to see why the other formats shouldn't
> > work? Are these just bugs or do I make any assumption in the test script
> > that is invalid for other image formats?
> >
>
> Well, the following formats have live migration blockers:
>
> vmdk, vhdx, vdi, vpc, qcow, vvfat
Oh, good point. We also need to fix 181 then.
> So that leaves qed, dmg, quorum.
>
> Of those, qed is the only one that fails. And I would guess it is a
> bug?
It is. Manual testing of a simple block migration fails, too, with qed.
It's hanging while it tries to activate the image:
#0 0x00007f5770684eef in __GI_ppoll (fds=0x7f577e322790, nfds=1, timeout=<optimized out>, timeout@entry=0x0, sigmask=sigmask@entry=0x0)
at ../sysdeps/unix/sysv/linux/ppoll.c:56
#1 0x00007f577d11280b in qemu_poll_ns (__ss=0x0, __timeout=0x0, __nfds=<optimized out>, __fds=<optimized out>) at /usr/include/bits/poll2.h:77
#2 0x00007f577d11280b in qemu_poll_ns (fds=<optimized out>, nfds=<optimized out>, timeout=timeout@entry=-1) at util/qemu-timer.c:322
#3 0x00007f577d1144ca in aio_poll (ctx=ctx@entry=0x7f577e2d8f40, blocking=<optimized out>) at util/aio-posix.c:622
#4 0x00007f577d083bb1 in qed_read_l1_table_sync (s=s@entry=0x7f577e322d90) at block/qed-table.c:183
#5 0x00007f577d082ac7 in bdrv_qed_do_open (bs=bs@entry=0x7f577e317b50, flags=8194, errp=errp@entry=0x7f57593ffbb0, options=0x0) at block/qed.c:519
#6 0x00007f577d082cc7 in bdrv_qed_invalidate_cache (bs=0x7f577e317b50, errp=0x7f57593ffbe0) at block/qed.c:1646
#7 0x00007f577d054cb4 in bdrv_invalidate_cache (bs=0x7f577e317b50, errp=errp@entry=0x7f57593ffc88) at block.c:4041
#8 0x00007f577d08f77c in blk_invalidate_cache (blk=blk@entry=0x7f577e317970, errp=errp@entry=0x7f57593ffc88) at block/block-backend.c:1537
#9 0x00007f577cfff729 in block_load (f=<optimized out>, opaque=<optimized out>, version_id=<optimized out>) at migration/block.c:930
#10 0x00007f577cff4d97 in vmstate_load (f=0x7f577e2dc750, se=0x7f577e3109e0, version_id=1) at migration/savevm.c:728
#11 0x00007f577cff5232 in qemu_loadvm_state_main (mis=0x7f577d76d860 <mis_current.32100>, f=0x7f577e2dc750) at migration/savevm.c:1917
#12 0x00007f577cff5232 in qemu_loadvm_state_main (f=f@entry=0x7f577e2dc750, mis=mis@entry=0x7f577d76d860 <mis_current.32100>) at migration/savevm.c:1948
#13 0x00007f577cff7e26 in qemu_loadvm_state (f=f@entry=0x7f577e2dc750) at migration/savevm.c:2014
#14 0x00007f577cff0113 in process_incoming_migration_co (opaque=0x7f577e2dc750) at migration/migration.c:391
#15 0x00007f577d127c6a in coroutine_trampoline (i0=<optimized out>, i1=<optimized out>) at util/coroutine-ucontext.c:79
#16 0x00007f57705decf0 in __start_context () at /lib64/libc.so.6
> So I would make _supported_fmt be 'qcow2 raw qed dmg quorum', and the
> failure of qed is an actual failure.
Yes, sounds good.
> > What would you think about squashing this in:
> >
> > --- a/tests/qemu-iotests/183
> > +++ b/tests/qemu-iotests/183
> > @@ -96,11 +96,8 @@ if echo "$reply" | grep "compiled without old-style" > /dev/null; then
> > _notrun "migrate -b support not compiled in"
> > fi
> >
> > -while _send_qemu_cmd $src "{ 'execute': 'query-migrate' }" "return" |
> > - grep '"status": "active"' > /dev/null
> > -do
> > - sleep 0.1
> > -done
> > +QEMU_COMM_TIMEOUT=0.1 qemu_cmd_repeat=50 silent=yes \
> > + _send_qemu_cmd $src "{ 'execute': 'query-migrate' }" '"status": "completed"'
> > _send_qemu_cmd $src "{ 'execute': 'query-status' }" "return"
> >
> > echo
> >
>
> I like that approach. It also worked well with the qed failure case.
> Rather than hanging during the query-migrate, it times out nicely. All
> other formats expected to work, still worked for me.
Okay, I'll send a v3.
Kevin
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2017-05-31 9:00 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-30 15:22 [Qemu-devel] [PATCH v2 0/4] Block migration (migrate -b) fixes Kevin Wolf
2017-05-30 15:22 ` [Qemu-devel] [PATCH v2 1/4] block: Fix anonymous BBs in blk_root_inactivate() Kevin Wolf
2017-05-30 15:28 ` [Qemu-devel] [Qemu-block] " Jeff Cody
2017-05-30 15:22 ` [Qemu-devel] [PATCH v2 2/4] migration: Inactivate images after .save_live_complete_precopy() Kevin Wolf
2017-05-30 15:29 ` [Qemu-devel] [Qemu-block] " Jeff Cody
2017-05-30 15:22 ` [Qemu-devel] [PATCH v2 3/4] migration/block: Clean up BBs in block_save_complete() Kevin Wolf
2017-05-30 15:32 ` [Qemu-devel] [Qemu-block] " Jeff Cody
2017-05-30 15:22 ` [Qemu-devel] [PATCH v2 4/4] qemu-iotests: Block migration test Kevin Wolf
2017-05-30 15:52 ` [Qemu-devel] [Qemu-block] " Jeff Cody
2017-05-30 16:57 ` Kevin Wolf
2017-05-30 18:48 ` Jeff Cody
2017-05-31 9:00 ` Kevin Wolf
2017-05-30 16:03 ` [Qemu-devel] " Eric Blake
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.