| Message ID | 1385430206-26900-4-git-send-email-famz@redhat.com |
|---|---|
| State | New |
| Headers | show |
This patch override only when default is specified, I am +1 with it. I think we had discuss before, just want a double check, stefan, do you agree with this? > This replaces _unsupported_qemu_io_options and check for support of > current cache mode, and allow to provide a default if user didn't > specify. > > Signed-off-by: Fam Zheng <famz@redhat.com> > --- > tests/qemu-iotests/026 | 3 ++- > tests/qemu-iotests/039 | 3 ++- > tests/qemu-iotests/052 | 4 ++-- > tests/qemu-iotests/common.rc | 25 +++++++++++++++---------- > 4 files changed, 21 insertions(+), 14 deletions(-) > > diff --git a/tests/qemu-iotests/026 b/tests/qemu-iotests/026 > index ebe29d0..c9c5f83 100755 > --- a/tests/qemu-iotests/026 > +++ b/tests/qemu-iotests/026 > @@ -44,7 +44,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 > _supported_fmt qcow2 > _supported_proto generic > _supported_os Linux > - > +_default_cache_mode "writethrough" > +_supported_cache_modes "writethrough" "none" > Why forbid mode = writeback? > echo "Errors while writing 128 kB" > echo > diff --git a/tests/qemu-iotests/039 b/tests/qemu-iotests/039 > index 8bade92..6abf472 100755 > --- a/tests/qemu-iotests/039 > +++ b/tests/qemu-iotests/039 > @@ -44,7 +44,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 > _supported_fmt qcow2 > _supported_proto generic > _supported_os Linux > -_unsupported_qemu_io_options --nocache > +_default_cache_mode "writethrough" > +_supported_cache_modes "writethrough" > same here. > size=128M > > diff --git a/tests/qemu-iotests/052 b/tests/qemu-iotests/052 > index f5f9683..4d4e411 100755 > --- a/tests/qemu-iotests/052 > +++ b/tests/qemu-iotests/052 > @@ -41,8 +41,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 > _supported_fmt generic > _supported_proto generic > _supported_os Linux > -_unsupported_qemu_io_options --nocache > - > +_default_cache_mode "writethrough" > +_supported_cache_modes "writethrough" > same question. > size=128M > _make_test_img $size > diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc > index 7f62457..47cef6d 100644 > --- a/tests/qemu-iotests/common.rc > +++ b/tests/qemu-iotests/common.rc > @@ -387,18 +387,23 @@ _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 > + for mode; do > + if [ "$mode" = "$CACHEMODE" ]; then > + return > + fi > done > + _notrun "not suitable for cache mode: $CACHEMODE" > +} > + > +_default_cache_mode() > +{ > + if $CACHEMODE_IS_DEFAULT; then > + CACHEMODE="$1" > + QEMU_IO="$QEMU_IO --cache $1" > + return > + fi > } > > # this test requires that a specified command (executable) exists >
On 2013年12月03日 14:05, Wenchao Xia wrote: > > Why forbid mode = writeback? > These test cases used to run with only possibly "writethrough", or "none". And they don't work with writeback, at least in my case. So I didn't add other modes here. Fam
On Tue, Dec 03, 2013 at 02:05:32PM +0800, Wenchao Xia wrote: > This patch override only when default is specified, I am +1 with it. > I think we had discuss before, just want a double check, stefan, do you > agree with this? Yes, I'm happy with this. I just want to avoid test cases with custom code that silently changes settings. As long as the default cache mode is declarative it's fine: _default_cache_mode "writethrough"
Am 03.12.2013 um 09:21 hat Fam Zheng geschrieben: > On 2013年12月03日 14:05, Wenchao Xia wrote: > > > > > Why forbid mode = writeback? > > > > These test cases used to run with only possibly "writethrough", or "none". > And they don't work with writeback, at least in my case. So I didn't add > other modes here. I suspect that the tests allowing writethrough might also work with directsync. Did you give that one a try? 026 has different output for wt and wb modes, this is why writeback fails. It should probably check against 026.out.nocache (which would have to be renamed as 026.out.wb) instead of 026.out. That's something for a separate series, though. Kevin
On 2013年12月03日 17:28, Kevin Wolf wrote: > Am 03.12.2013 um 09:21 hat Fam Zheng geschrieben: >> On 2013年12月03日 14:05, Wenchao Xia wrote: >> >>> >>> Why forbid mode = writeback? >>> >> >> These test cases used to run with only possibly "writethrough", or "none". >> And they don't work with writeback, at least in my case. So I didn't add >> other modes here. > > I suspect that the tests allowing writethrough might also work with > directsync. Did you give that one a try? > I guess so. Except for my /tmp is tmpfs and complains about O_DIRECT. It works for me with "TMPDIR=/var/tmp". > 026 has different output for wt and wb modes, this is why writeback > fails. It should probably check against 026.out.nocache (which would > have to be renamed as 026.out.wb) instead of 026.out. That's something > for a separate series, though. > Yes, good idea. I'll do this and the directsync including in the next series. Fam
diff --git a/tests/qemu-iotests/026 b/tests/qemu-iotests/026 index ebe29d0..c9c5f83 100755 --- a/tests/qemu-iotests/026 +++ b/tests/qemu-iotests/026 @@ -44,7 +44,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _supported_fmt qcow2 _supported_proto generic _supported_os Linux - +_default_cache_mode "writethrough" +_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..6abf472 100755 --- a/tests/qemu-iotests/039 +++ b/tests/qemu-iotests/039 @@ -44,7 +44,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _supported_fmt qcow2 _supported_proto generic _supported_os Linux -_unsupported_qemu_io_options --nocache +_default_cache_mode "writethrough" +_supported_cache_modes "writethrough" size=128M diff --git a/tests/qemu-iotests/052 b/tests/qemu-iotests/052 index f5f9683..4d4e411 100755 --- a/tests/qemu-iotests/052 +++ b/tests/qemu-iotests/052 @@ -41,8 +41,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _supported_fmt generic _supported_proto generic _supported_os Linux -_unsupported_qemu_io_options --nocache - +_default_cache_mode "writethrough" +_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..47cef6d 100644 --- a/tests/qemu-iotests/common.rc +++ b/tests/qemu-iotests/common.rc @@ -387,18 +387,23 @@ _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 + for mode; do + if [ "$mode" = "$CACHEMODE" ]; then + return + fi done + _notrun "not suitable for cache mode: $CACHEMODE" +} + +_default_cache_mode() +{ + if $CACHEMODE_IS_DEFAULT; then + CACHEMODE="$1" + QEMU_IO="$QEMU_IO --cache $1" + return + fi } # this test requires that a specified command (executable) exists
This replaces _unsupported_qemu_io_options and check for support of current cache mode, and allow to provide a default if user didn't specify. Signed-off-by: Fam Zheng <famz@redhat.com> --- tests/qemu-iotests/026 | 3 ++- tests/qemu-iotests/039 | 3 ++- tests/qemu-iotests/052 | 4 ++-- tests/qemu-iotests/common.rc | 25 +++++++++++++++---------- 4 files changed, 21 insertions(+), 14 deletions(-)