diff mbox

[v3,3/6] qemu-iotests: Add _supported_cache_modes

Message ID 1384933457-26953-4-git-send-email-famz@redhat.com
State New
Headers show

Commit Message

Fam Zheng Nov. 20, 2013, 7:44 a.m. UTC
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(-)

Comments

Stefan Hajnoczi Nov. 21, 2013, 12:39 p.m. UTC | #1
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
Fam Zheng Nov. 21, 2013, 12:46 p.m. UTC | #2
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 mbox

Patch

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