[Gluster-devel] Mark all the xlator fops 'static '

[Rajesh Joseph]

----- Original Message -----
> From: "Niels de Vos" <ndevos at redhat.com>
> To: "Soumya Koduri" <skoduri at redhat.com>
> Cc: "Gluster Devel" <gluster-devel at gluster.org>
> Sent: Friday, July 31, 2015 2:46:52 AM
> Subject: Re: [Gluster-devel] Mark all the xlator fops 'static '
> 
> On Thu, Jul 30, 2015 at 08:27:15PM +0530, Soumya Koduri wrote:
> > Hi,
> > 
> > With the applications using and loading different libraries, the function
> > symbols with the same name may get resolved incorrectly depending on the
> > order in which those libraries get dynamically loaded.
> > 
> > Recently we have seen an issue with 'snapview-client' xlator lookup fop -
> > 'svc_lookup' which matched with one of the routines provided by libntirpc,
> > used by NFS-Ganesha. More details are in [1], [2].
> 
> Indeed, the problem seems to be caused in an execution flow like this:
> 
> 1. nfs-ganesha main binary starts
> 2. the dynamic linker loads libntirpc (and others)
> 3. the dynamic linker retrieves symbols from the libntirpc (and others)
> 4. 'svc_lookup' is amoung the symbols added to a lookup table (or such)
> 5. during execution, ganesha loads plugins with dlopen()
> 6. the fsalgluster.so plugin is linked against libgfapi and gfapi gets
>    loaded
> 7. libgfapi retrieves the .vol file and loads the xlators, including
>    snapview-client
> 8. snapview-client provices a 'svc_lookup' symbol, same name as
>    libntirpc provides, complete different functionality
> 9. the dynamic linker already has a symbol called 'svc_lookup' and does
>    not add the later loaded version (or it gets added down in a list)
> 10. when snapview-client calls 'svc_lookup', the first symbol with that
>     name is provided by the dynamic-linker
> 11. snapview-client calls the wrong 'svc_lookup' function
> 
> The above (and below) might not be technically correct in relation to
> the dynamic linker, it is more a description of my understanding of the
> issue and how I imagine things work.
> 
> Adding the 'static' keyword to functions keeps their scope inside the
> compiled object code (.o files) and references are not dynamically
> loaded through the dynamic linker. Because the dynamic linker is not
> involved in the 'static' functions, the problem of conflicting symbol
> names does not occur anymore.
> 
> Functions that are called from other components (library-like functions)
> can not have the 'static' keyword. If there is a conflict between
> function names provided in an OS library and a Gluster component, we
> should prepend the function name with "gf_" like we have done in the
> uuid case mentioned below.
> 

I think as a general guideline all those functions which does not require 
external linkage should be made static. And those with external linkage 
should have some unique prefix. e.g. as Niels mentioned "gf_".

Regards,
Rajesh

> Cheers,
> Niels
> 
> > 
> > Similar issue with respect to uuid* routines was already reported in [3].
> > 
> > To avoid such issues, it may be better and a cleaner way to mark all the
> > xlator fops static, like in [4].
> > 
> > We have raised bug1248669 to track all the required changes. Request the
> > component maintainers to take a look and change the respective xlator fop
> > definitions.
> > 
> > Thanks,
> > Soumya
> > 
> > [1] - https://bugzilla.redhat.com/show_bug.cgi?id=1245636#c6
> > [2] - https://bugzilla.redhat.com/show_bug.cgi?id=1248669
> > [3] - http://article.gmane.org/gmane.comp.file-systems.gluster.devel/10074
> > [4] - http://review.gluster.org/#/c/11805
> _______________________________________________
> Gluster-devel mailing list
> Gluster-devel at gluster.org
> http://www.gluster.org/mailman/listinfo/gluster-devel
>