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

[Jiffin Tony Thottan]

On 12/07/16 16:00, Kaleb KEITHLEY wrote:
> On 07/12/2016 03:00 AM, Jiffin Tony Thottan wrote:
>>
>> On 31/07/15 19:29, Kaleb S. KEITHLEY wrote:
>>> On 07/30/2015 05:16 PM, Niels de Vos wrote:
>>>> 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
>>> So far so good. But I would have expected the compiler to have populated
>>> the function pointers in snapview-client's fops table at compile time;
>>> the dynamic loader should not have been needed to resolve
>>> snapview-client's svc_lookup, because it was (should have been) already
>>> resolved at compile time.
>>>
>>> And in fact it is, but, there are semantics for global (.globl) symbols
>>> and run-time linkage that are biting us.
>>>
>>>
>> Hi all,
>>
>> I am hitting a similar type of collision on ganesha 2.4.  In ganesha
>> 2.4, we introduced stackable
>> mdcache at top of every FSAL. The lookup(mdc_lookup) function has
>> similar signature to gluster
>> mdcache lookup fop. In my case ganesha always pick up mdc_lookup from
>> its layer not from
>> gfapi graph. When I disabled md-cache it worked perfectly. As Soumya
>> suggested before do we
>> need to change every xlator fop to static?
>>
> The xlator fops are already effectively static, at least in 3.8 and later.
>
> Starting in 3.8 the xlators are linked with an export map that only
> exposes init(), fini(), fops, cbks, options, notify(), mem_acct_init(),
> reconfigure(), and dumpops. (A few xlators export other symbols too.)
>
> If this is biting us in 3.7 then we need to make the mdcache fops static.
>
> This isn't C++, so all it takes for a symbol name collision is for the
> functions to have the same name, i.e. mdc_lookup() in this case. :-/

Thanks Kaleb  for the information. I was using gluster 3.7 in my set up
--
Jiffin

> --
>
> Kaleb
>