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.
This needs an explicit satement that it will only work for non-empty files and directories
>
> - 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 <wqu@xxxxxxxx>
> ---
> 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)
> +{
> + struct btrfs_key key;
> + struct btrfs_inode_item iitem;
> + const u32 priv = 0700;
> + bool found = false;
> + u64 ino;
> + u32 imode;
> + int ret = 0;
> +
> + btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
> + ino = key.objectid;
> + read_extent_buffer(path->nodes[0], &iitem,
> + btrfs_item_ptr_offset(path->nodes[0], path->slots[0]),
> + sizeof(iitem));
> + /* root inode */
> + if (ino == BTRFS_FIRST_FREE_OBJECTID) {
> + imode = S_IFDIR;
> + found = true;
> + goto out;
> + }
> +
> + while (1) {
> + struct btrfs_inode_ref *iref;
> + struct extent_buffer *leaf;
> + unsigned long cur;
> + unsigned long end;
> + char namebuf[BTRFS_NAME_LEN] = {0};
> + u64 index;
> + u32 namelen;
> + int slot;
> +
> + ret = btrfs_next_item(root, path);
> + if (ret > 0) {
> + /* falls back to rdev check */
> + ret = 0;
> + goto out;
> + }
In my testing if an inode is the last one in the leaf and it doesn't have
an INODE_REF item then it won't be repaired. But e.g. it can have perfectly
valid DIR_ITEM/DIR_INDEX entries which describe this inode as being a file. E.g.
item 2 key (256 DIR_ITEM 388586943) itemoff 16076 itemsize 35
location key (260 INODE_ITEM 0) type FILE
transid 7 data_len 0 name_len 5
name: file3
.....
item 15 key (260 INODE_ITEM 0) itemoff 15184 itemsize 160
generation 7 transid 7 size 0 nbytes 0
block group 0 mode 26772225102 links 1 uid 0 gid 0 rdev 0
sequence 1 flags 0x0(none)
atime 1568127261.284993602 (2019-09-10 14:54:21)
ctime 1568127261.284993602 (2019-09-10 14:54:21)
mtime 1568127261.284993602 (2019-09-10 14:54:21)
otime 1568127261.284993602 (2019-09-10 14:54:21)
I have intentionally deleted INODE_REF too see what's happening. Is this intended?
> + if (ret < 0)
> + goto out;
> + leaf = path->nodes[0];
> + slot = path->slots[0];
> + btrfs_item_key_to_cpu(leaf, &key, slot);
> + if (key.objectid != ino)
> + goto out;
> +
> + /*
> + * We ignore some types to make life easier:
> + * - XATTR
> + * Both REG and DIR can have xattr, so not useful
> + */
> + switch (key.type) {
> + case BTRFS_INODE_REF_KEY:
> + /* The most accurate way to determine filetype */
> + cur = btrfs_item_ptr_offset(leaf, slot);
> + end = cur + btrfs_item_size_nr(leaf, slot);
> + while (cur < end) {
> + iref = (struct btrfs_inode_ref *)cur;
> + namelen = min_t(u32, end - cur - sizeof(&iref),
> + btrfs_inode_ref_name_len(leaf, iref));
> + index = btrfs_inode_ref_index(leaf, iref);
> + read_extent_buffer(leaf, namebuf,
> + (unsigned long)(iref + 1), namelen);
> + ret = find_file_type(root, ino, key.offset,
> + index, namebuf, namelen,
> + &imode);
> + if (ret == 0) {
> + found = true;
> + goto out;
> + }
> + cur += sizeof(*iref) + namelen;
> + }
> + break;
> + case BTRFS_DIR_ITEM_KEY:
> + case BTRFS_DIR_INDEX_KEY:
> + imode = S_IFDIR;
You should set found here. Otherwise this function returns ENOENT in those 2 branches.
BTW in relation to out private conversation I found this while trying to create test
cases which will trigger all branches. The fact it found bugs proves we should strive
for as much testing coverage as possible.
> + 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;
ditto
> + goto out;
> + }
> + }
> +
> +out:
> + /*
> + * Both CHR and BLK uses rdev, no way to distinguish them, so fall back
> + * to BLK. But either way it doesn't really matter, as CHR/BLK on btrfs
> + * should be pretty rare, and no real data will be lost.
> + */
> + if (!found && btrfs_stack_inode_rdev(&iitem) != 0) {
> + imode = S_IFBLK;
> + found = true;
> + }
> +
> + if (found)
> + *imode_ret = (imode | priv);
> + else
> + ret = -ENOENT;
> + return ret;
> +}
> +
> /*
> * Reset the inode mode of the inode specified by @path.
<snip>
