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

[Sam Lang]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: eattr-callouts.patch
Type: application/octet-stream
Size: 20267 bytes
Desc: not available
Url : http://www.beowulf-underground.org/pipermail/pvfs2-developers/attachments/20070321/b68ddb78/eattr-callouts.obj
-------------- next part --------------

>
>>> 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
>
>