xfs
[Top] [All Lists]

Re: [PATCH] xfs: don't ASSERT on corrupt ftype

To: Dave Chinner <david@xxxxxxxxxxxxx>, Eric Sandeen <sandeen@xxxxxxxxxx>
Subject: Re: [PATCH] xfs: don't ASSERT on corrupt ftype
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Mon, 08 Sep 2014 09:02:27 -0500
Cc: xfs-oss <xfs@xxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <540DB3FE.6010309@xxxxxxxxxxx>
References: <540D011B.2000807@xxxxxxxxxx> <20140908130507.GN30012@dastard> <540DB3FE.6010309@xxxxxxxxxxx>
On 9/8/14 8:49 AM, Eric Sandeen wrote:
On 9/8/14 8:05 AM, Dave Chinner wrote:
On Sun, Sep 07, 2014 at 08:06:35PM -0500, Eric Sandeen wrote:
xfs_dir3_data_get_ftype() and xfs_dir2_sf_check() get
the file type off disk, but ASSERT if it's invalid:

    ASSERT(type < XFS_DIR3_FT_MAX);

This might be cut & paste from the "put" functions,
which should be checking that they've not been passed
bad values, but we shouldn't ASSERT on bad values
read from disk.

No, they weren't cut-n-paste from the put functions. They were
actually designed for a metadata block where bad values would not be
written to disk, and corrupted disk blocks would be detected by CRC
validation failures. So on debug kernels it's quite appropriate to
assert fail on a "should never, ever happen" condition.

hohum, ok.

So then presumably the reason there is no ASSERT in xfs_dir3_sfe_get_ftype
(vs in xfs_dir3_data_get_ftype) is also purely intentional and
part of the design, but I'm unable to divine that logic... can you
help me out?

I guess the only way forward is to create a 3rd set of ops, and have
one for dir2, one for dir2-with-ftype, and one for dir3?  Because
in the op, there's no way to discern between the latter 2, and
know if we're previously CRC-protected or not...

-Eric

<Prev in Thread] Current Thread [Next in Thread>