On Fri, Nov 22, 2013 at 10:30:30AM +0100, Kevin Wolf wrote: > Am 22.11.2013 um 09:57 hat Stefan Hajnoczi geschrieben: > > On Thu, Nov 21, 2013 at 03:16:23PM +0100, Kevin Wolf wrote: > > > Am 21.11.2013 um 13:43 hat Stefan Hajnoczi geschrieben: > > > > On Wed, Nov 20, 2013 at 03:44:11PM +0800, Fam Zheng wrote: > > > > > This series adds cache mode option in the iotests framework. Test > > > > > cases are > > > > > updated to make use of cache mode and mask supported modes. > > > > > > > > > > v3: Change _unsupported_qemu_io_options to _supported_cache_modes. > > > > > Change default mode to "writeback". > > > > > Clean up some whitespaces in the end of series. > > > > > Fix "026.out.nocache" case. > > > > > Fix 048 case on tmpfs. > > > > > > > > > > > > > > > Fam Zheng (6): > > > > > qemu-iotests: Add "-c <cache-mode>" option > > > > > qemu-iotests: Honour cache mode in iotests.py > > > > > qemu-iotests: Add _supported_cache_modes > > > > > qemu-iotests: Change default cache mode to "writeback" > > > > > qemu-iotests: Force qcow2 in error path test in 048 > > > > > qemu-iotests: Clean up spaces in usage output > > > > > > > > > > tests/qemu-iotests/026 | 2 +- > > > > > tests/qemu-iotests/039 | 2 +- > > > > > tests/qemu-iotests/048 | 8 +++++++- > > > > > tests/qemu-iotests/052 | 3 +-- > > > > > tests/qemu-iotests/check | 2 +- > > > > > tests/qemu-iotests/common | 37 > > > > > +++++++++++++++++++++++++++---------- > > > > > tests/qemu-iotests/common.rc | 20 ++++++++++---------- > > > > > tests/qemu-iotests/iotests.py | 3 ++- > > > > > 8 files changed, 50 insertions(+), 27 deletions(-) > > > > > > > > In principle I'm happy with the series but there are two instances where > > > > you are silently changing what the test does (overriding cache mode and > > > > image format). > > > > > > > > Please skip tests that cannot run instead of silently testing something > > > > else. > > > > > > Skipping isn't really a good solution either. For example, I almost > > > never test 039, because I always run ./check -nocache. What would be > > > most useful is to use a working cache mode for this test rather than > > > leaving it untested over weeks and months. > > > > > > I mean we can make things as sophisticated as we want, and introduce a > > > list of cache modes in the order of their preference, so that I can say > > > ./check -c none,writethrough, but that's probably a bit overengineered. > > > > Why not just: > > > > # Before sending a block pull request > > ./check -c none > > ./check -c writeback > > > > That covers both O_DIRECT and buffered I/O. > > And takes about twice the time. Sorry, no, in practice it means that 039 > (and I think one or two more) stays untested by me, because running all > test cases a second time with writethrough (not writeback) would take > forever. > > > I think 1:1 behavior is important to keep it easy to reason about test > > cases. If test cases go off an change behavior behind the scenes it's > > possible to get confused or draw the wrong conclusions. > > I can understand your position and would agree in theory. I just think > that running the test with more difficult reasoning is preferable to > not running the test and therefore not reasoning about it even though > the reasoning would have been easy.
I think we've gotten some good ideas from an IRC discussion with Fam and Wenchao. A "./check" default mode that runs every test case with appropriate settings is useful. This makes it easy to run the tests and gives us good coverage. Stefan