xfs
[Top] [All Lists]

Re: [PATCH v2 1/5] xfs_repair: refuse to run if we don't recognize versi

To: "Darrick J. Wong" <darrick.wong@xxxxxxxxxx>, david@xxxxxxxxxxxxx
Subject: Re: [PATCH v2 1/5] xfs_repair: refuse to run if we don't recognize version or feature flags
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Wed, 27 May 2015 10:27:33 -0500
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <5565DFEB.9040000@xxxxxxxxxxx>
References: <20150526225126.26434.69010.stgit@xxxxxxxxxxxxxxxx> <20150526225132.26434.82404.stgit@xxxxxxxxxxxxxxxx> <20150527054536.GB10175@xxxxxxxxxxxxxxxx> <5565DAA7.4040406@xxxxxxxxxxx> <5565DFEB.9040000@xxxxxxxxxxx>
On 5/27/15 10:16 AM, Eric Sandeen wrote:
> On 5/27/15 9:54 AM, Eric Sandeen wrote:
>> On 5/27/15 12:45 AM, Darrick J. Wong wrote:
>>> Apparently xfs_repair running on a v5 filesystem doesn't check the
>>> compat, rocompat, or incompat feature flags for bits that it doesn't
>>> know about, which means that old xfs_repairs can wreak havoc.  So,
>>> strengthen the checks to prevent repair from "repairing" anything it
>>> doesn't understand.
>>>
>>> v2: Move the complaint code after the version number check, and print
>>> the actual feature bits that we don't recognize.
>>
>> Thanks.  That's a bit more wordy, but probably better, in the end.
>>
>> Reviewed-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> 
> Whoops, retract that, thanks for spotting those C&P errors, Fanael!

I wonder if something like:

        /* Look for V5 feature flags we don't know about */
        if (XFS_SB_VERSION_NUM(sb) >= XFS_SB_VERSION_5 &&
            (xfs_sb_has_ro_compat_feature(sb, XFS_SB_FEAT_RO_COMPAT_UNKNOWN) ||
             xfs_sb_has_incompat_feature(sb, XFS_SB_FEAT_INCOMPAT_UNKNOWN) ||
             xfs_sb_has_compat_feature(sb, XFS_SB_FEAT_COMPAT_UNKNOWN))) {
                printf("unknown feature flags 0x%x/0x%x/0x%x\n", 
                        sb->sb_features_ro_compat & 
XFS_SB_FEAT_RO_COMPAT_UNKNOWN,
                        sb->sb_features_incompat & XFS_SB_FEAT_INCOMPAT_UNKNOWN,
                        sb->sb_features_compat & XFS_SB_FEAT_COMPAT_UNKNOWN);
                ...

would suffice, given that the user will need to read code to unerstand the hex
values, anyway.
        
Hm, and as Fanael also pointed out, "compat" features ... should be ok, right,
and can be removed from the exclusions?

-Eric

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