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

[Niels de Vos]

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.

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