Re: [PATCH] xfs: don't fail when converting shortform attr to long form during ATTR_REPLACE

[Dave Chinner <david@fromorbit.com>]

On Tue, Apr 17, 2018 at 05:31:30PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Kanda Motohiro reported that expanding a tiny xattr into a large xattr
> fails on XFS because we remove the tiny xattr from a shortform fork and
> then try to re-add it after converting the fork to extents format having
> not removed the ATTR_REPLACE flag.  This fails because the attr is no
> longer present, causing a fs shutdown.
> 
> This is derived from the patch in his bug report, but we really
> shouldn't ignore a nonzero retval from the remove call.  Something's
> broken and we should bail out and shut down.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=199119
> Reported-by: kanda.motohiro@gmail.com
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/libxfs/xfs_attr.c |    9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index ce4a34a..2588c73 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -511,7 +511,14 @@ xfs_attr_shortform_addname(xfs_da_args_t *args)
>  		if (args->flags & ATTR_CREATE)
>  			return retval;
>  		retval = xfs_attr_shortform_remove(args);
> -		ASSERT(retval == 0);
> +		if (retval)
> +			return retval;
> +		/*
> +		 * Since we have removed the old attr here,
> +		 * further lookup will fail with ENOATTR.
> +		 * Ignore this was a replace and go on creating new attr.

The last line of that comment doesn't make sense to me. not sure
what it's meant to say...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com