| Message ID | 1467296007-12252-6-git-send-email-kwolf@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 30.06.2016 16:13, Kevin Wolf wrote: > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > 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 > +owner=kwolf@redhat.com > + > +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 >
Am 02.07.2016 um 18:15 hat Max Reitz geschrieben: > On 30.06.2016 16:13, Kevin Wolf wrote: > > + 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. Yes, I know. Originally I wanted to add test cases that use node-name, but then it occurred to me that this would be pretty pointless. I'm not sure yet what the conclusion is. Change query-block to include anonymous BBs that are owned by devices? A new query command? Add the information to info qtree and whatever the QMP version of it is (if it even exists)? Kevin
On 04.07.2016 12:50, Kevin Wolf wrote: > Am 02.07.2016 um 18:15 hat Max Reitz geschrieben: >> On 30.06.2016 16:13, Kevin Wolf wrote: >>> + 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. > > Yes, I know. Originally I wanted to add test cases that use node-name, > but then it occurred to me that this would be pretty pointless. > > I'm not sure yet what the conclusion is. Change query-block to include > anonymous BBs that are owned by devices? A new query command? Add the > information to info qtree and whatever the QMP version of it is (if it > even exists)? Well, since you are basically trying to purge the BB from the user's field of view, it would probably make sense to display all the BB-level information (like the writethrough mode) as part of the device information; that is, probably in info qtree. That information should then probably also include the node name of the root BDS, and the user/management application can find out all about that with query-named-block-nodes (although it'd probably makes sense to add a query-block-node command for a single node). Max
On 07/05/2016 08:57 AM, Max Reitz wrote: >> I'm not sure yet what the conclusion is. Change query-block to include >> anonymous BBs that are owned by devices? A new query command? Add the >> information to info qtree and whatever the QMP version of it is (if it >> even exists)? > > Well, since you are basically trying to purge the BB from the user's > field of view, it would probably make sense to display all the BB-level > information (like the writethrough mode) as part of the device > information; that is, probably in info qtree. That information should > then probably also include the node name of the root BDS, and the > user/management application can find out all about that with > query-named-block-nodes (although it'd probably makes sense to add a > query-block-node command for a single node). Or teach query-named-block-nodes to take an optional parameter for filtering the results to a single node (either way is introspectible, so I don't have a strong opinion which is nicer)
Am 02.07.2016 um 18:15 hat Max Reitz geschrieben: > On 30.06.2016 16:13, Kevin Wolf wrote: > > +if [ "$QEMU_DEFAULT_MACHINE" != "pc" ]; then > > + _notrun "Test uses IDE devices" > > Why not just use virtio? Hm, and I guess use virtio-blk rather than virtio-blk-pci so that it works on all platforms? > Which brings me to my second question: Why does this test fail with > virtio (the BB always takes precedence then)? :-) Because you didn't fix or even mention the bug in patch 2. :-) Kevin
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 +owner=kwolf@redhat.com + +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" +fi + +size=128M +drive="if=none,file="$TEST_IMG",driver=$IMGFMT" + +_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 + echo "info block" \ + | run_qemu -drive "$drive,cache=$cache" \ + -device "ide-hd,drive=none0$wce" \ + | grep -e "Testing" -e "Cache mode" + 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
Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- 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