[PATCH, RFC] xfs_repair - clear inodes in incorrect btree format

[Eric Sandeen]

Olaf Weber wrote:
> Eric Sandeen writes:
> 
> [...]
> 
>> But questions remain...
> 
>> 1) How'd it get into this state? ... but maybe more importantly...
>> 2) Should these really get cleared?  It's possibly a sane extent list,
>> it's just that it -could- be in extents rather than btree format...
>> 3) By the same token, should the kernel really be choking on it?
> 
> It is not clear to me yet how you could get into this state, but this
> is clearly an invariant the kernel actively maintains.
> 
> If the kernel "just" missed the underflow and kept the extents in
> btree format, then I don't see an apriori reason why the extent list
> as such would be invalid (as opposed to inefficiently stored).  If
> that's the primary model for getting into this state, then the file
> contents can be rescued and kernel-side the event should be
> survivable.
> 
> But if the kernel tried to convert, failed, and didn't properly detect
> failure...  Without having a good answer for (1) I find it hard to
> convince myself that to be more forgiving wrt to (2) and (3) is safe.

True.

Though FWIW, in this case, the user edited out the kernel check,
mounted, and successfully copied off the files before running xfs_repair
and nuking it with this patch.

So I guess bonus points would be teaching repair to fix up the problem
if the list seems valid ... and maybe even the same kernelside....

Thanks,
-Eric

> Olaf
> 
> 
>> Signed-off-by: Eric Sandeen <sandeen at sandeen.net>
> 
> Acked-By: Olaf Weber <olaf at sgi.com>
> 
>> ---
> 
>> diff --git a/repair/dinode.c b/repair/dinode.c
>> index 84e1d05..3fc6cac 100644
>> --- a/repair/dinode.c
>> +++ b/repair/dinode.c
>> @@ -1280,6 +1280,14 @@ process_btinode(
>>  			last_key = cursor.level[level-1].first_key;
>>  		}
>>  	}
>> +	if (*nex <= XFS_DFORK_SIZE(dip, mp, whichfork) / sizeof(xfs_bmbt_rec_t)) {
>> +		do_warn(_("extent count for ino %lld %s fork too low "
>> +			  "(%d) for file format\n"),
>> +				lino,
>> +				whichfork == XFS_DATA_FORK ? _("data") : _("attr"),
>> +				*nex);
>> +		return(1);
>> +	}
>>  	/*
>>  	 * Check that the last child block's forward sibling pointer
>>  	 * is NULL.
> 
>