lkml.org 
[lkml]   [2009]   [Aug]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 1/1] XFS: xfs_iformat realtime device target pointer check
    From
    Date
    On Tue, 2009-08-04 at 23:15 -0500, Eric Sandeen wrote:
    > Ramon de Carvalho Valle wrote:
    > > On Tue, 2009-08-04 at 14:11 -0500, Eric Sandeen wrote:
    > >> Ramon de Carvalho Valle wrote:
    > >>> The xfs_iformat function does not check if the realtime device target pointer
    > >>> is valid when the XFS_DIFLAG_REALTIME flag is set on the ondisk inode
    > >>> structure.
    > >>>
    > >>> Signed-off-by: Christoph Hellwig <hch@infradead.org>
    > >>> Signed-off-by: Ramon de Carvalho Valle <ramon@risesecurity.org>
    > >>> Cc: stable <stable@kernel.org>
    > >>> ---
    > >>> fs/xfs/xfs_inode.c | 23 +++++++++++++++++------
    > >>> 1 files changed, 17 insertions(+), 6 deletions(-)
    > >>>
    > >>> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
    > >>> index 1f22d65..37d3ee5 100644
    > >>> --- a/fs/xfs/xfs_inode.c
    > >>> +++ b/fs/xfs/xfs_inode.c
    > >>> @@ -343,13 +343,24 @@ xfs_iformat(
    > >>> return XFS_ERROR(EFSCORRUPTED);
    > >>> }
    > >>>
    > >>> + if (unlikely((ip->i_d.di_flags & XFS_DIFLAG_REALTIME) &&
    > >>> + !ip->i_mount->m_rtdev_targp)) {
    > >>> + xfs_fs_repair_cmn_err(CE_WARN, ip->i_mount,
    > >>> + "corrupt dinode %Lu, flags = 0x%x.",
    > >>> + (unsigned long long)ip->i_ino,
    > >>> + ip->i_d.di_flags);
    > >>> + XFS_CORRUPTION_ERROR("xfs_iformat(3)", XFS_ERRLEVEL_LOW,
    > >>> + ip->i_mount, dip);
    > >> I think I'd rather not change all the corruption text tag ordering;
    > >> it'll make it harder to track down any common occurrences of
    > >> "xfs_iformat(X)" corruption in the future if they get renumbered now.
    > >>
    > >> I'd either make this xfs_iformat(2.1) ;) or just leave it as Christoph
    > >> had. "realtime" is a lot more informative than "3" anyway.
    > >
    > > I don't think this is a bad decision, because the corruption errors can
    > > be easily identified by the output of xfs_fs_repair_cmn_err and the
    > > source line. I think this is a reasonable change that will keep the code
    > > clean and consistent.
    >
    > Until you wind up looking at a problem from some old kernel, or modified
    > vendor kernel, and you realize that now you really don't know which
    > error "xfs_iformat(6)" is anymore, and you either have to go digging
    > through trees that aren't handy, or you just give up and don't bother to
    > help because now it's too much of a pain. ;)
    >
    > But I can leave it up to the folks @ sgi, I can see both sides of the
    > argument, and I won't care too much either way.

    Yes, whatever they decide should be ok. Thanks for your feedback Eric.

    -Ramon

    >
    > Thanks,
    > -Eric
    >
    > > -Ramon
    > >
    > >> -Eric
    >
    >
    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2009-08-05 15:23    [W:0.028 / U:0.288 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site