> > > 3) Either change common/dump or xfs/068.out doesn't take much time/code, so I > > > decided to change 068.out to make x/068 to do operations. > > > > Did you validate that the file count is actually correct? > > > > The file count in these dump/restore tests is how we tell we've > > broken bulkstat iteration - any mismatch on the file count indicates > > bulkstat didn't find something it should have or it counted things > > twice. Hence I'm wary making changes to the file set size the > > dump/restore tests use. > > > > And, really, changing the file set size somewhat invalidates the > > long regression history we have with these existing tests. i.e. > > don't change tests that work correctly unless it's absolutely > > necessary. It only adds to the risk of introducing new bugs. > > Thanks so much for your detailed description :) I'll turn to change > FSSTRESS_AVOID in next V4 patch. > On the same discussion when Zorro added "insert" op, I asked why not convert test to whitelist of ops instead of having to update the blacklist every time fsstress is extended (which happened 3 times since we had this discussion). To my question, Dave answered: (https://marc.info/?l=fstests&m=149013211607727&w=2) > That means dump tests will /never/ exercise new fsstress operations > which, IMO, is much worse than having to keep a test up to date with > new operations. > > i.e. The whole point of adding new operations to fsstress and have > tests run them is that we automatically get coverage of new > functionality. Neutering this for all xfsdump tests by using "-z" is > - philosophically speaking - the wrong thing to do. For the life of me, I cannot follow the logic of how this argument ends up with a decision to update the blacklist. It is just me? Yes, if we use a whitelist we will /never/ exercise new fsstress operations for xfs/068. It may be the wrong thing to do "philosophically speaking" but in practice, we are doing this anyway, just manually every time an op is added. This is just madness. Eryu, can you please step in and end this ordeal once and for all. If we still want to exercise new fsstress ops for xfs/022 and future xfsdump tests (do we?) the solution is quite simple: factor out _create_dumpdir_stress_num_param and pass in whitelist of ops only from xfs/068 and be done with that. Zorro, be the final decision as it may, blacklist or whitelist, the test 068 is currently verifying that the file count is what happened to be the file count after addition of "insert" and "mread/mwrite" ops, which to the best of my knowledge, no one verified is the correct file count. So if we want to leave this test "unchanged", we'd better revert your last two changes to 068.out and resort to Eric's original test output and also exclude (or not include in whitelist) insert/mread/mwrite ops. Thanks, Amir.
