On 06/23/13 18:44, Dave Chinner wrote:
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.
... and the reason for the open question. Should there be a group
notation that says the verifiers will *correctly* find a real and known
problem on this test.
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*
That works too.
Thanks,
--Mark.
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
