[Top] [All Lists]

Re: [PATCH] xfs: return ENOSPC when trying to set more ACLs than XFS_ACL

To: Jeff Liu <jeff.liu@xxxxxxxxxx>, "xfs@xxxxxxxxxxx" <xfs@xxxxxxxxxxx>
Subject: Re: [PATCH] xfs: return ENOSPC when trying to set more ACLs than XFS_ACL_MAX_ENTRIES
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Sun, 24 Nov 2013 21:34:47 -0600
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <52921D12.7040104@xxxxxxxxxx>
References: <52921D12.7040104@xxxxxxxxxx>
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:24.0) Gecko/20100101 Thunderbird/24.1.1
On 11/24/13, 9:36 AM, Jeff Liu wrote:
> From: Jie Liu <jeff.liu@xxxxxxxxxx>
> We currently return EINVAL when trying to set more ACL entries than
> XFS_ACL_MAX_ENTRIES(), but it would be a bit more meaningful to return
> ENOSPC in this situation, because the later is used to indicate there
> is no more space to store new ACLs IMHO.

I'm not quite convinced that it's better; the user will get an
error string of "no space left on device" which is misleading too,
and I'd argue that it's no better than "invalid argument."

To me, I think it's not worth changing, but others may disagree.

(I guess looking at ext4, it uses ENOSPC for some similar constraints,
so maybe three is precedent for this)

The setxattr(2) man page says:

> If there is insufficient space remaining to store the extended
> attribute, errno is set to either ENOSPC

but it doesn't say space in _what_ ...


> Signed-off-by: Jie Liu <jeff.liu@xxxxxxxxxx>
> ---
>  fs/xfs/xfs_acl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> diff --git a/fs/xfs/xfs_acl.c b/fs/xfs/xfs_acl.c
> index 370eb3e..4e54a4d 100644
> --- a/fs/xfs/xfs_acl.c
> +++ b/fs/xfs/xfs_acl.c
> @@ -397,7 +397,7 @@ xfs_xattr_acl_set(struct dentry *dentry, const char *name,
>       if (error)
>               goto out_release;
> -     error = -EINVAL;
> +     error = -ENOSPC;
>       if (acl->a_count > XFS_ACL_MAX_ENTRIES(XFS_M(inode->i_sb)))
>               goto out_release;

<Prev in Thread] Current Thread [Next in Thread>