[Gluster-devel] RFC/Review: libgfapi object handle based extensions

[Shyamsundar Ranganathan]

----- Original Message ----- 

> From: "Anand Avati" <avati at gluster.org>
> To: "Shyamsundar Ranganathan" <srangana at redhat.com>
> Cc: "Gluster Devel" <gluster-devel at nongnu.org>
> Sent: Monday, September 30, 2013 10:27:56 AM
> Subject: Re: [Gluster-devel] RFC/Review: libgfapi object handle based
> extensions

> On Thu, Sep 26, 2013 at 3:55 AM, Shyamsundar Ranganathan <
> srangana at redhat.com > wrote:

> > ----- Original Message -----
> 
> > > From: "Shyamsundar Ranganathan" < srangana at redhat.com >
> 
> > > To: gluster-devel at nongnu.org
> 
> > > Cc: anands at redhat.com
> 
> > > Sent: Friday, September 13, 2013 1:48:19 PM
> 
> > > Subject: RFC/Review: libgfapi object handle based extensions
> 

> > >
> 
> > > - We do need the APIs to extend themselves to do any ID based operations,
> > > say
> 
> > > creating with a specific UID/GID rather than the running process UID/GID
> 
> > > that can prove detrimental in a multi threaded, multi connection handling
> 
> > > server protocol like the NFS Ganesha implementation
> 
> > >
> 

> > In continuation of the original mail, we need to handle the one item above.
> > Where we need to pass in the UID/GID to be used when performing the
> > operations.
> 

> > Here is a suggestion for review on achieving the same, (for current code
> > implementation of handle APIs look at, http://review.gluster.org/#/c/5936/
> > )
> 

> > 1) Modify the handle based APIs to take in a opctx (operation context,
> > concept borrowed from Ganesha)
> 

> > So, instead of,
> 
> > glfs_h_creat (struct glfs *fs, struct glfs_object *parent, const char
> > *path,
> > int flags, mode_t mode, struct stat *stat)
> 
> > it would be,
> 
> > glfs_h_creat (struct glfs *fs, <struct glfs_optctx *opctx>, struct
> > glfs_object *parent, const char *path, int flags, mode_t mode, struct stat
> > *stat)
> 

> > Where,
> 
> > struct glfs_optctx {
> 
> > uid_t caller_uid;
> 
> > gid_t caller_gid;
> 
> > }
> 

> > Later as needed this operation context can be extended for other needs
> > like,
> > client connection address or ID, supplementary groups, etc.
> 

> > 2) Internal to the glfs APIs (esp. handle based APIs), use this to set
> > thread
> > local variables (UID/GID) that the syncop frame creation can pick up in
> > addition to the current probe of geteuid/egid. (as suggested by Avati)
> 

> > If the basic construct looks fine I will amend my current review with this
> > change in the create API and syncop.h (etc.), and once reviewed extend it
> > to
> > other handle based APIs as appropriate.
> 

> I am somewhat hesitant to expose a structure to be filled by the user, where
> the structure can "grow" over time. Providing APIs like
> glfs_setfsuid()/glfs_setfsgid()/glfs_setgroups(), which internally uses
> thread local variables to communicate the values to syncop_create_frame() is
> probably a cleaner approach.

Agreed, this makes it extensible to the non-handle based APIs as well to leverage the same instead of the effective UID/GID or groups as appropriate.

I was thinking of versioning the structure with the bottom half extensible, but that is a bit of an overhead in terms of the problem of it changing over time.

With multiple APIs to set various thread specific variables, we may have an small explosion of the thread variables, and multiple calls to achieve the three settings, but it is extensible and will help current path based implementation as well so should be fine from my perspective.

Shyam

> Avati