Message ID | 1384933457-26953-4-git-send-email-famz@redhat.com |
---|---|
State | New |
Headers | show |
On Wed, Nov 20, 2013 at 03:44:14PM +0800, Fam Zheng wrote: > -_unsupported_qemu_io_options() > +_supported_cache_modes() > { > - for bad_opt > - do > - for opt in $QEMU_IO_OPTIONS > - do > - if [ "$bad_opt" = "$opt" ] > - then > - _notrun "not suitable for qemu-io option: $bad_opt" > - fi > - done > + if $CACHEMODE_IS_DEFAULT; then > + QEMU_IO="$QEMU_IO -t $1" > + return > + fi > + for mode; do > + if [ "$mode" = "$CACHEMODE" ]; then > + return > + fi > done > + _notrun "not suitable for cache mode: $CACHEMODE" > } This seems weird to me: By default tests run with CACHEMODE=writethrough but test cases can use _supported_cache_modes() to switch to a different "default" behind the scenes? Why not keep it simple: If a test doesn't support CACHEMODE, it gets skipped. Stefan
On 2013年11月21日 20:39, Stefan Hajnoczi wrote: > On Wed, Nov 20, 2013 at 03:44:14PM +0800, Fam Zheng wrote: >> -_unsupported_qemu_io_options() >> +_supported_cache_modes() >> { >> - for bad_opt >> - do >> - for opt in $QEMU_IO_OPTIONS >> - do >> - if [ "$bad_opt" = "$opt" ] >> - then >> - _notrun "not suitable for qemu-io option: $bad_opt" >> - fi >> - done >> + if $CACHEMODE_IS_DEFAULT; then >> + QEMU_IO="$QEMU_IO -t $1" >> + return >> + fi >> + for mode; do >> + if [ "$mode" = "$CACHEMODE" ]; then >> + return >> + fi >> done >> + _notrun "not suitable for cache mode: $CACHEMODE" >> } > > This seems weird to me: > By default tests run with CACHEMODE=writethrough but test cases can use > _supported_cache_modes() to switch to a different "default" behind the > scenes? > > Why not keep it simple: > If a test doesn't support CACHEMODE, it gets skipped. > I thought Kevin wanted to override the cache mode (if not given by user), so the cases doesn't get skipped by default. I also feel that a test case used to actually run by default shouldn't be skipped unnoticed. I'm basically adding a meta cache mode "default" here, which means we will not have a "implicit yet forced" cache mode any more. On 2013年11月19日 18:21, Kevin Wolf wrote: > > Okay, I gave it some testing, too, and it looks like we need some > additional changes. There are some test cases that use: > > _unsupported_qemu_io_options --nocache > > Which obviously doesn't work any more. We need to replace it by a check > against $CACHEMODE (or, perhaps preferably, even override it > automatically, so that test cases aren't left out just because of the > cache mode) > > Test case 026 uses the option of having a 026.out.nocache, which differs > from the normal output. I suspect the correct differentiation is here > between writethrough and writeback modes. And of course, grepping for > '--nocache' to detect the condition doesn't work any more. > > Kevin > Fam
diff --git a/tests/qemu-iotests/026 b/tests/qemu-iotests/026 index ebe29d0..9078cd8 100755 --- a/tests/qemu-iotests/026 +++ b/tests/qemu-iotests/026 @@ -44,7 +44,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _supported_fmt qcow2 _supported_proto generic _supported_os Linux - +_supported_cache_modes "writethrough" "none" echo "Errors while writing 128 kB" echo diff --git a/tests/qemu-iotests/039 b/tests/qemu-iotests/039 index 8bade92..a298b50 100755 --- a/tests/qemu-iotests/039 +++ b/tests/qemu-iotests/039 @@ -44,7 +44,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _supported_fmt qcow2 _supported_proto generic _supported_os Linux -_unsupported_qemu_io_options --nocache +_supported_cache_modes "writethrough" size=128M diff --git a/tests/qemu-iotests/052 b/tests/qemu-iotests/052 index f5f9683..6d96c99 100755 --- a/tests/qemu-iotests/052 +++ b/tests/qemu-iotests/052 @@ -41,8 +41,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _supported_fmt generic _supported_proto generic _supported_os Linux -_unsupported_qemu_io_options --nocache - +_supported_cache_modes "writethrough" size=128M _make_test_img $size diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc index 7f62457..15d5bbd 100644 --- a/tests/qemu-iotests/common.rc +++ b/tests/qemu-iotests/common.rc @@ -387,18 +387,18 @@ _supported_os() _notrun "not suitable for this OS: $HOSTOS" } -_unsupported_qemu_io_options() +_supported_cache_modes() { - for bad_opt - do - for opt in $QEMU_IO_OPTIONS - do - if [ "$bad_opt" = "$opt" ] - then - _notrun "not suitable for qemu-io option: $bad_opt" - fi - done + if $CACHEMODE_IS_DEFAULT; then + QEMU_IO="$QEMU_IO -t $1" + return + fi + for mode; do + if [ "$mode" = "$CACHEMODE" ]; then + return + fi done + _notrun "not suitable for cache mode: $CACHEMODE" } # this test requires that a specified command (executable) exists
This replaces _unsupported_qemu_io_options and check for support of current cache mode. If user dosen't give "-c <mode>" or "-nocache", the first supported cache mode is used in qemu-io. Signed-off-by: Fam Zheng <famz@redhat.com> --- tests/qemu-iotests/026 | 2 +- tests/qemu-iotests/039 | 2 +- tests/qemu-iotests/052 | 3 +-- tests/qemu-iotests/common.rc | 20 ++++++++++---------- 4 files changed, 13 insertions(+), 14 deletions(-)