On 4/18/21 10:15 PM, Chandan Babu R wrote:
On 16 Apr 2021 at 14:50, Allison Henderson wrote:
This patch separate xfs_attr_node_addname into two functions. This will
help to make it easier to hoist parts of xfs_attr_node_addname that need
state management
Signed-off-by: Allison Henderson <allison.henderson@xxxxxxxxxx>
Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
---
fs/xfs/libxfs/xfs_attr.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index fff1c6f..d9dfc8d2 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -54,6 +54,7 @@ STATIC int xfs_attr_leaf_hasname(struct xfs_da_args *args, struct xfs_buf **bp);
STATIC int xfs_attr_node_get(xfs_da_args_t *args);
STATIC int xfs_attr_node_addname(xfs_da_args_t *args);
STATIC int xfs_attr_node_removename(xfs_da_args_t *args);
+STATIC int xfs_attr_node_addname_clear_incomplete(struct xfs_da_args *args);
STATIC int xfs_attr_node_hasname(xfs_da_args_t *args,
struct xfs_da_state **state);
STATIC int xfs_attr_fillstate(xfs_da_state_t *state);
@@ -1062,6 +1063,25 @@ xfs_attr_node_addname(
return error;
}
+ error = xfs_attr_node_addname_clear_incomplete(args);
Ok, so i think what we need here is an extra few lines below. That's
similar to the original exit handling
if (error)
goto out;
retval = error = 0;
Looks good?
+out:
+ if (state)
+ xfs_da_state_free(state);
+ if (error)
+ return error;
+ return retval;
I think the above code incorrectly returns -ENOSPC when the user is performing
an xattr rename operation and the call to xfs_attr3_leaf_add() resulted in
returning -ENOSPC,
1. xfs_attr3_leaf_add() returns -ENOSPC.
2. xfs_da3_split() allocates a new leaf and inserts the new xattr into it.
3. If the user was performing a rename operation (i.e. XFS_DA_OP_RENAME is
set), we flip the "incomplete" flag.
4. Remove the old xattr's remote blocks (if any).
5. Remove old xattr's name.
6. If "error" has zero as its value, we return the value of "retval". At this
point in execution, "retval" would have -ENOSPC as its value.
--
chandan
Thanks for the catch!
Allison
