On 26.09.19 г. 12:31 ч., Filipe Manana wrote:
> On Thu, Sep 26, 2019 at 10:27 AM Nikolay Borisov <nbori...@suse.com> 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 <nbori...@suse.com>
>> ---
>>
>> 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.
Yes but this is for a follow up patch. The current patch does not make
the code any more broken than it currently is.
>
> Thanks.
>
>>
>> return false;
>> --
>> 2.17.1
>>
>
>
