Re: setfacl Recursive Functionality

Attached is a new patch using the acl(3) API for removing invalid
inheritance. Unless there are other suggestions, I'll likely submit this
upstream. I didn't implement the -P flag since that's the default behavior.

Thanks,

Shawn

On Wed, Feb 9, 2011 at 2:10 PM, Shawn Webb <lattera_at_gmail.com> wrote:

> Included in the attached patch is the refactor using fts(3) and with the -L
> and -H options. I'm still looking for feedback and suggestions on how to
> improve the patch. I'll also port these changes over to my getfacl patch.
>
> If anyone's interested in following up-to-date development of the patch,
> the link to it on github is
> https://github.com/lattera/patches/blob/master/freebsd/setfacl_recursive.patch
>
> I'd like to take the time to address why I created the
> remove_invalid_inherit function since I got a private email asking why it
> existed. Other than symbolic links, non-directory entries cannot have
> inheritance set. That function prevents attempting to set inheritance flags
> on non-directory entries when doing a recursive call. That way, you can run
> `setfacl -R -m user:<user>:read_data:file_inherit/dir_inherit:allow
> <directory>` and not run into errors.
>
> Thanks,
>
> Shawn
>
>
> On Tue, Feb 8, 2011 at 7:51 PM, Shawn Webb <lattera_at_gmail.com> wrote:
>
>> On Tue, Feb 8, 2011 at 7:35 PM, Tim Kientzle <tim_at_kientzle.com> wrote:
>>
>>> On Feb 8, 2011, at 9:58 AM, Shawn Webb wrote:
>>> > I've just finished a patch to add recursive functionality to setfacl.
>>> Before
>>> > I officially submit it, I'd like a few suggestions on how to improve
>>> the
>>> > patch.
>>> >
>>> > The part I'm worried about involves the #define directive at top. I'm
>>> not
>>> > sure what ramifications using that define might have. I needed it for
>>> my
>>> > remove_invalid_inherit() function to work.
>>>
>>> You should certainly not need
>>>   #define _ACL_PRIVATE
>>> for any user-space utilities.  What exactly is the
>>> problem without that?
>>>
>>> Your approach to directory walking here
>>> is a little simplistic.  In particular, you're storing
>>> every filename for the entire tree in memory,
>>> which is a problem for large filesystems.
>>>
>>> It would be much better to refactor the code so that
>>> the actual ACL update was in a function and then
>>> recurse_directory should call that function for
>>> each filename as it visited it.  That will reduce
>>> the memory requirements significantly.
>>>
>>> You should also take a look at fts(3).  In particular,
>>> you'll want to implement the BSD-standard
>>> -L/-P/-H options, and fts(3) makes that much easier.
>>> (-L always follows symlinks, -P never follows symlinks,
>>> -H follows symlinks on the command line).
>>>
>>> Tim
>>>
>>>
>> Great suggestions. I'll definitely look at implementing that
>> functionality.
>>
>> As a side note, it looks like my setfacl patch segfaults on
>> freebsd-current r218075 with the zpool v28 patchset applied. I wrote it on
>> freebsd 8.2-RC3 with zpool v15.
>>
>> Thanks,
>>
>> Shawn
>>
>
>