Re: [Qemu-block] [PATCH v7 24/24] iotests: Add test for block jobs and BDS ejection

2015-11-30 Thread Max Reitz 
On 30.11.2015 17:23, Kevin Wolf wrote:
> Am 09.11.2015 um 23:39 hat Max Reitz geschrieben:
>> Suggested-by: Paolo Bonzini 
>> Signed-off-by: Max Reitz 
>> ---
>>  tests/qemu-iotests/141 | 166 
>> +
>>  tests/qemu-iotests/141.out |  47 +
>>  tests/qemu-iotests/group   |   1 +
>>  3 files changed, 214 insertions(+)
>>  create mode 100755 tests/qemu-iotests/141
>>  create mode 100644 tests/qemu-iotests/141.out
>>
>> diff --git a/tests/qemu-iotests/141 b/tests/qemu-iotests/141
>> new file mode 100755
>> index 000..6a32d56
>> --- /dev/null
>> +++ b/tests/qemu-iotests/141
>> @@ -0,0 +1,166 @@
>> +#!/bin/bash
>> +#
>> +# Test case for ejecting BDSs with block jobs still running on them
>> +#
>> +# Copyright (C) 2015 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 .
>> +#
>> +
>> +# creator
>> +owner=mre...@redhat.com
>> +
>> +seq="$(basename $0)"
>> +echo "QA output created by $seq"
>> +
>> +here="$PWD"
>> +tmp=/tmp/$$
>> +status=1# failure is the default!
>> +
>> +_cleanup()
>> +{
>> +_cleanup_test_img
>> +rm -f "$TEST_DIR/{b,o}.$IMGFMT"
>> +}
>> +trap "_cleanup; exit \$status" 0 1 2 3 15
>> +
>> +# get standard environment, filters and checks
>> +. ./common.rc
>> +. ./common.filter
>> +. ./common.qemu
>> +
>> +# Needs backing file support
>> +_supported_fmt qcow qcow2 qed
> 
> The test doesn't work for me on qcow1.

Hm, and I thought I had tested it. Well, block jobs creating an overlay
file not being supported on qcow1 is probably all right.

>> +echo
>> +echo '=== Testing block-commit ==='
>> +echo
>> +
>> +# block-commit will send BLOCK_JOB_READY basically immediately, and 
>> cancelling
>> +# the job will consequently result in BLOCK_JOB_COMPLETED being emitted.
>> +
>> +test_blockjob \
>> +"{'execute': 'block-commit',
>> +  'arguments': {'device': 'drv0'}}" \
>> +'BLOCK_JOB_READY' \
>> +'BLOCK_JOB_COMPLETED'
> 
> This is commit of the active layer, i.e. just a mirror in disguise.
> Should we test a "real" commit block job as well?

Well, the op blocker we are testing is set by block_job_create(), so a
single block job would have sufficed. But now that I'm trying to test
them all, there's no reason not to test the real commit job, too.

> Anyway, with qcow1 removed from the list:
> Reviewed-by: Kevin Wolf 

Thanks!

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-block] [PATCH v7 24/24] iotests: Add test for block jobs and BDS ejection

2015-11-30 Thread Kevin Wolf 
Am 09.11.2015 um 23:39 hat Max Reitz geschrieben:
> Suggested-by: Paolo Bonzini 
> Signed-off-by: Max Reitz 
> ---
>  tests/qemu-iotests/141 | 166 
> +
>  tests/qemu-iotests/141.out |  47 +
>  tests/qemu-iotests/group   |   1 +
>  3 files changed, 214 insertions(+)
>  create mode 100755 tests/qemu-iotests/141
>  create mode 100644 tests/qemu-iotests/141.out
> 
> diff --git a/tests/qemu-iotests/141 b/tests/qemu-iotests/141
> new file mode 100755
> index 000..6a32d56
> --- /dev/null
> +++ b/tests/qemu-iotests/141
> @@ -0,0 +1,166 @@
> +#!/bin/bash
> +#
> +# Test case for ejecting BDSs with block jobs still running on them
> +#
> +# Copyright (C) 2015 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 .
> +#
> +
> +# creator
> +owner=mre...@redhat.com
> +
> +seq="$(basename $0)"
> +echo "QA output created by $seq"
> +
> +here="$PWD"
> +tmp=/tmp/$$
> +status=1 # failure is the default!
> +
> +_cleanup()
> +{
> +_cleanup_test_img
> +rm -f "$TEST_DIR/{b,o}.$IMGFMT"
> +}
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +# get standard environment, filters and checks
> +. ./common.rc
> +. ./common.filter
> +. ./common.qemu
> +
> +# Needs backing file support
> +_supported_fmt qcow qcow2 qed

The test doesn't work for me on qcow1.

> +echo
> +echo '=== Testing block-commit ==='
> +echo
> +
> +# block-commit will send BLOCK_JOB_READY basically immediately, and 
> cancelling
> +# the job will consequently result in BLOCK_JOB_COMPLETED being emitted.
> +
> +test_blockjob \
> +"{'execute': 'block-commit',
> +  'arguments': {'device': 'drv0'}}" \
> +'BLOCK_JOB_READY' \
> +'BLOCK_JOB_COMPLETED'

