Re: [PATCH v17 05/11] xfs: Separate xfs_attr_node_addname and xfs_attr_node_addname_clear_incomplete

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 4/20/21 5:14 AM, Chandan Babu R wrote:
On 20 Apr 2021 at 00:02, Allison Henderson wrote:
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;

         "retval = 0;" should suffice.
Ok, will that add in.  Thanks!
Allison


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.

Thanks for the catch!
Allison




[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux