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

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

[Kevin Wolf]

v3:
- Restricted test to formats without migration blockers [Jeff]
- Added timeout for migration completion in test [Jeff]
- Removed remaining tabs copied from another test case [Eric]
- Updated reference output (SHUTDOWN events) after rebase

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     | 140 +++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/183.out |  46 +++++++++++++++
 tests/qemu-iotests/group   |   1 +
 6 files changed, 212 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 v3 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

[Qemu-devel] [PATCH v3 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

[Qemu-devel] [PATCH v3 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

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

[Kevin Wolf]

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/183     | 140 +++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/183.out |  46 +++++++++++++++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 187 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..0a87541
--- /dev/null
+++ b/tests/qemu-iotests/183
@@ -0,0 +1,140 @@
+#!/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 qcow2 raw qed dmg quorum
+_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
+
+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
+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..103fdc7
--- /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", "data": {"guest": false}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
+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] [PATCH v3 4/4] qemu-iotests: Block migration test

[Jeff Cody]

On Wed, May 31, 2017 at 11:52:47AM +0200, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  tests/qemu-iotests/183     | 140 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/183.out |  46 +++++++++++++++
>  tests/qemu-iotests/group   |   1 +
>  3 files changed, 187 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..0a87541
> --- /dev/null
> +++ b/tests/qemu-iotests/183
> @@ -0,0 +1,140 @@
> +#!/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 qcow2 raw qed dmg quorum
> +_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
> +
> +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
> +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..103fdc7
> --- /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", "data": {"guest": false}}
> +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
> +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
>

Reviewed-by: Jeff Cody <jcody@redhat.com>

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 924 bytes --]

On 05/31/2017 04:52 AM, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  tests/qemu-iotests/183     | 140 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/183.out |  46 +++++++++++++++
>  tests/qemu-iotests/group   |   1 +
>  3 files changed, 187 insertions(+)
>  create mode 100755 tests/qemu-iotests/183
>  create mode 100644 tests/qemu-iotests/183.out

> +qemu_comm_method="qmp"
> +
> +_launch_qemu \
> +    -drive file="${TEST_IMG}",cache=${CACHEMODE},driver=$IMGFMT,id=disk

Inconsistent use of ${CACHEMODE} vs. $IMGFMT (I prefer the version with
less typing, but others prefer always using ${}).  In both _launch_qemu
lines.  Up to you if you want to tweak it, but either way,

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

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

[Kevin Wolf]

[-- Attachment #1: Type: text/plain, Size: 966 bytes --]

Am 31.05.2017 um 19:03 hat Eric Blake geschrieben:
> On 05/31/2017 04:52 AM, Kevin Wolf wrote:
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >  tests/qemu-iotests/183     | 140 +++++++++++++++++++++++++++++++++++++++++++++
> >  tests/qemu-iotests/183.out |  46 +++++++++++++++
> >  tests/qemu-iotests/group   |   1 +
> >  3 files changed, 187 insertions(+)
> >  create mode 100755 tests/qemu-iotests/183
> >  create mode 100644 tests/qemu-iotests/183.out
> 
> > +qemu_comm_method="qmp"
> > +
> > +_launch_qemu \
> > +    -drive file="${TEST_IMG}",cache=${CACHEMODE},driver=$IMGFMT,id=disk
> 
> Inconsistent use of ${CACHEMODE} vs. $IMGFMT (I prefer the version with
> less typing, but others prefer always using ${}).  In both _launch_qemu
> lines.  Up to you if you want to tweak it, but either way,

That's what you get for copying the line and then appending one more
option... I'll remove the extra {} for $CACHEMODE.

Kevin

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]