Re: [Qemu-block] [PATCH v2 5/6] qemu-iotests: Test setting WCE with qdev

[Max Reitz]

On 30.06.2016 16:13, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
>  tests/qemu-iotests/157     | 92 
> ++++++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/157.out | 22 +++++++++++
>  tests/qemu-iotests/group   |  1 +
>  3 files changed, 115 insertions(+)
>  create mode 100755 tests/qemu-iotests/157
>  create mode 100644 tests/qemu-iotests/157.out
> 
> diff --git a/tests/qemu-iotests/157 b/tests/qemu-iotests/157
> new file mode 100755
> index 0000000..956cbdb
> --- /dev/null
> +++ b/tests/qemu-iotests/157
> @@ -0,0 +1,92 @@
> +#!/bin/bash
> +#
> +# Test command line configuration of block devices with qdev
> +#
> +# Copyright (C) 2016 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
> address@hidden
> +
> +seq="$(basename $0)"
> +echo "QA output created by $seq"
> +
> +here="$PWD"
> +status=1     # failure is the default!
> +
> +_cleanup()
> +{
> +     _cleanup_test_img
> +}
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +# get standard environment, filters and checks
> +. ./common.rc
> +. ./common.filter
> +
> +_supported_fmt generic
> +_supported_proto file
> +_supported_os Linux
> +
> +function do_run_qemu()
> +{
> +    echo Testing: "$@"
> +    (
> +        if ! test -t 0; then
> +            while read cmd; do
> +                echo $cmd
> +            done
> +        fi
> +        echo quit
> +    ) | $QEMU -nodefaults -nographic -monitor stdio -serial none "$@"
> +    echo
> +}
> +
> +function run_qemu()
> +{
> +    do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qemu | 
> _filter_generated_node_ids
> +}
> +
> +
> +if [ "$QEMU_DEFAULT_MACHINE" != "pc" ]; then
> +    _notrun "Test uses IDE devices"

Why not just use virtio? Which brings me to my second question: Why does
this test fail with virtio (the BB always takes precedence then)? :-)

> +fi
> +
> +size=128M
> +drive="if=none,file="$TEST_IMG",driver=$IMGFMT"

I don't think the quotation marks around $TEST_IMG are right.


> +
> +_make_test_img $size
> +
> +echo
> +echo "=== Setting WCE with qdev and with manually created BB ==="
> +echo
> +
> +# The qdev option takes precedence, but if it isn't given or 'auto', the BB
> +# option is used instead.
> +
> +for cache in "writeback" "writethrough"; do
> +    for wce in "" ",write-cache=auto", ",write-cache=on", 
> ",write-cache=off"; do

Commas between the values are wrong. They don't hurt, but they're part
of each value then.

> +        echo "info block" \
> +            | run_qemu -drive "$drive,cache=$cache" \
> +                       -device "ide-hd,drive=none0$wce" \
> +            | grep -e "Testing" -e "Cache mode"

Something interesting: If you'd specify the drive through a node name,
then the BDS tree has two BBs, one implicitly created with -drive (this
one is named (automatically) and owned by the monitor) and an anonymous
one for the device. If the device then overrides the cache mode, this
will not be reflected in the monitor-owned BB. "info block" (and
query-block) use the monitor BBs, however, so they'll report the BB on
top of the BDS tree in question to be in whatever mode has been
specified in -drive, whereas the mode the device uses for accessing that
BDS tree actually has nothing to do with that.

So the user has no way of inquiring the cache mode used by the device,
and they actually get presented misleading information.

Max

> +    done
> +done
> +
> +# success, all done
> +echo "*** done"
> +rm -f $seq.full
> +status=0
> diff --git a/tests/qemu-iotests/157.out b/tests/qemu-iotests/157.out
> new file mode 100644
> index 0000000..2e41e83
> --- /dev/null
> +++ b/tests/qemu-iotests/157.out
> @@ -0,0 +1,22 @@
> +QA output created by 157
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
> +
> +=== Setting WCE with qdev and with manually created BB ===
> +
> +Testing: -drive if=none,file=TEST_DIR/t.qcow2,driver=qcow2,cache=writeback 
> -device ide-hd,drive=none0
> +    Cache mode:       writeback
> +Testing: -drive if=none,file=TEST_DIR/t.qcow2,driver=qcow2,cache=writeback 
> -device ide-hd,drive=none0,write-cache=auto,
> +    Cache mode:       writeback
> +Testing: -drive if=none,file=TEST_DIR/t.qcow2,driver=qcow2,cache=writeback 
> -device ide-hd,drive=none0,write-cache=on,
> +    Cache mode:       writeback
> +Testing: -drive if=none,file=TEST_DIR/t.qcow2,driver=qcow2,cache=writeback 
> -device ide-hd,drive=none0,write-cache=off
> +    Cache mode:       writethrough
> +Testing: -drive 
> if=none,file=TEST_DIR/t.qcow2,driver=qcow2,cache=writethrough -device 
> ide-hd,drive=none0
> +    Cache mode:       writethrough
> +Testing: -drive 
> if=none,file=TEST_DIR/t.qcow2,driver=qcow2,cache=writethrough -device 
> ide-hd,drive=none0,write-cache=auto,
> +    Cache mode:       writethrough
> +Testing: -drive 
> if=none,file=TEST_DIR/t.qcow2,driver=qcow2,cache=writethrough -device 
> ide-hd,drive=none0,write-cache=on,
> +    Cache mode:       writeback
> +Testing: -drive 
> if=none,file=TEST_DIR/t.qcow2,driver=qcow2,cache=writethrough -device 
> ide-hd,drive=none0,write-cache=off
> +    Cache mode:       writethrough
> +*** done
> diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
> index 1c6fcb6..3a3973e 100644
> --- a/tests/qemu-iotests/group
> +++ b/tests/qemu-iotests/group
> @@ -156,3 +156,4 @@
>  154 rw auto backing quick
>  155 rw auto
>  156 rw auto quick
> +157 auto
>

signature.asc
Description: OpenPGP digital signature