On Sat, Jan 18, 2020 at 03:50:30PM -0700, Allison Collins wrote:
> Delayed operations cannot return error codes. So we must check for
> these conditions first before starting set or remove operations
Answering my own question from earlier -- I see here you actually /are/
checking the attr existence w.r.t. ATTR_{CREATE,REPLACE} right after we
allocate a transaction and ILOCK the inode, so
Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
Though I am wondering if you could discard the predicates from the
second patch in favor of doing a normal lookup of the attr with a zero
valuelen to determine if there's already an attribute?
--D
> Signed-off-by: Allison Collins <allison.henderson@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_attr.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index a2673fe..e9d22c1 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -457,6 +457,14 @@ xfs_attr_set(
> goto out_trans_cancel;
>
> xfs_trans_ijoin(args.trans, dp, 0);
> +
> + error = xfs_has_attr(&args);
> + if (error == -EEXIST && (name->type & ATTR_CREATE))
> + goto out_trans_cancel;
> +
> + if (error == -ENOATTR && (name->type & ATTR_REPLACE))
> + goto out_trans_cancel;
> +
> error = xfs_attr_set_args(&args);
> if (error)
> goto out_trans_cancel;
> @@ -545,6 +553,10 @@ xfs_attr_remove(
> */
> xfs_trans_ijoin(args.trans, dp, 0);
>
> + error = xfs_has_attr(&args);
> + if (error != -EEXIST)
> + goto out;
> +
> error = xfs_attr_remove_args(&args);
> if (error)
> goto out;
> --
> 2.7.4
>
