xfstests - unchecked mount failures

xfstests - unchecked mount failures

[David Sterba]

Hi,

I've encountered a bad situation when a failed mount in test 269 did not stop
the test and continued to use the mount point and exhausted space on the root
partition. A quick grep revealed that there are more tests with unchecked
_scratch_mount calls.

The underlying problem with failed mount was observed when the mount comes in a
quick sequence after mount, I saw it with btrfs, and don't know if it affects
other filesystems.

So, either all callers should check the return value or _scratch_mount
calls _fail. I'd go for the latter as it will make it more resilient
against unintentional ommision of checking the retval in new tests and
reviewer does not have keep that in mind.


david

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: xfstests - unchecked mount failures

[Eric Sandeen]

On 3/9/12 7:28 AM, David Sterba wrote:
> Hi,
> 
> I've encountered a bad situation when a failed mount in test 269 did not stop
> the test and continued to use the mount point and exhausted space on the root
> partition. A quick grep revealed that there are more tests with unchecked
> _scratch_mount calls.
> 
> The underlying problem with failed mount was observed when the mount comes in a
> quick sequence after mount, I saw it with btrfs, and don't know if it affects
> other filesystems.
> 
> So, either all callers should check the return value or _scratch_mount
> calls _fail. I'd go for the latter as it will make it more resilient
> against unintentional ommision of checking the retval in new tests and
> reviewer does not have keep that in mind.

Sounds good to me; _test_mount() should probably do the same?

I guess it'd be worth investigating exactly why it failed, though.

Still, if you'd like to send a patch to _fail in the mount helpers
if they fail, that sounds reasonable to me.

Thanks,
-Eric

> 
> david
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: xfstests - unchecked mount failures

[Dave Chinner]

On Fri, Mar 09, 2012 at 02:28:28PM +0100, David Sterba wrote:
> Hi,
> 
> I've encountered a bad situation when a failed mount in test 269 did not stop
> the test and continued to use the mount point and exhausted space on the root
> partition. A quick grep revealed that there are more tests with unchecked
> _scratch_mount calls.
>
> The underlying problem with failed mount was observed when the mount comes in a
> quick sequence after mount,

Sorry, what? Do you mean mount after mkfs? 

> I saw it with btrfs, and don't know if it affects
> other filesystems.

If btrfs is failing to mount because it happens too soon after mkfs,
then that's a btrfs bug, not a xfstests problem.

> So, either all callers should check the return value or _scratch_mount
> calls _fail.

Some tests expect _scratch_mount to fail, so you can't change how
_scratch_mount behaves....

> I'd go for the latter as it will make it more resilient
> against unintentional ommision of checking the retval in new tests and
> reviewer does not have keep that in mind.

I think it is fine to assume that you can mount a filesystem that
you just run mkfs on. If you are testing something that you expect
failure, then sure, check the return, but immeidately after mkfs
(that will fail the test if it fails) it is reasonable to assume
that mount will work.

Of course, if you want to add return value checking to all the
current unchecked callers of _scratch_mount then send a patch
for review. ;)

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs