On Thu, Oct 11, 2012 at 05:41:39PM -0400, Christoph Hellwig wrote:
> On Tue, Oct 09, 2012 at 02:50:55PM +1100, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@xxxxxxxxxx>
> >
> > Add a superblock verify callback function and pass it into the
> > buffer read functions. Remove the now redundant verification code
> > that is currently in use.
> >
> > Adding verification shows that secondary superblocks never have
> > their "sb_inprogress" flag cleared by mkfs.xfs, so when validating
> > the secondary superblocks during a grow operation we have to avoid
> > checking this field. Even if we fix mkfs, we will still have to
> > ignore this field for verification purposes unless a version of mkfs
> > that does not have this bug was used.
>
> > @@ -304,9 +304,8 @@ STATIC int
> > xfs_mount_validate_sb(
> > xfs_mount_t *mp,
> > xfs_sb_t *sbp,
> > - int flags)
> > + bool check_inprogress)
> > {
> > - int loud = !(flags & XFS_MFSI_QUIET);
>
> I don't think removing this is a good idea. The quiet flag is used
> to silence all filesystem warnings when the kernel is blindly trying
> all filesystem types when searching for the correct root fs type.
>
> If we always print warnings here people will get annoying messages
> when that happens for a non-XFS rootfs that we're asked to verify.
>
> I'd rather make check_inprogress another flag here.
It's a little more complex than that - I'd like to have the warnings
emitted on every superblock read (primary or secondary), but only
some of them come through the mount path. e.g. we re-read the
superblock during the log recovery sequence, so even if it is a
silent mount, I want to know that log recovery corrupted the
superblock and what it corrupted....
Indeed, if the kernel is trying random filesystem mounts on a block
device, then we'll fail the magic number check and abort
immediately, so I think that's really the only message we need "loud"
protection on. If you agree that's all we'll need to check, then I
can write a couple of simple wrappers that do:
xfs_sb_read_verify()
{
/* validate contents of superblock loudly */
}
xfs_sb_quiet_read_verify()
{
if (!magic num mismatch) {
/* XFS filesystem, be loud now */
xfs_sb_read_verify();
return;
}
/* quitely fail now */
xfs_buf_ioerror(bp, EFSCORRUPTED);
....
}
Would that solve the problem?
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
