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

[Murali Vilayannur]

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