On Fri, Nov 02, 2018 at 03:39:51PM -0500, Jayashree Mohan wrote: > Hi Filipe, > > Thanks for the feedback on the patch. Will fix the coding style as you > suggested. > > > For this type of tests, I think it's a good idea to let fsck run. > > > > Even if all of the links are persisted, the log/journal replay might > > have caused metadata inconsistencies in the fs for example - this was > > true for many cases I fixed over the years in btrfs. > > Even if fsck doesn't report any problem now, it's still good to run > > it, to help prevent future regressions. > > > > Plus this test creates a very small fs, it's not like fsck will take a > > significant time to run. > > So for all these reasons I would unmount and fsck after each test. This looks reasonable to me. > > Originally, there are 300 odd crash-consistency tests generated by > CrashMonkey. To run fsck after each test, we will have to convert each > of these tests into an equivalent xfstest test-case. We previously had > a discussion about this, on the thread here ( > https://www.spinics.net/lists/fstests/msg10718.html ) and the > suggestion was to batch similar tests together to reduce the external > per-test overhead due to scrub/fsck. You could batch similar tests together but still do fsck after each sub-test by calling _check_scratch_fs manually, and call _require_scratch_nocheck to indicate there's no need to do fsck at the end of test. > Additionally, batching similar tests will result in around 15 new test > cases that could be added to the 'quick group', as opposed to adding > 300 new tests. I think we could batch similar tests and create relatively small fs (e.g. 256M, as btrfs defaults to mixed mode if fs < 256M, and btrfs folks wanted to test non-mixed mode by default) and run fsck after each sub-test first, then see how long the tests take. Thanks, Eryu > > However, if you still recommend that fsck be run after each test, I > can submit patches for 300 individual test cases. Let me know which > one is preferable. > > Thanks, > Jayashree.
