[Pvfs2-developers] patches: permission/acl bug fixes

[Sam Lang]

On Apr 5, 2007, at 10:55 AM, Murali Vilayannur wrote:

> Hi Sam,
> Never mind my previous email. :) Did not see the thread carefully..
> Patch looks great! I see that it has been checked in already.. Sorry
> for the late feedback :(

Its cool.  Any feedback is great!

-sam

> Murali
>
> On 3/21/07, Sam Lang <slang at mcs.anl.gov> wrote:
>>
>> On Mar 21, 2007, at 8:35 AM, Phil Carns wrote:
>>
>> > Sam Lang wrote:
>> >
>> >>> acl-check-assert.patch:
>> >>> ------------------------
>> >> It seems like it should be possible to do that format checking of
>> >> the  acl when the system.posix_acl_access extended attribute is
>> >> set.  Does  it make sense to add a callouts framework to set-eattr
>> >> to do format  checking for specific xattrs?
>> >
>> > I'm not sure- maybe?  I don't actually know how the file system
>> > that triggered this problem got bad acls in the first place.
>>
>> I thought it would be easy to add some framework code to do the
>> callouts I was thinking of, but it ended up being a little more
>> complicated, and I started feeling like I was painting over the shed,
>> but not even with a different color.  But at that point though I was
>> already too far in to go back.  So I've attached a patch, which I'm
>> not sure was worth it, but might be in the long run if we get more
>> extended attributes in pvfs and want to do proper checking of their
>> keys and values.
>>
>> As for the bad acls showing up, it probably didn't happen on the set-
>> eattr anyway, so this patch isn't going to help that.  Most likely
>> there's a bug somewhere else in the code that we haven't found yet.
>>
>> -sam
>>
>>
>>
>> >
>> >>> root-squash.patch:
>> >>> ------------------
>> >> For root-squash: I've wondered why the dspace entries for
>> >> datafile  handles don't carry the ownership and permissions, and
>> >> it seems like  its only because we don't pass the attributes along
>> >> with the create  call.  The setattr does set the attrs on the
>> >> metadata handle, but its  primary purpose is to set the datafile
>> >> handles list in the metadata.   We already have the file's
>> >> attributes -- they get passed in with the  PVFS_sys_create call.
>> >> Could we possibly add an object attr field to  the create so that
>> >> the attr gets set on dspace entry for datafile  handles as well?
>> >> Once that's done, the credentials passed in the  write request
>> >> could be checked against the attributes.  I think that  would
>> >> allow us to get the proper semantics for squashing.
>> >> The drawback I see in doing this would be that a chmod/chown/
>> >> chgrp  would require doing setattrs to all the IO servers as well
>> >> as the  metadata server.  It seems like those operations are
>> >> infrequent  enough that doing so wouldn't be a big deal.  Also,
>> >> the create state  machine on the server would have to do a
>> >> trove_dspace_setattr after  the trove_dspace_create completed.  We
>> >> could avoid being 2x slower by  not syncing on the create though.
>> >
>> > I think the biggest challenge of putting attrs on the dfiles would
>> > be keeping them in sync, for example if a client died halfway
>> > through a chown and only modified a subset of the dfiles.
>> >
>> > Thanks for applying the patches!
>> >
>> > -Phil
>> >
>> >
>>
>>
>> _______________________________________________
>> Pvfs2-developers mailing list
>> Pvfs2-developers at beowulf-underground.org
>> http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers
>>
>>
>>
>