On 2019/9/9 下午10:17, Nikolay Borisov wrote:
>
>
> On 5.09.19 г. 10:57 ч., Qu Wenruo wrote:
>> Before this patch, repair_imode_common() can only handle two types of
>> inodes:
>> - Free space cache inodes
>> - ROOT DIR inodes
>>
>> For inodes in subvolume trees, the core complexity is how to determine the
>> correct imode, thus it was not implemented.
>>
>> However there are more reports of incorrect imode in subvolume trees, we
>> need to support such fix.
>>
>> So this patch adds a new function, detect_imode(), to detect imode for
>> inodes in subvolume trees.
>>
>> That function will determine imode by:
>> - Search for INODE_REF
>> If we have INODE_REF, we will then try to find DIR_ITEM/DIR_INDEX.
>> As long as one valid DIR_ITEM or DIR_INDEX can be found, we convert
>> the BTRFS_FT_* to imode, then call it a day.
>> This should be the most accurate way.
>>
>> - Search for DIR_INDEX/DIR_ITEM
>> If above search fails, we falls back to locate the DIR_INDEX/DIR_ITEM
>> just after the INODE_ITEM.
>> If any can be found, it's definitely a directory.
>>
>> - Search for EXTENT_DATA
>> If EXTENT_DATA can be found, it's either REG or LNK.
>> For this case, we default to REG, as user can inspect the file to
>> determine if it's a file or just a path.
>>
>> - Use rdev to detect BLK/CHR
>> If all above fails, but INODE_ITEM has non-zero rdev, then it's either
>> a BLK or CHR file. Then we default to BLK.
>>
>> - Fail out if none of above methods succeeded
>> No educated guess to make things worse.
>>
>> Signed-off-by: Qu Wenruo <w...@suse.com>
>> ---
>> check/mode-common.c | 130 +++++++++++++++++++++++++++++++++++++++-----
>> 1 file changed, 117 insertions(+), 13 deletions(-)
>>
>> diff --git a/check/mode-common.c b/check/mode-common.c
>> index c0ddc50a1dd0..abea2ceda4c4 100644
>> --- a/check/mode-common.c
>> +++ b/check/mode-common.c
>> @@ -935,6 +935,113 @@ out:
>> return ret;
>> }
>>
>> +static int detect_imode(struct btrfs_root *root, struct btrfs_path *path,
>> + u32 *imode_ret)
>
> I think the imode is less than u32 so it should be possible to return it
> directly from the function as a positive number and error as negative?
It still doesn't look good enough to me.
Combining data value from error number is not a good idea to me, even
even the range doesn't overlap.
>
>> +{
>> + struct btrfs_key key;
>> + struct btrfs_inode_item iitem;
>> + const u32 priv = 0700;
>
> Having this in a variable doesn't bring more clarity, just use 0700
> directly at the end of the function.
I'm OK with that.
Thanks,
Qu
