[Qemu-devel] [PATCH v2 0/4] Block migration (migrate -b) fixes

[Qemu-devel] [PATCH v2 0/4] Block migration (migrate -b) fixes

[Kevin Wolf]

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

[Qemu-devel] [PATCH v2 1/4] block: Fix anonymous BBs in blk_root_inactivate()

[Kevin Wolf]

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

Re: [Qemu-devel] [Qemu-block] [PATCH v2 1/4] block: Fix anonymous BBs in blk_root_inactivate()

[Jeff Cody]

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>

[Qemu-devel] [PATCH v2 2/4] migration: Inactivate images after .save_live_complete_precopy()

[Kevin Wolf]

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

Re: [Qemu-devel] [Qemu-block] [PATCH v2 2/4] migration: Inactivate images after .save_live_complete_precopy()

[Jeff Cody]

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>

[Qemu-devel] [PATCH v2 3/4] migration/block: Clean up BBs in block_save_complete()

[Kevin Wolf]

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

Re: [Qemu-devel] [Qemu-block] [PATCH v2 3/4] migration/block: Clean up BBs in block_save_complete()

[Jeff Cody]

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>

[Qemu-devel] [PATCH v2 4/4] qemu-iotests: Block migration test

[Kevin Wolf]

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

Re: [Qemu-devel] [Qemu-block] [PATCH v2 4/4] qemu-iotests: Block migration test

[Jeff Cody]

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
> 
> 

Re: [Qemu-devel] [Qemu-block] [PATCH v2 4/4] qemu-iotests: Block migration test

[Kevin Wolf]

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

Re: [Qemu-devel] [Qemu-block] [PATCH v2 4/4] qemu-iotests: Block migration test

[Jeff Cody]

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

Re: [Qemu-devel] [Qemu-block] [PATCH v2 4/4] qemu-iotests: Block migration test

[Kevin Wolf]

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

Re: [Qemu-devel] [PATCH v2 4/4] qemu-iotests: Block migration test

[Eric Blake]

[-- 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 --]