On 2019/9/4 下午3:37, Nikolay Borisov wrote:
>
>
> On 3.09.19 г. 11:24 ч., 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 problem 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 (or call it
>> educated guess) imode for inodes in subvolume trees.
>>
>> Signed-off-by: Qu Wenruo <w...@suse.com>
>> ---
>> check/mode-common.c | 96 +++++++++++++++++++++++++++++++++++++++------
>> 1 file changed, 83 insertions(+), 13 deletions(-)
>>
>> diff --git a/check/mode-common.c b/check/mode-common.c
>> index 195b6efaa7aa..807d7daf98a6 100644
>> --- a/check/mode-common.c
>> +++ b/check/mode-common.c
>> @@ -836,6 +836,80 @@ int reset_imode(struct btrfs_trans_handle *trans,
>> struct btrfs_root *root,
>> return ret;
>> }
>>
>> +static int detect_imode(struct btrfs_root *root, struct btrfs_path *path,
>> + u32 *imode_ret)
>> +{
>> + struct btrfs_key key;
>> + struct btrfs_inode_item *iitem;
>> + const u32 priv = 0700;
>> + u64 ino;
>> + u32 imode = S_IFREG;
>> + int ret = 0;
>> +
>> + btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
>> + ino = key.objectid;
>> + iitem = btrfs_item_ptr(path->nodes[0], path->slots[0],
>> + struct btrfs_inode_item);
>> +
>> + /*
>> + * Both CHR and BLK uses rdev, no way to distinguish them, so fall back
>> + * to BLK.
>> + */
>> + if (btrfs_inode_rdev(path->nodes[0], iitem) != 0) {
>> + imode = S_IFBLK;
>> + goto out;
>> + }
>> +
>> + /* root inode */
>> + if (ino == BTRFS_FIRST_FREE_OBJECTID) {
>> + imode = S_IFDIR;
>> + goto out;
>> + }
>> +
>> + while (1) {
>> + ret = btrfs_next_item(root, path);
>> + if (ret > 0) {
>> + /* No determining result found, falls back to REG */
>> + ret = 0;
>> + goto out;
>> + }
>> + if (ret < 0)
>> + goto out;
>> + btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
>> + if (key.objectid != ino)
>> + goto out;
>> +
>> + /*
>> + * We ignore some types to make life easier:
>> + * - INODE_REF
>> + * We need to do a full tree search, which can fail for
>> + * corrupted fs, but not worthy compared to other easier
>> + * to determine types.
>> + * - XATTR
>> + * Both REG and DIR can have xattr, so not useful
>> + */
>> + switch (key.type) {
>> + case BTRFS_DIR_ITEM_KEY:
>> + case BTRFS_DIR_INDEX_KEY:
>> + imode = S_IFDIR;
>> + goto out;
>> + case BTRFS_EXTENT_DATA_KEY:
>> + /*
>> + * Both REG and LINK could have EXTENT_DATA.
>> + * We just fall back to REG as user can inspect the
>> + * content.
>> + */
>> + imode = S_IFREG;
>> + goto out;
>> + }
>> + }
>> +
>> +out:
>> + /* Set default value even when something wrong happened */
>> + *imode_ret = (imode | priv);
>> + return ret;
>> +}
>> +
>> /*
>> * Reset the inode mode of the inode specified by @path.
>> *
>> @@ -852,22 +926,18 @@ int repair_imode_common(struct btrfs_root *root,
>> struct btrfs_path *path)
>> u32 imode;
>> int ret;
>>
>> - if (root->root_key.objectid != BTRFS_ROOT_TREE_OBJECTID) {
>> - error(
>> - "repair inode mode outside of root tree is not supported yet");
>> - return -ENOTTY;
>> - }
>> btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
>> ASSERT(key.type == BTRFS_INODE_ITEM_KEY);
>> - if (key.objectid != BTRFS_ROOT_TREE_DIR_OBJECTID &&
>> - !is_fstree(key.objectid)) {
>> - error("unsupported ino %llu", key.objectid);
>> - return -ENOTTY;
>> + if (root->objectid == BTRFS_ROOT_TREE_OBJECTID) {
>> + /* In root tree we only have two possible imode */
>> + if (key.objectid == BTRFS_ROOT_TREE_OBJECTID)
>> + imode = S_IFDIR | 0755;
>> + else
>> + imode = S_IFREG | 0600;
>> + } else {
>> + detect_imode(root, path, &imode);
>> + /* Ignore error returned, just use the default value returned */
>
> Is this safe enough though?
It depends. But I'd say if we failed in detect_imode(), then it doesn't
matter whatever the type we're putting here.
> What if due to an error a directory is
> corrected to be file?
If a failure happens, it means btrfs-progs fails to read the next leaf.
Then it really doesn't make much sense whatever the type is.
But I get your point, indeed we should error out without trying out to
fix the inode.
I'll change this behavior in next version.
Thanks,
Qu
> Let's not forget the context we are oprating here
> - a broken fs so it's possible (if not likely) that any of the search
> functions inside detect_imode could return negative error value. OTOH
> I'd expect the transaction commit to fail if that's the case e.g. faulty
> device.
>
>> }
>> - if (key.objectid == BTRFS_ROOT_TREE_DIR_OBJECTID)
>> - imode = 040755;
>> - else
>> - imode = 0100600;
>>
>> trans = btrfs_start_transaction(root, 1);
>> if (IS_ERR(trans)) {
>>
