On Sun, Jun 23, 2013 at 05:57:49PM -0500, Eric Sandeen wrote: > On 6/23/13 5:50 PM, Dave Chinner wrote: > > On Fri, Jun 21, 2013 at 01:45:46PM -0500, Eric Sandeen wrote: > >> On 6/20/13 12:54 PM, Mark Tinguely wrote: > >>> Do we need a xfstest verifier dangerous group? > >>> > >>> xfstest 111 purposely damages inodes. In hindsight it make sense > >>> that it asserts when running with verifiers. > >> > >> But it only asserts on a debug kernel... > > > > Right, and it has done so for years - blaming verifiers for > > triggering the assert failure is simply shooting the messenger. > > But this test *intentionally* corrupts, right? So it's prudent > to not run a test which you *know* will explode if it runs > as designed. Common sense, really. > >> This isn't the only place where corruption could ASSERT on debug; > >> see xlog_recover_add_to_trans() for example. > >> > >> But if the test intentionally corrupts it and that leads to > >> an ASSERT that does seem problematic for anyone testing w/ debug > >> enabled. > > > > Yup, it runs src/itrash.c which corrupts every inode it can find. > > > > That's the reason this test is not part of the auto group - it's > > a test that will cause the system to stop. We've got other tests > > that are not part of the auto group for exactly the same reason - > > they cause some kind of terminal failure and so aren't candidates > > for regression testing. > > Then maybe just part of the normal dangerous group would be enough. It will only run from the ioctl group today (bulkstat, I guess), so I'd say that adding it to the dangerous group doesn't add any real value except documentation. And it's just as easy to remove the ASSERT() as it is really unnecessary.... > Except this isn't transient (today) - it's not a case where old kernels > may oops, it's where it's *designed* to oops on this test, with a debug > kernel. > > So I guess I could see a debug-dangerous group ;) > > >> I guess I'd vote for removing the ASSERT unless there's > >> some reason it should be there - Dave? > > > > I'm fine with it being removed - we catch the failure just fine. If > > that then makes 111 work as a regression test (i.e. doesn't trigger > > the bad-inode bulkstat loop it was designed to test) then perhaps we > > can consider making that part of the auto group, too... > > Removing it sounds like the best option then. *nod* Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
