From: Nikolay Borisov <nborisov@suse.com>
To: Qu Wenruo <wqu@suse.com>, linux-btrfs@vger.kernel.org
Subject: Re: [PATCH v2 3/6] btrfs-progs: check/common: Make repair_imode_common() to handle inodes in subvolume trees
Date: Mon, 9 Sep 2019 17:17:31 +0300 [thread overview]
Message-ID: <43ed2a81-aa0e-22ff-a3db-202b5c945d18@suse.com> (raw)
In-Reply-To: <20190905075800.1633-4-wqu@suse.com>
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 <wqu@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?
> +{
> + 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.
> + 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;
> + }
> + 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;
> + 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:
> + /*
> + * 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.
> *
> @@ -951,22 +1058,19 @@ 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 {
> + ret = detect_imode(root, path, &imode);
> + if (ret < 0)
> + return ret;
> }
> - if (key.objectid == BTRFS_ROOT_TREE_DIR_OBJECTID)
> - imode = 040755;
> - else
> - imode = 0100600;
>
> trans = btrfs_start_transaction(root, 1);
> if (IS_ERR(trans)) {
>
next prev parent reply other threads:[~2019-09-09 14:17 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-05 7:57 [PATCH v2 0/6] btrfs-progs: check: Repair invalid inode mode in Qu Wenruo
2019-09-05 7:57 ` [PATCH v2 1/6] btrfs-progs: check: Export btrfs_type_to_imode Qu Wenruo
2019-09-05 7:57 ` [PATCH v2 2/6] btrfs-progs: check/common: Introduce a function to find imode using INODE_REF Qu Wenruo
2019-09-09 13:25 ` Nikolay Borisov
2019-09-09 14:24 ` Qu Wenruo
2019-09-09 14:34 ` Nikolay Borisov
2019-09-09 13:42 ` Nikolay Borisov
2019-09-09 14:26 ` Qu Wenruo
2019-09-09 14:35 ` Nikolay Borisov
2019-09-05 7:57 ` [PATCH v2 3/6] btrfs-progs: check/common: Make repair_imode_common() to handle inodes in subvolume trees Qu Wenruo
2019-09-09 14:17 ` Nikolay Borisov [this message]
2019-09-09 14:27 ` Qu Wenruo
2019-09-10 4:27 ` Su Yue
2019-09-10 16:14 ` Nikolay Borisov
2019-09-11 0:39 ` Qu Wenruo
2019-09-11 12:27 ` Nikolay Borisov
2019-09-11 12:44 ` Qu Wenruo
2019-09-05 7:57 ` [PATCH v2 4/6] btrfs-progs: check/lowmem: Repair bad imode early Qu Wenruo
2019-09-09 14:55 ` Nikolay Borisov
2019-09-10 2:35 ` Qu Wenruo
2019-09-05 7:57 ` [PATCH v2 5/6] btrfs-progs: check/original: Fix inode mode in subvolume trees Qu Wenruo
2019-09-05 7:58 ` [PATCH v2 6/6] btrfs-progs: tests/fsck: Add new images for inode mode repair functionality Qu Wenruo
2019-09-09 15:37 ` Nikolay Borisov
2019-09-09 23:22 ` Qu Wenruo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=43ed2a81-aa0e-22ff-a3db-202b5c945d18@suse.com \
--to=nborisov@suse.com \
--cc=linux-btrfs@vger.kernel.org \
--cc=wqu@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).