Re: [PATCH v4 15/17] xfs: Check for -ENOATTR or -EEXIST

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Wed, Nov 06, 2019 at 06:27:59PM -0700, Allison Collins wrote:
> Delayed operations cannot return error codes.  So we must check for
> these conditions first before starting set or remove operations
> 
> Signed-off-by: Allison Collins <allison.henderson@oracle.com>
> ---
>  fs/xfs/libxfs/xfs_attr.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index 5dcb19f..626d4a98 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -458,6 +458,27 @@ xfs_attr_set(
>  		goto out_trans_cancel;
>  
>  	xfs_trans_ijoin(args.trans, dp, 0);
> +
> +	error = xfs_has_attr(&args);
> +	if (error == -EEXIST) {
> +		if (name->type & ATTR_CREATE)
> +			goto out_trans_cancel;
> +		else
> +			name->type |= ATTR_REPLACE;
> +	}
> +
> +	if (error == -ENOATTR && (name->type & ATTR_REPLACE))
> +		goto out_trans_cancel;
> +
> +	if (name->type & ATTR_REPLACE) {
> +		name->type &= ~ATTR_REPLACE;
> +		error = xfs_attr_remove_args(&args);
> +		if (error)
> +			goto out_trans_cancel;
> +
> +		name->type |= ATTR_CREATE;

I thought _set_args already handled the remove part of replacing an
attr?  And I thought that it did this with an atomic rename?  Won't this
break the atomicity of attr replacement?

--D

> +	}
> +
>  	error = xfs_attr_set_args(&args);
>  	if (error)
>  		goto out_trans_cancel;
> @@ -543,6 +564,10 @@ xfs_attr_remove(
>  	 */
>  	xfs_trans_ijoin(args.trans, dp, 0);
>  
> +	error = xfs_has_attr(&args);
> +	if (error == -ENOATTR)
> +		goto out;
> +
>  	error = xfs_attr_remove_args(&args);
>  	if (error)
>  		goto out;
> -- 
> 2.7.4
>