[Lustre-discuss] problem with too many (default) ACLs on a directory

[Andreas Dilger]

On 2010-05-13, at 04:38, Frederik Ferner wrote:
> Andreas Dilger wrote:
>> On 2010-05-12, at 06:15, Frederik Ferner wrote:
>>> we are having problems with ACLs at the moment. As far as we understand
>>> this is what has happened.
>>> 
>>> We have a directory with 33 default ACLs on it in addition to 32 other ACLs.
>>> 
>>> Our problem started when a user created a subdirectory in the directory
>>> with the 33 default ACLs. This worked but the new directory now is
>>> inaccessible. The number of ACLs on the initial directory does not seem
>>> to matter.
>> 
>> For a long time there was a kernel limit of 32 ACLs on a single file.
>> Looking at newer kernel code it seems this limit is not longer
>> present (it just tries to store the maximum xattr size possible to
>> hold the ACL).
> 
> You are correct, the limit is at least higher than 32 ACLs currently. In my tests on a RHEL5 system on an ext3 FS, I managed to create and read 100 ACLs with no problem.

Definitely worth filing a bug for this.

>> I see in the Lustre code that we have some constants still related to this:
>> # define LUSTRE_POSIX_ACL_MAX_ENTRIES   (32)
>> #  define LUSTRE_POSIX_ACL_MAX_SIZE      \
>>                (mds_xattr_acl_size(LUSTRE_POSIX_ACL_MAX_ENTRIES))
> 
>> It does seem that the MDS should prevent storing an xattr that is
>> larger than this size, but it is possible that if you are building
>> the ACL incrementally it misses this limit check.  It may very well
>> be that by creating a default ACL will bypass this limit and then
>> when it is inherited by the new directory it breaks...
> 
> That seems to be what's happening, yes. I can set 33 default ACLs on a directory which then breaks access to newly created files and directories in the directory.
> 
> Note that I can still access all files when mounting a LVM snapshot of the MDT as ldiskfs. I may try to shutdown our test file system MDT later and reduce the number of ACLs on my inaccessible test files. Should I expect any problems from this?

No, this should work OK.

>> The relevant code is:
>> int mds_setxattr_internal(struct ptlrpc_request *req, struct mds_body *body)
>> {
>>        /* currently lustre limit xattr size */
>>        if (body->valid & OBD_MD_FLXATTR &&
>>            !strcmp(xattr_name, XATTR_NAME_ACL_ACCESS)) {
>>                xattrlen = lustre_msg_buflen(req->rq_reqmsg,
>>                                             REQ_REC_OFF + 2);
>>                if (xattrlen > LUSTRE_POSIX_ACL_MAX_SIZE)
>>                        GOTO(out, -ERANGE);
>>        }
>> but it should also check if the xattr_name is XATTR_NAME_ACL_DEFAULT.
>>        if (body->valid & OBD_MD_FLXATTR &&
>>            (!strcmp(xattr_name, XATTR_NAME_ACL_ACCESS) ||
>>             !strcmp(xattr_name, XATTR_NAME_ACL_DEFAULT)) {
> 
> Should we open a bug to track this?

Please do.  If it includes this comment as a patch, it will likely make it into 1.8.4.

>> Having a fixed maximum number of ACLs is important for Lustre, since
>> the RDMA reply buffers have to be allocated before the client knows
>> how many ACLs are stored on the file.
> 
> Ah yes. Would it be possible to increase this number? I guess not easily as I can imaging it might break interoperability between clients which still use the old limit and a MDS with the new limit.

Yes, it won't be trivial.  That said, if we change the client now to allow more ACLs (say, up to 128 or so), we can change the server at some later date to start sending more.  It would be difficult to make it dynamic based on the reply size, because one client may allow the larger size, but it makes the file inaccessible from another client that doesn't handle this larger size.  I think we could only start using the larger size at some major version change where we know older clients will no longer be supported.

Cheers, Andreas
--
Andreas Dilger
Lustre Technical Lead
Oracle Corporation Canada Inc.