This is commit of the active layer, i.e. just a mirror in disguise.
Should we test a "real" commit block job as well?

Anyway, with qcow1 removed from the list:
Reviewed-by: Kevin Wolf

[Qemu-block] [PATCH v7 24/24] iotests: Add test for block jobs and BDS ejection

2015-11-09 Thread Max Reitz 
Suggested-by: Paolo Bonzini 
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/141 | 166 +
 tests/qemu-iotests/141.out |  47 +
 tests/qemu-iotests/group   |   1 +
 3 files changed, 214 insertions(+)
 create mode 100755 tests/qemu-iotests/141
 create mode 100644 tests/qemu-iotests/141.out

diff --git a/tests/qemu-iotests/141 b/tests/qemu-iotests/141
new file mode 100755
index 000..6a32d56
--- /dev/null
+++ b/tests/qemu-iotests/141
@@ -0,0 +1,166 @@
+#!/bin/bash
+#
+# Test case for ejecting BDSs with block jobs still running on them
+#
+# Copyright (C) 2015 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 .
+#
+
+# creator
+owner=mre...@redhat.com
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+here="$PWD"
+tmp=/tmp/$$
+status=1   # failure is the default!
+
+_cleanup()
+{
+_cleanup_test_img
+rm -f "$TEST_DIR/{b,o}.$IMGFMT"
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.qemu
+
+# Needs backing file support
+_supported_fmt qcow qcow2 qed
+_supported_proto file
+_supported_os Linux
+
+
+test_blockjob()
+{
+_send_qemu_cmd $QEMU_HANDLE \
+"{'execute': 'blockdev-add',
+  'arguments': {
+  'options': {
+  'id': 'drv0',
+  'driver': '$IMGFMT',
+  'file': {
+  'driver': 'file',
+  'filename': '$TEST_IMG'
+  " \
+'return'
+
+_send_qemu_cmd $QEMU_HANDLE \
+"$1" \
+"$2" \
+| _filter_img_create
+
+# We want this to return an error because the block job is still running
+_send_qemu_cmd $QEMU_HANDLE \
+"{'execute': 'blockdev-remove-medium',
+  'arguments': {'device': 'drv0'}}" \
+'error'
+
+_send_qemu_cmd $QEMU_HANDLE \
+"{'execute': 'block-job-cancel',
+  'arguments': {'device': 'drv0'}}" \
+"$3"
+
+_send_qemu_cmd $QEMU_HANDLE \
+"{'execute': 'x-blockdev-del',
+  'arguments': {'id': 'drv0'}}" \
+'return'
+}
+
+
+TEST_IMG="$TEST_DIR/b.$IMGFMT" _make_test_img 1M
+_make_test_img -b "$TEST_DIR/b.$IMGFMT" 1M
+
+_launch_qemu -nodefaults
+
+_send_qemu_cmd $QEMU_HANDLE \
+"{'execute': 'qmp_capabilities'}" \
+'return'
+
+echo
+echo '=== Testing drive-backup ==='
+echo
+
+# drive-backup will not send BLOCK_JOB_READY by itself, and cancelling the job
+# will consequently result in BLOCK_JOB_CANCELLED being emitted.
+
+test_blockjob \
+"{'execute': 'drive-backup',
+  'arguments': {'device': 'drv0',
+'target': '$TEST_DIR/o.$IMGFMT',
+'format': '$IMGFMT',
+'sync': 'none'}}" \
+'return' \
+'BLOCK_JOB_CANCELLED'
+
+echo
+echo '=== Testing drive-mirror ==='
+echo
+
+# drive-mirror will send BLOCK_JOB_READY basically immediately, and cancelling
+# the job will consequently result in BLOCK_JOB_COMPLETED being emitted.
+
+test_blockjob \
+"{'execute': 'drive-mirror',
+  'arguments': {'device': 'drv0',
+'target': '$TEST_DIR/o.$IMGFMT',
+'format': '$IMGFMT',
+'sync': 'none'}}" \
+'BLOCK_JOB_READY' \
+'BLOCK_JOB_COMPLETED'
+
+echo
+echo '=== Testing block-commit ==='
+echo
+
+# block-commit will send BLOCK_JOB_READY basically immediately, and cancelling
+# the job will consequently result in BLOCK_JOB_COMPLETED being emitted.
+
+test_blockjob \
+"{'execute': 'block-commit',
+  'arguments': {'device': 'drv0'}}" \
+'BLOCK_JOB_READY' \
+'BLOCK_JOB_COMPLETED'
+
+echo
+echo '=== Testing block-stream ==='
+echo
+
+# Give block-stream something to work on, otherwise it would be done
+# immediately, send a BLOCK_JOB_COMPLETED and ejecting the BDS would work just
+# fine without the block job still running.
+
+$QEMU_IO -c 'write 0 1M' "$TEST_DIR/b.$IMGFMT" | _filter_qemu_io
+
+# With some data to stream (and @speed set to 1), block-stream will not 
complete
+# until we send the block-job-cancel command. Therefore, no event other than
+# BLOCK_JOB_CANCELLED will be emitted.
+
+test_blockjob \
+"{'execute': 'block-stream',
+  'arguments': {'device': 'drv0',
+'speed': 1}}" \
+'return' \
+'BLOCK_JOB_CANCELLED