Liu Bo
2012-Aug-16 06:10 UTC
[PATCH v2 1/2] Btrfs: fix a bug in parsing return value in logical resolve
In logical resolve, we parse extent_from_logical()''s
''ret'' as a kind of flag.
It is possible to lose our errors because
(-EXXXX & BTRFS_EXTENT_FLAG_TREE_BLOCK) is true.
I''m not sure if it is on purpose, it just looks too hacky if it is.
I''d rather use a real flag and a ''ret'' to catch
errors.
Acked-by: Jan Schmidt <list.btrfs@jan-o-sch.net>
Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
---
fs/btrfs/backref.c | 24 ++++++++++++++++--------
fs/btrfs/backref.h | 3 ++-
fs/btrfs/ioctl.c | 6 ++++--
fs/btrfs/scrub.c | 14 ++++++++------
fs/btrfs/send.c | 7 ++++---
5 files changed, 34 insertions(+), 20 deletions(-)
diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
index a256f3b..08d5385 100644
--- a/fs/btrfs/backref.c
+++ b/fs/btrfs/backref.c
@@ -1193,7 +1193,8 @@ char *btrfs_iref_to_path(struct btrfs_root *fs_root,
struct btrfs_path *path,
* tree blocks and <0 on error.
*/
int extent_from_logical(struct btrfs_fs_info *fs_info, u64 logical,
- struct btrfs_path *path, struct btrfs_key *found_key)
+ struct btrfs_path *path, struct btrfs_key *found_key,
+ u64 *flags_ret)
{
int ret;
u64 flags;
@@ -1237,10 +1238,17 @@ int extent_from_logical(struct btrfs_fs_info *fs_info,
u64 logical,
(unsigned long long)found_key->objectid,
(unsigned long long)found_key->offset,
(unsigned long long)flags, item_size);
- if (flags & BTRFS_EXTENT_FLAG_TREE_BLOCK)
- return BTRFS_EXTENT_FLAG_TREE_BLOCK;
- if (flags & BTRFS_EXTENT_FLAG_DATA)
- return BTRFS_EXTENT_FLAG_DATA;
+
+ WARN_ON(!flags_ret);
+ if (flags_ret) {
+ if (flags & BTRFS_EXTENT_FLAG_TREE_BLOCK)
+ *flags_ret = BTRFS_EXTENT_FLAG_TREE_BLOCK;
+ else if (flags & BTRFS_EXTENT_FLAG_DATA)
+ *flags_ret = BTRFS_EXTENT_FLAG_DATA;
+ else
+ BUG_ON(1);
+ return 0;
+ }
return -EIO;
}
@@ -1432,13 +1440,13 @@ int iterate_inodes_from_logical(u64 logical, struct
btrfs_fs_info *fs_info,
{
int ret;
u64 extent_item_pos;
+ u64 flags = 0;
struct btrfs_key found_key;
int search_commit_root = path->search_commit_root;
- ret = extent_from_logical(fs_info, logical, path,
- &found_key);
+ ret = extent_from_logical(fs_info, logical, path, &found_key, &flags);
btrfs_release_path(path);
- if (ret & BTRFS_EXTENT_FLAG_TREE_BLOCK)
+ if (flags & BTRFS_EXTENT_FLAG_TREE_BLOCK)
ret = -EINVAL;
if (ret < 0)
return ret;
diff --git a/fs/btrfs/backref.h b/fs/btrfs/backref.h
index 032f4dc..4fda5d8 100644
--- a/fs/btrfs/backref.h
+++ b/fs/btrfs/backref.h
@@ -40,7 +40,8 @@ int inode_item_info(u64 inum, u64 ioff, struct btrfs_root
*fs_root,
struct btrfs_path *path);
int extent_from_logical(struct btrfs_fs_info *fs_info, u64 logical,
- struct btrfs_path *path, struct btrfs_key *found_key);
+ struct btrfs_path *path, struct btrfs_key *found_key,
+ u64 *flags);
int tree_backref_for_extent(unsigned long *ptr, struct extent_buffer *eb,
struct btrfs_extent_item *ei, u32 item_size,
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 7bb7556..405b279 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -3211,6 +3211,7 @@ static long btrfs_ioctl_logical_to_ino(struct btrfs_root
*root,
int ret = 0;
int size;
u64 extent_item_pos;
+ u64 flags = 0;
struct btrfs_ioctl_logical_ino_args *loi;
struct btrfs_data_container *inodes = NULL;
struct btrfs_path *path = NULL;
@@ -3240,10 +3241,11 @@ static long btrfs_ioctl_logical_to_ino(struct btrfs_root
*root,
goto out;
}
- ret = extent_from_logical(root->fs_info, loi->logical, path, &key);
+ ret = extent_from_logical(root->fs_info, loi->logical, path, &key,
+ &flags);
btrfs_release_path(path);
- if (ret & BTRFS_EXTENT_FLAG_TREE_BLOCK)
+ if (flags & BTRFS_EXTENT_FLAG_TREE_BLOCK)
ret = -ENOENT;
if (ret < 0)
goto out;
diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index b223620..5def223 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -352,13 +352,14 @@ static void scrub_print_warning(const char *errstr, struct
scrub_block *sblock)
struct extent_buffer *eb;
struct btrfs_extent_item *ei;
struct scrub_warning swarn;
- u32 item_size;
- int ret;
+ unsigned long ptr = 0;
+ u64 extent_item_pos;
+ u64 flags = 0;
u64 ref_root;
+ u32 item_size;
u8 ref_level;
- unsigned long ptr = 0;
const int bufsize = 4096;
- u64 extent_item_pos;
+ int ret;
path = btrfs_alloc_path();
@@ -375,7 +376,8 @@ static void scrub_print_warning(const char *errstr, struct
scrub_block *sblock)
if (!path || !swarn.scratch_buf || !swarn.msg_buf)
goto out;
- ret = extent_from_logical(fs_info, swarn.logical, path, &found_key);
+ ret = extent_from_logical(fs_info, swarn.logical, path, &found_key,
+ &flags);
if (ret < 0)
goto out;
@@ -387,7 +389,7 @@ static void scrub_print_warning(const char *errstr, struct
scrub_block *sblock)
item_size = btrfs_item_size_nr(eb, path->slots[0]);
btrfs_release_path(path);
- if (ret & BTRFS_EXTENT_FLAG_TREE_BLOCK) {
+ if (flags & BTRFS_EXTENT_FLAG_TREE_BLOCK) {
do {
ret = tree_backref_for_extent(&ptr, eb, ei, item_size,
&ref_root, &ref_level);
diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index fb5ffe9..7186ae8 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -1143,6 +1143,7 @@ static int find_extent_clone(struct send_ctx *sctx,
u64 logical;
u64 num_bytes;
u64 extent_item_pos;
+ u64 flags = 0;
struct btrfs_file_extent_item *fi;
struct extent_buffer *eb = path->nodes[0];
struct backref_ctx backref_ctx;
@@ -1181,13 +1182,13 @@ static int find_extent_clone(struct send_ctx *sctx,
}
logical += btrfs_file_extent_offset(eb, fi);
- ret = extent_from_logical(sctx->send_root->fs_info,
- logical, tmp_path, &found_key);
+ ret = extent_from_logical(sctx->send_root->fs_info, logical, tmp_path,
+ &found_key, &flags);
btrfs_release_path(tmp_path);
if (ret < 0)
goto out;
- if (ret & BTRFS_EXTENT_FLAG_TREE_BLOCK) {
+ if (flags & BTRFS_EXTENT_FLAG_TREE_BLOCK) {
ret = -EIO;
goto out;
}
--
1.7.7.6
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
We already have a helper, iterate_inodes_from_logical(), for logical resolve,
so just use it.
Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
---
v1->v2: patch -EINVAL to -ENOENT to keep the behaviour, thanks to Jan.
fs/btrfs/ioctl.c | 19 +++----------------
1 files changed, 3 insertions(+), 16 deletions(-)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 405b279..be51870 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -3210,12 +3210,9 @@ static long btrfs_ioctl_logical_to_ino(struct btrfs_root
*root,
{
int ret = 0;
int size;
- u64 extent_item_pos;
- u64 flags = 0;
struct btrfs_ioctl_logical_ino_args *loi;
struct btrfs_data_container *inodes = NULL;
struct btrfs_path *path = NULL;
- struct btrfs_key key;
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
@@ -3241,23 +3238,13 @@ static long btrfs_ioctl_logical_to_ino(struct btrfs_root
*root,
goto out;
}
- ret = extent_from_logical(root->fs_info, loi->logical, path, &key,
- &flags);
- btrfs_release_path(path);
-
- if (flags & BTRFS_EXTENT_FLAG_TREE_BLOCK)
+ ret = iterate_inodes_from_logical(loi->logical, root->fs_info, path,
+ build_ino_list, inodes);
+ if (ret == -EINVAL)
ret = -ENOENT;
if (ret < 0)
goto out;
- extent_item_pos = loi->logical - key.objectid;
- ret = iterate_extent_inodes(root->fs_info, key.objectid,
- extent_item_pos, 0, build_ino_list,
- inodes);
-
- if (ret < 0)
- goto out;
-
ret = copy_to_user((void *)(unsigned long)loi->inodes,
(void *)(unsigned long)inodes, size);
if (ret)
--
1.7.7.6
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html