On Thu, Sep 26, 2019 at 10:27 AM Nikolay Borisov <nborisov@xxxxxxxx> wrote:
>
> This function can return a negative error value if btrfs_search_slot
> errors for whatever reason or if btrfs_alloc_path runs out of memory.
> This is currently problemattic because backref_in_log is treated by its
> callers as if it returns boolean.
>
> Fix this by adding proper error handling in callers. That also enables
> the function to return the direct error code from btrfs_search_slot.
>
> Signed-off-by: Nikolay Borisov <nborisov@xxxxxxxx>
> ---
>
> v2:
> * Return 0 from backref_in_log in case btrfs_search_slot returns 1 (meaning it
> didn't find appropriate item in the btree).
>
> fs/btrfs/tree-log.c | 32 +++++++++++++++++++++-----------
> 1 file changed, 21 insertions(+), 11 deletions(-)
>
> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> index 7cac09a6f007..7332f7a00790 100644
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -952,7 +952,9 @@ static noinline int backref_in_log(struct btrfs_root *log,
> return -ENOMEM;
>
> ret = btrfs_search_slot(NULL, log, key, path, 0, 0);
> - if (ret != 0) {
> + if (ret < 0) {
> + goto out;
> + } else if (ret == 1) {
> ret = 0;
> goto out;
> }
> @@ -1026,10 +1028,13 @@ static inline int __add_inode_ref(struct btrfs_trans_handle *trans,
> (unsigned long)(victim_ref + 1),
> victim_name_len);
>
> - if (!backref_in_log(log_root, &search_key,
> - parent_objectid,
> - victim_name,
> - victim_name_len)) {
> + ret = backref_in_log(log_root, &search_key,
> + parent_objectid, victim_name,
> + victim_name_len);
> + if (ret < 0) {
> + kfree(victim_name);
> + return ret;
> + } else if (!ret) {
> inc_nlink(&inode->vfs_inode);
> btrfs_release_path(path);
>
> @@ -1091,10 +1096,12 @@ static inline int __add_inode_ref(struct btrfs_trans_handle *trans,
> search_key.offset = btrfs_extref_hash(parent_objectid,
> victim_name,
> victim_name_len);
> - ret = 0;
> - if (!backref_in_log(log_root, &search_key,
> - parent_objectid, victim_name,
> - victim_name_len)) {
> + ret = backref_in_log(log_root, &search_key,
> + parent_objectid, victim_name,
> + victim_name_len);
> + if (ret < 0) {
> + return ret;
> + } else if (!ret) {
> ret = -ENOENT;
> victim_parent = read_one_inode(root,
> parent_objectid);
> @@ -1869,16 +1876,19 @@ static bool name_in_log_ref(struct btrfs_root *log_root,
> const u64 dirid, const u64 ino)
> {
> struct btrfs_key search_key;
> + int ret;
>
> search_key.objectid = ino;
> search_key.type = BTRFS_INODE_REF_KEY;
> search_key.offset = dirid;
> - if (backref_in_log(log_root, &search_key, dirid, name, name_len))
> + ret = backref_in_log(log_root, &search_key, dirid, name, name_len);
> + if (ret == 1)
> return true;
>
> search_key.type = BTRFS_INODE_EXTREF_KEY;
> search_key.offset = btrfs_extref_hash(dirid, name, name_len);
> - if (backref_in_log(log_root, &search_key, dirid, name, name_len))
> + ret = backref_in_log(log_root, &search_key, dirid, name, name_len);
> + if (ret == 1)
> return true;
This function also needs to be able to return errors and its caller
check for errors.
Thanks.
>
> return false;
> --
> 2.17.1
>
--
Filipe David Manana,
“Whether you think you can, or you think you can't — you're right.”
